Merge branch 'kaedroho-fill-operation-fixes'

wagtail/wagtailimages/image_operations.py

@@ -1,9 +1,9 @@
 from __future__ import division
 
 import inspect
-import math
 
 from wagtail.wagtailimages.exceptions import InvalidFilterSpecError
+from wagtail.wagtailimages.rect import Rect
 
 
 class Operation(object):
@@ -118,53 +118,17 @@ class FillOperation(Operation):
         crop_y = fp_y - (fp_v - 0.5) * crop_height
 
         # Convert crop box into rect
-        left = crop_x - crop_width / 2
-        top = crop_y - crop_height / 2
-        right = crop_x + crop_width / 2
-        bottom = crop_y + crop_height / 2
+        rect = Rect.from_point(crop_x, crop_y, crop_width, crop_height)
 
         # Make sure the entire focal point is in the crop box
         if focal_point is not None:
-            if left > focal_point.left:
-                right -= left - focal_point.left
-                left = focal_point.left
-
-            if top > focal_point.top:
-                bottom -= top - focal_point.top
-                top = focal_point.top
-
-            if right < focal_point.right:
-                left += focal_point.right - right
-                right = focal_point.right
-
-            if bottom < focal_point.bottom:
-                top += focal_point.bottom - bottom
-                bottom = focal_point.bottom
+            rect = rect.move_to_cover(focal_point)
 
         # Don't allow the crop box to go over the image boundary
-        if left < 0:
-            right -= left
-            left = 0
-
-        if top < 0:
-            bottom -= top
-            top = 0
-
-        if right > image_width:
-            left -= right - image_width
-            right = image_width
-
-        if bottom > image_height:
-            top -= bottom - image_height
-            bottom = image_height
+        rect = rect.move_to_clamp(Rect(0, 0, image_width, image_height))
 
         # Crop!
-        willow.crop((
-            int(math.floor(left)),
-            int(math.floor(top)),
-            int(math.ceil(right)),
-            int(math.ceil(bottom))
-        ))
+        willow.crop(rect.round())
 
         # Get scale for resizing
         # The scale should be the same for both the horizontal and

wagtail/wagtailimages/rect.py

@@ -1,45 +1,172 @@
 from __future__ import division
 
+import math
+
+
+class Vector(object):
+    def __init__(self, x, y):
+        self.x = x
+        self.y = y
+
+    def __iter__(self):
+        return iter((self.x, self.y))
+
+    def __getitem__(self, key):
+        return (self.x, self.y)[key]
+
+    def __eq__(self, other):
+        return tuple(self) == tuple(other)
+
+    def __ne__(self, other):
+        return not (self == other)
+
+    def __repr__(self):
+        return 'Vector(x: %d, y: %d)' % (
+            self.x, self.y
+        )
+
 
 class Rect(object):
     def __init__(self, left, top, right, bottom):
-        self.left = int(left)
-        self.top = int(top)
-        self.right = int(right)
-        self.bottom = int(bottom)
+        self.left = left
+        self.top = top
+        self.right = right
+        self.bottom = bottom
+
+    def _get_size(self):
+        return Vector(self.right - self.left, self.bottom - self.top)
+
+    def _set_size(self, new_size):
+        centroid = self.centroid
+        self.left = centroid[0] - new_size[0] / 2
+        self.right = centroid[0] + new_size[0] / 2
+        self.top = centroid[1] - new_size[1] / 2
+        self.bottom = centroid[1] + new_size[1] / 2
+
+    size = property(_get_size, _set_size)
+
+    @property
+    def width(self):
+        return self.size.x
+
+    @property
+    def height(self):
+        return self.size.y
+
+    def _get_centroid(self):
+        return Vector((self.left + self.right) / 2, (self.top + self.bottom) / 2)
+
+    def _set_centroid(self, new_centroid):
+        size = self.size
+        self.left = new_centroid[0] - size[0] / 2
+        self.right = new_centroid[0] + size[0] / 2
+        self.top = new_centroid[1] - size[1] / 2
+        self.bottom = new_centroid[1] + size[1] / 2
+
+    centroid = property(_get_centroid, _set_centroid)
+
+    @property
+    def x(self):
+        return self.centroid.x
+
+    @property
+    def y(self):
+        return self.centroid.y
+
+    @property
+    def centroid_x(self):
+        # Included for backwards compatibility
+        return self.centroid.x
+
+    @property
+    def centroid_y(self):
+        # Included for backwards compatibility
+        return self.centroid.y
+
+    def as_tuple(self):
+        # No longer needed, this class should behave like a tuple
+        # Included for backwards compatibility
+        return self.left, self.top, self.right, self.bottom
+
+    def clone(self):
+        return type(self)(self.left, self.top, self.right, self.bottom)
+
+    def round(self):
+        """
+        Returns a new rect with all attributes rounded to integers
+        """
+        clone = self.clone()
+
+        # Round down left and top
+        clone.left = int(math.floor(clone.left))
+        clone.top = int(math.floor(clone.top))
+
+        # Round up right and bottom
+        clone.right = int(math.ceil(clone.right))
+        clone.bottom = int(math.ceil(clone.bottom))
+
+        return clone
+
+    def move_to_clamp(self, other):
+        """
+        Moves this rect so it is completely covered by the rect in "other" and
+        returns a new Rect instance.
+        """
+        other = Rect(*other)
+        clone = self.clone()
+
+        if clone.left < other.left:
+            clone.right -= clone.left - other.left
+            clone.left = other.left
+
+        if clone.top < other.top:
+            clone.bottom -= clone.top - other.top
+            clone.top = other.top
+
+        if clone.right > other.right:
+            clone.left -= clone.right - other.right
+            clone.right = other.right
+
+        if clone.bottom > other.bottom:
+            clone.top -= clone.bottom - other.bottom
+            clone.bottom = other.bottom
+
+        return clone
+
+    def move_to_cover(self, other):
+        """
+        Moves this rect so it completely covers the rect specified in the
+        "other" parameter and returns a new Rect instance.
+        """
+        other = Rect(*other)
+        clone = self.clone()
+
+        if clone.left > other.left:
+            clone.right -= clone.left - other.left
+            clone.left = other.left
+
+        if clone.top > other.top:
+            clone.bottom -= clone.top - other.top
+            clone.top = other.top
+
+        if clone.right < other.right:
+            clone.left += other.right - clone.right
+            clone.right = other.right
+
+        if clone.bottom < other.bottom:
+            clone.top += other.bottom - clone.bottom
+            clone.bottom = other.bottom
+
+        return clone
+
+    def __iter__(self):
+        return iter((self.left, self.top, self.right, self.bottom))
 
     def __getitem__(self, key):
         return (self.left, self.top, self.right, self.bottom)[key]
 
-    @property
-    def width(self):
-        return self.right - self.left
-
-    @property
-    def height(self):
-        return self.bottom - self.top
-
-    @property
-    def size(self):
-        return self.width, self.height
-
-    @property
-    def centroid_x(self):
-        return (self.left + self.right) / 2
-
-    @property
-    def centroid_y(self):
-        return (self.top + self.bottom) / 2
-
-    @property
-    def centroid(self):
-        return self.centroid_x, self.centroid_y
-
-    def as_tuple(self):
-        return self.left, self.top, self.right, self.bottom
-
     def __eq__(self, other):
-        return self.as_tuple() == other.as_tuple()
+        return tuple(self) == tuple(other)
 
     def __ne__(self, other):
         return not (self == other)

wagtail/wagtailimages/tests/tests.py

@@ -10,7 +10,7 @@ from taggit.forms import TagField, TagWidget
 from wagtail.tests.testapp.models import CustomImage
 from wagtail.tests.utils import WagtailTestUtils
 from wagtail.wagtailimages.utils import generate_signature, verify_signature
-from wagtail.wagtailimages.rect import Rect
+from wagtail.wagtailimages.rect import Rect, Vector
 from wagtail.wagtailimages.formats import Format, get_image_format, register_image_format
 from wagtail.wagtailimages.models import Image as WagtailImage
 from wagtail.wagtailimages.forms import get_image_form
@@ -240,16 +240,40 @@ class TestRect(TestCase):
 
     def test_size(self):
         rect = Rect(100, 150, 200, 350)
+        self.assertIsInstance(rect.size, Vector)
         self.assertEqual(rect.size, (100, 200))
         self.assertEqual(rect.width, 100)
         self.assertEqual(rect.height, 200)
 
+    def test_set_size_with_tuple(self):
+        rect = Rect(100, 150, 200, 350)
+        rect.size = (200, 400)
+        self.assertEqual(rect, (50, 50, 250, 450))
+
+    def test_set_size_with_vector(self):
+        rect = Rect(100, 150, 200, 350)
+        rect.size = Vector(200, 400)
+        self.assertEqual(rect, (50, 50, 250, 450))
+
     def test_centroid(self):
         rect = Rect(100, 150, 200, 350)
+        self.assertIsInstance(rect.centroid, Vector)
         self.assertEqual(rect.centroid, (150, 250))
+        self.assertEqual(rect.x, 150)
+        self.assertEqual(rect.y, 250)
         self.assertEqual(rect.centroid_x, 150)
         self.assertEqual(rect.centroid_y, 250)
 
+    def test_set_centroid_with_tuple(self):
+        rect = Rect(100, 150, 200, 350)
+        rect.centroid = (500, 500)
+        self.assertEqual(rect, (450, 400, 550, 600))
+
+    def test_set_centroid_with_vector(self):
+        rect = Rect(100, 150, 200, 350)
+        rect.centroid = Vector(500, 500)
+        self.assertEqual(rect, (450, 400, 550, 600))
+
     def test_repr(self):
         rect = Rect(100, 150, 200, 250)
         self.assertEqual(repr(rect), "Rect(left: 100, top: 150, right: 200, bottom: 250)")