Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Combines the Scale and Resize image effects into a single Scale effect with a checkbox for "Preserve aspect ratio" that defaults to checked. This is intended to simplify the image styles UI and make it more unified with what users have come to expect from applications like Photoshop and other image manipulation software.
This patch is dependent on #371374: Add ImageCache UI Core and is based off the discussion started by kika in that thread (link). I've separated it from that issue so that it doesn't get held up any longer than it has to and this is really just polish that is not absolutely necessary and because after working on it a bit I think it needs a bit more discussion regarding API changes.
The attached patch only modifies the UI aspect of things but it raises the question of wether or not we should change this in the API as well. Currently we have image_scale() and image_resize() functions. The image_scale() function resizes an image and maintains aspect ratio requiring only one dimension to be specified. The image_resize() function scales an image to an exact set of given dimensions.
Should we rewrite image_scale() with a maintain aspect ratio flag and get rid of image_resize() or should be leave it as is?
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 524562-7-eojthebrave_preserve_aspect_ratio.patch | 12.45 KB | eojthebrave |
| #4 | 524562-4-eojthebrave_preserve_aspect_ratio.patch | 12.45 KB | eojthebrave |
| ic-ui-add_scale_effect.png | 33.82 KB | eojthebrave | |
| eojthebrave-ic_preserve_aspect_ratio.patch | 10.17 KB | eojthebrave | |
Comments
Comment #1
drewish CreditAttribution: drewish commentedI think now that we pass an image object between functions and don't save the image to disk until all the transformations are complete it makes sense to revisit the API to remove some functions that were written to avoid the overhead and loss of quality or writing to disk and reloading (namely scale_and_crop).
Comment #3
eojthebravedrewish and I discussed this in IRC, the outcome of that discussion was that do to the way images are handled now we could remove the Scale & Crop effect and associated API function
image_scale_and_cropand achieve the same results by chaining together a scale effect and a crop effect with no performance/quality loss. Makes sense to me, especially on the UI side. Do we have any good reason to maintain the APIimage_scale_and_cropfunction as a convenience? Do we want to tackle this change here along with removing the resize effect and adding a maintain aspect ratio option to the scale effect.Do we want to keep the
image_resizeAPI function around or modifyimage_scaleso that it takes a preserve aspect ratio flag? I'm personally inclined to keep the function as it makes the API simpler, but could go either way.Comment #4
eojthebraveRerolled. Patch should apply properly now, and tests that would not have passed even if the previous patch did apply should pass now.
Still open to removing the "Scale & Crop" effect in favor of just chaining a Scale effect and a Crop effect together. Do we still want to try and tackle that here?
Comment #6
sun.core CreditAttribution: sun.core commentedComment #7
eojthebraveRe-roll.
Comment #8
moshe weitzman CreditAttribution: moshe weitzman commentedComment #9
eojthebraveTalked with drewish about this on IRC and the outcome of that was.
1. Remove resize effect
2. Add preserve aspect ratio option to scale effect
3. Add an inside/outside dimensions scaling option to crop effect
Will re-roll in the near future.
Comment #11
sun.core CreditAttribution: sun.core commentedComment #14
fietserwinI just filed issue #1554074: image scale does not work when current dimensions are unknown which will influence the patch for this issue.
Setting to D8, as this is a UI change that will likely not receive a backport to D7.
Comment #23
claudiu.cristeaComment #30
candelas CreditAttribution: candelas as a volunteer commentedAny news on this one for Drupal 11 or 10?
Thanks