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

Add canvas parameter to product images #206

Merged
merged 1 commit into from
Sep 13, 2018

Conversation

michaelcasey316
Copy link
Contributor

Change description:

We noticed an issue with the fastly-magento2 module when Deep Image Optimization is enabled. Product images are being delivered that aren't exactly matching the requested size, causing issues with Magento 2 templates.

This code proposal changes the fastly-magento2 module request a canvas size equal to the target product image size. Aspect ratio is maintained, just like before.

Example product image URL without this change:
lower_manhattan_from_empire_state_building.jpg?width=88&height=110&quality=80&bg-color=255,255,255&fit=bounds

Example product image URL with this change:
lower_manhattan_from_empire_state_building.jpg?width=88&height=110&quality=80&bg-color=255,255,255&fit=bounds&canvas=88,110

Adding empty pixels using formats without an alpha channel requires picking a background color, and this is supported by the built-in image resizers. Conveniently, the background color is already being specified in the URL parameters sent by the fastly-magento2 module without making any changes to the code.

If the addition of the canvas parameter should be in another method or should be implemented in a different way, please let me know.

Why this change is needed:

Fastly Deep Image Optimization, as configured by the fastly-magento2 module, delivers product images that are cropped to remove empty canvas space. This causes issues with store themes.

Deep image optimization disabled:
image_optimization_disabled_product_nav_wrap

Deep image optimization enabled:
image_optimization_enabled_product_nav_wrap

Deep image optimization disabled (open the image in a new tab to see the difference):
pigments_deep_io_disabled

Deep image optimization enabled (open the image in a new tab to see the difference):
pigments_deep_io_enabled

The built-in Magento 2 image resizers based on ImageMagick and GD2 behave differently than the fastly-magento2 module with Deep Image Optimization enabled. With both built-in and Deep Image Optimization resizers, product images are resized to fit entirely within a target height and width. Also, they both preserve the aspect ratio of the source image to avoid distortion. The difference is that the built-in resizers add empty space to either the top and bottom or the left and right so that the canvas size exactly matches the target height and width.

The current Deep Image Optimization behavior causes template issues because of templates scaling product images by changing their shortest edge to match their container. The problem is images delivered to the browser in this way can be smaller than the image that's ultimately displayed. This results in images that are first scaled down only to be scaled up again. This can look pretty bad depending on how much upscaling is performed.

There's an edge case when the source image and the image container size have the exact same aspect ratio. When this happens, no upscaling is performed by the template and the resulting image looks fine. This is demonstrated in the examples above by the picture of the duffel bag. In every other case, the problem occurs.

I tested this change with several different Magento 2 themes, though I was unable to test any paid themes. In all cases, the problem is present with the original fastly-magento2 module and goes away with both the built-in resizers and with the modified Fastly module.

@vvuksan
Copy link
Contributor

vvuksan commented Sep 13, 2018

Thanks Michael. We'll review it.

@vvuksan
Copy link
Contributor

vvuksan commented Sep 13, 2018

Looks good. More efficient than using fit=cover to address situations like these.

@vvuksan vvuksan merged commit 5d4e57d into fastly:master Sep 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants