Custom image sizes skipped if original is smaller than the custom size definition

I had the same problem (which also has appeared elsewhere, see e.g. http://drupal.org/node/97308).

If the original image is small enough to fit with the required image size, the image module (image.module,v 1.197.2.6 2007/03/01) does not record any information about that image, i.e. _image_insert() will ignore it, so as not to save duplicate filepaths in the {files} table. But This means that image_load() will not have any information to load about them, although it does check for "thumbnail" and "preview" and recreate them if they are missing.

But elsewhere (see e.g. http://drupal.org/node/99395) it seems that sometimes the {files} can be lost, so the assumption of "no information = original size" is not reliable.

So here's an alternative and more explicit update to image_display() for this, including commenting (for my own benefit as much as anything!). If the required size has not been loaded, it will regenerate the image derivatives if smaller (the original doesn't fit), or use the original image if the original is small enough already to fit in the required size.

This also seems to avoid the repeated derivative generation problem noted at http://drupal.org/node/75238.

Mark

Patch updated to properly fix the "Thumbnails resizing to original" reported at e.g. http://drupal.org/node/99395. The new bit removes the assumption from image_load() that missing "thumbnail" or "preview" size must be original size, and instead uses the new (in the patch) image_display() to (re)create them at the right size as required. Mark,

i don't think this is the correct fix. we shouldn't create duplicate images if the original is smaller than one of the derivative sizes. the right way to addresses this would be finding and fixing the bug with the {files} going missing and / or checking that the correct derivative images exist at load time.

I was having this same issue when I moved form 1.2 to 1.4. I have since moved back to 1.2.

Sorry, I didn't really explain what the patch at http://drupal.org/node/68623#comment-277959 above does. In particular it doesn't duplicate images if they are not needed (the approach taken by jjef's patch at http://drupal.org/node/75238), and only generates the image derivatives if one is needed. Here's an expansion on my thinking...

The image module feels like it was originally designed for uploading large images like photographs, that would always be larger than the "preview" and the "thumbnail" derivatives. So rather than having its own table to remember the derivatives, it uses the {files} table to hold the derivatives, using the "filename" field to hold the size label ("thumbnail", "_original", "preview", etc), and the "filepath" to hold the path to the image file.

There isn't really an API for the upload to handle the insert into the {files} table, so it does it directly to the database in _image_insert():

    db_query("INSERT INTO {files} (fid, nid, filename, filepath, filemime, filesize) VALUES (%d, %d, '%s', '%s', '%s', '%s')",
             $fid, $node->nid, $file->filename, $file->filepath, $file->filemime, $file->filesize);
    db_query("INSERT INTO {file_revisions} (fid, vid, description, list) VALUES (%d, %d, '%s', %d)",
             $fid, $node->vid, '', 0);

(Note in passing that {file_revisions} is never used again, and neither image_delete() or image_update() remove that entry when the remove an image, leading to many orphan entries in the {file_revisions} tables. This patch doesn't try to fix that.)

OK, so an "image" node is defined by the standard node fields (title, body, etc) stored in the "node" table, together with the additional set of derivatives in the {files} table, indexed by (nid, filename). So we load the derivatives for by loading all derivatives from {files} indexed by $node->nid:

function image_load(&$node) {
  $result = db_query("SELECT filename, filepath FROM {files} WHERE nid=%d", $node->nid);
  $node->images = array();
  while ($file = db_fetch_object($result)) {
    $node->images[$file->filename] = $file->filepath;
  }
  ...

So on load, the image node has an array of the form

$node->images = array{
  "_original" => "images/abc.jpg"
  "preview" => "images/abc.preview.jpg"
  "thumbnail" => "images/abc.thumbnail.jpg"
}

(Note in passing that in 4.7 the {files} table only has an index on the "fid" column (primary key) with none on "filename" column, or even the "nid" column, so this could be a slow query if you have lots of files.)

So far so good.

But now suppose that the original is no larger than the preview size - e.g. the image is 200 x 200 pixels, with the preview size set to 640 x 640. Or that its really small, like 32 x 32, when the thumbnail size is 100 x 100. That means we don't need to create any image derivatives, size the original file is already small enough and doesn't need to be shrunk. So we don't need to store anything in the {files} table.

Ah, now here's a problem. We create our 32 x 32 image and save it. It is already small enough that we don't need to create "preview" and "thumbnail" derivatives. So on save, only the "_original" gets saved to the {files} table. So on load, instead of an image node with three derivatives, we only get

$node->images = array{
  "_original" => "images/abc.jpg"
}

Now this isn't what we wanted - we always want "preview" and "thumbnail" images as well (they are hardwired). So, if we don't have any, it must be the case that they are the same size as the original. So we put them back in again, in the second half of hook_load():

function image_load(&$node) {
  ...
  // special images
  if (empty($node->images['thumbnail'])) {
    $node->images['thumbnail'] = $node->images['_original'];
  }
  if (empty($node->images['preview'])) {
    $node->images['preview'] = $node->images['_original'];
  }
}

Now it looks like we can simply pass the resulting $node to image_display() and assume that the $node->images contains the array of all possible image derivatives, indexed by the size label.

Sadly no. Suppose we have created another label size (e.g. "logosize" at 200 x 200), and the original is smaller (e.g. 32 x 32). Then according to the logic above, a derivative file is not necessary, so it is not created, but neither is a "logosize" entry stored in the {files} table. But the special hack for thumbnail and preview doesn't work for logosize. So by the time the node gets to image_display(), we don't have a "logosize" entry in the $node->images[] array. So image_display() doesn't know what to do and shows nothing. Oops.

Hence, the current patch includes a test in image_display() to check if the original image is already smaller than the requested size, and if so simply shows the original.

[This special treatment of thumbnail and preview is also why thumbnails show at the size of the original if they get deleted from the {files} table (see e.g. http://drupal.org/node/99395).]

Here's another potential problem. Suppose we upload an image which is larger than preview size, so the preview and thumbnail derivatives are created. But now we decide we want another custom sized image, e.g. "gallerysize". But the old image doesn't have this size yet. So when its loaded, the $node->images[] array doesn't include a gallerysize image, so nothing gets displayed for this size. Oops. So the current patch checks if a derivative is actually required, and calls _image_build_derivatives() if necessary (which in fact rebuilds *all* the image derivatives).

(In practice you may not have noticed this problem. This is because image_display() checks if the settings have changed more recently than the image derivative file when it is about to display a derivative it knows about. So as soon as you view the thumbnail, you recreate all the derivatives, including the new gallerysize one that it didn't have before. So most of the time it works, as long as you always view thumbnails or previews first. But if you only use gallerysize and not thumbnails in your gallery, image_display() may not get that far.)

Another problem with using the {files} table is the danger of side-effects from elsewhere. After all its not really "our" table: we could say it really belongs to upload.module, and we are trusting to luck (or other developers!) that nobody messes it up for us. After all, if we use it, anyone else could too. And other image-related modules don't seem to use our API - e.g. img_assist happily duplicates bits of our code. So arguably the "correct" solution would be to abandon the use of the {files} table altogether, and create a new {images} table (image HEAD is already doing that to some extent). But that would break e.g. img_assist and who knows what else (and image.module has been around a long time that other modules will have come to rely on our workings) so the current patch for 4.7 doesn't do that.

In the patch, if the required derivative has not been loaded, we check whether the original fits the required size, and if yes, simply display the original, and don't create any derivatives. If no, it means the derivative hasn't (yet) been created - maybe a new size was created since we saved, or maybe we had it once but something else has now lost it for us. So in this case we create the new derivatives with _image_build_derivatives(). (The updated patch at http://drupal.org/node/68623#comment-277959 above was just realizing that the preview+thumbnail hack in image_load() wasn't needed anymore if all size labels were going to be checked on display anyway.)

For the "custom image sizes skipped" problems, I believe this is a pragmatic approach for 4.7, although I notice a major upgrade is happening for Drupal 5 but have not looked at that much. From the HEAD version's _image_save() though, it still doesn't seem to save information about same-size derivatives, so and still takes the same approach on load, so I wouldn't be surprised if people still see the same problem happening there. (Maybe the HEAD version of _image_save() should save the (nid, image_size, fid) to the {image} table for all derivatives, but for same-size derivatives use the same fid as the original image?)

For the issue of something being deleted from {files}, the current patch is only a defence: there must be a problem elsewhere, with some other module that uses the {files} table accidentally deleting something that belongs to us. But this is the type of side effect we shouldn't really be leaving ourselves open to in the long term, and it may be better to either move away from the {files} table altogether?

In summary, I commend this patch to the house, and bless all who sail in her :-).

Mark

(Updating status.)

@drewish: you are right, of course.

Load time is the correct place to fix this, not image_display(), since otherwise other objects and methods (not only image_display()) each have to deal with a partially-completed object. (Sometimes it takes me a long ramble to see the hole in my own argument.)

So here's a replacement patch for 4.7 that completes the object at load time instead. (Something similar to this may also apply to HEAD)

In outline, it first checks if a rebuild is required (if e.g. the image module settings have been updated or any required image derivatives are missing), and if so rebuilds them. A rebuild, via _image_build_derivatives(), is sufficient to get a complete object. (The rebuild check in image_display() is no longer necessary, so has also been removed in this patch.)

If a rebuild is not required, any remaining empty labels with nonzero size are set to the original image. I've included some comments in the code that I hope help explain what's going on.

Mark

PS Note that if _image_build_derivatives() fails to build a required image file, e.g. for a file permission problem, it will be called again each time the image is loaded. This currently would happen in the current 4.7 version of image_display() anyway. The new 5.1 version can probably use the {image} table to spot this eventuality, but I'm not sure the best way to tackle this for 4.7 here. Maybe save a special flag in the filepath column (e.g. "$$FAILED$$") and have _image_build_derivatives() check for that before it wastes time trying to build again? But that would probably need an admin setting to to clear these etc, and I would guess is probably best handled in a separate orthogonal patch...

this is also present in HEAD. i committing the attached patch to HEAD. need to back port to the 5.x-1.x branch.

if someone wants to come up with a simple version of this i'll commit it.

Nifty.
I think I have had a similar problem; I half-solved it using .htaccess, but a code/db fix would be nice!

The bug #68623 is still active. The version 5.x-1.x-dev doesn't fix this. I used Drupal 5.2 for tests!

heribert, please provide more details. a link to your site would be helpful.

It occurs exactly the same error described above. No images were created if the height and width of the image were smaller than the custom defined sizes.

I've the same problem of heribert.
I've set a custom image format called "slide" with size of 600x600. When I import in a gallery or upload a single image with a resolution of 485x500, no [filename].slide.jpg is generated.

Please help!

charly71, the behavior you describe is correct. new files are not created for images that are larger than the uploaded version.

Yes, but I've installed thickbox with a target dimension (slide) smaller then the original. Then, when I try to open a slide 600x600 format Drupal doesn't show it....

I know... I can set the target thickbox's format to "original", but it isn't what I want. For me this is still an issue.

Carlo

This problem still exists with Drupal 5.6 and Image 5.x-2.x-dev and 5.x-1.x-dev dated Jan 8, 2008.

I use the 3 default sizes - original, preview and thumbnail. The preview is set to 640x640. I use Image Import to import a folder full of images (sizes ranging from 640x480 to 2272x1704 and larger. All the images, except the 640x480 ones get imported and downsized fine. For the 640x480 images, no "_preview" image gets generated.

I use Lightbox v2 to display the images in my gallery and use the Preview image as my display image. So, while the 640x480 images have the proper 200x200 thumbnail, Lightbox chokes because it cannot find the "_preview" image to display.

Please let me know if you need any further information.

Marked http://drupal.org/node/214581 as a duplicate of this.

You could try your hand at a patch which makes the check for original size smaller than preview (or whatever) optional.

Here, in image.module:

  // Derivative sizes that are larger than the original get set to the 
  // original.
  foreach ($unneeded_sizes as $key) {
    if (empty($node->images[$key])) {
      $node->images[$key] = $original_path;
    } 
    else {
      // Need to remove an extra derivative image in the database.
      drupal_set_message(t("%title's has an unneeded %key derivative images. The derivatives will be rebuilt to remove it.", array('%title' => $node->title, '%key' => $key)));
      $node->rebuild_images = TRUE;
    }
  }

- add an if clause to that, and add a tickmark to the admin interface, and roll a patch.

Tracking.

Sorry, this issue is way too old to have a chance to get fixed. Feel free to re-open this issue if you want to supply a working patch.