Skip to content
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

Image option 'quality' and 'format' do not work with derivatives() and responsive images #3146

Open
NicoHood opened this issue Jan 12, 2021 · 7 comments
Labels

Comments

@NicoHood
Copy link
Contributor

{{ image.derivatives([480, 600, 1000]).quality(1)|html|raw }}

This does not work with derivatives. It only changes the quality of the main image to 1, but not the others in srcset. Originally reported here: #3144 (comment)

@NicoHood
Copy link
Contributor Author

First debugging results:
When calling format or quality the call functions of ImageMedia are called, but not the __call function:
https://github.com/getgrav/grav/blob/develop/system/src/Grav/Common/Page/Medium/ImageMedium.php#L423
https://github.com/getgrav/grav/blob/develop/system/src/Grav/Common/Page/Medium/ImageMedium.php#L535

__call Only gets called in the derivatives() function via resize:
https://github.com/getgrav/grav/blob/develop/system/src/Grav/Common/Page/Medium/ImageMedium.php#L323

That is the reason why the alternatives are never called via format and quality. It only works for the single image, as it is passed here as parameter:
https://github.com/getgrav/grav/blob/develop/system/src/Grav/Common/Page/Medium/ImageMedium.php#L627

I am not sure how to fix this yet.

@pamtbaau
Copy link
Contributor

pamtbaau commented Jan 19, 2021

Combining cropZoom() or flip() with derivatives() also fails to work. So maybe all functions in combination with derivatives may yield incorrect results.

The results depends on the order of applying flip(), cropZoom() and derivatives().

Flip:
When using, flip.derivatives, only the last is flipped. When using derivatives.flip, all images are flipped.

zoomCrop:
zoomCrop shows a similar result, but adds a sizing issue.

Original: 400x800 (portrait)

{{ image.cropZoom(800, 400).derivatives([100, 200, 300]).html() | raw }}
Only one image, the original, is cropZoomed, the rest are only resized to right proportions.

  • 100 -> 100 x 200
  • 200 -> 200 x 400
  • 300 -> 300 x 600
  • original -> 800 x 400        <- cropZoomed

{{ image.derivatives([100, 200, 300]).cropZoom(800, 400).html() | raw }}
All images are cropZoomed, but the sizes are way off:

  • 100 -> 3200 x 1600          <- cropZoomed
  • 200 -> 1600 x 800            <- cropZoomed
  • 300 -> 1066 x 533            <- cropZoomed
  • original -> 800 x 400        <- cropZoomed

@rhukster rhukster added the bug label Jan 20, 2021
@rhukster
Copy link
Member

I really think this whole media handling needs a rewrite as it's a confusing mess in there. Unfortunately, that is probably not going to happen until version 2.0.

@Karmalakas
Copy link
Contributor

So until then, it's not really possible to use srcset in some easy way with manipulated images, right? Any timeline for v2.0?

@pamtbaau
Copy link
Contributor

Being a different discussion and not knowing your use-case...

As a 'photography enthusiast' I would hand craft my images using Photoshop, Gimp or the likes. Then, when satisfied with the result, apply image.derivatives().sizes() to create srcsets as the last stage of the workflow.

@Karmalakas
Copy link
Contributor

That's a great advice and most likely will have to do this. It's just that these images are already crafted in Ps to be published :) They only wouldn't fit the layout of a webpage. But it seems will have to create cropped versions too :)

@pamtbaau
Copy link
Contributor

pamtbaau commented Jan 21, 2021

If crafted images don't fit the layout of the website, they either have not been crafted for the website's layout, or the website isn't designed properly to accommodate the crafted images...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants