Currently if a user uploads a transparent gif, when Drupal resizes it, it makes all those transparent spots black, which is not desirably at all. You can imagine the users frustration. This problem has bugging me for a long time but I finally came up with a solution that is elegant and fixes it in core, where the root of the problem lies.

See the before and after screenshots for this patch in action. Test it with transparent gifs, adding them to your user profile as your user image.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

m3avrck’s picture

This should be backported to Drupal 5 as well.

m3avrck’s picture

This should be backported to Drupal 5 as wel.

quicksketch’s picture

Status: Needs review » Needs work
FileSize
2.99 KB
21.17 KB

I'd *love* to get this fixed! Pngs have worked fine for some time because of the special case made for them when transparency exists. We should do the same for gifs.

However, this patch produced some unexpected behavior and I couldn't get it to work with the images I was trying. The most peculiar was when uploading an image with an index color marked as transparent that wasn't black, such as the drupalicon3.gif attached shows up with no transparency and white spaces filled with black.

m3avrck’s picture

Status: Needs work » Needs review
FileSize
1.77 KB

Updated patch now works with both types of transparent gifs.

quicksketch’s picture

FileSize
3.43 KB
61.02 KB

This patch definitely makes headway. I'm now getting mostly transparent gifs but looks like the resizing is creating some distortion in the image somehow. Uploading a transparent GIF ends up with most of the originally transparent area trasparent but some other places filled with black. See the attached images.

Minor note: we should use elseif instead of else if.

m3avrck’s picture

FileSize
1.77 KB
39.48 KB

Hmm, you sure you applied the correct patch? I just tried it with your image and got the following (see attached). Seems to be working well over here.

Info...

PHP 5.2.0
GD 2.0.28

Updated coding style patch.

quicksketch’s picture

Status: Needs review » Needs work

I tried using the patch in #6 on a fresh D6, the problem is still occurring.

GD bundled (2.0.34 compatible)
PHP 5.2.3

Curiously, I got the gif-drupalicon3.gif to resize without the distortion if I set the user size dimensions to 200x200 (exactly 50% of the original). The default 85x85, as well as 100x100 and 300x300 all result in the distortion problem.

I haven't been able to make the flower image to resize without the unexpected black dots.

m3avrck’s picture

Ok here's an updated patch which fixes those issues.

After some research, it turns out those odd colors were showing up because of mixing TrueColor GIFs and regular palette ones and then trying to draw a transparent background.

The magic line is:

     // Convert from true color to palette to fix transparency issues.
     imagetruecolortopalette($res, TRUE, $number_colors);

This seems to solve those issues. However, some resized GIFs look a bit granulated because of it. There is no way around this unfort, based on the current GD library.

However, this is far better than a really bad black GIF image and is quite tolerable. I'm just being picky designer here :)

m3avrck’s picture

It seems attachments aren't working on Drupal right now, here is the patch, make yer own ;-)

Index: image.gd.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/image.gd.inc,v
retrieving revision 1.2
diff -u -F^f -r1.2 image.gd.inc
--- image.gd.inc	7 Aug 2007 08:39:35 -0000	1.2
+++ image.gd.inc	14 Dec 2007 22:37:00 -0000
@@ -80,7 +80,7 @@ function image_gd_resize($source, $desti
     return FALSE;
   }
 
-  $res = imageCreateTrueColor($width, $height);
+  $res = imagecreatetruecolor($width, $height);
   if ($info['extension'] == 'png') {
     $transparency = imagecolorallocatealpha($res, 0, 0, 0, 127);
     imagealphablending($res, FALSE);
@@ -88,11 +88,29 @@ function image_gd_resize($source, $desti
     imagealphablending($res, TRUE);
     imagesavealpha($res, TRUE);
   }
-  imageCopyResampled($res, $im, 0, 0, 0, 0, $width, $height, $info['width'], $info['height']);
+  elseif ($info['extension'] == 'gif') {
+    // If we have a specific transparent color.
+    $transparency_index = imagecolortransparent($im);
+    if ($transparency_index >= 0) {
+      // Get the original image's transparent color's RGB values.
+      $transparent_color = imagecolorsforindex($im, $transparency_index);
+      // Allocate the same color in the new image resource.
+      $transparency_index = imagecolorallocate($res, $transparent_color['red'], $transparent_color['green'], $transparent_color['blue']);
+      // Completely fill the background of the new image with allocated color.
+      imagefill($res, 0, 0, $transparency_index);
+      // Set the background color for new image to transparent.
+      imagecolortransparent($res, $transparency_index);
+      // Find number of colors in the images palette.
+      $number_colors = imagecolorstotal($im);
+      // Convert from true color to palette to fix transparency issues.
+      imagetruecolortopalette($res, TRUE, $number_colors);
+    }
+  }
+  imagecopyresampled($res, $im, 0, 0, 0, 0, $width, $height, $info['width'], $info['height']);
   $result = image_gd_close($res, $destination, $info['extension']);
 
-  imageDestroy($res);
-  imageDestroy($im);
+  imagedestroy($res);
+  imagedestroy($im);
 
   return $result;
 }
m3avrck’s picture

Status: Needs work » Needs review
quicksketch’s picture

FileSize
2.04 KB

Awesome! This last patch works great with all the images I tested. This is a great fix that is long overdue for proper GIF scaling.

I rolled an actual patch which should be the same as the one posted above. m3avrck could you confirm I didn't make any mistakes and RTBC?

m3avrck’s picture

Status: Needs review » Reviewed & tested by the community

Confirmed, glad to see attachments are working again, thanks for creating that file!

This should be applied to 5.x after 6.x too.

Gábor Hojtsy’s picture

Version: 6.x-dev » 5.x-dev

Committed to 6.x, thanks.

quicksketch’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
2.04 KB

Same patch for Drupal 5's image.inc file, before it was split into image.inc and image.gd.inc. Tested and confirmed the same fix works perfectly in Drupal 5. Could we get another RTBC?

m3avrck’s picture

Status: Needs review » Reviewed & tested by the community

Yup looks good. Patch is exactly the same as for 6.x but just a different file. Works the same so we're good to go, rock!

mokargas’s picture

I'm trying to apply this patch to 5.1, image.inc and get this message

missing header for unified diff at line 18 of patch
can't find file to patch at input line 18
Perhaps you used the wrong -p or --strip option?

I'm not terribly clear on patch for windows, so can someone shed light on why this is occuring?

drumm’s picture

Status: Reviewed & tested by the community » Fixed

COmmitted to 5.x.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.