From c16aba915f62a6fe39334e14e29a7532f3138790 Mon Sep 17 00:00:00 2001 From: Miguel Pozo Date: Mon, 12 May 2025 19:54:03 +0200 Subject: [PATCH] GPU: Add GPU_shader_batch_cancel Fix the recently implemented ShaderCompiler::batch_cancel. Expose it with GPU_shader_batch_cancel and GPU_shader_specialization_batch_cancel. Use them in the EEVEE ShaderModule destructor, to prevent blocking on destruction when there are in-flight compilations. Pull Request: https://projects.blender.org/blender/blender/pulls/138774 --- .../draw/engines/eevee/eevee_shader.cc | 16 +++++++------- source/blender/gpu/GPU_shader.hh | 11 ++++++++++ source/blender/gpu/intern/gpu_init_exit.cc | 4 ++-- source/blender/gpu/intern/gpu_shader.cc | 21 +++++++++++++++++-- .../blender/gpu/intern/gpu_shader_private.hh | 8 +++++++ source/blender/gpu/opengl/gl_shader.cc | 3 +++ 6 files changed, 51 insertions(+), 12 deletions(-) diff --git a/source/blender/draw/engines/eevee/eevee_shader.cc b/source/blender/draw/engines/eevee/eevee_shader.cc index 578cddbaa94..20e4448bcea 100644 --- a/source/blender/draw/engines/eevee/eevee_shader.cc +++ b/source/blender/draw/engines/eevee/eevee_shader.cc @@ -72,19 +72,19 @@ ShaderModule::ShaderModule() ShaderModule::~ShaderModule() { - /* Finish compilation to avoid asserts on exit at GLShaderCompiler destructor. */ - - if (compilation_handle_) { - static_shaders_are_ready(true); - } + /* Cancel compilation to avoid asserts on exit at ShaderCompiler destructor. */ + /* Specializations first, to avoid releasing the base shader while the specialization compilation + * is still in flight. */ for (SpecializationBatchHandle &handle : specialization_handles_.values()) { if (handle) { - while (!GPU_shader_batch_specializations_is_ready(handle)) { - /* Block until ready. */ - } + GPU_shader_batch_specializations_cancel(handle); } } + + if (compilation_handle_) { + GPU_shader_batch_cancel(compilation_handle_); + } } /** \} */ diff --git a/source/blender/gpu/GPU_shader.hh b/source/blender/gpu/GPU_shader.hh index b0c198d4ac7..52bc60c15da 100644 --- a/source/blender/gpu/GPU_shader.hh +++ b/source/blender/gpu/GPU_shader.hh @@ -100,6 +100,11 @@ bool GPU_shader_batch_is_ready(BatchHandle handle); * WARNING: The handle will be invalidated by this call, you can't request the same batch twice. */ blender::Vector GPU_shader_batch_finalize(BatchHandle &handle); +/** + * Cancel the compilation of the batch. + * WARNING: The handle will be invalidated by this call. + */ +void GPU_shader_batch_cancel(BatchHandle &handle); /** \} */ @@ -267,6 +272,12 @@ SpecializationBatchHandle GPU_shader_batch_specializations( */ bool GPU_shader_batch_specializations_is_ready(SpecializationBatchHandle &handle); +/** + * Cancel the specialization batch. + * WARNING: The handle will be invalidated by this call. + */ +void GPU_shader_batch_specializations_cancel(SpecializationBatchHandle &handle); + /** \} */ /* -------------------------------------------------------------------- */ diff --git a/source/blender/gpu/intern/gpu_init_exit.cc b/source/blender/gpu/intern/gpu_init_exit.cc index 59f82db54e4..636389809bb 100644 --- a/source/blender/gpu/intern/gpu_init_exit.cc +++ b/source/blender/gpu/intern/gpu_init_exit.cc @@ -45,11 +45,11 @@ void GPU_exit() gpu_codegen_exit(); + gpu_backend_delete_resources(); + gpu_shader_dependency_exit(); gpu_shader_create_info_exit(); - gpu_backend_delete_resources(); - initialized = false; } diff --git a/source/blender/gpu/intern/gpu_shader.cc b/source/blender/gpu/intern/gpu_shader.cc index d246565948d..62d03b785ac 100644 --- a/source/blender/gpu/intern/gpu_shader.cc +++ b/source/blender/gpu/intern/gpu_shader.cc @@ -380,6 +380,11 @@ Vector GPU_shader_batch_finalize(BatchHandle &handle) return reinterpret_cast &>(result); } +void GPU_shader_batch_cancel(BatchHandle &handle) +{ + GPUBackend::get()->get_compiler()->batch_cancel(handle); +} + void GPU_shader_compile_static() { printf("Compiling all static GPU shaders. This process takes a while.\n"); @@ -552,6 +557,11 @@ bool GPU_shader_batch_specializations_is_ready(SpecializationBatchHandle &handle return GPUBackend::get()->get_compiler()->specialization_batch_is_ready(handle); } +void GPU_shader_batch_specializations_cancel(SpecializationBatchHandle &handle) +{ + GPUBackend::get()->get_compiler()->batch_cancel(handle); +} + /** \} */ /* -------------------------------------------------------------------- */ @@ -1002,7 +1012,7 @@ BatchHandle ShaderCompiler::batch_compile(Span void ShaderCompiler::batch_cancel(BatchHandle &handle) { - std::lock_guard lock(mutex_); + std::unique_lock lock(mutex_); Batch *batch = batches_.pop(handle); @@ -1015,7 +1025,14 @@ void ShaderCompiler::batch_cancel(BatchHandle &handle) compilation_queue_.erase(std::remove_if(compilation_queue_.begin(), compilation_queue_.end(), - [](const ParallelWork &work) { return !work.batch; })); + [](const ParallelWork &work) { return !work.batch; }), + compilation_queue_.end()); + + if (batch->is_specialization_batch()) { + /* For specialization batches, we block until ready, since base shader compilation may be + * cancelled afterwards, leaving the specialization with a deleted base shader. */ + compilation_finished_notification_.wait(lock, [&]() { return batch->is_ready(); }); + } if (batch->is_ready()) { batch->free_shaders(); diff --git a/source/blender/gpu/intern/gpu_shader_private.hh b/source/blender/gpu/intern/gpu_shader_private.hh index 6784b52db04..9a9bc1f6fcf 100644 --- a/source/blender/gpu/intern/gpu_shader_private.hh +++ b/source/blender/gpu/intern/gpu_shader_private.hh @@ -211,6 +211,14 @@ class ShaderCompiler { BatchHandle next_batch_handle_ = 1; + protected: + /* Must be called earlier from the destructor of the subclass if the compilation process relies + * on subclass resources. */ + void destruct_compilation_worker() + { + compilation_worker_.reset(); + } + public: ShaderCompiler(uint32_t threads_count = 1, GPUWorker::ContextType context_type = GPUWorker::ContextType::PerThread, diff --git a/source/blender/gpu/opengl/gl_shader.cc b/source/blender/gpu/opengl/gl_shader.cc index 9fb114a1695..64404e16653 100644 --- a/source/blender/gpu/opengl/gl_shader.cc +++ b/source/blender/gpu/opengl/gl_shader.cc @@ -1740,6 +1740,9 @@ void GLCompilerWorker::release() GLShaderCompiler::~GLShaderCompiler() { + /* Must be called before we destruct the GLCompilerWorkers. */ + destruct_compilation_worker(); + for (GLCompilerWorker *worker : workers_) { delete worker; }