Opened 3 years ago

Last modified 5 months ago

#13461 new defect (bug)

Preserve GIF transparency/alpha during thumbnail creation

Reported by: javitxu123 Owned by:
Priority: normal Milestone: Future Release
Component: Media Version: 2.9.2
Severity: normal Keywords: needs-testing needs-patch 2nd-opinion
Cc: marko@…, bpetty

Description

GIF images with transparent backgrounds get thumbnails with black backgrounds.

It was a similar ticket for PNG images in: http://core.trac.wordpress.org/ticket/2805

Attachments (9)

13461.patch (1.1 KB) - added by SergeyBiryukov 2 years ago.
13461.2.patch (2.4 KB) - added by SergeyBiryukov 11 months ago.
xparent.gif (61.0 KB) - added by SergeyBiryukov 11 months ago.
13461.3.patch (2.5 KB) - added by SergeyBiryukov 10 months ago.
Minor cleanup
13461.4.patch (1.4 KB) - added by SergeyBiryukov 10 months ago.
xparent-resized.png (107.1 KB) - added by bpetty 7 months ago.
xparent-blue.gif (58.0 KB) - added by bpetty 7 months ago.
13461.5.patch (1.0 KB) - added by bpetty 7 months ago.
xparent-gimp.gif (47.2 KB) - added by bpetty 7 months ago.

Download all attachments as: .zip

Change History (28)

comment:1   dd323 years ago

  • Component changed from Post Thumbnails to Media
  • Keywords needs-patch added
  • Milestone changed from Unassigned to Future Release
  • Keywords gsoc added

Here's a little patch that helps at least keeping the appearence in most cases. It doesn't preserve the tranparency though but sets the black background in the image to transparent just before the thumbnail is created.

/wp-includes/media.php line 422:

$newimage = wp_imagecreatetruecolor( $dst_w, $dst_h );

if(IMAGETYPE_GIF == $orig_type){	
	$black=imagecolorallocate($image, 0,0,0);
	imagecolortransparent($image, $black);
}

imagecopyresampled( $newimage, $image, $dst_x, $dst_y, $src_x, $src_y, $dst_w, $dst_h, $src_w, $src_h);
  • Keywords has-patch needs-testing added; needs-patch gsoc removed

Added patch for preserving transparency.

  • Cc marko@… added

What is the status of this ticket? I did apply the patch at one site and it seems to work for the few images that it went wrong.

  • Milestone changed from Future Release to 3.4

Moving for review.

  • Keywords 3.5-early added
  • Milestone changed from 3.4 to Future Release

comment:10 follow-up: ↓ 11   markoheijnen11 months ago

I do wonder if the code should be inside wp_imagecreatetruecolor() since there it goes wrong

comment:11 in reply to: ↑ 10   SergeyBiryukov11 months ago

  • Milestone changed from Future Release to 3.5

Replying to markoheijnen:

I do wonder if the code should be inside wp_imagecreatetruecolor() since there it goes wrong

13461.2.patch moves the logic to wp_imagecreatetruecolor().

xparent.gif is the image I tested the patch with.

comment:12 follow-up: ↓ 13   n3k411 months ago

I've used this before to preserve transparencies for .png and .gif;

 if($type == "gif" or $type == "png"){
    imagecolortransparent($new, imagecolorallocatealpha($new, 0, 0, 0, 127));
    imagealphablending($new, false);
    imagesavealpha($new, true);
  }

Minor cleanup

comment:13 in reply to: ↑ 12   SergeyBiryukov10 months ago

Replying to n3k4:

I've used this before to preserve transparencies for .png and .gif;

Thanks for the hint, that's much simpler and seemingly works as well.

Not sure if the new function_exists() checks are necessary, since imagecreatetruecolor() call doesn't have one, but added them just in case.

bpetty7 months ago

bpetty7 months ago

bpetty7 months ago

  • Cc bpetty added
  • Keywords needs-patch added; has-patch removed

Just to clarify since this has changed in SVN trunk since the last update here (and latest patch), I am using the latest SVN trunk with the new WP_Image_Editor, and I have disabled the imagick editor (filtering "wp_editors" - which might be renamed "wp_image_editors" soon - see ticket:6821#comment:90) so it falls back on GD - which is what this patch is intended for.

I can't get (an un-touched copy of) xparent.gif to show transparency after being resized, cropped, flipped, or rotated using 13461.4.patch. It continues to use a white background just like SVN trunk. This is while using PHP 5.3.10, libgd 2.0.36 (not bundled, which is the default for Ubuntu 12.04).

However, I was able to take the same GIF image, make some minor modifications in GIMP (see xparent-gimp.gif - the darker levels are intentional), and get transparency to work using 13461.4.patch, but it is very temperamental, and can result in very negative results. See xparent-resized.png.

I have refreshed the latest patch anyway for others wanting to re-test on latest SVN trunk: 13461.5.patch

For what it's worth, WordPress does currently use the same color used for transparent values in GIF images - which is the appropriate decision when transparency can not be preserved. This is why the GIF image attached here uses white while some others use black. It's also possible that some rare GIF images use a different color altogether (try using xparent-blue.gif). If the image uses a black background where it shouldn't, it's because the GIF image was not ideally created with this in mind. If we can't find a way to perfectly handle GIF image transparency, this behavior should remain. We shouldn't use any approach like comment:3 that arbitrarily chooses a color to use for transparency, or overrides the color values saved in the GIF in transparent locations. I do still plan on taking another shot at a better patch though still.

bpetty7 months ago

I should also mention that when using the new ImageMagick editor (that is used by default if it is installed now), every last one of the GIF images mentioned in this ticket work perfectly with transparency without any weird side-effects. So while GD may not work well with this, it won't be a problem any longer on most hosting providers with WordPress 3.5 as it stands already since they will likely end up using ImageMagick anyway.

  • Keywords 2nd-opinion added; 3.5-early removed
  • Milestone changed from 3.5 to Future Release

This is fixed by ImageMagick. GD is not easy to fix. Punting this for some additional opinions/patches. If we can't fix this sanely with GD, we can call IM good enough.

Just some additional notes based on my own findings...

While I was able to modify wp_imagecreatetruecolor() in a way that gets this working well with resizing and cropping, this isn't enough.

  • The same approach can not be used with the GD imagerotate() method which throws away transparency info in the image in it's own way too, so it needs it's own patch.
  • Also, image flipping is in it's own boat too where the same tricks applied to get resize/crop to work with GIF transparency actually completely break image flipping to where the whole image is lost.

I'll attach my semi-working patch (resize/crop working, but rotate/flip broken) later here for anyone that wants to take over.

Can you still add the patch you had. I'm thinking of fixing it for resize/crop only. For me that is enough to fix this ticket and if people have issues that they then need to install IM or GM

I still have the changes I was making saved off here:
https://github.com/tierra/wordpress/compare/master...ticket-13461-custom

(add .diff to that URL for a raw patch)

Note that two different approaches are there between commits, so you might want to look at that first commit on it's own too. It's all rough code since I couldn't get this working as well as I had hoped, but also includes some CSS changes that makes testing much easier.

Note: See TracTickets for help on using tickets.