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

Lubos VR180 immersive mode #32

Merged
merged 17 commits into from Jan 30, 2024
Merged

Lubos VR180 immersive mode #32

merged 17 commits into from Jan 30, 2024

Conversation

amwatson
Copy link
Owner

This changeset adds Lubos' VR180-style immersive-mode -- behind an experimental feature -- to the core CitraVR project

amwatson and others added 6 commits January 24, 2024 19:57
…t if 5 is a special constant or based on resolution)
This request can cause issues if the user denies the microphone permission.
Calling requestPermissions causes GrantPermissionsActivity to be launched.
Citra's MainActivity will pause. GrantPermissionsActivity will immediately
finish as the user has already denied the permission. Citra's MainActivity then
gets resumed and then tries to request the permission again forming a loop.
 Move request for microphone permission to onCreate
@amwatson amwatson linked an issue Jan 25, 2024 that may be closed by this pull request
1 task
Copy link

@lvonasek lvonasek left a comment

Choose a reason for hiding this comment

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

LGTM, I added two minor comments.

@@ -300,4 +300,6 @@
<string name="vr_extra_performance_mode">VR Extra Performance Mode</string>
<string name="vr_cpu_level">CPU level</string>
<string name="vr_cpu_level_description">Higher levels may improve audio/emulation performance, but will drain the battery faster</string>
<string name="vr_immersive_mode_title">(Experimental): VR180 Immersive Mode</string>
<string name="vr_immersive_mode_description">Extends the top screen around a cylinder for VR180-style gameplay (Note: resolution is low and cannot be changed)</string>

Choose a reason for hiding this comment

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

It would be good to mention that extending the screen changes the depth value.

: panelHeight + verticalBorderTex + 2 * panelWidth / 5 - 4 * 5;
layer.subImage.imageRect.extent.width =
!useImmersiveMode_ ? panelWidth - cropHoriz : (panelWidth - 90 * resolutionFactor_) / 5;
layer.subImage.imageRect.extent.height = !useImmersiveMode_ ? panelHeight : panelHeight / 5;

Choose a reason for hiding this comment

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

Probably worth of a comment like this:

//TODO:As a sideeffect useImmersiveMode_ changes the viewport for the bottom screen.
When applying useImmersiveMode_ on the bottom screen is gone this cropping could be removed.
Right now the bottom screen is more pixelated than it should.

Uses a smaller radius screen which fits just within the view bounds and therefore has a higher resolution, but still feels "immersive". I also lowered the lower screen so it doesn't obscure the smaller main panel.
@amwatson amwatson force-pushed the master branch 2 times, most recently from e528693 to b562399 Compare January 27, 2024 02:02
@lvonasek
Copy link

lvonasek commented Jan 27, 2024

I see you removed my variant :-( Pity. I just tested it and the immersion is much lower. You shouldn't call it VR180 anymore as the view range is below 180°.

DrBeef and others added 2 commits January 28, 2024 23:07
This means the same shaders can be used for both immersive and non-immersive as the uniform itself it the toggle in the GLSL rather than generating two different instances of the shader.
Furthermore, only one place where the gl_Position was being modified was actually needed; the other two seem to be able to be removed without it preventing immersive mode from working.
Pass the vr_use_immersive_mode as a Vertex Shader Uniform flag
@amwatson
Copy link
Owner Author

I see you removed my variant :-( Pity.

I mean, it was effectively the same one, just a different constant. Just added it back, though (passing the scale factor as a uniform)

I was using "VR180" because I thought that's what you were calling it. I'll rename it to "Immersive Mode" because that avoids any specific connotations

@lvonasek
Copy link

I mean, it was effectively the same one, just a different constant.

I know it is just a constant but it makes a difference between a "very big screen" and "VR-like experience".

Just added it back, though (passing the scale factor as a uniform)

Great. I do not see it in the code, I guess it is not pushed yet. Thank you :)

@amwatson amwatson merged commit 5cab348 into master Jan 30, 2024
@amwatson amwatson deleted the lubos_immersive_mode branch January 30, 2024 03:24
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.

Multiview
4 participants