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: Use the XR_FB_foveation_vulkan extension to get the density map for VRS #99768

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Nov 27, 2024

This builds on top of PR #99551 and is consequently marked as a DRAFT until it can be merged

But instead of manually building the density map, it uses the XR_FB_foveation_vulkan extension to get the density map from the OpenXR runtime.

So, you can use the same foveation settings that are currently used for OpenGL (setting the "Foveation Level" or "Foveation Dynamic"), and that should get used by the OpenXR runtime in order to build the density map that we're using.

In order to do that, it adds XRInterface::get_vrs_texture_format() which the interface can return any of these:

  • XR_VRS_TEXTURE_FORMAT_UNIFIED: This means XRInterface::get_vrs_texture() uses the same format as previously, and it doesn't matter whether or not the underlying mechanism is FDM or FSR, it runs through the shader and is converted to the right format. I'm open to renaming this!
  • XR_VRS_TEXTURE_FORMAT_FRAGMENT_SHADING_RATE: The VRS texture is in the format expected by fragment shading rate. I don't know that we'll ever use it this, but it's here for completeness
  • XR_VRS_TEXTURE_FORMAT_FRAGMENT_DENSITY_MAP: The VRS texture is in the format expected by fragment density map, which is what XR_FB_foveation_vulkan will give us

If XR_FB_foveation_vulkan isn't supported, then the OpenXRInterface will fallback on the original "unified" texture that is generated within Godot, so VRS will continue to work as it has for any OpenXR runtime that doesn't support this extension.

@BastiaanOlij
Copy link
Contributor

I'm really on the fence about this one. Was just talking to @DarioSamo about density maps in general, what my ideas are for improving what we have right now, and if we do this right, our build in solution will be as good as, if not better than what is on offer here. And it works on any platform (well currently any Vulkan based platform).

So far the feedback I've gotten talking to people in the know, most game engines have already solved this the same way we have, with their own logic for generating density maps, because they ran into the same problems we did with what OpenXR offers. Most vendors this don't have a need to supporting something in the runtime. Especially when looking at PCVR vendors who are dealing with a wide hardware spectrum. That is not to say that in some point in the future we will see a vendor neutral extension for this but thats a big unknown.

So doing structural changes to our rendering engine to support an edge case currently only supported by Meta, and potentially not something that will be introduced by others, while we already have a viable solution that we can improve open is.. uhm.. Meh..

The other thing that is a potential problem here, is that the density map has to be adjusted to the resolution at which we're rendering, and since we're allowing that to be overridden, we might not be getting a density map at the right size.

@dsnopek
Copy link
Contributor Author

dsnopek commented Nov 29, 2024

I think there's a few advantages to using the density map from OpenXR:

  • The "dynamic" feature of foveated rendering works! I tested it by doing two RenderDoc captures: one where I was looking at a part of the scene where it tanked the FPS (down to like 36fps), one where I was looking at a single color quad (and fps went back up to 72fps). I could see that density map changed, so that it was really scaling down resolution in the first capture, but was a pure (1.0, 1.0, 0.0) texture in the second (so not scaling resolution down at all)
  • The density map can be geared to the hardware. When looking at RenderDoc with this PR, Dario said that the binning stage was much shorter with this PR than with the custom density map from his PR. He said something like he thought it had an easier time with the density map from OpenXR because it was "more binary" (ie less gradient, more large patches of a single value). But this is something that could be hardware specific - maybe different hardware would be able to work better with differently generated maps?
  • Because "dynamic" works, I suspect that XR_META_foveation_eye_tracked would work as well, allowing the OpenXR runtime to update the density map based on eye position, and it'd be able to use the right "filters" and latency for foveated rendering. Our manual density map generation can use XR_EXT_eye_gaze_interaction, but that really isn't the right extension for foveated rendering: it uses "filters" and latency that are designed for interacting with UIs, which means it will prioritize things like stability (ie not bouncing around too quickly), rather than prioritizing the things that matter for foveated rendering (like latency)

So, I'd really like to be able to have the option to let the OpenXR runtime generate the density map!

I'm not crazy about my implementation so far, because it couples a whole bunch of things that I don't want coupled, but that's hopefully something I can work out before taking this out of draft.

@dsnopek
Copy link
Contributor Author

dsnopek commented Nov 29, 2024

A point I forgot to respond to in my last comment:

So doing structural changes to our rendering engine to support an edge case currently only supported by Meta, and potentially not something that will be introduced by others, while we already have a viable solution that we can improve open is.. uhm.. Meh..

We shouldn't have to make any structural changes to the renderer to do this! All the rendering changes in my commit on this PR are just temporary hacks in order to test this - it's only a draft :-)

In the end, I expect all changes for this to be in the "openxr" module. The renderer is already calling XRInterface::get_vrs_texture() to get the texture - it's just a question of where we get that texture from? If this extension is available, and the developer enabled foveated rendering in project settings, we can return the one from the extension; if not, we generate one manually.

@dsnopek dsnopek marked this pull request as draft November 29, 2024 13:22
@BastiaanOlij
Copy link
Contributor

@dsnopek the problem is, the solution Dario and I were discussing tonight kind of changes things a lot. What we're thinking of is to leave providing a texture for non XR usage only, and instead provide eye center and radius data if XR_VRS is specified, and generating the density map in the shader that is currently in the rendering engine (and eventually move that into the rendering driver so that we're independent of hardware/GPU architecture). This removes a bunch of overhead and solves a bunch of problems with an externally supplied density map not matching our further rendering settings. It's not a big step to make the density map adjust based on frame timing.

I do hear what you're saying, but many of those issues are because we need to improve our implementation, and so far its just not gotten the attention it deserves. Solving that properly means that we have a good solution for all platforms, and seeing this extension does not seem to be getting wide adoption, we're going to have to do that anyway.
There is a difficulty that right now those with the knowhow, aren't sharing, so its going to take more experimentation to get it right. But its worth doing.

My only concern is that right now, there is no proper extension in OpenXR that allows us to get usable eye tracking data, so supporting eye tracked foveated rendering alongside fixed foveated rendering is a problem. We're using the eye gaze extension which is not really suitable for this.
That is definitely a place where using this extension is a plus.

@dsnopek dsnopek force-pushed the openxr-vulkan-foveated-rendering branch from c646106 to 177d119 Compare December 5, 2024 15:30
@akien-mga akien-mga modified the milestones: 4.4, 4.x Feb 24, 2025
@dsnopek dsnopek changed the title [DRAFT] OpenXR: Use the XR_FB_foveation_vulkan extension to generate the density map for VRS [DRAFT] OpenXR: Use the XR_FB_foveation_vulkan extension to get the density map for VRS Mar 24, 2025
@dsnopek dsnopek force-pushed the openxr-vulkan-foveated-rendering branch 3 times, most recently from d665edb to 7f9324e Compare March 24, 2025 20:43
@dsnopek
Copy link
Contributor Author

dsnopek commented Mar 24, 2025

Previously, this PR just hacked the renderer to use the FDM from OpenXR. I've just updated it into a non-hacky form, that will hopefully be an acceptable approach - details are added to the PR description.

(I'm going to wait until either PR #99551 is merged or rebased to deal with the merge conflicts - they shouldn't be a big deal, but I don't want to diverge from that PR, so it's easy to update for any changes)

@dsnopek dsnopek force-pushed the openxr-vulkan-foveated-rendering branch from 7f9324e to e9abe8d Compare March 28, 2025 14:11
@dsnopek dsnopek changed the title [DRAFT] OpenXR: Use the XR_FB_foveation_vulkan extension to get the density map for VRS OpenXR: Use the XR_FB_foveation_vulkan extension to get the density map for VRS Mar 28, 2025
@dsnopek
Copy link
Contributor Author

dsnopek commented Mar 28, 2025

@BastiaanOlij:

the problem is, the solution Dario and I were discussing tonight kind of changes things a lot. What we're thinking of is to leave providing a texture for non XR usage only, and instead provide eye center and radius data if XR_VRS is specified, and generating the density map in the shader that is currently in the rendering engine (and eventually move that into the rendering driver so that we're independent of hardware/GPU architecture).

I'm pretty sure we already talked about this outside of GitHub some months ago, but posting here so it's visible on GitHub: I don't think the changes here would impact implementing what you describe above. In the end, there are two paths that can happen: (1) Godot generates the FDM via some shader (whether that's based on a texture or input like eye center and radius), or (2) Godot grabs an already made FDM from XR_FB_foveation_vulkan and so doesn't run that shader at all. The existence of path nr 2 doesn't prevent path nr 1 from being iterated and improved upon.

And discussed above, I think there are a number of advantages of having path nr 2 be an option (and it is an option, if developers don't set xr/openxr/foveation_level then path nr 1 will still be followed), which are best summarized in this comment.

@dsnopek
Copy link
Contributor Author

dsnopek commented Mar 28, 2025

Now that PR #99551 is merged, I'm finally taking this one out of DRAFT!

@dsnopek dsnopek marked this pull request as ready for review March 28, 2025 14:22
@dsnopek dsnopek requested review from DarioSamo and m4gr3d March 28, 2025 14:34
@dsnopek dsnopek force-pushed the openxr-vulkan-foveated-rendering branch from 677218b to bf99c4f Compare March 28, 2025 16:25
Copy link
Contributor

@devloglogan devloglogan left a comment

Choose a reason for hiding this comment

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

There are still some notes in ProjectSettings.xml for Foveation Level / Foveation Dynamic that say they only work for the Compatibility renderer.

@dsnopek dsnopek force-pushed the openxr-vulkan-foveated-rendering branch from bf99c4f to 7d91d37 Compare March 28, 2025 17:53
@dsnopek
Copy link
Contributor Author

dsnopek commented Mar 28, 2025

There are still some notes in ProjectSettings.xml for Foveation Level / Foveation Dynamic that say they only work for the Compatibility renderer.

Thanks! I removed the notes that said the foveation settings only work on the compatibility renderer in my latest push

@dsnopek dsnopek force-pushed the openxr-vulkan-foveated-rendering branch from 7d91d37 to 76af8c5 Compare March 28, 2025 18:12
Copy link
Contributor

@devloglogan devloglogan left a comment

Choose a reason for hiding this comment

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

Some other minor things but otherwise I think the XR code is looking good! I tested the changes out locally on my Quest 3 and even took a look at the different textures in renderdoc and it worked/looked as I expected.

Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

The OpenXR code looks good.

@dsnopek dsnopek force-pushed the openxr-vulkan-foveated-rendering branch from 76af8c5 to b9294f9 Compare March 31, 2025 12:19

Verified

This commit was signed with the committer’s verified signature.
dalexeev Danil Alexeev
… map for VRS
@dsnopek dsnopek force-pushed the openxr-vulkan-foveated-rendering branch from b9294f9 to 79f5a4d Compare March 31, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants