Closed Bug 1663555 (sw-wr-perf-discard) Opened 4 years ago Closed 4 years ago

Remove remaining discard usage from WebRender shaders

Categories

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

task

Tracking

()

RESOLVED FIXED

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.

Blocks: wr-gpu-time
Depends on: 1664459

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

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

Assignee: nobody → nical.bugzilla

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.

Will this make clipping slower?

Flags: needinfo?(nical.bugzilla)

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.

Flags: needinfo?(nical.bugzilla)

Nical, where did we end up with this?

Flags: needinfo?(nical.bugzilla)

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.

Blocks: 1655530
Alias: sw-wr-perf-discard

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

Flags: needinfo?(nical.bugzilla)
Attachment #9180418 - Attachment is obsolete: true

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

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

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

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

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

Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: