ref(snapshots): Make diffing logic more strict + better handle different sized images when comparing

Author: NicoHinderling

src/sentry/preprod/snapshots/image_diff/compare.py

@@ -13,7 +13,13 @@
 
 logger = logging.getLogger(__name__)
 
-DIFF_THRESHOLD = 0
+# odiff color-distance sensitivity: pixels within this threshold are treated
+# as identical. 0.01 tolerates sub-pixel rendering variance (anti-aliasing,
+# font smoothing) while still catching meaningful visual changes.
+#
+# This is NOT a minimum % changed value, but
+# rather adjusts the sensitivity of pixel change detection.
+ODIFF_SENSITIVITY_DIFF_THRESHOLD = 0.01
 
 
 def _as_image(source: bytes | Image.Image) -> Image.Image:
@@ -28,6 +34,14 @@ def _as_image(source: bytes | Image.Image) -> Image.Image:
     return source
 
 
+def _pad_to(img: Image.Image, width: int, height: int) -> Image.Image:
+    if img.size == (width, height):
+        return img
+    padded = Image.new("RGBA", (width, height), (0, 0, 0, 0))
+    padded.paste(img, (0, 0))
+    return padded
+
+
 def _mask_from_diff_output(output_path: Path) -> Image.Image:
     with Image.open(output_path) as img:
         rgba = img.convert("RGBA")
@@ -88,6 +102,8 @@ def _compare_single_pair(
 ) -> DiffResult | None:
     before_img: Image.Image | None = None
     after_img: Image.Image | None = None
+    before_padded: Image.Image | None = None
+    after_padded: Image.Image | None = None
     diff_mask: Image.Image | None = None
     try:
         before_img = _as_image(before)
@@ -97,20 +113,22 @@ def _compare_single_pair(
         max_w = max(bw, aw)
         max_h = max(bh, ah)
 
+        before_padded = _pad_to(before_img, max_w, max_h)
+        after_padded = _pad_to(after_img, max_w, max_h)
+
         before_path = tmpdir_path / f"before_{idx}.png"
         after_path = tmpdir_path / f"after_{idx}.png"
-        before_img.save(before_path, "PNG")
-        after_img.save(after_path, "PNG")
+        before_padded.save(before_path, "PNG")
+        after_padded.save(after_path, "PNG")
 
         output_path = tmpdir_path / f"diff_{idx}.png"
         resp = server.compare(
             before_path,
             after_path,
             output_path,
-            threshold=DIFF_THRESHOLD,
+            threshold=ODIFF_SENSITIVITY_DIFF_THRESHOLD,
             antialiasing=True,
             outputDiffMask=True,
-            failOnLayoutDiff=False,
         )
         changed_pixels = resp.diffCount or 0
         total_pixels = max_w * max_h
@@ -121,10 +139,6 @@ def _compare_single_pair(
             raise RuntimeError(f"odiff did not produce output file: {output_path}")
         else:
             diff_mask = _mask_from_diff_output(output_path)
-            if diff_mask.size != (max_w, max_h):
-                old_mask = diff_mask
-                diff_mask = diff_mask.resize((max_w, max_h), Image.NEAREST)
-                old_mask.close()
 
         diff_mask_png = _encode_mask_png(diff_mask)
 
@@ -142,6 +156,10 @@ def _compare_single_pair(
         logger.exception("Failed to compare image pair %d", idx)
         return None
     finally:
+        if before_padded is not None and before_padded is not before_img:
+            before_padded.close()
+        if after_padded is not None and after_padded is not after_img:
+            after_padded.close()
         if before_img is not None and isinstance(before, bytes):
             before_img.close()
         if after_img is not None and isinstance(after, bytes):

tests/sentry/preprod/snapshots/image_diff/test_image_diff.py

@@ -24,14 +24,53 @@ def test_different_sizes(self) -> None:
         large = _make_solid_image(50, 50, (100, 100, 100, 255))
         result = compare_images(small, large)
         assert result is not None
-        assert result.changed_pixels == 0
+        assert result.changed_pixels == 50 * 50 - 30 * 30
         assert result.total_pixels == 50 * 50
         assert result.aligned_height == 50
         assert result.before_width == 30
         assert result.before_height == 30
         assert result.after_width == 50
         assert result.after_height == 50
 
+    def test_different_sizes_with_content_diff(self) -> None:
+        before = _make_solid_image(50, 50, (100, 100, 100, 255))
+        after = _make_solid_image(80, 80, (200, 200, 200, 255))
+        result = compare_images(before, after)
+        assert result is not None
+        non_overlap = 80 * 80 - 50 * 50
+        assert result.changed_pixels > non_overlap
+        assert result.total_pixels == 80 * 80
+
+    def test_different_height_same_width(self) -> None:
+        before = _make_solid_image(100, 50, (100, 100, 100, 255))
+        after = _make_solid_image(100, 80, (100, 100, 100, 255))
+        result = compare_images(before, after)
+        assert result is not None
+        assert result.changed_pixels == 100 * 80 - 100 * 50
+        assert result.total_pixels == 100 * 80
+        assert result.before_width == 100
+        assert result.after_width == 100
+        assert result.before_height == 50
+        assert result.after_height == 80
+
+    def test_different_width_same_height(self) -> None:
+        before = _make_solid_image(50, 100, (100, 100, 100, 255))
+        after = _make_solid_image(80, 100, (100, 100, 100, 255))
+        result = compare_images(before, after)
+        assert result is not None
+        assert result.changed_pixels == 80 * 100 - 50 * 100
+        assert result.total_pixels == 80 * 100
+
+    def test_smaller_after_image(self) -> None:
+        before = _make_solid_image(50, 50, (100, 100, 100, 255))
+        after = _make_solid_image(30, 30, (100, 100, 100, 255))
+        result = compare_images(before, after)
+        assert result is not None
+        assert result.changed_pixels == 50 * 50 - 30 * 30
+        assert result.total_pixels == 50 * 50
+        assert result.before_width == 50
+        assert result.after_width == 30
+
     def test_modified_block(self) -> None:
         before = _make_solid_image(100, 100, (100, 100, 100, 255))
         after = _make_solid_image(100, 100, (100, 100, 100, 255))