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

FEATURE: Add focal-point support for ImageVariants (crops) and Thumbnails #5127

Draft
wants to merge 4 commits into
base: 8.4
Choose a base branch
from

Conversation

mficzel
Copy link
Member

@mficzel mficzel commented Jun 5, 2024

In many cases the crops taken from the center of images are not optimal and important parts of an image get lost. This pr allows to specify a focal-point for imageVariants that is later on respected when crops of the imageVariant are generated.

The implementation uses an preliminary crop for images with a focalPoint that ensures:

  • The target image is as large as possible inside the original image dimensions
  • The focal point is as central as possible inside the generated image

In addition this change calculates the focal-point and image dimensions for async thumbnails on generation. That way async thumbnails can be rendered with full width and height informations which allows to make use of those informations in the rendering to prevent layout shifts or to specify the center of an object-fit css rule.

Resolves: #2296

Required Neos.UI Changes neos/neos-ui#3835

  • The Neos.UI should be extended and allow to specify a focal point in the crop-image dialog. The given values should then be passed as focalPointX ?int and focalPointY ?int of the new ImageVariant that is created
    via endpoint neos/content/create-image-variant which points to \Neos\Neos\Controller\Backend\ContentController::createImageVariantAction
  • The Image Preview in the Inspector should visualize an existing focal point

Review instructions

!!! By intention the focal point is not added to images in the media library (yet). The main reason for that is that this would require a mechanism to re-render all thumbnails of an asset if the focal point is changed. Also i personally see more value to specify the focal point by editors for a specific crop that is why the implementation cover ImageVariants and Thumbnails. If we see value in focal point support via media library we can add this in future !!!

The main part of this solution is that the calculation of the cropped dimensions was moved out from the ResizeImageAdjustment to a new ResizeDimensionCalculator where it can be accessed from other code like the ImageThumbnailGenerator and the ThumbnailService that need to know about image dimensions beforehand. The tests for the now deprecated methods in ResizeImageAdjustment are still in place to prove that the calculations are the same.

For now the generated thumbnails will get a small green circle rendered on the focal-point if a focal point was set. This allows to visually verify the results in a human understandable way. After approval this will be removed.

Also the tests with php 8.0 and 8.1 are failing since this will be rebased on to 8.4 once this branch is green again. I used 8.2 features that will not work with lower php versions.

Final Todos before merge - after review and general approval

  • Remove the MarkPointAdjustment and the code using it in the ImageThumbnailGenerator
  • Generate postgres db-migrations
  • Rebase on to Neos 8.4 ... this is based on 8.3 as this branch is green at this time

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@github-actions github-actions bot added the 9.0 label Jun 5, 2024
@mficzel mficzel changed the base branch from 9.0 to 8.4 June 5, 2024 07:28
@github-actions github-actions bot added 8.4 and removed 9.0 labels Jun 5, 2024
@mficzel mficzel force-pushed the feature/imageFocalPoints branch 2 times, most recently from c9b88f8 to 0cb0548 Compare June 5, 2024 07:32
@ahaeslich ahaeslich marked this pull request as draft June 5, 2024 07:40
@mficzel mficzel force-pushed the feature/imageFocalPoints branch 2 times, most recently from f5cba4d to e63bfeb Compare June 5, 2024 08:37
@mficzel mficzel changed the title WIP : FEATRUE: Image focal points WIP : FEATURE: Image focal points Jun 5, 2024
@mficzel mficzel force-pushed the feature/imageFocalPoints branch 5 times, most recently from 2bbe2f2 to 1efd301 Compare June 5, 2024 10:43
@mficzel mficzel force-pushed the feature/imageFocalPoints branch 4 times, most recently from 6bd01c1 to 0a4e5cb Compare July 25, 2024 12:47
@mficzel mficzel changed the title WIP : FEATURE: Image focal points FEATURE: Image focal points Jul 25, 2024
@mficzel mficzel changed the title FEATURE: Image focal points FEATURE: Add focal-point support for ImageVariants (crops) and Thumbnails Jul 25, 2024
@mficzel mficzel force-pushed the feature/imageFocalPoints branch 2 times, most recently from 4421359 to a59b1ca Compare July 25, 2024 14:25
@mficzel mficzel marked this pull request as ready for review July 25, 2024 14:36
@mficzel mficzel requested review from grebaldi, mhsdesign and Sebobo July 25, 2024 14:56
@mficzel mficzel force-pushed the feature/imageFocalPoints branch from a59b1ca to bdb28e1 Compare July 26, 2024 08:45
@mficzel mficzel force-pushed the feature/imageFocalPoints branch from bdb28e1 to 17eac5d Compare July 26, 2024 08:52
@mficzel mficzel changed the base branch from 8.4 to 8.3 July 26, 2024 08:54
@github-actions github-actions bot added 8.3 8.4 and removed 8.4 labels Jul 26, 2024
@mficzel
Copy link
Member Author

mficzel commented Jul 26, 2024

close and reopen to reset ci

@mficzel mficzel closed this Jul 26, 2024
@mficzel mficzel reopened this Jul 26, 2024
@mficzel
Copy link
Member Author

mficzel commented Jul 26, 2024

This at least prooves that the tests are running for php 8.2 and 8.3. The failing Stan Tests correctly identifies that this core required php 8.2.

After #5188 is fixed this branch should be rebased on 8.4 and should become green.

@mficzel mficzel changed the base branch from 8.3 to 8.4 July 26, 2024 09:56
@github-actions github-actions bot removed the 8.3 label Jul 26, 2024
@mficzel mficzel changed the base branch from 8.4 to 8.3 July 26, 2024 10:04
@mficzel mficzel marked this pull request as draft July 26, 2024 10:04
@github-actions github-actions bot added 8.3 and removed 8.4 labels Jul 26, 2024
@mficzel
Copy link
Member Author

mficzel commented Jul 26, 2024

Converted to draft until 8.4 is green again and this can be rebased

@kdambekalns kdambekalns self-requested a review August 5, 2024 16:53
…perThingy

This obviously will need a better name once we better understand the tasks it will have to perform.
…th focalPoint

The preliminary crop ensures:
1. The target image is as large as possible inside the original image dimensions
2. The focal point is as central as possible inside the generated image
@mficzel mficzel force-pushed the feature/imageFocalPoints branch from 17eac5d to 4265151 Compare August 27, 2024 13:43
@mficzel mficzel changed the base branch from 8.3 to 8.4 August 27, 2024 13:44
@github-actions github-actions bot added 8.4 and removed 8.3 labels Aug 27, 2024
@mficzel mficzel closed this Aug 27, 2024
@mficzel mficzel reopened this Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant