-
Notifications
You must be signed in to change notification settings - Fork 220
Enable Oculus Fixed Foveated Rendering #654
Enable Oculus Fixed Foveated Rendering #654
Conversation
// Enable fixed-foveated rendering. | ||
// TODO - This should be user configurable. VRAPI_SYS_PROP_FOVEATION_AVAILABLE | ||
// can be checked to determine if the option should be presented. | ||
vrapi_SetPropertyInt(&java, VRAPI_FOVEATION_LEVEL, 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to enable this only for the chrome scene. You can do that viaDeviceDelegateOculusVR::SetRenderMode()
According to the spec foveated rendering for WebVR should be up to the JS code right? via WebGL extensions or WebXR when available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, we should probably have this preference affect the browser's 2d modes separately.
There is no defined API for setting foveated level in the WebXR spec yet. Unlike MRS (Mixed resolution shading) and LMS (Lens Matched Shading), this kind of fixed-foveated-rendering does not need changes to the content to function. Turning it on manually for immersive content could make the difference between being able to see some immersive sites and it not working. Perhaps we should give the user choice in this case.
@@ -172,6 +172,10 @@ struct DeviceDelegateOculusVR::State { | |||
vrapi_SetPropertyInt(&java, VRAPI_BLOCK_REMOTE_BUTTONS_WHEN_NOT_EMULATING_HMT, 0); | |||
// Reorient the headset after controller recenter. | |||
vrapi_SetPropertyInt(&java, VRAPI_REORIENT_HMD_ON_CONTROLLER_RECENTER, 1); | |||
// Enable fixed-foveated rendering. | |||
// TODO - This should be user configurable. VRAPI_SYS_PROP_FOVEATION_AVAILABLE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check VRAPI_SYS_PROP_FOVEATION_AVAILABLE
before calling vrapi_SetPropertyInt(&java, VRAPI_FOVEATION_LEVEL, 2);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially (my earlier patch), I was attempting to check VRAPI_SYS_PROP_FOVEATION_AVAILABLE before enabling the UI, but it would require many more changes (eg, getting the value asynchronously from the render thread while building the UI). Setting this value when it is not supported will simply be a NULL operation. I've hidden the UI when not using the Oculus API, but perhaps we could enhance this at a later time, together with better adaptive UI selections for other rendering features?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine the only case right now where VRAPI_SYS_PROP_FOVEATION_AVAILABLE would return false would be for GearVR / clip-in headsets using the Oculus API. In those cases with the current patch, the option would appear but not have any effect.
b05e3b1
to
d6a20ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work on adding this 👍
it's quite possible my eyes aren't trained well enough, but I can't discern any visual difference with two Oculus Go headsets side by side. what should I notice? what are good examples of objects to look at or any test cases to assess the difference?
<!-- This string is used to label the Foveated radio button that disables fixed-foveated-rendering in immersive mode. --> | ||
<string name="developer_options_foveated_disabled">Disabled</string> | ||
<!-- This string is used to label the Foveated radio button that enables low-level fixed-foveated-rendering. --> | ||
<string name="developer_options_foveated_1">1</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we call these Low
, Medium
, and High
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can only really notice the difference between #2 and #3. With #3, you can see the edges having a slightly lower resolution when you move the headset so something with contrast is near the top or sides.
I originally wanted to use "low", "medium", and "high" but the longer strings broke the UI causing overlaps. Perhaps we could improve this later with some changes to the developer options panel layout?
app/src/main/res/values/strings.xml
Outdated
@@ -102,6 +102,16 @@ | |||
<string name="developer_options_msaa_2">2x</string> | |||
<!-- This string is used to label the MSAA radio button that enables four times MSAA in immersive mode. --> | |||
<string name="developer_options_msaa_4">4x</string> | |||
<!-- This string is used to label radio buttons for setting fixed-foveated-rendering --> | |||
<string name="developer_options_foveated">Foveated Level</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a note that explains in a little bit more detail what this means. I assume this string will be difficult to translate for most.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added some more comments. I hope it helps!
I suspected "foveated" would be a non-localized term.
|
||
int32_t | ||
DeviceDelegateNoAPI::GetMaxFoveatedLevel() { | ||
// Return 0 to indicate that it is not supported |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can add puncutation to these so it's similar to the punctation in the comments elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed the GetMaxFoveatedLevel() related code and replaced it with a simple check for the Oculus build in DeveloperOptionsWidget.java. My original implementation was not thread safe, and would take a lot longer to fix. I'd like to see a better mechanism for selectively enabling platform specific UI in the options widget.
if (available == VRAPI_TRUE) { | ||
return 3; | ||
} | ||
return 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious: why do we return 0
instead of available
? what if it's 1
or 2
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code as been removed. It wasn't clear how we plan to make the options dialogue dynamic to support other headsets that also support foveated rendering, so I opted for a simpler solution for now in DeveloperOptionsWidget.java
@@ -127,6 +128,15 @@ public void onClick(View view) { | |||
mMSAARadio.setOnCheckedChangeListener(mMSSAChangeListener); | |||
setMSAAMode(mMSAARadio.getIdForValue(msaaLevel), false); | |||
|
|||
int foveatedLevel = SettingsStore.getInstance(getContext()).getFoveatedLevel(); | |||
mFoveatedRadio = findViewById(R.id.foveated_radio); | |||
if (!true) { // FINDME!! KIP!! HACK!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be:
if (BuildConfig.FLAVOR_platform != "oculusvr") {
ade3aa2
to
9bcc906
Compare
- Option to change value added to the developer settings UI. - Default with a value of 2 (0 = none, 3 = max).
9bcc906
to
19418e0
Compare
Sorry for the multiple pushes.. This final one seems to be working well. I've tested on Oculus Go and HTC Vive Focus. (On the Vive Focus, the foveated option disappears as intended) |
@kearwood could you rebase this PR with master? The other change required is disable/enable foveated when the user starts/exits a WebVR session. You could do that in SetRenderMode() based on the device::RenderMode::StandAlone or device::RenderMode::Immersive parameter |
@kearwood It'd be great to get this into 1.1.3. |
@kearwood Do you have time to work on this? I can work in the remaining bits if you want |
I'm still not clear on why we'd not just apply fixed foveation mode all the time? |
The WebXR spec says: implementations should not enable foveated rendering or performance optimizations that may negatively affect the correctness of an application unless the application explicitly opts-in to that behavior. |
True, except that we don't have WebXR yet, and according to @kearwood above, there is no defined API for setting foveated level in the WebXR spec yet. I think we should make the choice in favour of performance now and offer the user an option to enable/disable when WebXR lands. |
PR moved to #1064 |
A value of 2 was selected through experimentation on the Oculus Go. (0 = none, 3 = max).
Ideally, this would be a user-select-able value; however, a default of 2 seems reasonable as I could not notice any artifacts in the peripheral vision.