WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 2 months ago

#39216 new defect (bug)

PDFs with non-opaque alpha channels can result in previews with black backgrounds.

Reported by: gitlost Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.7
Component: Media Keywords: dev-feedback 2nd-opinion has-patch has-unit-tests
Focuses: Cc:
PR Number:

Description

If a PDF has a non-opaque alpha channel it can result in a black background when rendering the preview. Attached is a real-world example and also a simple blank(ish) test pdf.

One way around this is to use the Imagick::ALPHACHANNEL_REMOVE mode of setImageAlphaChannel(), as in the attached patch, added as an extra pdf_process() method in view of #38832. The unittest requires the blank test pdf to be uploaded to the test data images directory.

The blank test pdf was generated using TCPDF (https://tcpdf.org):

<?php
$dirname = dirname( __FILE__ );
$tcpdf_dirname = $dirname . '/tcpdf';
require $tcpdf_dirname . '/tcpdf.php';

$pdf = new TCPDF();
$pdf->AddPage();
$pdf->SetAlpha(0);
$pdf->Output( $dirname . '/test_alpha.pdf', 'F' );

Attachments (7)

contents-hs-issue-41.pdf (257.9 KB) - added by gitlost 3 years ago.
Real-world example.
test_alpha.pdf (7.0 KB) - added by gitlost 3 years ago.
Simple blank test pdf.
39216.patch (3.7 KB) - added by gitlost 3 years ago.
Patch to remove alpha channel. Unittest requires blank test pdf.
test_cmyk.pdf (7.4 KB) - added by gitlost 3 years ago.
Test CMYK PDF.
39216.2.patch (4.9 KB) - added by gitlost 3 years ago.
Refresh of patch with unsharpMaskImage not called for CMYK.
39216.3.patch (2.9 KB) - added by gitlost 3 years ago.
Refresh of original patch without the unsharpMaskImage() change.
39216.4.patch (2.9 KB) - added by gitlost 3 years ago.
Refresh, comment changes only.

Download all attachments as: .zip

Change History (20)

@gitlost
3 years ago

Real-world example.

@gitlost
3 years ago

Simple blank test pdf.

@gitlost
3 years ago

Patch to remove alpha channel. Unittest requires blank test pdf.

#1 @gitlost
3 years ago

There's also an issue with PDFs with CMYK color spaces, as the resulting "full" jpegs will also have CMYK color spaces, and it turns out that Imagick::unsharpMaskImage() (as called in WP_Image_Editor_Imagick::thumbnail_image()) messes them up. In particular black text becomes fuzzy and over-black, and colors change. This can be seen using the attached "test_cmyk.pdf", generated using TCPDF:

<?php
$dirname = dirname( __FILE__ );
$tcpdf_dirname = $dirname . '/tcpdf';
require $tcpdf_dirname . '/tcpdf.php';

$pdf = new TCPDF();
$pdf->AddSpotColor( 'spot_black', 0, 0, 0, 100 );
$pdf->AddSpotColor( 'spot_dark_blue', 89, 42, 0, 48 );
$pdf->AddPage();
$pdf->SetFontSize( 30 );
$pdf->SetTextSpotColor( 'spot_black' );
$pdf->Text( 20, 20, 'Spot Black' );
$pdf->SetFillSpotColor( 'spot_dark_blue' );
$pdf->Rect( 20, 40, 30, 30, 'DF' );
$pdf->SetFontSize( 18 );
$pdf->SetTextSpotColor( 'spot_dark_blue' );
$pdf->Text( 20, 70, 'Dark Blue' );
$pdf->Output( $dirname . '/test_cmyk.pdf', 'F' );

Not calling Imagick::unsharpMaskImage() if the color space is CMYK fixes the fuzzy black text and severe color change issues, but still results in noticeable color change. This could be ameliorated by adding ICC profiles in the proposed pdf_process() method, but it's messy, and anyway I now think that a much better solution to all these issues is to use GhostScript directly - see #39262.

FWIW the following is a refreshed patch not calling Imagick::unsharpMaskImage() if the color space is CMYK. The unit test requires "test_cmyk.pdf" to be uploaded to the test data images directory.

@gitlost
3 years ago

Test CMYK PDF.

@gitlost
3 years ago

Refresh of patch with unsharpMaskImage not called for CMYK.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


3 years ago

#3 @joemcgill
3 years ago

  • Keywords dev-feedback 2nd-opinion has-patch has-unit-tests added

Hi @gitlost,

Thanks for digging into this. I'm hoping to get feedback from @mikeschroder and/or @markoheijnen on the approach, but in the mean time I have an initial question. Unless I'm mistaken, it seems to me that the CMYK issue is distinct from the alpha channel issue. Is that correct? If so, we should probably separate that issue into a separate ticket.

This ticket was mentioned in Slack in #core-media by mike. View the logs.


3 years ago

#5 @gitlost
3 years ago

Hi, thanks for responding. Yes you're right the Imagick::unsharpMaskImage() CMYK issue is distinct from the alpha channel issue, as it applies to any jpeg with a CMYK color space, so I'll split it off into a new ticket and refresh the patch here without it. The noticeable color change for CMYK PDFs issue still remains though.

Was messing about with ImageMagick's "delegate.xml" ("/etc/ImageMagick-6/delegate.xml" on Ubuntu 16.10) and found that if on the decode="ps:cmyk" line you change -sDEVICE=pam to -sDEVICE=jpeg then both the CMYK and the alpha issues disappear and you get perfect previews.

Similarly if on the decode="ps:alpha line you change -sDEVICE=pngalpha to -sDEVICE=jpeg and add -dUseCIEColor then the alpha channel issue disappears for the "test_alpha.pdf" uploaded here.

So there may be a way through Imagick to get the behaviour from ImageMagick that one gets by calling GhostScript directly (where none of these issues arises), but it's not easy to see. It's a bit like fishing through a keyhole.

@gitlost
3 years ago

Refresh of original patch without the unsharpMaskImage() change.

#6 @launchinteractive
3 years ago

I've been getting the black issue and this patch fixes it for me. Is there a reason why it hasn't been added to WordPress core?

@gitlost
3 years ago

Refresh, comment changes only.

#7 @gitlost
3 years ago

On investigating #40537, found out this only affects systems with Ghostscript versions >= 9.14, probably due to the change in color management https://www.ghostscript.com/doc/9.21/History9.htm#Version9.14.

The new patch is just a refresh with comment changes only. It doesn't try to determine the Ghostscript version (and act accordingly) as it's messy to do in a system-independent way (and would require an exec()).

(Pssttt! For all your PDF preview needs - at very competitive prices - see https://wordpress.org/plugins/gs-only-pdf-preview/.)

#8 @emirpprime
11 months ago

We've just upgraded ~20 servers to Debian 9 from Deb7/8 and been bitten by this. Unfortunately in all the testing no-one happened to upload a PDF with transparency in it.

So far, modifying the decode="ps:alpha line in /etc/ImageMagick-6/delegates.xml from -sDEVICE=pngalpha to -sDEVICE=jpeg has fixed it.


Original error shown in wp-admin (when using Regenerate Thumbnails plugin, otherwise only error is black images):

ERROR: FailedToExecuteCommand `'gs' -sstdout=%stderr -dQUIET -dSAFER -dBATCH -dNOPAUSE -dNOPROMPT -dMaxBitmap=500000000 -dAlignToPixels=0 -dGridFitTT=2 '-sDEVICE=pngalpha' -dTextAlphaBits=4 -dGraphicsAlphaBits=4 '-r128x128' -dFirstPage=1 -dLastPage=1 '-sOutputFile=/tmp/magick-7338Pz_rSv55v5nG%d' '-f/tmp/magick-7338lWRCG1kb4w9Z' '-f/tmp/magick-7338WA7HNhRpf3Uj (-1) @ error/delegate.c/ExternalDelegateCommand/462

System info:

Debian 9, PHP7.2, Apache 2.4

Media Debugging Info:

Active Editor	WP_Image_Editor_Imagick
Imagick Module Number	1687
ImageMagick Version	ImageMagick 6.9.7-4 Q16 x86_64 20170114 http://www.imagemagick.org
GD Version	2.2.5
Ghostscript Version	9.26
Memory Limit	256M
Max Execution Time	30
Max Input Time	60
Upload Max Filesize	20M
Post Max Size	20MImagick Resource Limits
Imagick area limit	122 MB
Imagick disk limit	1073741824
Imagick file limit	6144
Imagick map limit	512 MB
Imagick memory limit	256 MB
Imagick thread limit	2

#9 follow-ups: @mwtsn
10 months ago

Hi all,

I've been faced with the black PDF thumbnail image issue, and I've come up with a solution. Mine is specific to Imagick.

In the file wp-includes/class-wp-image-editor-imagick.php after line 153 ($this->image->readImage( $filename );) you can put in the code:

$this->image->setImageBackgroundColor('white');
$this->image = $this->image->flattenImages();

This fixes my issue. So I was wondering if it would be useful for us to add an action into the code at this point, so developers can easily add code into the solution to alter the image output as they need?

Let me know if you think this approach is workable, and I will submit a patch.

If not, I'll create a new version of WP_Image_Editor_Imagick and use the 'wp_image_editors' filter to pull this in.

#10 in reply to: ↑ 9 @ceer
8 months ago

Let me know if you think this approach is workable, and I will submit a patch.

I've tested without success.
Furthermore, I have this notice:

Deprecated: Imagick::flattenImages method is deprecated and it's use should be avoided in /www/wp-includes/class-wp-image-editor-imagick.php on line 157

The only working solution for me yet is the @gitlost one: https://core.trac.wordpress.org/attachment/ticket/39216/39216.4.patch

Last edited 8 months ago by ceer (previous) (diff)

#11 in reply to: ↑ 9 ; follow-up: @maysi
4 months ago

Replying to mwtsn:

In the file wp-includes/class-wp-image-editor-imagick.php after line 153 ($this->image->readImage( $filename );) you can put in the code:

$this->image->setImageBackgroundColor('white');
$this->image = $this->image->flattenImages();

This worked for me, too.

#12 in reply to: ↑ 11 @madejackson
2 months ago

Replying to maysi:

Replying to mwtsn:

In the file wp-includes/class-wp-image-editor-imagick.php after line 153 ($this->image->readImage( $filename );) you can put in the code:

$this->image->setImageBackgroundColor('white');
$this->image = $this->image->flattenImages();

This worked for me, too.

Can confirm, This worked for me, too.

The two other Solutions proposed in ticket #45982 did break the thumbnail-generation for me.

#13 @ceer
2 months ago

Hi @mwtsn, I confirm that your solution (ticket:39216#comment:9) now worked for me too! (ImageMagick 6.8.9-9, Ubuntu 16.04.6, PHP 7.0.33).

Note: See TracTickets for help on using tickets.