contao/image

Percentage values for the important part

ausi opened this issue · 6 comments

ausi commented

Working on contao/core-bundle#651 I realized that it would have made much more sense to use percentages instead of integer-pixel-values for the ImportantPart. I see some benefits with it:

  1. If an image has no important part it just defaults to {x: 0%, y: 0%, width: 100%, height: 100%} and we don’t have to read the dimensions from the image as in Image::getImportantPart()
  2. An important part object can be validated very easily without needing to know anything about the image.
  3. Features like contao/core#8360 (comment) would be easier to implement.

Is it too late to change that in image 0.4 / core-bundle 4.4?
cc @contao/developers

discordier commented

I fail to see why this should be a breaking change and therefore "too late".
You are only talking about supporting percentage values in addition to pixel values, aren't you?

ausi commented

No, switching away from pixel values to percentage values completely. Supporting both would be confusing IMO and also not possible I think because Imagine’s PointInterface and BoxInterface don’t support percentages.

leofeyer commented

If there is an upgrade path for existing images, I don't see a problem.

Toflar commented

It's still a 0.x version so you can change that for sure. Anyone already using it (so the Contao core bundle) has to be prepared for breaking changes. For the sake of our users we should try to provide an upgrade path for Contao :)

ausi commented

Note to myself:

  • Create a new class and interface for the important part.
  • Accept both the new and the old ImportantPart everywhere in the library.
  • Trigger deprecation warnings if the old class was used.
  • pull request for core-bundle 4.4 to use the new class everywhere and update the tl_files fields to percentages (float values between 0 and 1).
ausi commented

Pull request: #51