Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: walkah Reported by: walkah Updated by: walkah Status: patch Attachment: http://drupal.org/files/issues/image-core.patch (7.22 KB) and the patch (as mentioned above). walkah Previous comments: ------------------------------------------------------------------------ January 28, 2005 - 15:31 : walkah Attachment: http://drupal.org/files/issues/image_0.inc (7.44 KB) Due to popular demand - attached here is an image.inc for inclusion (in includes/), that will offer some basic image detection and manipulation functionality to drupal's core. currently image.inc is responsible for managing image 'toolkits' - these are either PHP modules (e.g. GD) or external programs (e.g. imagemagick) that handle the actual manipulation of images. also included is the default "GD" toolkit (that supports both GD1 or GD2, depending on which is installed). PHP comes bundled with gd as of version 4.3 - therefore this is a reasonable default. However, other toolkits will follow (I presume) - they can be installed in includes/ (as image.$toolkit.inc). There are 5 main functions that modules may now utilize to handle images: * image_get_info() - this function checks a file - if it exists and is a valid image file , it will return an array containing things like the pixel dimensions of the image, plus the 'type' and common extension. * image_scale - resizes a given image to fit within a given width / height dimensions, while maintaining aspect ratio (not distorting the image). - this function can be used to generate thumbnails, or ensure a maximum resolution, etc. * image_resize - similar to image_scale (but will not respect aspect ratio - may well distort the image). * image_rotate - rotate an image by X degrees * image_crop - crops an image to a given rectangle (defined as top-left x/y coordinates plus a width & height of the rectangle). In a follow up I will post a patch that does the following: * adds some default settings to admin/settings : * toolkit selection (and possibly configuration) * default thumbnail dimensions (WxH) - to be used as a system-wide default * maximum dimensions (WxH ) - to be used as a system-wide default * changes the user picture handling to silently resize uploaded pictures to fit within the specified dimensions (rather than throwing errors). * uses image_get_info() to verify logo uploads in admin/theme configuration * patches upload.module to respect maximum dimensions for uploaded images. Contribution modules will now be able to rely on these base manipulation functions to offer additional functionality (such as image nodes, photo galleries, advanced image manipulation, etc) -- View: http://drupal.org/node/16358 Edit: http://drupal.org/project/comments/add/16358

Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: walkah Reported by: walkah Updated by: walkah Status: patch 'type' - is used primarily by the gd functions, which open image files using createFromPng / createFromJPEG / etc... and they use 'JPEG' not 'JPG' 'ext' - is the common file extension - i.e. 'JPG' not 'JPEG'. i agree, kind of a shame, but mostly harmless (imo) walkah Previous comments: ------------------------------------------------------------------------ January 28, 2005 - 15:31 : walkah Attachment: http://drupal.org/files/issues/image_0.inc (7.44 KB) Due to popular demand - attached here is an image.inc for inclusion (in includes/), that will offer some basic image detection and manipulation functionality to drupal's core. currently image.inc is responsible for managing image 'toolkits' - these are either PHP modules (e.g. GD) or external programs (e.g. imagemagick) that handle the actual manipulation of images. also included is the default "GD" toolkit (that supports both GD1 or GD2, depending on which is installed). PHP comes bundled with gd as of version 4.3 - therefore this is a reasonable default. However, other toolkits will follow (I presume) - they can be installed in includes/ (as image.$toolkit.inc). There are 5 main functions that modules may now utilize to handle images: * image_get_info() - this function checks a file - if it exists and is a valid image file , it will return an array containing things like the pixel dimensions of the image, plus the 'type' and common extension. * image_scale - resizes a given image to fit within a given width / height dimensions, while maintaining aspect ratio (not distorting the image). - this function can be used to generate thumbnails, or ensure a maximum resolution, etc. * image_resize - similar to image_scale (but will not respect aspect ratio - may well distort the image). * image_rotate - rotate an image by X degrees * image_crop - crops an image to a given rectangle (defined as top-left x/y coordinates plus a width & height of the rectangle). In a follow up I will post a patch that does the following: * adds some default settings to admin/settings : * toolkit selection (and possibly configuration) * default thumbnail dimensions (WxH) - to be used as a system-wide default * maximum dimensions (WxH ) - to be used as a system-wide default * changes the user picture handling to silently resize uploaded pictures to fit within the specified dimensions (rather than throwing errors). * uses image_get_info() to verify logo uploads in admin/theme configuration * patches upload.module to respect maximum dimensions for uploaded images. Contribution modules will now be able to rely on these base manipulation functions to offer additional functionality (such as image nodes, photo galleries, advanced image manipulation, etc) ------------------------------------------------------------------------ January 28, 2005 - 15:50 : walkah Attachment: http://drupal.org/files/issues/image-core.patch (7.22 KB) and the patch (as mentioned above). ------------------------------------------------------------------------ January 28, 2005 - 19:41 : rkendall Thanks for that, I think I could find that useful. Not meaning to criticize your very nice looking work, but I don't quite get the use of the ext info in image_get_info() , it just seems to be duplicating information from the type info. Thanks again, Ross. -- View: http://drupal.org/node/16358 Edit: http://drupal.org/project/comments/add/16358

Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: walkah Reported by: walkah Updated by: Bèr Kessels Status: patch Is there a good reason for introducing YAPS (yet another plugin system) and not using tour well tested, optimised and wellknown .module system for the toolkits? Is there really a differerence in performance and complexity between: image.$toolkit.inc and image_gd.module? If not, please (with sugar on top and such), we should not make more/new plugin systems. Bèr Bèr Kessels Previous comments: ------------------------------------------------------------------------ January 28, 2005 - 21:31 : walkah Attachment: http://drupal.org/files/issues/image_0.inc (7.44 KB) Due to popular demand - attached here is an image.inc for inclusion (in includes/), that will offer some basic image detection and manipulation functionality to drupal's core. currently image.inc is responsible for managing image 'toolkits' - these are either PHP modules (e.g. GD) or external programs (e.g. imagemagick) that handle the actual manipulation of images. also included is the default "GD" toolkit (that supports both GD1 or GD2, depending on which is installed). PHP comes bundled with gd as of version 4.3 - therefore this is a reasonable default. However, other toolkits will follow (I presume) - they can be installed in includes/ (as image.$toolkit.inc). There are 5 main functions that modules may now utilize to handle images: * image_get_info() - this function checks a file - if it exists and is a valid image file , it will return an array containing things like the pixel dimensions of the image, plus the 'type' and common extension. * image_scale - resizes a given image to fit within a given width / height dimensions, while maintaining aspect ratio (not distorting the image). - this function can be used to generate thumbnails, or ensure a maximum resolution, etc. * image_resize - similar to image_scale (but will not respect aspect ratio - may well distort the image). * image_rotate - rotate an image by X degrees * image_crop - crops an image to a given rectangle (defined as top-left x/y coordinates plus a width & height of the rectangle). In a follow up I will post a patch that does the following: * adds some default settings to admin/settings : * toolkit selection (and possibly configuration) * default thumbnail dimensions (WxH) - to be used as a system-wide default * maximum dimensions (WxH ) - to be used as a system-wide default * changes the user picture handling to silently resize uploaded pictures to fit within the specified dimensions (rather than throwing errors). * uses image_get_info() to verify logo uploads in admin/theme configuration * patches upload.module to respect maximum dimensions for uploaded images. Contribution modules will now be able to rely on these base manipulation functions to offer additional functionality (such as image nodes, photo galleries, advanced image manipulation, etc) ------------------------------------------------------------------------ January 28, 2005 - 21:50 : walkah Attachment: http://drupal.org/files/issues/image-core.patch (7.22 KB) and the patch (as mentioned above). ------------------------------------------------------------------------ January 29, 2005 - 01:41 : rkendall Thanks for that, I think I could find that useful. Not meaning to criticize your very nice looking work, but I don't quite get the use of the /ext/ info in image_get_info() , it just seems to be duplicating information from the /type/ info. Thanks again, Ross. ------------------------------------------------------------------------ January 29, 2005 - 13:13 : walkah 'type' - is used primarily by the gd functions, which open image files using createFromPng / createFromJPEG / etc... and they use 'JPEG' not 'JPG' 'ext' - is the common file extension - i.e. 'JPG' not 'JPEG'. i agree, kind of a shame, but mostly harmless (imo) ------------------------------------------------------------------------ January 30, 2005 - 19:07 : Dries Please rename 'ext' to 'extension' and document the difference between 'type' and 'extension'. It is confusing at best. Personally, I prefer to use 'resolution' instead of 'res', and 'destination' instead of 'dest'. The punctuation in the PHPdoc code seems random: some sentences end with a point, others do not. Some code seems to wrap but that might be due to the fact that the image.inc file has been uploaded with the wrong MIME-type. Otherwise this patch looks good, but I haven't really tested it yet. -- View: http://drupal.org/node/16358 Edit: http://drupal.org/project/comments/add/16358

Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: walkah Reported by: walkah Updated by: walkah Status: patch ber - re the toolkit "framework" vs. modules - the issue is that there is 0 overlap between toolkit functionality and the current set of module hooks. also, the point of image.inc is to encapsulate the toolkit functionality so that module developers, etc need not worry about which toolkit(s) are installed and available. i'm not sure, though, that toolkits really fit as modules. i.e. can you throttle a toolkit? is there any case where a toolkit could or should implement /any/ of the current module hooks? anyone else have some thoughts on this? I think polluting the module namespace with a ton of image support files would be worse than the 20 or so lines to implement this "custom plugin" framework. maybe i'm off base though. walkah Previous comments: ------------------------------------------------------------------------ January 28, 2005 - 15:31 : walkah Attachment: http://drupal.org/files/issues/image_0.inc (7.44 KB) Due to popular demand - attached here is an image.inc for inclusion (in includes/), that will offer some basic image detection and manipulation functionality to drupal's core. currently image.inc is responsible for managing image 'toolkits' - these are either PHP modules (e.g. GD) or external programs (e.g. imagemagick) that handle the actual manipulation of images. also included is the default "GD" toolkit (that supports both GD1 or GD2, depending on which is installed). PHP comes bundled with gd as of version 4.3 - therefore this is a reasonable default. However, other toolkits will follow (I presume) - they can be installed in includes/ (as image.$toolkit.inc). There are 5 main functions that modules may now utilize to handle images: * image_get_info() - this function checks a file - if it exists and is a valid image file , it will return an array containing things like the pixel dimensions of the image, plus the 'type' and common extension. * image_scale - resizes a given image to fit within a given width / height dimensions, while maintaining aspect ratio (not distorting the image). - this function can be used to generate thumbnails, or ensure a maximum resolution, etc. * image_resize - similar to image_scale (but will not respect aspect ratio - may well distort the image). * image_rotate - rotate an image by X degrees * image_crop - crops an image to a given rectangle (defined as top-left x/y coordinates plus a width & height of the rectangle). In a follow up I will post a patch that does the following: * adds some default settings to admin/settings : * toolkit selection (and possibly configuration) * default thumbnail dimensions (WxH) - to be used as a system-wide default * maximum dimensions (WxH ) - to be used as a system-wide default * changes the user picture handling to silently resize uploaded pictures to fit within the specified dimensions (rather than throwing errors). * uses image_get_info() to verify logo uploads in admin/theme configuration * patches upload.module to respect maximum dimensions for uploaded images. Contribution modules will now be able to rely on these base manipulation functions to offer additional functionality (such as image nodes, photo galleries, advanced image manipulation, etc) ------------------------------------------------------------------------ January 28, 2005 - 15:50 : walkah Attachment: http://drupal.org/files/issues/image-core.patch (7.22 KB) and the patch (as mentioned above). ------------------------------------------------------------------------ January 28, 2005 - 19:41 : rkendall Thanks for that, I think I could find that useful. Not meaning to criticize your very nice looking work, but I don't quite get the use of the /ext/ info in image_get_info() , it just seems to be duplicating information from the /type/ info. Thanks again, Ross. ------------------------------------------------------------------------ January 29, 2005 - 07:13 : walkah 'type' - is used primarily by the gd functions, which open image files using createFromPng / createFromJPEG / etc... and they use 'JPEG' not 'JPG' 'ext' - is the common file extension - i.e. 'JPG' not 'JPEG'. i agree, kind of a shame, but mostly harmless (imo) ------------------------------------------------------------------------ January 30, 2005 - 13:07 : Dries Please rename 'ext' to 'extension' and document the difference between 'type' and 'extension'. It is confusing at best. Personally, I prefer to use 'resolution' instead of 'res', and 'destination' instead of 'dest'. The punctuation in the PHPdoc code seems random: some sentences end with a point, others do not. Some code seems to wrap but that might be due to the fact that the image.inc file has been uploaded with the wrong MIME-type. Otherwise this patch looks good, but I haven't really tested it yet. ------------------------------------------------------------------------ January 31, 2005 - 09:44 : Bèr Kessels Is there a good reason for introducing YAPS (yet another plugin system) and not using tour well tested, optimised and wellknown .module system for the toolkits? Is there really a differerence in performance and complexity between: image.$toolkit.inc and image_gd.module? If not, please (with sugar on top and such), we should not make more/new plugin systems. Bèr ------------------------------------------------------------------------ January 31, 2005 - 09:45 : Bèr Kessels forgot: But other than that, great work, a big +1 from me to go into core. -- View: http://drupal.org/node/16358 Edit: http://drupal.org/project/comments/add/16358

Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: walkah Reported by: walkah Updated by: killes@www.drop.org Status: patch I don't think having the .inc files as modules makes sense. killes@www.drop.org Previous comments: ------------------------------------------------------------------------ January 28, 2005 - 21:31 : walkah Attachment: http://drupal.org/files/issues/image_0.inc (7.44 KB) Due to popular demand - attached here is an image.inc for inclusion (in includes/), that will offer some basic image detection and manipulation functionality to drupal's core. currently image.inc is responsible for managing image 'toolkits' - these are either PHP modules (e.g. GD) or external programs (e.g. imagemagick) that handle the actual manipulation of images. also included is the default "GD" toolkit (that supports both GD1 or GD2, depending on which is installed). PHP comes bundled with gd as of version 4.3 - therefore this is a reasonable default. However, other toolkits will follow (I presume) - they can be installed in includes/ (as image.$toolkit.inc). There are 5 main functions that modules may now utilize to handle images: * image_get_info() - this function checks a file - if it exists and is a valid image file , it will return an array containing things like the pixel dimensions of the image, plus the 'type' and common extension. * image_scale - resizes a given image to fit within a given width / height dimensions, while maintaining aspect ratio (not distorting the image). - this function can be used to generate thumbnails, or ensure a maximum resolution, etc. * image_resize - similar to image_scale (but will not respect aspect ratio - may well distort the image). * image_rotate - rotate an image by X degrees * image_crop - crops an image to a given rectangle (defined as top-left x/y coordinates plus a width & height of the rectangle). In a follow up I will post a patch that does the following: * adds some default settings to admin/settings : * toolkit selection (and possibly configuration) * default thumbnail dimensions (WxH) - to be used as a system-wide default * maximum dimensions (WxH ) - to be used as a system-wide default * changes the user picture handling to silently resize uploaded pictures to fit within the specified dimensions (rather than throwing errors). * uses image_get_info() to verify logo uploads in admin/theme configuration * patches upload.module to respect maximum dimensions for uploaded images. Contribution modules will now be able to rely on these base manipulation functions to offer additional functionality (such as image nodes, photo galleries, advanced image manipulation, etc) ------------------------------------------------------------------------ January 28, 2005 - 21:50 : walkah Attachment: http://drupal.org/files/issues/image-core.patch (7.22 KB) and the patch (as mentioned above). ------------------------------------------------------------------------ January 29, 2005 - 01:41 : rkendall Thanks for that, I think I could find that useful. Not meaning to criticize your very nice looking work, but I don't quite get the use of the /ext/ info in image_get_info() , it just seems to be duplicating information from the /type/ info. Thanks again, Ross. ------------------------------------------------------------------------ January 29, 2005 - 13:13 : walkah 'type' - is used primarily by the gd functions, which open image files using createFromPng / createFromJPEG / etc... and they use 'JPEG' not 'JPG' 'ext' - is the common file extension - i.e. 'JPG' not 'JPEG'. i agree, kind of a shame, but mostly harmless (imo) ------------------------------------------------------------------------ January 30, 2005 - 19:07 : Dries Please rename 'ext' to 'extension' and document the difference between 'type' and 'extension'. It is confusing at best. Personally, I prefer to use 'resolution' instead of 'res', and 'destination' instead of 'dest'. The punctuation in the PHPdoc code seems random: some sentences end with a point, others do not. Some code seems to wrap but that might be due to the fact that the image.inc file has been uploaded with the wrong MIME-type. Otherwise this patch looks good, but I haven't really tested it yet. ------------------------------------------------------------------------ January 31, 2005 - 15:44 : Bèr Kessels Is there a good reason for introducing YAPS (yet another plugin system) and not using tour well tested, optimised and wellknown .module system for the toolkits? Is there really a differerence in performance and complexity between: image.$toolkit.inc and image_gd.module? If not, please (with sugar on top and such), we should not make more/new plugin systems. Bèr ------------------------------------------------------------------------ January 31, 2005 - 15:45 : Bèr Kessels forgot: But other than that, great work, a big +1 from me to go into core. ------------------------------------------------------------------------ January 31, 2005 - 15:58 : walkah ber - re the toolkit "framework" vs. modules - the issue is that there is 0 overlap between toolkit functionality and the current set of module hooks. also, the point of image.inc is to encapsulate the toolkit functionality so that module developers, etc need not worry about which toolkit(s) are installed and available. i'm not sure, though, that toolkits really fit as modules. i.e. can you throttle a toolkit? is there any case where a toolkit could or should implement /any/ of the current module hooks? anyone else have some thoughts on this? I think polluting the module namespace with a ton of image support files would be worse than the 20 or so lines to implement this "custom plugin" framework. maybe i'm off base though. ------------------------------------------------------------------------ January 31, 2005 - 16:08 : moshe weitzman i agree that the toolkit framework proposed here is justified. This is a plugin system just like our DB abstraction layer, which has served us very well. It is also analogous to the flexinode plugin system, which everyone seems to love. +1 -- View: http://drupal.org/node/16358 Edit: http://drupal.org/project/comments/add/16358

Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: walkah Reported by: walkah Updated by: walkah Status: patch Attachment: http://drupal.org/files/issues/image_1.inc (7.64 KB) attached is an updated image.inc - taking into consideration dries' suggestions. biggest is : removing 'type' from image_get_info - moving that to the GD functions where the jpg vs. JPEG distinction is required. also, tried to clean up some phpdoc comments, and use "destination" and "extension" instead of "dest" and "ext". (updated patch to follow). walkah Previous comments: ------------------------------------------------------------------------ January 28, 2005 - 15:31 : walkah Attachment: http://drupal.org/files/issues/image_0.inc (7.44 KB) Due to popular demand - attached here is an image.inc for inclusion (in includes/), that will offer some basic image detection and manipulation functionality to drupal's core. currently image.inc is responsible for managing image 'toolkits' - these are either PHP modules (e.g. GD) or external programs (e.g. imagemagick) that handle the actual manipulation of images. also included is the default "GD" toolkit (that supports both GD1 or GD2, depending on which is installed). PHP comes bundled with gd as of version 4.3 - therefore this is a reasonable default. However, other toolkits will follow (I presume) - they can be installed in includes/ (as image.$toolkit.inc). There are 5 main functions that modules may now utilize to handle images: * image_get_info() - this function checks a file - if it exists and is a valid image file , it will return an array containing things like the pixel dimensions of the image, plus the 'type' and common extension. * image_scale - resizes a given image to fit within a given width / height dimensions, while maintaining aspect ratio (not distorting the image). - this function can be used to generate thumbnails, or ensure a maximum resolution, etc. * image_resize - similar to image_scale (but will not respect aspect ratio - may well distort the image). * image_rotate - rotate an image by X degrees * image_crop - crops an image to a given rectangle (defined as top-left x/y coordinates plus a width & height of the rectangle). In a follow up I will post a patch that does the following: * adds some default settings to admin/settings : * toolkit selection (and possibly configuration) * default thumbnail dimensions (WxH) - to be used as a system-wide default * maximum dimensions (WxH ) - to be used as a system-wide default * changes the user picture handling to silently resize uploaded pictures to fit within the specified dimensions (rather than throwing errors). * uses image_get_info() to verify logo uploads in admin/theme configuration * patches upload.module to respect maximum dimensions for uploaded images. Contribution modules will now be able to rely on these base manipulation functions to offer additional functionality (such as image nodes, photo galleries, advanced image manipulation, etc) ------------------------------------------------------------------------ January 28, 2005 - 15:50 : walkah Attachment: http://drupal.org/files/issues/image-core.patch (7.22 KB) and the patch (as mentioned above). ------------------------------------------------------------------------ January 28, 2005 - 19:41 : rkendall Thanks for that, I think I could find that useful. Not meaning to criticize your very nice looking work, but I don't quite get the use of the /ext/ info in image_get_info() , it just seems to be duplicating information from the /type/ info. Thanks again, Ross. ------------------------------------------------------------------------ January 29, 2005 - 07:13 : walkah 'type' - is used primarily by the gd functions, which open image files using createFromPng / createFromJPEG / etc... and they use 'JPEG' not 'JPG' 'ext' - is the common file extension - i.e. 'JPG' not 'JPEG'. i agree, kind of a shame, but mostly harmless (imo) ------------------------------------------------------------------------ January 30, 2005 - 13:07 : Dries Please rename 'ext' to 'extension' and document the difference between 'type' and 'extension'. It is confusing at best. Personally, I prefer to use 'resolution' instead of 'res', and 'destination' instead of 'dest'. The punctuation in the PHPdoc code seems random: some sentences end with a point, others do not. Some code seems to wrap but that might be due to the fact that the image.inc file has been uploaded with the wrong MIME-type. Otherwise this patch looks good, but I haven't really tested it yet. ------------------------------------------------------------------------ January 31, 2005 - 09:44 : Bèr Kessels Is there a good reason for introducing YAPS (yet another plugin system) and not using tour well tested, optimised and wellknown .module system for the toolkits? Is there really a differerence in performance and complexity between: image.$toolkit.inc and image_gd.module? If not, please (with sugar on top and such), we should not make more/new plugin systems. Bèr ------------------------------------------------------------------------ January 31, 2005 - 09:45 : Bèr Kessels forgot: But other than that, great work, a big +1 from me to go into core. ------------------------------------------------------------------------ January 31, 2005 - 09:58 : walkah ber - re the toolkit "framework" vs. modules - the issue is that there is 0 overlap between toolkit functionality and the current set of module hooks. also, the point of image.inc is to encapsulate the toolkit functionality so that module developers, etc need not worry about which toolkit(s) are installed and available. i'm not sure, though, that toolkits really fit as modules. i.e. can you throttle a toolkit? is there any case where a toolkit could or should implement /any/ of the current module hooks? anyone else have some thoughts on this? I think polluting the module namespace with a ton of image support files would be worse than the 20 or so lines to implement this "custom plugin" framework. maybe i'm off base though. ------------------------------------------------------------------------ January 31, 2005 - 10:08 : moshe weitzman i agree that the toolkit framework proposed here is justified. This is a plugin system just like our DB abstraction layer, which has served us very well. It is also analogous to the flexinode plugin system, which everyone seems to love. +1 ------------------------------------------------------------------------ January 31, 2005 - 10:08 : killes@www.drop.org I don't think having the .inc files as modules makes sense. ------------------------------------------------------------------------ January 31, 2005 - 10:08 : adrian I agree with you and dries on this one. This is kind of analogous to the file api, whereby this api just extends file support, for image filetypes. Any modules can then be built to use images , for instance the image.module which just becomes an image node type, and the flexinode image field type. Plus, having it in core and maintained as such is a big improvement over the current situation (re: the old image module) -- View: http://drupal.org/node/16358 Edit: http://drupal.org/project/comments/add/16358

Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: walkah Reported by: walkah Updated by: stefan nagtegaal Status: patch Okay, I tested the patch and it works like a charm! Still, i like to see some things changed: - You set the default thumbnail dimension to 100x100, set this to 180x120 which is also a 4:3 ratio (just like the suggested max image dimension 640x480); - In your descriptions you use 'WIDTHxHEIGHT', which is in no other core module used like this. IMO you should use lowercase characters and introduce separate textboxes for $width and $height, or something... I only could find these 2 nitpic, which is absolutely a very big +1 from my side. As a photographers point of view this is a very big step forward for drupal.. Thank you for all the work you put into this patch, I know you worked hard on this.. stefan nagtegaal Previous comments: ------------------------------------------------------------------------ January 28, 2005 - 21:31 : walkah Attachment: http://drupal.org/files/issues/image_0.inc (7.44 KB) Due to popular demand - attached here is an image.inc for inclusion (in includes/), that will offer some basic image detection and manipulation functionality to drupal's core. currently image.inc is responsible for managing image 'toolkits' - these are either PHP modules (e.g. GD) or external programs (e.g. imagemagick) that handle the actual manipulation of images. also included is the default "GD" toolkit (that supports both GD1 or GD2, depending on which is installed). PHP comes bundled with gd as of version 4.3 - therefore this is a reasonable default. However, other toolkits will follow (I presume) - they can be installed in includes/ (as image.$toolkit.inc). There are 5 main functions that modules may now utilize to handle images: * image_get_info() - this function checks a file - if it exists and is a valid image file , it will return an array containing things like the pixel dimensions of the image, plus the 'type' and common extension. * image_scale - resizes a given image to fit within a given width / height dimensions, while maintaining aspect ratio (not distorting the image). - this function can be used to generate thumbnails, or ensure a maximum resolution, etc. * image_resize - similar to image_scale (but will not respect aspect ratio - may well distort the image). * image_rotate - rotate an image by X degrees * image_crop - crops an image to a given rectangle (defined as top-left x/y coordinates plus a width & height of the rectangle). In a follow up I will post a patch that does the following: * adds some default settings to admin/settings : * toolkit selection (and possibly configuration) * default thumbnail dimensions (WxH) - to be used as a system-wide default * maximum dimensions (WxH ) - to be used as a system-wide default * changes the user picture handling to silently resize uploaded pictures to fit within the specified dimensions (rather than throwing errors). * uses image_get_info() to verify logo uploads in admin/theme configuration * patches upload.module to respect maximum dimensions for uploaded images. Contribution modules will now be able to rely on these base manipulation functions to offer additional functionality (such as image nodes, photo galleries, advanced image manipulation, etc) ------------------------------------------------------------------------ January 28, 2005 - 21:50 : walkah Attachment: http://drupal.org/files/issues/image-core.patch (7.22 KB) and the patch (as mentioned above). ------------------------------------------------------------------------ January 29, 2005 - 01:41 : rkendall Thanks for that, I think I could find that useful. Not meaning to criticize your very nice looking work, but I don't quite get the use of the /ext/ info in image_get_info() , it just seems to be duplicating information from the /type/ info. Thanks again, Ross. ------------------------------------------------------------------------ January 29, 2005 - 13:13 : walkah 'type' - is used primarily by the gd functions, which open image files using createFromPng / createFromJPEG / etc... and they use 'JPEG' not 'JPG' 'ext' - is the common file extension - i.e. 'JPG' not 'JPEG'. i agree, kind of a shame, but mostly harmless (imo) ------------------------------------------------------------------------ January 30, 2005 - 19:07 : Dries Please rename 'ext' to 'extension' and document the difference between 'type' and 'extension'. It is confusing at best. Personally, I prefer to use 'resolution' instead of 'res', and 'destination' instead of 'dest'. The punctuation in the PHPdoc code seems random: some sentences end with a point, others do not. Some code seems to wrap but that might be due to the fact that the image.inc file has been uploaded with the wrong MIME-type. Otherwise this patch looks good, but I haven't really tested it yet. ------------------------------------------------------------------------ January 31, 2005 - 15:44 : Bèr Kessels Is there a good reason for introducing YAPS (yet another plugin system) and not using tour well tested, optimised and wellknown .module system for the toolkits? Is there really a differerence in performance and complexity between: image.$toolkit.inc and image_gd.module? If not, please (with sugar on top and such), we should not make more/new plugin systems. Bèr ------------------------------------------------------------------------ January 31, 2005 - 15:45 : Bèr Kessels forgot: But other than that, great work, a big +1 from me to go into core. ------------------------------------------------------------------------ January 31, 2005 - 15:58 : walkah ber - re the toolkit "framework" vs. modules - the issue is that there is 0 overlap between toolkit functionality and the current set of module hooks. also, the point of image.inc is to encapsulate the toolkit functionality so that module developers, etc need not worry about which toolkit(s) are installed and available. i'm not sure, though, that toolkits really fit as modules. i.e. can you throttle a toolkit? is there any case where a toolkit could or should implement /any/ of the current module hooks? anyone else have some thoughts on this? I think polluting the module namespace with a ton of image support files would be worse than the 20 or so lines to implement this "custom plugin" framework. maybe i'm off base though. ------------------------------------------------------------------------ January 31, 2005 - 16:08 : moshe weitzman i agree that the toolkit framework proposed here is justified. This is a plugin system just like our DB abstraction layer, which has served us very well. It is also analogous to the flexinode plugin system, which everyone seems to love. +1 ------------------------------------------------------------------------ January 31, 2005 - 16:08 : killes@www.drop.org I don't think having the .inc files as modules makes sense. ------------------------------------------------------------------------ January 31, 2005 - 16:08 : adrian I agree with you and dries on this one. This is kind of analogous to the file api, whereby this api just extends file support, for image filetypes. Any modules can then be built to use images , for instance the image.module which just becomes an image node type, and the flexinode image field type. Plus, having it in core and maintained as such is a big improvement over the current situation (re: the old image module) ------------------------------------------------------------------------ January 31, 2005 - 16:41 : walkah Attachment: http://drupal.org/files/issues/image_1.inc (7.64 KB) attached is an updated image.inc - taking into consideration dries' suggestions. biggest is : removing 'type' from image_get_info - moving that to the GD functions where the jpg vs. JPEG distinction is required. also, tried to clean up some phpdoc comments, and use "destination" and "extension" instead of "dest" and "ext". (updated patch to follow). ------------------------------------------------------------------------ January 31, 2005 - 16:41 : walkah Attachment: http://drupal.org/files/issues/image-core2.patch (7.25 KB) here's the updated patch. -- View: http://drupal.org/node/16358 Edit: http://drupal.org/project/comments/add/16358

Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: walkah Reported by: walkah Updated by: Dries Status: patch I'm slightly unhappy with the new settings. They are inconsistent with the upload.module that does define its own settings (eg. 'maximum file size') rather than defining system-wide settings/defaults (eg. the proposed 'maximum image resolution'). To me, it makes sense to (i) group all file-related settings on the same page and (ii) to be consistent with regard to system-wide defaults. Dries Previous comments: ------------------------------------------------------------------------ January 28, 2005 - 22:31 : walkah Attachment: http://drupal.org/files/issues/image_0.inc (7.44 KB) Due to popular demand - attached here is an image.inc for inclusion (in includes/), that will offer some basic image detection and manipulation functionality to drupal's core. currently image.inc is responsible for managing image 'toolkits' - these are either PHP modules (e.g. GD) or external programs (e.g. imagemagick) that handle the actual manipulation of images. also included is the default "GD" toolkit (that supports both GD1 or GD2, depending on which is installed). PHP comes bundled with gd as of version 4.3 - therefore this is a reasonable default. However, other toolkits will follow (I presume) - they can be installed in includes/ (as image.$toolkit.inc). There are 5 main functions that modules may now utilize to handle images: * image_get_info() - this function checks a file - if it exists and is a valid image file , it will return an array containing things like the pixel dimensions of the image, plus the 'type' and common extension. * image_scale - resizes a given image to fit within a given width / height dimensions, while maintaining aspect ratio (not distorting the image). - this function can be used to generate thumbnails, or ensure a maximum resolution, etc. * image_resize - similar to image_scale (but will not respect aspect ratio - may well distort the image). * image_rotate - rotate an image by X degrees * image_crop - crops an image to a given rectangle (defined as top-left x/y coordinates plus a width & height of the rectangle). In a follow up I will post a patch that does the following: * adds some default settings to admin/settings : * toolkit selection (and possibly configuration) * default thumbnail dimensions (WxH) - to be used as a system-wide default * maximum dimensions (WxH ) - to be used as a system-wide default * changes the user picture handling to silently resize uploaded pictures to fit within the specified dimensions (rather than throwing errors). * uses image_get_info() to verify logo uploads in admin/theme configuration * patches upload.module to respect maximum dimensions for uploaded images. Contribution modules will now be able to rely on these base manipulation functions to offer additional functionality (such as image nodes, photo galleries, advanced image manipulation, etc) ------------------------------------------------------------------------ January 28, 2005 - 22:50 : walkah Attachment: http://drupal.org/files/issues/image-core.patch (7.22 KB) and the patch (as mentioned above). ------------------------------------------------------------------------ January 29, 2005 - 02:41 : rkendall Thanks for that, I think I could find that useful. Not meaning to criticize your very nice looking work, but I don't quite get the use of the /ext/ info in image_get_info() , it just seems to be duplicating information from the /type/ info. Thanks again, Ross. ------------------------------------------------------------------------ January 29, 2005 - 14:13 : walkah 'type' - is used primarily by the gd functions, which open image files using createFromPng / createFromJPEG / etc... and they use 'JPEG' not 'JPG' 'ext' - is the common file extension - i.e. 'JPG' not 'JPEG'. i agree, kind of a shame, but mostly harmless (imo) ------------------------------------------------------------------------ January 30, 2005 - 20:07 : Dries Please rename 'ext' to 'extension' and document the difference between 'type' and 'extension'. It is confusing at best. Personally, I prefer to use 'resolution' instead of 'res', and 'destination' instead of 'dest'. The punctuation in the PHPdoc code seems random: some sentences end with a point, others do not. Some code seems to wrap but that might be due to the fact that the image.inc file has been uploaded with the wrong MIME-type. Otherwise this patch looks good, but I haven't really tested it yet. ------------------------------------------------------------------------ January 31, 2005 - 16:44 : Bèr Kessels Is there a good reason for introducing YAPS (yet another plugin system) and not using tour well tested, optimised and wellknown .module system for the toolkits? Is there really a differerence in performance and complexity between: image.$toolkit.inc and image_gd.module? If not, please (with sugar on top and such), we should not make more/new plugin systems. Bèr ------------------------------------------------------------------------ January 31, 2005 - 16:45 : Bèr Kessels forgot: But other than that, great work, a big +1 from me to go into core. ------------------------------------------------------------------------ January 31, 2005 - 16:58 : walkah ber - re the toolkit "framework" vs. modules - the issue is that there is 0 overlap between toolkit functionality and the current set of module hooks. also, the point of image.inc is to encapsulate the toolkit functionality so that module developers, etc need not worry about which toolkit(s) are installed and available. i'm not sure, though, that toolkits really fit as modules. i.e. can you throttle a toolkit? is there any case where a toolkit could or should implement /any/ of the current module hooks? anyone else have some thoughts on this? I think polluting the module namespace with a ton of image support files would be worse than the 20 or so lines to implement this "custom plugin" framework. maybe i'm off base though. ------------------------------------------------------------------------ January 31, 2005 - 17:08 : moshe weitzman i agree that the toolkit framework proposed here is justified. This is a plugin system just like our DB abstraction layer, which has served us very well. It is also analogous to the flexinode plugin system, which everyone seems to love. +1 ------------------------------------------------------------------------ January 31, 2005 - 17:08 : killes@www.drop.org I don't think having the .inc files as modules makes sense. ------------------------------------------------------------------------ January 31, 2005 - 17:08 : adrian I agree with you and dries on this one. This is kind of analogous to the file api, whereby this api just extends file support, for image filetypes. Any modules can then be built to use images , for instance the image.module which just becomes an image node type, and the flexinode image field type. Plus, having it in core and maintained as such is a big improvement over the current situation (re: the old image module) ------------------------------------------------------------------------ January 31, 2005 - 17:41 : walkah Attachment: http://drupal.org/files/issues/image_1.inc (7.64 KB) attached is an updated image.inc - taking into consideration dries' suggestions. biggest is : removing 'type' from image_get_info - moving that to the GD functions where the jpg vs. JPEG distinction is required. also, tried to clean up some phpdoc comments, and use "destination" and "extension" instead of "dest" and "ext". (updated patch to follow). ------------------------------------------------------------------------ January 31, 2005 - 17:41 : walkah Attachment: http://drupal.org/files/issues/image-core2.patch (7.25 KB) here's the updated patch. ------------------------------------------------------------------------ January 31, 2005 - 18:08 : stefan nagtegaal Okay, I tested the patch and it works like a charm! Still, i like to see some things changed: - You set the default thumbnail dimension to 100x100, set this to 180x120 which is also a 4:3 ratio (just like the suggested max image dimension 640x480); - In your descriptions you use 'WIDTHxHEIGHT', which is in no other core module used like this. IMO you should use lowercase characters and introduce separate textboxes for $width and $height, or something... I only could find these 2 nitpic, which is absolutely a very big +1 from my side. As a photographers point of view this is a very big step forward for drupal.. Thank you for all the work you put into this patch, I know you worked hard on this.. ------------------------------------------------------------------------ January 31, 2005 - 19:02 : rkendall Comment on the default thumbnail resolution: I think 100x100 is fine, as far as I can tell it _does_ maintain aspect ratio. I don't think a thumbnail should presume that an image will be a particular aspect ratio, or that it will be landscape or portrait. If anything, I would say that the default 'image_max_resolution' should be something like 640x640! (as in, mixing 640x480 with 480x640 would be reasonable expectation). Anyway, people can set it to whatever they want. just my 0.2 cents. -- View: http://drupal.org/node/16358 Edit: http://drupal.org/project/comments/add/16358

Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: walkah Reported by: walkah Updated by: walkah Status: patch Attachment: http://drupal.org/files/issues/image_2.inc (7.65 KB) ok, one more try. Dries: this one removes the "troublesome" settings from admin/settings (thumbnail and max res) - therefore, when only the GD toolkit is avaiable, there are no other configuration options required. image_max_resolution has been changed to upload_max_resolution (and moved to the upload settings page). also, upload.module now provides feedback (via drupal_set_message) for when an image was resized. attached is a new image.inc with patch to follow. walkah Previous comments: ------------------------------------------------------------------------ January 28, 2005 - 15:31 : walkah Attachment: http://drupal.org/files/issues/image_0.inc (7.44 KB) Due to popular demand - attached here is an image.inc for inclusion (in includes/), that will offer some basic image detection and manipulation functionality to drupal's core. currently image.inc is responsible for managing image 'toolkits' - these are either PHP modules (e.g. GD) or external programs (e.g. imagemagick) that handle the actual manipulation of images. also included is the default "GD" toolkit (that supports both GD1 or GD2, depending on which is installed). PHP comes bundled with gd as of version 4.3 - therefore this is a reasonable default. However, other toolkits will follow (I presume) - they can be installed in includes/ (as image.$toolkit.inc). There are 5 main functions that modules may now utilize to handle images: * image_get_info() - this function checks a file - if it exists and is a valid image file , it will return an array containing things like the pixel dimensions of the image, plus the 'type' and common extension. * image_scale - resizes a given image to fit within a given width / height dimensions, while maintaining aspect ratio (not distorting the image). - this function can be used to generate thumbnails, or ensure a maximum resolution, etc. * image_resize - similar to image_scale (but will not respect aspect ratio - may well distort the image). * image_rotate - rotate an image by X degrees * image_crop - crops an image to a given rectangle (defined as top-left x/y coordinates plus a width & height of the rectangle). In a follow up I will post a patch that does the following: * adds some default settings to admin/settings : * toolkit selection (and possibly configuration) * default thumbnail dimensions (WxH) - to be used as a system-wide default * maximum dimensions (WxH ) - to be used as a system-wide default * changes the user picture handling to silently resize uploaded pictures to fit within the specified dimensions (rather than throwing errors). * uses image_get_info() to verify logo uploads in admin/theme configuration * patches upload.module to respect maximum dimensions for uploaded images. Contribution modules will now be able to rely on these base manipulation functions to offer additional functionality (such as image nodes, photo galleries, advanced image manipulation, etc) ------------------------------------------------------------------------ January 28, 2005 - 15:50 : walkah Attachment: http://drupal.org/files/issues/image-core.patch (7.22 KB) and the patch (as mentioned above). ------------------------------------------------------------------------ January 28, 2005 - 19:41 : rkendall Thanks for that, I think I could find that useful. Not meaning to criticize your very nice looking work, but I don't quite get the use of the /ext/ info in image_get_info() , it just seems to be duplicating information from the /type/ info. Thanks again, Ross. ------------------------------------------------------------------------ January 29, 2005 - 07:13 : walkah 'type' - is used primarily by the gd functions, which open image files using createFromPng / createFromJPEG / etc... and they use 'JPEG' not 'JPG' 'ext' - is the common file extension - i.e. 'JPG' not 'JPEG'. i agree, kind of a shame, but mostly harmless (imo) ------------------------------------------------------------------------ January 30, 2005 - 13:07 : Dries Please rename 'ext' to 'extension' and document the difference between 'type' and 'extension'. It is confusing at best. Personally, I prefer to use 'resolution' instead of 'res', and 'destination' instead of 'dest'. The punctuation in the PHPdoc code seems random: some sentences end with a point, others do not. Some code seems to wrap but that might be due to the fact that the image.inc file has been uploaded with the wrong MIME-type. Otherwise this patch looks good, but I haven't really tested it yet. ------------------------------------------------------------------------ January 31, 2005 - 09:44 : Bèr Kessels Is there a good reason for introducing YAPS (yet another plugin system) and not using tour well tested, optimised and wellknown .module system for the toolkits? Is there really a differerence in performance and complexity between: image.$toolkit.inc and image_gd.module? If not, please (with sugar on top and such), we should not make more/new plugin systems. Bèr ------------------------------------------------------------------------ January 31, 2005 - 09:45 : Bèr Kessels forgot: But other than that, great work, a big +1 from me to go into core. ------------------------------------------------------------------------ January 31, 2005 - 09:58 : walkah ber - re the toolkit "framework" vs. modules - the issue is that there is 0 overlap between toolkit functionality and the current set of module hooks. also, the point of image.inc is to encapsulate the toolkit functionality so that module developers, etc need not worry about which toolkit(s) are installed and available. i'm not sure, though, that toolkits really fit as modules. i.e. can you throttle a toolkit? is there any case where a toolkit could or should implement /any/ of the current module hooks? anyone else have some thoughts on this? I think polluting the module namespace with a ton of image support files would be worse than the 20 or so lines to implement this "custom plugin" framework. maybe i'm off base though. ------------------------------------------------------------------------ January 31, 2005 - 10:08 : moshe weitzman i agree that the toolkit framework proposed here is justified. This is a plugin system just like our DB abstraction layer, which has served us very well. It is also analogous to the flexinode plugin system, which everyone seems to love. +1 ------------------------------------------------------------------------ January 31, 2005 - 10:08 : killes@www.drop.org I don't think having the .inc files as modules makes sense. ------------------------------------------------------------------------ January 31, 2005 - 10:08 : adrian I agree with you and dries on this one. This is kind of analogous to the file api, whereby this api just extends file support, for image filetypes. Any modules can then be built to use images , for instance the image.module which just becomes an image node type, and the flexinode image field type. Plus, having it in core and maintained as such is a big improvement over the current situation (re: the old image module) ------------------------------------------------------------------------ January 31, 2005 - 10:41 : walkah Attachment: http://drupal.org/files/issues/image_1.inc (7.64 KB) attached is an updated image.inc - taking into consideration dries' suggestions. biggest is : removing 'type' from image_get_info - moving that to the GD functions where the jpg vs. JPEG distinction is required. also, tried to clean up some phpdoc comments, and use "destination" and "extension" instead of "dest" and "ext". (updated patch to follow). ------------------------------------------------------------------------ January 31, 2005 - 10:41 : walkah Attachment: http://drupal.org/files/issues/image-core2.patch (7.25 KB) here's the updated patch. ------------------------------------------------------------------------ January 31, 2005 - 11:08 : stefan nagtegaal Okay, I tested the patch and it works like a charm! Still, i like to see some things changed: - You set the default thumbnail dimension to 100x100, set this to 180x120 which is also a 4:3 ratio (just like the suggested max image dimension 640x480); - In your descriptions you use 'WIDTHxHEIGHT', which is in no other core module used like this. IMO you should use lowercase characters and introduce separate textboxes for $width and $height, or something... I only could find these 2 nitpic, which is absolutely a very big +1 from my side. As a photographers point of view this is a very big step forward for drupal.. Thank you for all the work you put into this patch, I know you worked hard on this.. ------------------------------------------------------------------------ January 31, 2005 - 12:02 : rkendall Comment on the default thumbnail resolution: I think 100x100 is fine, as far as I can tell it _does_ maintain aspect ratio. I don't think a thumbnail should presume that an image will be a particular aspect ratio, or that it will be landscape or portrait. If anything, I would say that the default 'image_max_resolution' should be something like 640x640! (as in, mixing 640x480 with 480x640 would be reasonable expectation). Anyway, people can set it to whatever they want. just my 0.2 cents. ------------------------------------------------------------------------ January 31, 2005 - 17:47 : Dries I'm slightly unhappy with the new settings. They are inconsistent with the upload.module that does define its own settings (eg. 'maximum file size') rather than defining system-wide settings/defaults (eg. the proposed 'maximum image resolution'). To me, it makes sense to (i) group all file-related settings on the same page and (ii) to be consistent with regard to system-wide defaults. ------------------------------------------------------------------------ January 31, 2005 - 18:47 : drumm I was just talking with Music for America's graphic designer (http://www.musicforamerica.org/user/8) and she mentioned that the new site design which they are working on will want a lot of uploaded images to be scaled and cropped. This new inc file looks like it will greatly reduce the effort needed to develop such functionality. I haven't tested this out by running it, but I definately like the features it provides to developers. -- View: http://drupal.org/node/16358 Edit: http://drupal.org/project/comments/add/16358

Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: walkah Reported by: walkah Updated by: Anonymous Status: patch Is it really neccessary to include the image.inc file for each page view? We already include too much stuff. Gerhard Anonymous Previous comments: ------------------------------------------------------------------------ January 28, 2005 - 20:31 : walkah Attachment: http://drupal.org/files/issues/image_0.inc (7.44 KB) Due to popular demand - attached here is an image.inc for inclusion (in includes/), that will offer some basic image detection and manipulation functionality to drupal's core. currently image.inc is responsible for managing image 'toolkits' - these are either PHP modules (e.g. GD) or external programs (e.g. imagemagick) that handle the actual manipulation of images. also included is the default "GD" toolkit (that supports both GD1 or GD2, depending on which is installed). PHP comes bundled with gd as of version 4.3 - therefore this is a reasonable default. However, other toolkits will follow (I presume) - they can be installed in includes/ (as image.$toolkit.inc). There are 5 main functions that modules may now utilize to handle images: * image_get_info() - this function checks a file - if it exists and is a valid image file , it will return an array containing things like the pixel dimensions of the image, plus the 'type' and common extension. * image_scale - resizes a given image to fit within a given width / height dimensions, while maintaining aspect ratio (not distorting the image). - this function can be used to generate thumbnails, or ensure a maximum resolution, etc. * image_resize - similar to image_scale (but will not respect aspect ratio - may well distort the image). * image_rotate - rotate an image by X degrees * image_crop - crops an image to a given rectangle (defined as top-left x/y coordinates plus a width & height of the rectangle). In a follow up I will post a patch that does the following: * adds some default settings to admin/settings : * toolkit selection (and possibly configuration) * default thumbnail dimensions (WxH) - to be used as a system-wide default * maximum dimensions (WxH ) - to be used as a system-wide default * changes the user picture handling to silently resize uploaded pictures to fit within the specified dimensions (rather than throwing errors). * uses image_get_info() to verify logo uploads in admin/theme configuration * patches upload.module to respect maximum dimensions for uploaded images. Contribution modules will now be able to rely on these base manipulation functions to offer additional functionality (such as image nodes, photo galleries, advanced image manipulation, etc) ------------------------------------------------------------------------ January 28, 2005 - 20:50 : walkah Attachment: http://drupal.org/files/issues/image-core.patch (7.22 KB) and the patch (as mentioned above). ------------------------------------------------------------------------ January 29, 2005 - 00:41 : rkendall Thanks for that, I think I could find that useful. Not meaning to criticize your very nice looking work, but I don't quite get the use of the /ext/ info in image_get_info() , it just seems to be duplicating information from the /type/ info. Thanks again, Ross. ------------------------------------------------------------------------ January 29, 2005 - 12:13 : walkah 'type' - is used primarily by the gd functions, which open image files using createFromPng / createFromJPEG / etc... and they use 'JPEG' not 'JPG' 'ext' - is the common file extension - i.e. 'JPG' not 'JPEG'. i agree, kind of a shame, but mostly harmless (imo) ------------------------------------------------------------------------ January 30, 2005 - 18:07 : Dries Please rename 'ext' to 'extension' and document the difference between 'type' and 'extension'. It is confusing at best. Personally, I prefer to use 'resolution' instead of 'res', and 'destination' instead of 'dest'. The punctuation in the PHPdoc code seems random: some sentences end with a point, others do not. Some code seems to wrap but that might be due to the fact that the image.inc file has been uploaded with the wrong MIME-type. Otherwise this patch looks good, but I haven't really tested it yet. ------------------------------------------------------------------------ January 31, 2005 - 14:44 : Bèr Kessels Is there a good reason for introducing YAPS (yet another plugin system) and not using tour well tested, optimised and wellknown .module system for the toolkits? Is there really a differerence in performance and complexity between: image.$toolkit.inc and image_gd.module? If not, please (with sugar on top and such), we should not make more/new plugin systems. Bèr ------------------------------------------------------------------------ January 31, 2005 - 14:45 : Bèr Kessels forgot: But other than that, great work, a big +1 from me to go into core. ------------------------------------------------------------------------ January 31, 2005 - 14:58 : walkah ber - re the toolkit "framework" vs. modules - the issue is that there is 0 overlap between toolkit functionality and the current set of module hooks. also, the point of image.inc is to encapsulate the toolkit functionality so that module developers, etc need not worry about which toolkit(s) are installed and available. i'm not sure, though, that toolkits really fit as modules. i.e. can you throttle a toolkit? is there any case where a toolkit could or should implement /any/ of the current module hooks? anyone else have some thoughts on this? I think polluting the module namespace with a ton of image support files would be worse than the 20 or so lines to implement this "custom plugin" framework. maybe i'm off base though. ------------------------------------------------------------------------ January 31, 2005 - 15:08 : moshe weitzman i agree that the toolkit framework proposed here is justified. This is a plugin system just like our DB abstraction layer, which has served us very well. It is also analogous to the flexinode plugin system, which everyone seems to love. +1 ------------------------------------------------------------------------ January 31, 2005 - 15:08 : killes@www.drop.org I don't think having the .inc files as modules makes sense. ------------------------------------------------------------------------ January 31, 2005 - 15:08 : adrian I agree with you and dries on this one. This is kind of analogous to the file api, whereby this api just extends file support, for image filetypes. Any modules can then be built to use images , for instance the image.module which just becomes an image node type, and the flexinode image field type. Plus, having it in core and maintained as such is a big improvement over the current situation (re: the old image module) ------------------------------------------------------------------------ January 31, 2005 - 15:41 : walkah Attachment: http://drupal.org/files/issues/image_1.inc (7.64 KB) attached is an updated image.inc - taking into consideration dries' suggestions. biggest is : removing 'type' from image_get_info - moving that to the GD functions where the jpg vs. JPEG distinction is required. also, tried to clean up some phpdoc comments, and use "destination" and "extension" instead of "dest" and "ext". (updated patch to follow). ------------------------------------------------------------------------ January 31, 2005 - 15:41 : walkah Attachment: http://drupal.org/files/issues/image-core2.patch (7.25 KB) here's the updated patch. ------------------------------------------------------------------------ January 31, 2005 - 16:08 : stefan nagtegaal Okay, I tested the patch and it works like a charm! Still, i like to see some things changed: - You set the default thumbnail dimension to 100x100, set this to 180x120 which is also a 4:3 ratio (just like the suggested max image dimension 640x480); - In your descriptions you use 'WIDTHxHEIGHT', which is in no other core module used like this. IMO you should use lowercase characters and introduce separate textboxes for $width and $height, or something... I only could find these 2 nitpic, which is absolutely a very big +1 from my side. As a photographers point of view this is a very big step forward for drupal.. Thank you for all the work you put into this patch, I know you worked hard on this.. ------------------------------------------------------------------------ January 31, 2005 - 17:02 : rkendall Comment on the default thumbnail resolution: I think 100x100 is fine, as far as I can tell it _does_ maintain aspect ratio. I don't think a thumbnail should presume that an image will be a particular aspect ratio, or that it will be landscape or portrait. If anything, I would say that the default 'image_max_resolution' should be something like 640x640! (as in, mixing 640x480 with 480x640 would be reasonable expectation). Anyway, people can set it to whatever they want. just my 0.2 cents. ------------------------------------------------------------------------ January 31, 2005 - 22:47 : Dries I'm slightly unhappy with the new settings. They are inconsistent with the upload.module that does define its own settings (eg. 'maximum file size') rather than defining system-wide settings/defaults (eg. the proposed 'maximum image resolution'). To me, it makes sense to (i) group all file-related settings on the same page and (ii) to be consistent with regard to system-wide defaults. ------------------------------------------------------------------------ January 31, 2005 - 23:47 : drumm I was just talking with Music for America's graphic designer (http://www.musicforamerica.org/user/8) and she mentioned that the new site design which they are working on will want a lot of uploaded images to be scaled and cropped. This new inc file looks like it will greatly reduce the effort needed to develop such functionality. I haven't tested this out by running it, but I definately like the features it provides to developers. ------------------------------------------------------------------------ February 1, 2005 - 03:01 : walkah Attachment: http://drupal.org/files/issues/image_2.inc (7.65 KB) ok, one more try. Dries: this one removes the "troublesome" settings from admin/settings (thumbnail and max res) - therefore, when only the GD toolkit is avaiable, there are no other configuration options required. image_max_resolution has been changed to upload_max_resolution (and moved to the upload settings page). also, upload.module now provides feedback (via drupal_set_message) for when an image was resized. attached is a new image.inc with patch to follow. ------------------------------------------------------------------------ February 1, 2005 - 03:01 : walkah Attachment: http://drupal.org/files/issues/image-core3.patch (7.6 KB) said patch. -- View: http://drupal.org/node/16358 Edit: http://drupal.org/project/comments/add/16358