matthewwithanm / django-imagekit

Automated image processing for Django. Currently v4.0
http://django-imagekit.rtfd.org/
BSD 3-Clause "New" or "Revised" License
2.26k stars 276 forks source link

How to skip image processing for ProcessedImageField if no processing is needed #484

Open a1tus opened 5 years ago

a1tus commented 5 years ago

So I have a field like that:

class MyModel(models.Model):   
    photo = ProcessedImageField(
        processors=[
            OptionalResizeToFit(3000, 3000, upscale=False),
        ]
    )

and OptionalResizeToFit as follows:

class OptionalResizeToFit(ResizeToFit):
    """
    Do the same as `ResizeToFit` but keep original image
    if its dimensions are less than passed fo resizing

    """

    def process(self, img):
        cur_width, cur_height = img.size
        if (
            (self.width is None or cur_width <= self.width) and
            (self.height is None or cur_height <= self.height)
        ):
            return img
        return super().process(img)

So the goal is to upload files less than 3000x3000 without any processing but resize too big ones. I think it's quite a common case.

But the problem is that processing happens anyway.

ProcessedImageFieldFile calls ImageSpec.generate() inside. It then calls pilkit.utils.process_image which applies all processors (actually doing nothing in my case) and saves image via PIL save and thus modify it.

So I'm not sure how to do it without a lot of monkeypatching.

vstoykov commented 5 years ago

Probably we need to check somehow if the original image is returned from the processor(s) and than do something clever. Instead of monkeypatching on your side do you think that you can make PR with changes directly in ImageKit?

a1tus commented 5 years ago

I've solved the problem with a few tricks and may be this approach after some cleanup can be used. Consider:

class SkipResizeException(Exception):
    pass

class PatchedProcessedImageField(ProcessedImageField):
    attr_class = PatchedProcessedImageFieldFile

class PatchedProcessedImageFieldFile(ImageFieldFile):
    def save(self, name, content, save=True):
        filename, ext = os.path.splitext(name)
        spec = self.field.get_spec(source=content)
        ext = suggest_extension(name, spec.format)
        new_name = '%s%s' % (filename, ext)

        try:
            content = generate(spec)
        except SkipResizeException:
            pass

        return super(PatchedProcessedImageFieldFile, self).save(new_name, content, save)

class OptionalResizeToFit(ResizeToFit):
    """
    Do the same as `ResizeToFit` but return original image
    if its dimensions are less than passed fo resizing

    """

    def process(self, img):
        cur_width, cur_height = img.size
        if (
            (self.width is None or cur_width <= self.width) and
            (self.height is None or cur_height <= self.height)
        ):
            raise SkipResizeException
        return super().process(img)

So basically I just raise exception and pass any processing after that. But it works well only if we have one processor in pipeline.

a1tus commented 5 years ago

Main problem at the moment is in ImageSpec.generate/process_image method/func:

...
def generate(self):
    ...
        try:
            img = open_image(self.source)
            new_image = process_image(img,
                                      processors=self.processors,
                                      format=self.format,
                                      autoconvert=self.autoconvert,
                                      options=self.options)
...
def process_image(img, processors=None, format=None, autoconvert=True, options=None):
    from .processors import ProcessorPipeline

    original_format = img.format

    # Run the processors
    img = ProcessorPipeline(processors or []).process(img)

    format = format or img.format or original_format or 'JPEG'
    options = options or {}
    return img_to_fobj(img, format, autoconvert, **options)

process_image don't check if any processors were applied. In general I would say that we should return the original image if no actions were performed but it will break some backwards compatibility for those people who already rely that all images are resaved. May be we can introduce some setting for that.

Implementation seems quite easy to me so if you are interested I can prepare it.