Client-side OpenXR post-processing #2808
Conversation
|
Thank you so much for opening the PR! |
alvr/client_openxr/src/graphics.rs
Outdated
| composition_layer_settings: Option<&'a xr::sys::CompositionLayerSettingsFB>, | ||
| ) -> Self { | ||
| Self { | ||
| reference_space, | ||
| layers, | ||
| alpha, | ||
| composition_layer_settings, | ||
| } |
There was a problem hiding this comment.
Here I meant you should pass our struct ClientsidePostProcessingConfig, and then construct CompositionLayerSettingsFB inside here. To convert Switch to Option you can use into_option() or as_option()
There was a problem hiding this comment.
I figured I could build the CompositionLayerSettingsFB only once when the StreamContext is initialized on streaming start, and reuse it on every frame, like is done for other xr objects stored in the StreamContext.
Do you think it would be preferrable to pass the config struct and build a new layer settings on each frame?
There was a problem hiding this comment.
Yes, construct CompositionLayerSettingsFB inside the layer builder each time, in new()
There was a problem hiding this comment.
I see, I can do that if you wish. But I feel like this will just add a bit of overhead to rebuild the layer settings every frame, instead of building it once and for all and keeping it saved in the stream context. Do you fear that reusing the same object may have an adverse effect in the long run? It seems to work pretty well as it is.
Unless the settings can be updated by the dashboard and sent dynamically to the headset without restarting the stream? In which case, yes, rebuilding it would avoid running on "obsolete" settings, if the user changes the post processing options on the fly.
There was a problem hiding this comment.
I think keeping the same struct instance really makes no difference performance-wise. My advice is purely to avoid leaky abstractions. xr::sys structs deal with raw pointers and sometimes they are platform-specific (FB in this case), which i prefer them to be hidden at lower levels of abstractions
There was a problem hiding this comment.
Makes sense, I moved its initialization into the projection layer builder.
|
Are there still things I need to change or fix? |
zmerp
left a comment
There was a problem hiding this comment.
Overall I would say this PR is mergeable as is. @The-personified-devil let me know if you'd like to drop a review or comment
| let composition_layer_settings = clientside_post_processing_config | ||
| .map(|post_processing| { | ||
| xr::CompositionLayerSettingsFlagsFB::from_raw( | ||
| (post_processing.sharpening as u64) | (post_processing.super_sampling as u64), |
There was a problem hiding this comment.
Half-nit, it's not intuitive how this works if you don't know that post_processing.sharpening and post_processing.super_sampling are actually compatible bitfields. it's probably inferred from the context (we are filling in flags)
* Client-side OpenXR post-processing * Remove unnecessary closure * Refactor composition layer settings creation and reference passing * Add client-side post-processing to real-time config * Build composition layer settings struct within projection layer builder
* Client-side OpenXR post-processing * Remove unnecessary closure * Refactor composition layer settings creation and reference passing * Add client-side post-processing to real-time config * Build composition layer settings struct within projection layer builder
This is an implementation of Meta’s composition layer filtering OpenXR extension.
It simply consists in detecting said extension, enabling it if available, and adding a set flags on each submitted composition layer to specify the desired level of super sampling and/or sharpening.
I tested it successfully on my Quest 3, and from tests on other OpenXR tools, this should also work at least on Pico headsets.
If the extension is unavailable on the target headset, or if both post-processing settings are left to Disabled in the dashboard, it will fallback to the current existing behavior.
This is my first time working with production-level Rust code, so please correct me on all my non-idiomatic code!