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

Add VIEW_INDEX variable in shader so we know which eye/view we're rendering for #48011

Merged
merged 1 commit into from
May 5, 2021

Conversation

BastiaanOlij
Copy link
Contributor

@BastiaanOlij BastiaanOlij commented Apr 19, 2021

This was requested a long long time ago, finally got around to adding it for both GLES2 and GLES3.
I will be doing a different implementation with the same end result for Godot 4/Vulkan in the very near future.

This PR introduces a new shader variable called VIEW_INDEX that will be :

  • VIEW_MONO_LEFT (0) if we're rendering a normal view or the left eye in a stereoscopic view
  • VIEW_RIGHT (1) if we're rendering the right eye in a stereoscopic view

Ergo:

shader_type spatial;

void fragment() {
	if (VIEW_INDEX == VIEW_MONO_LEFT) {
		ALBEDO = vec3(1.0, 0.0, 0.0);
	} else if (VIEW_INDEX == VIEW_RIGHT) {
		ALBEDO = vec3(0.0, 0.0, 1.0);
	}
}

In normal:
image

In stereo:
image

Just changing colors like this doesn't look too spectacular but there are various use cases from showing different textures in each eye to create a depth effect or for visual aid applications where a difference in what each eye sees is created on purpose

note For Godot 4 this will tie in with stereoscopic rendering through multiview, current work in progress PR: stereoscopic proposal

// make sure this struct is padded to be a multiple of 16 bytes for webgl
float pad[2];
float pad[1];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm actually not sure this ends up being a multiple of 16 even before this change. I might be counting wrong but possibly other variables have been added since this was originally set to 2

@Calinou
Copy link
Member

Calinou commented Apr 19, 2021

This will likely be useful to implement godotengine/godot-proposals#437, although we'll want to do this in core since full-screen post-processing shaders have many limitations right now (at least in 3.x).

@clayjohn
Copy link
Member

I wonder if we should add some defines to make this a little easier like:

#define BOTH_EYES 0
#define LEFT EYE 1
#define RIGHT_EYE 2

@aaronfranke
Copy link
Member

aaronfranke commented Apr 20, 2021

Maybe it should be bitwise? Also, EYE should go first.

#define EYE_NONE 0 // Optional, we can exclude this.
#define EYE_LEFT 1
#define EYE_RIGHT 2
#define EYE_BOTH 3

@lawnjelly
Copy link
Member

Maybe it should be bitwise? Also, EYE should go first.

#define EYE_NONE 0 // Optional, we can exclude this.
#define EYE_LEFT 1
#define EYE_RIGHT 2
#define EYE_BOTH 3

Maybe for future compatibility, but as of now I don't think GPUs support bitwise integer operations in GLES.

@BastiaanOlij
Copy link
Contributor Author

BastiaanOlij commented Apr 20, 2021

Maybe it should be bitwise? Also, EYE should go first.

#define EYE_NONE 0 // Optional, we can exclude this.
#define EYE_LEFT 1
#define EYE_RIGHT 2
#define EYE_BOTH 3

Maybe for future compatibility, but as of now I don't think GPUs support bitwise integer operations in GLES.

the options are:

  • EYE_MONO = 0 (no stereo)
  • EYE_LEFT = 1
  • EYE_RIGHT = 2

There is no both, you are either rendering for the left eye, or right eye, or you're not rendering stereoscopic. So they aren't bits.

Happy to add them as defines

edit owh and these values have been like this for a long time and are used throughout the system so changing them is a huge breaking change.

@BastiaanOlij
Copy link
Contributor Author

BastiaanOlij commented Apr 25, 2021

Adding the defines doesn't make them available to Godot shader compiler so they can't be used in code.

I've tried implementing this by adding actions[VS::SHADER_SPATIAL].renames["EYE_MONO"] = "0"; to the shader compile implementation but weirdly enough that doesn't work. Still testing out options :)

edit never mind, found what I was missing...

@m4gr3d
Copy link
Contributor

m4gr3d commented Apr 26, 2021

note If this stereoscopic proposal we'll be re-introducing EYE as ViewIndex in Godot 4
In this case both EYE_MONO and EYE_LEFT will be 0 while EYE_RIGHT will be 1 as we'll be working with a buffer index.

Should we then just use VIEW_INDEX instead of EYE_* for the shader variable with the described values?

@BastiaanOlij
Copy link
Contributor Author

note If this stereoscopic proposal we'll be re-introducing EYE as ViewIndex in Godot 4
In this case both EYE_MONO and EYE_LEFT will be 0 while EYE_RIGHT will be 1 as we'll be working with a buffer index.

Should we then just use VIEW_INDEX instead of EYE_* for the shader variable with the described values?

I we don't mind loosing the distinction between EYE_MONO and EYE_LEFT as they both will have VIEW_INDEX = 0 I do believe this change will better align things with how they are likely going to be in Godot 4

@aaronfranke
Copy link
Member

aaronfranke commented May 1, 2021

What if the view index was calculated from EYE with & 1? I can imagine the distinction between the desktop view and both eyes to be useful, especially if you want to display something on the desktop that's different from either eye.

@BastiaanOlij
Copy link
Contributor Author

What if the view index was calculated from EYE with & 1? I can imagine the distinction between the desktop view and both eyes to be useful, especially if you want to display something on the desktop that's different from either eye.

The problem is that once we go to Godot 4 it actually reflects the layer we're rendering into, it's actually a build in variable in Vulkan, layer 0 for Mono/left eye, layer 1 for right eye, so we would need a separate variable to indicate if we're rendering mono/stereo.

In that respect it does make sense to just already bite that bullet and accept that limitation so people get used to VIEW_INDEX as the variable within the shader.

Obviously you as a developer know whether you are dealing with stereo scopic or a mono view so if you do need to make that difference you can always just add a uniform.

@BastiaanOlij
Copy link
Contributor Author

I've added a new commit that renames it from EYE to VIEW_INDEX so that this will work comparable to how this will likely end up working in Godot 4.

The example shader now looks like this:

shader_type spatial;

void fragment() {
	if (VIEW_INDEX == VIEW_MONO_LEFT) {
		ALBEDO = vec3(1.0, 0.0, 0.0);
	} else if (VIEW_INDEX == VIEW_RIGHT) {
		ALBEDO = vec3(0.0, 1.0, 0.0);
	}
}

We can't tell mono and stereo apart anymore but we're going down that path anyway.

If we're happy with this direction I will squash the changes.

@lawnjelly
Copy link
Member

A small extra point of discussion (that very probably doesn't require any changes) - if EYE is not used in the shader, there is no need to send the uniform.

Although a lot of uniforms are sent, I don't know if we should have any expectation that they should be cheap, especially not compared to checking a bool on the CPU, and especially on GLES2. There's already an existing mechanism for detecting whether BUILTINs and such are used in the shader, this could potentially be used to store whether this (or several in flags) uniforms are used and only send them in that case. This could be adding up if there are several unused uniforms (e.g. TIME is similar).

EYE is just a very obvious case of this - presumably 95%+ of people won't be using stereo rendering. Although it is equally relevant for the other uniforms.

Having said that this is more of an issue in the canvas shaders and perhaps something we should look at there, because there are likely to be far more drawcalls there than in 3d. It is probably academic in 3D unless you are using a boatload of drawcalls. It would be an interesting thing to test on desktop and mobile - certainly it wouldn't be worth changing unless it did cause an impact in terms of performance or hardware with limited number of uniforms.

@BastiaanOlij
Copy link
Contributor Author

@lawnjelly totally agree with that, my only remark would be that it may be worth doing a separate PR for that change to encompass other variables too.

@BastiaanOlij BastiaanOlij changed the title Add EYE variable in shader so we know which eye we're rendering for Add VIEW_INDEX variable in shader so we know which eye/view we're rendering for May 5, 2021
@BastiaanOlij
Copy link
Contributor Author

Squashed and rebased this, I think this is fine as it is.

@akien-mga akien-mga merged commit 16bc2a3 into godotengine:3.x May 5, 2021
@akien-mga
Copy link
Member

Thanks!

@mgpadalkar
Copy link

This was requested a long long time ago, finally got around to adding it for both GLES2 and GLES3. I will be doing a different implementation with the same end result for Godot 4/Vulkan in the very near future.

This PR introduces a new shader variable called VIEW_INDEX that will be :

* VIEW_MONO_LEFT (0) if we're rendering a normal view or the left eye in a stereoscopic view

* VIEW_RIGHT (1) if we're rendering the right eye in a stereoscopic view

Ergo:

shader_type spatial;

void fragment() {
	if (VIEW_INDEX == VIEW_MONO_LEFT) {
		ALBEDO = vec3(1.0, 0.0, 0.0);
	} else if (VIEW_INDEX == VIEW_RIGHT) {
		ALBEDO = vec3(0.0, 0.0, 1.0);
	}
}

In normal: image

In stereo: image

Just changing colors like this doesn't look too spectacular but there are various use cases from showing different textures in each eye to create a depth effect or for visual aid applications where a difference in what each eye sees is created on purpose

note For Godot 4 this will tie in with stereoscopic rendering through multiview, current work in progress PR: stereoscopic proposal

My question may have a simple solution, but I am unable to figure it out. @BastiaanOlij, could you please help?
How to display the window with two views (in your case the window labeled LeftRightEye (DEBUG)). Although I can see two different colors in the VR HMD, on the screen it only shows the left view.

@BastiaanOlij
Copy link
Contributor Author

My question may have a simple solution, but I am unable to figure it out. @BastiaanOlij, could you please help?
How to display the window with two views (in your case the window labeled LeftRightEye (DEBUG)). Although I can see two different colors in the VR HMD, on the screen it only shows the left view.

Indeed, OpenXR only shows one eye in preview, the side by side view you see in the screen shot only shows when using the native mobile interface. If you're using SteamVR, there is an option in the steamvr menu to show the output to the headset and that shows both eyes.

@mgpadalkar
Copy link

My question may have a simple solution, but I am unable to figure it out. @BastiaanOlij, could you please help?
How to display the window with two views (in your case the window labeled LeftRightEye (DEBUG)). Although I can see two different colors in the VR HMD, on the screen it only shows the left view.

Indeed, OpenXR only shows one eye in preview, the side by side view you see in the screen shot only shows when using the native mobile interface. If you're using SteamVR, there is an option in the steamvr menu to show the output to the headset and that shows both eyes.

Thank you!

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.

8 participants