[Bf-blender-cvs] [16099c00d05] master: Fix cycles crash when changing viewport display pass

Sergey Sharybin noreply at git.blender.org
Fri Jul 9 10:44:03 CEST 2021


Commit: 16099c00d05dcd1be3d462ddeb96a182679b5e90
Author: Sergey Sharybin
Date:   Tue Jul 6 17:23:26 2021 +0200
Branches: master
https://developer.blender.org/rB16099c00d05dcd1be3d462ddeb96a182679b5e90

Fix cycles crash when changing viewport display pass

It was possible that render buffers and scene kernel data will be out
of sync because reset and scene update happens in different locks.

This is similar issue we've fixed in the Cycles X branch, so backported
relevant changes from there.

This change removes what seems to be unused feature kernel.

Differential Revision: https://developer.blender.org/D11828

===================================================================

M	intern/cycles/render/session.cpp
M	intern/cycles/render/session.h

===================================================================

diff --git a/intern/cycles/render/session.cpp b/intern/cycles/render/session.cpp
index ca90af77f6e..19d4a66353d 100644
--- a/intern/cycles/render/session.cpp
+++ b/intern/cycles/render/session.cpp
@@ -227,66 +227,25 @@ void Session::run_gpu()
   progress.set_render_start_time();
 
   while (!progress.get_cancel()) {
-    /* advance to next tile */
-    bool no_tiles = !tile_manager.next();
+    const bool no_tiles = !run_update_for_next_iteration();
 
-    DeviceKernelStatus kernel_state = DEVICE_KERNEL_UNKNOWN;
     if (no_tiles) {
-      kernel_state = device->get_active_kernel_switch_state();
-    }
-
-    if (params.background) {
-      /* if no work left and in background mode, we can stop immediately */
-      if (no_tiles) {
+      if (params.background) {
+        /* if no work left and in background mode, we can stop immediately */
         progress.set_status("Finished");
         break;
       }
     }
 
-    else if (no_tiles && kernel_state == DEVICE_KERNEL_FEATURE_KERNEL_AVAILABLE) {
-      reset_gpu(tile_manager.params, params.samples);
+    if (run_wait_for_work(no_tiles)) {
+      continue;
     }
 
-    else {
-      /* if in interactive mode, and we are either paused or done for now,
-       * wait for pause condition notify to wake up again */
-      thread_scoped_lock pause_lock(pause_mutex);
-
-      if (!pause && !tile_manager.done()) {
-        /* reset could have happened after no_tiles was set, before this lock.
-         * in this case we shall not wait for pause condition
-         */
-      }
-      else if (pause || no_tiles) {
-        update_status_time(pause, no_tiles);
-
-        while (1) {
-          scoped_timer pause_timer;
-          pause_cond.wait(pause_lock);
-          if (pause) {
-            progress.add_skip_time(pause_timer, params.background);
-          }
-
-          update_status_time(pause, no_tiles);
-          progress.set_update();
-
-          if (!pause)
-            break;
-        }
-      }
-
-      if (progress.get_cancel())
-        break;
+    if (progress.get_cancel()) {
+      break;
     }
 
     if (!no_tiles) {
-      /* update scene */
-      scoped_timer update_timer;
-      if (update_scene()) {
-        profiler.reset(scene->shaders.size(), scene->objects.size());
-      }
-      progress.add_skip_time(update_timer, params.background);
-
       if (!device->error_message().empty())
         progress.set_error(device->error_message());
 
@@ -729,82 +688,27 @@ void Session::run_cpu()
   last_update_time = time_dt();
   last_display_time = last_update_time;
 
-  {
-    /* reset once to start */
-    thread_scoped_lock reset_lock(delayed_reset.mutex);
-    thread_scoped_lock buffers_lock(buffers_mutex);
-    thread_scoped_lock display_lock(display_mutex);
-
-    reset_(delayed_reset.params, delayed_reset.samples);
-    delayed_reset.do_reset = false;
-  }
-
   while (!progress.get_cancel()) {
-    /* advance to next tile */
-    bool no_tiles = !tile_manager.next();
+    const bool no_tiles = !run_update_for_next_iteration();
     bool need_copy_to_display_buffer = false;
 
-    DeviceKernelStatus kernel_state = DEVICE_KERNEL_UNKNOWN;
     if (no_tiles) {
-      kernel_state = device->get_active_kernel_switch_state();
-    }
-
-    if (params.background) {
-      /* if no work left and in background mode, we can stop immediately */
-      if (no_tiles) {
+      if (params.background) {
+        /* if no work left and in background mode, we can stop immediately */
         progress.set_status("Finished");
         break;
       }
     }
 
-    else if (no_tiles && kernel_state == DEVICE_KERNEL_FEATURE_KERNEL_AVAILABLE) {
-      reset_cpu(tile_manager.params, params.samples);
+    if (run_wait_for_work(no_tiles)) {
+      continue;
     }
 
-    else {
-      /* if in interactive mode, and we are either paused or done for now,
-       * wait for pause condition notify to wake up again */
-      thread_scoped_lock pause_lock(pause_mutex);
-
-      if (!pause && delayed_reset.do_reset) {
-        /* reset once to start */
-        thread_scoped_lock reset_lock(delayed_reset.mutex);
-        thread_scoped_lock buffers_lock(buffers_mutex);
-        thread_scoped_lock display_lock(display_mutex);
-
-        reset_(delayed_reset.params, delayed_reset.samples);
-        delayed_reset.do_reset = false;
-      }
-      else if (pause || no_tiles) {
-        update_status_time(pause, no_tiles);
-
-        while (1) {
-          scoped_timer pause_timer;
-          pause_cond.wait(pause_lock);
-          if (pause) {
-            progress.add_skip_time(pause_timer, params.background);
-          }
-
-          update_status_time(pause, no_tiles);
-          progress.set_update();
-
-          if (!pause)
-            break;
-        }
-      }
-
-      if (progress.get_cancel())
-        break;
+    if (progress.get_cancel()) {
+      break;
     }
 
     if (!no_tiles) {
-      /* update scene */
-      scoped_timer update_timer;
-      if (update_scene()) {
-        profiler.reset(scene->shaders.size(), scene->objects.size());
-      }
-      progress.add_skip_time(update_timer, params.background);
-
       if (!device->error_message().empty())
         progress.set_error(device->error_message());
 
@@ -894,6 +798,63 @@ void Session::run()
     progress.set_update();
 }
 
+bool Session::run_update_for_next_iteration()
+{
+  thread_scoped_lock scene_lock(scene->mutex);
+  thread_scoped_lock reset_lock(delayed_reset.mutex);
+
+  if (delayed_reset.do_reset) {
+    thread_scoped_lock buffers_lock(buffers_mutex);
+    reset_(delayed_reset.params, delayed_reset.samples);
+    delayed_reset.do_reset = false;
+  }
+
+  const bool have_tiles = tile_manager.next();
+
+  if (have_tiles) {
+    scoped_timer update_timer;
+    if (update_scene()) {
+      profiler.reset(scene->shaders.size(), scene->objects.size());
+    }
+    progress.add_skip_time(update_timer, params.background);
+  }
+
+  return have_tiles;
+}
+
+bool Session::run_wait_for_work(bool no_tiles)
+{
+  /* In an offline rendering there is no pause, and no tiles will mean the job is fully done. */
+  if (params.background) {
+    return false;
+  }
+
+  thread_scoped_lock pause_lock(pause_mutex);
+
+  if (!pause && !no_tiles) {
+    return false;
+  }
+
+  update_status_time(pause, no_tiles);
+
+  while (true) {
+    scoped_timer pause_timer;
+    pause_cond.wait(pause_lock);
+    if (pause) {
+      progress.add_skip_time(pause_timer, params.background);
+    }
+
+    update_status_time(pause, no_tiles);
+    progress.set_update();
+
+    if (!pause) {
+      break;
+    }
+  }
+
+  return no_tiles;
+}
+
 bool Session::draw(BufferParams &buffer_params, DeviceDrawParams &draw_params)
 {
   if (device_use_gl)
@@ -1012,8 +973,6 @@ void Session::wait()
 
 bool Session::update_scene()
 {
-  thread_scoped_lock scene_lock(scene->mutex);
-
   /* update camera if dimensions changed for progressive render. the camera
    * knows nothing about progressive or cropped rendering, it just gets the
    * image dimensions passed in */
diff --git a/intern/cycles/render/session.h b/intern/cycles/render/session.h
index 43ff07e5884..bc3b8366c05 100644
--- a/intern/cycles/render/session.h
+++ b/intern/cycles/render/session.h
@@ -178,6 +178,9 @@ class Session {
 
   void run();
 
+  bool run_update_for_next_iteration();
+  bool run_wait_for_work(bool no_tiles);
+
   void update_status_time(bool show_pause = false, bool show_done = false);
 
   void render(bool use_denoise);



More information about the Bf-blender-cvs mailing list