Skip to content

Purpose of the ImageProcessor's "crop" functionality #113

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

Closed
lelit opened this issue Aug 3, 2018 · 5 comments
Closed

Purpose of the ImageProcessor's "crop" functionality #113

lelit opened this issue Aug 3, 2018 · 5 comments
Labels
Milestone

Comments

@lelit
Copy link
Contributor

lelit commented Aug 3, 2018

I hesitate to pose a probably dummy question, but here it is anyway, possibly just for my own curiosity: what's the "use case" for the hardwired crop functionality in the ImageProcessor?

While implementing the PIL variant (issue #84, now in its own branch), I struggled to come up with a compatible implementation of the existing wand-based crop functionality: even if it basically works, I think it would need some more tests and refinements.

Recently, actually using the library, I could not find a reasonable use case for an hardwired definition of the crop-area: if any, that comes from the final user (maybe thru some UI), that alone can say what the interesting part of the image is.

Reading issue #107, I wonder if we should remove the declarative crop functionality from the processor, replacing it with a get_crop() API, similar to get_thumbnail() as suggested there...

What do you think?

@meyt
Copy link
Contributor

meyt commented Aug 3, 2018

@lelit you've referred to a very good point, as what i recently involved with, cropping is not simple, and currently in sqlalchemy-media we have this useless functionality.

crop should be content-aware, we cannot crop an image just with hard positioning!

@pylover what you think about drop the crop ?

@jpmn
Copy link
Contributor

jpmn commented Aug 3, 2018

I would suggest splitting the work on the image from the generate_x function (it could simply call it though). Also, the attachment currently does pop('thumbnails, None) in attach, and we should consider removing that, since it shouldn't know about images and thumbnails (would I need to add pop('crops', None) in attach too?).

Here's what I came up with quickly to see how I could do it (mostly copy pasted from the thumbnails).

  • It still requires work on validation of inputs, for example: (left, top) and ((right, bottom) or (width, height)).
  • It would probably best to generate thumbnails and crop using the same format as the original image as we most likely want to keep transparency when using pngs (maybe by default or specified as an argument).
  • Should it handle an auto_adjust flag to automatically adjust the bounding box if the values are "outside" the original image's dimensions?
BoundingBox = Tuple[int, int, int, int]
class CropIsNotAvailableError(SqlAlchemyMediaException):
    """
    Raised when requested crop is not available(generated) yet.

    """
class CropImage(BaseImage):
    """
    Representing an image crop.

    You may use :meth:`.generate_crop` and or :meth:`.get_crop` with ``auto_generate=True``
    to get one.

    """

    __directory__ = 'crops'
    __prefix__ = 'crop'

    def attach(self, *args, boundingBox: BoundingBox=None, **kwargs):
        """
        A new overload for :meth:`.Attachment.attach`, which accepts one additional
        argument: ``dimension``.

        :param args: The same as the: :meth:`.Attachment.attach`.
        :param dimension: Image (width, height).
        :param kwargs: The same as the: :meth:`.Attachment.attach`.

        :return: The same as the: :meth:`.Attachment.attach`.

        """
        if boundingBox:
            kwargs['left'], kwargs['top'], kwargs['width'], kwargs['height'] = boundingBox

        return super().attach(*args, **kwargs)

    @property
    def left(self):
        return self.get('left')

    @property
    def top(self):
        return self.get('top')
class ImageWithCrop(Image):

    __crop_type__ = CropImage

    @property
    def crops(self) -> List[Tuple[int, int, int, int, CropImage]]:
        """
        A ``List[Tuple[int, int, int, int, CropImage]]``, to hold crops.

        You may use :meth:`.generate_crop` and or :meth:`.get_crop` with ``auto_generate=True``
        to fill it.

        """
        return self.get('crops', [])

    def generate_crop(self, left: int=0, top: int=0, right: int=None, bottom: int=None,
                      width: int=None, height: int=None) -> CropImage:
        """

        .. versionadded:: 0.X

        Generates and stores a crop with the given arguments.

        :param left: The left of the crop.
        :param top: The top of the crop.
        :param right: The right of the crop.
        :param bottom: The bottom of the crop.
        :param width: The width of the crop.
        :param height: The height of the crop.
        :return: the Newly generated :class:`.CropImage` instance.

        """

        def validate_right_bottom_width_height(right, bottom, width, height):
            return width, height

        # Validating parameters
        width, height = validate_right_bottom_width_height(right, bottom, width, height)

        CompatibleImage = get_image_factory()
        # opening the original file
        crop_buffer = io.BytesIO()
        store = self.get_store()
        with store.open(self.path) as original_file:

            # generating crop and storing in buffer
            # noinspection PyTypeChecker
            img = CompatibleImage(file=original_file)
            img.format = self.get('content_type').split('/')[1]

            with img:
                img.crop(left=left, top=top, width=width, height=height)
                img.save(file=crop_buffer)

        crop_buffer.seek(0)

        crop = self.__crop_type__.create_from(
            crop_buffer,
            content_type='image/jpeg',
            extension='.jpg',
            boundingBox=(left, top, width, height)
        )
        self.crops.append((left, top, width, height, crop))

        return crop

    def get_crop(self, left: int=0, top: int=0, right: int=None, bottom: int=None,
                 width: int=None, height: int=None, auto_generate: bool=False) -> CropImage:
        """

        .. versionadded:: 0.X

        Search for the crop with given arguments, if ``auto_generate`` is :data:`.False`, the
        :exc:`.CropIsNotAvailableError` will be raised, else tries to call
        the :meth:`generate_crop` to create a new one.

        :param left: Left of the crop to search for.
        :param top: Top of the crop to search for.
        :param right: Right of the crop to search for.
        :param bottom: Bottom of the crop to search for.
        :param width: Width of the crop to search for.
        :param height: Height of the crop to search for.
        :param auto_generate: If :data:`.True`, tries to generate a new crop.

        :return: :class:`.CropImage` instance.

        .. warning:: if ``auto_generate`` is :data:`.True`, you have to commit the session,
        to store the generated crops.

        """

        for x, y, w, h, c in self.crops:
            if x == left and y == top and w == width and h == height:
                return self.__crop_type__(c)

        # crop not found
        if auto_generate:
            return self.generate_crop(left=left, top=top, width=width, height=height)
        else:
            raise CropIsNotAvailableError(
                f'Crop is not available with these criteria:
                left={left} top={top} width={width} height={height}'
            )

    def get_objects_to_delete(self):
        """
        Returns the files to be deleted, if the attachment is marked for deletion.

        """
        yield from super().get_objects_to_delete()
        if self.crops:
            for c in self.crops:
                yield self.__crop_type__(**c[4])

    def get_orphaned_objects(self):
        """
        Mark crops for deletion when the :class:`.Image` is being deleted.
        :return: An iterable of :class:`.CropImage` to mark as orphan.

        .. versionadded:: 0.X.0

        """
        yield from super().get_objects_to_delete()

        if not self.crops:
            return

        for c in self.crops:
            yield self.__crop_type__(c[4])

@pylover
Copy link
Owner

pylover commented Aug 3, 2018

Users may use the crop functionality with known coordinations.

For example, getting the crop area from the user while uploading the photo within mobile app and or web front-end. Also, I already used this functionality. How you found it useless?

First of all, it's not useless and if you cannot use it, please do not use it.

@pylover
Copy link
Owner

pylover commented Aug 3, 2018

@jpmn, I suggest you to just crop one time, right when importing the image. and all thumbnails will be generated from the cropped image, So I think the store of a collection of cropped images is not necessary.

@jpmn
Copy link
Contributor

jpmn commented Aug 3, 2018

My use case is to keep the original image and all the crops coming from it. For example, I take an image of an overview of multiple objects, and I detect all objects in the image and generate cropped images for each of them. I don't need thumbnails of the cropped images.

Using what is already available, how can I create a crop from dynamic parameters? The usage of the ImageProcessor seems to be using hardcoded values only. I would like to apply the ImageProcessor at anytime on an existing image, maybe simply creating a new Image object. I could handle the relationship between the cropped images and the original image externally too.

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

No branches or pull requests

4 participants