Skip to content
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

GifImagePlugin normalize_palette destroys bounding box of frame #3704

Closed
sircinnamon opened this issue Mar 8, 2019 · 2 comments
Closed

GifImagePlugin normalize_palette destroys bounding box of frame #3704

sircinnamon opened this issue Mar 8, 2019 · 2 comments

Comments

@sircinnamon
Copy link
Contributor

What did you do?

Ran single colour frames through GifImagePlugin _normalize_palette

What did you expect to happen?

Image object palette changes, nothing else

What actually happened?

Image.getbbox() went from 100x100 to None

What are your OS, Python and Pillow versions?

  • OS: Archlinux
  • Python: 3.4.9 (in venv)
  • Pillow: 5.4.1

To reproduce:
(This is a modification of the test_save_dispose test from the test suite - I think it's preventing a valid fix for something else but unsure)

from PIL import Image
out = "realtest.gif"
im_list = [
	Image.new('L', (100, 100), 255),
	Image.new('L', (100, 100), 127),
	Image.new('L', (100, 100), 0),
]
for method in range(0, 4):
	print("========= {} ========".format(method))
	im_list[0].save(
		str(method)+out,
		save_all=True,
		append_images=im_list[1:],
		disposal=method
	)
	img = Image.open(str(method)+out)
	for _ in range(2):
		img.seek(img.tell() + 1)
		print("{} == {}".format(img.disposal_method, method))
# check per frame disposal
im_list[0].save(
	out,
	save_all=True,
	append_images=im_list[1:],
	disposal=tuple(range(len(im_list)))
	)

img = Image.open(out)

for i in range(2):
	img.seek(img.tell() + 1)
	print("{} == {}".format(img.disposal_method, i+1))

Also, modify _write_multiple_frames in GifImagePlugin.py to contain a log of the image bounding box before and after the '_normalize_palette` call (around line 407):

for imSequence in itertools.chain([im],
                                      im.encoderinfo.get("append_images", [])):
        for im_frame in ImageSequence.Iterator(imSequence):
            # a copy is required here since seek can still mutate the image
            print("NEWFRAME")
            print(im_frame.getbbox())
            im_frame = _normalize_mode(im_frame.copy())
            print("MODE")
            print(im_frame.getbbox())
            im_frame = _normalize_palette(im_frame, palette, im.encoderinfo)
            print("PALETTE")
            print(im_frame.getbbox())

            encoderinfo = im.encoderinfo.copy()
            if isinstance(duration, (list, tuple)):
                encoderinfo['duration'] = duration[frame_count]
            if isinstance(disposal, (list, tuple)):
                encoderinfo["disposal"] = disposal[frame_count]
            frame_count += 1

Each time I see:

NEWFRAME
(0, 0, 100, 100)
MODE
(0, 0, 100, 100)
PALETTE
None

Is it possible that the Image.remap_palette being done by _normalize_palette is putting the only colour as colour 0, which causes the Image.getbbox to see the whole image as colour 0 and therefore not at all filled?

The frame having no bounding box results in the gif optimisation essentially ignoring the frame and extending the previous one.

I ran into this while trying to work on #3665

@radarhere radarhere changed the title Flaw in GifImagePlugin normalize_palette destroys bounding box of frame GifImagePlugin normalize_palette destroys bounding box of frame Mar 23, 2019
@aclark4life aclark4life added this to Backlog in Pillow May 11, 2019
@aclark4life aclark4life moved this from Backlog to In progress in Pillow May 11, 2019
@radarhere
Copy link
Member

radarhere commented Mar 17, 2021

This is an unusual issue, because it's not really a discussion about Pillow's API, or an issue with an example of a broken handling of an image, but about Pillow's internal operations.

I don't think it matters that im_frame.getbbox() is empty - delta.getbbox() is what is used to determine whether or not this frame is different to the previous one.

@radarhere
Copy link
Member

Yes though, you are correct in the behaviour of getbbox() -

Pillow/src/PIL/Image.py

Lines 1233 to 1236 in e2ac1d1

def getbbox(self):
"""
Calculates the bounding box of the non-zero regions in the
image.

And I agree that calling remap_palette() on a single color image with the single item list of that used color does change the color to zero.

return im.remap_palette(used_palette_colors, source_palette)

from PIL import Image
zeros = 0
nonzeros = 0
for c in range(0, 256):
    if Image.new("P", (10, 10), 0).remap_palette([0]).load()[0, 0] == 0:
        zeros += 1
    else:
        nonzeros +=1
print(zeros, nonzeros)

gives

256 0

Comment if you have any further questions.

Also for the record, #3665 has been resolved.

Pillow automation moved this from In progress to Closed Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Pillow
  
Closed
Development

No branches or pull requests

2 participants