From 58d32b482bde1aedd340a1476ca37e3c8108c78a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Foucault?= Date: Wed, 7 May 2025 16:42:01 +0200 Subject: [PATCH] Fix #138354: GPU: Race condition with SRGB builtin uniform Multiple threads would be setting the globals `g_shader_builtin_srgb_transform` and `g_shader_builtin_srgb_is_dirty`. These are use for color management inside the builtin shaders. But the render thread could modify these values even if its shader have no use of these. The fix is to move these globals to the `gpu::Context` class. This way we remove the race condition. --- .../blender/gpu/intern/gpu_context_private.hh | 4 +++ source/blender/gpu/intern/gpu_immediate.cc | 1 - source/blender/gpu/intern/gpu_shader.cc | 27 +++++++------------ .../blender/gpu/intern/gpu_shader_private.hh | 3 +-- 4 files changed, 15 insertions(+), 20 deletions(-) diff --git a/source/blender/gpu/intern/gpu_context_private.hh b/source/blender/gpu/intern/gpu_context_private.hh index b8138c55ffd..a51352324a6 100644 --- a/source/blender/gpu/intern/gpu_context_private.hh +++ b/source/blender/gpu/intern/gpu_context_private.hh @@ -74,6 +74,10 @@ class Context { /** Texture pool used to recycle temporary texture (or render target) memory. */ TexturePool *texture_pool = nullptr; + /** Global state to avoid setting the srgb builtin uniform for every shader bind. */ + int shader_builtin_srgb_transform = 0; + bool shader_builtin_srgb_is_dirty = false; + protected: /** Thread on which this context is active. */ pthread_t thread_; diff --git a/source/blender/gpu/intern/gpu_immediate.cc b/source/blender/gpu/intern/gpu_immediate.cc index 68ebb3f1c30..cb087f9b63b 100644 --- a/source/blender/gpu/intern/gpu_immediate.cc +++ b/source/blender/gpu/intern/gpu_immediate.cc @@ -56,7 +56,6 @@ void immBindShader(GPUShader *shader) GPU_shader_bind(shader); GPU_matrix_bind(shader); - Shader::set_srgb_uniform(shader); } void immBindBuiltinProgram(eGPUBuiltinShader shader_id) diff --git a/source/blender/gpu/intern/gpu_shader.cc b/source/blender/gpu/intern/gpu_shader.cc index 6e67613a264..25bfe6bec45 100644 --- a/source/blender/gpu/intern/gpu_shader.cc +++ b/source/blender/gpu/intern/gpu_shader.cc @@ -407,7 +407,7 @@ void GPU_shader_bind(GPUShader *gpu_shader) ctx->shader = shader; shader->bind(); GPU_matrix_bind(gpu_shader); - Shader::set_srgb_uniform(gpu_shader); + Shader::set_srgb_uniform(ctx, gpu_shader); shader->constants.is_dirty = false; } else { @@ -415,8 +415,8 @@ void GPU_shader_bind(GPUShader *gpu_shader) shader->bind(); shader->constants.is_dirty = false; } - if (Shader::srgb_uniform_dirty_get()) { - Shader::set_srgb_uniform(gpu_shader); + if (ctx->shader_builtin_srgb_is_dirty) { + Shader::set_srgb_uniform(ctx, gpu_shader); } if (GPU_matrix_dirty_get()) { GPU_matrix_bind(gpu_shader); @@ -790,28 +790,21 @@ namespace blender::gpu { * frame-buffer color-space. * \{ */ -static int g_shader_builtin_srgb_transform = 0; -static bool g_shader_builtin_srgb_is_dirty = false; - -bool Shader::srgb_uniform_dirty_get() -{ - return g_shader_builtin_srgb_is_dirty; -} - -void Shader::set_srgb_uniform(GPUShader *shader) +void Shader::set_srgb_uniform(Context *ctx, GPUShader *shader) { int32_t loc = GPU_shader_get_builtin_uniform(shader, GPU_UNIFORM_SRGB_TRANSFORM); if (loc != -1) { - GPU_shader_uniform_int_ex(shader, loc, 1, 1, &g_shader_builtin_srgb_transform); + GPU_shader_uniform_int_ex(shader, loc, 1, 1, &ctx->shader_builtin_srgb_transform); } - g_shader_builtin_srgb_is_dirty = false; + ctx->shader_builtin_srgb_is_dirty = false; } void Shader::set_framebuffer_srgb_target(int use_srgb_to_linear) { - if (g_shader_builtin_srgb_transform != use_srgb_to_linear) { - g_shader_builtin_srgb_transform = use_srgb_to_linear; - g_shader_builtin_srgb_is_dirty = true; + Context *ctx = Context::get(); + if (ctx->shader_builtin_srgb_transform != use_srgb_to_linear) { + ctx->shader_builtin_srgb_transform = use_srgb_to_linear; + ctx->shader_builtin_srgb_is_dirty = true; } } diff --git a/source/blender/gpu/intern/gpu_shader_private.hh b/source/blender/gpu/intern/gpu_shader_private.hh index 00bed6f0b14..2d1416b50ad 100644 --- a/source/blender/gpu/intern/gpu_shader_private.hh +++ b/source/blender/gpu/intern/gpu_shader_private.hh @@ -131,8 +131,7 @@ class Shader { return parent_shader_; } - static bool srgb_uniform_dirty_get(); - static void set_srgb_uniform(GPUShader *shader); + static void set_srgb_uniform(Context *ctx, GPUShader *shader); static void set_framebuffer_srgb_target(int use_srgb_to_linear); protected: