Use imagecache as a process for normal image.module presets

Hey dman I totally love this patch, but Feel it could be a great standalone module... I'm not sure that I really have the personal bandwidth to track the changes in image.module as well as imagecache.module...

I've even been thinking of splitting out a separate imagefield_imagecache.module to expose imagecache formatters to CCK. To make my life as the maintainer of imagecache easier...

How would you feel about making this a stand alone module? I'd really love to see it in the wild... or making it a part of image.module officially. I really want to keep imagecache simple..

.darrel.

both of the patches produced Hunk lines

Yeah I'm not really sure whether the image/imagefield integration should be a part of imagecache or image.module.... I think I may roll this as a contrib imagecache module and move the imagefield integration to the same... maybe move imagecache_profiles there as well... I want people to be able to pick and choose the middleware for their Drupal... not to get a kitchen sink with imagecache.

+1

Providing this as an optional contrib module in the ImageCache project/package sounds good. But I think this patch enhances Image.module in reality, not ImageCache. So providing this feature in the Image package might make more sense.

Moving to Image's queue to hear about drewish's opinion on this.

subscribing.

dman - anything come of this with image.module? should we be checking an issue over there too?

I tried this patch out today. As the patch doesn't remove any existing code, I just added it as _form_alter() and _image_alter() in a separate module.

Here are my results:
* It adds the UI to image.module's settings page. I can add a new image preset and choose an imagecache profile.
* It adds the matching select option to my image_assist popup. However, imagecache profile 1 replaces image setting 1 - so my image.module setting "Original" is replaced with the first imagecache preset.
* I don't see any difference in the resulting inserted image.

dman, I'm intrigued by your statement,

I want to run imagecache over the built-ins, and not have to screw up other modules and themes by using imagecache pathnames.

I hadn't noticed imagecache do this - can you elaborate some?

Would love to have a hand in pushing this further. It is driving me batty that a great toolkit like ImageCache doesn't play friends with TinyMCE :)

This episode was brought to you by the version numbers:
* ImageAPI + ImageAPI GD 5.x-1.1
* ImageCache 5.x-2.0
* Image Assist 5.x-1.6
* TinyMCE 5.x-1.9

Thanks for the reply. I agree that it's better to get this into core image.module than imagecache - and also that on-the-fly generation is generally more efficient (because images don't get created until they're used).

I'll have a look at why the imagecache presets are overwriting, rather than being appended to, the image presets in that form.

Would be keen to liase with you in pushing this one forward. We have an (NZ-based) project which wants some similar functionality, so it would be good to touch base. (Did I email you separately? I meant to!)

subscribe

works like a charm. I love it. doesn't it just make sense? how come no one'd thought of that before?

here's a patched version of the module, for all those not-tech-savvy such as meself; just rename it to imagecache.module and replace the original file. I didn't patch this properly, I only added the last lines. but worked anyway and might come in handy for some folks out there.

(I don't have to tell you to backup and test, do I?)

I'd like to ask developers: PLEASE post PATCHED versions of modules. the whole point of using Drupal is to abandon lines of code. I know patching is simple for a webdeveloper, but webdesigners get really stuck in that point. ok? ;]

anyway, kudos to ye, dman. keep up the good work.

commited with UI modifications as imagecache_image.module...

Functionality untested. cheers. open new bugs for it if they arise...

seems to work fine. but if one changes the imagecache-preset the derivates are not updated. one have to manually delete all the preview/thumbnail files on server.
derivate-images should also be put on the corresponding imagecache-folder not directly in the files/images folder (actually they are named xyz.preview.jpg & xyz.thumbnail.jpg directly in the files/images folder)

I'm keeping this closed.. the integration is only to allow image.module to use the imagecache processing pipeline. You'll have to rebuild derivatives from image.module to update your thumbnails. It can be a time consuming process, but is intended to do things the way image.module does things.

When trying to use this, I pretty much immediately get errors (posting the form on the Image settings page) complaining about how a width and height weren't set. Then, it seems that the regular image derivative building is still taking place, and so I get a dozen pages of errors about trying to generate ""x"" images. I certainly expect bugs in any new or existing release, but am I completely missing something here or does this just not work at all?

I am also suffering this problem...

My 'hack' was to force the form value to 1 by adding this below the form_alter 'hidden' lines...

 $form['image_sizes'][$key]['width']['#value'] = 1;

I've just learned the implication of this... When I submit an image using image_attach the image ends up being 1x1!

I might re-checkout the module and try your method @dman.

dman - I just tried your patch and am still getting the following issue when I submit an image...

The original image was resized to fit within the maximum allowed resolution of 1 x 1 pixels.

Is this related?

Ah fixed - using your patch I had to change the "Original" preset for an Image to Scale Image and then leave the width and height blank. Works perfectly now!

I can +1 this as "good". Nice one dman! :)

Noob here seeking clarification, please correct any of the below summary if I'm mistaken:

Function:

This allows one to use imagecache with the Image module rather than CCK/Imagefield, but loses the dynamic caching behaviour, only creating static derivatives, which will then be used as if they had been created by the Image module by all the other related modules that work with such files (files? - see below)

Procedure:

install and configure imagecache as usual

download imagecache_image.module.txt from http://drupal.org/node/231622#comment-973178 and rename it to imagecache.module

Overwrite imagecache.module in the imagecache folder with the new version

Questions:

Are the images ("original" + derivatives) files stored in the filesystem, or BLOBs stored in the database?

How specifically does one delete the derivative images previously created by Image module and force their regeneration with the new imagecache process?

How should this be set up to protect from this scenario: Image and Imagecache move on in the development cycle and no one does anything further with this fork?

TIA. . .

Thanks very much for your clarifications, seems like a good way to go and thanks for putting in your time to help END THE MADNESS that is images in Drupal

I'd also greatly appreciate (if you have time) your letting me know if I'm completely off-base with this idea on combining the two main methodologies:

"Images in Drupal - Future-proof methodology (Image module + imagefield together)"
http://groups.drupal.org/node/14974

Sorry to bother you Dan, but in following my above step-by-stop, I've just downloaded the current imagecache-5.x-2.x-dev from 2008-Jul-24. When I went to enable it, I saw along with imagecache module itself and its UI, a third module called "ImageCache 4 Image 5.x-2.x-dev Imagecache integration for Image module"

Q1 So is this equivalent to or later than your imagecache_image.module.txt contained in http://drupal.org/node/231622#comment-973178?

In other words am I good to go with the former without having to install anything else? Or is the latter one later or otherwise still needed?

And Q2 I've currently got Image v5.x-1.9 working, should I upgrade to 5.x-2.x-dev or 5.x-2.0-alpha3 (looks like they're the same) before enabling your fork? Note this is all going to be for a production site. . .

Finally Q3 is there anything we non-coding hoi polloi can do to help campaign for this functionality to get rolled into the mainstream Image?

Thanks for your help, and all your great work for the community.

Thanks for the lightning response Dan, and rest assured any and all brain-farts are completely by the by, your brain's forgotten 100x of what's in most of ours'.

I'm happy to know I'm good to go with that dev version of IC without needing anything further from this issue thread. And although I like the "just work" comment, I'm still a bit confused by your D6-related comments since I'm on Drupal 5.

Therefore I'm still not clear on what version of Image module I should be on to work with your "ImageCache 4 Image" module - a one-letter answer will suffice:

>> I've currently got Image v5.x-1.9 working, should I upgrade to A 5.x-2.x-dev or B 5.x-2.0-alpha3 or stick with C stable v1?

Thanks ²

Hope you don't mind me talking to myself here, but for the benefit of those coming here later. . .

I went ahead and tried implementing "ImageCache 4 Image" on a scratch site with Image v5.x-1.9, and so far everything looked A-OK, as long as I only selected IC presets for the first three rows in the Image UI (original, thumbnail and preview, left column options were grayed out.) But when I tried to fill in from no. 4 on, where there's empty left-hand fields to fill in with my own preset names, Image kicked out an error message about width and/or height not being set, as per http://drupal.org/node/231622#comment-938919 above. However, the traditional fields in the middle columns that were used by Image before for setting the WxH parameters are now completely missing from the UI.

I figured this may well mean I should be using Image 5.x-2.x-dev version (2008-Aug-17) so I upgraded, ran update.php selecting Image "5200", tables updated but no joy.

Trying another tack, I disabled "ImageCache 4 Image", went to the original Image config UI, put in dummy width values and saved, then re-enabled ImageCache4Image everything's hunky dory.

Uploaded a JPG and bah-da-bing bah-da-boom, one big original and 5 derivatives sitting in the Images folder!

BTW Q1 I still don't know whether I should have upgraded the Image to 5.x-2.x-dev or not, good thing I did all this on a scratch site :)

Subsidiary Q's:

Q2 I guess the imagecache folder will remain empty unless I start to also use imagefield access methods?

Q3 As long as I stick to Image type nodes (as opposed to Imagefield CCK ones), then Views (and everything else) should be able to pick these up as if they were Image-generated, right?

Q4 I wonder if I do start to use Imagefield nodes, can I configure it to use the same directory/filenames as Image? If not, NBD, disk space is cheap eh?

Finally, I'm so far a very happy camper Dan, thanks so much for this brilliant effort.

IMO this should get folded into the mainstream Image module now, one step closer to the grail of ending the insanity that is images in Drupal.

Maybe this is a good place for you (that is everyone but Dan da man) to put your +1:

http://drupal.org/node/145071

I can personally "+1" DMans patch from #22 with one "minor" tweak...

The patch worked PERFECTLY if I added one preset, but the second preset caused headaches. I ended up rounding it down to the fact that derivative generation was passing a "preset" over which has zero width and height. Fix? Simply add a hardcoded 1x1 width and height... Seems to fix everything :-)

I've attached my revised version of the patch... Anyone care to review? :-)

@nicholasThompson - I don't yet know how to patch so I can't test your code, but rather than working from the code in dman's post above, I reckon using the "imagecache 4 image" module shipping as part of the latest dev release of Imagecache would be the way to go. I had no problems at all with it once I created the "dummy presets" in Image's config screens before enabling I4I - these settings get ignored, but apparently need to be there.

Answering some of my own questions from knowledge gained in the meantime, again for the benefit of those coming here later:

Q1 still don't know, if someone gets the latest dev of Imagecache's imagecache4images working with the stable version of Image, please post here to let us know.

Q2 yes, Imagefield will dynamically generate its derivatives of images stored in ANY location and uploaded from ANY source simply by using its URL access scheme, and nothing will appear until then.

Q3 yes, so far all Image-module compatible add-ons treat the derivatives created by imagcache4image as if they were natively created by Image module.

Q4 haven't done any testing, but from discussions elsewhere seems to be a bad idea to have different modules "sharing" the same files - let each have full "ownership" over its own files, and if you need to say save images as nodes with Image module AND embed them in Imagefields, you need to accept the image files being duplicated in different folders.

this thread was about D5 and image-cache 2.x

And just to confirm re D5, I've had zero problems with the "imagecache 4 image" module contained in

imagecache-5.x-2.2

working together with

image-5.x-2.x-dev-2008-Aug-17

I used the dev Image module when testing and never did try it with the released one, still waiting for feedback on whether that should work or not (I suspect yes)

+1

this module is definitelly "TOP 1" glue module for Drupal.

one small question: have any plans for Drupal 6 version?

though now that imagecache is getting pretty stable i've come around to the idea of making the image module dependent on it for thumbnails. what do you think of moving this issue back over there?

subscribe

I'd actually like to see the integration. Looking forward to your decision.

@drewish: I'm in favor of moving it back to image.module as well...

Yesterday I had a weird problem with the page where I'm using this page. An image where the derivatives had to be regenerated called convert with size parameter "0x0!" and I had a duplicate entry for "Original" in the image settings. I don't know if it's because of this patch or because of my test site (im trying out a lot of stuff, so it might be my fault). Did someone else see this already?

Image 6.x-1.0-alpha4, ImageCache 6.x-2.0-beta5 and this ImageCache for Image - Image derivatives cannot be properly set using ImageCache presets.

# warning: imagecreatetruecolor() [function.imagecreatetruecolor]: Invalid image dimensions in /public_html/d6abc/includes/image.gd.inc on line 95.
# warning: imagecolorallocatealpha(): supplied argument is not a valid Image resource in /public_html/d6abc/includes/image.gd.inc on line 97.
# warning: imagealphablending(): supplied argument is not a valid Image resource in /public_html/d6abc/includes/image.gd.inc on line 98.
and so on ....

subscribe

I tried the imagecache for image yesterday with a fresh Acquia Drupal install, but it did not work.

I set image to use imagecache to create the Original, Thumbnail, Embedded sizes. After submitting the dimension set automatically 1x1, and it is not ignored: when I create a new image it is inserted 1px x 1px size. :o(

I had the problem with the 1x1 images too. I found that any time I tried to let imagecache do ANYTHING with the original, it and all my other derived images became 1x1. I had to remove the imagecache preset and let the image.module handle the original image (you can still use the basic scale/scale & crop provided by image.module). All the other imagecache presets worked fine after I changed that back.

Edit: I may have spoken too soon. It seemed to be working fine for a while, but the 1px images are happening again.

Another edit: Okay, now it works again. Sort of. I flushed all the imagecache presets (again), and new images seem to generate derivatives properly.

Problem is, even though all the derivatives show up in the right sizes on the image node, if I try to insert them into a page, then img_assist turns them into thumbnails or 1px images (depending on the browser). Grr. Doesn't seem like an img_assist issue, because if I revert to the image.module presets, the image is inserted at the proper size (after recreating the derivative images). However, when I view the img_assist Filter tags, it gives the dimensions as 1x1 again. So I think somewhere along the line, the imagecache presets are failing to override the image.module dimensions (which I notice become 1x1 when using the imagecache for image module).

In case anybody is curious, I tried a fresh install with a different set-up. In both cases I started with the Image module, Imagecache, and Imagecache for Image (of course). However, instead of using Img_assist (a longtime staple of mine) and FCKeditor for inserting photos, I tried Image Browser and FCKeditor.

Works like a charm so far. No 1x1 images or reducing everything to thumbnails. Of course, imagebrowser is intended to work with both image and Imagecache, so..

Anyway, it looks like Img_assist was the source of my troubles. This new set up allows me to use Imagecache/Imagecache_actions on both image nodes and on images inserted into a text field via FCKeditor, thus meeting both my site requirements (images as nodes and automatic image sizing/watermarking).

I need to test this out a bit more, but I'd love to see this module committed to the appropriate project.

Image Assist has nothing to do with this issue.

I'll test it any patch you post here. I need this integration working ;)

@dman
Image module will be patched if patches have been tested and the maintainers agree to committing the patch.
I see sun has left this issue as being on 6.x-1.x-dev, so I assume he is in favour of this going into that branch. So am I :)

I've had a super-quick glance at what you've linked in the comment above and the first thing that springs to mind -- if you're patching image.module anyway, why not patch it to add a hook_image_operations, rather than shove your presets in with a hook_form_alter? Then image.module implements it, and so does your module. (And perhaps others, who knows?)

subscribe

subscribe

I lost Imagecache integration when "upgrading" from image module 5.x-2.0-alpha3 to 6.x-1.0-alpha4 and would like to get it back. subscribing ;)

Greetings, -asb

can anyone at last combine this peace of code into these modules???

subscribe

subscribe

and are people trying to make this work for both D6 and D5? I'm on 5.

@tevih i'm here for Drupal 6 and this issue also assigned to 6.x-1.x-dev

@tevih New features are implemented on D6. Once that is done, we can think about a backport.

@joachim, @tevih:

> New features are implemented on D6. Once that is done, we can think about a backport.

I'm getting very confused. The functionality dman is developing for D6 (integrating imagecache presets into the image module) exists for D5 for quite a while, at least I have a sub-module "ImageCache 4 Image" running in version 5.x-2.4 that works pretty well; according to /admin/build/modules, that module belongs ImageCache (does it? 437966). So I don't quite understand why it would be necessary to backport this (very cool!) feature since it seems to already exist...

A second thing I don't understand is the relation of the (latest) 5.x-2.0-alpha3 branch to the (latest) 6.x-1.0-alpha4 branch; if new features are developed for D6, I don't understand how this branch (with more features than the 5.x-1.x branches) came into existence; as far as I can see on my D6 sites, stuff like Image Galleries through views does not exist in the 6.x-1.0 branch yet while it is working quite nicely in the 5.x-2.0 branch already.

Sorry, but I'm really confused about this...

@dman:

I tried the imagecache_image module together with ImageAPI 6.x-1.6, ImageCache 6.x-2.0-beta9, and Image 6.x-1.0-alpha4:

1. Created a folder ./sites/all/modules/imagecache_image/
2. Copied imagecache_image.info and imagecache_image.module into this folder
3. Downloaded image-operations_hook-20090419.patch into ./sites/all/modules/image/ and applied the patch
4. Applied Imagecache's presets at ./admin/settings/image/nodes

Parts of the patch are rejected:

$ cat image.admin.inc.rej 
*************** function image_settings_sizes_validate($
*** 110,117 ****
    foreach (element_children($form) as $key) {
      // If there's a label they must provide at either a height or width.
      if ($key != IMAGE_ORIGINAL && !empty($form[$key]['label']['#value'])) {
-       if (empty($form[$key]['width']['#value']) && empty($form[$key]['height']['#value'])) {
-         form_set_error("image_sizes][$key][width", t('You must specify width, height or both dimensions.'));
        }
      }
    }
--- 110,120 ----
    foreach (element_children($form) as $key) {
      // If there's a label they must provide at either a height or width.
      if ($key != IMAGE_ORIGINAL && !empty($form[$key]['label']['#value'])) {
+       // Only validate our own known operations, allow for other operations to be available without defined dimensions
+       if (($form[$key]['operation']['#value'] == 'scale') || ($form[$key]['operation']['#value'] == 'scale_crop')) {
+         if (empty($form[$key]['width']['#value']) && empty($form[$key]['height']['#value'])) {
+           form_set_error("image_sizes][$key][width", t('You must specify width, height or both dimensions.'));
+         }
        }
      }
    }

However, the Imagecache intregration appears to work quite well.

What can we do to get this into an official/packaged release, either inside the image module's contribs or as separate project?

Greetings, -asb

@asb:

- I don't know all the details of the feature in question here. Policy is to develop on the most recent version first. If something already works on 5 then great.
- about the branches: drewish began working on the 5-2 branch before Drupal 6 was released. I contributed the views stuff for image gallery. Then Drupal 6 was released, it took a long time for Views 2 to be released, and it took me a long time to get image support into galleries. There is now a patch for this by the way :)

I would say what needs to be done for this to be included is just the changes outlined in #66.

I don't know if this module is best kept as a contrib in image or imagecache, but I'm happy to have it here. Let's see what the other maintainer thinks :)

I thought that imagecache (API) is going to core in D7?

reply to http://drupal.org/node/231622#comment-857078

Apart from this transparancy, what's the use of creating static derivatives through ImageCache? I thought that ImageCache was all about generating derivatives on the fly and thus removing the clutter of having an_image.preview.jpg, an_image.thumb. jpg, an_image.blablabla.jpg, an_image.so_and_so.jpg, etc.

When I came across this issue, I thought that this would finally suit my needs, i.e. using Image module (+ Image Gallery and Image Assist) but with dynamically generated derivatives from ImageCache.
Alas...

Are there any objections to this idea? Would it be so difficult to alter all the Image dependent modules to use ImageCache?

Quick idea: route the requests for a certain file to a function that determines whether to take a file from disk or to generate a derivative.

function get_image($filename){
  global $conf;
  //general form of file name: [name].[preset].jpg
  if (!exists($filename)) {
    if(module_exists('imagecache')){
      $parts = explode($filename, '.');
      $preset = $parts[1];
      $path = $conf[$preset];
      return "{$path}/{$parts[0]}.{$parts[2]}";
    }
  } else {
    //get file from file system
  }
}

I don't know the inner workings of ImageCache or the Image module, so this piece of code may utterly useless.

+1 for imagecache_image

just subscribing.

Hello,

I would like to use imagecache 4 image, but i don't know how to patch original image module file with image-operations_hook-20090419.patch !

I understand that we have to modify these files by hand :
modules/image/image.admin.inc
modules/image/image.module
modules/image/contrib/image_import/image_import.pages.inc

But i'm not familiar with diff files found here http://cvs.drupal.org/viewvc.py/drupal/contributions/sandbox/dman/imagec......

=> have i to remove line with "-" and add line with "+" ?
I will try that... and post here again...

Regards,
Vincent

OK, Great !
Work just perfectly !

but no file created in imagecache subfolder...
is it normal because file creation is handled by image module ?

Vincent

Working BUT not for original image !

I created 3 presets for original (resize to 1280pixels if image is larger than that), preview and thumbnail.
Preview and thumbnail images are successfuly created, but NOT the original one !

I have the original image file i uploaded, but not the "original" file that imagecache should create with my original "preset"...

Could anybody help me with that problem ?

Regards,
Vincent

Hello and thanks for your answer !

For #2 the drupal image module original behavior was to scale to height x weight fixed in the "original" image "label".
with that behavio it is possible tu upload uge image file but to scale and autorotate them to "normal 1280x1024" resolution...

So question is HOW could we handle original image file ?
We could not use original image moodule scale because it does not recognize EXIF, and don't autorotate files...
(autorotate with imagecache action in dev module)

Thanks again for your quick answer,
Regards,
Vincent

You can see just above the image size boxes : Note: 'Original' dimensions will only be used to resize images when they are first uploaded. Existing originals will not be modified.

The original image is modified if we put values in height and weight !
My Goal is to prevent keeping too big jpg files... that why i want to downscale image (if necessary)

The only solution is use original drupal image sacle... but then loosing autorotate... not possible...
AAAhhhhh !!
:o(

Thanks for your help !

Regards,
Vincent

dman, if you know, how to disable the UI check, or just know where to search... tell me !
Because with autorotate it is more important to use the first derivative rotate correctly than the original !
Vincent

I think it could be here, in image.module :

$all_sizes = image_get_sizes(NULL, $image_info['height'] / $image_info['width']);
foreach ($all_sizes as $key => $size) {
// We don't want to include the original.
if ($key == IMAGE_ORIGINAL) {
continue;
...
Vincent

I was look to apply this patch to 6.x-1.0-beta3 but assuming that won't work as the code seems to be all different. Is that correct?

I confess I no longer have any idea what this issue is doing.

I wonder whether the best thing would be to work in a new 6.x branch on the conversion of image module to be a simple wrapper for imagefield+imagecache. This would mean an upgrade script to convert all image data to CCK and the preset data to imagecache.

Perhaps that would be a good idea. It could also have a successor to image_attach through filefield_sources, with optional integration of img_assist/imagebrowser, or a successor for image_gallery either taxonomy- or views-based.

I think that this is a great idea. Please start a 6.x-3.x as soon as possible. It seems that the conversion scripts have already been written, such as http://drupal.org/node/432860 and http://drupal.org/node/201983, so why not make it official?

A new branch would then give direction to the dozen or so other modules that depend on Image, so that they can upgrade in an orderly manner. I see that someone has already forked img_assist module on their own, to become imagefield_assist; this has got to be a bad thing in the already confused state of Drupal image support.

With Drupal 7 on the horizon, it becomes all the more important for Image to be all tidied up and 'standardised' ready for more conversion at that point. And just think, your support queue should be much shorter for a wrapper/helper/glue module, as problems can just be reassigned up to the parent modules!

Image module has withstood the test of time, proving that a lot of people find it useful to have a single integrated, all-in-one place to go for images, attachments and galleries, so I would hate to see it lose momentum now by people looking for a 'more modern' or standard/cck method for their sites.

I would love to have this patch.

Subscribing. :)

Subscribe

Subscribe

For reference, here's what I explained to panhead on IRC just now about what I meant in comment #66 above:

[21:32] joachim_: what I mean is create a new hook
[21:32] joachim_: same idea as something like hook_node_info
[21:33] joachim_: it merely asks modules "what image operations do you provide?"
[21:33] joachim_: image module would then say 'scale, crop'
[21:33] joachim_: this new module in the patch would say 'I have all these shiny imagecache presets'
[21:33] joachim_: the form would invoke the hook to discover the possible presets and use that data to populate the dropdown
[21:34] joachim_: the hook would need to return: name, label, some sort of callback so image module knows how to make it happen, and as we've seen, whether you want x*y imension fields
[21:35] joachim_: so it does involve ripping up a chunk of image.module

> I found 70 coder style errors pre-existing in image.module -dev which I'd like to tidy up also if I'm allowed

In haste, just seen this as about to leave... don't include extra tidying up in a patch; it makes it much much harder to review.

Does this need work to happen on imagecache too?

I reckon you can put that in imagecache_image and add that as a contrib module here.

subscribe

subscribing

subscribing

It's going to be a few weeks before I have enough time and a clear enough head to properly review this, but here is a very quick review just from a read of the code:

- rather than call the hook again to get $our_operations, I would add a key to what the hook returns, like:
'needs_dimensions' => TRUE,
It's unlikely that any other module implementing this hook would need dimensions too... but it makes the code that little bit smoother, as you just test that value when you iterate the operations to see whether to build the height and width form elements.

(We could even have each operation specify names of form elements it wants, and therefore hypothetically allow other hooks to say they want a text field there, but I think this is overly complex and is never going to be used!)

Similarly in the validation handler, there's probably no need to call the hook again to see which operations need those values checking -- just look in $form to see if that operation has those in the first place.

_image_build_derivative() vs _image_build_derivative() -- avoid having two function names differ by a plural like the plague! it makes code harder to read, trips you up unless you've noticed both exist, and is an invitation to make typos.

Also:

> + * CHANGELOG - this was using $form[...]['#value'] where it should have been
+ * using $form_state['values']. Changed it to behave normally, but unsure if the
+ * previous method was intentional. - dman

I've a feeling that's D5 code that never got updated. Could someone file an issue with this and check in old FormAPI docs if that's the case? We should fix this as a separate patch.

> at least a flag would remove any assumptions.

It's more about making the code simpler and easier to read :D

> I think we can discard the old $form[...]['#value']. It looked like a left-over.

I think so too, but I'd prefer this got fixed in its own issue.

imagecache_imagesize_integration.patch queued for re-testing.

No, it needs work. The current patch has already passed tests.

Thanks for filing that and rolling the separate patch. I've committed that fix.

#104: imagecache.image_integration.20100604.patch queued for re-testing.

subscribe

- for people just joining this 2-year thread, do we have any (semi)working code, and do we apply it to both modules? This is so needed - thank you!

oops sorry, I did not mean to change any status

AFAIK the patch needs work because we fixed a problem in another issue, and also for the other things I mentioned in comment #116.

Wow. I'm impressed -- within moments of applying these patches I had a desaturated photo showing in my image node!

Now for the detail...

+++ image.admin.inc	14 Jul 2010 02:43:21 -0000
@@ -77,20 +88,33 @@ function image_admin_settings() {
+      $operation = @$operations[$size['operation']];

Too many @s in here. They're odd and scary, and as much as I hate how wordy PHP gets with endless checks to isset() and empty(), we should use those.

Here, I assume we're allowing for the fact that there might not be an operation for the current form element, in the event that a module has been uninstalled?
In which case, we can test for that in the else just above -- I've made that change in my version of the patch.

+++ image.admin.inc	14 Jul 2010 02:43:21 -0000
@@ -77,20 +88,33 @@ function image_admin_settings() {
+        '#default_value' => @$size['height'],

No need for the scary @ -- the size key is always set, just it may be an empty string. Changed that in my patch.

+++ image.admin.inc	14 Jul 2010 02:43:21 -0000
@@ -172,7 +204,7 @@ function image_admin_settings_submit($fo
+        $rebuild |= (@$form_state['values']['image_sizes'][$key][$field] != @$old_sizes[$key][$field]);

This is scary too.

Can we expand this, or wrap in a test on ['needs_dimensions'] ?

+++ image.module	14 Jul 2010 02:43:22 -0000
@@ -787,6 +787,73 @@ function _image_check_settings() {
+function _image_build_derivative_scale($original_path, $destination, $derivative_info = array()) {
+  return image_scale($original_path, $destination, $derivative_info['width'], $derivative_info['height']);

Why don't we just change the signature of image_scale() and use it directly?
Ditto scale and crop.

- return FALSE;
+ //return FALSE;
+ // One failure may not mean that we should give up entirely. Make the other derivatives anyway.

Just kill the return line if it's not needed, though having the comment to explain what we're doing is good.

+ $operations = module_invoke_all('image_operations', TRUE);

What's the TRUE for?

> - so this current version has a small gap in the validation if you switch from a third-party preset back to an image.module operation - it allows you to do so without immediately forcing you to enter new valid dimensions.

Ah, yes, I see what you mean. This is where ideally we'd have the widthxheight portion of the form AJAX itself in. Which I doubt anyone has time to implement, so I'm happy to leave it as is.

Also, the doc header for image_image_operations() needs some work -- probably put the array details in the actual @return bit -- but it's breakfast time here :)

> But yeah, we can add another handful of isset()s around the place, just to be really thingy about it.

I think isset()s are the best of two bad options.

> Yeah I looked at that. reason: because it's core. includes/image.inc. Not gonna go there.

Oops. I acked it in the code and thought I saw it defined in the module.

Having two functions that are mere wrappers seems a bit wasteful. If you want to switch back to a single wrapper that switches, I'm fine with that -- it was just the actual name of the function I didn't like.

> Sorta debug FYI.

:D

> I was floating on the fringe of going AJAXy with this, but it meant too much surgery on the current form.

Yeah. Let's leave it, and if someone wants to have a go implementing some basic form ajax, they can tackle it in a subsequent patch. The gain in functionality here is definitely worth a minor UI glitch.

> True. The docblock and API definition floated around between various files as it developed.

An image.api file is part of the spec for D7. Not needed here yet.
What I meant is that the array you return should be laid out in the actual @return thingy. Look at http://api.drupal.org/api/function/hook_menu for an example.

> Coming back to it after a week/month showed me there was no compelling reason to scrunch them together.

Ok let's keep that as two stubs then.

+++ image.admin.inc	14 Jul 2010 07:18:08 -0000
@@ -110,12 +137,20 @@ function image_admin_settings() {
   $image_sizes = $form_state['values']['image_sizes'];
...
-    if ($key != IMAGE_ORIGINAL && !empty($image_sizes[$key]['label'])) {
-      if (empty($image_sizes[$key]['width']) && empty($image_sizes[$key]['height'])) {
-        form_set_error("image_sizes][$key][width", t('You must specify width, height or both dimensions.'));
...
+    if ($key != IMAGE_ORIGINAL && !empty($size['label']) && !empty($operation['needs_dimensions']) && isset($size['width']) && isset($size['height'])) {
+      if (empty($size['width']) && empty($size['height'])) {
+        form_set_error("image_sizes][$key][label", t('You must specify width, height or both dimensions.'));

The previous code rightfully tested values that have been submitted; the new code checks already stored values.

+++ image.admin.inc	14 Jul 2010 07:18:08 -0000
@@ -110,12 +137,20 @@ function image_admin_settings() {
+    // If there's a label they may be required to provide at either a height or width.
+    ¶

Trailing white-space and comments exceeding 80 chars here and elsewhere in this patch.

+++ image.admin.inc	14 Jul 2010 07:18:08 -0000
@@ -172,7 +207,7 @@ function image_admin_settings_submit($fo
       foreach (array('operation', 'height', 'width') as $field) {
-        $rebuild |= ($form_state['values']['image_sizes'][$key][$field] != $old_sizes[$key][$field]);
+        $rebuild |= (@$form_state['values']['image_sizes'][$key][$field] != @$old_sizes[$key][$field]);
       }

Error suppression is slow and poor code. Here, the inner foreach can be wrapped into a condition with two isset()s.

+++ image.module	14 Jul 2010 07:18:10 -0000
@@ -787,6 +787,73 @@ function _image_check_settings() {
+ * The operation definition may contain the keys:
+ *   'name' => "Human-readable name for this action",   ¶

See http://drupal.org/node/1354 for formatting standards for lists.

+++ image.module	14 Jul 2010 07:18:10 -0000
@@ -847,24 +919,35 @@ function _image_build_derivatives($node,
+    if (empty($operation)) {
...
+      watchdog('image', 'When building image derivative %key, it appears that the image operation %operation is invalid. Perhaps it depends on an unavailable module. Not building this derivative image.', array('%operation_name' => $operation['name'], '%key' => $key), WATCHDOG_ERROR );
+      $status = FALSE;
+    }
+    if (!empty($operation['callback']) && function_exists($operation['callback'])) {
...
+    }
+    else {
+      watchdog('image', 'When building image derivative %key, it appears that the image operation %operation is invalid. Could not access the callback function [%operation_callback] to process the image. Not building this derivative image.', array('%key' => $key, '%operation_name' => $operation['name'], '%operation_callback' => $operation['callback']), WATCHDOG_ERROR);

Two watchdog messages will be fired here.

+++ image.module	14 Jul 2010 07:18:10 -0000
@@ -847,24 +919,35 @@ function _image_build_derivatives($node,
     if (!$status) {
       drupal_set_message(t('Unable to create scaled %label image.', array('%label' => $size['label'])), 'error');
-      return FALSE;
+      //return FALSE; ¶
+      // One failure may not mean that we should give up entirely. Make the other derivatives anyway.
     }

The return is required to prevent PHP warnings for attempting to work with a non-existing file.

Powered by Dreditor.

subscribe

subbing for updates

#137: image.imagecache_integration-20100715.patch queued for re-testing.

I think this patch doesn't work anymore? I got 1 out of 3 hunks FAILED when I tried to apply to the latest development release.

That is most probably because of the code clean-up patch that was committed last week.

Can someone reroll this?

Subscribing...

Is there a chance that this issue will get into the 1.0 release of image?
by the way - Great work so far!

@145 see #143.

Hey dman! Just applied patch #147, but not getting any change in the module's ui. No other options for image manipulation.
Ran the update.php. Also deselected Image module, then reapplied. Any ideas?

Thanks Dman! works for me now :D

Question.. .I am using feeds to import feeds and find that the images imported and inclued in the the body of the teaser are breaking my layout because they are too big.is this something that you think that can correct this issue?

Thanks!

awsome. does your image ownage have a project/module drupal project page?

Patch applies and looks good.
Great work, dman. Thanks for all your hard work and persistence on this! :D

I'm going to commit this when I have a spare moment.

I'd like to rework a few of the comments (just nitpicking ;).
If anyone wants to reroll this to remove all the trailing whitespace that would be a huge help.

Further testing of this patch would be appreciated too!

I have the patches applied to the image and imagecache modules and all seems well.
I am trying to get the module 'Imagecache javascript crop' ( http://drupal.org/project/imagecrop ) to work with my imagecache presets, but it seems to be ignored. I think it might have something to do with a special way the cropped image is called from imagecache. Anyone used the imagecrop module successfully with these patches?

@bluegray: Thanks for testing these patches :)

Regarding imagecrop, it looks like that module is going in to alter imagecache images itself -- see this on the project page description:

> at which time your custom cropped version will overwrite the default file at that preset's location in the Imagecache filesystem.

Hence like you say, doing 'something special'. Not something we can support over here. That module should really be hooking into imagecache properly, rather than stepping on its toes to overwrite its images.

Thanks for your hard work on this!

+++ image.admin.inc	27 Aug 2010 13:52:38 -0000
@@ -60,6 +60,20 @@ function image_admin_settings() {
+  // Allow for third-party modules (imagecache) to add to the ¶
+  // available preset operations. ¶

Minor: Trailing white-space and not using available space of 80 chars.

Trailing white-space happens to be all over the place in this patch, so needs to be fixed elsewhere, too.

+++ image.admin.inc	27 Aug 2010 13:52:38 -0000
@@ -60,6 +60,20 @@ function image_admin_settings() {
+  $operations = module_invoke_all('image_operations', TRUE);
...
+  foreach ($operations as $operation_id => $operation) {
...
+    if ($operation['module'] != 'image') {

It is odd that a module has to specify it's own name in the registered operations. We should use module_implements() here and automatically set the 'module' key for each operation.

+++ image.admin.inc	27 Aug 2010 13:52:38 -0000
@@ -77,20 +91,34 @@ function image_admin_settings() {
+    // Only show size fields for operations that ask for it
+    if (empty($size['operation']) || $operations[$size['operation']]['needs_dimensions']) {

What means the empty($size['operation'])? The comment only explains the latter condition.

+++ image.admin.inc	27 Aug 2010 13:52:38 -0000
@@ -77,20 +91,34 @@ function image_admin_settings() {
+    elseif (isset($operations[$size['operation']])) {
+      // Allow the providing module to put something informative here ¶
+      // instead of the dimensions.
+      $operation = $operations[$size['operation']];
+      if (!empty($operation['extra'])) {
+        $form['image_sizes'][$key]['extra'] = array(
+          '#type' => 'markup',
+          '#value' => $operation['extra'],
+        );
+      }
+    }

Shouldn't this rather be a hook to invoke? Or what is the typical use-case for 'extra'? (could be stated in the comment)

Not sure whether we need this. Looks like it complicates only and isn't necessary for the essential functionality of this patch.

+++ image.admin.inc	27 Aug 2010 13:52:38 -0000
@@ -110,12 +138,21 @@ function image_admin_settings() {
+  $operations = module_invoke_all('image_operations', TRUE);

What means TRUE? Based on the rest of the patch, I guess that's just stale debugging code.

+++ image.admin.inc	27 Aug 2010 13:52:38 -0000
@@ -110,12 +138,21 @@ function image_admin_settings() {
+    // If there's a label, may be required to provide either a height or width.
+    ¶
+    // Note, this validation is weak here now, as it will only validate when ¶
+    // all inputs are as expected. ¶
+    // Checking only 'needs_dimensions' or only width & height makes it ¶
+    // impossible to switch operation types and still validate without a form ¶
+    // rebuild (which I cannot force from here).
+    if ($key != IMAGE_ORIGINAL && !empty($size['label']) && !empty($operation['needs_dimensions']) && isset($size['width']) && isset($size['height'])) {
+      if (empty($size['width']) && empty($size['height'])) {
+        form_set_error("image_sizes][$key][label", t('You must specify width, height or both dimensions.'));
       }
     }

This looks like a wild assumption. I do not really understand the explanation for the "weak validation". I'd expect that defining 'needs_dimensions' will trigger this validation, regardless of whether there is label or any other size property elsewhere.

+++ image.admin.inc	27 Aug 2010 13:52:38 -0000
@@ -172,7 +209,9 @@ function image_admin_settings_submit($fo
+        if (isset($form_state['values']['image_sizes'][$key][$field]) && isset($old_sizes[$key][$field]) && ($form_state['values']['image_sizes'][$key][$field] != $old_sizes[$key][$field])) {

The additional parenthesis around the last condition is superfluous.

+++ image.module	27 Aug 2010 13:52:39 -0000
@@ -787,6 +787,73 @@ function _image_check_settings() {
+      'needs_dimensions' => TRUE,
+      // This indicates that basic width x height fields should be displayed ¶
+      // on the configuration form

The comment should be above the remarked line, not below.

+++ image.module	27 Aug 2010 13:52:39 -0000
@@ -810,6 +877,11 @@ function image_get_derivative_sizes($ima
+    if ($size['operation'] != 'scale' && $size['operation'] != 'scale_crop') {
+      // Not one of our own processess, Always rebuild.
+      $sizes[$key] = $size;
+      continue;
+    }

Hm. I fear that this can easily lead to a performance issue. We've had quite major problems with the rebuilding of image derivatives in the past, mainly caused by needless rebuilds. Most probably, this would have to be a hook.

+++ image.module	27 Aug 2010 13:52:39 -0000
@@ -849,24 +921,38 @@ function _image_build_derivatives($node,
+    $operation = $operations[$size['operation']];
+    // Be very careful not to die if the needed module or setting is missing.
+    if (empty($operation)) {
+      // The module that provides this operation is now unavailable?
+      watchdog('image', 'When building image derivative %key, it appears that the image operation %operation is invalid. Perhaps it depends on an unavailable module. Not building this derivative image.', array('%operation_name' => $operation['name'], '%key' => $key), WATCHDOG_ERROR );
+      $status = FALSE;
+    }
+    else {
+      if (!empty($operation['callback']) && function_exists($operation['callback'])) {
+        $status = $operation['callback']($original_path, $destination, $size);
+      }
+      else {
+        watchdog('image', 'When building image derivative %key, it appears that the image operation %operation is invalid. Could not access the callback function [%operation_callback] to process the image. Not building this derivative image.', array('%key' => $key, '%operation_name' => $operation['name'], '%operation_callback' => $operation['callback']), WATCHDOG_ERROR);
+        $status = FALSE;
+      }
     }

1) The watchdog message reads a bit lengthy. Can we shorten it up to the point?

2) The code's logic can be heavily simplified here by using the following condition in the first place:

if (!empty($operation['callback']) && function_exists($operation['callback'])) {

This leads to a single else condition without duplicate code.

+++ image.module	27 Aug 2010 13:52:39 -0000
@@ -849,24 +921,38 @@ function _image_build_derivatives($node,
     if (!$status) {
       drupal_set_message(t('Unable to create scaled %label image.', array('%label' => $size['label'])), 'error');
-      return FALSE;
+      // One failure may not mean that we should give up entirely. ¶
+      // Make the other derivatives anyway.
     }

Similar to my performance remark above, this makes me a little nervous. As of now, a negative status means that we were not able to generate a derivative, so it does not make sense to try again for any other derivative. We need to retain the possibility of a hard loop cancellation for failures that won't change, regardless of which operation is used.

Lastly, we can only commit this patch if we are 100% sure that all of the current functionality is retained and does not lead to negative or unexpected side effects on existing sites. We also need to take other modules integrating with Image into account, e.g., Image Assist and others, which means they need to be tested manually.

Powered by Dreditor.

Thanks sun for your very thorough review. Looks like this patch needs a bit more work than I thought :/ and you're right we should leave it to a 1.1 to finally get 1.0 out of the door.

> We also need to take other modules integrating with Image into account, e.g., Image Assist and others, which means they need to be tested manually.

Yup, I hadn't thought of all the modules that depend on image. We're at the base of a pretty large ecosystem.

However, I think the changes we make should be transparent. The derivative files get created in the same place, with the same data being stored, right?

That said, some manual testing by users of various image ecosystem modules would be great. Could people report back here on which modules they've tried?

Finally, just an aside:

> It is odd that a module has to specify it's own name in the registered operations. We should use module_implements() here and automatically set the 'module' key for each operation.

See #890660: add an alternative way of doing module_invoke_all() which sets a 'module' key in the returned info ;)

> Now anyone trying to apply this patch on a live site must visit and save the image presets page at least once. Maybe this can also be done with a hook_update,

Sounds to me like it could, and it's what hook_update is for.

> I usually put checkpoint statements "If we've got here, this must be the case" inside the conditional for easier scanning

I can't remember what Drupal code standards say on this. Check! If there's nothing stated about this, file a bug :)

this is going to be too late for the 1.0, as we'd really have to roll another beta to get it widely tested. So what I think is best to do at this point is release 1.0 (this week, as soon as I get spare moment, as the minor bug reports seem to have dried up), and then once I've had a proper look at this patch and it's had a bit of testing, commit it to the -dev release and encourage people to try that.

Yes! This really helped.

with the patches applied to dev of both image and imagecache everything seems to be functioning great.

@mcaden: is there also patch that needs to be applied to imagecache for this to work?

UPDATE: yes, I see that I need to also patch imagecache with:
http://drupal.org/files/issues/imagecache.image_integration-20100714.patch

@157 & 158

Ahh.. too bad. I was hoping to do the same thing as bluegray.

Any other way to manually control the crop area via the image module?

UPDATE:

Actually, there is a way to use imagecache presets with the image module without patches. It involves creating a View that manually applies the imagecache preset to the image field (not imagefield). You'll need views customfield.

Create a view and add the relationships "Image:File" and "Image:Node." Then add fields "File:path," "Node->title," and "Customfield: PHP Code." Add this to your php customfield script:

//print var_export($data, TRUE);  //reveals all available variables

$image_src = $data->files_image_filepath;
$imagecache_preset = "Front-Slide-ic"; //enter your own imagecache preset here

$attributes = array('class' => 'lightbox');  //if you want to add attributes
$image_title = $data->node_image_title; 

print theme('imagecache', $imagecache_preset, $image_src, $image_title, $image_title, $attributes);

Using this method allows you to use Image Crop to manually select the crop area.

> Now anyone trying to apply this patch on a live site must visit and save the image presets page at least once. Maybe this can also be done with a hook_update,

@dman: Could you explain what you mean by this?

I've just tried applying the patch to an install of image module *without* admin-defined image sizes (or rather, I deleted the image_sizes variable to simulate this ;) and visited the image admin page and all seems fine.

> Now anyone trying to apply this patch on a live site must visit and save the image presets page at least once. Maybe this can also be done with a hook_update,

Just verifying I did have to do this on mine. I hadn't made any new presets, but changed the size for the "preview" image preset. When I went to "Image" there was no dropdown box for anything dealing with imagecache. I clicked "save configuration" and on the next page load, the new dropdowns for imagecache presets were there. I didn't see any errors or anything, there simply wasn't anything dealing with imagecache until I had done so.

Could someone explain exactly the steps to reproduce the problem? I'm not quite clear on that yet.

The one I encountered? It was as predicted in one of the posts above.
Here's the steps I followed and what I encountered:

Steps:

1. Install 'Recommended' Image
2. Install 'Recommended' Imagecache
3. Configure Both
4. Now apply the patches to imagecache and image dev from Comment #162 and Comment #167 above
5. Upload the patched dev branches of Image and Imagecache, overwriting old installs
6. Run Update.php

Expected: Imagecache presets would appear as options in the admin page for Image

Actual: No change in the Image Admin

WorkAround:

1. Go to Image Admin
2. Click 'Save Configuration' without changing anything
3. Imagecache presets should now show up as options

I'm not seeing that problem at all. Admittedly, this is on my test rig which has been kicking around for a while, but didn't yet have imagecache on it.

Steps I took:

1. downloaded and enabled imagecache.
2. patched all modules
3. created some IC presets
4. went to admin/settings/image. IC presets showed as expected.

There's no need to run update.php btw, as neither patch has update functions.

> The one I encountered? It was as predicted in one of the posts above.

Do you know what causes it?

@Dan: Have you given up on this issue? What can we do to help?

> A requested nice-to-have is an update hook that, um, ensures all your old image settings are re-saved in the new format.

That's not how I understood it... earlier comments seemed to indicate that right now, applying this patch breaks something. That's no good. If some sort of cleanup or conversion is required, it MUST be provided in an update function.

Thing is, I don't understand what breaks and I've not been able to reproduce it. Can anyone explain this clearly please?

Didn't break anything for me. All it involved was the extra step in going in and clicking "save configuration" before anything kicked in as dman has mentioned and as I've confirmed.

> And the level of potential broken-ness it : that you get a few php notices

Could you tell us what the notices actually are please?

> Heck, maybe an empty hook_update that just tells you to visit update.php would clear the pipes.

If it's just a matter of clearing Drupal's cache, then that's to be expected of module upgrades anyway.

Thanks for that very comprehensive report :)

I think 5a is obsolete -- nowhere is the index 'scale' actually used as an array key in the patch or indeed the entire module or admin files.

5b is solved by changing the offending line to:

    if (isset($operations[$size['operation']]['needs_dimensions'])) {

This is the D7 way -- doesn't hurt to get used to it now ;)

5c -- it's not just disabling imagecache that will cause this error. You can get it more easily by deleting a preset, and that's something users expect to be able to do without problems.

There's already this code in the imagecache patch:

  // Check the current image.module derivative definitions.
  $sizes = image_get_sizes() ;
  foreach ($sizes as $sizename => $size_def) {
    if (@$size_def['operation'] == 'imagecache-' . $presetid) {
      // Yes, this preset is being used by an image operation.
      drupal_set_message(t("Due do a change in the '%preset' preset, all derived image.module '%sizename' images need updating. This should happen over time as needed.", array('%preset' => $preset['presetname'], '%sizename' => $sizename)));
      // Trigger expiry on (all!) image.module derivatives.
      // This is inefficient, but image.module doesn't let us trigger just one 
      // preset change.
      variable_set('image_updated', time());
      // Once is enough.
      $rebuild_triggered = TRUE;
      return;
    }
  }

This will need changing -- for starters, it introduces a silent dependency with the call to image_get_sizes(). But what I would do instead is invoke a hook_imagecache_preset_changed() in various places so you can tell image module's implementation whether this was a change or a deletion.

(aside:
// This is inefficient, but image.module doesn't let us trigger just one
// preset change.
That's something that could be improved in a follow-up patch -- store times per preset.)

Also, I had another look at the patch...

+++ image.admin.inc	12 Oct 2010 07:51:56 -0000
@@ -110,12 +138,21 @@ function image_admin_settings() {
+    // rebuild (which I cannot force from here).

The convention is first person plural -- use 'we'.

+++ image.module	12 Oct 2010 07:51:57 -0000
@@ -849,24 +921,31 @@ function _image_build_derivatives($node,
     if (!$status) {
       drupal_set_message(t('Unable to create scaled %label image.', array('%label' => $size['label'])), 'error');
-      return FALSE;
+      // One failure may not mean that we should give up entirely. ¶
+      // Make the other derivatives anyway.
     }
     // Set standard file permissions for webserver-generated files
     @chmod($destination, 0664);

This doesn't look right. Yes to not just bailing out. But are those subsequent operations (the chmod, the alter) on the newly generated derivative? If so, those will surely cause problems. I reckon we need a 'continue' here.

Powered by Dreditor.

Oh yeah... PHP's empty/isset/is_null always confuses me :/

@Dan, @joachim: Again this patch seems to vanish into data nirvana. If I was able to follow this issue, there were a couple of success reports (the patches seem to work), and there were some coding style issues (which were fixed?). So how are the chances to get this into a dev release?

From my (naive) point of view as user it is very hard to understand what problems are left. Thus I'd simply like to remind of two facts:

• Imagecache was already used in the 5.x branch years ago; I can't remember if this was a patch or an unpatched release, but I used it for many months on several sites, and had no Imagecache related issues; in fact I was quite surprised that the Imagecache integration was gone after upgrading to D6;
• I believe that integrating Imagecache in 6.x should ease the migration to 7.x where Imagecache becomes the standard. If other modules from contrib have issues with derivates created/managed by Imagecache, these problems will surface anyway, be it now on D6 or be it later on D7.

Thank you!

The problems left -- at least the ones I found -- are outlined in 184 above :)

Patch fails on the latest version of Image(6.x-1.1 ) module ...

Found bug (maybe?) with Image after patching with #163 and #128. Not sure if I am supposed to post it here though.

Normally with ImageCache, you use Scale to create a preset if you want to resize using one value only. I made the error of selecting Resize instead of Scale. However, unlike what happened prior to patching, where you couldn't even create a Resize preset with a missing value, it allows you to create it and then when adding other presets, throws a bunch of errors at you.

warning: imagecreatetruecolor() [function.imagecreatetruecolor]: Invalid image dimensions in /home/treefest/public_html/dev/sites/all/modules/imageapi/imageapi_gd.module on line 274.
warning: imagealphablending(): supplied argument is not a valid Image resource in /home/treefest/public_html/dev/sites/all/modules/imageapi/imageapi_gd.module on line 291.
warning: imagecolorallocatealpha(): supplied argument is not a valid Image resource in /home/treefest/public_html/dev/sites/all/modules/imageapi/imageapi_gd.module on line 292.
warning: imagefill(): supplied argument is not a valid Image resource in /home/treefest/public_html/dev/sites/all/modules/imageapi/imageapi_gd.module on line 293.
warning: imagealphablending(): supplied argument is not a valid Image resource in /home/treefest/public_html/dev/sites/all/modules/imageapi/imageapi_gd.module on line 294.
warning: imagesavealpha(): supplied argument is not a valid Image resource in /home/treefest/public_html/dev/sites/all/modules/imageapi/imageapi_gd.module on line 295.
warning: imagecopyresampled(): supplied argument is not a valid Image resource in /home/treefest/public_html/dev/sites/all/modules/imageapi/imageapi_gd.module on line 157.

Removing the faulty Resize preset has fixed the errors and I can proceed as normal.

Seriously, thank you so much for the patches. It was a life saver!

Encountered major problem tonight and had to rollback the patches. I receiving a WSOD for pages that used many images and it was due to imagecache regenerating images when there was already an image available.

imagecache_imagesize_integration.patch queued for re-testing.

This doesn't need testing, it needs work.

subscribing

I use Imagecache 6.x-2.x and Image 6.x-1.1 and want intergration with imagecache presets for image module. Could you please indicate me if there is any patch that works?

It seems that nobody sees a future for 'Image' module beyond D7. Considering the age, and activity of this discussion, I'd suggest to "won't fix" it.

I think it's more that 'Nobody who needs image module to have a future is able or willing to write the code'.

I do wonder what everyone who's using image on D6 is going to do about upgrading to D7 though :/

At any rate, if someone works on this patch, I'll review and eventually commit it.

I do wonder what everyone who's using image on D6 is going to do about upgrading to D7 though

<cynical>It's currently just 85,266 'Image' module user that won't qualify as early adopters of D7, or won't qualify as D7 adopters at all ;)</cynical>

However, my Chipin proposal still stands (esp. to resolve the Image Attach issue), as there will come a time where my dozend D6 sites with 'Image' module and together half a million of image nodes want to become migrated. But for now, 'Image' module by far isn't the only showstopper.

as there will come a time where my dozend D6 sites with 'Image' module and together half a million of image nodes want to become migrated

Amen to that, but I'm comfortably in a mental state of denial towards that fact right now. I like being sane too much atm. Life is tough enough.

hey there,

just dropping by to share this link with you folks: http://drupal.org/node/201983#comment-4483072.

hope it helps :] I haven't tested yet, though.

cheers