1663555 - (sw-wr-perf-discard) Remove remaining discard usage from WebRender shaders

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, task, P3)

Description

[Lee Salzman [:lsalzman]]

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.

Comment 1

[Nicolas Silva [:nical]]

Relevant comment about the remaining discard in the clip shader: https://bugzilla.mozilla.org/show_bug.cgi?id=1616326#c11

Comment 2

[Nicolas Silva [:nical]]

We talked about this with Glenn and it looks like we should be able to remove the discard by outputing 1.0 instead.

Comment 3

[Nicolas Silva [:nical]]

Attachment: Bug 1663555 - Remove discard usage in the clip shader. r=gw

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.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Will this make clipping slower?

Comment 5

[Nicolas Silva [:nical]]

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.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Nical, where did we end up with this?

Comment 7

[Lee Salzman [:lsalzman]]

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.

Comment 8

[Nicolas Silva [:nical]]

(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.

Comment 9

[Nicolas Silva [:nical]]

Lee fixed this a little while ago, I don't have the bug number handy.

Comment 10

[Lee Salzman [:lsalzman]]

This is still an issue because cs_clip_image still uses discard. We never finished fixing this yet.

Comment 11

[Lee Salzman [:lsalzman]]

Bug 1682195 resolved the issue with cs_clip_image using discard, so we can close this now.