Merge branch 'blender-v3.1-release'

intern/cycles/blender/display_driver.cpp

@@ -480,26 +480,12 @@ class DrawTile {
       return false;
     }
 
-    if (!gl_vertex_buffer) {
-      glGenBuffers(1, &gl_vertex_buffer);
-      if (!gl_vertex_buffer) {
-        LOG(ERROR) << "Error allocating tile VBO.";
-        gl_resources_destroy();
-        return false;
-      }
-    }
-
     return true;
   }
 
   void gl_resources_destroy()
   {
     texture.gl_resources_destroy();
-
-    if (gl_vertex_buffer) {
-      glDeleteBuffers(1, &gl_vertex_buffer);
-      gl_vertex_buffer = 0;
-    }
   }
 
   inline bool ready_to_draw() const
@@ -512,9 +498,6 @@ class DrawTile {
 
   /* Display parameters the texture of this tile has been updated for. */
   BlenderDisplayDriver::Params params;
-
-  /* OpenGL resources needed for drawing. */
-  uint gl_vertex_buffer = 0;
 };
 
 class DrawTileAndPBO {
@@ -560,6 +543,30 @@ struct BlenderDisplayDriver::Tiles {
       tiles.clear();
     }
   } finished_tiles;
+
+  /* OpenGL vertex buffer needed for drawing. */
+  uint gl_vertex_buffer = 0;
+
+  bool gl_resources_ensure()
+  {
+    if (!gl_vertex_buffer) {
+      glGenBuffers(1, &gl_vertex_buffer);
+      if (!gl_vertex_buffer) {
+        LOG(ERROR) << "Error allocating tile VBO.";
+        return false;
+      }
+    }
+
+    return true;
+  }
+
+  void gl_resources_destroy()
+  {
+    if (gl_vertex_buffer) {
+      glDeleteBuffers(1, &gl_vertex_buffer);
+      gl_vertex_buffer = 0;
+    }
+  }
 };
 
 BlenderDisplayDriver::BlenderDisplayDriver(BL::RenderEngine &b_engine, BL::Scene &b_scene)
@@ -626,6 +633,12 @@ bool BlenderDisplayDriver::update_begin(const Params &params,
     need_clear_ = false;
   }
 
+  if (!tiles_->gl_resources_ensure()) {
+    tiles_->gl_resources_destroy();
+    gl_context_disable();
+    return false;
+  }
+
   if (!tiles_->current_tile.gl_resources_ensure()) {
     tiles_->current_tile.gl_resources_destroy();
     gl_context_disable();
@@ -825,7 +838,8 @@ static void vertex_buffer_update(const DisplayDriver::Params &params)
 static void draw_tile(const float2 &zoom,
                       const int texcoord_attribute,
                       const int position_attribute,
-                      const DrawTile &draw_tile)
+                      const DrawTile &draw_tile,
+                      const uint gl_vertex_buffer)
 {
   if (!draw_tile.ready_to_draw()) {
     return;
@@ -834,9 +848,9 @@ static void draw_tile(const float2 &zoom,
   const GLTexture &texture = draw_tile.texture;
 
   DCHECK_NE(texture.gl_id, 0);
-  DCHECK_NE(draw_tile.gl_vertex_buffer, 0);
+  DCHECK_NE(gl_vertex_buffer, 0);
 
-  glBindBuffer(GL_ARRAY_BUFFER, draw_tile.gl_vertex_buffer);
+  glBindBuffer(GL_ARRAY_BUFFER, gl_vertex_buffer);
 
   /* Draw at the parameters for which the texture has been updated for. This allows to always draw
    * texture during bordered-rendered camera view without flickering. The validness of the display
@@ -956,10 +970,14 @@ void BlenderDisplayDriver::draw(const Params &params)
   glEnableVertexAttribArray(texcoord_attribute);
   glEnableVertexAttribArray(position_attribute);
 
-  draw_tile(zoom_, texcoord_attribute, position_attribute, tiles_->current_tile.tile);
+  draw_tile(zoom_,
+            texcoord_attribute,
+            position_attribute,
+            tiles_->current_tile.tile,
+            tiles_->gl_vertex_buffer);
 
   for (const DrawTile &tile : tiles_->finished_tiles.tiles) {
-    draw_tile(zoom_, texcoord_attribute, position_attribute, tile);
+    draw_tile(zoom_, texcoord_attribute, position_attribute, tile, tiles_->gl_vertex_buffer);
   }
 
   display_shader_->unbind();
@@ -1062,6 +1080,7 @@ void BlenderDisplayDriver::gl_resources_destroy()
 
   tiles_->current_tile.gl_resources_destroy();
   tiles_->finished_tiles.gl_resources_destroy_and_clear();
+  tiles_->gl_resources_destroy();
 
   gl_context_disable();
 

intern/cycles/session/session.cpp

@@ -49,12 +49,9 @@ Session::Session(const SessionParams &params_, const SceneParams &scene_params)
 {
   TaskScheduler::init(params.threads);
 
-  session_thread_ = nullptr;
-
   delayed_reset_.do_reset = false;
 
   pause_ = false;
-  cancel_ = false;
   new_work_added_ = false;
 
   device = Device::create(params.device, stats, profiler);
@@ -73,48 +70,79 @@ Session::Session(const SessionParams &params_, const SceneParams &scene_params)
     }
     full_buffer_written_cb(filename);
   };
+
+  /* Create session thread. */
+  session_thread_ = new thread(function_bind(&Session::thread_run, this));
 }
 
 Session::~Session()
 {
+  /* Cancel any ongoing render operation. */
   cancel();
 
-  /* Make sure path tracer is destroyed before the device. This is needed because destruction might
-   * need to access device for device memory free. */
-  /* TODO(sergey): Convert device to be unique_ptr, and rely on C++ to destruct objects in the
+  /* Signal session thread to end. */
+  {
+    thread_scoped_lock session_thread_lock(session_thread_mutex_);
+    session_thread_state_ = SESSION_THREAD_END;
+  }
+  session_thread_cond_.notify_all();
+
+  /* Destroy session thread. */
+  session_thread_->join();
+  delete session_thread_;
+
+  /* Destroy path tracer, before the device. This is needed because destruction might need to
+   * access device for device memory free.
+   * TODO(sergey): Convert device to be unique_ptr, and rely on C++ to destruct objects in the
    * pre-defined order. */
   path_trace_.reset();
 
+  /* Destroy scene and device. */
   delete scene;
   delete device;
 
+  /* Stop task scheduler. */
   TaskScheduler::exit();
 }
 
 void Session::start()
 {
-  if (!session_thread_) {
-    session_thread_ = new thread(function_bind(&Session::run, this));
+  {
+    /* Signal session thread to start rendering. */
+    thread_scoped_lock session_thread_lock(session_thread_mutex_);
+    assert(session_thread_state_ == SESSION_THREAD_WAIT);
+    session_thread_state_ = SESSION_THREAD_RENDER;
   }
+
+  session_thread_cond_.notify_all();
 }
 
 void Session::cancel(bool quick)
 {
-  if (quick && path_trace_) {
-    path_trace_->cancel();
+  /* Check if session thread is rendering. */
+  bool rendering;
+  {
+    thread_scoped_lock session_thread_lock(session_thread_mutex_);
+    rendering = (session_thread_state_ == SESSION_THREAD_RENDER);
   }
 
-  if (session_thread_) {
-    /* wait for session thread to end */
+  if (rendering) {
+    /* Cancel path trace operations. */
+    if (quick && path_trace_) {
+      path_trace_->cancel();
+    }
+
+    /* Cancel other operations. */
     progress.set_cancel("Exiting");
 
+    /* Signal unpause in case the render was paused. */
     {
       thread_scoped_lock pause_lock(pause_mutex_);
       pause_ = false;
-      cancel_ = true;
     }
     pause_cond_.notify_all();
 
+    /* Wait for render thread to be cancelled or finished. */
     wait();
   }
 }
@@ -192,11 +220,46 @@ void Session::run_main_render_loop()
       break;
     }
   }
-
-  path_trace_->flush_display();
 }
 
-void Session::run()
+void Session::thread_run()
+{
+  while (true) {
+    {
+      thread_scoped_lock session_thread_lock(session_thread_mutex_);
+
+      if (session_thread_state_ == SESSION_THREAD_WAIT) {
+        /* Continue waiting for any signal from the main thread. */
+        session_thread_cond_.wait(session_thread_lock);
+        continue;
+      }
+      else if (session_thread_state_ == SESSION_THREAD_END) {
+        /* End thread immediately. */
+        break;
+      }
+    }
+
+    /* Execute a render. */
+    thread_render();
+
+    /* Go back from rendering to waiting. */
+    {
+      thread_scoped_lock session_thread_lock(session_thread_mutex_);
+      if (session_thread_state_ == SESSION_THREAD_RENDER) {
+        session_thread_state_ = SESSION_THREAD_WAIT;
+      }
+    }
+    session_thread_cond_.notify_all();
+  }
+
+  /* Flush any remaining operations and destroy display driver here. This ensure
+   * graphics API resources are created and destroyed all in the session thread,
+   * which can avoid problems contexts and multiple threads. */
+  path_trace_->flush_display();
+  path_trace_->set_display_driver(nullptr);
+}
+
+void Session::thread_render()
 {
   if (params.use_profiling && (params.device.type == DEVICE_CPU)) {
     profiler.start();
@@ -338,9 +401,9 @@ bool Session::run_wait_for_work(const RenderWork &render_work)
   const bool no_work = !render_work;
   update_status_time(pause_, no_work);
 
-  /* Only leave the loop when rendering is not paused. But even if the current render is un-paused
-   * but there is nothing to render keep waiting until new work is added. */
-  while (!cancel_) {
+  /* Only leave the loop when rendering is not paused. But even if the current render is
+   * un-paused but there is nothing to render keep waiting until new work is added. */
+  while (!progress.get_cancel()) {
     scoped_timer pause_timer;
 
     if (!pause_ && (render_work || new_work_added_ || delayed_reset_.do_reset)) {
@@ -427,7 +490,8 @@ void Session::do_delayed_reset()
   tile_manager_.update(buffer_params_, scene);
 
   /* Update temp directory on reset.
-   * This potentially allows to finish the existing rendering with a previously configure temporary
+   * This potentially allows to finish the existing rendering with a previously configure
+   * temporary
    * directory in the host software and switch to a new temp directory when new render starts. */
   tile_manager_.set_temp_dir(params.temp_dir);
 
@@ -544,12 +608,14 @@ double Session::get_estimated_remaining_time() const
 
 void Session::wait()
 {
-  if (session_thread_) {
-    session_thread_->join();
-    delete session_thread_;
+  /* Wait until session thread either is waiting or ending. */
+  while (true) {
+    thread_scoped_lock session_thread_lock(session_thread_mutex_);
+    if (session_thread_state_ != SESSION_THREAD_RENDER) {
+      break;
+    }
+    session_thread_cond_.wait(session_thread_lock);
   }
-
-  session_thread_ = nullptr;
 }
 
 bool Session::update_scene(int width, int height)

intern/cycles/session/session.h

@@ -172,7 +172,8 @@ class Session {
     BufferParams buffer_params;
   } delayed_reset_;
 
-  void run();
+  void thread_run();
+  void thread_render();
 
   /* Update for the new iteration of the main loop in run implementation (run_cpu and run_gpu).
    *
@@ -205,10 +206,19 @@ class Session {
 
   int2 get_effective_tile_size() const;
 
-  thread *session_thread_;
+  /* Session thread that performs rendering tasks decoupled from the thread
+   * controlling the sessions. The thread is created and destroyed along with
+   * the session. */
+  thread *session_thread_ = nullptr;
+  thread_condition_variable session_thread_cond_;
+  thread_mutex session_thread_mutex_;
+  enum {
+    SESSION_THREAD_WAIT,
+    SESSION_THREAD_RENDER,
+    SESSION_THREAD_END,
+  } session_thread_state_ = SESSION_THREAD_WAIT;
 
   bool pause_ = false;
-  bool cancel_ = false;
   bool new_work_added_ = false;
 
   thread_condition_variable pause_cond_;

tests/python/CMakeLists.txt

@@ -683,19 +683,16 @@ if(WITH_CYCLES OR WITH_OPENGL_RENDER_TESTS)
         set(_cycles_render_tests bake;${render_tests};osl)
 
         foreach(render_test ${_cycles_render_tests})
-          # Enable just one simple test for Metal until more tests are passing.
-          if((NOT (_cycles_device MATCHES "METAL")) OR (render_test MATCHES "camera"))
-            add_python_test(
-              cycles_${render_test}_${_cycles_device_lower}
-              ${CMAKE_CURRENT_LIST_DIR}/cycles_render_tests.py
-              -blender "${TEST_BLENDER_EXE}"
-              -testdir "${TEST_SRC_DIR}/render/${render_test}"
-              -idiff "${OPENIMAGEIO_IDIFF}"
-              -outdir "${TEST_OUT_DIR}/cycles"
-              -device ${_cycles_device}
-              -blacklist ${_cycles_blacklist}
-            )
-          endif()
+          add_python_test(
+            cycles_${render_test}_${_cycles_device_lower}
+            ${CMAKE_CURRENT_LIST_DIR}/cycles_render_tests.py
+            -blender "${TEST_BLENDER_EXE}"
+            -testdir "${TEST_SRC_DIR}/render/${render_test}"
+            -idiff "${OPENIMAGEIO_IDIFF}"
+            -outdir "${TEST_OUT_DIR}/cycles"
+            -device ${_cycles_device}
+            -blacklist ${_cycles_blacklist}
+          )
         endforeach()
       endforeach()
     endif()