matthewwithanm / django-imagekit

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

Replacement for crop_horz_field and crop_vert_field? #108

Closed Matt408 closed 12 years ago

Matt408 commented 12 years ago

I'm attempting to upgrade from a previous version (0.3.6) and have been changing my code to refer to use the new API.

I haven't been able to find a replacement, however, for crop_horz_field and crop_vert_field in the former IKOptions, which allow my users to specify anchor points in the database record when they upload a photo.

I've come across the new way of specifying an anchor point in the ImageSpec, but I'm not certain how to tie it to a database field.

matthewwithanm commented 12 years ago

You should be able to do that like this:

class MyModel(models.Model):
    thumbnail = ImageSpecField(processors=get_processors, ...)

def get_processors(file, instance):
    return [Crop(100, 100, anchor=(instance.horiz_field, instance.vert_field))]

(This is with 2.0.)

Note that the components of anchor are normalized to the range 0-1 instead of being arbitrary integers. 0 represents left in the horizontal position and top in the vertical; 1 represents right in the horizonal position and bottom in the vertial; anything between 0 and 1 represents a percentage in the same difficult-to-put-into-words-but-not-difficult-to-understand way as CSS background position percentages.

Matt408 commented 12 years ago

Thanks for your help and all the improvements you're making.

I tried your suggested code, and it partially works. (Side note: I needed to define get_processors before my model to avoid a NameError.)

Unfortunately, when I change the anchor field values in the admin, it doesn't update the cached image. Deleting the cached image file causes it to be recreated, but still using the original anchor field values. For the new image to reflect the updated anchor values, I need to restart Django.

matthewwithanm commented 12 years ago

Can you share your model please?

Matt408 commented 12 years ago

Here it is:

def get_processors(file, instance):
    return [resize.ResizeToFill(200,200, anchor=(instance.horizontal_anchor, instance.vertical_anchor))]

class Photo(models.Model):
    title = models.CharField(max_length=255, blank=False)
    original_image = models.ImageField(upload_to='photos')
    credit = models.CharField(max_length=255, blank=True)
    vertical_crop = models.SmallIntegerField(choices=VERTICAL_CROP_CHOICES, default=1)
    horizontal_crop = models.SmallIntegerField(choices=HORIZONTAL_CROP_CHOICES, default=1)
    source_note = models.TextField(blank=True, help_text="Notes about the origin of an image - for internal use.")
    big = ImageSpecField(processors=get_processors, image_field="original_image", format="JPEG")
    vertical_anchor = models.DecimalField(null=True, default=0.5, max_digits=4, decimal_places=3)
    horizontal_anchor = models.DecimalField(null=True, default=0.5, max_digits=4, decimal_places=3)

    class Meta:
        ordering = ["title"]

    def __unicode__(self):
        return self.title

Thanks again.

matthewwithanm commented 12 years ago

Sorry, I was too busy to realize this right away: this is actually expected behavior. As of 1956e16, spec images are only invalidated when the name of the source file changes. (This was something that was discussed a little in the original image cache backend thread — #89.) The previous behavior was that the file was invalidated every time the model was saved. The reasoning behind the change is that it drastically reduces the number of unnecessary invalidations.

The argument could be made that this is not desirable and that the previous behavior made more sense; in other words, that the file should be invalidated on every save, regardless of whether the source file has changed. Right now, I think that's probably not a good idea; here's why:

Validating on every save obviously produces false positives (extra invalidations). As this situation shows, validating only when the source file changes produces false negatives (doesn't invalidate enough). If it were just a matter of choosing between the two, I'd say we should go with the false positives; however, it turns out that validating on every save also allows for false negatives! Here are two simple examples:

def get_processors(file, instance):
    return [resize.ResizeToFill(200, 200,
            anchor=(instance.related_object.horizontal_anchor,
            instance.related_object.vertical_anchor))]

Or

def get_processors(file, instance):
    if datetime.now() > instance.some_prop:
        return [SOME_PROCESSOR]
    else:
        return [SOME_OTHER_PROCESSOR]

In both cases, the processors have changed even though the model has not been saved. In fact, there are infinite ways to accomplish this when your processors are dynamic, and invalidating every save only catches an arbitrary handful of these.

So what's the solution? Well, right now it's "invalidate the image when you change something." In your case, that probably means adding receivers for the post_init and post_save signals, comparing the property values in post_save, and calling instance.spec.invalidate() if they've changed:

def post_init_receiver(sender, instance, **kwargs):
    instance._old_anchor_x = instance.anchor_x
    instance._old_anchor_y = instance.anchor_y

def _post_save_receiver(sender, instance=None, created=False, raw=False, **kwargs):
    if not raw:
        if instance._old_anchor_x != instance.anchor_x or instance._old_anchor_y != instance.anchor_y:
            instance._old_anchor_x = instance.anchor_x
            instance._old_anchor_y = instance.anchor_y
            instance.myspec.invalidate()

Actually, this is basically the exact same thing we're doing to see if the source file has changed.

Obviously, at the very very least, we need to document the necessity of manual invalidation when using a callable for a processor list. That's a minimum.

The next question is, "Can we make this easier?" I think we can but, honestly, I hadn't really considered this issue before so I'm not ready with an answer. If you have any thoughts, I'd love to hear them.

Matt408 commented 12 years ago

This rationale makes sense, even thought the behavior isn't what I would have ordinarily expected. While I haven't used Django receivers before, they seem reasonably straightforward.

For now, I'm going to postpone further work on my own project until you release version 2.0, but it's been helpful to try some of this out and get a better sense of what I'll need to do to upgrade.

Thanks again.

kuyucakli commented 12 years ago

def post_init_receiver(sender, instance, **kwargs): instance._old_anchor_x = instance.anchor_x instance._old_anchor_y = instance.anchor_y

def _post_save_receiver(sender, instance=None, created=False, raw=False, **kwargs): if not raw: if instance._old_anchor_x != instance.anchor_x or instance._old_anchor_y != instance.anchor_y: instance._old_anchor_x = instance.anchor_x instance._old_anchor_y = instance.anchor_y instance.myspec.invalidate()

Can you please explain that part deeper.. Because i am using callables on "cache_dir" and "processors". And when saving file with dynamic names sometimes file can not be saved with the updated names. (is it because of that i do not write the two functions you mentioned). Thank you for your patience...

matthewwithanm commented 12 years ago

@mthueson Do you have any ideas about how we can improve it? I have one floating around in my head but I'd like to hear other people's thoughts before I get started on it.

@burakk I'm not sure I understand your problem—specifically, what you mean by the "file can not be saved with the updated names." Do you get an error?

As for explaining the code: let's say you have a processor that (for some reason) imprints the current date on your image. ImageKit has no way of knowing that a new image needs to be regenerated every day, so you have to tell it, by calling invalidate()—perhaps in a cron job that runs at a minute past midnight every day. In @mthueson's case, the images need to be regenerated every time certain properties on his model are changed. ImageKit doesn't know about this behavior so, again, the image needs to be invalidated manually. The receivers are just a means for recognizing when those properties change.

Matt408 commented 12 years ago

No specific ideas, but thanks for checking. Perhaps there's some way to simplify the creation of the necessary receivers in the most common scenarios, but proposing anything more specific than that is probably beyond my own knowledge.

kuyucakli commented 12 years ago

I persist in using imagekit in my very first django project so I want to write my problem in details.

1 - In my admin add image form i am using an ajax uploader. After the user uploads the image (i have now first temp image in my upload folder that must be deleted) via ajax uploader i am showing the image in a crop resize area whenever user changes top left, width, height values, passing them to form dynamically.

2- In my model I am using django "upload_to" with a callable function and in that function i am renaming file with the slug field's value. (i have now second temp image in my upload folder that must be deleted)

3- Again in my model i have two ImageSpecFields, "big" and "thumb" (these are the final images that must be in upload folder ) with callables for processors and cache_to. I am again renaming file with the slug field's value in the cache_to function because i want it to override the second saved image. (that its path saved to the database)

And this is the code i am using... It gives me " [Errno 2] No such file or directory: u'/home/burak/mydjangosite/static/uploads/images/bigs/blog_main/fetze.jpeg' " like errors

Again thank you for your patience.

# Models.py

#callable-imageSpec-#

def getThumb(instance, file):         
        return [Adjust(contrast=1.2, sharpness=1.1),
                resize.BasicResize(width=instance.thumbW, height=instance.thumbH),
                crop.BasicCrop(x=instance.left_thumb, y=instance.top_thumb, width=instance.thumb_width, height=instance.thumb_height) ]

#callable-imageSpec-#

def getBig(instance, file):         
        return [Adjust(contrast=1.2, sharpness=1.1),
                resize.BasicResize(width=instance.bigW, height=instance.bigH),
                crop.BasicCrop(x=instance.left, y=instance.top, width=instance.width, height=instance.height) ]

#callable-imageSpec-#

def cache_to_big(instance, path, specname, extension):
        path="static/uploads/images/bigs/"+instance.album.name+"/"+instance.slug+extension
        return path

#callable-imageSpec-#

def cache_to_thumb(instance, path, specname, extension):
        path="static/uploads/images/thumbs/"+instance.album.name+"/"+instance.slug+extension
        return path

#callable-django image field-#

def get_path(instance, filename):       
    directory = "static/uploads/images/bigs/"+instance.album.name+"/"      
    name = instance.slug    
    return "%s/%s.%s" % (directory, name, "jpeg")

class Image(Media): 
    tags = TagField()     
    width = models.IntegerField(blank=True, null=True)
    height = models.IntegerField(blank=True, null=True)
    thumb_width = models.IntegerField(blank=True, null=True) 
    thumb_height = models.IntegerField(blank=True, null=True) 
    top = models.IntegerField(default=0, blank=True, null=True)
    left = models.IntegerField(default=0, blank=True, null=True)
    top_thumb = models.IntegerField(default=0, blank=True, null=True)
    left_thumb = models.IntegerField(default=0, blank=True, null=True)
    thumbW = models.IntegerField(blank=True, null=True )
    thumbH = models.IntegerField(blank=True, null=True)
    bigW = models.IntegerField(blank=True, null=True)
    bigH = models.IntegerField(blank=True, null=True)
    file = models.ImageField(upload_to=get_path)    

    thumbnail = ImageSpecField( processors = getThumb, format="JPEG", image_field='file', options={'quality': 90},     cache_to=cache_to_thumb)

    big = ImageSpecField( processors = getBig, format="JPEG", image_field='file', options={'quality': 90}, cache_to=cache_to_big)

    def save(self, *args, **kwargs):  
        super(Image, self).save(*args, **kwargs) 
        test = Image.objects.all()[0]  
        test.thumbnail.width  
        test.big.width        

    def __unicode__(self):  
        return self.title
matthewwithanm commented 12 years ago

@burakk Sorry it's taken so long to get back to you, but there's a lot of stuff to digest there! If you want us to help you troubleshoot something specifically related to IK, really the best thing you could do is create a reduced test case: boil your model down to the bare minimum thing that still gives you the problem and share that.

I'm a little confused by some of your comments and I think you may be misunderstanding how the upload_to argument works; you're not creating additional temporary images (or renaming existing images) by providing upload_to—just providing a filename that Django will use when it saves the file.

A couple of other comments:

  1. It looks like you're trying to store these file in your '/static/' directory (STATIC_ROOT), but that's not where they go. Django will put them in your media directory (MEDIA_ROOT) automatically, appending the path you return from your upload_to or cache_to functions. So if cache_to returns 'uploads/images/thumbs/a.jpeg', the file will be stored in '/path/to/media/root/uploads/images/thumbs/a.jpeg'.
  2. Instead of using string concatenation to create your paths, take a look at os.path.join.
  3. Be careful with the values you're using as arguments to your processors. It seems like some of them are nullable. Is that really what you want?
  4. Along the same lines, make sure that your paths are valid. Since I haven't seen your Media class, I can't really tell what's going on there, but you're using instance.slug and instance.album.name in your filepaths so you'll need to make sure those are populated.

Having said that, I boiled your example down to something a little more bare and (providing I set the properties that cache_to depends on) didn't have any issues. (You didn't say when you were actually getting the error, though. I assume it was in the browser when you submitted the form?) In any case, hopefully my example will help some.

I think we're getting pretty far astray from @mthueson's original question now (it seems like the question of recreating crop_horiz_field and crop_vert_field has been answered), so I'm going to close this issue. If my example doesn't point you in the right direction, though, please don't hesitate to open a new issue with your reduced test case and we'll see what we can do to help you out from there.

Matt408 commented 12 years ago

Following up on this old issue, now that I'm trying to implement it: is there any way to pass values (e.g. height and width) to the get_processors callable? Currently, it looks like I need to make a get_processors function for each size... but I might just be missing something.

Thanks again for your help.

Matt408 commented 12 years ago

Thanks for the sample receiver code as well. One other question: is there any way to invalidate all specs on an instance? I'm trying to avoid hard-coding the spec names into my post-save receiver function.

matthewwithanm commented 12 years ago

@mthueson There's a utility function called get_spec_files. If you pass it a model instance, it will return a list of specs that you can iterate over; you should be able to accomplish what you want by calling invalidate on each item in the list. Check out how invalidate_app_cache works for an example.

Matt408 commented 12 years ago

That worked perfectly. Thanks again.