Page MenuHome

GSoC 2019: Core Support of Virtual Reality Headsets through OpenXR
AbandonedPublic

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 [[https://developer.blender.org/diffusion/B/history/soc-2019-openxr/ | soc-2019-openxr]] branch.
Changes from the [[https://developer.blender.org/diffusion/B/history/temp-vr-draw-thread/ | 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 [[https://developer.blender.org/diffusion/BA/history/soc-2019-openxr/ | 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 5386
Build 5386: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
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
311–332

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

424–427

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
1732–1734

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
209–210

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)
  • 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.Aug 23 2019, 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)Aug 24 2019, 11:37 PM
Dalai Felinto (dfelinto) requested changes to this revision.Sep 3 2019, 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
2853
-  _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
433

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
971

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
296

Extra space between functions.

381

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

400

Full period in the end of comment.

457

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

intern/ghost/intern/GHOST_XrEvent.cpp
30

Comment style

47

Comment style.

53

style, remove extra line

intern/ghost/intern/GHOST_XrGraphicsBinding.cpp
134

Comment style

200

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

249

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
44

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).

411

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.

437

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.Sep 3 2019, 6:05 PM
build_files/cmake/platform/platform_unix.cmake
433

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
44

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.

411

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

437

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
4669

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
580

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
1706

Comments style even for single lines.

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

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
453

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.

480

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

Julian Eisel (Severin) marked 20 inline comments as done.
  • Fix mistakes in openxr build options
  • Fix Linux linker config name for the OpenXR-SDK
  • CMake: Slight adjustment to warning message
  • Cleanup: Comment style
  • Cleanup: Don't pass unused lens value
  • Strictly follow specification to get OpenXR extension functions
  • Support VR Session settings & add some basic ones
  • Wrap WM XR data into own struct
  • Improve error messages & cleanup
  • Cleanup: Avoid duplication of projection matrix calculation
  • Correct/remove disabled code for space conversion
  • Refactor GPU viewport onscreen drawing for "upside down" drawing
  • Fix too dark rendering on the Monado runtime
build_files/build_environment/install_deps.sh
2853

I fixed this differently so that we can keep the -sdk suffix. I prefer to make clear we're actually compiling sources from the SDK.

intern/ghost/intern/GHOST_XrContext.cpp
296

I like to keep functions that are closely related visually connected, without extra space. Our clang-format config doesn't forbid this, I suppose for this exact reason.

457

See comment above regarding this.

intern/ghost/intern/GHOST_XrGraphicsBinding.cpp
249

I'll leave the code in for a bit, just because it might be handy for testing on different hardware.
But besides that agreed, we should not keep this around for longer than useful.

intern/ghost/intern/GHOST_XrSession.cpp
411

Turns out we don't have to perform any conversion, the specification is unclear about the fact that its space description is actually for view local space.
I'm certain that I've checked if this was the case (I remember the very moment I tested this...), but I must have messed something up doing so.

Anyway, the Blender and the OpenXR space use the same coordinate system, the specification could just describe things better.
Removed the conversion code.

437

Jeroen and I plan to look at this issue in depth after the conference. Either the Oculus, or the WMR and Monado runtimes have a messed up color pipeline.

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

When multiplying them by nearClip the output is heavily distorted, so it seems we shouldn't apply this. The OpenXR reference implementations don't apply it either, which this implementation was based on.

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

No, the lens would only be used if we didn't pass pre-calculated matrices to the viewport drawing. The clipping default is now taken from the View3D default initialization.

Alright, Jeroen just came over and we looked into the color pipeline issue in detail.

His analysis confirms what Troy was saying, WinMR and Monado don't manage colors correctly. He also agreed that what I'm doing, applying an sRGB OETF transform on the completed offscreen buffer for these runtimes, is the best way to go about it for now. To our understanding of the OpenXR specification, the runtimes should do this part, but those two fail to do so. (Strictly speaking they should apply whatever OETF transform their device needs, but for the foreseeable future that's likely sRGB for all devices.)

Hello, I'm one of the developers on Monado. Color space conversion isn't one of my strong sides, and I will admit that we could be doing things wrong inside of Monado. Right now we expose two swapchain formats GL_SRGB8_ALPHA8, and GL_RGBA8[1]. Which are converted to VK_FORMAT_R8G8B8A8_SRGB and VK_FORMAT_R8G8B8A8_UNORM in the Vulkan side of the compositor[2]. We are also by default using a sRGB non-linear color space for the swapchain[3], so it should be all working as long as you use the right format.

[1]: https://gitlab.freedesktop.org/monado/monado/blob/master/src/xrt/compositor/client/comp_gl_client.c#L237-238
[2]: https://gitlab.freedesktop.org/monado/monado/blob/master/src/xrt/compositor/client/comp_gl_client.c#L156-157
[3]: https://gitlab.freedesktop.org/monado/monado/blob/master/src/xrt/compositor/main/comp_settings.c#L34

intern/ghost/intern/GHOST_XrGraphicsBinding.cpp
134

I think you want to use GL_SRGB8_ALPHA8 here otherwise you are indicating that the pixel data is linear.

Split the patch in multiple smaller chunks now: D6188, D6190, D6192, D6193.
Review will happen in these. Parent task to follow updates is T71365.

Closing this, thanks all!


@Jakob Bornecrantz (wallbraker) got a bit too much to care for right now, will try to come back to you soon. This not something we can easily communicate in two sentences, I'll have to show you the details leading to our conclusion.