Merge pull request #3388 from manyfold3d/fix-preview-choice

Choose renderable files for previews if possible

app/components/model_component.html.erb

@@ -4,7 +4,7 @@
       <% if file.is_image? %>
         <%= content_tag :div, nil, class: "card-img-top card-img-top-background", style: "background-image: url(#{model_model_file_path(@model, file, format: file.extension)})" %>
         <%= image_tag model_model_file_path(@model, file, format: file.extension), class: "card-img-top image-preview #{"sensitive" if helpers.needs_hiding?(@model)}", alt: file.name %>
-      <% elsif helpers.renderable?(file.extension) %>
+      <% elsif file.is_renderable? %>
         <div class="card-img-top <%= "sensitive" if helpers.needs_hiding?(@model) %>">
           <%= render partial: "object_preview", locals: {model: @model, file: file} %>
         </div>

app/helpers/application_helper.rb

@@ -81,15 +81,6 @@ def card(style, title = nil, options = {}, &content)
     end
   end
 
-  def renderable?(format)
-    case format
-    when "stl", "obj", "3mf", "ply", "gltf", "glb"
-      true
-    else
-      false
-    end
-  end
-
   def text_input_row(form, name, options = {})
     content_tag :div, class: "row mb-3 input-group" do
       safe_join [

app/jobs/model_scan_job.rb

@@ -28,7 +28,7 @@ def perform(model_id, include_all_subfolders: false)
     end
     # Set tags and default files
     model.model_files.reload
-    model.preview_file = model.model_files.min_by { it.is_image? ? 0 : 1 } unless model.preview_file
+    model.preview_file = model.model_files.min_by { preview_priority(it) } unless model.preview_file
     if model.tags.empty?
       model.generate_tags_from_directory_name! if SiteSettings.model_tags_tag_model_directory_name
       if SiteSettings.model_tags_auto_tag_new.present?
@@ -41,4 +41,10 @@ def perform(model_id, include_all_subfolders: false)
     model.save!
     # Integrity check will run automatically after save
   end
+
+  def preview_priority(file)
+    return 0 if file.is_image?
+    return 1 if file.is_renderable?
+    100
+  end
 end

app/models/model_file.rb

@@ -84,6 +84,10 @@ def is_3d_model?
     SupportedMimeTypes.model_extensions.include? extension
   end
 
+  def is_renderable?
+    ["stl", "obj", "3mf", "ply", "gltf", "glb"].include? extension
+  end
+
   def mime_type
     Mime::Type.lookup_by_extension(extension)
   end

app/views/application/_object_preview.html.erb

@@ -1,4 +1,4 @@
-<% if renderable?(file.extension) %>
+<% if file.is_renderable? %>
   <div class="position-relative">
     <canvas id="preview-file-<%= file.to_param %>" class="object-preview position-relative" tabindex="0" data-preview
       data-preview-url="<%= model_model_file_path(model, file, file.extension&.to_sym) %>"

app/views/collections/_collection.html.erb

@@ -6,7 +6,7 @@
       <% if file.is_image? %>
         <%= content_tag :div, nil, class: "card-img-top card-img-top-background", style: "background-image: url(#{model_model_file_path(model, file, format: file.extension)})" %>
         <%= image_tag model_model_file_path(model, file, format: file.extension), class: "card-img-top image-preview ", alt: file.name %>
-      <% elsif renderable?(file.extension) %>
+      <% elsif file.is_renderable? %>
         <div class="card-img-top">
           <%= render partial: "object_preview", locals: {model: model, file: file} %>
         </div>

app/views/model_files/show.html.erb

@@ -15,7 +15,7 @@
       <%= video_tag model_model_file_path(@model, @file, format: @file.extension), alt: @file.name, style: "width: 100%", controls: true %>
     <% elsif @file.is_document? %>
       <%= tag.iframe src: model_model_file_path(@model, @file, format: @file.extension), alt: @file.name, style: "width: 100%; aspect-ratio: 0.707" %>
-    <% elsif renderable?(@file.extension) %>
+    <% elsif @file.is_renderable? %>
       <% if @file.presupported_version || @file.unsupported_version %>
         <ul class="nav nav-tabs">
           <li class="nav-item">

app/views/models/_file.html.erb

@@ -3,7 +3,7 @@
     <% if file.is_image? %>
       <%= content_tag :div, nil, class: "card-img-top card-img-top-background", style: "background-image: url(#{model_model_file_path(@model, file, format: file.extension)})" %>
       <%= image_tag model_model_file_path(@model, file, format: file.extension), class: "card-img-top image-preview", alt: file.name %>
-    <% elsif renderable?(file.extension) %>
+    <% elsif file.is_renderable? %>
       <div class="card-img-top">
         <%= render partial: "object_preview", locals: {model: @model, file: file} %>
       </div>

spec/helpers/application_helper_spec.rb

@@ -27,20 +27,6 @@
     end
   end
 
-  describe "#renderable?" do
-    it "returns true for renderable formats" do # rubocop:todo RSpec/MultipleExpectations
-      expect(helper.renderable?("stl")).to be true
-      expect(helper.renderable?("obj")).to be true
-      expect(helper.renderable?("3mf")).to be true
-      expect(helper.renderable?("ply")).to be true
-    end
-
-    it "returns false for non-renderable formats" do # rubocop:todo RSpec/MultipleExpectations
-      expect(helper.renderable?("txt")).to be false
-      expect(helper.renderable?("doc")).to be false
-    end
-  end
-
   describe "#text_input_row" do
     it "returns the correct HTML for the text input row" do # rubocop:todo RSpec/MultipleExpectations
       form = ActionView::Helpers::FormBuilder.new(:test, nil, helper, {})

spec/jobs/model_scan_job_spec.rb

@@ -5,7 +5,7 @@
   context "with a simple model folder" do
     around do |ex|
       MockDirectory.create([
-        "model_one/part_1.obj",
+        "model_one/part_1.lys",
         "model_one/part_2.obj"
       ]) do |path|
         @library_path = path
@@ -21,13 +21,13 @@
 
     it "detects model files" do # rubocop:todo RSpec/MultipleExpectations
       expect { described_class.perform_now(model.id) }.to change { model.model_files.count }.to(2)
-      expect(model.model_files.map(&:filename)).to eq ["part_1.obj", "part_2.obj"]
+      expect(model.model_files.map(&:filename)).to eq ["part_1.lys", "part_2.obj"]
     end
 
-    it "sets the preview file to the first scanned file by default" do # rubocop:todo RSpec/MultipleExpectations
+    it "sets the preview file to the first renderable scanned file by default" do # rubocop:todo RSpec/MultipleExpectations
       expect { described_class.perform_now(model.id) }.to change { model.model_files.count }.to(2)
       model.reload
-      expect(model.preview_file.filename).to eq "part_1.obj"
+      expect(model.preview_file.filename).to eq "part_2.obj"
     end
 
     it "queues up individual file scans" do

spec/models/model_file_spec.rb

@@ -150,4 +150,16 @@
       expect(unsupported.reload.presupported_version).to be_nil
     end
   end
+
+  {
+    stl: true,
+    png: false,
+    pdf: false,
+    lys: false
+  }.each_pair do |extension, result|
+    it "shows that #{extension} files are#{"n't" if result == false} renderable" do
+      file = create(:model_file, filename: "test.#{extension}")
+      expect(file.is_renderable?).to be result
+    end
+  end
 end