FEATURE: do not switch to JPEG unless you meet 75k byte savings

FEATURE: do not switch to JPEG unless you meet 75k byte savings

This also adjusts the algorithm to expect

  • 30% saving for JPEG conversion

AND

  • Minimum of 75K bytes saved

The reasoning for increase of saving requirements is cause PNG may have been
uploaded unoptimized, 30% saving on PNG is very possible

From 86255faa08ed01930b55888cf52bc75d339f9766 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Wed, 21 Nov 2018 11:00:52 +1100
Subject: [PATCH] FEATURE: do not switch to JPEG unless you meet 75k byte
 savings

This also adjusts the algorithm to expect

- 30% saving for JPEG conversion

AND

- Minimum of 75K bytes saved

The reasoning for increase of saving requirements is cause PNG may have been
uploaded unoptimized, 30% saving on PNG is very possible

diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb
index 33a6704..35fa225 100644
--- a/lib/upload_creator.rb
+++ b/lib/upload_creator.rb
@@ -168,7 +168,15 @@ class UploadCreator
     pixels > MIN_PIXELS_TO_CONVERT_TO_JPEG
   end
 
+  MIN_CONVERT_TO_JPEG_BYTES_SAVED = 75_000
+  MIN_CONVERT_TO_JPEG_SAVING_RATIO = 0.70
+
   def convert_to_jpeg!
+
+    if filesize < MIN_CONVERT_TO_JPEG_BYTES_SAVED
+      return
+    end
+
     jpeg_tempfile = Tempfile.new(["image", ".jpg"])
 
     from = @file.path
@@ -186,8 +194,12 @@ class UploadCreator
       execute_convert(from, to, true)
     end
 
-    # keep the JPEG if it's at least 15% smaller
-    if File.size(jpeg_tempfile.path) < filesize * 0.85
+    new_size = File.size(jpeg_tempfile.path)
+
+    keep_jpeg = new_size < filesize * MIN_CONVERT_TO_JPEG_SAVING_RATIO
+    keep_jpeg &&= (filesize - new_size) > MIN_CONVERT_TO_JPEG_BYTES_SAVED
+
+    if keep_jpeg
       @file = jpeg_tempfile
       extract_image_info!
     else
diff --git a/spec/fixtures/images/should_be_jpeg.png b/spec/fixtures/images/should_be_jpeg.png
new file mode 100644
index 0000000..b3723f3
Binary files /dev/null and b/spec/fixtures/images/should_be_jpeg.png differ
diff --git a/spec/lib/upload_creator_spec.rb b/spec/lib/upload_creator_spec.rb
index 43cd261..04ac72f 100644
--- a/spec/lib/upload_creator_spec.rb
+++ b/spec/lib/upload_creator_spec.rb
@@ -99,13 +99,37 @@ RSpec.describe UploadCreator do
     end
 
     describe 'converting to jpeg' do
-      let(:filename) { "logo.png" }
+      let(:filename) { "should_be_jpeg.png" }
       let(:file) { file_from_fixtures(filename) }
 
+      let(:small_filename) { "logo.png" }
+      let(:small_file) { file_from_fixtures(small_filename) }
+
       before do
         SiteSetting.png_to_jpg_quality = 1
       end
 
+      it 'should not store file as jpeg if it does not meet absolute byte saving requirements' do
+
+        # logo.png is 2297 bytes, converting to jpeg saves 30% but does not meet
+        # the absolute savings required of 25_000 bytes, if you save less than that
+        # skip this
+
+        expect do
+          UploadCreator.new(small_file, small_filename,
+            pasted: true,
+            force_optimize: true
+          ).create_for(user.id)
+        end.to change { Upload.count }.by(1)
+
+        upload = Upload.last
+
+        expect(upload.extension).to eq('png')
+        expect(File.extname(upload.url)).to eq('.png')
+        expect(upload.original_filename).to eq('logo.png')
+
+      end
+
       it 'should store the upload with the right extension' do
         expect do
           UploadCreator.new(file, filename,
@@ -118,7 +142,7 @@ RSpec.describe UploadCreator do
 
         expect(upload.extension).to eq('jpeg')
         expect(File.extname(upload.url)).to eq('.jpeg')
-        expect(upload.original_filename).to eq('logo.jpg')
+        expect(upload.original_filename).to eq('should_be_jpeg.jpg')
       end
     end
   end

GitHub

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there: