Page MenuHomePhabricator

Apply -background none to palette PNGs
ClosedPublic

Authored by Gilles on Sep 18 2018, 9:07 AM.

Details

Maniphest Tasks
T198370: Transparent background renders as white in PNG thumbnails
Reviewers
fgiunchedi
Commits
rTHMBREXTb8dda36d73fd: Apply -background none to palette PNGs
Patch without arc
git checkout -b D1113 && curl -L https://phabricator.wikimedia.org/D1113?download=true | git apply
Summary

PNGs with indexed colors can have transparency too.

Refs T198370

Diff Detail

Repository
rTHMBREXT Thumbor Plugins
Branch
T198370 (branched from master)
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 3126
Build 5225: differential-jessieJenkins
Build 5224: arc lint + arc unit

Event Timeline

Gilles created this revision.Sep 18 2018, 9:07 AM
Gilles requested review of this revision.Sep 18 2018, 9:08 AM
Gilles added a comment.EditedSep 18 2018, 9:14 AM

Actually not without consequence, this is turning RGB thumbnails into RGBA, and the extra transparency layer is making the files bigger and some tests are failing as a result (good thing I added tight file size checks!). Instead I'll try to restrict to RGBA + indexed.

Gilles updated this revision to Diff 2903.Sep 18 2018, 9:20 AM

Simply add palette PNGs to the rule

Gilles retitled this revision from Don't limit -background none to RGBA images to Apply -background none to palette PNGs.Sep 18 2018, 9:24 AM
Gilles edited the summary of this revision. (Show Details)

Worth adding a test case for this to catch regressions I think

Gilles updated this revision to Diff 2904.Sep 18 2018, 1:45 PM

Add test case

Because of the other things going on that affect the SSIM, it wouldn't really catch the same regression happening, though...

fgiunchedi accepted this revision.Sep 18 2018, 4:43 PM
In D1113#22217, @Gilles wrote:

Because of the other things going on that affect the SSIM, it wouldn't really catch the same regression happening, though...

I see, if there's no other easy way to catch this particular regression then what we have now will do

This revision is now accepted and ready to land.Sep 18 2018, 4:43 PM
This revision was automatically updated to reflect the committed changes.