Bump pillow to 10.4.0 and mode check PyMunkHitBoxAlgorithm (#2375)

* Bump pillow to 10.4.0

* Replace the use of deprecated / optional attributes

* Use getattr for is_animated attribute per the pillow doc

* Use getattr for the n_frames attribute as well

* Annotate and document load_animated_gif

* Use str | Path annotation

* Add an Args block documenting resource_name

* Cross-ref the resource handles page in the programming guide

* Use 10.4.0 transpose constants

* Fix PymunkHitBoxAlgorithm.trace_image types + doc

* Add ValueError when given non-RGBA image

* Add a test for the ValueError

* Annotate trace_image's argument

* Correct pyright issues inside it

* Elaborate on the doc and fix issues with it

* Fix pyright issue with tilemap loading

* Explain the tile map changes

* Silence mypy's issue with Image.Image.open returning ImageFile instead of Image

* Run ./make.py format

Author: pushfoo

arcade/context.py

@@ -492,7 +492,7 @@ def load_texture(
 
         path = resolve(path)
 
-        image = Image.open(str(path))
+        image: Image.Image = Image.open(str(path))  # type: ignore
 
         if flip:
             image = image.transpose(Image.Transpose.FLIP_TOP_BOTTOM)

arcade/hitbox/pymunk.py

@@ -9,7 +9,7 @@
     simplify_curves,
 )
 
-from arcade.types import Point2, Point2List
+from arcade.types import RGBA255, Point2, Point2List
 
 from .base import HitBoxAlgorithm
 
@@ -92,18 +92,40 @@ def to_points_list(self, image: Image, line_set: list[Vec2d]) -> Point2List:
 
     def trace_image(self, image: Image) -> PolylineSet:
         """
-        Trace the image and return a list of line sets.
+        Trace the image and return a :py:class:~collections.abc.Sequence` of line sets.
 
-        These line sets represent the outline of the image or the outline of the
-        holes in the image. If more than one line set is returned it's important
-        to pick the one that covers the most of the image.
+        .. important:: The image :py:attr:`~PIL.Image.Image.mode` must be ``"RGBA"``!
+
+                       * This method raises a :py:class:`TypeError` when it isn't
+                       * Use :py:meth:`convert("RGBA") <PIL.Image.Image.convert>` to
+                         convert
+
+        The returned object will be a :py:mod:`pymunk`
+        :py:class:`~pymunk.autogeometry.PolylineSet`. Each
+        :py:class:`list` inside it will contain points as
+        :py:class:`pymunk.vec2d.Vec2d` instances. These lists
+        may represent:
+
+        * the outline of the image's contents
+        * the holes in the image
+
+        When this method returns more than one line set,
+        it's important to pick the one which covers the largest
+        portion of the image.
 
         Args:
-            image: Image to trace.
+            image: A :py:class:`PIL.Image.Image` to trace.
+
+        Returns:
+            A :py:mod:`pymunk` object which is a :py:class:`~collections.abc.Sequence`
+            of :py:class:`~pymunk.autogeometry.PolylineSet` of line sets.
         """
+        if image.mode != "RGBA":
+            raise ValueError("Image's mode!='RGBA'! Try using image.convert(\"RGBA\").")
 
         def sample_func(sample_point: Point2) -> int:
-            """Method used to sample image."""
+            """Function used to sample image."""
+            # Return 0 when outside of bounds
             if (
                 sample_point[0] < 0
                 or sample_point[1] < 0
@@ -113,7 +135,8 @@ def sample_func(sample_point: Point2) -> int:
                 return 0
 
             point_tuple = int(sample_point[0]), int(sample_point[1])
-            color = image.getpixel(point_tuple)
+            color: RGBA255 = image.getpixel(point_tuple)  # type: ignore
+
             return 255 if color[3] > 0 else 0
 
         # Do a quick check if it is a full tile

arcade/sprite/__init__.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+from pathlib import Path
+
 import PIL.Image
 
 from arcade.texture import Texture
@@ -22,9 +24,9 @@
 )
 
 
-def load_animated_gif(resource_name) -> TextureAnimationSprite:
+def load_animated_gif(resource_name: str | Path) -> TextureAnimationSprite:
     """
-    Attempt to load an animated GIF as an :class:`TextureAnimationSprite`.
+    Attempt to load an animated GIF as a :class:`TextureAnimationSprite`.
 
     .. note::
 
@@ -33,16 +35,27 @@ def load_animated_gif(resource_name) -> TextureAnimationSprite:
         the format better, loading animated GIFs will be pretty buggy. A
         good workaround is loading GIFs in another program and exporting them
         as PNGs, either as sprite sheets or a frame per file.
+
+    Args:
+        resource_name: A path to a GIF as either a :py:class:`pathlib.Path`
+            or a :py:class:`str` which may include a
+            :ref:`resource handle <resource_handles>`.
+
     """
 
     file_name = resolve(resource_name)
     image_object = PIL.Image.open(file_name)
-    if not image_object.is_animated:
+
+    # Pillow doc recommends testing for the is_animated attribute as of 10.0.0
+    # https://pillow.readthedocs.io/en/stable/deprecations.html#categories
+    if not getattr(image_object, "is_animated", False) or not (
+        n_frames := getattr(image_object, "n_frames", 0)
+    ):
         raise TypeError(f"The file {resource_name} is not an animated gif.")
 
     sprite = TextureAnimationSprite()
     keyframes = []
-    for frame in range(image_object.n_frames):
+    for frame in range(n_frames):
         image_object.seek(frame)
         frame_duration = image_object.info["duration"]
         image = image_object.convert("RGBA")

arcade/texture/loading.py

@@ -2,7 +2,7 @@
 
 from pathlib import Path
 
-import PIL.Image
+from PIL import Image
 
 from arcade.hitbox import HitBoxAlgorithm
 from arcade.resources import resolve
@@ -52,7 +52,7 @@ def load_texture(
     if isinstance(file_path, str):
         file_path = resolve(file_path)
 
-    im = PIL.Image.open(file_path)
+    im: Image.Image = Image.open(file_path)  # type: ignore
     if im.mode != "RGBA":
         im = im.convert("RGBA")
 
@@ -66,7 +66,7 @@ def load_image(
     file_path: str | Path,
     *,
     mode: str = "RGBA",
-) -> PIL.Image.Image:
+) -> Image.Image:
     """
     Load a Pillow image from disk (no caching).
 
@@ -86,9 +86,10 @@ def load_image(
     if isinstance(file_path, str):
         file_path = resolve(file_path)
 
-    im = PIL.Image.open(file_path)
+    im: Image.Image = Image.open(file_path)  # type: ignore
     if im.mode != mode:
         im = im.convert(mode)
+
     return im
 
 
@@ -103,7 +104,7 @@ def load_spritesheet(file_name: str | Path) -> SpriteSheet:
     if isinstance(file_name, str):
         file_name = resolve(file_name)
 
-    im = PIL.Image.open(file_name)
+    im: Image.Image = Image.open(file_name)
     if im.mode != "RGBA":
         im = im.convert("RGBA")
 

arcade/texture/spritesheet.py

@@ -4,6 +4,7 @@
 from typing import TYPE_CHECKING, Literal
 
 from PIL import Image
+from PIL.Image import Transpose
 
 from arcade.resources import resolve
 
@@ -91,14 +92,14 @@ def flip_left_right(self) -> None:
         """
         Flips the internal image left to right.
         """
-        self._image = self._image.transpose(Image.FLIP_LEFT_RIGHT)
+        self._image = self._image.transpose(Transpose.FLIP_LEFT_RIGHT)
         self._flip_flags = (not self._flip_flags[0], self._flip_flags[1])
 
     def flip_top_bottom(self) -> None:
         """
         Flip the internal image top to bottom.
         """
-        self._image = self._image.transpose(Image.FLIP_TOP_BOTTOM)
+        self._image = self._image.transpose(Transpose.FLIP_TOP_BOTTOM)
         self._flip_flags = (self._flip_flags[0], not self._flip_flags[1])
 
     def get_image(

arcade/texture_atlas/atlas_default.py

@@ -14,6 +14,7 @@
 
 import PIL.Image
 from PIL import Image, ImageDraw
+from PIL.Image import Resampling
 from pyglet.image.atlas import (
     Allocator,
     AllocatorException,
@@ -504,10 +505,10 @@ def write_image(self, image: PIL.Image.Image, x: int, y: int) -> None:
 
             # Resize the strips to the border size if larger than 1
             if self._border > 1:
-                strip_top = strip_top.resize((image.width, self._border), Image.NEAREST)
-                strip_bottom = strip_bottom.resize((image.width, self._border), Image.NEAREST)
-                strip_left = strip_left.resize((self._border, image.height), Image.NEAREST)
-                strip_right = strip_right.resize((self._border, image.height), Image.NEAREST)
+                strip_top = strip_top.resize((image.width, self._border), Resampling.NEAREST)
+                strip_bottom = strip_bottom.resize((image.width, self._border), Resampling.NEAREST)
+                strip_left = strip_left.resize((self._border, image.height), Resampling.NEAREST)
+                strip_right = strip_right.resize((self._border, image.height), Resampling.NEAREST)
 
             tmp.paste(strip_top, (self._border, 0))
             tmp.paste(strip_bottom, (self._border, tmp.height - self._border))

arcade/tilemap/tilemap.py

@@ -13,6 +13,7 @@
 import math
 import os
 from collections import OrderedDict
+from collections.abc import Sequence
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, Callable, cast
 
@@ -30,6 +31,7 @@
     get_window,
 )
 from arcade.hitbox import HitBoxAlgorithm, RotatableHitBox
+from arcade.types import RGBA255
 from arcade.types import Color as ArcadeColor
 
 if TYPE_CHECKING:
@@ -699,7 +701,10 @@ def _process_image_layer(
         )
 
         if layer.transparent_color:
-            data = my_texture.image.getdata()
+            # The pillow source doesn't annotate a return type for this method, but:
+            # 1. The docstring does specify the returned object is sequence-like
+            # 2. We convert to RGBA mode implicitly in load_or_get_texture above
+            data: Sequence[RGBA255] = my_texture.image.getdata()  # type:ignore
 
             target = layer.transparent_color
             new_data = []

pyproject.toml

@@ -24,7 +24,7 @@ dependencies = [
     # "pyglet@git+https://github.com/pyglet/pyglet.git@development#egg=pyglet",
     # Expected future dev preview release on PyPI (not yet released)
     'pyglet==2.1.dev5',
-    "pillow~=10.2.0",
+    "pillow~=10.4.0",
     "pymunk~=6.6.0",
     "pytiled-parser~=2.2.5",
 ]

tests/unit/physics_engine/test_pymunk.py

@@ -1,5 +1,9 @@
+import pymunk.autogeometry
 import pytest
+from PIL import Image
+
 import arcade
+from arcade.hitbox import PymunkHitBoxAlgorithm
 
 
 def test_pymunk():
@@ -55,3 +59,38 @@ def test_pymunk_add_sprite_moment_backwards_compatibility(moment_of_inertia_arg_
     set_moment = physics_engine.get_physics_object(sprite).body.moment
 
     assert set_moment == arcade.PymunkPhysicsEngine.MOMENT_INF
+
+
+def test_pymunk_hitbox_algorithm_trace_image_only_takes_rgba():
+    """Test whether non-RGBA modes raise a ValueError.
+
+    We expect the hitbox algo to take RGBA image because the alpha
+    channel is how we determine whether a pixel is empty. See the
+    pillow doc for more on the modes offered:
+    https://pillow.readthedocs.io/en/stable/handbook/concepts.html#modes
+    """
+
+    algo = PymunkHitBoxAlgorithm()
+    def mode(m: str) -> Image.Image:
+        return Image.new(
+            m, # type: ignore
+            (10, 10), 0)
+
+    with pytest.raises(ValueError):
+        algo.trace_image(mode("1"))
+
+    with pytest.raises(ValueError):
+        algo.trace_image(mode("L"))
+
+    with pytest.raises(ValueError):
+        algo.trace_image(mode("P"))
+
+    with pytest.raises(ValueError):
+        algo.trace_image(mode("RGB"))
+
+    with pytest.raises(ValueError):
+        algo.trace_image(mode("HSV"))
+
+    assert isinstance(
+        algo.trace_image(mode("RGBA")), pymunk.autogeometry.PolylineSet)
+