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: Refactored render loop and fix transmission in VR #22426

Merged
merged 7 commits into from
Aug 25, 2021

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Aug 25, 2021

Related issue: #21911

Description

Live link: https://raw.githack.com/mrdoob/three.js/dev/examples/#webxr_vr_sandbox

Screen Shot 2021-08-25 at 4 48 12 PM

In order to get transmission working in VR I had to refactor the render loop following what @davehill00 did in #22123.

Before this PR the logic was:

  1. Render Camera 1 Opaque List
  2. Render Camera 2 Opaque List
  3. Render Camera 1 Transparent List
  4. Render Camera 2 Transparent List

After this PR the logic is:

  1. Render Camera 1 Opaque List
  2. Render Camera 1 Transparent List
  3. Render Camera 2 Opaque List
  4. Render Camera 2 Transparent List

/cc @takahirox @davehill00

@davehill00
Copy link
Contributor

Were you able to make any optimizations to per-object render state setup costs, or does this just roll back the performance gain from my previous change?

@mrdoob
Copy link
Owner Author

mrdoob commented Aug 25, 2021

It's an evolution of what you did. Even less camera changes.

Before your PR the logic was:

  1. Render Camera 1 Opaque Object 1
  2. Render Camera 2 Opaque Object 1
  3. ...
  4. Render Camera 1 Transparent Object 1
  5. Render Camera 2 Transparent Object 1
  6. ...

@mrdoob
Copy link
Owner Author

mrdoob commented Aug 25, 2021

I'll push the builds temporarily so we can test this easily.

@mrdoob
Copy link
Owner Author

mrdoob commented Aug 25, 2021

@davehill00 Any chance you can try this a bit? I do not have my Quest this week.

Edit: Would be good to test that, say, this example has not regressed in performance/calls:

Old: https://raw.githack.com/mrdoob/three.js/r131/examples/#webxr_vr_cubes
New: https://threejs.org/examples/#webxr_vr_cubes

@davehill00
Copy link
Contributor

Yep, should be able to take a look sometime this week.

@phoenixbf
Copy link

link tested on my quest 2: transmission looks fine now in VR. For performance should compare more in depth

@mrdoob
Copy link
Owner Author

mrdoob commented Aug 25, 2021

Hmm... I think I'll merge this for r132 and revert and release a point release if something goes wrong 🤞

This reverts commit c5e295f.
@mrdoob mrdoob added this to the r132 milestone Aug 25, 2021
@mrdoob mrdoob merged commit 14663f5 into dev Aug 25, 2021
@mrdoob mrdoob deleted the transmission-vr branch August 25, 2021 16:13
@davehill00
Copy link
Contributor

Finally got around to testing this on Quest 2 (got sidelined by the bundler issue). Draw call perf looks comparable to the what we landed in r131. 👍

@mrdoob
Copy link
Owner Author

mrdoob commented Aug 28, 2021

Sweet! Thanks!

chubei-oppen added a commit to chubei-oppen/three.js that referenced this pull request Apr 12, 2022
Before mrdoob#22426,
when not using array camera, the background doesn't participate
in layer calculation.

We feel the old behaviour is better because we don't need to
always add layer `0` to the camera when doing multipass rendering.

This commit enables all layer for the background.

Note that the behaviour is still slightly different from r115:

When the camera's layer **mask** is set to 0, r115 will render
background and now won't.
chubei-oppen added a commit to oppenfuture/three.js that referenced this pull request Apr 12, 2022
Before mrdoob#22426,
when not using array camera, the background doesn't participate
in layer calculation.

We feel the old behaviour is better because we don't need to
always add layer `0` to the camera when doing multipass rendering.

This commit enables all layer for the background.

Note that the behaviour is still slightly different from r115:

When the camera's layer **mask** is set to 0, r115 will render
background and now won't.
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.

3 participants