Closed Bug 1496838 Opened Last year Closed Last year

Use a shared depth buffer for all render targets

Categories

(Core :: Graphics: WebRender, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

Per IRC discussion yesterday, the current setup uses a lot more memory than necessary.

Fixing this also removes our implicit dependency on reinitializing textures to add depth buffers when we need them, and thus helps with bug 1496168.
In GL and ES3+, FB attachments don't need to be same-size, so you can reuse larger depth Resources on smaller RTs.
I don't know how ANGLE polyfills this, since D3D11 does *not* allow this. It might create shadow surfaces on the fly, so we might not want to try this on ANGLE. I'll ask them.
(In reply to Dzmitry Malyshau [:kvark] from comment #1)
> We already use the same depth surface for all slices of a render target:
> https://github.com/servo/webrender/blob/
> d84c919414667c9fc88621013c0297f0d3787845/webrender/src/device/gl.rs#L1367

Fair. Updating the bug title.

(In reply to Jeff Gilbert [:jgilbert] from comment #2)
> In GL and ES3+, FB attachments don't need to be same-size, so you can reuse
> larger depth Resources on smaller RTs.
> I don't know how ANGLE polyfills this, since D3D11 does *not* allow this. It
> might create shadow surfaces on the fly, so we might not want to try this on
> ANGLE. I'll ask them.

Please do. If this is a problem, we could just force all render target slices to be the same size (2048 x 2048). This might use a bit more storage in some cases, but would also improve reuse.
Summary: Use a shared depth buffer for all render targets and slices → Use a shared depth buffer for all render targets
Also, any suggestions on how to test that the z-buffer is still working correctly? Would rendering be obviously wrong if it weren't?
Jamie on ANGLE said they just reject differing-sized FB attachments, in part because WebGL is the primary user of ANGLE, and WebGL dropped support for such FBs due to D3D11 limitations.
So I think that means we can't share a single texture, but I think I cans still get us a fair amount of sharing by tweaking the patch I have. I'll do that next week.
(In reply to Bobby Holley (:bholley) from comment #5)
> Also, any suggestions on how to test that the z-buffer is still working
> correctly? Would rendering be obviously wrong if it weren't?

glDepthTest(GL_NEVER) will reject all fragments if you have a depth buffer, but is ignored if you don't have a depth buffer.
Generally yeah, you'd get a bunch of weird misrenderings (or nothing rendered) if you aren't clearing the depth buffer correctly.
Depends on: 1497665
Priority: -- → P2
We're moving to a model where depth is an optional bind-time decision,
so this needs to go. Luckily, it's not used anymore.

Depends on D8797
The semantics of this get a little fuzzy when we introduce separate FBO
Id vectors for depth/nodepth, so it's easier to just drop it.

Depends on D8798
Depends on: 1499494
Pushed by bholley@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/31175b7c468b
Add memory reporters for shared depth targets. r=kvark
https://hg.mozilla.org/mozilla-central/rev/31175b7c468b
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.