885632 - SkiaGL fails clip winding test

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

We do not currently pass test_2d.canvas.clip.winding.html. Seems important.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Always use soft clipping with SkiaGL

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Not entirely sure why, but SkiaGL clipping only works correctly if we have antialiasing enabled. The attached patch ensures we also do that when using the GL backend. The current behavior is maintained for the software one.

Comment 3

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 766030 [details] [diff] [review]
Always use soft clipping with SkiaGL

Review of attachment 766030 [details] [diff] [review]:
-----------------------------------------------------------------

For the record, the initial decision to not do soft clipping on android was based off Jeff's suggestion here:

https://bugzilla.mozilla.org/show_bug.cgi?id=716415#c7

I don't think anyone every measured the performance impact of this.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5e051e9912f8

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5e051e9912f8

Comment 6

[Andreas Gal :gal]

For the record, switching off soft clipping on Android broke FFOS / E.me homescreen.

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(It didn't break it -- it just caused it to render without antialiasing.  That's considered broken from a UX perpsective, but not from code.  The ANDROID define is defined by both B2G and Fx Android, because B2G is ANDROID... but the original reasons for disabling it here seem to apply equally to both, esp. if it's performance.)

Comment 8

[Andreas Gal :gal]

Well if we break anti aliasing or component alpha on desktop, I am pretty sure we would consider that a regression, no? :) Anyway, we have a patch up in the other bug to remove the code. I didn't find any clear data on clipping being slow with soft clipping on mobile, and Chrome doesn't disable this, so we should be en par performance wise, at worst.