Use PorterDuff blend mode when below Android 29 #34800
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
bparrishMines
changed the title
| @@ -308,7 +309,7 @@ public void draw(Canvas canvas) { | |||
| if (Build.VERSION.SDK_INT >= 29) { | |||
| surfaceCanvas.drawColor(Color.TRANSPARENT, BlendMode.CLEAR); | |||
| } else { | |||
| surfaceCanvas.drawColor(Color.TRANSPARENT); | |||
| surfaceCanvas.drawColor(Color.TRANSPARENT, PorterDuff.Mode.CLEAR); | |||
There was a problem hiding this comment.
These blocks can be simplified by using drawPaint and setting the color and blend mode on the paint. That's all the Android impl is doing here anyway
There was a problem hiding this comment.
You then won't have the branch here or need extra tests for different api levels
There was a problem hiding this comment.
Are you referring to this method? https://developer.android.com/reference/android/graphics/Paint#setBlendMode(android.graphics.BlendMode).
Unfortunately, it also requires API 29.
Although, I agree that we don't need both methods. The PorterDuf blend mode isn't deprecated so I think we could just switch to that.
There was a problem hiding this comment.
Ahh ok, I didn't realize that. This looks good to me though.
bparrishMines
added
the
autosubmit
Engine fix for flutter/flutter#104686.
As of now, this is solved at the plugin level by switching to Hybrid Composition when the view has a transparent/translucent background: flutter/plugins#6063
This should hopefully remove the need to do this. I tested it locally and the previous rendering issue was fixed when this flag was used.
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.