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

Convert output of GLES2 to linear color space #51780

Merged
merged 1 commit into from
Aug 26, 2021

Conversation

BastiaanOlij
Copy link
Contributor

In XR the render output is sent to a compositor that will do further processing of the images and output them to an HMD, one of the adjustments made here is converting to the color space that best suits the HMD.

The problem we've run into a few times is that as a result many of the compositors expect the render images to be in linear color space.
Some like OpenVR and Oculus' VrAPI do have support for sRGB input (though it doesn't work in OpenVR when RGBA16F is used) but it looks like OpenXR only offers it through hardware sRGB buffers which don't seem to work as advertised when using GLES2. The bottom line is that OpenXR simply expects us to supply render buffers in linear color space.

For the GLES3 renderer we solved this issue a long time ago by introducing the keep_3d_linear switch as rendering happens in linear color space and we simply convert this to sRGB as part of our tonemapping post process. The introduced switch skips this conversion.

For the GLES2 renderer however we render everything in sRGB color space and this is all we have to offer, the keep_3d_linear switch has no effect. As it would be way to much work to convert the GLES2 renderer to work in linear color space and it requiring a post process to then convert to sRGB this isn't really an option.
This PR takes the opposite approach letting the renderer do its work in sRGB color space but converting the sRGB result to linear color space if keep_3d_linear is enabled.

This PR also changes the arvr blit functions for creating the spectator view that if it is provided with a render buffer that has keep_3d_linear turned on, it converts the result to sRGB. This was a long outstanding wish I implemented while working on this anyway.

I need to do some more testing but this PR is basically ready.

@BastiaanOlij
Copy link
Contributor Author

Tested this both on desktop and on the Quest. In both it seems to do the conversions just fine. The way it looks inside of the headset is now the same between GLES2 converting sRGB -> Linear and GLES3 where we keep the results linear. It does come across slightly darker in the headset but I think this is because the OpenXR uses a slightly different color setup. It's heaps better then it looking waaaay too bright and oversaturated.

@lawnjelly
Copy link
Member

Looks good to me from looking through the source code 👍 , I'll leave for @clayjohn to review.

@m4gr3d
Copy link
Contributor

m4gr3d commented Aug 18, 2021

Looks great!
Curious if there is any performance impact with that switch turned on?

@BastiaanOlij
Copy link
Contributor Author

Looks great!
Curious if there is any performance impact with that switch turned on?

I haven't tested but if there is any it should be really minor. It's just applying a gradient to the color that is output. Also keep in mind that for the XR interfaces that do display the right color, they are doing this color conversion within the lens distortion shader most likely so any loss is just moved.

The "proper" way to solve this is to redo the entire 3D rendering in GLES2 in linear by converting all the color inputs. That would work great for our needs but it would make everyone elses games much slower as suddenly they need a post process to convert linear to sRGB (as we do in the GLES3 renderer).

That all said, the mobile renderer in Vulkan does do this correctly. All 3D rendering happens in linear and during the tonemapping post process we optionally convert to sRGB. Here it is less of a problem because where performance is needed we can do this within a subpass.

Copy link
Member

@clayjohn clayjohn 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! It's a shame we have to do this, but it's one more hack we can do away with for 4.0!

@BastiaanOlij
Copy link
Contributor Author

Looks good! It's a shame we have to do this, but it's one more hack we can do away with for 4.0!

Yup, and it's a defensible one that only effects VR so...

@clayjohn clayjohn merged commit 46ad256 into godotengine:3.x Aug 26, 2021
@clayjohn
Copy link
Member

Thanks!

@BastiaanOlij BastiaanOlij deleted the output_linear_gles2 branch August 27, 2021 01:10
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.

4 participants