From f96ef338562ff1fce9127c45fa2ed91cb5a2e5df Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 15 Feb 2023 12:41:04 +1100 Subject: [PATCH] FIX: dominant color not working for 16bit images (#20300) 16 bit images were not returning the correct dominant color due truncation The routine expected an 8bit color eg: #FFAA00, but ended up getting a 16bit one eg: #FFFAAA000. This caused a truncation, which leads to wildly off colors. --- app/models/upload.rb | 2 ++ spec/fabricators/upload_fabricator.rb | 3 ++- spec/models/upload_spec.rb | 9 +++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/models/upload.rb b/app/models/upload.rb index 249c50f4be..8f069ec65d 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -384,6 +384,8 @@ class Upload < ActiveRecord::Base "10", "convert", local_path, + "-depth", + "8", "-resize", "1x1", "-define", diff --git a/spec/fabricators/upload_fabricator.rb b/spec/fabricators/upload_fabricator.rb index 3101a110b2..d5c9b88846 100644 --- a/spec/fabricators/upload_fabricator.rb +++ b/spec/fabricators/upload_fabricator.rb @@ -31,10 +31,11 @@ end Fabricator(:image_upload, from: :upload) do transient color: "white" + transient color_depth: 8 after_create do |upload, transients| file = Tempfile.new(%w[fabricated .png]) - `convert -size #{upload.width}x#{upload.height} xc:#{transients[:color]} "#{file.path}"` + `convert -size #{upload.width}x#{upload.height} -depth #{transients[:color_depth]} xc:#{transients[:color]} "#{file.path}"` upload.url = Discourse.store.store_upload(file, upload) upload.sha1 = Upload.generate_digest(file.path) diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index a29df2e1cb..ac80783a36 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -611,6 +611,7 @@ RSpec.describe Upload do describe "#dominant_color" do let(:white_image) { Fabricate(:image_upload, color: "white") } let(:red_image) { Fabricate(:image_upload, color: "red") } + let(:high_color_image) { Fabricate(:image_upload, color: "#000A00F00", color_depth: 16) } let(:not_an_image) do upload = Fabricate(:upload) @@ -640,6 +641,14 @@ RSpec.describe Upload do expect(red_image.dominant_color).to eq(nil) expect(red_image.dominant_color(calculate_if_missing: true)).to eq("FF0000") expect(red_image.dominant_color).to eq("FF0000") + + expect(high_color_image.dominant_color).to eq(nil) + # original is: #000A00F00 + # downsamples to: #009FEF + # A00 is closer to 9F than A0 + # EF is closer to F00 than F0 + expect(high_color_image.dominant_color(calculate_if_missing: true)).to eq("009FEF") + expect(high_color_image.dominant_color).to eq("009FEF") end it "can be backfilled" do