Remove remaining discard usage from WebRender shaders
Categories
(Core :: Graphics: WebRender, task, P3)
Tracking
()
People
(Reporter: lsalzman, Assigned: nical)
References
(Blocks 2 open bugs)
Details
Attachments
(1 obsolete file)
We only have two remaining shaders in WR that use discard - cs_clip_image and brush_radial_gradient. It would be beneficial to eliminate discard from these remaining cases. It is beneficial for performance reasons in accelerated WR. It also would enable removing a bunch of complicated code-paths from SWGL regarding supporting discard in shaders.
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
Relevant comment about the remaining discard in the clip shader: https://bugzilla.mozilla.org/show_bug.cgi?id=1616326#c11
Assignee | ||
Comment 2•4 years ago
|
||
We talked about this with Glenn and it looks like we should be able to remove the discard by outputing 1.0 instead.
Assignee | ||
Comment 3•4 years ago
|
||
The solution ended up quite unsavoury because in order to use blending instead of discard we first have to ensure blending is enabled which, if this is the first clip, requires ensuring the target was initialized with ones, and which is decided somewhere else much earlier part.
So this patch allows the clip batcher to request a clear mode (with the constraint that it doesn't contradict with the initially requested one).
The patch changes the is_first_clip term for disable_blending in order to be more upfront about the implications and not to have to lie about it in the case of non-axis-aligned tiled masks.
Assignee | ||
Comment 5•4 years ago
|
||
It only affects the non-axis-aligned case. I suspect it will be a bit faster in this case because we don't render the whole area for each tile anymore (just have a 1px overlap to avoid artifacts), but on the other hand we have to clear first and and use blending if the tiled mask isn't axis aligned.
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Comment 7•4 years ago
|
||
I could maybe adapt swgl_clipMask() to do something similar here if we need, but that will still leave the expensive discard in there for HW cases, so it would possibly be a win if we could clip these things earlier in the pipeline before they hit the fragment shader at all.
Reporter | ||
Updated•4 years ago
|
Assignee | ||
Comment 8•4 years ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #6)
Nical, where did we end up with this?
My conclusion was that to move forward with this we have to rethink how tiled clip masks are generated because the current implementation relies too much on tile segments overlapping. If someone puts the time it'd be worth it if only because the current way this is implemented is quite hard to understand.
Updated•4 years ago
|
Assignee | ||
Comment 9•4 years ago
|
||
Lee fixed this a little while ago, I don't have the bug number handy.
Reporter | ||
Comment 10•4 years ago
|
||
This is still an issue because cs_clip_image still uses discard. We never finished fixing this yet.
Reporter | ||
Comment 11•4 years ago
|
||
Bug 1682195 resolved the issue with cs_clip_image using discard, so we can close this now.
Description
•