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

OpenXR - 6DoF support #15768

Merged
merged 41 commits into from
Aug 24, 2022
Merged

OpenXR - 6DoF support #15768

merged 41 commits into from
Aug 24, 2022

Conversation

lvonasek
Copy link
Contributor

@lvonasek lvonasek commented Jul 31, 2022

Introduction

In PR #15659 I added support for OpenXR on Oculus Quest VR headsets. Intent of this PR is to add 6 dimensions of freedom for head movement (so players can look arround in the game). For now I use so called mono VR (the frames for both eyes are the same).

Tasks for this PR

  • Integrate head rotation
  • Integrate head movement
  • Render HUD and game layers separately

Tasks for the future PR(s)

  • Do not use OPENXR define (code refactor)
  • Geometry culling problem (probably not universally solvable)
  • 3D stereo view (this will be optional and will cost some performance)

Related #11979, #15427

@lvonasek lvonasek changed the title [WIP] Feature OpenXR 6DoF support [WiP][OpenXR] 6DoF support Jul 31, 2022
@hrydgard
Copy link
Owner

Cool! I'll review this weekend.

Copy link
Owner

@hrydgard hrydgard left a comment

Choose a reason for hiding this comment

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

Looks fairly close to be able to get it in but will need some refactoring either before or after merge as some comments indicate.

Also, it's a bit unfortunate to build yet another short-vector math lib, we already have one or two in the code base, but it's ok for now, can be refactored later.

Really want to minimize the ifdef usage, since VR support will be a runtime toggle on Windows.

And as I mentioned in one comment, the more logic we can put in GPU/Common as opposed to GPU/GLES, the better, since I want VR to work with Vulkan eventually (where I think we can do stereo more efficiently than in GL, though there might be enough multiview GL extensions?).

Core/HLE/sceDisplay.cpp Outdated Show resolved Hide resolved
GPU/Common/VertexShaderGenerator.cpp Show resolved Hide resolved
GPU/GLES/ShaderManagerGLES.cpp Show resolved Hide resolved
@lvonasek
Copy link
Contributor Author

Looks fairly close to be able to get it in but will need some refactoring either before or after merge as some comments indicate.

Thanks for review. Do you require to do the changes as part of this PR or could we continue resolving it in the future PR(s)?


Also, it's a bit unfortunate to build yet another short-vector math lib, we already have one or two in the code base, but it's ok for now, can be refactored later.

The main advantage of having it in the codebase is you find the same in OpenXR sample codes and it is very nice if you can copy&paste piece of code and test it there.


Really want to minimize the ifdef usage, since VR support will be a runtime toggle on Windows.

In this PR I removed the OPENXR ifdef from UI, I am going to remove it also from other places once I figure the best way to do that.


And as I mentioned in one comment, the more logic we can put in GPU/Common as opposed to GPU/GLES, the better, since I want VR to work with Vulkan eventually (where I think we can do stereo more efficiently than in GL, though there might be enough multiview GL extensions?).

AFAIK Vulkan has no benefit against GL from multiview estensions. I am close to make multiview working on GL:
https://github.com/lvonasek/ppsspp/tree/feature_openxr_stereo

There are only missing a few shader changes and passing matrices for both eyes.

@iota97
Copy link
Contributor

iota97 commented Aug 15, 2022

Just wondering, if we get proper stereo rendering (not really sure if this is doing it already tbh) it would be nice to be able to access the different eyes in post processing shaders. In this way even with no VR it would be easy to set up thing like side by side or Cyan-Red glasses.

Not really that important, just tossing it here :)

@lvonasek
Copy link
Contributor Author

Just wondering, if we get proper stereo rendering (not really sure if this is doing it already tbh) it would be nice to be able to access the different eyes in post processing shaders. In this way even with no VR it would be easy to set up thing like side by side or Cyan-Red glasses.

Not really that important, just tossing it here :)

The stereo is not ready yet. Stereo is an ongoing work in my other branch. There is going to be GL_TEXTURE_2D_ARRAY for the stereo framebuffer.

@lvonasek
Copy link
Contributor Author

Do you require to do the changes as part of this PR or could we continue resolving it in the future PR(s)?

This question got overlooked. I am putting a small bug fixes into this branch to prevent merge madness if I created many small PRs. What do you think @hrydgard?

@lvonasek lvonasek requested a review from hrydgard August 15, 2022 15:58
@hrydgard
Copy link
Owner

To answer your question that I didn't before, it does really bother me to have an "ifdef VR" right in the core of display timing.

Could accept some global variable though, which we set from elsewhere through an accessor function. Maybe g_InternalFrameRate. It could be interesting to experiment with different internal framerates even in non-VR (for example on 90Hz monitors).

@lvonasek
Copy link
Contributor Author

lvonasek commented Aug 17, 2022

Ok, I will do that.

@lvonasek
Copy link
Contributor Author

To answer your question that I didn't before, it does really bother me to have an "ifdef VR" right in the core of display timing.

2887671
Is this fine for you? If yes, I would really like to merge it.

@hrydgard
Copy link
Owner

hrydgard commented Aug 19, 2022

Technically this "/ 1001" thing is part of the weird framerate 59.94 hz which is common due to legacy reasons. I don't think the Quest has a 71.93 or whatever framerate, for example. Though I guess that part hardly matters...

I'm sorry I haven't had time to fully review the latest version of this yet, but I will very soon.

@lvonasek
Copy link
Contributor Author

Thank you, I will be away for about a week, so I so I will not be able to do any changes during that time.

Let me share my plans a bit:
After this PR I plan three huge changes:

  • Stereoscopy with multiview (this is already WIP but it is bigger than I thought because I will have to add code to framebuffers in PPSSPP code to support multiview)
  • Getting rid of OPENXR ifdefs (I tried to do it already but I just found a way how to do it wrong, next time I will do it properly)
  • Support for another Android VR headset (I plan to add support for on Pico Neo headsets, then it should be much easier to add another OpenXR device support)

Afterwards there should be just small PRs coming fixing VR support for specific games (mostly 2D/3D rendering separation which causes 99% of VR problems).

@hrydgard
Copy link
Owner

Sounds great!

As for the game specific tricks, we're gonna have to systemize it and cleanly separate it from the main emulation code as much as possible.

@lvonasek
Copy link
Contributor Author

lvonasek commented Aug 20, 2022

Agreed 💯. Once you merge this PR, I will create the draft for stereo PR. As the next PR will be full-feature VR version, I will do the OPENXR removal there.

Copy link
Owner

@hrydgard hrydgard left a comment

Choose a reason for hiding this comment

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

Alright, I'm happy with this now as an intermediate step.

There will be cleanup required later (some of which I will help out with) but it's a good step forward and not too intrusive.

@hrydgard hrydgard merged commit 416d8b4 into hrydgard:master Aug 24, 2022
@lvonasek lvonasek deleted the feature_openxr_6dof branch August 24, 2022 17:49
@lvonasek lvonasek mentioned this pull request Aug 24, 2022
5 tasks
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.

4 participants