Fix load_texture not caching cropped textures

arcade/cache/texture.py

@@ -91,10 +91,14 @@ def put(
         name = texture.cache_name
         if strong:
             self._strong_entries.put(name, texture)
+            if self._strong_file_entries.get(file_path):
+                raise ValueError(f"File path {file_path} already in cache")
             if file_path:
                 self._strong_file_entries.put(str(file_path), texture)
         else:
             self._weak_entires.put(name, texture)
+            if self._weak_file_entries.get(file_path):
+                raise ValueError(f"File path {file_path} already in cache")
             if file_path:
                 self._weak_file_entries.put(str(file_path), texture)
 

arcade/texture/loading.py

@@ -9,7 +9,7 @@
 from arcade.types import RectList
 from arcade.resources import resolve_resource_path
 from arcade.hitbox import HitBoxAlgorithm
-from arcade import cache
+from arcade import cache as _cache
 from arcade import hitbox
 from .texture import Texture, ImageData
 
@@ -43,32 +43,91 @@ def load_texture(
     """
     LOG.info("load_texture: %s ", file_path)
     file_path = resolve_resource_path(file_path)
-    file_path_str = str(file_path)
-    hit_box_algorithm = hit_box_algorithm or hitbox.algo_default
-    image_cache_name = Texture.create_image_cache_name(file_path_str, (x, y, width, height))
+    crop = (x, y, width, height)
+    return _load_or_get_texture(
+        file_path,
+        hit_box_algorithm=hit_box_algorithm,
+        crop=crop,
+    )
 
-    # Check if ths file was already loaded and in cache
-    image_data = cache.image_data_cache.get(image_cache_name)
-    if not image_data:
-        image_data = ImageData(PIL.Image.open(file_path).convert("RGBA"))
-        cache.image_data_cache.put(image_cache_name, image_data)
 
-    # Attempt to find a texture with the same configuration
-    texture = cache.texture_cache.get_with_config(image_data.hash, hit_box_algorithm)
-    if not texture:
+def _load_or_get_texture(
+    file_path: Union[str, Path],
+    hit_box_algorithm: Optional[HitBoxAlgorithm] = None,
+    crop: Tuple[int, int, int, int] = (0, 0, 0, 0),
+    hash: Optional[str] = None,
+) -> Texture:
+    """Load a texture, or return a cached version if it's already loaded."""
+    file_path = resolve_resource_path(file_path)
+    hit_box_algorithm = hit_box_algorithm or hitbox.algo_default
+    image_data: Optional[ImageData] = None
+    texture = None
+
+    # Load the image data from disk or get from cache
+    image_data, cached = _load_or_get_image(file_path, hash=hash)
+    # If the image was fetched from cache we might have cached texture
+    if cached:
+        texture = _cache.texture_cache.get_with_config(image_data.hash, hit_box_algorithm)
+    # If we still don't have a texture, create it
+    if texture is None:
         texture = Texture(image_data, hit_box_algorithm=hit_box_algorithm)
-        texture._file_path = file_path
-        texture._crop_values = x, y, width, height
-        cache.texture_cache.put(texture, file_path=file_path_str)
-
-    # If the crop values give us a different texture, return that instead
-    texture_cropped = texture.crop(x, y, width, height)
-    if texture_cropped != texture:
-        texture = texture_cropped
+        texture.file_path = file_path
+        texture.crop_values = crop
+        _cache.texture_cache.put(texture, file_path=file_path)
+
+    # If we have crop values we need to dig deeper looking for cached versions
+    if crop != (0, 0, 0, 0):
+        image_data = _cache.image_data_cache.get(Texture.create_image_cache_name(file_path, crop))
+        # If we don't have and cached image data we can crop from the base texture
+        if image_data is None:
+            texture = texture.crop(*crop)
+            _cache.texture_cache.put(texture)
+            _cache.image_data_cache.put(Texture.create_image_cache_name(file_path, crop), texture.image_data)
+        else:
+            # We might have a texture for this image data
+            texture = _cache.texture_cache.get_with_config(image_data.hash, hit_box_algorithm)
+            if texture is None:
+                texture = Texture(image_data, hit_box_algorithm=hit_box_algorithm)
+                texture.file_path = file_path
+                texture.crop_values = crop
+                _cache.texture_cache.put(texture, file_path=file_path)
 
     return texture
 
 
+def _load_or_get_image(
+    file_path: Union[str, Path],
+    hash: Optional[str] = None,
+) -> Tuple[ImageData, bool]:
+    """
+    Load an image, or return a cached version
+
+    :param str file_path: Path to image
+    :param str hit_box_algorithm: The hit box algorithm
+    :param hash: Hash of the image
+    :return: Tuple of image data and a boolean indicating if the image
+             was fetched from cache
+    """
+    file_path = resolve_resource_path(file_path)
+    file_path_str = str(file_path)
+    cached = True
+
+    # Do we have cached image data for this file?
+    image_data = _cache.image_data_cache.get(
+        Texture.create_image_cache_name(file_path_str)
+    )
+    if not image_data:
+        cached = False
+        im = PIL.Image.open(file_path).convert("RGBA")
+        image_data = ImageData(im, hash)
+        _cache.image_data_cache.put(
+            Texture.create_image_cache_name(file_path_str),
+            image_data,
+        )
+
+    return image_data, cached
+
+
 def load_texture_pair(
     file_name: str,
     hit_box_algorithm: Optional[HitBoxAlgorithm] = None
@@ -122,10 +181,10 @@ def load_textures(
     image_cache_name = Texture.create_image_cache_name(file_name_str)
 
     # Do we have the image in the cache?
-    image_data = cache.image_data_cache.get(image_cache_name)
+    image_data = _cache.image_data_cache.get(image_cache_name)
     if not image_data:
         image_data = ImageData(PIL.Image.open(resolve_resource_path(file_name)))
-        cache.image_data_cache.put(image_cache_name, image_data)
+        _cache.image_data_cache.put(image_cache_name, image_data)
     image = image_data.image
 
     texture_sections = []
@@ -134,18 +193,18 @@ def load_textures(
 
         # Check if we have already created this sub-image
         image_cache_name = Texture.create_image_cache_name(file_name_str, (x, y, width, height))
-        sub_image = cache.image_data_cache.get(image_cache_name)
+        sub_image = _cache.image_data_cache.get(image_cache_name)
         if not sub_image:
             Texture.validate_crop(image, x, y, width, height)
             sub_image = ImageData(image.crop((x, y, x + width, y + height)))
-            cache.image_data_cache.put(image_cache_name, sub_image)            
+            _cache.image_data_cache.put(image_cache_name, sub_image)            
 
         # Do we have a texture for this sub-image?
         texture_cache_name = Texture.create_cache_name(hash=sub_image.hash, hit_box_algorithm=hit_box_algorithm)
-        sub_texture = cache.texture_cache.get(texture_cache_name)
+        sub_texture = _cache.texture_cache.get(texture_cache_name)
         if not sub_texture:
             sub_texture = Texture(sub_image, hit_box_algorithm=hit_box_algorithm)
-            cache.texture_cache.put(sub_texture)
+            _cache.texture_cache.put(sub_texture)
 
         if mirrored:
             sub_texture = sub_texture.flip_left_to_right()

arcade/texture/texture.py

@@ -17,12 +17,11 @@
     Rotate270Transform,
     TransposeTransform,
     TransverseTransform,
-    # get_shortest_transform,
 )
 from arcade.types import PointList
 from arcade.color import TRANSPARENT_BLACK
 from arcade.hitbox import HitBoxAlgorithm
-from arcade import cache
+from arcade import cache as _cache
 from arcade import hitbox
 
 if TYPE_CHECKING:
@@ -493,7 +492,7 @@ def remove_from_cache(self, ignore_error: bool = True) -> None:
         :param bool ignore_error: If True, ignore errors if the texture is not in the cache
         :return: None
         """
-        cache.texture_cache.delete(self)
+        _cache.texture_cache.delete(self)
 
     def flip_left_to_right(self) -> "Texture":
         """
@@ -640,7 +639,13 @@ def validate_crop(image: PIL.Image.Image, x: int, y: int, width: int, height: in
         if y + height - 1 >= image.height:
             raise ValueError(f"height is outside of texture: {height + y}")
 
-    def crop(self, x: int, y: int, width: int, height: int) -> "Texture":
+    def crop(
+        self,
+        x: int,
+        y: int,
+        width: int,
+        height: int,
+    ) -> "Texture":
         """
         Create a new texture from a sub-section of this texture.
 
@@ -652,6 +657,7 @@ def crop(self, x: int, y: int, width: int, height: int) -> "Texture":
         :param int y: Y position to start crop
         :param int width: Width of crop
         :param int height: Height of crop
+        :param bool cache: If True, the cropped texture will be cached
         :return: Texture 
         """
         # Return self if the crop is the same size as the original image
@@ -667,10 +673,12 @@ def crop(self, x: int, y: int, width: int, height: int) -> "Texture":
         area = (x, y, x + width, y + height)
         image = self.image.crop(area)
         image_data = ImageData(image)
-        return Texture(
+        texture = Texture(
             image_data,
             hit_box_algorithm=self._hit_box_algorithm,
-        )        
+        )
+        texture.crop_values = (x, y, width, height)
+        return texture
 
     def _new_texture_transformed(
         self,
@@ -739,14 +747,14 @@ def _calculate_hit_box_points(self) -> PointList:
         or when the hit box points are requested the first time.
         """
         # Check if we have cached points
-        points = cache.hit_box_cache.get(self.cache_name)
+        points = _cache.hit_box_cache.get(self.cache_name)
         if points:
             return points
 
         # Calculate points with the selected algorithm
         points = self._hit_box_algorithm.calculate(self.image)
         if self._hit_box_algorithm.cache:
-            cache.hit_box_cache.put(self.cache_name, points)
+            _cache.hit_box_cache.put(self.cache_name, points)
 
         return points
 

tests/unit2/test_hit_box_cache.py

@@ -75,6 +75,7 @@ def test_load_texture():
     # We don't cache hit boxes with no algo
     texture = load_texture(file, hit_box_algorithm=hitbox.algo_bounding_box)
     assert arcade.cache.hit_box_cache.get(texture.cache_name) is None
+    assert len(arcade.cache.hit_box_cache) == 0
 
     # We cache hit boxes with an algo
     texture_1 = load_texture(file, hit_box_algorithm=hitbox.algo_simple)

tests/unit2/test_textures.py

@@ -65,8 +65,6 @@ def test_load_texture():
     assert tex.width == 128
     assert tex.height == 128
     assert tex.size == (128, 128)
-    # cache_name = ":resources:images/test_textures/test_texture.png-0-0-0-0-False-False-False-Simple "
-    # assert tex.name == cache_name
     assert tex.hit_box_points is not None
     assert tex._sprite is None
     assert tex._sprite_list is None
@@ -75,6 +73,14 @@ def test_load_texture():
         arcade.load_texture("moo")
 
 
+def test_load_texture_with_cached():
+    path = ":resources:images/test_textures/test_texture.png"
+    texture = arcade.load_texture(path)
+    assert id(texture) == id(arcade.load_texture(path))
+    texture = arcade.load_texture(path, x=0, y=0, width=64, height=64)
+    assert id(texture) == id(arcade.load_texture(path, x=0, y=0, width=64, height=64))
+
+
 def test_load_textures(window):
     """Test load_textures with various parameters"""
     path = ":resources:images/test_textures/test_texture.png"
@@ -147,9 +153,9 @@ def test_texture_equality():
     assert id(t1.image) == id(t2.image)
     # Handle comparing with other objects
     assert t1 != "moo"
-    assert t1 != None
-    assert (t1 == None) is False
-    assert (t1 == "moo") is False
+    assert t1 is not None
+    assert t1 is not None
+    assert t1 != "moo"
 
 
 def test_crate_empty():