Fix Imagick detecting partial transparency #1215
Conversation
westonruter
added
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @JacobDB. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| @@ -82,7 +82,7 @@ public function has_transparency() { | |||
| for ( $x = 0; $x < $w; $x++ ) { | |||
| for ( $y = 0; $y < $h; $y++ ) { | |||
| $pixel = $this->image->getImagePixelColor( $x, $y ); | |||
| $color = $pixel->getColor(); | |||
| $color = $pixel->getColor( 2 ); | |||
There was a problem hiding this comment.
This is the crux of the fix.
There was a problem hiding this comment.
Because of #1215 (comment), however, this is not currently getting tested in PHPUnit in wp-env.
|
I discovered that the tests were not testing both of the Imagick and GD codepaths. It was rather just testing the GD codepath twice, since it was letting WP select the implementation that was available in both the GD and Imagick test cases. As part of this, I found that Imagick seems to not be configured properly in wp-env because |
westonruter
changed the title
|
Yes, @westonruter. I have explained it in - #1013 (comment) and #1013 (comment) |
Fixes #1214
By default
ImagickPixel::getColor()returns an array with the 4th alpha array item being a binary0or1. This doesn't work when the image has partial transparency. To detect that, you have to pass a$normalizedparameter to this method, either1(to return a 0.0-1.0 float) or2(to return a 0-1255 int). So this passes 2 so we can correctly compare if the alpha value is greater than 0.This PR also improves the test cases by removing duplicate tests and ensuring that the GD and Imagick code paths are both tested. Previously it was doing the same tests twice for GD since the tests weren't forcing either the GD or Imagick code path. As part of this, the Imagick tests now show up as being skipped since wp-env appears to have a misconfigured Imagick as detailed by @thelovekesh.