From d88a5e596bf2cd6cdf10600c70d23750d86840c4 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Mon, 2 Jun 2014 09:36:06 +0100 Subject: [PATCH 1/6] Cleanup wagtailimages backends code --- wagtail/wagtailimages/backends/base.py | 21 +++++++-------------- wagtail/wagtailimages/backends/pillow.py | 7 +++++-- wagtail/wagtailimages/backends/wand.py | 2 +- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/wagtail/wagtailimages/backends/base.py b/wagtail/wagtailimages/backends/base.py index 25b7e882d2..43ea977c95 100644 --- a/wagtail/wagtailimages/backends/base.py +++ b/wagtail/wagtailimages/backends/base.py @@ -1,6 +1,7 @@ from django.conf import settings from django.core.exceptions import ImproperlyConfigured + class InvalidImageBackendError(ImproperlyConfigured): pass @@ -8,23 +9,22 @@ class InvalidImageBackendError(ImproperlyConfigured): class BaseImageBackend(object): def __init__(self, params): self.quality = getattr(settings, 'IMAGE_COMPRESSION_QUALITY', 85) - + def open_image(self, input_file): """ - Open an image and return the backend specific image object to pass - to other methods. The object return has to have a size attribute + Open an image and return the backend specific image object to pass + to other methods. The object return has to have a size attribute which is a tuple with the width and height of the image and a format attribute with the format of the image. """ raise NotImplementedError('subclasses of BaseImageBackend must provide an open_image() method') - - + def save_image(self, image, output): """ Save the image to the output """ raise NotImplementedError('subclasses of BaseImageBackend must provide a save_image() method') - + def resize(self, image, size): """ resize image to the requested size, using highest quality settings @@ -32,11 +32,9 @@ class BaseImageBackend(object): """ raise NotImplementedError('subclasses of BaseImageBackend must provide an resize() method') - def crop_to_centre(self, image, size): raise NotImplementedError('subclasses of BaseImageBackend must provide a crop_to_centre() method') - def resize_to_max(self, image, size): """ Resize image down to fit within the given dimensions, preserving aspect ratio. @@ -58,10 +56,8 @@ class BaseImageBackend(object): final_size = (target_width, int(original_height * horz_scale)) else: final_size = (int(original_width * vert_scale), target_height) - - return self.resize(image, final_size) - + return self.resize(image, final_size) def resize_to_min(self, image, size): """ @@ -87,7 +83,6 @@ class BaseImageBackend(object): return self.resize(image, final_size) - def resize_to_width(self, image, target_width): """ Resize image down to the given width, preserving aspect ratio. @@ -104,7 +99,6 @@ class BaseImageBackend(object): return self.resize(image, final_size) - def resize_to_height(self, image, target_height): """ Resize image down to the given height, preserving aspect ratio. @@ -121,7 +115,6 @@ class BaseImageBackend(object): return self.resize(image, final_size) - def resize_to_fill(self, image, size): """ Resize down and crop image to fill the given dimensions. Most suitable for thumbnails. diff --git a/wagtail/wagtailimages/backends/pillow.py b/wagtail/wagtailimages/backends/pillow.py index 05d12d291c..96976c277f 100644 --- a/wagtail/wagtailimages/backends/pillow.py +++ b/wagtail/wagtailimages/backends/pillow.py @@ -1,6 +1,9 @@ -from base import BaseImageBackend +from __future__ import absolute_import + +from .base import BaseImageBackend import PIL.Image + class PillowBackend(BaseImageBackend): def __init__(self, params): super(PillowBackend, self).__init__(params) @@ -32,4 +35,4 @@ class PillowBackend(BaseImageBackend): top = (original_height - final_height) / 2 return image.crop( (left, top, left + final_width, top + final_height) - ) \ No newline at end of file + ) diff --git a/wagtail/wagtailimages/backends/wand.py b/wagtail/wagtailimages/backends/wand.py index 36e352715d..0c493eb521 100644 --- a/wagtail/wagtailimages/backends/wand.py +++ b/wagtail/wagtailimages/backends/wand.py @@ -1,9 +1,9 @@ from __future__ import absolute_import from .base import BaseImageBackend - from wand.image import Image + class WandBackend(BaseImageBackend): def __init__(self, params): super(WandBackend, self).__init__(params) From 351b74c94b547b70902bab4a0e6e028738021349 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Mon, 2 Jun 2014 09:52:33 +0100 Subject: [PATCH 2/6] Wand image backend: Always clone images before performing oprations on them --- wagtail/wagtailimages/backends/wand.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/wagtail/wagtailimages/backends/wand.py b/wagtail/wagtailimages/backends/wand.py index 0c493eb521..d1a8774366 100644 --- a/wagtail/wagtailimages/backends/wand.py +++ b/wagtail/wagtailimages/backends/wand.py @@ -18,8 +18,9 @@ class WandBackend(BaseImageBackend): image.save(file=output) def resize(self, image, size): - image.resize(size[0], size[1]) - return image + new_image = image.clone() + new_image.resize(size[0], size[1]) + return new_image def crop_to_centre(self, image, size): (original_width, original_height) = image.size @@ -34,7 +35,9 @@ class WandBackend(BaseImageBackend): left = (original_width - final_width) / 2 top = (original_height - final_height) / 2 - image.crop( + + new_image = image.clone() + new_image.crop( left=left, top=top, right=left + final_width, bottom=top + final_height ) - return image + return new_image From e438a6d99b9d936c2a7ea730a3d09da102dc9d85 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Mon, 2 Jun 2014 10:51:47 +0100 Subject: [PATCH 3/6] Minor wagtailimages cleanup --- wagtail/wagtailimages/models.py | 11 ++++------- wagtail/wagtailimages/tests.py | 16 ++++++++-------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/wagtail/wagtailimages/models.py b/wagtail/wagtailimages/models.py index 24fd1f0bf1..a2be29486e 100644 --- a/wagtail/wagtailimages/models.py +++ b/wagtail/wagtailimages/models.py @@ -180,27 +180,25 @@ class Filter(models.Model): generate an output image with this filter applied, returning it as another django.core.files.File object """ - backend = get_image_backend(backend_name) - + if not self.method: self._parse_spec_string() - + # If file is closed, open it input_file.open('rb') image = backend.open_image(input_file) file_format = image.format - + method = getattr(backend, self.method_name) image = method(image, self.method_arg) output = StringIO.StringIO() backend.save_image(image, output, file_format) - + # and then close the input file input_file.close() - # generate new filename derived from old one, inserting the filter spec string before the extension input_filename_parts = os.path.basename(input_file.name).split('.') @@ -210,7 +208,6 @@ class Filter(models.Model): output_filename = '.'.join(output_filename_parts) output_file = File(output, name=output_filename) - return output_file diff --git a/wagtail/wagtailimages/tests.py b/wagtail/wagtailimages/tests.py index 00d1245e64..1bc2acc93b 100644 --- a/wagtail/wagtailimages/tests.py +++ b/wagtail/wagtailimages/tests.py @@ -84,10 +84,10 @@ class TestRenditions(TestCase): # default backend should be pillow backend = get_image_backend() self.assertTrue(isinstance(backend, PillowBackend)) - + def test_minification(self): rendition = self.image.get_rendition('width-400') - + # Check size self.assertEqual(rendition.width, 400) self.assertEqual(rendition.height, 300) @@ -114,7 +114,7 @@ class TestRenditions(TestCase): # Check that they are the same object self.assertEqual(first_rendition, second_rendition) - + class TestRenditionsWand(TestCase): def setUp(self): @@ -134,18 +134,18 @@ class TestRenditionsWand(TestCase): def test_minification(self): rendition = self.image.get_rendition('width-400') - + # Check size self.assertEqual(rendition.width, 400) self.assertEqual(rendition.height, 300) - + def test_resize_to_max(self): rendition = self.image.get_rendition('max-100x100') - + # Check size self.assertEqual(rendition.width, 100) self.assertEqual(rendition.height, 75) - + def test_resize_to_min(self): rendition = self.image.get_rendition('min-120x120') @@ -160,7 +160,7 @@ class TestRenditionsWand(TestCase): # Check that they are the same object self.assertEqual(first_rendition, second_rendition) - + class TestImageTag(TestCase): def setUp(self): From b5586102ed5ae4bb3f56c33f2dc9de56628fdd2b Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Mon, 2 Jun 2014 10:52:34 +0100 Subject: [PATCH 4/6] Cleanup Filter.process_image a bit --- wagtail/wagtailimages/models.py | 37 ++++++++++++++++----------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/wagtail/wagtailimages/models.py b/wagtail/wagtailimages/models.py index a2be29486e..ddde18d295 100644 --- a/wagtail/wagtailimages/models.py +++ b/wagtail/wagtailimages/models.py @@ -161,15 +161,17 @@ class Filter(models.Model): # and save the results to self.method_name and self.method_arg try: (method_name_simple, method_arg_string) = self.spec.split('-') - self.method_name = Filter.OPERATION_NAMES[method_name_simple] + method_name = Filter.OPERATION_NAMES[method_name_simple] if method_name_simple in ('max', 'min', 'fill'): # method_arg_string is in the form 640x480 (width, height) = [int(i) for i in method_arg_string.split('x')] - self.method_arg = (width, height) + method_arg = (width, height) else: # method_arg_string is a single number - self.method_arg = int(method_arg_string) + method_arg = int(method_arg_string) + + return method_name, method_arg except (ValueError, KeyError): raise ValueError("Invalid image filter spec: %r" % self.spec) @@ -180,36 +182,33 @@ class Filter(models.Model): generate an output image with this filter applied, returning it as another django.core.files.File object """ + # Get image backend backend = get_image_backend(backend_name) - if not self.method: - self._parse_spec_string() - - # If file is closed, open it + # Open image file input_file.open('rb') image = backend.open_image(input_file) file_format = image.format - method = getattr(backend, self.method_name) + # Parse filter spec string + method_name, method_arg = self._parse_spec_string() - image = method(image, self.method_arg) + # Run filter + method_f = getattr(backend, method_name) + image = method_f(image, method_arg) + # Save output = StringIO.StringIO() backend.save_image(image, output, file_format) - # and then close the input file + # Close input file input_file.close() - # generate new filename derived from old one, inserting the filter spec string before the extension - input_filename_parts = os.path.basename(input_file.name).split('.') - filename_without_extension = '.'.join(input_filename_parts[:-1]) - filename_without_extension = filename_without_extension[:60] # trim filename base so that we're well under 100 chars - output_filename_parts = [filename_without_extension, self.spec] + input_filename_parts[-1:] - output_filename = '.'.join(output_filename_parts) + # Generate new filename derived from old one, inserting the filter spec string before the extension + filename_noext, ext = os.path.splitext(input_file.name) + output_filename = '.'.join([filename_noext[:60], self.spec]) + ext - output_file = File(output, name=output_filename) - - return output_file + return File(output, name=output_filename) class AbstractRendition(models.Model): From a6df4699979988bed0098c128490ddf24c4a91f7 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 10 Jun 2014 15:31:03 +0100 Subject: [PATCH 5/6] Coalesce animated gifs before resizing them. Fixes #264 In an animated gif, each frame is only the size it needs to be to cover the changed part of the image. This is often much lower than the actual image size and the Wand image backend was using this incorrect size in its resizing calculations, causing all animated gifs to be resized incorrectly or not at all. This commit calls ImageMagicks "MagickCoalesceImages" on the wand image. This resizes every frame to the full size of the image so the resizing calculations will work properly. --- wagtail/wagtailimages/backends/wand.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/wagtail/wagtailimages/backends/wand.py b/wagtail/wagtailimages/backends/wand.py index d1a8774366..91f2d255a7 100644 --- a/wagtail/wagtailimages/backends/wand.py +++ b/wagtail/wagtailimages/backends/wand.py @@ -2,6 +2,7 @@ from __future__ import absolute_import from .base import BaseImageBackend from wand.image import Image +from wand.api import library class WandBackend(BaseImageBackend): @@ -10,6 +11,7 @@ class WandBackend(BaseImageBackend): def open_image(self, input_file): image = Image(file=input_file) + image.wand = library.MagickCoalesceImages(image.wand) return image def save_image(self, image, output, format): From a26c0bcfa0c3d4994dc72ea206a34c029cbbd185 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 10 Jun 2014 15:39:28 +0100 Subject: [PATCH 6/6] Revert "Cleanup Filter.process_image a bit" This reverts commit b5586102ed5ae4bb3f56c33f2dc9de56628fdd2b. --- wagtail/wagtailimages/models.py | 37 +++++++++++++++++---------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/wagtail/wagtailimages/models.py b/wagtail/wagtailimages/models.py index ddde18d295..a2be29486e 100644 --- a/wagtail/wagtailimages/models.py +++ b/wagtail/wagtailimages/models.py @@ -161,17 +161,15 @@ class Filter(models.Model): # and save the results to self.method_name and self.method_arg try: (method_name_simple, method_arg_string) = self.spec.split('-') - method_name = Filter.OPERATION_NAMES[method_name_simple] + self.method_name = Filter.OPERATION_NAMES[method_name_simple] if method_name_simple in ('max', 'min', 'fill'): # method_arg_string is in the form 640x480 (width, height) = [int(i) for i in method_arg_string.split('x')] - method_arg = (width, height) + self.method_arg = (width, height) else: # method_arg_string is a single number - method_arg = int(method_arg_string) - - return method_name, method_arg + self.method_arg = int(method_arg_string) except (ValueError, KeyError): raise ValueError("Invalid image filter spec: %r" % self.spec) @@ -182,33 +180,36 @@ class Filter(models.Model): generate an output image with this filter applied, returning it as another django.core.files.File object """ - # Get image backend backend = get_image_backend(backend_name) - # Open image file + if not self.method: + self._parse_spec_string() + + # If file is closed, open it input_file.open('rb') image = backend.open_image(input_file) file_format = image.format - # Parse filter spec string - method_name, method_arg = self._parse_spec_string() + method = getattr(backend, self.method_name) - # Run filter - method_f = getattr(backend, method_name) - image = method_f(image, method_arg) + image = method(image, self.method_arg) - # Save output = StringIO.StringIO() backend.save_image(image, output, file_format) - # Close input file + # and then close the input file input_file.close() - # Generate new filename derived from old one, inserting the filter spec string before the extension - filename_noext, ext = os.path.splitext(input_file.name) - output_filename = '.'.join([filename_noext[:60], self.spec]) + ext + # generate new filename derived from old one, inserting the filter spec string before the extension + input_filename_parts = os.path.basename(input_file.name).split('.') + filename_without_extension = '.'.join(input_filename_parts[:-1]) + filename_without_extension = filename_without_extension[:60] # trim filename base so that we're well under 100 chars + output_filename_parts = [filename_without_extension, self.spec] + input_filename_parts[-1:] + output_filename = '.'.join(output_filename_parts) - return File(output, name=output_filename) + output_file = File(output, name=output_filename) + + return output_file class AbstractRendition(models.Model):