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
This commit is contained in:
Sam 2018-11-21 11:00:52 +11:00
parent 1def6c08ec
commit 86255faa08
3 changed files with 40 additions and 4 deletions

View File

@ -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

BIN
spec/fixtures/images/should_be_jpeg.png vendored Normal file

Binary file not shown.

After

Width:  |  Height:  |  Size: 199 KiB

View File

@ -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