The crop implementation crops an image regardless if there is really something to crop. I think it would make more sense to crop only if the image is actually bigger than the cropping area.

If I want to crop all images to a max height of 100px I don't want images with 50px height to be enlarged to 100px by inserting a transparent part. That's how it works today and it doesn't really make sense like that.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Tony Sharpe’s picture

Version: 6.x-2.0-beta5 » 6.x-2.x-dev

I have the same issue. I have the occasional image that is less than my desired final image size and in those cases I just want that image to be unchanged. Using 'scale and crop' a small image gets upsized. Using 'depricated scale (outside)' then 'crop' puts a black background around it to bring it up to the crop size. This is no good as I then process it further with imagecache_actions. Is there a way to only crop if required? Tested with both beta 8 and latest dev.

drewish’s picture

Title: croping adds transparent part to small images » Add option to stop up sizing during crops,
Category: bug » feature

well i think there's clearly a use for it. if you want to get a larger canvas you'd use the crop function to do so. that said adding an "allow upcropping" option (similar to the allow upscaling option) would be helpful for cases like yours where you don't want that behavior.

ambo’s picture

Category: feature » bug

I change that back to bug report because from the editorial point of view it is a bug that pictures that don't have enough sizes to be cropped are cropped.
The behaviour that is expected here is that images with smaller size are not upscaled. I don''t think there is at least one case that anybody wants to have upscaling...

thanks a lot

jddh’s picture

Anyone find a solution to this?

Bilmar’s picture

subscribing

jddh’s picture

I resolved this by hacking the module. Not a clean fix, as I haven't added an "upscale" toggle in admin, but I disabled all "upcrops".

greggles’s picture

Title: Add option to stop up sizing during crops, » Add option to stop up sizing during crops ("Allow upcrop")
Category: bug » feature
Status: Active » Needs review
FileSize
2.43 KB

Attached is a patch which adds this feature.

I felt like it makes sense to allow upcropping in either width or height or neither, so it's actually two new checkboxes in the UI. It defaults to match the current behavior.

I think that ideally we would add some more parameters to imageapi_image_crop, but for ease of releasing these two independent modules it seems best to put in just imagecache for now.

@ambo - it doesn't really matter what "editorial" says. This is an enhancement to the capabilities of the module so it is a feature. Arguing with the maintainer about "bug vs. feature" is unlikely to make them work faster and quite likely to make them frustrated.

The patch is against 6.x-2.0-beta10 from an svn repository, but it applies cleanly to the current 6.x-2.x-dev.

greggles’s picture

And a new patch with better t strings messages, more comments, and with isset it is now more e_notice compliant and keeps the default behavior exactly the same.

greggles’s picture

Typo in the comments. "check for is" vs. "check for isset"

ysjwang’s picture

Thanks, this patch worked. Awesome job!

PixelClever’s picture

Status: Needs review » Reviewed & tested by the community

This patch works perfectly, and in my opinion is very important for usability. +1 for getting it into an upcoming release.

YK85’s picture

subscribing

joeyabbs’s picture

subscribing

osopolar’s picture

Thank you greggles for the patch in #9, works as expected.

fizk’s picture

Status: Reviewed & tested by the community » Closed (fixed)

Please reopen if this is still an issue with ImageCache 6.x-2.0-rc1.

greggles’s picture

Status: Closed (fixed) » Needs review

Yes, still an issue.

fizk’s picture

Issue tags: +ImageCache 3

Marking as ImageCache 3.x Todo.

osopolar’s picture

Status: Needs review » Reviewed & tested by the community

State before change in #15 was and still should be: "reviewed & tested by the community".

c4rl’s picture

Looks good to me. Bump? :)

c4rl’s picture

FileSize
2.86 KB

Here's a Git-friendly version of the patch from #9

yannickoo’s picture

Status: Reviewed & tested by the community » Needs work

You should remove all the whitespaces.

+++ b/imagecache_actions.incundefined
@@ -176,20 +178,56 @@ function imagecache_crop_form($data = array()) {
+    $upcrop = t('width and height upcropping');  ¶
+++ b/imagecache_actions.incundefined
@@ -176,20 +178,56 @@ function imagecache_crop_form($data = array()) {
+  ¶
+++ b/imagecache_actions.incundefined
@@ -176,20 +178,56 @@ function imagecache_crop_form($data = array()) {
+    '@upcrop' => $upcrop, ¶
+++ b/imagecache_actions.incundefined
@@ -176,20 +178,56 @@ function imagecache_crop_form($data = array()) {
+  ¶
c4rl’s picture

Status: Needs work » Needs review
FileSize
2.85 KB

Okay

dargno’s picture

Any possibility to put this in Drupal7 core as well? Was kinda astonished to see this not work like described here actually :O

greggles’s picture

It could become a drupal 8 core feature, but probably not Drupal 7 since it is a new feature.