Skip to content

Enable default crops. #13

Closed
jensens opened this Issue Mar 21, 2013 · 11 comments

5 participants

@jensens
Plone Collective member
jensens commented Mar 21, 2013

Problem: User upload image and need to active select each crop. Theres no default crop.

New Enhancement: User Upload image and for scales where default cropping is enabled senseful defaults are set. There are two possible defaults: start from left top and scale crop or center and scale crop.

Benefit: Less work for editors.

Works best together with #11

@jensens
Plone Collective member
jensens commented Mar 21, 2013

memo: default centered scaled cropping is done in archetypes.clippingimage

@frisi
Plone Collective member
frisi commented Mar 21, 2013

this definitely would make the editor's lifes a lot easier.

we also thought about this (see section Possible extensions / changes for the future in the README )

technically we'd need to allow admins/integrators to define which scales are auto cropped (as you pointed out needs to be done the same way as #11)

the bigger part, however, is to make plone.app.imaging aware of auto-cropped scales so it does the magic in it's traverser.
pretty similar to https://github.com/plone/plone.app.imaging/blob/ggozad-cropping/src/plone/app/imaging/monkey.py

when creating this package we stuck to our design descisions to not touch/patch plone.app.imaging.

in case plone.app.imagecropping will make it into plone core, adapting plone.app.imaging and therefore supporting auto-crop functionality will be a huge improvement to the user experience.
(in this case the settings of #11 should go into the p.a.imaging control panel as p.a.imaging will need them to create scales)

as long as it's an addon, i think we'll get more happy users if it does not interfere with other parts of plone.

@petschki and @joka: what do you think about that?

@jensens
Plone Collective member
jensens commented Mar 21, 2013

I would like to write a subscriber to the appropriate event (when image was saved, probably some object event - need to read code first here) which sets the auto-calculated crop scale image as it is done in editor with the manual values. I hope we have such an event already in p.a.imaging - if not it would not be intrusive to add it. or are there any better ideas?

@petschki
Plone Collective member

i like the idea of using events. I think we can use zope.lifecycle.ObjectModified event on all objects which implements IImageCropping ... then it shouldn't be too hard to add an auto_crop function to the @@crop-image view which iters through the imagefields and applies autocropping information on the allowed scales.

maybe there should be a selection in the settings where you can choose top/left, center/center, etc autocropping behaviours?

@frisi @joka thoughts?

@frisi
Plone Collective member
frisi commented Mar 25, 2013

using events for this seems like an elevant solution. one downside is, that we create blobs for croppable scale in advanced (they might never get accessed) whereas p.a.imaging creates scales the time they are accessed the first time.

in case creating auto-cropped scales via a subscriber in p.a.imagecropping i'm fine with keeping the settings alongside with the croppable scales introduced in #11
if it's done in p.a.imaging, settings should be there.

not 100% sure if zope.lifecycle.ObjectModified is a good choice for archetypes content. it might get fired multiple times by portal_factory. if that's the case using the archetypes events and having a subscriber for created and modified will work for sure. alternatively we can add some check if there is a cropped scale for exactly this image already.

about the selection of cropping strategies:
i also think it's a good idea to have these configurable

this could either be set

  • on scale level

    auto-crop scales:
    
    thumb: top
    preview: bottom
    large: center
    
  • or be used for all auto-cropped scales

personally i think per scale level might be "too much". people can overrule the auto-cropped image anyway if it does not please them.

@witsch
Plone Collective member
witsch commented Mar 25, 2013

using events for this seems like an elevant solution. one downside is, that we create blobs for croppable scale in advanced (they might never get accessed)

another problem with pre-processing scales is when the definition of the default crops is changed. it's rather impractical to iterate through all images in the site and recreate their scales in that case.

whereas p.a.imaging creates scales the time they are accessed the first time.

that's also the reason for this "on demand" creation in p.a.imaging (even though it violates the principal that a GET shouldn't cause a db write)

@jensens
Plone Collective member
jensens commented Mar 29, 2013

I though that on-demand creation is what p.a.imagecropping does and on set just the parameters for the scales are written (in both cases, at+dx). Need to dig into that part a bit before coming up with an implementation.

@petschki
Plone Collective member

p.a.imagecropping (ab)uses the on-demand creation of plone.app.imaging and plone.scale (publishTraverse adapter ... see plone.app.imaging.traverse) when setting the cropping information. the harder part was to remove this information, because p.a.imaging never removes rendered scales. it just renders a new one with a new key, when modification date is after the existing scale. if rendering autocropped information maybe it could use the same logic of adapting the baseobject with a publishtraverser ...

@hvelarde
Plone Collective member

default centered scaled cropping is going to produce poor thumbs in so many cases that it will make this feature useless for most people; I would suggest to implement alternative cropping mechanisms as well.

I did a quick research on the web and found this Python code for automatic face detection and cropping:

http://stackoverflow.com/questions/13211745/detect-face-then-autocrop-pictures

I think this could add a lot of value to the overall solution (@jsbueno take a look on it, you'll love it).

we are currently testing collective.cover with plone.app.imagecropping and I can tell you it's a killer combination for news sites.

@frisi
Plone Collective member
frisi commented Apr 2, 2014

another idea for default cropping would be to allow users to define a center point for the cropped images. one could point on the person's nose and make sure the most relevant portion of the face is contained in every crop.
of course this would require to add the "centerpoint" functionality to the cropping view. for usecases with a lot of cropped scales this could make a lot of sense though.

@petschki petschki referenced this issue in 4teamwork/plone.app.imaging Nov 11, 2014
Open

Merging the cropping feature from 1.0.5-ftw1 into 1.0.9. #1

@jensens
Plone Collective member
jensens commented May 12, 2016

outdated, works in new implementation

@jensens jensens closed this May 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.