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.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_ALWAYS
with Image.open("Tests/images/chi.gif") as im:
assert im.mode == "RGB"
assert_image_equal(im, expected_zero)

GifImagePlugin.LOADING_STRATEGY = (
GifImagePlugin.LoadingStrategy.RGB_AFTER_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.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_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(
"loading_strategy, expected_colors",
(
(
GifImagePlugin.LoadingStrategy.RGB_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.LoadingStrategy.RGB_AFTER_DIFFERENT_PALETTE_ONLY,
(
(2, 1, 2),
(0, 1, 0),
(2, 1, 2),
),
),
),
)
def test_transparent_dispose(loading_strategy, expected_colors):
GifImagePlugin.LOADING_STRATEGY = loading_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.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_AFTER_FIRST


def test_dispose_previous():
Expand Down
24 changes: 22 additions & 2 deletions docs/handbook/image-file-formats.rst
Expand Up @@ -106,8 +106,28 @@ 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 contain
its own individual palette of up to 256 colors. When a new frame is placed onto a
previous frame, those colors may combine to exceed the ``P`` mode limit of 256
colors. Instead, the image is converted to ``RGB`` handle this.

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.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_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.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_AFTER_DIFFERENT_PALETTE_ONLY
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be worth noting the name of the default option:

Suggested change
GifImagePlugin.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_AFTER_DIFFERENT_PALETTE_ONLY
GifImagePlugin.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_AFTER_DIFFERENT_PALETTE_ONLY
To restore the default behavior of reading the first frame in ``P`` mode and converting to ``RGB`` for subsequent frames::
from PIL import GifImagePlugin
GifImagePlugin.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_AFTER_FIRST

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I've pushed a commit with a variation of this.


The :py:meth:`~PIL.Image.open` method sets the following
:py:attr:`~PIL.Image.Image.info` properties:
Expand Down
20 changes: 20 additions & 0 deletions docs/releasenotes/9.1.0.rst
Expand Up @@ -160,6 +160,26 @@ Added PyEncoder
written in Python. See :ref:`Writing Your Own File Codec in Python<file-codecs-py>` for
more information.

GifImagePlugin loading strategy
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Pillow 9.0.0 introduced the conversion of subsequent GIF frames to ``RGB`` or ``RGBA``. This
behaviour can now be changed so that the first ``P`` frame is converted to ``RGB`` as
well.

.. code-block:: python

from PIL import GifImagePlugin
GifImagePlugin.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_ALWAYS

Or subsequent frames can be kept in ``P`` mode as long as there is only a single
palette.

.. code-block:: python

from PIL import GifImagePlugin
GifImagePlugin.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_AFTER_DIFFERENT_PALETTE_ONLY

Other Changes
=============

Expand Down