Updated: Comment #0

Part of #2105863: [meta] Images, toolkits and operations.

Problem/Motivation

In the process of revamping the image system (see #2105863: [meta] Images, toolkits and operations) we've found that, before Image operations (resize, crop, rotate, etc.) are actually applied, there are some toolkit-agnostic tasks that are performed against operation arguments. Those tasks are referring mainly to normalizing dimensions, coordinates, etc. Example: \Drupal\Core\Image\Image::scale() effect is computing the height when only width is provided. Partially those tasks are deferred to an utility component: \Drupal\Component\Utility\Image but most of this algebra is still performed in the image layer.

This is inconsistent now. We can move this processing into toolkit operations but that would duplicate the code when adding new toolkits. We need a way to standardize all those tasks in the abstract Image layer and send the arguments already prepared to toolkit operations.

Proposed resolution

The solution to be decided and the issue summary to be updated!

Remaining tasks

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fietserwin’s picture

It is not only "normalizing dimensions, coordinates, etc". It is also processing input as entered on the forms to straight numbers. Think of cropping angle: center-center, center-bottom, or setting a new width: 90%, width - 20. The latter are quite effect specific as the effect determines what may be entered in its form fields.

So, yes there are probably a few calculations that can get moved, but a large part will stay in the effect. Moreover, contrib will come with enhancements, especially enhancing what can be filed in (I guess that the 2nd example currently is not allowed), where should contrib place it? (The answer is: contrib should keep it locally, thus within its own effects.)

But let's see what calculations we can find.

Please postpone working on this until the large API changing issues have landed.

claudiu.cristea’s picture

Status: Active » Needs review
FileSize
20.28 KB
85.86 KB

This is just a proof of concept. Is fixing also #2109375: Provide sane defaults for toolkit operation arguments.

Approach: The image geometry abstract layer is provided using the Symfony event dispatching/subscribing mechanism. Modules (core does it too) are able to subscribe to 3 events: DEFAULTS, PREPARE and ALTER. First I was tempted to implement the standard Drupal hook mechanism but ended with event subscriber/dispatcher.

Modules that want to provide operation arguments defaults or prepare arguments need to add an event subscriber by extending \Drupal\Core\Image\ImageOperationArgumentSubscriberBase.

This patch is build in top of #2073759: Convert toolkit operations to plugins. A combined patch is available for testing purposes.

This needs an agreement/amend or proposal of other approach. Let's discuss this.

Status: Needs review » Needs work

The last submitted patch, image-geometry-2108307-2-combined.patch, failed testing.

claudiu.cristea’s picture

Things that we should take care here, as soon as #2073759: Convert toolkit operations to plugins is in:

  • Figure out a way to document arguments sent to toolkit operations. In #2110499: Unify the call and interfaces of image toolkit operations arguments were collapsed into an associative array making hard to document them. We need a way to force developers to document their type and scope.
  • Convert unit tests to test this new functionality.
  • Drop \Drupal\Component\Utility\Image.
claudiu.cristea’s picture

Issue summary: View changes
Status: Needs work » Closed (duplicate)

The scope of this issues has moved to #2073759: Convert toolkit operations to plugins.

The arguments preprocessing is handled now by ImageToolkitOperationInterface::prepare().

Closing this as duplicate of #2073759: Convert toolkit operations to plugins.