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

highRefreshRate property to enable 72hz mode on Oculus Browser (fix #3943) #3967

Merged
merged 2 commits into from
Feb 7, 2019

Conversation

dmarcos
Copy link
Member

@dmarcos dmarcos commented Jan 24, 2019

the highRefreshRate flag on requestPresent does not seem to have any effect. Enabling high Refresh Rate on chrome://flags does work as expected.

@DigiTec Am I doing things correctly? I added a console message that shows the frame time.

This is WIP simplified for evaluation. The final API will use the renderer component.

@Artyom17
Copy link
Contributor

Artyom17 commented Feb 3, 2019

Sorry @dmarcos, didn't see this question. Yes, what you are doing - seems correct to me (https://developer.oculus.com/documentation/oculus-browser/latest/concepts/browser-optimize-rendering/).
Note, that even if highRefreshRate attribute is not supported, it will be just ignored (by other browsers).

@dmarcos
Copy link
Member Author

dmarcos commented Feb 5, 2019

@Artyom17 No problem. I know it's specific to Oculus Browser. Do you have an example with the highRefreshRate flag set to true that works? Is it a known browser bug or something in our end?

@Artyom17
Copy link
Contributor

Artyom17 commented Feb 6, 2019

@Artyom17 No problem. I know it's specific to Oculus Browser. Do you have an example with the highRefreshRate flag set to true that works? Is it a known browser bug or something in our end?

This example works for me on Go: https://artyom17.github.io/webvr.info/samples/03-vr-presentation-72Hz.html (source https://github.com/Artyom17/webvr.info/blob/gh-pages/samples/03-vr-presentation-72Hz.html).
var attributes = { highRefreshRate: true, }; vrDisplay.requestPresent([{ source: webglCanvas, attributes: attributes}]).then(function () ....

@dmarcos
Copy link
Member Author

dmarcos commented Feb 6, 2019

@Artyom17 Thanks. I see. It seems I have to pass the flag in an object on attributes property. That seemed to be my mystake.

Do you recommend us to default to 72Hz on Oculus Go or it should be opt-in for developers?

@Artyom17
Copy link
Contributor

Artyom17 commented Feb 6, 2019

@dmarcos I'd make it optional because of one issue - not all experiences would be able to make 72 fps on Go.

@dmarcos
Copy link
Member Author

dmarcos commented Feb 6, 2019

@Artyom17 Thanks for the feedback

@dmarcos dmarcos force-pushed the fps72 branch 2 times, most recently from d54a511 to 670c417 Compare February 7, 2019 02:20
@dmarcos dmarcos changed the title Set highRefreshRate (72fps) on Oculus Go (fix #3943) highRefreshRate property to enable 72hz mode on Oculus Browser (fix #3943) Feb 7, 2019
Copy link
Contributor

@Artyom17 Artyom17 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Member

@ngokevin ngokevin left a comment

Choose a reason for hiding this comment

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

r+wc

@@ -29,6 +29,7 @@ The `renderer` system configures a scene's
|-------------------------|---------------------------------------------------------------------------------|---------------|
| antialias | Whether to perform antialiasing. If `auto`, antialiasing is disabled on mobile. | auto |
| colorManagement | Whether to use a color-managed linear workflow. | false |
| highRefreshRate | Oculus Browser has a 72hz optional mode vs. the default 60hz | false |
Copy link
Member

Choose a reason for hiding this comment

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

Description can say what the property does a bit more directly.

Toggles 72fps/72hz on Oculus Browser. Will default to 60fps/60hz by default

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.

3 participants