WordPress.org

Make WordPress Core

Opened 12 months ago

Last modified 7 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:

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 12 months ago.
Real-world example.
test_alpha.pdf (7.0 KB) - added by gitlost 12 months ago.
Simple blank test pdf.
39216.patch (3.7 KB) - added by gitlost 12 months ago.
Patch to remove alpha channel. Unittest requires blank test pdf.
test_cmyk.pdf (7.4 KB) - added by gitlost 11 months ago.
Test CMYK PDF.
39216.2.patch (4.9 KB) - added by gitlost 11 months ago.
Refresh of patch with unsharpMaskImage not called for CMYK.
39216.3.patch (2.9 KB) - added by gitlost 11 months ago.
Refresh of original patch without the unsharpMaskImage() change.
39216.4.patch (2.9 KB) - added by gitlost 7 months ago.
Refresh, comment changes only.

Download all attachments as: .zip

Change History (14)

@gitlost
12 months ago

Real-world example.

@gitlost
12 months ago

Simple blank test pdf.

@gitlost
12 months ago

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

#1 @gitlost
11 months 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
11 months ago

Test CMYK PDF.

@gitlost
11 months ago

Refresh of patch with unsharpMaskImage not called for CMYK.

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


11 months ago

#3 @joemcgill
11 months 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.


11 months ago

#5 @gitlost
11 months 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
11 months ago

Refresh of original patch without the unsharpMaskImage() change.

#6 @launchinteractive
7 months 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
7 months ago

Refresh, comment changes only.

#7 @gitlost
7 months 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/.)

Note: See TracTickets for help on using tickets.