Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebGLRenderer: Rearrange logic in renderObjects to reduce CPU-side draw cost for multi-camera setups. #22123

Merged
merged 1 commit into from
Jul 13, 2021

Conversation

davehill00
Copy link
Contributor

Description

Rendering all objects for one camera at a time (vs. rendering each object for all cameras before moving to the next object) reduces the CPU cost of rendering fairly significantly. Specifically, reduces needing to recompute a bunch of stuff per-camera for each object, and also reduces the number of GL state changes made when rendering objects with similar materials.

You can see a before/after example in this sample - link. By default, it renders with the current Three.js behavior for WebGLRenderer::renderObjects. If you tap the right grip button, it will switch to the proposed renderObjects order. Tapping the left grip button will revert to the default behavior again. To most effectively evaluate this, run OVR Metrics HUD and keep an eye on framerate. On Quest 2, I see an initial framerate of ~60fps. Toggling to the proposed renderObjects order, I get a solid 90fps.

A note on the sample -- it is focused on stressing CPU draw calls. It draws a bunch of individual cubes as unique draw calls, and it's intentionally rendering headlocked and to low-resolution eye buffers to provide consistent perf results and to focus the timing results on CPU perf.

@mrdoob
Copy link
Owner

mrdoob commented Jul 13, 2021

Looks good to me!

@toji This replaces the approach you suggested a few years ago.

@mrdoob mrdoob added this to the r131 milestone Jul 13, 2021
@mrdoob mrdoob changed the title Rearrange logic in renderObjects to reduce CPU-side draw cost for multi-camera setups. WebGLRenderer: Rearrange logic in renderObjects to reduce CPU-side draw cost for multi-camera setups. Jul 13, 2021
@mrdoob mrdoob merged commit 5c90688 into mrdoob:dev Jul 13, 2021
@mrdoob
Copy link
Owner

mrdoob commented Jul 13, 2021

Thanks!

@toji
Copy link
Contributor

toji commented Jul 13, 2021

@toji This replaces the approach you suggested a few years ago.

Yeah, I did advocate for the other ordering but I think in hindsight this is better. My suggestion generally (and I don't believe it was targeted explicitly at Three) was to loop through each object in the scene and draw it for each camera before moving on to the next. It's the approach advocated for in this WebXR sample, and there it makes sense and has a measurable benefit. However, that makes the assumption that it is less expensive to frequently set the per-camera state (which in that sample is simply the viewport and projection/view matrices) than to double up on the number of times you set per-object state. It's easily possible that doesn't hold true in Three.

Worth noting that another change I suggested at that point was ensuring that rendering passes that didn't need to be done per-eye (such as producing shadow buffers) were only performed once. That's still critical and this doesn't change that. 👍

@mrdoob
Copy link
Owner

mrdoob commented Jul 13, 2021

Worth noting that another change I suggested at that point was ensuring that rendering passes that didn't need to be done per-eye (such as producing shadow buffers) were only performed once. That's still critical and this doesn't change that. 👍

This was implemented years ago! 🤓

@toji
Copy link
Contributor

toji commented Jul 13, 2021

This was implemented years ago! 🤓

Yeah, sorry! I meant to imply "This doesn't break that thing, which you already did and is working and is good." 😄

@cabanier
Copy link
Contributor

@mrdoob should we merge this then?

@mrdoob
Copy link
Owner

mrdoob commented Jul 20, 2021

It's merged already 🤓

zach-capalbo added a commit to zach-capalbo/three.js that referenced this pull request Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants