From a09b45c5db5a0bdd915ee7d33b21fb384f86f2ec Mon Sep 17 00:00:00 2001 From: Yay295 Date: Tue, 4 Jul 2023 01:43:46 -0500 Subject: [PATCH 1/6] add tp_richcompare handler for Imaging_Type/ImagingCore --- Tests/test_lib_image.py | 21 +++++ src/PIL/Image.py | 12 ++- src/_imaging.c | 201 ++++++++++++++++++++++++++++++++++------ 3 files changed, 204 insertions(+), 30 deletions(-) diff --git a/Tests/test_lib_image.py b/Tests/test_lib_image.py index 31548bbc9..447f90b4d 100644 --- a/Tests/test_lib_image.py +++ b/Tests/test_lib_image.py @@ -32,3 +32,24 @@ def test_setmode() -> None: im.im.setmode("L") with pytest.raises(ValueError): im.im.setmode("RGBABCDE") + + +@pytest.mark.parametrize("mode", Image.MODES) +def test_equal(mode): + 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.im == img_b.im + + +@pytest.mark.parametrize("mode", Image.MODES) +def test_not_equal(mode): + 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.im != img_b.im diff --git a/src/PIL/Image.py b/src/PIL/Image.py index 44270392c..5a3499978 100644 --- a/src/PIL/Image.py +++ b/src/PIL/Image.py @@ -683,13 +683,17 @@ class Image: if self.__class__ is not other.__class__: return False assert isinstance(other, Image) - return ( + if self is other: + return True + if not ( self.mode == other.mode and self.size == other.size and self.info == other.info - and self.getpalette() == other.getpalette() - and self.tobytes() == other.tobytes() - ) + ): + return False + self.load() + other.load() + return self.im == other.im def __repr__(self) -> str: return "<%s.%s image mode=%s size=%dx%d at 0x%X>" % ( diff --git a/src/_imaging.c b/src/_imaging.c index 2db4486b2..e1284501d 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -3789,6 +3789,155 @@ static PySequenceMethods image_as_sequence = { (ssizessizeobjargproc)NULL, /*sq_ass_slice*/ }; +/* +Returns 0 if all of the pixels are the same, otherwise 1. +Skips unused bytes based on the given mode. +*/ +static int +_compare_pixels( + const char *mode, + const int ysize, + const int linesize, + const UINT8 **pixels_a, + const UINT8 **pixels_b +) { + // Fortunately, all of the modes that have extra bytes in their pixels + // use four bytes for their pixels. + UINT32 mask = 0xffffffff; + if ( + !strcmp(mode, "RGB") || !strcmp(mode, "YCbCr") || + !strcmp(mode, "HSV") || !strcmp(mode, "LAB") + ) { + // These modes have three channels in four bytes, + // so we have to ignore the last byte. +#ifdef WORDS_BIGENDIAN + mask = 0xffffff00; +#else + mask = 0x00ffffff; +#endif + } else if (!strcmp(mode, "LA") || !strcmp(mode, "La") || !strcmp(mode, "PA")) { + // These modes have two channels in four bytes, + // so we have to ignore the middle two bytes. + mask = 0xff0000ff; + } + + PyErr_Format(PyExc_Exception, "mode mask : %s 0x%08x", mode, mask); + + if (mask == 0xffffffff) { + // If we aren't masking anything we can use memcmp. + for (int y = 0; y < ysize; y++) { + if (memcmp(pixels_a[y], pixels_b[y], linesize)) { + return 1; + } + } + } else { + const int xsize = linesize / 4; + for (int y = 0; y < ysize; y++) { + UINT32 *line_a = (UINT32 *)pixels_a[y]; + UINT32 *line_b = (UINT32 *)pixels_b[y]; + for (int x = 0; x < xsize; x++, line_a++, line_b++) { + if ((*line_a & mask) != (*line_b & mask)) { + PyErr_Format( + PyExc_Exception, + "line %i index %i value a b mask : 0x%08x 0x%08x 0x%08x", + y, + x, + *line_a, + *line_b, + mask + ); + return 1; + } + } + } + } + return 0; +} + +static PyObject * +image_richcompare(const ImagingObject *self, const PyObject *other, const int op) { + if (op != Py_EQ && op != Py_NE) { + Py_RETURN_NOTIMPLEMENTED; + } + + // If the other object is not an ImagingObject. + if (!PyImaging_Check(other)) { + return op == Py_EQ ? Py_False : Py_True; + } + + const Imaging img_a = self->image; + const Imaging img_b = ((ImagingObject *)other)->image; + + PyErr_Format( + PyExc_Exception, + "a mode, b mode, diff : %s %s %i", + img_a->mode, + img_b->mode, + strcmp(img_a->mode, img_a->mode) + ); + + if ( + strcmp(img_a->mode, img_b->mode) + || img_a->xsize != img_b->xsize + || img_a->ysize != img_b->ysize + ) { + return op == Py_EQ ? Py_False : Py_True; + } + + const ImagingPalette palette_a = img_a->palette; + const ImagingPalette palette_b = img_b->palette; + if (palette_a || palette_b) { + PyErr_Format( + PyExc_Exception, + "pa mode, pb mode, diff : %s %s %i", + palette_a->mode, + palette_b->mode, + strcmp(palette_a->mode, palette_b->mode) + ); + + const UINT8 *palette_a_data = palette_a->palette; + const UINT8 *palette_b_data = palette_b->palette; + const UINT8 **palette_a_data_ptr = &palette_a_data; + const UINT8 **palette_b_data_ptr = &palette_b_data; + if ( + !palette_a || !palette_b + || palette_a->size != palette_b->size + || strcmp(palette_a->mode, palette_b->mode) + || _compare_pixels( + palette_a->mode, + 1, + palette_a->size * 4, + palette_a_data_ptr, + palette_b_data_ptr + ) + ) { + return op == Py_EQ ? Py_False : Py_True; + } + } + + PyErr_Format( + PyExc_Exception, + "linesize a b : %i %i", + img_a->linesize, + img_b->linesize + ); + + if (_compare_pixels( + img_a->mode, + img_a->ysize, + img_a->linesize, + (const UINT8 **)img_a->image, + (const UINT8 **)img_b->image + ) + ) { + PyErr_Clear(); + return op == Py_EQ ? Py_False : Py_True; + } else { + PyErr_Clear(); + return op == Py_EQ ? Py_True : Py_False; + } +} + /* type description */ static PyTypeObject Imaging_Type = { @@ -3796,32 +3945,32 @@ static PyTypeObject Imaging_Type = { sizeof(ImagingObject), /*tp_basicsize*/ 0, /*tp_itemsize*/ /* methods */ - (destructor)_dealloc, /*tp_dealloc*/ - 0, /*tp_vectorcall_offset*/ - 0, /*tp_getattr*/ - 0, /*tp_setattr*/ - 0, /*tp_as_async*/ - 0, /*tp_repr*/ - 0, /*tp_as_number*/ - &image_as_sequence, /*tp_as_sequence*/ - 0, /*tp_as_mapping*/ - 0, /*tp_hash*/ - 0, /*tp_call*/ - 0, /*tp_str*/ - 0, /*tp_getattro*/ - 0, /*tp_setattro*/ - 0, /*tp_as_buffer*/ - Py_TPFLAGS_DEFAULT, /*tp_flags*/ - 0, /*tp_doc*/ - 0, /*tp_traverse*/ - 0, /*tp_clear*/ - 0, /*tp_richcompare*/ - 0, /*tp_weaklistoffset*/ - 0, /*tp_iter*/ - 0, /*tp_iternext*/ - methods, /*tp_methods*/ - 0, /*tp_members*/ - getsetters, /*tp_getset*/ + (destructor)_dealloc, /*tp_dealloc*/ + 0, /*tp_vectorcall_offset*/ + 0, /*tp_getattr*/ + 0, /*tp_setattr*/ + 0, /*tp_as_async*/ + 0, /*tp_repr*/ + 0, /*tp_as_number*/ + &image_as_sequence, /*tp_as_sequence*/ + 0, /*tp_as_mapping*/ + 0, /*tp_hash*/ + 0, /*tp_call*/ + 0, /*tp_str*/ + 0, /*tp_getattro*/ + 0, /*tp_setattro*/ + 0, /*tp_as_buffer*/ + Py_TPFLAGS_DEFAULT, /*tp_flags*/ + 0, /*tp_doc*/ + 0, /*tp_traverse*/ + 0, /*tp_clear*/ + (richcmpfunc)image_richcompare, /*tp_richcompare*/ + 0, /*tp_weaklistoffset*/ + 0, /*tp_iter*/ + 0, /*tp_iternext*/ + methods, /*tp_methods*/ + 0, /*tp_members*/ + getsetters, /*tp_getset*/ }; static PyTypeObject ImagingFont_Type = { From 38fd40a0cacdd90b2f5faba096f69fe2b8e420fa Mon Sep 17 00:00:00 2001 From: Yay295 Date: Wed, 5 Jul 2023 08:33:46 -0500 Subject: [PATCH 2/6] use Py_RETURN_* macros for Py_True/Py_False Py_True and Py_False were only made immortal in Python 3.12, so we have to properly increment their refcount for versions before that. --- src/_imaging.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/_imaging.c b/src/_imaging.c index e1284501d..f7f8b64c4 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -3862,7 +3862,11 @@ image_richcompare(const ImagingObject *self, const PyObject *other, const int op // If the other object is not an ImagingObject. if (!PyImaging_Check(other)) { - return op == Py_EQ ? Py_False : Py_True; + if (op == Py_EQ) { + Py_RETURN_FALSE; + } else { + Py_RETURN_TRUE; + } } const Imaging img_a = self->image; @@ -3881,7 +3885,11 @@ image_richcompare(const ImagingObject *self, const PyObject *other, const int op || img_a->xsize != img_b->xsize || img_a->ysize != img_b->ysize ) { - return op == Py_EQ ? Py_False : Py_True; + if (op == Py_EQ) { + Py_RETURN_FALSE; + } else { + Py_RETURN_TRUE; + } } const ImagingPalette palette_a = img_a->palette; @@ -3911,7 +3919,11 @@ image_richcompare(const ImagingObject *self, const PyObject *other, const int op palette_b_data_ptr ) ) { - return op == Py_EQ ? Py_False : Py_True; + if (op == Py_EQ) { + Py_RETURN_FALSE; + } else { + Py_RETURN_TRUE; + } } } @@ -3930,11 +3942,17 @@ image_richcompare(const ImagingObject *self, const PyObject *other, const int op (const UINT8 **)img_b->image ) ) { - PyErr_Clear(); - return op == Py_EQ ? Py_False : Py_True; + if (op == Py_EQ) { + Py_RETURN_FALSE; + } else { + Py_RETURN_TRUE; + } } else { - PyErr_Clear(); - return op == Py_EQ ? Py_True : Py_False; + if (op == Py_EQ) { + Py_RETURN_TRUE; + } else { + Py_RETURN_FALSE; + } } } From 26218998bb3aace6cd6a6463bc729fd25e6e640d Mon Sep 17 00:00:00 2001 From: Yay295 Date: Tue, 4 Jul 2023 01:46:33 -0500 Subject: [PATCH 3/6] remove debugging messages --- src/_imaging.c | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/src/_imaging.c b/src/_imaging.c index f7f8b64c4..f123e1a0a 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -3821,8 +3821,6 @@ _compare_pixels( mask = 0xff0000ff; } - PyErr_Format(PyExc_Exception, "mode mask : %s 0x%08x", mode, mask); - if (mask == 0xffffffff) { // If we aren't masking anything we can use memcmp. for (int y = 0; y < ysize; y++) { @@ -3837,15 +3835,6 @@ _compare_pixels( UINT32 *line_b = (UINT32 *)pixels_b[y]; for (int x = 0; x < xsize; x++, line_a++, line_b++) { if ((*line_a & mask) != (*line_b & mask)) { - PyErr_Format( - PyExc_Exception, - "line %i index %i value a b mask : 0x%08x 0x%08x 0x%08x", - y, - x, - *line_a, - *line_b, - mask - ); return 1; } } @@ -3872,14 +3861,6 @@ image_richcompare(const ImagingObject *self, const PyObject *other, const int op const Imaging img_a = self->image; const Imaging img_b = ((ImagingObject *)other)->image; - PyErr_Format( - PyExc_Exception, - "a mode, b mode, diff : %s %s %i", - img_a->mode, - img_b->mode, - strcmp(img_a->mode, img_a->mode) - ); - if ( strcmp(img_a->mode, img_b->mode) || img_a->xsize != img_b->xsize @@ -3895,14 +3876,6 @@ image_richcompare(const ImagingObject *self, const PyObject *other, const int op const ImagingPalette palette_a = img_a->palette; const ImagingPalette palette_b = img_b->palette; if (palette_a || palette_b) { - PyErr_Format( - PyExc_Exception, - "pa mode, pb mode, diff : %s %s %i", - palette_a->mode, - palette_b->mode, - strcmp(palette_a->mode, palette_b->mode) - ); - const UINT8 *palette_a_data = palette_a->palette; const UINT8 *palette_b_data = palette_b->palette; const UINT8 **palette_a_data_ptr = &palette_a_data; @@ -3927,13 +3900,6 @@ image_richcompare(const ImagingObject *self, const PyObject *other, const int op } } - PyErr_Format( - PyExc_Exception, - "linesize a b : %i %i", - img_a->linesize, - img_b->linesize - ); - if (_compare_pixels( img_a->mode, img_a->ysize, From 183856267cf1f97171003f564f70db30115a4cab Mon Sep 17 00:00:00 2001 From: Yay295 Date: Tue, 4 Jul 2023 16:55:03 -0500 Subject: [PATCH 4/6] extract mode "1" to its own test --- Tests/test_lib_image.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/Tests/test_lib_image.py b/Tests/test_lib_image.py index 447f90b4d..445f806ef 100644 --- a/Tests/test_lib_image.py +++ b/Tests/test_lib_image.py @@ -1,5 +1,7 @@ from __future__ import annotations +import itertools + import pytest from PIL import Image @@ -44,7 +46,24 @@ def test_equal(mode): assert img_a.im == img_b.im -@pytest.mark.parametrize("mode", Image.MODES) +# With mode "1" different bytes can map to the same value, +# so we have to be more specific with the values we use. +@pytest.mark.parametrize( + "bytes_a, bytes_b", + itertools.permutations( + (bytes(x) for x in itertools.product(b"\x00\xff", repeat=4)), 2 + ), +) +def test_not_equal_mode_1(bytes_a, bytes_b): + # 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.im != img_b.im + + +@pytest.mark.parametrize("mode", [mode for mode in Image.MODES if mode != "1"]) def test_not_equal(mode): num_img_bytes = len(Image.new(mode, (2, 2)).tobytes()) data_a = bytes(range(ord("A"), ord("A") + num_img_bytes)) From 0ec76d5d3afcf9d938156308f2ed0f98e93cce3a Mon Sep 17 00:00:00 2001 From: Yay295 Date: Tue, 11 Jul 2023 20:33:19 +1000 Subject: [PATCH 5/6] add tests for image modes with a different number of bytes and bands --- Tests/test_lib_image.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Tests/test_lib_image.py b/Tests/test_lib_image.py index 445f806ef..438ecd5ca 100644 --- a/Tests/test_lib_image.py +++ b/Tests/test_lib_image.py @@ -72,3 +72,24 @@ def test_not_equal(mode): img_b = Image.frombytes(mode, (2, 2), data_b) assert img_a.tobytes() != img_b.tobytes() assert img_a.im != img_b.im + + +@pytest.mark.parametrize("mode", ("RGB", "YCbCr", "HSV", "LAB")) +def test_equal_three_channels_four_bytes(mode): + # 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() == b"123" + assert img_b.tobytes() == b"123" + assert img_a.im == img_b.im + + +@pytest.mark.parametrize("mode", ("LA", "La", "PA")) +def test_equal_two_channels_four_bytes(mode): + img_a = Image.new(mode, (1, 1), 0x32000031) + img_b = Image.new(mode, (1, 1), 0x32FFFF31) + assert img_a.tobytes() == b"12" + assert img_b.tobytes() == b"12" + assert img_a.im == img_b.im From d226ab354bc5c4b18fd8b1dc36b41371a8329f12 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 12 Oct 2024 23:33:25 +0000 Subject: [PATCH 6/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/_imaging.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/src/_imaging.c b/src/_imaging.c index f123e1a0a..ab038b5a9 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -3804,10 +3804,8 @@ _compare_pixels( // Fortunately, all of the modes that have extra bytes in their pixels // use four bytes for their pixels. UINT32 mask = 0xffffffff; - if ( - !strcmp(mode, "RGB") || !strcmp(mode, "YCbCr") || - !strcmp(mode, "HSV") || !strcmp(mode, "LAB") - ) { + if (!strcmp(mode, "RGB") || !strcmp(mode, "YCbCr") || !strcmp(mode, "HSV") || + !strcmp(mode, "LAB")) { // These modes have three channels in four bytes, // so we have to ignore the last byte. #ifdef WORDS_BIGENDIAN @@ -3861,11 +3859,8 @@ image_richcompare(const ImagingObject *self, const PyObject *other, const int op const Imaging img_a = self->image; const Imaging img_b = ((ImagingObject *)other)->image; - if ( - strcmp(img_a->mode, img_b->mode) - || img_a->xsize != img_b->xsize - || img_a->ysize != img_b->ysize - ) { + if (strcmp(img_a->mode, img_b->mode) || img_a->xsize != img_b->xsize || + img_a->ysize != img_b->ysize) { if (op == Py_EQ) { Py_RETURN_FALSE; } else { @@ -3880,18 +3875,15 @@ image_richcompare(const ImagingObject *self, const PyObject *other, const int op const UINT8 *palette_b_data = palette_b->palette; const UINT8 **palette_a_data_ptr = &palette_a_data; const UINT8 **palette_b_data_ptr = &palette_b_data; - if ( - !palette_a || !palette_b - || palette_a->size != palette_b->size - || strcmp(palette_a->mode, palette_b->mode) - || _compare_pixels( + if (!palette_a || !palette_b || palette_a->size != palette_b->size || + strcmp(palette_a->mode, palette_b->mode) || + _compare_pixels( palette_a->mode, 1, palette_a->size * 4, palette_a_data_ptr, palette_b_data_ptr - ) - ) { + )) { if (op == Py_EQ) { Py_RETURN_FALSE; } else { @@ -3906,8 +3898,7 @@ image_richcompare(const ImagingObject *self, const PyObject *other, const int op img_a->linesize, (const UINT8 **)img_a->image, (const UINT8 **)img_b->image - ) - ) { + )) { if (op == Py_EQ) { Py_RETURN_FALSE; } else {