Skip to content

Commit fb19e35

Browse files
committed
Fix mis-clamped focal point crops
1 parent b3af89e commit fb19e35

File tree

3 files changed

+133
-4
lines changed

3 files changed

+133
-4
lines changed

src/imagetransforms/ImageTransform.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class ImageTransform extends \craft\models\ImageTransform
7070
public ?float $gamma = null;
7171

7272
/**
73-
* @var 'auto'|'face'|'left'|'right'|'top'|'bottom'|array{x?: float, y?: float}|null
73+
* @var 'auto'|'face'|'left'|'right'|'top'|'bottom'|array{x?: float, y?: float, mode?: 'remainder'|'box-center'}|null
7474
*/
7575
public string|array|null $gravity = null;
7676

src/imagetransforms/ImageTransformer.php

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,7 @@ public function getTransformUrl(Asset $asset, \craft\models\ImageTransform $imag
5050
$imageTransform = Craft::createObject(ImageTransform::class, [$imageTransform->toArray()]);
5151
}
5252

53-
if ($asset->getHasFocalPoint() && !isset($imageTransform->gravity)) {
54-
$imageTransform->gravity = $asset->getFocalPoint();
55-
}
53+
$this->applyAssetFocalPointGravity($asset, $imageTransform);
5654

5755
$query = Query::fromVariable($imageTransform->toOptions());
5856
$uri = Modifier::wrap(Uri::new($assetUrl))
@@ -66,6 +64,20 @@ public function invalidateAssetTransforms(Asset $asset): void
6664
{
6765
}
6866

67+
protected function applyAssetFocalPointGravity(Asset $asset, ImageTransform $imageTransform): void
68+
{
69+
if (!$asset->getHasFocalPoint() || isset($imageTransform->gravity) || $imageTransform->mode !== 'crop') {
70+
return;
71+
}
72+
73+
$focalPoint = $asset->getFocalPoint();
74+
$imageTransform->gravity = [
75+
'x' => $focalPoint['x'],
76+
'y' => $focalPoint['y'],
77+
'mode' => 'box-center',
78+
];
79+
}
80+
6981
private function sign(UriInterface $uri): UriInterface
7082
{
7183
$data = "{$uri->getPath()}#?{$uri->getQuery()}";

tests/unit/ImageTransformTest.php

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
<?php
2+
3+
namespace craft\cloud\tests\unit;
4+
5+
use Codeception\Test\Unit;
6+
use craft\cloud\imagetransforms\ImageTransform;
7+
use craft\cloud\imagetransforms\ImageTransformer;
8+
use craft\elements\Asset;
9+
10+
class ImageTransformTest extends Unit
11+
{
12+
/**
13+
* @var \UnitTester
14+
*/
15+
protected $tester;
16+
17+
public function testCropModeWithExplicitGravityModePreservesItInOptions(): void
18+
{
19+
$transform = new ImageTransform([
20+
'mode' => 'crop',
21+
'width' => 1200,
22+
'height' => 750,
23+
'gravity' => [
24+
'x' => 0.57,
25+
'y' => 0.7707,
26+
'mode' => 'box-center',
27+
],
28+
]);
29+
30+
$this->assertSame([
31+
'fit' => 'cover',
32+
'gravity' => [
33+
'x' => 0.57,
34+
'y' => 0.7707,
35+
'mode' => 'box-center',
36+
],
37+
'height' => 750,
38+
'width' => 1200,
39+
], $transform->toOptions());
40+
}
41+
42+
public function testCropModeWithoutGravityUsesPositionMapping(): void
43+
{
44+
$transform = new ImageTransform([
45+
'mode' => 'crop',
46+
'position' => 'top-center',
47+
'width' => 1200,
48+
'height' => 750,
49+
]);
50+
51+
$this->assertSame([
52+
'fit' => 'cover',
53+
'gravity' => [
54+
'x' => 0.5,
55+
'y' => 0,
56+
],
57+
'height' => 750,
58+
'width' => 1200,
59+
], $transform->toOptions());
60+
}
61+
62+
public function testFocalPointCropUsesBoxCenteredGravity(): void
63+
{
64+
$asset = $this->makeAssetStub(true, ['x' => 0.57, 'y' => 0.7707]);
65+
$transform = new ImageTransform([
66+
'mode' => 'crop',
67+
'width' => 1200,
68+
'height' => 750,
69+
'position' => 'top-center',
70+
]);
71+
72+
(new TestImageTransformer())->applyFocalPointGravity($asset, $transform);
73+
74+
$this->assertSame([
75+
'x' => 0.57,
76+
'y' => 0.7707,
77+
'mode' => 'box-center',
78+
], $transform->gravity);
79+
}
80+
81+
private function makeAssetStub(bool $hasFocalPoint, array $focalPoint): Asset
82+
{
83+
return new class($hasFocalPoint, $focalPoint) extends Asset {
84+
public function __construct(private bool $hasFocalPoint, private array $focalPoint)
85+
{
86+
parent::__construct();
87+
}
88+
89+
public function getHasFocalPoint(): bool
90+
{
91+
return $this->hasFocalPoint;
92+
}
93+
94+
public function getFocalPoint(bool $asCss = false): array|string|null
95+
{
96+
if ($asCss) {
97+
return ($this->focalPoint['x'] * 100) . '% ' . ($this->focalPoint['y'] * 100) . '%';
98+
}
99+
100+
return $this->focalPoint;
101+
}
102+
103+
public function getMimeType(mixed $transform = null): ?string
104+
{
105+
return 'image/jpeg';
106+
}
107+
};
108+
}
109+
}
110+
111+
class TestImageTransformer extends ImageTransformer
112+
{
113+
public function applyFocalPointGravity(Asset $asset, ImageTransform $imageTransform): void
114+
{
115+
$this->applyAssetFocalPointGravity($asset, $imageTransform);
116+
}
117+
}

0 commit comments

Comments
 (0)