Introduce mozjpeg and pngquant post-processors, add transform options. #717
Conversation
lsmith77
added
the
State: Reviewing
| * @param int|null $quality Quality factor | ||
| */ | ||
| public function __construct( | ||
| $mozjpegBin = '/opt/mozjpeg/bin/cjpeg', |
There was a problem hiding this comment.
could be this made configurable somehow?
There was a problem hiding this comment.
This is already configurable here as a parameter - the value in the constructor shouldn't ever need to be used. https://github.com/antoligy/LiipImagineBundle/blob/implement-pngquant-mozjpeg/Resources/config/imagine.xml#L79
|
Hiya @lsmith77, any updates with this PR? I'd love to start rolling this out to production as it really has been helping cut back on page weight. Please let me know! 😄 |
|
Hey @lsmith77. Are you actually going to respond to this PR anytime soon? It's been over a month.. Would love to see this merged sometime soon so i can take advantage of better compression 👯 |
| * | ||
| * @throws ProcessFailedException | ||
| * | ||
| * @return BinaryInterface | ||
| * | ||
| * @see Implementation taken from Assetic\Filter\optipngFilter | ||
| */ | ||
| public function process(BinaryInterface $binary) | ||
| public function process(BinaryInterface $binary, array $options) |
There was a problem hiding this comment.
this is BC break, not sure we can do it,
There was a problem hiding this comment.
do you have any idea BC way?
There was a problem hiding this comment.
I am the second argument was added to the interface
|
@antoligy Hi, I am fine to merge this if you can do it BC way. |
|
@makasim Cheers for reviewing! Just looking at this, instead of making the change to |
|
One alternative might be to introduce a new That said I'm not so sure that this does break BC - Looking at the iterator in FilterManager, an array is always passed to the processors and I can't see anywhere else from where these are called. I suppose this affects custom post processors, however if this change is documented somewhere or there's a new minor revision then to me this wouldn't be so much of an issue. What do you think? |
|
Let's create a new interface with a note "This for BC and so on" so while working on the 2.x version we can merge them into one again. |
|
How's this? (Separate interface, versus an extended one) StyleCI has failed on the Test bootstrap.php. |
|
I'll do a deeper review on wednesday when I finally come back to work. |
|
Hey @makasim, have you had a chance to look at this yet? 😄 |
lsmith77
removed
the
State: Reviewing
|
https://github.com/liip/LiipImagineBundle/releases/tag/1.5.3 @antoligy Thanks for the contribution |
|
Is there an option to always add (the same) post processor to every filter_set? |
|
@ceesvanegmond There is not. |
Implementing mozjpeg and pngquant post processors, as both allow for significantly greater lossy compression to the current options. This also allows for options to be passed at run-time, as different image filter sets may require different degrees of compression.
Concerning mozjpeg in particular I'd like to revisit this later on, as the current implementation can be improved to account for the level of detail required in an image (and dpr), but the current default is fairly safe and I'm already seeing reductions of 80-90% compared to the raw images.