Fix leaky "Weird Al" gravity.

Author: timkelty

src/imagetransforms/ImageTransformer.php

@@ -46,9 +46,7 @@ public function getTransformUrl(Asset $asset, \craft\models\ImageTransform $imag
 
         // ImageTransform DI will not work on Craft 4, so we convert the object.
         // @see https://github.com/craftcms/cms/pull/15646
-        if (!$imageTransform instanceof ImageTransform) {
-            $imageTransform = Craft::createObject(ImageTransform::class, [$imageTransform->toArray()]);
-        }
+        $imageTransform = Craft::createObject(ImageTransform::class, [$imageTransform->toArray()]);
 
         $this->applyAssetFocalPointGravity($asset, $imageTransform);
 
@@ -66,53 +64,11 @@ public function invalidateAssetTransforms(Asset $asset): void
 
     protected function applyAssetFocalPointGravity(Asset $asset, ImageTransform $imageTransform): void
     {
-        if (!$asset->getHasFocalPoint() || isset($imageTransform->gravity) || $imageTransform->mode !== 'crop') {
-            return;
-        }
-
-        $sourceWidth = $asset->getWidth();
-        $sourceHeight = $asset->getHeight();
-        $targetWidth = $imageTransform->width;
-        $targetHeight = $imageTransform->height;
-
-        if (!$sourceWidth || !$sourceHeight || !$targetWidth || !$targetHeight) {
-            $imageTransform->gravity = $asset->getFocalPoint();
+        if (!$asset->getHasFocalPoint() || isset($imageTransform->gravity)) {
             return;
         }
 
-        $focalPoint = $asset->getFocalPoint();
-        $factor = min($sourceWidth / $targetWidth, $sourceHeight / $targetHeight);
-        $newWidth = round($sourceWidth / $factor);
-        $newHeight = round($sourceHeight / $factor);
-
-        $centerX = $newWidth * $focalPoint['x'];
-        $centerY = $newHeight * $focalPoint['y'];
-        $x1 = $centerX - $targetWidth / 2;
-        $y1 = $centerY - $targetHeight / 2;
-        $x2 = $x1 + $targetWidth;
-        $y2 = $y1 + $targetHeight;
-
-        if ($x1 < 0) {
-            $x2 -= $x1;
-            $x1 = 0;
-        }
-        if ($y1 < 0) {
-            $y2 -= $y1;
-            $y1 = 0;
-        }
-        if ($x2 > $newWidth) {
-            $x1 -= ($x2 - $newWidth);
-            $x2 = $newWidth;
-        }
-        if ($y2 > $newHeight) {
-            $y1 -= ($y2 - $newHeight);
-            $y2 = $newHeight;
-        }
-
-        $imageTransform->gravity = [
-            'x' => $newWidth > $targetWidth ? $x1 / ($newWidth - $targetWidth) : $focalPoint['x'],
-            'y' => $newHeight > $targetHeight ? $y1 / ($newHeight - $targetHeight) : $focalPoint['y'],
-        ];
+        $imageTransform->gravity = $asset->getFocalPoint();
     }
 
     private function sign(UriInterface $uri): UriInterface

tests/unit/ImageTransformTest.php

@@ -75,6 +75,29 @@ public function testCropModeMapsFocalPointToClampedCropOriginGravity(): void
         ], $transform->gravity, 0.0001);
     }
 
+    public function testGetTransformUrlDoesNotLeakGravityBetweenAssets(): void
+    {
+        $transform = new ImageTransform([
+            'mode' => 'crop',
+            'position' => 'top-center',
+            'width' => 1200,
+            'height' => 750,
+        ]);
+
+        $firstAsset = $this->makeUrlAssetStub(1, 'first.jpg', 3402, 4253, ['x' => 0.57, 'y' => 0.7707]);
+        $secondAsset = $this->makeUrlAssetStub(2, 'second.jpg', 3402, 4253, ['x' => 0.4631, 'y' => 0.308]);
+
+        $transformer = new UrlTestImageTransformer();
+
+        $firstUrl = $transformer->getTransformUrl($firstAsset, $transform, true);
+        $secondUrl = $transformer->getTransformUrl($secondAsset, $transform, true);
+
+        $this->assertStringContainsString('gravity%5Bx%5D=0.57', $firstUrl);
+        $this->assertStringContainsString('gravity%5By%5D=0.9999999999999992', $firstUrl);
+        $this->assertStringContainsString('gravity%5Bx%5D=0.4631', $secondUrl);
+        $this->assertStringNotContainsString('gravity%5Bx%5D=0.57', $secondUrl);
+    }
+
     private function makeAssetStub(int $width, int $height, array $focalPoint): Asset
     {
         return new class($width, $height, $focalPoint) extends Asset {
@@ -109,6 +132,62 @@ public function getHeight(mixed $transform = null): ?int
         };
     }
 
+    private function makeUrlAssetStub(int $id, string $filename, int $width, int $height, array $focalPoint): Asset
+    {
+        return new class($id, $filename, $width, $height, $focalPoint) extends Asset {
+            public function __construct(
+                int $id,
+                private string $filenameValue,
+                private int $widthValue,
+                private int $heightValue,
+                private array $focalPointValue,
+            ) {
+                parent::__construct();
+                $this->id = $id;
+                $this->kind = self::KIND_IMAGE;
+            }
+
+            public function getHasFocalPoint(): bool
+            {
+                return true;
+            }
+
+            public function getFocalPoint(bool $asCss = false): array|string|null
+            {
+                if ($asCss) {
+                    return ($this->focalPointValue['x'] * 100) . '% ' . ($this->focalPointValue['y'] * 100) . '%';
+                }
+
+                return $this->focalPointValue;
+            }
+
+            public function getWidth(array|string|\craft\models\ImageTransform $transform = null): ?int
+            {
+                return $this->widthValue;
+            }
+
+            public function getHeight(mixed $transform = null): ?int
+            {
+                return $this->heightValue;
+            }
+
+            public function getFilename(bool $withExtension = true): string
+            {
+                return $this->filenameValue;
+            }
+
+            public function getPath(): ?string
+            {
+                return 'tests/' . $this->filenameValue;
+            }
+
+            public function getMimeType(mixed $transform = null): ?string
+            {
+                return 'image/jpeg';
+            }
+        };
+    }
+
 }
 
 class TestImageTransformer extends ImageTransformer
@@ -118,3 +197,14 @@ public function applyFocalPointGravity(Asset $asset, ImageTransform $imageTransf
         $this->applyAssetFocalPointGravity($asset, $imageTransform);
     }
 }
+
+class UrlTestImageTransformer extends ImageTransformer
+{
+    public function getTransformUrl(Asset $asset, \craft\models\ImageTransform $imageTransform, bool $immediately): string
+    {
+        $imageTransform = \Craft::createObject(ImageTransform::class, [$imageTransform->toArray()]);
+        $this->applyAssetFocalPointGravity($asset, $imageTransform);
+
+        return (string) \League\Uri\Components\Query::fromVariable($imageTransform->toOptions());
+    }
+}