Skip to content

Commit 3814dec

Browse files
author
irishdan
committed
Fix faulty crop rectabngle calculation
1 parent 05b7224 commit 3814dec

File tree

3 files changed

+98
-46
lines changed

3 files changed

+98
-46
lines changed

‎ImageProcessing/FocusCropDataCalculator.php

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@ class FocusCropDataCalculator
3333
*/
3434
public function __construct($cropCoordinates, $focusCoordinates, $styleWidth, $styleHeight)
3535
{
36-
$this->cropCoordinates = $cropCoordinates;
36+
$this->cropCoordinates = $cropCoordinates;
3737
$this->focusCoordinates = $focusCoordinates;
38-
$this->styleWidth = $styleWidth;
39-
$this->styleHeight = $styleHeight;
38+
$this->styleWidth = $styleWidth;
39+
$this->styleHeight = $styleHeight;
4040
}
4141

4242
/**
@@ -54,7 +54,7 @@ public function getFocusCropData()
5454
// The sides of the image will be cropped.
5555
// 2: The style rectangle fits inside the crop rectangle horizontally.
5656
// The top and bottom of the image will be cropped.
57-
// 3: The style rectangle fits inside the crop rectangle exactly.
57+
// 3: The style rectangle fits inside the crop rectangle perfectly.
5858
// no cropping in required
5959
//
6060
// To determine which type of cropping should be used, the aspect-ratio of the image/crop rectangle ($imageAspectRatio)
@@ -66,29 +66,28 @@ public function getFocusCropData()
6666
list($x1, $y1, $x2, $y2) = $this->cropCoordinates;
6767
$this->geometry = new CoordinateGeometry($x1, $y1, $x2, $y2);
6868

69-
$newWidth = $this->geometry->axisLength('x');
69+
$newWidth = $this->geometry->axisLength('x');
7070
$newHeight = $this->geometry->axisLength('y');
7171

7272
// Find out what type of style crop we are dealing with.
73-
$imageAspectRatio = $this->geometry->getAspectRatio();
74-
73+
// @TODO: Checkout the geometry calculation.
74+
// $imageAspectRatio = $this->geometry->getAspectRatio();
75+
$imageAspectRatio = $newWidth / $newHeight;
7576
$styleAspectRatio = $this->styleWidth / $this->styleHeight;
7677

7778
if ($imageAspectRatio > $styleAspectRatio) {
7879
$axis = 'x';
7980
}
81+
else if ($imageAspectRatio < $styleAspectRatio) {
82+
$axis = 'y';
83+
}
8084
else {
81-
if ($imageAspectRatio < $styleAspectRatio) {
82-
$axis = 'y';
83-
}
84-
else {
85-
return [
86-
'width' => $newWidth,
87-
'height' => $newHeight,
88-
'x' => $this->cropCoordinates[0],
89-
'y' => $this->cropCoordinates[1],
90-
];
91-
}
85+
return [
86+
'width' => $newWidth,
87+
'height' => $newHeight,
88+
'x' => $this->cropCoordinates[0],
89+
'y' => $this->cropCoordinates[1],
90+
];
9291
}
9392

9493
return $this->calculateAxisCropData($axis, $this->cropCoordinates, $styleAspectRatio, $newWidth, $newHeight);
@@ -105,8 +104,25 @@ public function getFocusCropData()
105104
*/
106105
protected function calculateAxisCropData($axis = 'x', $cropCoordinates, $aspectRatio, $newWidth, $newHeight)
107106
{
108-
$cropWidth = ($axis == 'y') ? $newWidth : $newHeight * $aspectRatio;
109-
$cropHeight = ($axis == 'y') ? $newWidth / $aspectRatio : $newHeight;
107+
if ($axis !== 'x' && $axis !== 'y') {
108+
throw new \InvalidArgumentException('$axis can only have a value of x or y. ' . $axis . ' given');
109+
}
110+
111+
if ($axis == 'x') {
112+
$cropHeight = $newHeight;
113+
114+
// How many times the style height goes into the new height
115+
$scaleFactor = $newHeight / $this->styleHeight;
116+
$cropWidth = $this->styleWidth * $scaleFactor;
117+
}
118+
else {
119+
$cropWidth = $newWidth;
120+
121+
// How many times the style height goes into the new height
122+
$scaleFactor = $newWidth / $this->styleWidth;
123+
$cropHeight = $this->styleHeight * $scaleFactor;
124+
}
125+
$data['scale_factor'] = $scaleFactor;
110126

111127
$cropXOffset = ($axis == 'y') ? $cropCoordinates[0] : $this->getFloatingOffset(
112128
'x',
@@ -194,7 +210,7 @@ protected function findFocusOffset($axis = 'x', $cropLength)
194210
// Offsetting on either the x or the y axis.
195211
// Subtract the crop rectangle.
196212
$focusNear = $this->getFocusPointForAxis($axis, 'near');
197-
$focusFar = $this->getFocusPointForAxis($axis, 'far');
213+
$focusFar = $this->getFocusPointForAxis($axis, 'far');
198214

199215
$focusLength = $focusFar - $focusNear;
200216
$focusCenter = round(($focusNear + $focusFar) / 2);
@@ -232,7 +248,7 @@ protected function getOptimalOffset(array $validOffsets)
232248
if (!empty($validOffsets)) {
233249
asort($validOffsets);
234250
$offsets = array_keys($validOffsets);
235-
$offset = reset($offsets);
251+
$offset = reset($offsets);
236252
}
237253

238254
return $offset;
@@ -248,8 +264,8 @@ protected function getOptimalOffset(array $validOffsets)
248264
*/
249265
protected function getValidOffsets($focusNear, $focusFar, $cropLength, $imageLength)
250266
{
251-
$nearGap = $focusNear;
252-
$farGap = $imageLength - $focusFar;
267+
$nearGap = $focusNear;
268+
$farGap = $imageLength - $focusFar;
253269
$offFactor = $nearGap / $farGap;
254270

255271
// Will need the maximum and minimum offset also.
@@ -262,12 +278,12 @@ protected function getValidOffsets($focusNear, $focusFar, $cropLength, $imageLen
262278
// Need a factor of near / far to compare to offFactor.
263279
// Closest to that wins.
264280
$near = $focusNear - $i;
265-
$far = ($i + $cropLength) - $focusFar;
281+
$far = ($i + $cropLength) - $focusFar;
266282
if ($near != 0 && $far != 0) {
267283
$optimalFactor = ($near / $far) / $offFactor;
268284
$optimalFactor = abs($optimalFactor);
269285

270-
$theTest = abs($optimalFactor - 1);
286+
$theTest = abs($optimalFactor - 1);
271287
$validOffsets[$i] = $theTest;
272288
}
273289
}

‎Tests/ImageProcessing/CoordinateGeometryTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ public function testAxisLength()
2525
{
2626
$this->assertEquals(80, $this->geometry->axisLength('x'));
2727
$this->assertEquals(80, $this->geometry->axisLength('y'));
28+
29+
$this->geometry->setPoints(283, 397, 991, 1289);
30+
$this->assertEquals(708, $this->geometry->axisLength('x'));
31+
$this->assertEquals(892, $this->geometry->axisLength('y'));
2832
}
2933

3034
public function testScaleSize()

‎Tests/ImageProcessing/FocusCropDataCalculatorTest.php

Lines changed: 52 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,32 +10,64 @@
1010

1111
namespace IrishDan\ResponsiveImageBundle\Tests\ImageProcessing;
1212

13-
1413
use IrishDan\ResponsiveImageBundle\ImageProcessing\FocusCropDataCalculator;
1514
use IrishDan\ResponsiveImageBundle\Tests\ResponsiveImageTestCase;
1615

1716
class FocusCropDataCalculatorTest extends ResponsiveImageTestCase
1817
{
19-
public function testGetFocusCropData()
18+
public function testGetFocusCropDataIsValid()
2019
{
21-
$focusOffsetFinder = new FocusCropDataCalculator(
22-
[10, 10, 90, 90],
23-
[40, 40, 60, 60],
24-
30,
25-
60
26-
);
27-
28-
$focusCropData = $focusOffsetFinder->getFocusCropData();
29-
30-
$this->assertArrayHasKey('width', $focusCropData);
31-
$this->assertArrayHasKey('height', $focusCropData);
32-
$this->assertArrayHasKey('x', $focusCropData);
33-
$this->assertArrayHasKey('y', $focusCropData);
34-
35-
$this->assertTrue($focusCropData['width'] < 80);
36-
$this->assertTrue($focusCropData['height'] >= 80);
37-
$this->assertTrue($focusCropData['x'] >= 10);
38-
$this->assertTrue($focusCropData['y'] >= 10);
20+
$testData = [
21+
[
22+
'crop_coordinates' => [10, 10, 100, 100],
23+
'focus_coordinates' => [10, 10, 90, 90],
24+
],
25+
[
26+
'crop_coordinates' => [283, 397, 991, 1289],
27+
'focus_coordinates' => [491, 514, 659, 709]
28+
],
29+
[
30+
'crop_coordinates' => [227, 291, 2826, 2114],
31+
'focus_coordinates' => [1209, 634, 1743, 1195],
32+
],
33+
];
34+
35+
$testStyles = [
36+
[30, 60],
37+
[40, 50],
38+
[170, 240],
39+
[240, 170],
40+
];
41+
42+
foreach ($testData as $imageCoordinates) {
43+
foreach ($testStyles as $style) {
44+
$focusOffsetFinder = new FocusCropDataCalculator(
45+
$imageCoordinates['crop_coordinates'],
46+
$imageCoordinates['focus_coordinates'],
47+
$style[0],
48+
$style[1]
49+
);
50+
51+
$focusCropData = $focusOffsetFinder->getFocusCropData();
52+
53+
$this->assertArrayHasKey('width', $focusCropData);
54+
$this->assertArrayHasKey('height', $focusCropData);
55+
$this->assertArrayHasKey('x', $focusCropData);
56+
$this->assertArrayHasKey('y', $focusCropData);
57+
58+
$width = round($imageCoordinates['crop_coordinates'][2]) - round($imageCoordinates['crop_coordinates'][0]); // x2 - x1
59+
$height = round($imageCoordinates['crop_coordinates'][3]) - round($imageCoordinates['crop_coordinates'][1]); // y2 - y1
60+
61+
$message = 'Too wide: ' . $focusCropData['width'] . '<=' . $width;
62+
$this->assertTrue($focusCropData['width'] <= $width, $message);
63+
64+
$message = 'Too high: ' . (int)round($focusCropData['height']) . ' <= ' . $height;
65+
$this->assertTrue((int)round($focusCropData['height']) <= (int)$height, $message);
66+
67+
$this->assertTrue($focusCropData['x'] >= $imageCoordinates['crop_coordinates'][0]); // x1
68+
$this->assertTrue($focusCropData['y'] >= $imageCoordinates['crop_coordinates'][1]); // y1
69+
}
70+
}
3971
}
4072

4173
public function testGetFocusCropDataWithVerticalStyle()

0 commit comments

Comments
 (0)