From ebd3c47425fe5535f8d1c8eab15bbf1e52899939 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 29 Apr 2023 15:02:11 +1000 Subject: [PATCH 1/8] When saving, allow alpha differences to indicate different frames --- Tests/test_file_apng.py | 14 ++++++++++++++ src/PIL/PngImagePlugin.py | 4 ++-- src/_imaging.c | 12 +++++++++--- src/libImaging/GetBBox.c | 7 ++++--- src/libImaging/Imaging.h | 2 +- 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/Tests/test_file_apng.py b/Tests/test_file_apng.py index f78c086eb0c..cf1312b770e 100644 --- a/Tests/test_file_apng.py +++ b/Tests/test_file_apng.py @@ -374,6 +374,20 @@ def test_apng_save(tmp_path): assert im.getpixel((64, 32)) == (0, 255, 0, 255) +def test_apng_save_alpha(tmp_path): + test_file = str(tmp_path / "temp.png") + + im = Image.new("RGBA", (1, 1), (255, 0, 0, 255)) + im2 = Image.new("RGBA", (1, 1), (255, 0, 0, 127)) + im.save(test_file, save_all=True, append_images=[im2]) + + with Image.open(test_file) as reloaded: + assert reloaded.getpixel((0, 0)) == (255, 0, 0, 255) + + reloaded.seek(1) + assert reloaded.getpixel((0, 0)) == (255, 0, 0, 127) + + def test_apng_save_split_fdat(tmp_path): # test to make sure we do not generate sequence errors when writing # frames with image data spanning multiple fdAT chunks (in this case diff --git a/src/PIL/PngImagePlugin.py b/src/PIL/PngImagePlugin.py index 82a74b26785..7afd6a2a6e8 100644 --- a/src/PIL/PngImagePlugin.py +++ b/src/PIL/PngImagePlugin.py @@ -1138,9 +1138,9 @@ def _write_multiple_frames(im, fp, chunk, rawmode, default_image, append_images) else: base_im = previous["im"] delta = ImageChops.subtract_modulo( - im_frame.convert("RGB"), base_im.convert("RGB") + im_frame.convert("RGBA"), base_im.convert("RGBA") ) - bbox = delta.getbbox() + bbox = delta.im.getbbox(False) if ( not bbox and prev_disposal == encoderinfo.get("disposal") diff --git a/src/_imaging.c b/src/_imaging.c index 281f3a4d2e6..62e51da26e2 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -2160,9 +2160,15 @@ _isblock(ImagingObject *self) { } static PyObject * -_getbbox(ImagingObject *self) { +_getbbox(ImagingObject *self, PyObject *args) { int bbox[4]; - if (!ImagingGetBBox(self->image, bbox)) { + + int consider_alpha = 1; + if (!PyArg_ParseTuple(args, "|i", &consider_alpha)) { + return NULL; + } + + if (!ImagingGetBBox(self->image, bbox, consider_alpha)) { Py_INCREF(Py_None); return Py_None; } @@ -3574,7 +3580,7 @@ static struct PyMethodDef methods[] = { {"isblock", (PyCFunction)_isblock, METH_NOARGS}, - {"getbbox", (PyCFunction)_getbbox, METH_NOARGS}, + {"getbbox", (PyCFunction)_getbbox, METH_VARARGS}, {"getcolors", (PyCFunction)_getcolors, METH_VARARGS}, {"getextrema", (PyCFunction)_getextrema, METH_NOARGS}, {"getprojection", (PyCFunction)_getprojection, METH_NOARGS}, diff --git a/src/libImaging/GetBBox.c b/src/libImaging/GetBBox.c index e73153600d0..c1570cd3e5a 100644 --- a/src/libImaging/GetBBox.c +++ b/src/libImaging/GetBBox.c @@ -19,7 +19,7 @@ #include "Imaging.h" int -ImagingGetBBox(Imaging im, int bbox[4]) { +ImagingGetBBox(Imaging im, int bbox[4], int consider_alpha) { /* Get the bounding box for any non-zero data in the image.*/ int x, y; @@ -58,10 +58,11 @@ ImagingGetBBox(Imaging im, int bbox[4]) { INT32 mask = 0xffffffff; if (im->bands == 3) { ((UINT8 *)&mask)[3] = 0; - } else if ( + } else if (consider_alpha && ( strcmp(im->mode, "RGBa") == 0 || strcmp(im->mode, "RGBA") == 0 || strcmp(im->mode, "La") == 0 || strcmp(im->mode, "LA") == 0 || - strcmp(im->mode, "PA") == 0) { + strcmp(im->mode, "PA") == 0 + )) { #ifdef WORDS_BIGENDIAN mask = 0x000000ff; #else diff --git a/src/libImaging/Imaging.h b/src/libImaging/Imaging.h index d9ded185238..2563a0c6271 100644 --- a/src/libImaging/Imaging.h +++ b/src/libImaging/Imaging.h @@ -317,7 +317,7 @@ ImagingMerge(const char *mode, Imaging bands[4]); extern int ImagingSplit(Imaging im, Imaging bands[4]); extern int -ImagingGetBBox(Imaging im, int bbox[4]); +ImagingGetBBox(Imaging im, int bbox[4], int consider_alpha); typedef struct { int x, y; INT32 count; From 96bdbc4afe8ea9c6f113c2676368b2a11bc9fefb Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 29 Apr 2023 19:11:02 +1000 Subject: [PATCH 2/8] Renamed variable --- src/_imaging.c | 6 +++--- src/libImaging/GetBBox.c | 4 ++-- src/libImaging/Imaging.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/_imaging.c b/src/_imaging.c index 62e51da26e2..87f5b67055d 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -2163,12 +2163,12 @@ static PyObject * _getbbox(ImagingObject *self, PyObject *args) { int bbox[4]; - int consider_alpha = 1; - if (!PyArg_ParseTuple(args, "|i", &consider_alpha)) { + int alpha_only = 1; + if (!PyArg_ParseTuple(args, "|i", &alpha_only)) { return NULL; } - if (!ImagingGetBBox(self->image, bbox, consider_alpha)) { + if (!ImagingGetBBox(self->image, bbox, alpha_only)) { Py_INCREF(Py_None); return Py_None; } diff --git a/src/libImaging/GetBBox.c b/src/libImaging/GetBBox.c index c1570cd3e5a..86c687ca0a8 100644 --- a/src/libImaging/GetBBox.c +++ b/src/libImaging/GetBBox.c @@ -19,7 +19,7 @@ #include "Imaging.h" int -ImagingGetBBox(Imaging im, int bbox[4], int consider_alpha) { +ImagingGetBBox(Imaging im, int bbox[4], int alpha_only) { /* Get the bounding box for any non-zero data in the image.*/ int x, y; @@ -58,7 +58,7 @@ ImagingGetBBox(Imaging im, int bbox[4], int consider_alpha) { INT32 mask = 0xffffffff; if (im->bands == 3) { ((UINT8 *)&mask)[3] = 0; - } else if (consider_alpha && ( + } else if (alpha_only && ( strcmp(im->mode, "RGBa") == 0 || strcmp(im->mode, "RGBA") == 0 || strcmp(im->mode, "La") == 0 || strcmp(im->mode, "LA") == 0 || strcmp(im->mode, "PA") == 0 diff --git a/src/libImaging/Imaging.h b/src/libImaging/Imaging.h index 2563a0c6271..42420887d30 100644 --- a/src/libImaging/Imaging.h +++ b/src/libImaging/Imaging.h @@ -317,7 +317,7 @@ ImagingMerge(const char *mode, Imaging bands[4]); extern int ImagingSplit(Imaging im, Imaging bands[4]); extern int -ImagingGetBBox(Imaging im, int bbox[4], int consider_alpha); +ImagingGetBBox(Imaging im, int bbox[4], int alpha_only); typedef struct { int x, y; INT32 count; From 15ef533df9847e556eca0eaa50b7738bb71b8c34 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 2 May 2023 08:41:18 +1000 Subject: [PATCH 3/8] Added alpha_only argument to getbbox() --- Tests/test_image_getbbox.py | 15 +++++++++++++++ src/PIL/Image.py | 7 +++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Tests/test_image_getbbox.py b/Tests/test_image_getbbox.py index af69ed57a60..eec97821098 100644 --- a/Tests/test_image_getbbox.py +++ b/Tests/test_image_getbbox.py @@ -1,3 +1,5 @@ +import pytest + from PIL import Image from .helper import hopper @@ -38,3 +40,16 @@ def check(im, fill_color): for color in ((0, 0), (127, 0), (255, 0)): im = Image.new(mode, (100, 100), color) check(im, (255, 255)) + + +@pytest.mark.parametrize("mode", ("RGBA", "RGBa", "La", "LA", "PA")) +def test_bbox_alpha_only_false(mode): + im = Image.new(mode, (100, 100)) + assert im.getbbox(False) is None + + fill_color = [1] * Image.getmodebands(mode) + fill_color[-1] = 0 + im.paste(tuple(fill_color), (25, 25, 75, 75)) + assert im.getbbox(False) == (25, 25, 75, 75) + + assert im.getbbox() is None diff --git a/src/PIL/Image.py b/src/PIL/Image.py index bee9e23d088..f5d1206717d 100644 --- a/src/PIL/Image.py +++ b/src/PIL/Image.py @@ -1279,11 +1279,14 @@ def getbands(self): """ return ImageMode.getmode(self.mode).bands - def getbbox(self): + def getbbox(self, alpha_only=True): """ Calculates the bounding box of the non-zero regions in the image. + :param alpha_only: Optional flag, defaulting to true. + If true and the image has an alpha channel, trim transparent pixels. + Otherwise, trim pixels when all channels are zero. :returns: The bounding box is returned as a 4-tuple defining the left, upper, right, and lower pixel coordinate. See :ref:`coordinate-system`. If the image is completely empty, this @@ -1292,7 +1295,7 @@ def getbbox(self): """ self.load() - return self.im.getbbox() + return self.im.getbbox(alpha_only) def getcolors(self, maxcolors=256): """ From bae918280d8bd74d2ace512a71eabc76e05a1d0f Mon Sep 17 00:00:00 2001 From: Andrew Murray <3112309+radarhere@users.noreply.github.com> Date: Wed, 14 Jun 2023 11:25:12 +1000 Subject: [PATCH 4/8] Changed alpha_only to keyword-only argument Co-authored-by: Hugo van Kemenade --- src/PIL/Image.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PIL/Image.py b/src/PIL/Image.py index f5d1206717d..74c1bd7f683 100644 --- a/src/PIL/Image.py +++ b/src/PIL/Image.py @@ -1279,7 +1279,7 @@ def getbands(self): """ return ImageMode.getmode(self.mode).bands - def getbbox(self, alpha_only=True): + def getbbox(self, *, alpha_only=True): """ Calculates the bounding box of the non-zero regions in the image. From d7c7b832f142aa5a66ee5dd9ad1ce4f7e2afa241 Mon Sep 17 00:00:00 2001 From: Andrew Murray <3112309+radarhere@users.noreply.github.com> Date: Wed, 14 Jun 2023 11:25:42 +1000 Subject: [PATCH 5/8] Highlight code Co-authored-by: Hugo van Kemenade --- src/PIL/Image.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/PIL/Image.py b/src/PIL/Image.py index 74c1bd7f683..8eebd28f970 100644 --- a/src/PIL/Image.py +++ b/src/PIL/Image.py @@ -1284,8 +1284,8 @@ def getbbox(self, *, alpha_only=True): Calculates the bounding box of the non-zero regions in the image. - :param alpha_only: Optional flag, defaulting to true. - If true and the image has an alpha channel, trim transparent pixels. + :param alpha_only: Optional flag, defaulting to ``True``. + If ``True`` and the image has an alpha channel, trim transparent pixels. Otherwise, trim pixels when all channels are zero. :returns: The bounding box is returned as a 4-tuple defining the left, upper, right, and lower pixel coordinate. See From 044de40c93064aa938dc45868ce52698062009da Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Wed, 14 Jun 2023 11:28:14 +1000 Subject: [PATCH 6/8] Document that alpha_only is a keyword-only argument --- src/PIL/Image.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/PIL/Image.py b/src/PIL/Image.py index 4413bae288e..340ba4e3bb3 100644 --- a/src/PIL/Image.py +++ b/src/PIL/Image.py @@ -1301,6 +1301,7 @@ def getbbox(self, *, alpha_only=True): :param alpha_only: Optional flag, defaulting to ``True``. If ``True`` and the image has an alpha channel, trim transparent pixels. Otherwise, trim pixels when all channels are zero. + Keyword-only argument. :returns: The bounding box is returned as a 4-tuple defining the left, upper, right, and lower pixel coordinate. See :ref:`coordinate-system`. If the image is completely empty, this From 119a0dfb0113391bbab34fcd7be94457877506cc Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Wed, 14 Jun 2023 11:29:22 +1000 Subject: [PATCH 7/8] Updated tests now that alpha_only is keyword-only --- Tests/test_image_getbbox.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/test_image_getbbox.py b/Tests/test_image_getbbox.py index eec97821098..afca6670305 100644 --- a/Tests/test_image_getbbox.py +++ b/Tests/test_image_getbbox.py @@ -45,11 +45,11 @@ def check(im, fill_color): @pytest.mark.parametrize("mode", ("RGBA", "RGBa", "La", "LA", "PA")) def test_bbox_alpha_only_false(mode): im = Image.new(mode, (100, 100)) - assert im.getbbox(False) is None + assert im.getbbox(alpha_only=False) is None fill_color = [1] * Image.getmodebands(mode) fill_color[-1] = 0 im.paste(tuple(fill_color), (25, 25, 75, 75)) - assert im.getbbox(False) == (25, 25, 75, 75) + assert im.getbbox(alpha_only=False) == (25, 25, 75, 75) assert im.getbbox() is None From 541d2605b9235a427379e3df51c9cdd4ffe59998 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Wed, 14 Jun 2023 14:21:07 +1000 Subject: [PATCH 8/8] Allow alpha differences to indicate different frames when saving GIF --- Tests/test_file_gif.py | 12 ++++++++++++ src/PIL/GifImagePlugin.py | 4 ++-- src/PIL/PngImagePlugin.py | 2 +- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Tests/test_file_gif.py b/Tests/test_file_gif.py index 0e50ee1abf9..f4a17264f4a 100644 --- a/Tests/test_file_gif.py +++ b/Tests/test_file_gif.py @@ -1130,6 +1130,18 @@ def test_bbox(tmp_path): assert reread.n_frames == 2 +def test_bbox_alpha(tmp_path): + out = str(tmp_path / "temp.gif") + + im = Image.new("RGBA", (1, 2), (255, 0, 0, 255)) + im.putpixel((0, 1), (255, 0, 0, 0)) + im2 = Image.new("RGBA", (1, 2), (255, 0, 0, 0)) + im.save(out, save_all=True, append_images=[im2]) + + with Image.open(out) as reread: + assert reread.n_frames == 2 + + def test_palette_save_L(tmp_path): # Generate an L mode image with a separate palette diff --git a/src/PIL/GifImagePlugin.py b/src/PIL/GifImagePlugin.py index 2f92e946751..cf2993e3892 100644 --- a/src/PIL/GifImagePlugin.py +++ b/src/PIL/GifImagePlugin.py @@ -569,9 +569,9 @@ def _getbbox(base_im, im_frame): delta = ImageChops.subtract_modulo(im_frame, base_im) else: delta = ImageChops.subtract_modulo( - im_frame.convert("RGB"), base_im.convert("RGB") + im_frame.convert("RGBA"), base_im.convert("RGBA") ) - return delta.getbbox() + return delta.getbbox(alpha_only=False) def _write_multiple_frames(im, fp, palette): diff --git a/src/PIL/PngImagePlugin.py b/src/PIL/PngImagePlugin.py index ea3052fbf7c..bfa8cb7ac66 100644 --- a/src/PIL/PngImagePlugin.py +++ b/src/PIL/PngImagePlugin.py @@ -1140,7 +1140,7 @@ def _write_multiple_frames(im, fp, chunk, rawmode, default_image, append_images) delta = ImageChops.subtract_modulo( im_frame.convert("RGBA"), base_im.convert("RGBA") ) - bbox = delta.im.getbbox(False) + bbox = delta.getbbox(alpha_only=False) if ( not bbox and prev_disposal == encoderinfo.get("disposal")