Add an "Perspective" effect

tagging

Very very nice.

Some comments:

1. Aspect coefficient - in the effect this is a 0-1 index, but in the summary template this is expressed as %. Can we go fully one way or the other. Personally I would go for 0-100%, since that's what we do in most (all?) other cases in this module, but this is just my opinion.
2. I would suggest to add a 'background color' as an effect parameter and into the image operations. Like e.g. the core rotate effect. Currently, GIF and JPEG have black color in the background after the transform. Converting to PNG may not be an option in some cases. Something similar to SetCanvasImageEffect:
   

   
       // Canvas color.
       $form['canvas_color'] = [
         '#type' => 'image_effects_color',
         '#title' => $this->t('Canvas color'),
         '#allow_null' => TRUE,
         '#allow_opacity' => TRUE,
         '#description'  => $this->t("This will have the effect of adding colored (or transparent) margins around the image."),
         '#default_value' => $this->configuration['canvas_color'],
       ];
   
   

+++ b/config/schema/image_effects.schema.yml
@@ -370,3 +370,23 @@ image.effect.image_effects_interlace:
+    distortion:
+      type: float
+      label: 'Distortion percentage of the vanish'

'integer' instead of 'float'? Same for 'opposite_distortion'. In the effect form the step is 1 so there won't be decimals.

+++ b/src/Plugin/ImageToolkit/Operation/gd/Perspective.php
@@ -0,0 +1,155 @@
+    // Increasing the source image width/height to reduce distortion
+    $color = imagecolorallocatealpha($original_resource, 0, 0, 0, 127);
+    $multiplier = 5;
+    $multiImage = imagecreatetruecolor($width * $multiplier, $height * $multiplier);

This is a memory killer :(
My suggestion would be at least to get the new resource via the 'create_new' toolkit op (see below copied from SetCanvas), instead of using imagecreatetruecolor():

    $data = array(
      'width' => $arguments['width'],
      'height' => $arguments['height'],
      'extension' => image_type_to_extension($this->getToolkit()->getType(), FALSE),
      'transparent_color' => $this->getToolkit()->getTransparentColor(),
      'is_temp' => TRUE,
    );
    if (!$this->getToolkit()->apply('create_new', $data)) {
      return FALSE;
    }

This would have the benefit of (a) get proper transparency support for GIF, and (b) memory availability check once #2583041: GD toolkit & operations should catch \Throwable to fail gracefully in case of errors will be committed. Same for other instances of calling that imagecreatetruecolor() - maybe not all need to go through 'create_new', though.

+++ b/src/Tests/ImageEffectsPerspectiveTest.php
@@ -0,0 +1,139 @@
+      // For the Imagemagick toolkit, toolkit should return
+      // image dimensions after applying the operation, but before
+      // saving.
+      $image = $image_factory->get($original_uri, 'gd');
+

aren't you testing with GD instead with Imagemagick here?

I am not sure about this effect adding a background image. effects should be simple and do 1 thing. Rotate also doesn't allow for a background image. So I would propose to only allow for a backgrond color (like rotate) but including a transparency factor (thus an RGBA value, not only an RGB value). So the background can be set to fully transparent. Another effect in the image style can then add a background image.

Note: even simpler and better would be to always use fully transparent to allow for a succeeding canvas effect to add a background color or image. This would also be better for rotate but as that is in core it cannot depend on an effect in contrib.

Thanks!

+++ b/config/schema/image_effects.schema.yml
@@ -370,3 +370,26 @@ image.effect.image_effects_interlace:
+image.effect.image_effects_perspective:
+  type: mapping
+  label: 'Perspective image effect'
+  mapping:
+    distortion:
+      type: float
+      label: 'Distortion percentage of the vanish'
+    vanish:
+      type: string
+      label: 'The relative position of the vanishing point to the source image'
+    symmetry:
+      type: string
+      label: 'Symmetry of image perspective'
+    opposite_distortion:
+      type: float
+      label: 'Distortion for opposite side'
+    aspect_coefficient:
+      type: float
+      label: 'Width/height Aspect ratio coefficient'
+    canvas_color:
+      type: string
+      label: 'RGBA color of the canvas under image after transform'

All floats should be integer.

Please make sure that if we return early because of any failure, we are not left over with orphaned resources that will take up memory. Unfortunately PHP is not helping much here, we need to do the tidying up ourselves - we enter in the operation with one resource and we should leave with one resource, otherwise they will not be cleaned up. See in core, the Convert, Crop andResize operations. Examples:

+++ b/src/Plugin/ImageToolkit/Operation/gd/Perspective.php
@@ -0,0 +1,192 @@
+    // Creating new large image for apply perspective effect
+    $data = array(
+      'width' => $width,
+      'height' => $height,
+      'extension' => image_type_to_extension($this->getToolkit()->getType(), FALSE),
+      'transparent_color' => $this->getToolkit()->getTransparentColor(),
+      'is_temp' => TRUE,
+    );
+    if (!$this->getToolkit()->apply('create_new', $data)) {
+      return FALSE;
+    }
+

If it fails here, we should restore the $original_resource at this point and destroy $increasing_resource

+++ b/src/Plugin/ImageToolkit/Operation/gd/Perspective.php
@@ -0,0 +1,192 @@
+    // Fill image underlay with required color.
+    $data = array(
+      'rectangle' => new PositionedRectangle($width, $height),
+      'fill_color' => $arguments['canvas_color'],
+    );
+    if (!$this->getToolkit()->apply('draw_rectangle', $data)) {
+      return FALSE;
+    }
+

Same here, restore the original resource and destroy any temp one before returning

+++ b/src/Plugin/ImageToolkit/Operation/gd/Perspective.php
@@ -0,0 +1,192 @@
+    if (!$this->getToolkit()->apply('create_new', $data)) {
+      return FALSE;
+    }
+

same here.

I'll do a round of manual testing later.

#7: in fact @lebster added option for background color, not image. It's just incorrectly stated in #6.

EDIT - and in fact the background color is just an option, background will be transparent by default.

Tested manually with an image style with the following effects:

• Scale 220×220
• Perspective transform - asymmetrical, Vanishing point position: left, Top distortion: 25%, Bottom distortion: 10%, Aspect coefficient: 85%, Color: #FFFF00 opacity: 20%

See results on different toolkit/image type:

GD

PNG

JPG

GIF

ImageMagick

PNG

JPG

GIF

Looking at results, it seems that asymmetrical perspective is reversed in GD vs ImageMagick. Also, background color in ImageMagick GIF derivative is not present (?).

EDIT - also, dimensions differ in GD vs Imagemagick - is this a sign of the test failing to capture that?

For reference and testing, I am attaching the three different image files used as source.

Added related issue in imagecache_actions queue.

Getting close, thanks.

We also changed the method of adding the background for the images. Before we used "canvas_color" that was an element of 'image_effects_color', the allows you to specify the color and opacity for the background (this item is used to set canvas effect). Now we just specify a background color in hex format, or leave blank for a transparent background (this item is used to rotate the effect).

I think we can still use the 'image_effects_color' form element here instead - this is just about selecting the color, not about its processing in the image toolkit operations.

Reason to change the element - ambiguous behavior for different types of images and various toolkits.

This is the real problem I think, and a bug in the 'Set canvas' effect. I will open an issue for this. When we have that solved I think we can apply the same concept here.

Created issue #2772619: Set canvas: inconsistent behaviour with different image types/toolkits when background color is partially transparent.

By mistake, I removed reference to the similar issue in ImageCache Actions module, #2760121: Add a "Perspective" effect. Adding back.

In the D7 issue queue we are still discussing details of the effect, what parameters are really necessary, better help texts, etc. Let's wait for the final result over there and then port the resulting effect to here.

It's a pity that this issue got stalled.

I think most of the concern is based on defining a background color for the outer part of the transformed image.

Can we revert the part that introduces the background color, leave that always transparent as suggested by @fietserwin in #7, and leave the background color setting to the Set Canvas effect and its issue #2772619: Set canvas: inconsistent behaviour with different image types/toolkits when background color is partially transparent?

Most probably #15 needs a reroll anyway.

It is not only the background, in D7 it is also the resize part. In other words it is coming down to what should constitute an effect and what should be done by chaining effects. IMO an effect should be as atomic as possible, thus this effect should not contain the resize nor the background.

Ah OK then, let's fix in Imagecache Actions first. Thanks.

It's a real shame this issue got postponed. If this module doesn't like the way this is done, then I'd be happy to download an additional module. I suggest promoting the Image Effects: Perspective to an official module.

Coincidence or not, but today I finally got an answer from @lebster about the imagecache_actions version of this effect and I plan to commit that in the coming days. After that adding it over here - with the same feature set - shouldn't be to difficult anymore.

@fietserwin good to know, thanks guys for all the hard work on this.

Closing any stale issue with latest comment over 5 years old.