Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
a1tus opened this issue Jan 19, 2019 · 3 comments

Comments

@a1tus
Copy link
Contributor

a1tus commented Jan 19, 2019

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
Copy link
Collaborator

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
Copy link
Contributor Author

a1tus commented Feb 3, 2019

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
Copy link
Contributor Author

a1tus commented Feb 3, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants