Refactor: Cycles: Move scene update earlier in render iteration update

This will be needed to determine if there are volumes in the scene, before
allocation passes to aid volume sampling.

The kernels are now also loaded in the middle of scene update, at a place
where kernel features are known but before the kernels are needed for
displacement and background light evaluation..

Updating the camera to final resolution for progressive refinement still
happens later.

Pull Request: https://projects.blender.org/blender/blender/pulls/137228

intern/cycles/scene/camera.cpp

@@ -469,9 +469,6 @@ void Camera::update(Scene *scene)
   kcam->nearclip = nearclip;
   kcam->cliplength = (farclip == FLT_MAX) ? FLT_MAX : farclip - nearclip;
 
-  /* Camera in volume. */
-  kcam->is_inside_volume = 0;
-
   /* Rolling shutter effect */
   kcam->rolling_shutter_type = rolling_shutter_type;
   kcam->rolling_shutter_duration = rolling_shutter_duration;
@@ -530,9 +527,10 @@ void Camera::device_update_volume(Device * /*device*/, DeviceScene *dscene, Scen
     return;
   }
 
+  kernel_camera.is_inside_volume = 0;
+
   KernelIntegrator *kintegrator = &dscene->data.integrator;
   if (kintegrator->use_volumes) {
-    KernelCamera *kcam = &dscene->data.cam;
     BoundBox viewplane_boundbox = viewplane_bounds_get();
 
     /* Parallel object update, with grain size to avoid too much threading overhead
@@ -546,18 +544,20 @@ void Camera::device_update_volume(Device * /*device*/, DeviceScene *dscene, Scen
                          viewplane_boundbox.intersects(object->bounds)) {
                        /* TODO(sergey): Consider adding more grained check. */
                        VLOG_INFO << "Detected camera inside volume.";
-                       kcam->is_inside_volume = 1;
+                       kernel_camera.is_inside_volume = 1;
                        parallel_for_cancel();
                        break;
                      }
                    }
                  });
 
-    if (!kcam->is_inside_volume) {
+    if (!kernel_camera.is_inside_volume) {
       VLOG_INFO << "Camera is outside of the volume.";
     }
   }
 
+  dscene->data.cam.is_inside_volume = kernel_camera.is_inside_volume;
+
   need_device_update = false;
   need_flags_update = false;
 }
@@ -824,13 +824,16 @@ bool Camera::use_motion() const
   return motion.size() > 1;
 }
 
-void Camera::set_screen_size(const int width_, int height_)
+bool Camera::set_screen_size(const int width_, int height_)
 {
   if (width_ != width || height_ != height) {
     width = width_;
     height = height_;
     tag_modified();
+    return true;
   }
+
+  return false;
 }
 
 float Camera::motion_time(const int step) const

intern/cycles/scene/camera.h

@@ -209,7 +209,7 @@ class Camera : public Node {
   int motion_step(const float time) const;
   bool use_motion() const;
 
-  void set_screen_size(const int width_, int height_);
+  bool set_screen_size(const int width_, int height_);
 
  private:
   /* Private utility functions. */

intern/cycles/scene/film.cpp

@@ -120,6 +120,8 @@ NODE_DEFINE(Film)
 
   SOCKET_BOOLEAN(use_approximate_shadow_catcher, "Use Approximate Shadow Catcher", false);
 
+  SOCKET_BOOLEAN(use_sample_count, "Use Sample Count Pass", false);
+
   return type;
 }
 
@@ -468,7 +470,7 @@ bool Film::update_lightgroups(Scene *scene)
   return false;
 }
 
-void Film::update_passes(Scene *scene, bool add_sample_count_pass)
+void Film::update_passes(Scene *scene)
 {
   const Background *background = scene->background;
   const BakeManager *bake_manager = scene->bake_manager.get();
@@ -561,7 +563,8 @@ void Film::update_passes(Scene *scene, bool add_sample_count_pass)
     }
   }
 
-  if (add_sample_count_pass) {
+  /* Add sample count pass for tiled rendering. */
+  if (use_sample_count) {
     if (!Pass::contains(scene->passes, PASS_SAMPLE_COUNT)) {
       add_auto_pass(scene, PASS_SAMPLE_COUNT);
     }

intern/cycles/scene/film.h

@@ -50,6 +50,8 @@ class Film : public Node {
    * shadows can be alpha-overed onto a backdrop. */
   NODE_SOCKET_API(bool, use_approximate_shadow_catcher)
 
+  NODE_SOCKET_API(bool, use_sample_count)
+
  private:
   size_t filter_table_offset_;
   bool prev_have_uv_pass = false;
@@ -70,10 +72,8 @@ class Film : public Node {
 
   bool update_lightgroups(Scene *scene);
 
-  /* Update passes so that they contain all passes required for the configured functionality.
-   *
-   * If `add_sample_count_pass` is true then the SAMPLE_COUNT pass is ensured to be added. */
-  void update_passes(Scene *scene, bool add_sample_count_pass);
+  /* Update passes so that they contain all passes required for the configured functionality. */
+  void update_passes(Scene *scene);
 
   uint get_kernel_features(const Scene *scene) const;
 

intern/cycles/scene/scene.cpp

@@ -188,15 +188,7 @@ void Scene::device_update(Device *device_, Progress &progress)
   });
 
   /* The order of updates is important, because there's dependencies between
-   * the different managers, using data computed by previous managers.
-   *
-   * - Image manager uploads images used by shaders.
-   * - Camera may be used for adaptive subdivision.
-   * - Displacement shader must have all shader data available.
-   * - Light manager needs lookup tables and final mesh data to compute emission
-   * CDF.
-   * - Lookup tables are done a second time to handle film tables
-   */
+   * the different managers, using data computed by previous managers. */
 
   if (film->update_lightgroups(this)) {
     light_manager->tag_update(this, ccl::LightManager::LIGHT_MODIFIED);
@@ -216,6 +208,26 @@ void Scene::device_update(Device *device_, Progress &progress)
     return;
   }
 
+  /* Passes. After shader manager as this depends on the shaders. */
+  film->update_passes(this);
+
+  /* Update kernel features. After shaders and passes since those affect features. */
+  update_kernel_features();
+
+  /* Load render kernels, before uploading most data to the GPU, and before displacement and
+   * background light need to run kernels.
+   *
+   * 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. */
+  mutex.unlock();
+  load_kernels(progress);
+  mutex.lock();
+
+  if (progress.get_cancel() || device->have_error()) {
+    return;
+  }
+
   procedural_manager->update(this, progress);
 
   if (progress.get_cancel()) {
@@ -229,6 +241,7 @@ void Scene::device_update(Device *device_, Progress &progress)
     return;
   }
 
+  /* Camera will be used by adaptive subdivision, so do early. */
   progress.set_status("Updating Camera");
   camera->device_update(device, &dscene, this);
 
@@ -237,11 +250,11 @@ void Scene::device_update(Device *device_, Progress &progress)
   }
 
   geometry_manager->device_update_preprocess(device, this, progress);
-
   if (progress.get_cancel() || device->have_error()) {
     return;
   }
 
+  /* Update objects after geometry preprocessing. */
   progress.set_status("Updating Objects");
   object_manager->device_update(device, &dscene, this, progress);
 
@@ -256,6 +269,7 @@ void Scene::device_update(Device *device_, Progress &progress)
     return;
   }
 
+  /* Camera and shaders must be ready here for adaptive subdivision and displacement. */
   progress.set_status("Updating Meshes");
   geometry_manager->device_update(device, &dscene, this, progress);
 
@@ -263,6 +277,7 @@ void Scene::device_update(Device *device_, Progress &progress)
     return;
   }
 
+  /* Update object flags with final geometry. */
   progress.set_status("Updating Objects Flags");
   object_manager->device_update_flags(device, &dscene, this, progress);
 
@@ -270,6 +285,7 @@ void Scene::device_update(Device *device_, Progress &progress)
     return;
   }
 
+  /* Update BVH primitive objects with final geometry. */
   progress.set_status("Updating Primitive Offsets");
   object_manager->device_update_prim_offsets(device, &dscene, this);
 
@@ -277,6 +293,8 @@ void Scene::device_update(Device *device_, Progress &progress)
     return;
   }
 
+  /* Images last, as they should be more likely to use host memory fallback than geometry.
+   * Some images may have been uploaded early for displacemnet already at this point. */
   progress.set_status("Updating Images");
   image_manager->device_update(device, this, progress);
 
@@ -298,6 +316,7 @@ void Scene::device_update(Device *device_, Progress &progress)
     return;
   }
 
+  /* Light manager needs shaders and final meshes for triangles in light tree. */
   progress.set_status("Updating Lights");
   light_manager->device_update(device, &dscene, this, progress);
 
@@ -319,6 +338,7 @@ void Scene::device_update(Device *device_, Progress &progress)
     return;
   }
 
+  /* Update lookup tabes a second time for film tables. */
   progress.set_status("Updating Lookup Tables");
   lookup_tables->device_update(device, &dscene, this);
 
@@ -466,8 +486,6 @@ void Scene::update_kernel_features()
     return;
   }
 
-  const 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);
@@ -561,6 +579,19 @@ bool Scene::update(Progress &progress)
   return true;
 }
 
+bool Scene::update_camera_resolution(Progress &progress, int width, int height)
+{
+  if (!camera->set_screen_size(width, height)) {
+    return false;
+  }
+
+  camera->device_update(device, &dscene, this);
+
+  progress.set_status("Updating Device", "Writing constant memory");
+  device->const_copy_to("data", &dscene.data, sizeof(dscene.data));
+  return true;
+}
+
 static void log_kernel_features(const uint features)
 {
   VLOG_INFO << "Requested features:\n";
@@ -593,8 +624,6 @@ static void log_kernel_features(const uint features)
 
 bool Scene::load_kernels(Progress &progress)
 {
-  update_kernel_features();
-
   const uint kernel_features = dscene.data.kernel_features;
 
   if (!kernels_loaded || loaded_kernel_features != kernel_features) {

intern/cycles/scene/scene.h

@@ -194,8 +194,8 @@ class Scene : public NodeOwner {
 
   void enable_update_stats();
 
-  bool load_kernels(Progress &progress);
   bool update(Progress &progress);
+  bool update_camera_resolution(Progress &progress, int width, int height);
 
   bool has_shadow_catcher();
   void tag_shadow_catcher_modified();
@@ -252,6 +252,8 @@ class Scene : public NodeOwner {
 
   /* Get size of a volume stack needed to render this scene. */
   int get_volume_stack_size() const;
+
+  bool load_kernels(Progress &progress);
 };
 
 template<> Light *Scene::create_node<Light>();

intern/cycles/session/session.cpp

@@ -305,38 +305,22 @@ RenderWork Session::run_update_for_next_iteration()
 
   thread_scoped_lock scene_lock(scene->mutex);
 
+  /* Perform delayed reset if requested. */
+  const bool reset_buffers = delayed_reset_buffer_params();
+
+  /* Update scene */
+  const bool reset_scene = update_scene(delayed_reset_.do_reset);
+
+  /* Update buffers for new paramters. After scene update which influences the passes used. */
   bool have_tiles = true;
   bool switched_to_new_tile = false;
-  bool did_reset = false;
 
-  /* Perform delayed reset if requested. */
-  {
-    const thread_scoped_lock reset_lock(delayed_reset_.mutex);
-    if (delayed_reset_.do_reset) {
-      did_reset = true;
+  if (reset_buffers) {
+    update_buffers_for_params();
 
-      const thread_scoped_lock buffers_lock(buffers_mutex_);
-      do_delayed_reset();
-
-      /* After reset make sure the tile manager is at the first big tile. */
-      have_tiles = tile_manager_.next();
-      switched_to_new_tile = true;
-    }
-  }
-
-  /* Update number of samples in the integrator.
-   * Ideally this would need to happen once in `Session::set_samples()`, but the issue there is
-   * the initial configuration when Session is created where the `set_samples()` is not used.
-   *
-   * NOTE: Unless reset was requested only allow increasing number of samples. */
-  if (did_reset || scene->integrator->get_aa_samples() < params.samples) {
-    scene->integrator->set_aa_samples(params.samples);
-  }
-
-  if (did_reset) {
-    scene->integrator->set_use_sample_subset(params.use_sample_subset);
-    scene->integrator->set_sample_subset_offset(params.sample_subset_offset);
-    scene->integrator->set_sample_subset_length(params.sample_subset_length);
+    /* After reset make sure the tile manager is at the first big tile. */
+    have_tiles = tile_manager_.next();
+    switched_to_new_tile = true;
   }
 
   /* Update denoiser settings. */
@@ -354,7 +338,7 @@ RenderWork Session::run_update_for_next_iteration()
   /* Update path guiding. */
   {
     const GuidingParams guiding_params = scene->integrator->get_guiding_params(device.get());
-    const bool guiding_reset = (guiding_params.use) ? scene->need_reset(false) : false;
+    const bool guiding_reset = (guiding_params.use) ? reset_scene : false;
     path_trace_->set_guiding_params(guiding_params, guiding_reset);
   }
 
@@ -402,28 +386,17 @@ RenderWork Session::run_update_for_next_iteration()
 
       tile_params.update_offset_stride();
 
-      path_trace_->reset(buffer_params_, tile_params, did_reset);
+      path_trace_->reset(buffer_params_, tile_params, reset_buffers);
     }
 
+    /* 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. */
     const int resolution = render_work.resolution_divider;
     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());
-    }
+    scene->update_camera_resolution(progress, width, height);
 
     /* Unlock scene mutex before loading denoiser kernels, since that may attempt to activate
      * graphics interop, which can deadlock when the scene mutex is still being held. */
@@ -521,11 +494,17 @@ int2 Session::get_effective_tile_size() const
   return make_int2(tile_size, tile_size);
 }
 
-void Session::do_delayed_reset()
+bool Session::delayed_reset_buffer_params()
 {
+  /* Reset buffer parameters, delayed from when we got the reset call so we can complete
+   * rendering the sample. Otherwise e.g. viewport navigation might reset without ever
+   * finishing anything. */
+  const thread_scoped_lock reset_lock(delayed_reset_.mutex);
   if (!delayed_reset_.do_reset) {
-    return;
+    return false;
   }
+
+  const thread_scoped_lock buffers_lock(buffers_mutex_);
   delayed_reset_.do_reset = false;
 
   params = delayed_reset_.session_params;
@@ -540,19 +519,18 @@ void Session::do_delayed_reset()
 
   /* Tile and work scheduling. */
   tile_manager_.reset_scheduling(buffer_params_, get_effective_tile_size());
+
+  return true;
+}
+
+void Session::update_buffers_for_params()
+{
   render_scheduler_.set_sample_params(params.samples,
                                       params.use_sample_subset,
                                       params.sample_subset_offset,
                                       params.sample_subset_length);
   render_scheduler_.reset(buffer_params_);
 
-  /* Passes. */
-  /* When multiple tiles are used SAMPLE_COUNT pass is used to keep track of possible partial
-   * tile results. It is safe to use generic update function here which checks for changes since
-   * changes in tile settings re-creates session, which ensures film is fully updated on tile
-   * changes. */
-  scene->film->update_passes(scene.get(), tile_manager_.has_multiple_tiles());
-
   /* Update for new state of scene and passes. */
   buffer_params_.update_passes(scene->passes);
   tile_manager_.update(buffer_params_, scene.get());
@@ -690,15 +668,32 @@ void Session::wait()
   }
 }
 
-bool Session::update_scene(const int width, const int height)
+bool Session::update_scene(const bool reset_samples)
 {
-  /* 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. */
-  Camera *cam = scene->camera;
-  cam->set_screen_size(width, height);
+  /* Update number of samples in the integrator.
+   * Ideally this would need to happen once in `Session::set_samples()`, but the issue there is
+   * the initial configuration when Session is created where the `set_samples()` is not used.
+   *
+   * NOTE: Unless reset was requested only allow increasing number of samples. */
+  if (reset_samples || scene->integrator->get_aa_samples() < params.samples) {
+    scene->integrator->set_aa_samples(params.samples);
+  }
 
-  return scene->update(progress);
+  scene->integrator->set_use_sample_subset(params.use_sample_subset);
+  scene->integrator->set_sample_subset_offset(params.sample_subset_offset);
+  scene->integrator->set_sample_subset_length(params.sample_subset_length);
+
+  /* When multiple tiles are used SAMPLE_COUNT pass is used to keep track of possible partial
+   * tile results. */
+  scene->film->set_use_sample_count(tile_manager_.has_multiple_tiles());
+
+  const bool reset = scene->need_reset(false);
+
+  if (scene->update(progress)) {
+    profiler.reset(scene->shaders.size(), scene->objects.size());
+  }
+
+  return reset;
 }
 
 static string status_append(const string &status, const string &suffix)

intern/cycles/session/session.h

@@ -204,11 +204,12 @@ class Session {
 
   void run_main_render_loop();
 
-  bool update_scene(const int width, const int height);
+  bool update_scene(const bool reset_samples);
 
   void update_status_time(bool show_pause = false, bool show_done = false);
 
-  void do_delayed_reset();
+  bool delayed_reset_buffer_params();
+  void update_buffers_for_params();
 
   int2 get_effective_tile_size() const;