Crop / Resize behavior

There has been quite some discussion in Slack about a change in resize/crop behavior introduced by me. Fact is, I didn’t see that you cannot give width and height to the ImageViewHelper but just maximum width and height.

Let me explain how the calculation of the ResizeAdjustment (that is also responsible for the ImageViewHelper in the end) works now and what use cases I thought about and what definitely needs to be done.

First of all, the ResizeAdjustment has the following parameter:

  • width
  • height
  • maximumWidth
  • maximumHeight
  • allowUpScaling
  • ratioMode (which can be ‘inset’ or ‘outbound’ and basically maps to allowCropping false/true)

I will list scenarios and results below, maybe someone with more talent in image manipulation can add images that visually show the bahavior?


  1. Only one dimension is set (lets take width) and allowUpScaling is not set, ratioMode will play no role in this scenario so it doesn’t matter if set or not.

Result: Width is checked against the original image width and if bigger reduced to the original image width, after that the image is scaled to the resulting width according to the original aspect ratio.


  1. Same as above but now allowUpScaling is set.

Result: Width is directly used to scale according to the original aspect ratio.


  1. Same as 1 but maximumWidth is set.

Result: After checking against the original image width, the maximumWidth is checked and if width is heigher it will be restrained to maximumWidth, after that the image is again scaled according to the original aspect ratio.


  1. Now the more complicated scenarios, width AND height are set to something. First with allowUpScaling false and ratiomode “inset” (cropping false).

Result: We calculate the ratios between requested and original for width and height and scale to the one with the smaller ratio (but still with the original aspect ratio) so that we can be sure that both sides are less than or equal the requested width/height. As allowUpScaling is false if the result would be larger than the original image we just deliver the original image instead.


  1. Same as 4 but with max width and/or height set.

Result: Same as above, but after we know the size that fullfills the requested width/height we check if the result would fit in the given max and otherwise scale further down to stay inside the max values.
Note that is scenario is useless, you would either set width/height or maxWidth/maxHeight


  1. Same as 4 but with ratiomode “outbound” (cropping true)

Result: First we take a box that is exactly the given width/height as the expected outcome, as allowUpScaling is false we shrink the box according to the box aspect ratio until it fits the original image so that no upscaling is needed. Then we scale the image according to the original aspect ratio so that the smaller side fits the calculated box (means the other dimension will have content outside the box). We center the box on the image and crop to the box.


  1. Same as 4 but with allowUpScaling true and ratiomode “outbound” (cropping true)

Result: First we take a box that is exactly the given width/height as the expected outcome. Then we scale the image according to the original aspect ratio so that the smaller side fits the calculated box (means the other dimension will have content outside the box). We center the box on the image and crop to the box.


  1. Same as 6 but with max width/height

Result: First we take a box that is exactly the given width/height as the expected outcome. Then the box is scaled to the given maximum according tot he box aspect ratio. Then we scale the image according to the original aspect ratio so that the smaller side fits the calculated box (means the other dimension will have content outside the box). We center the box on the image and crop to the box.


Ok I left out some scenarios but I think you get the big picture. All in all this doesn’t allow for distorted images currently (a scenario I don’t think we should support out of the box).

The big point is, you might not know what aspect ratio a given image has. So setting a width and height to something but without ratiomode outbound would just allow you to have an image with the original aspect ratio fitting into an area. Now here it seems this is maximum width/height. But what if you set only a width but at the same time want to constrain really tall images, you can use a combination of width and max height. Same vice versa. IMHO a valid use case.

Obivously there are a few scenarios where arguments will be useless but that cannot be avoided I guess.
If you set width AND height with ratiomode “inset” then max width/height should usually never affect the result unless they are smaller than width/height.
If you set only width OR height the ratiomode makes no difference at all.

So for me, we need to expose width/height to the Image VH and TS image object. We could set width/height to max width height if not given for now and deprecate this at the same time (roughly do what the hotfix https://bitbucket.org/snippets/vivomedia-neos/89pkx does with additionally allowing width/height directly).

So after a long discussion on Slack, check here:
https://neos-project.slack.com/archives/neos-general/p1441962620000585

We end up with this solution:

  • Introduce width + height in the VH + TS object
  • Setting width + height automatically enable cropping (if not set explicitly to allow 8.) in VH + TS object
  • Keep the ImageAdjustment as it is currently
  • Rename ratioMode to allowCropping (more human readable)
  • Try to be less breaking as possible

Typical scenarios:

  • width=100 <-- shrink to 100px width proportionally (same as maximumWidth)
  • width=100 height=100 <-- crop it
  • maximumWidth=100 maximumHeight=100 <-- scale down to fit the rectangle

During the discussion on Slack, we discuss about deprecating allowCropping in the VH, maybe this is not required, can be set to NULL by default, automatically set if width / height are set.

1 Like

I would do that in separate changes.

Especially renaming ratioMode, that is something I would do for master only as eventually breaking change. The rest needs to go into 3.0.

Fine for me:

  • Introduce width + height in the VH + TS object
  • Setting width + height automatically enable cropping (if not set explicitly to allow 8.) in VH + TS object
  • Keep the ImageAdjustment as it is currently
  • Rename ratioMode to allowCropping (more human readable) (only for master, separate commit)
  • Try to be less breaking as possible

+1 Sound good for me.

Affected methods:
https://github.com/neos/neos-development-collection/blob/master/TYPO3.Media/Classes/TYPO3/Media/ViewHelpers/ImageViewHelper.php#L119-L123

https://github.com/neos/neos-development-collection/blob/master/TYPO3.Media/Classes/TYPO3/Media/Domain/Service/AssetService.php#L44-L81

https://github.com/neos/neos-development-collection/blob/master/TYPO3.Media/Classes/TYPO3/Media/Domain/Service/ThumbnailService.php#L83-L101

https://github.com/neos/neos-development-collection/blob/master/TYPO3.Media/Classes/TYPO3/Media/Domain/Model/Thumbnail.php#L83-L92

Just one addition: deprecate allowCropping in VH/TS in favor of just setting width and height.

Making this change non breaking will be a hell :hankey:

Thumbnail::__construct(AssetInterface $originalAsset, $width = NULL, $maximumWidth = NULL, $height = NULL, $maximumHeight = NULL, $ratioMode = ImageInterface::RATIOMODE_INSET, $allowUpScaling = NULL)

make a lots more sense than:

Thumbnail::__construct(AssetInterface $originalAsset, $maximumWidth = NULL, $maximumHeight = NULL, $ratioMode = ImageInterface::RATIOMODE_INSET, $allowUpScaling = NULL, $width = NULL,  $height = NULL)

Same thing for at least all the other method involved …

No clue how to have a nice API and BC change … maybe we should introduce a Value Object ImageConfiguration to handle all those options and change the API to something like:

Thumbnail::__construct(AssetInterface $originalAsset, ImageConfiguration $configuration)

And use AOP (even if AOP is not targeted to this kind of things and add overhead and hell to debug) to make in non BC. In this case we can store a hash of the ImageConfiguration object in the database to avoid adding column for each options.

1 Like

A fix is ready for review:

This PR is pretty hard to test and is not BC currently. In Neos 1.2, this snippet will generate a square image of 200x200:

<typo3.media:image image="{image}" maximumWidth="200" maximumHeight="200" allowCropping="true" alt="{title}" />

With this PR this code need to be adjusted to:

<typo3.media:image image="{image}" width="200" height="200" alt="{title}" />

Will try to make this PR less breaking, but comments are welcome and testers required.

To test the PR:

Can’t we somehow use max w/h as w/h if not given? That would be bc. We can then deprecate this fallback behavior.

Good idea, at least the VH / TS changes will be BC.

Now what about the change in the API (with the introduction of the ThumbnailConfiguration value object) ?

The solution should be to use maximumWidth and maximumHeight -> width and height only if allowCropping is TRUE, in the case we stay BC, but allow to use maximumWidth and maximumHeight without cropping. And the behaviour of 1.2 if to produce “cropped” thumbs only in this case (is_int(maximumWidth) && is_int(maximumHeight) && $allowCropping === TRUE).

This could be done directly in the ThumbailConfiguration value object, check the last commit in the PR:

Any of this marked @api? Do you use it?
Otherwise I think it’s not a big deal…

1 Like

@christianm Asset::getThumbnail is marked @api, so I restore the interface to work as before all the other method are not marked @api

Let me install a DemoSite and buid a nice TS object to test if everything work as expected.

Just pushed a new version of the PR, with a gist for testing:

Juste update the gist, with all examples based on @christianm scenarios:

Here a screenshot of the rendering, with the current patch applied, there is some JS in the snippet to allow more easy review of the final image width / height and if the image is the original one or a thumbnail:

Last review for me, and the PR is ready to be pair reviewed. Maybe at some point we can use something like PhantomCSS to check for regression.

Edit: updated the screen shot with TS + Fluid rendering

Enough discussion, the PR is not ready for review: