#39216 closed defect (bug) (fixed)
PDFs with non-opaque alpha channels can result in previews with black backgrounds.
Reported by: | gitlost | Owned by: | antpb |
---|---|---|---|
Milestone: | 6.4 | Priority: | normal |
Severity: | normal | Version: | 4.7 |
Component: | Media | Keywords: | dev-feedback 2nd-opinion has-patch has-unit-tests early |
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 (8)
Change History (46)
#1
@
8 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.
This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.
8 years ago
#3
@
8 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.
8 years ago
#5
@
8 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.
#6
@
7 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?
#7
@
7 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
follow-up:
↓ 14
@
6 years 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:
↓ 10
↓ 11
@
6 years 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
@
6 years 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
#11
in reply to:
↑ 9
;
follow-up:
↓ 12
@
5 years 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
@
5 years 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
@
5 years 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).
#14
in reply to:
↑ 8
@
5 years ago
Replying to emirpprime:
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.
I can confirm this very simple mod do the trick for us too
Configuration:
Debian 10
PHP 7.2.27
Ghostscript 9.27
Command:
sed -i.back -e 's#pngalpha#jpeg#g' /etc/ImageMagick-6/delegates.xml
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
2 years ago
#16
@
2 years ago
- Milestone changed from Awaiting Review to 6.2
This ticket was discussed in a recent bug scrub. We are moving this ticket to 6.2 for investigation since it has been so long. Would any folks watching this ticket be able to provide an update if this is still impacting and if the fix is still valid?
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
21 months ago
#19
@
21 months ago
- Milestone changed from 6.2 to Future Release
Theres still investigation that needs to happen here so we'll need to move this to a future release to allow time to investigate and propose a fix.
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
17 months ago
#23
@
17 months ago
- Owner set to antpb
- Status changed from new to assigned
Just a high level test of the patch I can confirm it fixes the issue. Theres some formatting that needs to be adjusted in the patch and I need to investigate an issue with applying the tests in the patch but aside from that this should be good to go.
Assigning to myself to make those updates.
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
17 months ago
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
16 months ago
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
16 months ago
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
16 months ago
This ticket was mentioned in PR #4745 on WordPress/wordpress-develop by antpb.
16 months ago
#28
Creating this draft to check that the added files in these tests are okay.
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
16 months ago
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
16 months ago
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
15 months ago
This ticket was mentioned in Slack in #core by chaion07. View the logs.
15 months ago
#33
@
15 months ago
Thanks @gitlost for reporting this. We reviewed this ticket once during a recent bug-scrub session and based on the feedback we are hoping to update the milestone to 6.4 unless @antpb is available to take a look in the next few hours. Cheers!
Props to @costdev & @oglekler
This ticket was mentioned in Slack in #core-media by oglekler. View the logs.
15 months ago
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
15 months ago
#36
@
15 months ago
- Keywords early added
- Milestone changed from 6.3 to 6.4
Moving 6.4 and adding early. this has had a lot of review and ready to commit
Real-world example.