Posted by drewish on April 18, 2009 at 5:50pm
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | image system |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Issue Summary
I'd backported the image api changes that were committed to core in #373613: Create "Image" Objects, Operate on Images By Resource to the ImageAPI module. Over in #422836-6: ImageAPI GD2 6.x-1.5 ignores crop background color setting evolvingweb pointed out that you can use the crop to enlarge an image and expose the background which would default to black. We need to add a $background parameter to the image_crop() function to allow the user to specify the color and match up with image_rotate().
Comments
#1
This is a theme dependant color imo, and should be used like that.
If you want to do this right, the only thing you could do is to re-generate the image and have a transparent background which needs:
- your image to be converted to png to allow transluscency (gif isn't reliable when rotating your image if != 90, 180, 270 °C)
Would it make sense to make a theme dependant constant like:
<?phpdefine theme('IMAGE_BACKGROUND_COLOR', 'transparent');
?>
I'm not sure about this and we need to reach a concensus about this before I am going to start coding on this, which is pretty straight forward.
#2
i think this should be the right status for this...
#3
I disagree, I think it's a preset dependent setting. Some of my presets I want to have a white background, others gray and others transparent. I shouldn't have to pick one for the entire site.
#4
After spending a little time working on a patch for this I'm understanding what you're saying. But I think we actually need to take a slightly different tactic and specify color and transparency separately, e.g.
<?phpfunction image_crop(stdClass $image, $x, $y, $width, $height) {
?>
becomes:
<?phpfunction image_crop(stdClass $image, $x, $y, $width, $height, $transparent = TRUE, $background = 0xffffff) {
?>
and
<?phpfunction image_rotate(stdClass $image, $degrees, $background = NULL) {
?>
becomes:
<?phpfunction image_rotate(stdClass $image, $degrees, $transparent = TRUE, $background = 0xffffff) {
?>
This way the background color can be specified without needing to be concerned about the file format. If it supports transparency then you get transparent backgrounds and if not you get the specified color. Update: or in the case of GIF files, you can specify which color will be used for transparency if one isn't already in use.
#5
I spent some time trying to figure this out but imagecopyresampled was just too much of a beast.
Fortunately, the fantastic module Imagecache Actions provides a few additional actions that do what I need here.
Specifically, the "Define Canvas" action is just a negative crop with a color picker.
See http://drupal.org/project/imagecache_actions
Perhaps at least some of those actions should follow Imagecache into core?
#6
Man this code is really tricky to get right. Here's my broken but in progress patch that at least works for rotation. Cropping still needs a ton of work.
The code is borked enough that the unit tests aren't helpful since you can't see what's wrong. So I've been using this snip it in the PHP execute block to test it:
<?php$bg = 0xff00ff;
foreach (array(true =>'transparent', false => 'solid') as $transparent => $type) {
$img = "$type <br>";
foreach (array('jpg', 'png', 'gif') as $ext) {
#dsm($ext);
$input = 'sites/default/files/simpletest/image-test.' . $ext;
$output = 'sites/default/files/image-test.' . $ext;
$image = image_load($input, 'gd');
image_crop($image, -20, -10, 80, 40, $bg, $transparent);
# image_gd_rotate($image, 30, $bg, $transparent);
image_save($image, $output);
$img .= "<span style='background-color: #ccc'> $ext:" . theme('image', file_create_path($output)) ."</span>";
}
drupal_set_message($img);
}
?>
#7
Very interested in this issue as I am experiencing the black background behavior in D5 with version 1.5, not with 1.4.
#8
Any reason why you are not using imageapi_hex2rgba() ?
<?php+ $rgb = array();
+ for ($i = 16; $i >= 0; $i -= 8) {
+ $rgb[] = (($background >> $i) & 0xFF);
}
?>
This triggered me to write up my suggestion for improvements to color passing over at : #471816: API suggestion - use keyed color names, not ints in imageapi_hex2rgba()
#9
well imageapi_hex2rgba() isn't in core and i benchmarked it and the for loop is actually slower than just doing it in an array.
#10
#11
subscribe