New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow compress to be invoked directly #600

Merged
merged 3 commits into from Dec 17, 2018

Conversation

Projects
None yet
2 participants
@matslindh
Copy link
Contributor

matslindh commented Dec 13, 2018

Since the implementation of the Transformation Manager framework, we're not registered as an event listener by default. This allows the compress statement to be executed at the current location in the chain as necessary.

matslindh added some commits Dec 13, 2018

Allow compress to be invoked directly when not registered as event li…
…stener

Since the implementation of the Transformation Manager framework, we're
not registered as an event listener by default. This allows the
compress statement to be executed at the current location in the chain
as necessary.
Apply and track requested output compression in image model
Since the output converters now has the final say in how an image should
be rendered, and the old compress transformation was borked by the
introduction of the transformationManager, we now explicitly track the
requested compression quality on the image model.

This allows to apply the compression quality selectively on output
instead of having to try to work around when the different classes are
created and registered for transformations and events.
@@ -39,6 +39,14 @@ public function getSupportedMimeTypes() {
public function convert(Imagick $imagick, Image $image, $extension, $mimeType) {
try {
$imagick->setImageFormat($extension);

This comment has been minimized.

@christeredvartsen

christeredvartsen Dec 14, 2018

Member

Since this code is moved to the Basic output converter, it will no longer work for the WebP and the BMP output converters. I'm not sure if it has ever worked for them though.

This comment has been minimized.

@matslindh

matslindh Dec 14, 2018

Contributor

Seeing as the Compress transformation has been borked since the introduction of the transform manager (my guess) and the output converter was introduced after that, it never has worked. For BMP there isn't a quality/compress setting, but for webp it could be introduced - but I'll have to make a few further modifications and tests - so I'll leave that for another PR if needed.

@matslindh

This comment has been minimized.

Copy link
Contributor

matslindh commented Dec 17, 2018

Tracked in issue #601.

@christeredvartsen christeredvartsen merged commit 22125d1 into imbo:develop Dec 17, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment