Skip to content

Commit dd8841a

Browse files
authored
Merge pull request godotengine#74892 from BastiaanOlij/fix_hw_srgb_conversion
XR: When an sRGB target is used, check hardware sRGB conversion
2 parents 44cc6e5 + a1a52c5 commit dd8841a

File tree

4 files changed

+56
-3
lines changed

4 files changed

+56
-3
lines changed

modules/openxr/extensions/openxr_extension_wrapper.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@ class OpenXRExtensionWrapper {
8080
// this happens right before physics process and normal processing is run.
8181
// This is when controller data is queried and made available to game logic.
8282
virtual void on_process() {}
83-
virtual void on_pre_render() {} // `on_pre_render` is called right before we start rendering our XR viewport.
83+
virtual void on_pre_render() {} // `on_pre_render` is called right before we start rendering our XR viewports.
84+
virtual void on_pre_draw_viewport(RID p_render_target) {} // `on_pre_draw_viewport` is called right before we start rendering this viewport
85+
virtual void on_post_draw_viewport(RID p_render_target) {} // `on_port_draw_viewport` is called right after we start rendering this viewport (note that on Vulkan draw commands may only be queued)
8486

8587
virtual void on_state_idle() {} // `on_state_idle` is called when the OpenXR session state is changed to idle.
8688
virtual void on_state_ready() {} // `on_state_ready` is called when the OpenXR session state is changed to ready, this means OpenXR is ready to setup our session.

modules/openxr/extensions/openxr_opengl_extension.cpp

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,28 @@
3737
#include "servers/rendering/rendering_server_globals.h"
3838
#include "servers/rendering_server.h"
3939

40+
// OpenXR requires us to submit sRGB textures so that it recognises the content
41+
// as being in sRGB color space. We do fall back on "normal" textures but this
42+
// will likely result in incorrect colors as OpenXR will double the sRGB conversion.
43+
// All major XR runtimes support sRGB textures.
44+
45+
// In OpenGL output of the fragment shader is assumed to be in the color space of
46+
// the developers choice, however a linear to sRGB HW conversion can be enabled
47+
// through enabling GL_FRAMEBUFFER_SRGB if an sRGB color attachment is used.
48+
// This is a global setting.
49+
// See: https://www.khronos.org/opengl/wiki/Framebuffer
50+
51+
// In OpenGLES output of the fragment shader is assumed to be in linear color space
52+
// and will be converted by default to sRGB if an sRGB color attachment is used.
53+
// The extension GL_EXT_sRGB_write_control was introduced to enable turning this
54+
// feature off.
55+
// See: https://registry.khronos.org/OpenGL/extensions/EXT/EXT_sRGB_write_control.txt
56+
57+
// On OpenGLES this is not defined in our standard headers..
58+
#ifndef GL_FRAMEBUFFER_SRGB
59+
#define GL_FRAMEBUFFER_SRGB 0x8DB9
60+
#endif
61+
4062
HashMap<String, bool *> OpenXROpenGLExtension::get_requested_extensions() {
4163
HashMap<String, bool *> request_extensions;
4264

@@ -157,8 +179,8 @@ void *OpenXROpenGLExtension::set_session_create_and_get_next_pointer(void *p_nex
157179
}
158180

159181
void OpenXROpenGLExtension::get_usable_swapchain_formats(Vector<int64_t> &p_usable_swap_chains) {
160-
p_usable_swap_chains.push_back(GL_RGBA8);
161182
p_usable_swap_chains.push_back(GL_SRGB8_ALPHA8);
183+
p_usable_swap_chains.push_back(GL_RGBA8);
162184
}
163185

164186
void OpenXROpenGLExtension::get_usable_depth_formats(Vector<int64_t> &p_usable_depth_formats) {
@@ -168,6 +190,23 @@ void OpenXROpenGLExtension::get_usable_depth_formats(Vector<int64_t> &p_usable_d
168190
p_usable_depth_formats.push_back(GL_DEPTH_COMPONENT24);
169191
}
170192

193+
void OpenXROpenGLExtension::on_pre_draw_viewport(RID p_render_target) {
194+
if (srgb_ext_is_available) {
195+
hw_linear_to_srgb_is_enabled = glIsEnabled(GL_FRAMEBUFFER_SRGB);
196+
if (hw_linear_to_srgb_is_enabled) {
197+
// Disable this.
198+
glDisable(GL_FRAMEBUFFER_SRGB);
199+
}
200+
}
201+
}
202+
203+
void OpenXROpenGLExtension::on_post_draw_viewport(RID p_render_target) {
204+
if (srgb_ext_is_available && hw_linear_to_srgb_is_enabled) {
205+
// Re-enable this.
206+
glEnable(GL_FRAMEBUFFER_SRGB);
207+
}
208+
}
209+
171210
bool OpenXROpenGLExtension::get_swapchain_image_data(XrSwapchain p_swapchain, int64_t p_swapchain_format, uint32_t p_width, uint32_t p_height, uint32_t p_sample_count, uint32_t p_array_size, void **r_swapchain_graphics_data) {
172211
GLES3::TextureStorage *texture_storage = GLES3::TextureStorage::get_singleton();
173212
ERR_FAIL_NULL_V(texture_storage, false);

modules/openxr/extensions/openxr_opengl_extension.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ class OpenXROpenGLExtension : public OpenXRGraphicsExtensionWrapper {
7979
virtual void on_instance_created(const XrInstance p_instance) override;
8080
virtual void *set_session_create_and_get_next_pointer(void *p_next_pointer) override;
8181

82+
virtual void on_pre_draw_viewport(RID p_render_target) override;
83+
virtual void on_post_draw_viewport(RID p_render_target) override;
84+
8285
virtual void get_usable_swapchain_formats(Vector<int64_t> &p_usable_swap_chains) override;
8386
virtual void get_usable_depth_formats(Vector<int64_t> &p_usable_swap_chains) override;
8487
virtual String get_swapchain_format_name(int64_t p_swapchain_format) const override;
@@ -103,6 +106,9 @@ class OpenXROpenGLExtension : public OpenXRGraphicsExtensionWrapper {
103106
Vector<RID> texture_rids;
104107
};
105108

109+
bool srgb_ext_is_available = true;
110+
bool hw_linear_to_srgb_is_enabled = false;
111+
106112
bool check_graphics_api_support(XrVersion p_desired_version);
107113

108114
#ifdef ANDROID_ENABLED

modules/openxr/openxr_api.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1815,6 +1815,10 @@ bool OpenXRAPI::pre_draw_viewport(RID p_render_target) {
18151815
}
18161816
}
18171817

1818+
for (OpenXRExtensionWrapper *wrapper : registered_extension_wrappers) {
1819+
wrapper->on_pre_draw_viewport(p_render_target);
1820+
}
1821+
18181822
return true;
18191823
}
18201824

@@ -1839,7 +1843,9 @@ void OpenXRAPI::post_draw_viewport(RID p_render_target) {
18391843
return;
18401844
}
18411845

1842-
// Nothing to do here at this point in time...
1846+
for (OpenXRExtensionWrapper *wrapper : registered_extension_wrappers) {
1847+
wrapper->on_post_draw_viewport(p_render_target);
1848+
}
18431849
};
18441850

18451851
void OpenXRAPI::end_frame() {

0 commit comments

Comments
 (0)