From 0d73d5c1a2477bd7170fbb4306ec511ddcadefb1 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Tue, 22 Nov 2022 18:59:32 +0100 Subject: [PATCH] Fix frozen image editor when Cycles compiles kernels It is possible that the image editor redraw happens prior to the "Loading render kernels" status is reported from status but after the display driver is created. This will make the image editor to wait on the scene mutex to update the display pass in the film. If it happens to be that the kernels are actually to be compiled then the Blender interface appears to be completely frozen, without any information line in the image editor. This change makes it so the amount of time the scene mutex is held during the kernel compilation is minimal. It is a bit unideal to unlock and re-lock the scene mutex in the middle of update, while nested reset mutex is held, but this is already what is needed for the OptiX denoiser optimization some lines below. We can probably reduce the lifetime of some locks, avoiding such potential out-of-order re-locking. Doing so is outside of the scope of this patch. The scene update only happens from the single place in the session, which makes it easy to ensure the kernels are loaded prior the rest of the scene update. Not only this change makes it so that the "Loading render kernels" status appears in the image editor, but also allows to pan and zoom in the image editor, potentially allowing artists to re-adjust their point of interest. Differential Revision: https://developer.blender.org/D16581 --- intern/cycles/scene/scene.cpp | 12 +++--------- intern/cycles/scene/scene.h | 2 +- intern/cycles/session/session.cpp | 12 ++++++++++++ 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/intern/cycles/scene/scene.cpp b/intern/cycles/scene/scene.cpp index 3a05bede7a3..1a611b2a3db 100644 --- a/intern/cycles/scene/scene.cpp +++ b/intern/cycles/scene/scene.cpp @@ -488,6 +488,8 @@ void Scene::update_kernel_features() return; } + thread_scoped_lock scene_lock(mutex); + /* These features are not being tweaked as often as shaders, * so could be done selective magic for the viewport as well. */ uint kernel_features = shader_manager->get_kernel_features(this); @@ -574,9 +576,6 @@ bool Scene::update(Progress &progress) return false; } - /* Load render kernels, before device update where we upload data to the GPU. */ - load_kernels(progress, false); - /* Upload scene data to the GPU. */ progress.set_status("Updating Scene"); MEM_GUARDED_CALL(&progress, device_update, device, progress); @@ -616,13 +615,8 @@ static void log_kernel_features(const uint features) << "\n"; } -bool Scene::load_kernels(Progress &progress, bool lock_scene) +bool Scene::load_kernels(Progress &progress) { - thread_scoped_lock scene_lock; - if (lock_scene) { - scene_lock = thread_scoped_lock(mutex); - } - update_kernel_features(); const uint kernel_features = dscene.data.kernel_features; diff --git a/intern/cycles/scene/scene.h b/intern/cycles/scene/scene.h index d87cc37aafe..f2bc8f7b9f2 100644 --- a/intern/cycles/scene/scene.h +++ b/intern/cycles/scene/scene.h @@ -270,6 +270,7 @@ class Scene : public NodeOwner { void enable_update_stats(); + bool load_kernels(Progress &progress); bool update(Progress &progress); bool has_shadow_catcher(); @@ -333,7 +334,6 @@ class Scene : public NodeOwner { uint loaded_kernel_features; void update_kernel_features(); - bool load_kernels(Progress &progress, bool lock_scene = true); bool has_shadow_catcher_ = false; bool shadow_catcher_modified_ = true; diff --git a/intern/cycles/session/session.cpp b/intern/cycles/session/session.cpp index acaa55f4990..e5ece8dc0cb 100644 --- a/intern/cycles/session/session.cpp +++ b/intern/cycles/session/session.cpp @@ -378,6 +378,18 @@ RenderWork Session::run_update_for_next_iteration() const int width = max(1, buffer_params_.full_width / resolution); const int height = max(1, buffer_params_.full_height / resolution); + { + /* Load render kernels, before device update where we upload data to the GPU. + * Do it outside of the scene mutex since the heavy part of the loading (i.e. kernel + * compilation) does not depend on the scene and some other functionality (like display + * driver) might be waiting on the scene mutex to synchronize display pass. + * + * The scene will lock itself for the short period if it needs to update kernel features. */ + scene_lock.unlock(); + scene->load_kernels(progress); + scene_lock.lock(); + } + if (update_scene(width, height)) { profiler.reset(scene->shaders.size(), scene->objects.size()); }