Skip to content

Conversation

@cabanier
Copy link
Contributor

@cabanier cabanier commented Apr 12, 2025

Related issue: #20368, #28968

This contribution is funded by Meta

@github-actions
Copy link

github-actions bot commented Apr 12, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 336.49
78.38
336.49
78.38
+0 B
+0 B
WebGPU 542.52
150.29
544.68
150.82
+2.16 kB
+529 B
WebGPU Nodes 541.87
150.13
544.03
150.66
+2.16 kB
+528 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 465.45
112.23
465.58
112.26
+138 B
+37 B
WebGPU 615.32
166.3
618.38
167.07
+3.06 kB
+779 B
WebGPU Nodes 570.19
155.68
573.25
156.44
+3.06 kB
+766 B

const cacheKey = this.getOutputCacheKey();

const output = texture( outputTarget, screenUV ).renderOutput( renderer.toneMapping, renderer.currentColorSpace );
//builtin( 'gl_ViewID_OVR' )??
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mugen87 how can I incorporate the view id?

Copy link
Collaborator

@Mugen87 Mugen87 Apr 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why you need to evaluate gl_ViewID_OVR at this place? I've though this built-in is just required to detect the correct camera matrices for the current view.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we rendered to a multiview rendertarget in the first step and are rendering to the multiview xr target, we need to select the right texture in the texture array.
Without using gl_ViewID_OVR, the fragment shader will always pick the texture for the left eye.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mugen87 I fixed this in my latest diff. It was easier than I thought :-)
This node system is quite the change! Very nice

@cabanier cabanier marked this pull request as ready for review April 14, 2025 04:37
@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 14, 2025

I'll pushed some code clean up and additions to the documentation.

@cabanier cabanier force-pushed the multiview branch 2 times, most recently from f3eaba2 to aac046b Compare April 14, 2025 17:21
@cabanier
Copy link
Contributor Author

@Mugen87 I don't understand why the perlin example keeps failing...

@cabanier cabanier force-pushed the multiview branch 4 times, most recently from 00efce1 to b7250c8 Compare April 14, 2025 19:03
@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 14, 2025

I don't understand why the perlin example keeps failing...

Found the issue. I've introduced a bug when refactoring WebGLTextureUtils with an earlier commit. Should be fixed now. Sorry for the inconvenience.


this.setFoveation( this.getFoveation() );

this._referenceSpace = await session.requestReferenceSpace( this.getReferenceSpaceType() );
Copy link
Collaborator

@Mugen87 Mugen87 Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#30902 has moved this line so the non-layers code path uses requestReferenceSpace() as well.

If you revert the PR, I guess we have to copy the line into the non-layer else block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! I added it to the other path.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 14, 2025

Okay, CI runs now as expected 🎉 !

@cabanier
Copy link
Contributor Author

cabanier commented Apr 14, 2025

Okay, CI runs now as expected 🎉 !

awesome! I tested both xr test files so if we're in the green I think we're all set.

@Mugen87 Mugen87 merged commit 1497215 into mrdoob:dev Apr 14, 2025
12 checks passed
@Mugen87 Mugen87 added this to the r176 milestone Apr 14, 2025
@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 14, 2025

@cabanier Fantastic work!

@cabanier cabanier deleted the multiview branch April 14, 2025 21:06
RuthySheffi pushed a commit to RuthySheffi/three.js that referenced this pull request Jun 5, 2025
* Add support for multiview

* support multi-pass with webxr multiview

* update fragment shader for multiview

* Clean up.

* Update XRManager.js

* address review comment for multiview rendering

* more cleanup + fixes for multiview

* fix webxr layers bug + enabled multiview in native layers sample

* Update WebGLTextureUtils.js

* Update RenderTarget.js

* remove test code

* fix reference space for non-layers path

---------

Co-authored-by: Mugen87 <[email protected]>
RuthySheffi pushed a commit to RuthySheffi/three.js that referenced this pull request Jun 5, 2025
* Add support for multiview

* support multi-pass with webxr multiview

* update fragment shader for multiview

* Clean up.

* Update XRManager.js

* address review comment for multiview rendering

* more cleanup + fixes for multiview

* fix webxr layers bug + enabled multiview in native layers sample

* Update WebGLTextureUtils.js

* Update RenderTarget.js

* remove test code

* fix reference space for non-layers path

---------

Co-authored-by: Mugen87 <[email protected]>
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.

2 participants