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

Multiview support using OVR_multiview2 #16316

Merged
merged 94 commits into from Sep 11, 2019

Conversation

@fernandojsg
Copy link
Collaborator

commented Apr 23, 2019

Intro

This PR introduces multiview support using the current OVR_multiview2 API (spec https://www.khronos.org/registry/webgl/extensions/OVR_multiview2) as all the previous ones (WebVR multiview, WEBGL_multiview, OVR_multiview) are deprecated and should not be implemented even if they are still supported by some browsers.

The first implementations of multiview were attached to the WebVR API but this is not the case anymore and we could use this API to paint several views using a drawcall on the screen without need to have a VR device, eg for different views for a CAD editor.
With that in mind I tried to detach the implementation from VR details and rely on ArrayCamera instead to determine when to render multiple views.

The key point for me here is that the expected benefit for this feature when using 2 views (as currently in VR) is around 40% without any modification on the user's codebase.

Capture

Test

Currently this extension is just implement on Chrome Canary and must be run with the following command line arguments:

--use-cmd-decoder=passthrough --enable-webgl-draft-extensions

Implementation

  • Added multiview: boolean attribute to the WebGLRenderer to detect if you want to enable it if available. It's used by default if it's available and multiview !== false. But probably we could just enable it by default if vr.enabled = true
  • Introduced WebGLMultiview.js that should take care of all the binding/unbinding of the textures and framebuffers
  • Introduced WebGLMultiviewRenderTarget that will include the numViews that render target has.
  • Modify preffix on the shaders to inject an array of matrices that will be used based on the gl_ViewID_OVR attribute.
  • On setProgram switch from setting single matrices to array of matrices when using multiview and ArrayCamera

TO-DO / Open questions

This is still a WIP implementation, there are some issues and decisions that need to be addressed before merging it:

  • Enable by default just when extension is available and vr.enabled = true ?
  • Currently the code is open enough to support more than 2 views, but when creating the shader we should know the number of views that will be used to define the size of the matrices arrays. Currently I have set 2 by default, but not sure if it could be nice to try setting it to MAX_VIEWS_OVR even if we use less views in our app.
  • Split the renderObjects function into two, to call multiview vs single. >> It's been simplified enough like to keep both together on the same function
  • Remove the code to bind fbo, set viewport and clear it from the multiview block on renderObjects and inject it on setRenderTarget instead.
  • Create a temporary structure to hold the array of matrices instead of allocating them on every call to setProgram when using multiview.
  • Currently just viewMatrix and projectionMatrix are injected as an array, I'll include the rest of matrices affected by them per view.
  • Support for shadows, currently broken
  • Move creation of multiview fbo to WebGLTextures::setupRenderTarget() ?
@fernandojsg fernandojsg force-pushed the fernandojsg:ovr_multiview2 branch 4 times, most recently from 94726df to 2fc92a4 Apr 23, 2019
@takahirox

This comment has been minimized.

Copy link
Collaborator

commented Apr 23, 2019

Please add ShadowMap support to the TODO list. Currently this implementation doesn't work with Shadow.

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 23, 2019

Please add ShadowMap support to the TODO list. Currently this implementation doesn't work with Shadow.

Added

@mrdoob mrdoob added this to the rXX milestone Apr 24, 2019
@mrdoob

This comment has been minimized.

Copy link
Owner

commented Apr 24, 2019

Looking good!

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 25, 2019

DEPRECATED
Regarding the shadows issue: ShadowMap will use a custom render target and a MeshDepthMaterial. If multiview is enabled all the materials will get injected the multiview specific directives and uniforms in the shaders: https://github.com/mrdoob/three.js/pull/16316/files#diff-58e8247d21441c70f4f66e068eac5bfaR578 but as the shadowmap won't use the multiview fbo it will throw a warning because of the incompatible number of views on the shader vs the render target used.

To fix (1cd69c9) this I've introduced a new material attribute supportsMultiview (open to suggestions) that will be set to true by default for all the materials and currently just false for the MeshDepthMaterial used on the ShadowMap class.

What do you think @takahirox @mrdoob ?

PS: I have added temporarily a copy of the dragging demo using multiview too to check the shadows working.
~~

@fernandojsg fernandojsg force-pushed the fernandojsg:ovr_multiview2 branch from 40e382f to 08ded2c Apr 25, 2019
@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 25, 2019

I added support for modelViewMatrix and normalMatrix. I created a method WebGLMultiview::computeObjectMatrices() that will create the array of matrices to avoid allocating them on every call, and update them as needed.

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 25, 2019

I removed the array allocation and map calls for camera's viewMatrices and projectionMatrices by introducing a similar method to the object's WebGLMultiview::computeObjectMatrices() -> WebGLMultiview::computeCameraMatrices

@fernandojsg fernandojsg force-pushed the fernandojsg:ovr_multiview2 branch from 416694a to 255da13 Apr 26, 2019
@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 26, 2019

Use multiview.getNumViews() everywhere so if we define a different (default = 2) number of views on the WebGLMultiview the number of matrices, shaders and so will get the correct value.

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 29, 2019

Moved the creation of the rendertarget out from WebGLMultiview into WebGLMultiviewRenderTarget, and changed the update*Matrices to use aux matrices instead of creating one on each object and camera.
Kudos to @takahirox for the help refactoring it

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 29, 2019

Currently the only item that I haven't addressed from the list is:

Enable by default just when extension is available and vr.enabled = true ?

As the WebGLMultiview object will be created on the WebGLRenderer constructor, so currently we will just do that when the attribute multiview is set true on the constructor's options.

I have noticed that there are a huge impact in performance when updating uniforms that consist on array of matrices, but I'll like to move the discussion to #16355.
In the meantime I'll be working on a alternative solution by setting the multiview uniforms directly from WebGLMultiview without using any cache.

Another performance impact is that currently once you request multiview, you "should" use, as requesting it for rendering just 1 view doesn't make much sense, as you will still be rendering to a N-dimensional FBO (depending on the number of views) even if you just copy one of these layers to the final canvas. I created an issue to discuss it here #16356

Still I believe neither of these issues should be a blocking for this as the performance improvement that we will get specially on WebXR is going to be really good (Benchmarks soon)

Feedback? :)

@fernandojsg fernandojsg changed the title Multiview support using OVR_multiview2 (WIP) Multiview support using OVR_multiview2 Apr 29, 2019
@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 30, 2019

Added a new example using multiple views + multiview using just array cameras, currently using bounds to define the viewports, although I've opened an issue regarding it #16361

Introducing some proposals by @takahirox adding setNumViews() on the WebGLMultiview and automatically detect the array camera size.

Currently the user doesn't need to do anything else than just request multiview on the renderer and use WebVR or render with a ArrayCamera and it should just work.

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 30, 2019

I have created a different branch to try to fix the performance issue introduced because of the cache of uniforms (Described in #16355)
fernandojsg@d35d296

The performance difference between setting the uniforms "manually" and doing the flatten versus the standard version of calling uniforms.setValue results in the webgl_multiple_views_multiview example goes from 55ms measuring renderer.render() to 22ms...

Removing the cache on WebGLUniforms for array of Matrix4 and Matrix3 will improve, around ~30ms

@fernandojsg fernandojsg force-pushed the fernandojsg:ovr_multiview2 branch from 092e5a9 to af35612 Apr 30, 2019
for ( var x = 0; x < AMOUNT; x ++ ) {
var subcamera = new THREE.PerspectiveCamera( 40 + (x + y) * 20, ASPECT_RATIO, 0.25, 100 );
subcamera.bounds = new THREE.Vector4( x / AMOUNT, y / AMOUNT, SIZE, SIZE );

This comment has been minimized.

Copy link
@mrdoob

This comment has been minimized.

Copy link
@takahirox

takahirox May 1, 2019

Collaborator

How about merging this PR as is so far (if looking good) and then considering to switch from ArrayCamera bounds to ArrayCamera viewport (if needed) in another PR? That'd be easier to discuss and review maybe.

This comment has been minimized.

Copy link
@fernandojsg

fernandojsg May 1, 2019

Author Collaborator

Addressed on #16367 once that will get merged I'll push the changed to fix this PR

This comment has been minimized.

Copy link
@fernandojsg

fernandojsg May 1, 2019

Author Collaborator

Updated it 3c58278 /cc @mrdoob @takahirox

@fernandojsg fernandojsg force-pushed the fernandojsg:ovr_multiview2 branch from c2cc4eb to 3c58278 May 1, 2019
@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented May 3, 2019

@mrdoob @takahirox friendly ping :)

fernandojsg added 2 commits Sep 11, 2019
@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 11, 2019

@mrdoob Ok fixed all the minor issues I've found as linter warnings and the changes on the feedback you provided to me. I tested it already on desktop and quest.

@mrdoob mrdoob merged commit c680e4c into mrdoob:dev Sep 11, 2019
2 checks passed
2 checks passed
LGTM analysis: JavaScript 3 new alerts
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
WebGL2 automation moved this from Review / QA to Done Sep 11, 2019
@mrdoob

This comment has been minimized.

Copy link
Owner

commented Sep 11, 2019

Thanks for your patience! 🙏

@takahirox

This comment has been minimized.

Copy link
Collaborator

commented Sep 11, 2019

Yay, finally!

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 11, 2019

It was worth it! Is looking much better than at the beginning, thanks for the exhaustive review! ;)

@fernandojsg fernandojsg deleted the fernandojsg:ovr_multiview2 branch Sep 11, 2019
@arpu

This comment has been minimized.

Copy link

commented Sep 11, 2019

is it right this only works with webgl2 and alpha: false ?
like
canvas.getContext( 'webgl2', { alpha: false } )

@arpu

This comment has been minimized.

Copy link

commented Sep 11, 2019

ah i see in the example canvas.getContext( 'webgl2', { antialias: false } );

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 11, 2019

@arpu currently the multiview extension is just available on WebGL2 contexts because it needs glsl3 and also texture arrays. And currently the extension doesn't support multisampling so it should be antialias: false. The WebGL WG is working on a spec for a multisampled multiview extension that should come up soon ;)

@Mugen87

This comment has been minimized.

Copy link
Collaborator

commented Sep 12, 2019

Like mentioned here, many examples log now the following warning:

THREE.WebGLRenderer: OVR_multiview2 extension not supported.

@mrdoob Is this okay? Maybe it would be good to avoid this warning if possible (see #16316 (comment)).

https://raw.githack.com/mrdoob/three.js/dev/examples/index.html#webgl_animation_cloth

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 12, 2019

Yep I agree, it could be nice to get rid of these warnings. I'm happy to create a new PR with the preferred way by @mrdoob as discussed on #16316 (comment)

@carstenschwede

This comment has been minimized.

Copy link

commented Sep 13, 2019

@fernandojsg Congrats! I was looking forward to this merge. Could the OVR_multiview2 extension be used to increase performance for CubeCamera (i.e. "WebGLMultiViewRenderTargetCube")?

@mrdoob

This comment has been minimized.

Copy link
Owner

commented Sep 13, 2019

I think OVR_multiview2 is limited to 4 views max?

@mrdoob

This comment has been minimized.

Copy link
Owner

commented Sep 13, 2019

Yep I agree, it could be nice to get rid of these warnings. I'm happy to create a new PR with the preferred way by @mrdoob as discussed on #16316 (comment)

I'll look at this today.

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 13, 2019

I think OVR_multiview2 is limited to 4 views max?

Exactly, currently the maximum is 4, although is recommended to check for it using (As in WebGLCapabilities: https://github.com/mrdoob/three.js/pull/16316/files#diff-c286888722879bf7927784081e925eb4R91):
var maxMultiviewViews = multiview ? gl.getParameter( multiviewExt.MAX_VIEWS_OVR ) : 0;

I'll look at this today.

Cool

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 13, 2019

@fernandojsg Congrats! I was looking forward to this merge. Could the OVR_multiview2 extension be used to increase performance for CubeCamera (i.e. "WebGLMultiViewRenderTargetCube")?

I'm not familiar enough with how is implemented the cube camera, but because of the maximum number of views it won't be possible to render it in one drawcall but in 2

@carstenschwede

This comment has been minimized.

Copy link

commented Sep 14, 2019

Don't want to hijack the PR, so just as a sidenote: I couldn't find the Chrome commit that implemented multiview, but the Khronos spec suggests

What is the minimum values permitted for MAX_VIEWS_OVR?
2. But at least 6 is recommended for consistency with Vulkan.
Six views is desireable as it can be used to render all faces of a cube map
in a single pass.

Might be interesting for a future use case, once MAX_VIEWS_OVR has increased. Even a max. value of 4 views could reduce drawcalls from 6 to 2.

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 14, 2019

@carstenschwede the MAX_VIEWS_OVR in the browser should be populated with the GL_MAX_VIEWS_OVR value and so far I have seen just 4 as maximum on all the devices I have.
As you mention, depending on that max the logic of specific tasks could be divided into smaller number of drawcalls automatically, so currently from 6 to 2 and eventually if we get 6 as max, it will be done just in 1 drawcall.
Feel free to open another issue with this topic and we could keep the discussion there publicly as many people won't be reading this because is already closed

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 14, 2019

fwiw I wrote a short blog post around multiview support, focused on webxr though https://blog.mozvr.com/multiview-on-webxr/

@RemusMar

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2019

@fernandojsg

Fernando,
On the latest Firefox 69 (Windows) I get
"THREE.WebGLRenderer: OVR_multiview2 extension not supported."
on various machines.

@fernandojsg

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 16, 2019

@RemusMar it will just be enabled by default on Firefox 71. Give it a try with Firefox Nightly and go to about:config and set webgl.enable-draft-extensions to true

@mrdoob

This comment has been minimized.

Copy link
Owner

commented Sep 25, 2019

@Mugen87

Like mentioned here, many examples log now the following warning:

THREE.WebGLRenderer: OVR_multiview2 extension not supported.

Fixed #17575

@mrdoob

This comment has been minimized.

Copy link
Owner

commented Sep 25, 2019

@fernandojsg

Yep I agree, it could be nice to get rid of these warnings. I'm happy to create a new PR with the preferred way by @mrdoob as discussed on #16316 (comment)

Sorry. After studying the problem, the proposed solutions seemed a bit like "hiding the dust under the carpet". I think the refactored code is a bit more robust and easier to follow.

@mrdoob

This comment has been minimized.

Copy link
Owner

commented Sep 27, 2019

Additional PRs: #17596 #17599

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
WebGL2
  
Done
8 participants
You can’t perform that action at this time.