Fix note from review

src/pretix/helpers/templatetags/thumb.py

@@ -65,7 +65,7 @@ def thumbset(source, arg):
     if not cached_thumbset:
         for thumbsize, factor in get_srcset_sizes(arg):
             try:
-                t = get_thumbnail(source, thumbsize, formats=formats, only_if_resized=True)
+                t = get_thumbnail(source, thumbsize, formats=formats, skip_if_limited_by_input=True)
                 if t:
                     srcs.append(f"{t.thumb.url} {factor}")
             except:

src/pretix/helpers/thumb.py

@@ -114,6 +114,9 @@ def get_srcset_sizes(size):
 
 
 def get_sizes(size, imgsize):
+    """
+    :return: Tuple of (new_size, crop_box, size_limited_by_input)
+    """
     crop = False
     if size.endswith('^'):
         crop = True
@@ -129,38 +132,49 @@ def get_sizes(size, imgsize):
     else:
         size = [int(size), int(size)]
 
+    wfactor = min(1, size[0] / imgsize[0])
+    hfactor = min(1, size[1] / imgsize[1])
+    limited_by_input = size[1] / imgsize[1] > 1.0 and size[0] / imgsize[0] > 1.0
+    print(size, imgsize, size[1] / imgsize[1], size[0] / imgsize[0], limited_by_input)
+
     if crop:
         # currently crop and min-size cannot be combined
-        wfactor = min(1, size[0] / imgsize[0])
-        hfactor = min(1, size[1] / imgsize[1])
         if wfactor == hfactor:
-            return (int(imgsize[0] * wfactor), int(imgsize[1] * hfactor)), \
-                   (0, int((imgsize[1] * wfactor - imgsize[1] * hfactor) / 2),
-                    imgsize[0] * hfactor, int((imgsize[1] * wfactor + imgsize[1] * wfactor) / 2))
+            return (
+                (int(imgsize[0] * wfactor), int(imgsize[1] * hfactor)),
+                (0, int((imgsize[1] * wfactor - imgsize[1] * hfactor) / 2),
+                 imgsize[0] * hfactor, int((imgsize[1] * wfactor + imgsize[1] * wfactor) / 2)),
+                limited_by_input
+            )
         elif wfactor > hfactor:
-            return (int(size[0]), int(imgsize[1] * wfactor)), \
-                   (0, int((imgsize[1] * wfactor - size[1]) / 2), size[0], int((imgsize[1] * wfactor + size[1]) / 2))
+            return (
+                (int(size[0]), int(imgsize[1] * wfactor)),
+                (0, int((imgsize[1] * wfactor - size[1]) / 2), size[0], int((imgsize[1] * wfactor + size[1]) / 2)),
+                limited_by_input
+            )
         else:
-            return (int(imgsize[0] * hfactor), int(size[1])), \
-                   (int((imgsize[0] * hfactor - size[0]) / 2), 0, int((imgsize[0] * hfactor + size[0]) / 2), size[1])
+            return (
+                (int(imgsize[0] * hfactor), int(size[1])),
+                (int((imgsize[0] * hfactor - size[0]) / 2), 0, int((imgsize[0] * hfactor + size[0]) / 2), size[1]),
+                limited_by_input
+            )
     else:
-        wfactor = min(1, size[0] / imgsize[0])
-        hfactor = min(1, size[1] / imgsize[1])
-
         if wfactor == hfactor:
-            return (int(imgsize[0] * hfactor), int(imgsize[1] * wfactor)), None
+            return (int(imgsize[0] * hfactor), int(imgsize[1] * wfactor)), None, limited_by_input
         elif wfactor < hfactor:
-            return (size[0], int(imgsize[1] * wfactor)), None
+            return (size[0], int(imgsize[1] * wfactor)), None, limited_by_input
         else:
-            return (int(imgsize[0] * hfactor), size[1]), None
+            return (int(imgsize[0] * hfactor), size[1]), None, limited_by_input
 
 
 def resize_image(image, size):
+    """
+    :return: Tuple of (new_image, size_limited_by_input)
+    """
     # before we calc thumbnail, we need to check and apply EXIF-orientation
     image = ImageOps.exif_transpose(image)
-    old_size = image.size
 
-    new_size, crop = get_sizes(size, image.size)
+    new_size, crop, limited_by_input = get_sizes(size, image.size)
     image = image.resize(new_size, resample=Resampling.LANCZOS)
     if crop:
         image = image.crop(crop)
@@ -179,12 +193,11 @@ def resize_image(image, size):
         new_y = (image.height - new_height) // 2
 
         image = image.crop((new_x, new_y, new_x + new_width, new_y + new_height))
-        new_size = image.size
 
-    return image, new_size != old_size
+    return image, limited_by_input
 
 
-def create_thumbnail(source, size, formats=None, only_if_resized=False):
+def create_thumbnail(source, size, formats=None, skip_if_limited_by_input=False):
     source_name = str(source)
 
     # HACK: this ensures that the file is opened in binary mode, which is not guaranteed otherwise, esp. for
@@ -200,15 +213,15 @@ def create_thumbnail(source, size, formats=None, only_if_resized=False):
         raise ThumbnailError('Could not load image')
 
     frames = []
-    any_resized = False
+    any_limited_by_input = False
     durations = []
     for f in ImageSequence.Iterator(image):
         durations.append(f.info.get("duration", 1000))
-        img, resized = resize_image(f, size)
-        any_resized = any_resized or resized
+        img, limited_by_input = resize_image(f, size)
+        any_limited_by_input = any_limited_by_input or limited_by_input
         frames.append(img)
 
-    if not any_resized and only_if_resized:
+    if any_limited_by_input and skip_if_limited_by_input:
         return
 
     image_out = frames[0]
@@ -249,10 +262,10 @@ def create_thumbnail(source, size, formats=None, only_if_resized=False):
     return t
 
 
-def get_thumbnail(source, size, formats=None, only_if_resized=False):
+def get_thumbnail(source, size, formats=None, skip_if_limited_by_input=False):
     # Assumes files are immutable
     try:
         source_name = str(source)
         return Thumbnail.objects.get(source=source_name, size=size)
     except Thumbnail.DoesNotExist:
-        return create_thumbnail(source, size, formats=formats, only_if_resized=only_if_resized)
+        return create_thumbnail(source, size, formats=formats, skip_if_limited_by_input=skip_if_limited_by_input)

src/tests/helpers/test_thumb.py

@@ -19,105 +19,213 @@
 # You should have received a copy of the GNU Affero General Public License along with this program.  If not, see
 # <https://www.gnu.org/licenses/>.
 #
+import io
+import re
+
+import pytest
+from django.core.files.base import ContentFile
+from django.core.files.storage import default_storage
 from PIL import Image
 
+from pretix.helpers.templatetags.thumb import thumbset
 from pretix.helpers.thumb import resize_image
 
 
 def test_no_resize():
     img = Image.new('RGB', (40, 20))
-    img, resized = resize_image(img, "100x100")
+    img, limited_by_input = resize_image(img, "100x100")
     width, height = img.size
-    assert not resized
+    assert limited_by_input
     assert width == 40
     assert height == 20
 
     img = Image.new('RGB', (40, 20))
-    img, resized = resize_image(img, "100x100^")
+    img, limited_by_input = resize_image(img, "100x100^")
     width, height = img.size
-    assert not resized
+    assert limited_by_input
+    assert width == 40
+    assert height == 20
+
+    img = Image.new('RGB', (40, 20))
+    img, limited_by_input = resize_image(img, "40x20^")
+    width, height = img.size
+    assert not limited_by_input
     assert width == 40
     assert height == 20
 
 
 def test_resize():
     img = Image.new('RGB', (40, 20))
-    img, resized = resize_image(img, "10x10")
+    img, limited_by_input = resize_image(img, "10x10")
     width, height = img.size
-    assert resized
+    assert not limited_by_input
     assert width == 10
     assert height == 5
 
     img = Image.new('RGB', (40, 20))
-    img, resized = resize_image(img, "100x10")
+    img, limited_by_input = resize_image(img, "100x10")
     width, height = img.size
-    assert resized
+    assert not limited_by_input
     assert width == 20
     assert height == 10
 
     img = Image.new('RGB', (40, 20))
-    img, resized = resize_image(img, "10x100")
+    img, limited_by_input = resize_image(img, "10x100")
     width, height = img.size
-    assert resized
+    assert not limited_by_input
     assert width == 10
     assert height == 5
 
 
 def test_crop():
     img = Image.new('RGB', (40, 20))
-    img, resized = resize_image(img, "10x10^")
+    img, limited_by_input = resize_image(img, "10x10^")
     width, height = img.size
-    assert resized
+    assert not limited_by_input
     assert width == 10
     assert height == 10
 
+    img = Image.new('RGB', (40, 20))
+    img, limited_by_input = resize_image(img, "40x20^")
+    width, height = img.size
+    assert not limited_by_input
+    assert width == 40
+    assert height == 20
+
+    img = Image.new('RGB', (40, 20))
+    img, limited_by_input = resize_image(img, "50x30^")
+    width, height = img.size
+    assert limited_by_input
+    assert width == 40
+    assert height == 20
+
 
 def test_exactsize():
     img = Image.new('RGB', (6912, 3456))
-    img, resized = resize_image(img, "600_x5000")
+    img, limited_by_input = resize_image(img, "600_x5000")
     width, height = img.size
-    assert resized
+    assert not limited_by_input
     assert width == 600
     assert height == 300
 
     img = Image.new('RGB', (60, 20))
-    img, resized = resize_image(img, "10_x10")
+    img, limited_by_input = resize_image(img, "10_x10")
     width, height = img.size
-    assert resized
+    assert not limited_by_input
     assert width == 10
     assert height == 3
 
     img = Image.new('RGB', (10, 20))
-    img, resized = resize_image(img, "10_x10")
+    img, limited_by_input = resize_image(img, "10_x10")
     width, height = img.size
-    assert resized
+    assert not limited_by_input
     assert width == 10
     assert height == 10
 
     img = Image.new('RGB', (60, 20))
-    img, resized = resize_image(img, "10x10_")
+    img, limited_by_input = resize_image(img, "10x10_")
     width, height = img.size
-    assert resized
+    assert not limited_by_input
     assert width == 10
     assert height == 10
 
     img = Image.new('RGB', (20, 60))
-    img, resized = resize_image(img, "10x10_")
+    img, limited_by_input = resize_image(img, "10x10_")
     width, height = img.size
-    assert resized
+    assert not limited_by_input
     assert width == 3
     assert height == 10
 
     img = Image.new('RGB', (20, 60))
-    img, resized = resize_image(img, "10_x10_")
+    img, limited_by_input = resize_image(img, "10_x10_")
     width, height = img.size
-    assert resized
+    assert not limited_by_input
     assert width == 10
     assert height == 10
 
     img = Image.new('RGB', (20, 60))
-    img, resized = resize_image(img, "100_x100_")
+    img, limited_by_input = resize_image(img, "100_x100_")
     width, height = img.size
-    assert resized
+    assert limited_by_input
     assert width == 100
     assert height == 100
+
+    img = Image.new('RGB', (20, 60))
+    img, limited_by_input = resize_image(img, "20_x60_")
+    width, height = img.size
+    assert not limited_by_input
+    assert width == 20
+    assert height == 60
+
+
+def _create_img(size):
+    img = Image.new('RGB', size)
+    with io.BytesIO() as output:
+        img.save(output, format="PNG")
+        contents = output.getvalue()
+    return default_storage.save("_".join(str(a) for a in size) + ".png", ContentFile(contents))
+
+
+@pytest.mark.django_db
+def test_thumbset():
+    # Product picture example
+    img = _create_img((60, 60))
+    assert not thumbset(img, "60x60^")
+
+    img = _create_img((110, 110))
+    assert not thumbset(img, "60x60^")
+
+    img = _create_img((120, 120))
+    assert re.match(
+        r".*\.120x120c\.png 2x$",
+        thumbset(img, "60x60^"),
+    )
+
+    img = _create_img((150, 150))
+    assert re.match(
+        r".*\.120x120c\.png 2x$",
+        thumbset(img, "60x60^"),
+    )
+
+    img = _create_img((180, 180))
+    assert re.match(
+        r".*\.120x120c\.png 2x, .*\.180x180c.png 3x$",
+        thumbset(img, "60x60^"),
+    )
+
+    img = _create_img((500, 500))
+    assert re.match(
+        r".*\.120x120c\.png 2x, .*\.180x180c.png 3x$",
+        thumbset(img, "60x60^"),
+    )
+
+    # Event logo (large version) example
+    img = _create_img((400, 200))
+    assert not thumbset(img, "1170x5000")
+
+    img = _create_img((1170, 120))
+    assert not thumbset(img, "1170x5000")
+
+    img = _create_img((2340, 240))
+    assert re.match(
+        r".*\.2340x10000\.png 2x$",
+        thumbset(img, "1170x5000"),
+    )
+
+    img = _create_img((2925, 180))
+    assert re.match(
+        r".*\.2340x10000\.png 2x$",
+        thumbset(img, "1170x5000"),
+    )
+
+    img = _create_img((3510, 360))
+    assert re.match(
+        r".*\.2340x10000\.png 2x, .*\.3510x15000.png 3x$",
+        thumbset(img, "1170x5000"),
+    )
+
+    img = _create_img((4680, 480))
+    assert re.match(
+        r".*\.2340x10000\.png 2x, .*\.3510x15000.png 3x$",
+        thumbset(img, "1170x5000"),
+    )