Page MenuHome

GSoC 2019: Core Support of Virtual Reality Headsets through OpenXR
Needs RevisionPublic

Authored by Julian Eisel (Severin) on Aug 20 2019, 1:00 PM.
Tags
None
Tokens
"Love" token, awarded by Lumpengnom."Love" token, awarded by astrand130."Love" token, awarded by Jaydead."Love" token, awarded by antoniov.

Details

Summary

GSoC 2019: Core Support of Virtual Reality Headsets through OpenXR

This is a patch with all changes from the soc-2019-openxr branch.
Changes from the temp-vr-draw-thread branch are not included.

The project aimed at bringing stable, well performing VR rendering support to Blender, based on the new OpenXR specification. Further, debugging utilities should be added. A fully fledged VR experience, e.g. with support for editing 3D content with controllers, was not in scope of the project.

How to Test

NOTE: Requries an Add-on. Get this by checking out the soc-2019-openxr branch in the Add-on repository.

Testing this patch isn't as simple as applying it and compiling with it. Information on how to test can be found here.
In short, the following is needed:

  • Install an OpenXR runtime.
  • Checkout the soc-2019-openxr Add-ons branch.
  • Install/build the OpenXR-SDK (already bundled with precompiled Windows libs, use install_deps.sh on Linux).
  • After a successful build, enable Basic VR Viewer Add-on.
  • Launch the session through WindowToggle VR Session.

Main Features

  • OpenXR loader from the OpenXR SDK to connect to the System’s active OpenXR runtime.
  • OpenXR extension (and API-layer) management.
  • Basic OpenXR event management.
  • VR session management.
  • Well performing VR rendering - more performance improvements are possible, but we have a quite decent baseline.
  • Carefully designed error handling strategy, cancelling the VR session with a useful user error message (e.g. “Failed to get device information. Is a device plugged in?") and no side-effects to the rest of Blender.
  • Compatibility with DirectX-only runtimes.
  • --debug-xr command line option enabling our own debug/information prints, OpenXR debug prints and the OpenXR core validation layer.
  • --debug-xr-time command line option to print frame render times and FPS information.
  • wmSurface API to manage offscreen drawables without a wmWindow.
  • Abstraction (currently a GHOST_Xr-API) for all OpenXR specific code. Makes higher level usage easier, but most importantly, improves maintenance (esp. when updating OpenXR versions).
  • Add-on to hide VR features by default from the UI.

If needed I can explain features in more detail, for now keeping it short.

Visibility for Users

Showing a "Toggle VR Session" button in the UI by default may fool users into thinking there was full fledged VR support in Blender. To not disappoint users with false promises, I wrapped this button into an Add-on which is disabled by default. The Add-on description clearly warns that support is limited and an early preview.

Error Handling Strategy

The error handling strategy I chose uses C++ exceptions, a controversial feature. Let me explain why I think this is reasonable here.

The strategy requirements were:

  • If an error occurs, cleanly exit the VR session (or destroy the entire context), causing no resource leaks or side effects to the rest of Blender.
  • Show a *useful* error message to the user.
  • Don't impair readability of code too much with error handling.

Here's why I chose an exception based strategy:

  • Most alternatives require early exiting functions. This early exiting has to be 'bubbled up' the call stack to the point that performs error handling. For safe code, early exit checks have to be performed everywhere and code gets really impaired by error checking. Tried this first and wasn't happy at all. Even if error handling is wrapped into macros.
  • All GHOST_Xr resources are managed via RAII. So stack unwinding will cause them to be released cleanly whenever an exception is thrown.
  • GHOST_Xr has a clear boundary (the Ghost C-API) with only a handful of public functions. That is the only place we need to have try-catch blocks at. (Generally, try-catch blocks at kinda random places are a bad code smell IMHO. Module boundaries are a valid place to put them.)
  • Exceptions allow us to pass multiple bits of error information through mulitple layers of the call stack. This information can also be made specific with a useful error message. As of now, they conain a user error message, the OpenXR error code (if any), as well as the exact source code location the error was caught at.

So the strategy I went with works as follows:

  • If a VR related error occurs within GHOST_Xr, throw an exception (GHOST_XrException currently).
  • OpenXR calls are wrapped into a macro throwing an exception if the return value indicates an error.
  • Useful debugging information and user messages are stored in the exceptions.
  • All resources must be managed through RAII, so throwing an exception will release 'dangling' ones cleanly.
  • In the GHOST C-API wrappers, the exceptions are caught and contained error information is forwarded to a custom error handling callback.
  • The error handling callback is set in wm_xr.c, prior to creating the XR-Context, and implements clean destruction of the context.

Notes on the GHOST_Xr-API

Early on, I decided to do the OpenXR level access through GHOST. Main reasons:

  • OpenXR requires access to low level, OS dependent graphics lib data (e.g. see XrGraphicsBindingOpenGLXlibKHR)
  • Some C++ features appeared handy (std::vector, RAII + exception handling, cleaner code through object methods, etc.)
  • General low level nature of the OpenXR API

After all I think the functionality is too high level to live in GHOST however.
My proposal would be to add a new module to intern/ instead, named VAMR (virtual, augmented and mixed reality).
Getting the low level graphics lib data out of GHOST is tricky though. Maybe the best option is to add something like a GHOST_OpenXRGraphicsBinding class called from VAMR.

I'm not sure if this is something that should be done prior to merging, or if it's fine to plan this for further work.

Diff Detail

Repository
rB Blender
Branch
soc-2019-openxr
Build Status
Buildable 4582
Build 4582: arc lint + arc unit

Event Timeline

Added some inline notes for reviewers.
Note that I also plan to add/improve comments on API functions.

intern/ghost/intern/GHOST_Context.cpp
153–155

This is not used and I only added it for comparison with the implementation in GHOST_ContextD3D. It's not trivial to implement but handy to have. So leaving it in for now.

intern/ghost/intern/GHOST_ContextD3D.cpp
44

SharedOpenGLContext only allows one shared resource per DX context currently. And we have to re-register it all the time to make sure it's updated.
Instead a shared resource handle should be requested from external code which has ownership then and passes the handle to the DX context to use it. The implementation of the resource handle can be made smarter to only re-register when needed.

This is my main TODO currently.

intern/ghost/intern/GHOST_ContextGLX.cpp
276

OpenXR bindings for GLX need a GLXFBConfig handle, so we have to keep it around.

intern/ghost/intern/GHOST_ContextWGL.cpp
146–173 ↗(On Diff #17295)

Only needed because I use the default framebuffer for offscreen drawing. I wanted to avoid the overhead of creating additional framebuffers and attachements for them. Considering that ugly things like this are needed then, it may be better to use a different framebuffer though.

intern/ghost/intern/GHOST_SystemWin32.cpp
300–321

Only needed for setDefaultFramebufferSize to work without issues. See comment about it.

419–422

Again, see comment for setDefaultFramebufferSize.

intern/ghost/intern/GHOST_Xr.cpp
42–43

This comment only applies if we keep the GHOST_Xr-API. If we implement the VAMR proposal this should be handled slightly differently.

intern/ghost/intern/GHOST_XrSession.cpp
122–132

We'll need a proper way to define a VR reference space, e.g. a floor plane with a view origin. This is not in scope of this project, so for now we just use the world origin and an identity quaternion, and apply the pose OpenXR calculates to the camera matrix.
Maybe I should add a comment here.

release/windows/batch/blender_oculus.cmd
2–7

Will polish this a bit and fix the typos.

source/blender/editors/space_view3d/view3d_draw.c
1725–1727

Seems like there are actually some overlays that can't be disabled this way. Will check. (Would prefer to have the grid floor enabled either way, it's a useful orientation helper).

tests/python/bl_load_py_modules.py
207–208

IDE reformatted this, will remove from patch.

build_files/cmake/Modules/presentation.cmake
2–102

The entire file is copied 1:1 from the OpenXR SDK (I should add a license header). It prints a message to the CMake console which is a bit annoying. Didn't want to patch the file though.

build_files/cmake/Modules/xr_platform_defines.cmake
2–47

This is taken from the OpenXR SDK, but manually taken from a CMakeLists file. Will add a comment explaining what to take from there for lib updates. Might also propose to the SDK devs to put this into a separate file.

Julian Eisel (Severin) edited the summary of this revision. (Show Details)Aug 20 2019, 1:56 PM
Julian Eisel (Severin) edited the summary of this revision. (Show Details)Aug 20 2019, 2:13 PM
Julian Eisel (Severin) edited the summary of this revision. (Show Details)Fri, Aug 23, 4:24 PM
Julian Eisel (Severin) updated this revision to Diff 17444.
  • Windows/deps: Add/fix openxr_sdk dependency
  • Fix missing CMake hint for OpenXR SDK path on linux
  • Refactor OpenGL/DirectX resource sharing for multiple shared resources
  • Remove GHOST API functions for OpenGL offscreen blitting
  • DirectX: Create an own render-target, don't use swapchain for blitting
  • Don't create a DirectX swap-chain
  • Improve batch file text for starting Blender with Oculus support
  • Cleanup: Unused functions, add comments, sync to master
  • Address and remove some TODOs marked in code

This update includes a refactor of the DirectX resource sharing which
gives a significant speedup. In the classroom scene, I get from ~48FPS
to ~100FPS (that is, if extrapolated from frame render time, ignoring
blocking xrWait calls for frame rate syncing).

Julian Eisel (Severin) marked 8 inline comments as not done.Fri, Aug 23, 4:31 PM
  • Avoid redundant framebuffer resize and offscreen texture drawing
  • Add Monado to the list of known runtimes
  • Remove blitting from default framebuffer
  • Remove hack to resize the usable default framebuffer region
  • Avoid OpenGL context deactivation, just to reactivate it immediately
  • Own FBO for VR viewport to get previous changes to work
  • Fix warning in release builds
  • Cleanup: Refactor (hacky) swapchain image submission
  • Add Oculus to the list of known runtimes
  • Don't drwa relationship lines in the VR view

This basically refactors how we submit frames to the OpenXR swapchain.
It removes some hacks and improves performance further.
With this, I'd consider the patch ready.

intern/ghost/intern/GHOST_Context.cpp
153–155

Decided to remove this.

Julian Eisel (Severin) edited the summary of this revision. (Show Details)Sat, Aug 24, 11:37 PM
Dalai Felinto (dfelinto) requested changes to this revision.Tue, Sep 3, 6:05 PM

Note I did not test make deps, only install_deps.sh (which need a fix, mentioned in place).

Did a first pass all the way to BLI_math_geom.h. I will continue it later.

build_files/build_environment/install_deps.sh
2843
-  _inst_shortcut=$INST/openxr-sdk
+  _inst_shortcut=$INST/openxr

Otherwise it doesn't match the generated openxr.conf (in ld.so.conf.d).
Also that will then to require update FindOpenXR-SDK and other parts of the code that use openxr-sdk.

build_files/cmake/platform/platform_unix.cmake
422

Nitpick:

- OpenXR-SDK was not found (...)
+ OpenXR-SDK not found (...)

Now less nitpicking, if there is no SDK yet you tried to build with OpenXR on, I think it should fail to build, not throw an error.

intern/ghost/intern/GHOST_C-api.cpp
913

Please follow style guideline (comments with /* ... */)

intern/ghost/intern/GHOST_ContextD3D.cpp
44

What does it mean? That we cannot have a shared session where we have more than one OpenXR device operating at the same time? (assuming OpenXR even supports that)

intern/ghost/intern/GHOST_XrContext.cpp
295

Extra space between functions.

380

When not using BLI_assert I like to at least have a descriptive assert like: assert(!"No OpenXR graphic extension found");

399

Full period in the end of comment.

456

Nitpicking, add space (strange that clang-format didn't pick that up).

intern/ghost/intern/GHOST_XrEvent.cpp
29

Comment style

46

Comment style.

52

style, remove extra line

intern/ghost/intern/GHOST_XrGraphicsBinding.cpp
133

Comment style

199

Please use non-abbreviation in messages: -Min +Minimum

248

I would leave the comment but remove the code. The code will get outdated very fast, and to maintain code that doesn't run nor work is not fun,

intern/ghost/intern/GHOST_XrSession.cpp
43

What is the challenge of implementing XR_VIEW_CONFIGURATION_TYPE_PRIMARY_MONO? I would expect it to be even more simple. And I don't see it mentioned anywhere (e.g., T67083).

Also comments formatting (I will stop commenting on this).

122–132

It should be added as a TODO in the comment even. For me this (and lack of scale control of the experience) will be the first thing we need to tackle to even start making this useful.

In my addon I would use the camera as the 0,0,0. It actually works well (and Unity does the same).

410

So what you have is not working? Why the commented out code, is that supposed to be the correct approach?

Either way, it deserves a fix or a TODO(...) properly commented.

436

How can you even tell? It would be nice to document it somewhere when trying new HMDs what one should be looking at.
Also, has this been reported upstream? (is there a place to report this for WRM? Perhaps the OpenXR repo itself?)

This revision now requires changes to proceed.Tue, Sep 3, 6:05 PM
build_files/cmake/platform/platform_unix.cmake
422

Not sure if we ever do this for libs that aren't a hard requirement (fail if a lib is not found even though enabled), the way I did it should be the convention. E.g. see OSD above.

intern/ghost/intern/GHOST_ContextD3D.cpp
44

No it was relating to the DirectX-OpenGL resource sharing, required to output to a DirectX buffer. The comment is marked as Done, I refactored the entire resource sharing, see GHOST_SharedOpenGLResource.

intern/ghost/intern/GHOST_XrSession.cpp
43

There is no challenge, it should be simple. I'm just not sure if we have a use-case for it at all? Blender doesn't run on phones or tablets and the likes, are there any mono view XR devices that Blender runs on?

Either way, we could make this entirely supported (but untested I'm afraid :) ) at GHOST level, and just only enable stereo rendering through GHOST_XrSessionBeginInfo.

122–132

Sure can mark as TODO.
For me this is a UI design question (not in scope of this patch) and I agree, it's one of the first ones to find an answer to.

410

From what I can tell the commented out code does correct conversion between coordinate systems, however it only works without conversion - which is weird.

436

It's really obvious if you see it, basically everything is waay to dark. But yes, should be noted in a comment.
Troy concluded the issue was on WMR's side. I've been in direct contact with Alex Turner (you may have seen him giving a HoloLens demo at Siggraph) and he seemed very motivated to fix it. But I wanted to reconfirm it's really WMR being broken, not Oculus, before making more fuzz about it.

Alright, first pass finished. All in all I'm happy with how isolated the XR code is.
And I hope we can get to the integration part soon.

The one thing I'm missing and would propose, is to have in DNA/rna settings for the VR section. Even if the UI is only exposed via the addon. For now I would include just the bare minimum:

  • Shading mode
  • Shading flags (minus V3D_SHADING_SPECULAR_HIGHLIGHT, plus V3D_SHADING_WORLD_ORIENTATION)
  • Draw flags
  • Clipping

Because even without jumping into the UX discussion having this allow for more people to try the performance in production scenes with more or less drawing complexity.

source/blender/blenlib/intern/math_geom.c
4585

My suggestion, just remove the rest of the function, and call perspective_m4() with the tangents as arguments.

Also are you sure your mat[0][0] and mat[1][1] don't need to be multiplied by nearClip?

source/blender/editors/include/ED_view3d.h
581

We should have a comment either here or (preferably) in the C file with the difference between ED_view3d_draw_offscreen_simple and ED_view3d_draw_offscreen.

source/blender/editors/space_view3d/view3d_draw.c
1699

Comments style even for single lines.

source/blender/windowmanager/intern/wm_draw.c
751

Can't you just pass a flag to GPU_viewport_draw_to_screen_ex and handle the flipping there?

source/blender/windowmanager/intern/wm_xr.c
452

This lens setting is not really being used right?

As for clip_start/clip_end can we have 0.01f and 1000.0f? This is the default for v3d->clip_*.

That said, as soon as we expose this the better, either using the active camera and/or having a global scene vr setting.
That latter makes sense since we are using the scene shading settings for the VR viewport.

479

Why OB_SOLID only? I guess it is because we still don't have a place to set/edit the VR viewport, right?