-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Speed up image comparison #10030
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
akx
wants to merge
6
commits into
python-pillow:main
Choose a base branch
from
akx:image-equals-new
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+310
−7
Open
Speed up image comparison #10030
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
22ac079
Add image equality tests
Yay295 04ce23b
Adjust equality tests
akx a74c03d
Add more tests for equality
akx 6908311
Add tp_richcompare handler for Imaging_Type/ImagingCore
akx 9f90332
Use vectorisable form of masked comparison loop
akx 8602567
Apply suggestions from code review
radarhere File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import itertools | ||
| from unittest.mock import Mock | ||
|
|
||
| import pytest | ||
|
|
||
| from PIL import Image | ||
|
|
||
|
|
||
| def _make_rows(width: int, height: int) -> list[bytes]: | ||
| """Build test image data of width x height.""" | ||
| return [ | ||
| bytes((y * (width - 1) + x) % 256 for x in range(width)) for y in range(height) | ||
| ] | ||
|
|
||
|
|
||
| def _frombuffer( | ||
| mode: str, size: tuple[int, int], data: bytes, *, stride: int = 0, ystep: int = 1 | ||
| ) -> Image.Image: | ||
| im = Image.frombuffer(mode, size, data, "raw", mode, stride, ystep) | ||
| assert im.readonly # Sanity check (that we took the map_buffer path) | ||
| return im | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("mode", Image.MODES) | ||
| def test_equal(mode: str) -> None: | ||
| num_img_bytes = len(Image.new(mode, (2, 2)).tobytes()) | ||
| data = bytes(range(ord("A"), ord("A") + num_img_bytes)) | ||
| img_a = Image.frombytes(mode, (2, 2), data) | ||
| img_b = Image.frombytes(mode, (2, 2), data) | ||
| assert img_a.tobytes() == img_b.tobytes() | ||
| assert img_a == img_b | ||
|
|
||
|
|
||
| def test_not_equal_mode_1() -> None: | ||
| # With mode "1" different bytes can map to the same value, | ||
| # so we have to be more specific with the values we use. | ||
| for bytes_a, bytes_b in itertools.permutations( | ||
| (bytes(x) for x in itertools.product(b"\x00\xff", repeat=4)), 2 | ||
| ): | ||
| # Use rawmode "1;8" so that each full byte is interpreted as a value | ||
| # instead of the bits in the bytes being interpreted as values. | ||
| img_a = Image.frombytes("1", (2, 2), bytes_a, "raw", "1;8") | ||
| img_b = Image.frombytes("1", (2, 2), bytes_b, "raw", "1;8") | ||
| assert img_a.tobytes() != img_b.tobytes() | ||
| assert img_a != img_b | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("mode", [mode for mode in Image.MODES if mode != "1"]) | ||
| def test_not_equal(mode: str) -> None: | ||
| num_img_bytes = len(Image.new(mode, (2, 2)).tobytes()) | ||
| data_a = bytes(range(ord("A"), ord("A") + num_img_bytes)) | ||
| data_b = bytes(range(ord("Z"), ord("Z") - num_img_bytes, -1)) | ||
| img_a = Image.frombytes(mode, (2, 2), data_a) | ||
| img_b = Image.frombytes(mode, (2, 2), data_b) | ||
| assert img_a.tobytes() != img_b.tobytes() | ||
| assert img_a != img_b | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("mode", ("RGB", "YCbCr", "HSV", "LAB")) | ||
| def test_equal_three_channels_four_bytes(mode: str) -> None: | ||
| # The "A" and "B" values in LAB images are signed values from -128 to 127, | ||
| # but we store them as unsigned values from 0 to 255, so we need to use | ||
| # slightly different input bytes for LAB to get the same output. | ||
| img_a = Image.new(mode, (1, 1), 0x00B3B231 if mode == "LAB" else 0x00333231) | ||
| img_b = Image.new(mode, (1, 1), 0xFFB3B231 if mode == "LAB" else 0xFF333231) | ||
| assert img_a.tobytes() == img_b.tobytes() == b"123" | ||
| assert img_a == img_b | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("mode", ("LA", "La", "PA")) | ||
| def test_equal_two_channels_four_bytes(mode: str) -> None: | ||
| # Test that for LA/La/PA modes, where the data is stored in the 1st and 4th | ||
| # byte of each pixel, the middle bytes are masked off for comparison. | ||
| img_a = Image.new(mode, (1, 1), 0x32000031) | ||
| img_b = Image.new(mode, (1, 1), 0x32FFFF31) | ||
| assert img_a.tobytes() == img_b.tobytes() == b"12" | ||
| assert img_a == img_b | ||
|
|
||
|
|
||
| def test_not_equal_rgbx_padding() -> None: | ||
| # Ensure RGBX's last byte is compared, even if it has no image meaning. | ||
| img_a = Image.frombytes("RGBX", (1, 1), b"1234") | ||
| img_b = Image.frombytes("RGBX", (1, 1), b"123\xff") | ||
| assert img_a.tobytes() != img_b.tobytes() | ||
| assert img_a != img_b | ||
|
|
||
|
|
||
| def test_compare_with_other_type() -> None: | ||
| im = Image.new("L", (1, 1)) | ||
| assert im.im == im.im | ||
| assert im.im != 42 | ||
| # Check that the other object's comparison method is called. | ||
| x = Mock(__eq__=Mock(return_value=True)) | ||
| assert im.im == x | ||
| assert x.__eq__.called # type: ignore[attr-defined] | ||
|
|
||
|
|
||
| def test_equal_frombuffer_stride() -> None: | ||
| # Test that a buffer-mapped image's padding bytes (stride) | ||
| # are not part of the comparison. | ||
| width, height, stride = 3, 4, 8 | ||
| rows = _make_rows(width, height) | ||
| buffer_a = b"".join(row + b"\x00" * (stride - width) for row in rows) | ||
| buffer_b = b"".join(row + b"\xff" * (stride - width) for row in rows) | ||
| assert buffer_a != buffer_b | ||
| img_a = _frombuffer("L", (width, height), buffer_a, stride=stride) | ||
| img_b = _frombuffer("L", (width, height), buffer_b, stride=stride) | ||
| assert img_a.tobytes() == img_b.tobytes() # Padding bytes disappear | ||
| assert img_a == img_b | ||
|
|
||
|
|
||
| def test_not_equal_frombuffer_stride() -> None: | ||
| # Test that differences within strided rows are found, | ||
| # even if padding matches. | ||
| width, height, stride = 3, 4, 8 | ||
| rows = _make_rows(width, height) | ||
| padding = b"\xab" * (stride - width) | ||
| buffer_a = b"".join(row + padding for row in rows) | ||
| rows[height - 1] = bytes(42) + rows[height - 1][1:] | ||
| buffer_b = b"".join(row + padding for row in rows) | ||
|
|
||
| img_a = _frombuffer("L", (width, height), buffer_a, stride=stride) | ||
| img_b = _frombuffer("L", (width, height), buffer_b, stride=stride) | ||
| assert img_a.tobytes() != img_b.tobytes() | ||
| assert img_a != img_b | ||
|
|
||
|
|
||
| def test_equal_frombuffer_ystep() -> None: | ||
| # Test that ystep=-1 (rows in reverse order) is handled correctly. | ||
| width, height = 3, 4 | ||
| rows = _make_rows(width, height) | ||
| img_a = _frombuffer("L", (width, height), b"".join(rows)) | ||
| img_b = _frombuffer("L", (width, height), b"".join(reversed(rows)), ystep=-1) | ||
| assert img_a.tobytes() == img_b.tobytes() | ||
| assert img_a == img_b |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might just be me, but I find this kind of vague. It doesn't copy the images to
bytesfirst... but it still does eventually? What else does it do first?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT of
Is that too technical maybe?
would at least be succinct?