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

Added setting for converting GIF P frames to RGB #6150

Merged
merged 9 commits into from Mar 30, 2022
Binary file added Tests/images/no_palette.gif
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/images/no_palette_with_transparency.gif
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
84 changes: 69 additions & 15 deletions Tests/test_file_gif.py
Expand Up @@ -62,12 +62,46 @@ def test_invalid_file():
def test_l_mode_transparency():
with Image.open("Tests/images/no_palette_with_transparency.gif") as im:
assert im.mode == "L"
assert im.load()[0, 0] == 0
assert im.load()[0, 0] == 128
assert im.info["transparency"] == 255

im.seek(1)
assert im.mode == "LA"
assert im.load()[0, 0] == (0, 255)
assert im.mode == "L"
assert im.load()[0, 0] == 128


def test_strategy():
with Image.open("Tests/images/chi.gif") as im:
expected_zero = im.convert("RGB")

im.seek(1)
expected_one = im.convert("RGB")

try:
GifImagePlugin.PALETTE_TO_RGB = GifImagePlugin.ModeStrategy.ALWAYS
with Image.open("Tests/images/chi.gif") as im:
assert im.mode == "RGB"
assert_image_equal(im, expected_zero)

GifImagePlugin.PALETTE_TO_RGB = (
GifImagePlugin.ModeStrategy.DIFFERENT_PALETTE_ONLY
)
# Stay in P mode with only a global palette
with Image.open("Tests/images/chi.gif") as im:
assert im.mode == "P"

im.seek(1)
assert im.mode == "P"
assert_image_equal(im.convert("RGB"), expected_one)

# Change to RGB mode when a frame has an individual palette
with Image.open("Tests/images/iss634.gif") as im:
assert im.mode == "P"

im.seek(1)
assert im.mode == "RGB"
finally:
GifImagePlugin.PALETTE_TO_RGB = GifImagePlugin.ModeStrategy.AFTER_FIRST


def test_optimize():
Expand Down Expand Up @@ -394,18 +428,38 @@ def test_dispose_background_transparency():
assert px[35, 30][3] == 0


def test_transparent_dispose():
expected_colors = [
(2, 1, 2),
((0, 255, 24, 255), (0, 0, 255, 255), (0, 255, 24, 255)),
((0, 0, 0, 0), (0, 0, 255, 255), (0, 0, 0, 0)),
]
with Image.open("Tests/images/transparent_dispose.gif") as img:
for frame in range(3):
img.seek(frame)
for x in range(3):
color = img.getpixel((x, 0))
assert color == expected_colors[frame][x]
@pytest.mark.parametrize(
"mode_strategy, expected_colors",
(
(
GifImagePlugin.ModeStrategy.AFTER_FIRST,
(
(2, 1, 2),
((0, 255, 24, 255), (0, 0, 255, 255), (0, 255, 24, 255)),
((0, 0, 0, 0), (0, 0, 255, 255), (0, 0, 0, 0)),
),
),
(
GifImagePlugin.ModeStrategy.DIFFERENT_PALETTE_ONLY,
(
(2, 1, 2),
(0, 1, 0),
(2, 1, 2),
),
),
),
)
def test_transparent_dispose(mode_strategy, expected_colors):
GifImagePlugin.PALETTE_TO_RGB = mode_strategy
try:
with Image.open("Tests/images/transparent_dispose.gif") as img:
for frame in range(3):
img.seek(frame)
for x in range(3):
color = img.getpixel((x, 0))
assert color == expected_colors[frame][x]
finally:
GifImagePlugin.PALETTE_TO_RGB = GifImagePlugin.ModeStrategy.AFTER_FIRST


def test_dispose_previous():
Expand Down
23 changes: 21 additions & 2 deletions docs/handbook/image-file-formats.rst
Expand Up @@ -106,8 +106,27 @@ writes run-length encoded files in GIF87a by default, unless GIF89a features
are used or GIF89a is already in use.

GIF files are initially read as grayscale (``L``) or palette mode (``P``)
images, but seeking to later frames in an image will change the mode to either
``RGB`` or ``RGBA``, depending on whether the first frame had transparency.
images. Seeking to later frames in a ``P`` image will change the image to
``RGB`` (or ``RGBA`` if the first frame had transparency).

``P`` mode images are changed to ``RGB`` because each frame of a GIF may
introduce up to 256 colors. Because ``P`` can only have up to 256 colors, the
image is converted to handle all of the colors.

If you would prefer the first ``P`` image frame to be ``RGB`` as well, so that
every ``P`` frame is converted to ``RGB`` or ``RGBA`` mode, there is a setting
available::

from PIL import GifImagePlugin
GifImagePlugin.PALETTE_TO_RGB = GifImagePlugin.ModeStrategy.ALWAYS

GIF frames do not always contain individual palettes however. If there is only
a global palette, then all of the colors can fit within ``P`` mode. If you would
prefer the frames to be kept as ``P`` in that case, there is also a setting
available::
Comment on lines +124 to +127
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't what DIFFERENT_PALETTE_ONLY does. It still combines the frames. [...]

Oh, then I've misunderstood what DIFFERENT_PALETTE_ONLY does. What do you think of changing the part of the docs that threw me?

Suggested change
GIF frames do not always contain individual palettes however. If there is only
a global palette, then all of the colors can fit within ``P`` mode. If you would
prefer the frames to be kept as ``P`` in that case, there is also a setting
available::
GIF frames do not always contain individual palettes, however. If there is only
a global palette, then frames may still use values of previous frames, but all
of the colors can fit within ``P`` mode. If you would prefer the frames to be
served as ``P`` in that case, there is also a setting available::

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hope is that a previous part of the documentation already conveys the idea that new frames are layered on top of old modes.

P mode images are changed to RGB because each frame of a GIF may introduce up to 256 colors. Because P can only have up to 256 colors, the image is converted to handle all of the colors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the part that you referenced provides the motivation for why the image is converted to RGB. What isn't clear yet (or at least wasn't clear to me when I read it) is that this behavior also applies if we don't convert to RGB. Maybe we can rephrase the sentence you referenced to better convey that decoding happens in all cases:

Transparency allows a GIF frame to use more than 256 colors, 256 from a local palette + other colors from previous frames. To handle this correctly, images will - by default - be promoted to RGB. While this recombination (decoding) will always happen, you can control how the result will be returned.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed a commit. What do you think?


from PIL import GifImagePlugin
GifImagePlugin.PALETTE_TO_RGB = GifImagePlugin.ModeStrategy.DIFFERENT_PALETTE_ONLY

The :py:meth:`~PIL.Image.open` method sets the following
:py:attr:`~PIL.Image.Image.info` properties:
Expand Down