From 5d9fdf69adb428c3060e14426b0a539ad770a22d Mon Sep 17 00:00:00 2001
From: Martin Owens <doctormo@gmail.com>
Date: Sun, 13 Mar 2022 21:09:20 -0400
Subject: [PATCH] Improve pixmap testing and update clipart extension

---
 inkex/gui/listview.py           |  14 +++--
 inkex/gui/pixmap.py             | 107 ++++++++++++++++++++------------
 other/clipart                   |   2 +-
 tests/test_inkex_gui_pixmaps.py |  69 ++++++++++----------
 4 files changed, 111 insertions(+), 81 deletions(-)

diff --git a/inkex/gui/listview.py b/inkex/gui/listview.py
index 34f448102..56939e721 100644
--- a/inkex/gui/listview.py
+++ b/inkex/gui/listview.py
@@ -458,13 +458,15 @@ class ViewColumn(object):
          size - Restrict the images to this size.
         """
         # Manager where icons will be pulled from
-        pixmaps = pixmaps or PixmapManager("", pixmap_dir="./", size=size)
-        size = SizeFilter(pixmaps, size=size) if size else None
+        filters = [SizeFilter] if size else []
+        pixmaps = pixmaps or PixmapManager(
+            "", pixmap_dir="./", filters=filters, size=size
+        )
 
         renderer = Gtk.CellRendererPixbuf()
         renderer.set_property("ypad", pad)
         renderer.set_property("xpad", pad)
-        func = self.image_func(icon or self.default_icon, pixmaps, size)
+        func = self.image_func(icon or self.default_icon, pixmaps)
         return self.add_renderer(renderer, func, expand=False)
 
     def add_text_renderer(self, text, wrap=None, template=None):
@@ -526,7 +528,7 @@ class ViewColumn(object):
 
         return internal
 
-    def image_func(self, call, pixmaps=None, size=None):
+    def image_func(self, call, pixmaps=None):
         """Wrap, wrap wrap the func"""
         callout = self.get_callout(call)
 
@@ -539,8 +541,8 @@ class ViewColumn(object):
             if isinstance(icon or "", str) and pixmaps:
                 # Expect a Gnome theme icon
                 icon = pixmaps.get(icon)
-            elif icon and size:
-                icon = size.filter(icon)
+            elif icon:
+                icon = pixmaps.apply_filters(icon)
 
             cell.set_property("pixbuf", icon)
             cell.set_property("visible", True)
diff --git a/inkex/gui/pixmap.py b/inkex/gui/pixmap.py
index f805905c3..02f4ce803 100644
--- a/inkex/gui/pixmap.py
+++ b/inkex/gui/pixmap.py
@@ -52,15 +52,19 @@ class PixmapFilter:  # pylint: disable=too-few-public-methods
     """
 
     required: List[str] = []
+    optional: List[str] = []
 
-    def __init__(self, manager, **kwargs):
-        self.manager = manager
-        missing = self.required[:]
-        for key in kwargs:
-            if key in missing:
-                missing.remove(key)
-            setattr(self, key, kwargs[key])
-        self.enabled = len(missing) == 0
+    def __init__(self, **kwargs):
+        self.enabled = True
+        for key in self.required:
+            if key not in kwargs:
+                self.enabled = False
+            else:
+                setattr(self, key, kwargs[key])
+
+        for key in self.optional:
+            if key in kwargs:
+                setattr(self, key, kwargs[key])
 
     def filter(self, img, **kwargs):
         """Run filter, replace this methodwith your own"""
@@ -69,6 +73,15 @@ class PixmapFilter:  # pylint: disable=too-few-public-methods
             % type(self).__name__
         )
 
+    @staticmethod
+    def to_size(dat):
+        """Tries to calculate a size that will work for the data"""
+        if isinstance(dat, (int, float)):
+            return (dat, dat)
+        if isinstance(dat, Iterable) and len(dat) >= 2:
+            return (dat[0], dat[1])
+        return None
+
 
 class OverlayFilter(PixmapFilter):
     """Adds an overlay to output images, overlay can be any name that
@@ -82,19 +95,27 @@ class OverlayFilter(PixmapFilter):
 
     """
 
+    optional = ["position", "overlay", "alpha"]
+
     def __init__(self, *args, **kwargs):
         self.position = (0, 0)
         self.overlay = None
         self.alpha = 255
         super().__init__(*args, **kwargs)
-        self.pad_x, self.pad_y = SizeFilter.to_size(self.position)
+        self.pad_x, self.pad_y = self.to_size(self.position)
 
     def get_overlay(self, **kwargs):
-        return self.manager.get(
-            kwargs.get("overlay", None) or self.overlay, exempt=True
+        if "manager" not in kwargs:
+            raise ValueError("PixmapManager must be provided when adding an overlay.")
+        return kwargs["manager"].get(
+            kwargs.get("overlay", None) or self.overlay, no_overlay=True
         )
 
-    def filter(self, img, **kwargs):
+    def filter(self, img, no_overlay=False, **kwargs):
+        # Recursion protection
+        if no_overlay:
+            return img
+
         overlay = self.get_overlay(**kwargs)
         if overlay:
             img = img.copy()
@@ -128,6 +149,7 @@ class SizeFilter(PixmapFilter):
     """
 
     required = ["size"]
+    optional = ["resize_mode"]
 
     def __init__(self, *args, **kwargs):
         self.size = None
@@ -135,15 +157,6 @@ class SizeFilter(PixmapFilter):
         super().__init__(*args, **kwargs)
         self.img_w, self.img_h = self.to_size(self.size) or (0, 0)
 
-    @staticmethod
-    def to_size(dat):
-        """Tries to calculate a size that will work for the data"""
-        if isinstance(dat, (int, float)):
-            return (dat, dat)
-        if isinstance(dat, Iterable) and len(dat) >= 2:
-            return (dat[0], dat[1])
-        return None
-
     def aspect(self, img_w, img_h):
         """Get the aspect ratio of the image resized"""
         if self.resize_mode == SIZE_STRETCH:
@@ -171,6 +184,8 @@ class SizeFilter(PixmapFilter):
 class PadFilter(SizeFilter):
     """Add padding to the image to make it a standard size"""
 
+    optional = ["padding"]
+
     def __init__(self, *args, **kwargs):
         self.size = None
         self.padding = 0.5
@@ -205,7 +220,7 @@ class PixmapManager:
     default_image = "application-default-icon"
     icon_theme = ICON_THEME
     theme_size = 32
-    filters = [SizeFilter]
+    filters: List[type] = []
     pixmap_dir = None
 
     def __init__(self, location="", **kwargs):
@@ -213,16 +228,16 @@ class PixmapManager:
         if self.pixmap_dir and not os.path.isabs(location):
             self.location = os.path.join(self.pixmap_dir, location)
 
-        self.loader_size = SizeFilter.to_size(kwargs.pop("load_size", None))
-        if "size" not in kwargs:
-            kwargs["size"] = self.theme_size
+        self.loader_size = PixmapFilter.to_size(kwargs.pop("load_size", None))
 
         # Add any instance specified filters first
-        self._filters = kwargs.get("filters", [])
-        for lens in self.filters:
-            # Now add any class specified filters with optional kwargs
-            # Default: SizeFiler( size=required_field )
-            self._filters.append(lens(self, **kwargs))
+        self._filters = []
+        for item in kwargs.get("filters", []) + self.filters:
+            if isinstance(item, PixmapFilter):
+                self._filters.append(item)
+            elif callable(item):
+                # Now add any class specified filters with optional kwargs
+                self._filters.append(item(**kwargs))
 
         self.cache = {}
         self.get_pixmap(self.default_image)
@@ -231,12 +246,16 @@ class PixmapManager:
         """Get a pixmap of any kind"""
         return self.get_pixmap(*args, **kwargs)
 
+    def get_missing_image(self):
+        """Get a missing image when other images aren't found"""
+        return self.get(self.missing_image)
+
     @staticmethod
     def data_is_file(data):
         """Test the file to see if it's a filename or not"""
         return isinstance(data, str) and "<svg" not in data
 
-    def get_pixmap(self, data, exempt=False, **kwargs):
+    def get_pixmap(self, data, **kwargs):
         """
         There are three types of images this might return.
 
@@ -246,6 +265,9 @@ class PixmapManager:
 
         All pixmaps are cached for multiple use.
         """
+        if "manager" not in kwargs:
+            kwargs["manager"] = self
+
         if not data:
             if not self.default_image:
                 return None
@@ -254,22 +276,27 @@ class PixmapManager:
         key = data[-30:]  # bytes or string
         if not key in self.cache:
             # load the image from data or a filename/theme icon
+            img = None
             try:
                 if self.data_is_file(data):
-                    self.cache[key] = self.load_from_name(data)
+                    img = self.load_from_name(data)
                 else:
-                    self.cache[key] = self.load_from_data(data)
+                    img = self.load_from_data(data)
             except PixmapLoadError as err:
                 logging.warning(str(err))
-                return self.missing_image
+                return self.get_missing_image()
+
+            if img is not None:
+                self.cache[key] = self.apply_filters(img, **kwargs)
 
-            # Filer the new image if not exempt from such things
-            if key in self.cache and self.cache[key] and not exempt:
-                for lens in self._filters:
-                    if lens.enabled:
-                        self.cache[key] = lens.filter(self.cache[key], **kwargs)
+        return self.cache[key]
 
-        return self.cache.get(key, self.missing_image)
+    def apply_filters(self, img, **kwargs):
+        """Apply all the filters to the given image"""
+        for lens in self._filters:
+            if lens.enabled:
+                img = lens.filter(img, **kwargs)
+        return img
 
     def load_from_data(self, data):
         """Load in memory picture file (jpeg etc)"""
diff --git a/other/clipart b/other/clipart
index 2c19fe9e9..7655ce9b6 160000
--- a/other/clipart
+++ b/other/clipart
@@ -1 +1 @@
-Subproject commit 2c19fe9e9b9d634086bffe7edeb826ecf54affc0
+Subproject commit 7655ce9b696e8b736e30d035dadcb419a50e393a
diff --git a/tests/test_inkex_gui_pixmaps.py b/tests/test_inkex_gui_pixmaps.py
index e282c27db..6e9c9ec52 100644
--- a/tests/test_inkex_gui_pixmaps.py
+++ b/tests/test_inkex_gui_pixmaps.py
@@ -69,25 +69,21 @@ class GtkPixmapsTest(TestCase):
 
     def test_filter(self):
         """Test building filters and errors"""
-        null_filter = NullFilter(None, underpants=True)
+        null_filter = NullFilter(underpants=True)
         self.assertRaises(NotImplementedError, null_filter.filter, "not")
+        self.assertRaises(ValueError, OverlayFilter().filter, None)
 
     def test_manager_filter_overlay(self):
         """Test overlay filter in use"""
-
-        class MyOverlayFilter(OverlayFilter):
-            overlay = "application-default-icon"
-            placement = (0.25, 0.25)
-
-        pixmaps = self.construct_manager(filters=[MyOverlayFilter])("svg")
-        self.assertTrue(pixmaps.get("colors.svg"))
+        pixmaps = self.construct_manager(filters=[OverlayFilter])("svg")
+        self.assertTrue(pixmaps.get("colors.svg", overlay="application-default-icon"))
 
     def test_manger_filter_size(self):
         """Test resizing a file pixmap"""
-        pixmaps = self.construct_manager()("svg", size=150)
-        pix = pixmaps.get("colors.svg")
-        self.assertEqual(pix.get_width(), 150)
-        self.assertEqual(pix.get_height(), 50)
+        pixmaps = self.construct_manager(filters=[SizeFilter])("svg", size=150)
+        ret = pixmaps.get("colors.svg")
+        self.assertEqual(ret.get_width(), 150)
+        self.assertEqual(ret.get_height(), 50)
         self.assertRaises(PixmapLoadError, pixmaps.load_from_name, "no-file.svg")
         self.assertRaises(
             PixmapLoadError,
@@ -95,72 +91,84 @@ class GtkPixmapsTest(TestCase):
             os.path.join(self.datadir(), "ui", "window-test.ui"),
         )
 
+    def test_missing_image(self):
+        pixmaps = PixmapManager("svg", filters=[SizeFilter(size=25)])
+        img_a = pixmaps.get("NeverExisted.svg")
+        img_b = pixmaps.get(PixmapManager.missing_image)
+        self.assertEqual(img_a.get_width(), 25)
+        self.assertPixbuf(img_a, img_b)
+
     def test_overlay_filter(self):
         """Test overlay filter"""
-        pixmaps = self.construct_manager(filters=[])("svg")
+        pixmaps = self.construct_manager()("svg")
         start = pixmaps.get("gradient_with_mixed_offsets.svg")
 
         # 1. Simple overlay
-        ret = OverlayFilter(pixmaps).filter(start, overlay="colors.svg")
+        ret = OverlayFilter().filter(start, manager=pixmaps, overlay="colors.svg")
         comp = pixmaps.get("img/color_overlay_a.png")
         self.assertPixbuf(ret, comp)
 
         # 2. Overlay at bottom
-        ret = OverlayFilter(pixmaps, position=1).filter(start, overlay="colors.svg")
+        ret = OverlayFilter(position=1).filter(
+            start, manager=pixmaps, overlay="colors.svg"
+        )
         comp = pixmaps.get("img/color_overlay_b.png")
         self.assertPixbuf(ret, comp)
 
     def test_pad_filter(self):
         """Test padding filter"""
-        pixmaps = self.construct_manager(filters=[])("svg")
+        pixmaps = self.construct_manager()("svg")
         start = pixmaps.get("colors.svg")
 
         # 1. Add no padding
-        ret = PadFilter(pixmaps, size=(300, 100)).filter(start)
+        ret = PadFilter(size=(300, 100)).filter(start)
         self.assertPixbuf(ret, start)
 
         # 2a. Add padding at top-left
-        ret = PadFilter(pixmaps, size=300, padding=0.0).filter(start)
+        ret = PadFilter(size=300, padding=0.0).filter(start)
         comp = pixmaps.get("img/color_pad_a.png")
         self.assertPixbuf(ret, comp)
 
         # 12b. Add padding at top-left
-        ret = PadFilter(pixmaps, size=300, padding=1.0).filter(start)
+        ret = PadFilter(size=300, padding=1.0).filter(start)
         comp = pixmaps.get("img/color_pad_b.png")
         self.assertPixbuf(ret, comp)
 
         # 12c. Add padding at top-left
-        ret = PadFilter(pixmaps, size=300, padding=0.5).filter(start)
+        ret = PadFilter(size=300, padding=0.5).filter(start)
         comp = pixmaps.get("img/color_pad_c.png")
         self.assertPixbuf(ret, comp)
 
         # 3. Take image and pad to 1px x 150px
-        ret = PadFilter(pixmaps, size=(1, 150)).filter(start)
+        ret = PadFilter(size=(1, 150)).filter(start)
         self.assertEqual((ret.get_width(), ret.get_height()), (300, 150))
 
     def test_size_filter(self):
         """Test the size pixbuf filter"""
-        pixmaps = self.construct_manager(filters=[])("svg")
+        pixmaps = self.construct_manager()("svg")
         start = pixmaps.get("colors.svg")
         self.assertEqual((start.get_width(), start.get_height()), (300, 100))
 
-        ret = SizeFilter(pixmaps, size=600, resize_mode=SIZE_ASPECT).filter(start)
+        ret = SizeFilter().filter(start)
+        self.assertEqual((ret.get_width(), ret.get_height()), (300, 100))
+
+        ret = SizeFilter(size=600, resize_mode=SIZE_ASPECT).filter(start)
         self.assertEqual((ret.get_width(), ret.get_height()), (300, 100))
         self.assertEqual((start.get_width(), start.get_height()), (300, 100))
 
-        ret = SizeFilter(pixmaps, size=60, resize_mode=SIZE_ASPECT).filter(start)
+        ret = SizeFilter(size=60, resize_mode=SIZE_ASPECT).filter(start)
         self.assertEqual((ret.get_width(), ret.get_height()), (60, 20))
         self.assertEqual((start.get_width(), start.get_height()), (300, 100))
 
-        ret = SizeFilter(pixmaps, size=600, resize_mode=SIZE_ASPECT_GROW).filter(start)
+        ret = SizeFilter(size=600, resize_mode=SIZE_ASPECT_GROW).filter(start)
         self.assertEqual((ret.get_width(), ret.get_height()), (600, 200))
         self.assertEqual((start.get_width(), start.get_height()), (300, 100))
 
-        ret = SizeFilter(pixmaps, size=60, resize_mode=SIZE_ASPECT_CROP).filter(start)
+        ret = SizeFilter(size=60, resize_mode=SIZE_ASPECT_CROP).filter(start)
         self.assertEqual((ret.get_width(), ret.get_height()), (180, 60))
         self.assertEqual((start.get_width(), start.get_height()), (300, 100))
 
-        ret = SizeFilter(pixmaps, size=600, resize_mode=SIZE_STRETCH).filter(start)
+        ret = SizeFilter(size=600, resize_mode=SIZE_STRETCH).filter(start)
         self.assertEqual((ret.get_width(), ret.get_height()), (600, 600))
         self.assertEqual((start.get_width(), start.get_height()), (300, 100))
 
@@ -196,13 +204,6 @@ class GtkPixmapsTest(TestCase):
         pixmaps = self.construct_manager(missing_image="image-missing")()
         self.assertTrue(pixmaps.get("bad-image"))
 
-    def assertFilter(self, filter_cls, input_img, output_img, **kwargs):
-        """Test lots of different filters"""
-        pixmaps = self.construct_manager(filters=[filter_cls])("svg")
-        output_img = pixmaps.get(output_img)
-        compare_img = pixmaps._filters[0].filter(input_img, **kwargs)
-        self.assertPixbuf(output_img, compare_img)
-
     def assertPixbuf(self, img_a, img_b, forgive=0.02):
         """Compare to Gobject pixbufs"""
         try:
-- 
GitLab