In my sandbox (contributions/sandbox/robrechtj/image) I'm doing the following things:

  1. Split the current image.module in a image.module and imagegallery.module:
    • image.module will only contain the image node type and the necessary hooks to operate on the image and its derivative sizes
    • imagegallery.module will only contain the image gallery functionality. I don't have big plans here: just provide a simple gallery, not unlike the current implementation (based on "taxonomy" etc).

    This way image.module provides the bare minimum and the gallery stuff will not get in the way of third-party gallery modules.
    This part will not involve any new code.

  2. Add the necessary hooks to image.module to allow modules to generate custom derivative sizes
  3. Add the necessary hooks to image.module to allow modules to limit the access on derivative sizes
  4. Some cleaning up of documentation and API.
  5. Some examples of how to use the hooks above to do all kinds of cool stuff.

I hope these things will be incorporated, as a whole or in part, in the main image.module some day.

I'll post my progress on this issue every now and then as comments on this issue.
If you have questions or remarks on the way things are implemented or about the API, please post a comment here (or contact me by email) and I'll get back to you.

Comments

Robrecht Jacques’s picture

Robrecht Jacques commented

Status update:

  • image node type has been split from gallery functionality
  • no work has been done on the imagegallery.module: untested
  • most work has been done on the image.module:
    • access control should work
    • creating, editing and deleting images should work
    • defining your own sizes will probably not work yet, but I've documented the API in image_hook_docs.php
    • running your own filters will probably not work yet, but I've documented the API in image_hook_docs.php

Next steps:

  1. create a example module that implements the "eCommerce" use case. This will allow me to test:
    • access control
    • filter (watermark)
  2. squash some bugs

I'll keep you posted. Comments on API are allowed of course. It may still change, but will probably remain more or less the same in spirit.

Robrecht Jacques’s picture

Robrecht Jacques commented

Status update:

  1. I've split the hook into 3 hooks, seems cleaner as they do different things.
  2. I've implemented the "eCommerce" example. If you enable "image" and "image_example1" you get:
    • a watermark ("powered by drupal") on all bigger images (not thumbnail etc);
    • access to "_original" is limited (only users with even uid will see it).

    Note that the examples should not be included in the final "image.module": they are just examples of how to use the hooks. So they are just documentation and proof-of-concepts.

  3. Next things to do:
    • Handling of the "hook_image_sizes" that allows you to edit the standard sizes and/or provide your own.
    • Rewriting "image_settings" so we can validate the settings. This will become a "image_admin_page". It will not allow you to set filters, permissions or the link/edit state... just like now only width and height of sizes will be setable. More settings should be provided by other modules.
    • Write another example that shows how to use "hook_image_sizes".
    • Squash some bugs.
  4. Some random remarks
    • maybe "hook_image_access" should be handled at a lower level (hook_file or something like that).

No comments? Great!

anders.fajerson’s picture

anders.fajerson commented

Nice to see someonetaking a lead on this. Splitting the gallery and image functionalities sounds like an excellent approach.

Bèr Kessels’s picture

Bèr Kessels commented

just one comment: Great work!

Robrecht Jacques’s picture

Robrecht Jacques commented

Thanks for the kind words. :-)

Status update:

  • image_admin(): new settings page which allows for validation
  • image_get_sizes(), image_del_size(): rewrite to use hook_image_sizes('list')
  • new hook hook_image_filters('source'): this allows you to set a different image as starting point for the chain of filters
  • image_load(), image_insert(), image_delete(): rewrite to make sure that we don't delete files from other modules - this will not always work, see: #29297
  • checked image.module to make sure the paths are always correct: we always save a relative (to 'files/') path in $node->images and {files}

What works, what doesn't:

  • image.module: except for a couple of details (see TODO in source), all should work
  • hook_image_access(): should work
  • hook_image_sizes(): should work
  • hook_image_filters(): should work
  • imagegallery.module: no idea...

Next on list:

  • imagegallery.module
  • remove TODO's (by DOing them)
  • some more examples

There is only one incompatible change compared to CVS-HEAD (I think): the sizes on the settings page were saved in an array with for each size a size['label'], but now the list is an associative array ($label => $size). Resetting to defaults on "admin/settings/image" will clear it.

Anyone up for testing this? Don't use it on your production server! :-)

psicomante’s picture

psicomante commented

I'm testing it. It's a very good work. unfortunately admin/settings/image doesn't work (i see a blank page). i don't know what's the problem. Are there some requirements for these modules (GD, chmod, etc..)? The watermark in the image_example1.module isn't added. Module shows up this error:

    * Could not add watermark: composition failed.
    * Unable to create preview image.
    * Could not add watermark: composition failed.
    * Unable to create 1 image.
Robrecht Jacques’s picture

Robrecht Jacques commented

The requirements are the same as for image.module. Basically, a writable 'files/' directory.

The watermark example will only work with the GD toolkit. It uses the file misc/powered-gray-135x42.pgn image in the drupal root. I forgot to update the example (a filter must use relative paths instead of absolute ones).

I'm not sure what is wrong with the settings page. Only users with the 'administer images' permission are able to see this page.
I've updated the module in the sandbox.

Thx for testing, any feedback welcome.

Robrecht Jacques’s picture

Robrecht Jacques commented

OK sandbox/robrechtj/image should work as image.module before.

Some remaining issues:
- hook_imageapi_build('source') doesn't work as I would like it too, I'll need to do some thinking whether this was a good idea.
- hook_imageapi_sizes() may need some adjustements because most likely you will want to call imageapi_get_sizes() inside this hook which will cause recursion... something we don't want obviously.
- image-update.php not yet done, so updating won't work. The format in {files} has changed as did some of the settings, so it will need updating. So: running your current site with this code will not work.

So on my list now is:
- think some more
- sleep
- sleep some more
- finish it :-)

Seriously: it works unless you use the 2 hooks above which need some thinking.
Comment on it please. Maybe I'm making things way harder then needed...

Robrecht Jacques’s picture

Robrecht Jacques commented

Had a discussion with walkah yesterday on IRC. He has a number of problems/questions with the rewrite. I quote them here for reference and hope to resolve them.

  1. hook_imageapi_access():

    sep 09 01:18:35 walkah looking through, and thinking about it... i don't like image doing it's own private access layer for individual files
    sep 09 01:18:43 walkah that should be a fileAPI level thing
    sep 09 01:18:50 walkah imo
    sep 09 01:19:03 walkah because as soon as we release it for image - audio.module is gonna want it
    sep 09 01:19:09 walkah and upload will too, i'd bet
    sep 09 01:19:33 walkah i think {files} needs to get fixed to be "right" for files, and file.inc needs to handle more of this

    I agree with that and already said so (see comment #2, point 4).
    So I'll look into this. Some issues to make {files} more general and to add a "hook_file()" are already proposed, so let's see if I can use those or propose some kind of access there.

  2. hook_imageapi_sizes():

    sep 09 01:19:51 walkah as for your $sizes changes, i like them... as we've discussed before
    sep 09 01:20:17 walkah including an API for other size definitions

    OK. That's great :)

  3. hook_imageapi_build():

    sep 09 01:20:31 walkah i also mostly like the filter stuff
    sep 09 01:20:37 robrechtj ok
    sep 09 01:21:10 robrechtj mostly?
    sep 09 01:21:16 walkah the tricky part here... and i should go back and look how gallery2 is handling it... is when you get into lots of filters... the weighting needs to be done carefully
    sep 09 01:21:56 walkah i.e.different order of operations can give you very different results
    sep 09 01:22:10 walkah (steef can whip up some examples, i'm sure)
    sep 09 01:22:19 walkah but, i think you're on the right track
    sep 09 01:22:38 walkah just may need to introduce an admin ui for sorting them
    sep 09 01:22:46 robrechtj yes I agree, modules should agree on weights, now only "0" is fixed: the rescaling
    sep 09 01:23:13 robrechtj don't like an adimn page for that in image, but should like it in another module (imagetools or whatever)
    sep 09 01:23:31 walkah somewhere, but it should exist for advanced filter usage

    So in general it is "ok". Interesting enough, I got an email from Bruno Rocci:

    While I cannot spend as much time I would like on this module, I have been
    recently working on implementing an easy way to create 'image variations'
    (in fact a kind of filtering system). This allows a user with proper
    rights to define several variants of an image by combining, if needed,
    resize, watermark, color filter, etc, operations, operations based on
    functions found in image.inc

    This is not finished yet, but we may share our views on this, if you are
    interested.

    My original idea was actually to develop such a module. The thing is: hook_imageapi_build() allows for modules to add filters, but it doesn't allow for users to do this. Actually that is by design to keep imageapi.inc / image.module as simple as possible, so without providing a GUI for the users.
    If there is a "image_filters" (or "imagetools" or "...") module available that enables users to attach (and order) filters to each size, I think many of walkahs objections are met.
    So I hope to work with Bruno to see if we can use this module he is developing to allow this kind of user interface.

  4. user feedback of image_filters:

    sep 09 01:23:45 walkah the other thing is - not every filter will be available for every toolkit
    sep 09 01:24:08 walkah and some toolkits are better at certain operations than others
    sep 09 01:24:34 walkah we haven't really gotten into this yet - but there's lots to learn from g2 here
    sep 09 01:24:52 robrechtj so? that's the responsability of the module defining the filter, no?
    sep 09 01:24:55 walkah but *that* is the kind of stuff i think is perfect for an image api
    sep 09 01:25:09 walkah well, maybe
    sep 09 01:25:34 robrechtj if the toolkit doesn't define it, you can return $source and the filter will be skipped
    sep 09 01:25:44 walkah but if the site is using a toolkit that's not supporting something that the filter needs...
    sep 09 01:25:47 walkah ok
    sep 09 01:25:49 walkah right
    sep 09 01:25:53 walkah but that needs to be accounted for / noted / UI'ed somehow
    sep 09 01:25:55 robrechtj or if the module thinks the filter needs to be applied, it should return NULL and it fails
    sep 09 01:26:14 robrechtj well, a filter can drupal_set_message
    sep 09 01:26:20 walkah yes
    sep 09 01:26:37 walkah but i don't see them doing so in your examples... just noting it :)
    sep 09 01:27:03 robrechtj ah, just proof-of-concept :) doesn't the watermarking do it... hmm I can fix that
    sep 09 01:27:11 walkah yeah
    sep 09 01:27:15 walkah that's a minor thing
    sep 09 01:27:26 walkah but UI / usability is a hot button for Dries :)

    So here imageapi.inc needs to think a bit more about how it returns "failure" of image_filters. Currently a filter can return 3 things:

    • $destination: if the filter succeeded, and so the result is saved at that location;
    • $source: if the filter didn't need to do anything with the image, so it just returns the original image;
    • NULL: if the filter failed.

    If the filter returns NULL, it was assumed they would use drupal_set_message() to tell the user why.
    The problem is a bit that the building of derivatives is not always done when creating the image node, but different sizes are created on-demand if they are out of date. So it is possible that you (or a visitor) is looking at a node and that you get an 'error' about a failing filter.
    Need to think about it a bit.

  5. $fieldname:
    In imageapi.inc all functions have a $fieldname parameter that tells imageapi.inc in which node-field the images should be saved. walkah didn't really like that. If I understand him correctly: this smells too much like CCK and isn't quite thought through: CCK will want to say something like I want an image field added to a node, which only has these "$sizes" and with its own set of filters if needed.
    In some ways imageapi.inc will already allow for this:
    • a module (eg CCK) could define its own sizes for an image field (hook_imageapi_sizes())
    • a module can attach filters only to the fields he defines as hook_imageapi_build($op, $label, $node, $fieldname) passes $fieldname as parameter too
    • one thing that is not really possible is to limit the $sizes build for an image field: imageapi.inc will happely create a derivative size for each $label / $node / $fieldname combination it gets. As long as nothing calls "imageapi_display(...)" with a $size that the module doesn't want to see created, it will not get created. Still, it might be nicer if there is some $sizes limitation somewhere.

    I think I understand this, but need to think a bit about a solution.
    One solutino is to "remove" the $fieldname stuff and only readd something similar later (like when CCK is "ready/mature").

I'll try to solve these objections but if someone has ideas, lemme know.

garyd9’s picture

garyd9 commented

Some random thoughts (based on stuff I'm doing myself)

When there are thousands of potential images, it's a Bad Thing for the image module to stuff all files into the same filesystem directory. My current image.module hacks make a derived path based on the nid for the image, using the last three digits of the nid as subdirectories. I pass the nid as a parameter to _image_filename() and if its set, and $temp is FALSE, I do this (sloppy draft code): 

$x .= (int) (($nid % 1000) / 100);
$tout .= $x . '/';
// for some reason, its not letting me create the entire dir tree at once, even
// though PHP's mkdir() DOES allow that..
file_check_directory($tout, FILE_CREATE_DIRECTORY);
$y = (int) (($nid % 100) / 10);
$tout .= '/' . $y . '/';
file_check_directory($tout, FILE_CREATE_DIRECTORY);
$z = $nid % 10;
$tout .= '/' . $z . '/';
file_check_directory($tout, FILE_CREATE_DIRECTORY);

$output .= "$x/$y/$z/";

The image module should probably be somewhat aware of EXIF tags. Then again, that would be better for a "photograph" module than an image module. Perhaps, then, image.module should allow for extra fields and have it's own unique insert/update/delete hooks similar to generic nodes.

In my own case, I'm adding the following fields: date/time created (which is unique from date/time submitted), artist (again, this is likely to be different from the drupal ID that's submitting the image), caption/title (which I'm pushing into the drupal node title), keywords (I'm adding this as an extra field and using drupal's search hooks to add them to the index - though they are never visible directly) and finally some kind of 'sort' field. This is similar to how 'weight' is used to sort books, but much more open ended. When there are 40 images in a given album, a weight range of -10 to +10 doesn't work out. In my own case, I'm happily using G2's idea of a nice integer...

The problem I see is that everyone uses "images" for different things. For me, it's a photo album that has artists, captions, creation dates, etc. Another person might want a more photography oriented album, in which case they'd want more technical information such as camera make/model, shutter speeds, etc. For someone else, it's a collection of clipart where only a title is needed. Another person might (does) use this for game wallpapers. I honestly don't know what fields they want... Some of these fields should be stored in the database so they can be indexed and searched/sorted. Other's might be okay outside of the database, but then should probably be cached for fast display (such as certain technical EXIF info.)

Then we get into the whole "imagegallery.module" thing... :) How seperate should it be, and how seperate CAN it be? There are bound to be elements of an imagegallery that are coded into the image.module - such as any type of sort weight.

I haven't started digging into the taxonomy code quite yet (which is what the existing image.module uses for albums) but I don't think there are any hooks for taxonomy terms similar to node hooks. ie: there's no built-in mechanism to add tid specific fields to a term. That will make my life interesting when I want to specify that album "xxx" should have 4 images per page with 200px thumbnails and album "yyy" should have 21 images per page with 150px thumbnails... Even should I accomplish that, what will happen when a photo is in BOTH "xxx" and "yyy" albums?

Now, I know that some people will read this and suggest I just use the menalto gallery (either G1 or G2), but I've been there and done that. I started with G1, and ended up having to modify it greatly to work the way I wanted. Unfortunatly, my mods made patching to new versions/revisions nearly impossible. I had great hopes for G2 and their supposedly modular and object oriented approach. Unfortunatly, it wasn't nearly as OO or modular as I had hoped, and my "suggestions" on fixing that were mostly ignored. In other words, to get the functionality I wanted, I'd have to make changes to the G2 core, leaving me in the same boat as I'm already in with G1 (and, oddly, the same boat that the menalto site itself claimed to be in when changing to use drupal...)

Robrecht Jacques’s picture

Robrecht Jacques commented
  • large directories: this kind of scheme of saving images in different directories based on $nid was exactly something I was thinking about too. Not sure yet if it will make it in the final result.
  • exif: this is exactly an example that should be provided by a different module: image.module takes care of the images itself (and making thumbnails) while exif.module would parse the _original image to get the tags and do something with it. That is all quite easy with the nodeapi hooks I think. I have a "exif.module" on my todo list btw.

    Something like setting a title is not possible until the "pre validate" hooks are in place. Setting a title of an image node is not different from setting the title of a normal story node, so why should there be code in image.module to handle this? There shouldn't.

  • imagegallery: everyone has different needs with this one, one of the reasons why splitting it out of image.module. I don't plan to do much with imagegallery.module: it will basically be the same as what is in the current image.module. There are other album or gallery modules that provide an alternative... maybe with time the One True Gallery will emerge.

    But imagegallery can be done completely independant from image.module. If you have tid specific fields you'll need to look at "hook_taxonomy()" which provides all you need (similar to hook_nodeapi()). If your gallery.module needs some kind of weights attached to a image node, you'll need to extend the image node to have such a field settable (probably using hook_nodeapi()).

  • That will make my life interesting when I want to specify that album "xxx" should have 4 images per page with 200px thumbnails and album "yyy" should have 21 images per page with 150px thumbnails... Even should I accomplish that, what will happen when a photo is in BOTH "xxx" and "yyy" albums?

    You could do this by defining a "gallery-xxx" size and a "gallery-yyy" size. In "xxx" gallery tids you would then use "gallery-xxx" as thumbnails. Both sizes (gallery-xxx, gallery-yyy) will be generated on demand by image.module. With some theming of imagegallery.module you could choose the right $size in the right gallery. You would still need to define an own table which stores "this gallery is a xxx or yyy" property of the gallery tids.

The idea of the "restructuring" of image.module is to provide a _simple_ capable image.module, not to implement every kind of feature possible. Eg many people want sexy squared thumbnails. Great, but image.module will not give them to you, image.module will just allow you implement this in a third-party module. That's it.
All things you have mentioned can be done by image.module or by image.module plus extra (to be written) modules. For everyone this list of (to be written) modules will be different. I don't want to clobber up the image settings page with all kind of options.

In the end image.module will just provide a image node type where you can plug into the building system of derivatives and nothing more.

garyd9’s picture

garyd9 commented

I'll try to find your code in the sandbox later this evening (or at least sometime this week - time isn't something I can commit freely [I have 2 children]) and play around with it some. Does this work with drupal 4.6, or do I need to pull from HEAD?

Depending on whats there, my needs, the phase of the moon, etc, I might backport the stuff I've already done with the current image.module into your split and send you the changes. That might save you a bit of time on some things, though I tend to code for my own needs instead of generics (meaning it might be useless for you.) (Feel free to use the directory stuff I posted above - though I'm not sure how platform portable it is... and I know it's ugly)

I'm not sure that exif functionality "works" as a conceptual module. The "least desired" functionality would probably be something that would read EXIF info on demand, which would basically be a single function call that takes a node as a parameter, and returns an associative array of exif fields. This, in turn, is merely a wrapper for one of the many available exif/IPTC libraries available - simply translating a "node" into a filename. Would that be better as a module, or as an .inc? Yes, a more involved system would likely store some or all of the exif fields to a table, but I get the impression that you prefer to take a more layered approach, providing a basic API at the lowest level. Let me whip up something basic on this in the next couple of days (again, when I have more than 1 hour of free time) and show you what I mean. I'll probably end up with an image_exif.inc file with a single image_exif_getExif() function, and then 1 or 2 other files (such as the GPL'd exifer code.)

Something like setting a title is not possible until the "pre validate" hooks are in place. Setting a title of an image node is not different from setting the title of a normal story node, so why should there be code in image.module to handle this? There shouldn't.

I disagree that it's not different from setting the title of a normal story node. A story (and therefore page/blog) node is a title and body. The title describes the body, and drupal is set up so that you manually edit the body of the node along with the title. In other words, when you create story content, you start with nothing and create both the title and body. There is no provision within drupal to 'import' pre-made stories into story nodes. An image differs in that the "body" of the node (the image) is pre-existing. The image module actually imports the existing image file into the drupal system. Therefore, its reasonable to assume that the title is also pre-existing... (There, a good argument for why image.module should handle setting the title...:P) I'm not saying that image.module should specifically set a title, but that image.module should provide, at least, a hook to allow something else to set a title. Perhaps the validate hook in image.module call any hooks, and if the title was still empty, set it to be the same as the base image filename.

Do you have plans of starting a new project for this image stuff, or is it destined to replace walkah's project, or ??. The reason I ask is that it might get confusing to submit patches to the 'image.module' project when there are two different active ones at the moment...

BTW, is your imagegallery.module split working yet? I suppose I need to just pop into CVS and take a look...

bruno’s picture

bruno commented

I agree completly with Jacques when he says:

The idea of the "restructuring" of image.module is to provide a _simple_ capable image.module, not to implement every kind of feature possible. Eg many people want sexy squared thumbnails. Great, but image.module will not give them to you, image.module will just allow you implement this in a third-party module. That's it.

Previous versions of image.module have failed short because everybody wanted "his" feature into one single module, one single source, which became very difficult to maintain. Splitting the module into "core subsets", letting people use basic functionality if they are happy with or allowing them to create their customized version with well documented API looks more sound to me. "One size doesn't fit all", right?

Keep up with the good work!

Note:
For me "core subsets" means:

  • image operations

    • Basicaly, what we have in image.inc: a set of low level function based on image libraries such as GD or ImageMagick to transform images (resize, rotate, ...). Needs to be extended.

  • image variations (or derivatives)

    • This is a kind of abstraction layer which enables creation of new images based on a combination of operations. Each image variation is named. For example, I want a "Preview" image variation which is my original image resized and in black & white. I want also a "Normal" image which is also a resized version, which keeps the colors but has a watermark. Once you have "named variations", you could imagine that only registered users could have access to the "Normal" variation while visitors could see only "Thumbnails" and "Preview" images.

  • image API

    • Here we want to provide to external modules functions to deal with image upload, image management (hidding underlaying file system), image integration support into other nodes (filters?), etc.

  • image applications examples

    • Typicaly, a simple image gallery. The one we have today is a good start.

Stefan Nagtegaal’s picture

Stefan Nagtegaal commented

Robrecht please let me know if your still around. I would like to discuss some things with you about this truly interesting project you once started, and abandonded too fast.. ;-)

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Active » Closed (won't fix)

@Robrecht Jacques: Sorry, I have to close this issue now. I would love to see your contributions in Image module though! So please do not hesitate to submit some patches.