diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb index 01f870baa6b..5e5e8e41919 100644 --- a/app/models/optimized_image.rb +++ b/app/models/optimized_image.rb @@ -318,9 +318,13 @@ class OptimizedImage < ActiveRecord::Base convert_with(instructions, to, opts) end + MAX_PNGQUANT_SIZE = 500_000 + def self.convert_with(instructions, to, opts = {}) Discourse::Utils.execute_command(*instructions) - FileHelper.optimize_image!(to) + + allow_pngquant = to.downcase.ends_with?(".png") && File.size(to) < MAX_PNGQUANT_SIZE + FileHelper.optimize_image!(to, allow_pngquant: allow_pngquant) true rescue => e if opts[:raise_on_error] diff --git a/lib/file_helper.rb b/lib/file_helper.rb index f146b8b139d..3f148fc278c 100644 --- a/lib/file_helper.rb +++ b/lib/file_helper.rb @@ -82,7 +82,12 @@ class FileHelper tmp end - def self.optimize_image!(filename) + def self.optimize_image!(filename, allow_pngquant: false) + pngquant_options = false + if allow_pngquant + pngquant_options = { allow_lossy: true } + end + ImageOptim.new( # GLOBAL timeout: 15, @@ -92,7 +97,7 @@ class FileHelper advpng: false, pngcrush: false, pngout: false, - pngquant: false, + pngquant: pngquant_options, # JPG jpegoptim: { strip: SiteSetting.strip_image_metadata ? "all" : "none" }, jpegtran: false, diff --git a/spec/fixtures/images/pngquant.png b/spec/fixtures/images/pngquant.png new file mode 100644 index 00000000000..2ffb274f20a Binary files /dev/null and b/spec/fixtures/images/pngquant.png differ diff --git a/spec/lib/upload_creator_spec.rb b/spec/lib/upload_creator_spec.rb index 04ac72f00b1..897c946aeea 100644 --- a/spec/lib/upload_creator_spec.rb +++ b/spec/lib/upload_creator_spec.rb @@ -98,6 +98,27 @@ RSpec.describe UploadCreator do end end + describe 'pngquant' do + let(:filename) { "pngquant.png" } + let(:file) { file_from_fixtures(filename) } + + it 'should apply pngquant to optimized images' do + upload = UploadCreator.new(file, filename, + pasted: true, + force_optimize: true + ).create_for(user.id) + + # no optimisation possible without losing details + expect(upload.filesize).to eq(9558) + + thumbnail_size = upload.get_optimized_image(upload.width, upload.height, {}).filesize + + # pngquant will lose some colors causing some extra size reduction + expect(thumbnail_size).to be < 7500 + end + + end + describe 'converting to jpeg' do let(:filename) { "should_be_jpeg.png" } let(:file) { file_from_fixtures(filename) } diff --git a/spec/models/optimized_image_spec.rb b/spec/models/optimized_image_spec.rb index 45c96abbcd2..9aaa86442c8 100644 --- a/spec/models/optimized_image_spec.rb +++ b/spec/models/optimized_image_spec.rb @@ -6,7 +6,7 @@ describe OptimizedImage do unless ENV["TRAVIS"] describe '.crop' do - it 'should work correctly (requires correct version of image optim)' do + it 'should produce cropped images' do tmp_path = "/tmp/cropped.png" begin @@ -17,12 +17,15 @@ describe OptimizedImage do 5 ) - fixture_path = "#{Rails.root}/spec/fixtures/images/cropped.png" - fixture_hex = Digest::MD5.hexdigest(File.read(fixture_path)) + # we don't want to deal with something new here every time image magick + # is upgraded or pngquant is upgraded, lets just test the basics ... + # cropped image should be less than 120 bytes - cropped_hex = Digest::MD5.hexdigest(File.read(tmp_path)) + cropped_size = File.size(tmp_path) + + expect(cropped_size).to be < 120 + expect(cropped_size).to be > 50 - expect(cropped_hex).to eq(fixture_hex) ensure File.delete(tmp_path) if File.exists?(tmp_path) end @@ -128,7 +131,7 @@ describe OptimizedImage do end describe '.downsize' do - it 'should work correctly (requires correct version of image optim)' do + it 'should downsize logo' do tmp_path = "/tmp/downsized.png" begin @@ -138,12 +141,10 @@ describe OptimizedImage do "100x100\>" ) - fixture_path = "#{Rails.root}/spec/fixtures/images/downsized.png" - fixture_hex = Digest::MD5.hexdigest(File.read(fixture_path)) + info = FastImage.new(tmp_path) + expect(info.size).to eq([100, 27]) + expect(File.size(tmp_path)).to be < 2300 - downsized_hex = Digest::MD5.hexdigest(File.read(tmp_path)) - - expect(downsized_hex).to eq(fixture_hex) ensure File.delete(tmp_path) if File.exists?(tmp_path) end