Cleanup: move Cycles display driver context handling to render module

This is highly coupled to Blender logic so doesn't belong in Cycles.

intern/cycles/blender/CMakeLists.txt

@@ -9,6 +9,7 @@ set(INC
   ../../../source/blender/makesrna
   ../../../source/blender/blenlib
   ../../../source/blender/gpu
+  ../../../source/blender/render
   ${CMAKE_BINARY_DIR}/source/blender/makesrna/intern
 )
 

intern/cycles/blender/display_driver.cpp

@@ -9,21 +9,7 @@
 
 #include "GPU_platform.h"
 
-extern "C" {
-struct RenderEngine;
-
-bool RE_engine_has_render_context(struct RenderEngine *engine);
-void RE_engine_render_context_enable(struct RenderEngine *engine);
-void RE_engine_render_context_disable(struct RenderEngine *engine);
-
-bool DRW_opengl_context_release();
-void DRW_opengl_context_activate(bool drw_state);
-
-void *WM_opengl_context_create();
-void WM_opengl_context_activate(void *gl_context);
-void WM_opengl_context_dispose(void *gl_context);
-void WM_opengl_context_release(void *context);
-}
+#include "RE_engine.h"
 
 CCL_NAMESPACE_BEGIN
 
@@ -559,18 +545,21 @@ struct BlenderDisplayDriver::Tiles {
   }
 };
 
-BlenderDisplayDriver::BlenderDisplayDriver(BL::RenderEngine &b_engine, BL::Scene &b_scene)
+BlenderDisplayDriver::BlenderDisplayDriver(BL::RenderEngine &b_engine,
+                                           BL::Scene &b_scene,
+                                           const bool background)
     : b_engine_(b_engine),
+      background_(background),
       display_shader_(BlenderDisplayShader::create(b_engine, b_scene)),
       tiles_(make_unique<Tiles>())
 {
   /* Create context while on the main thread. */
-  gl_context_create();
+  gpu_context_create();
 }
 
 BlenderDisplayDriver::~BlenderDisplayDriver()
 {
-  gl_resources_destroy();
+  gpu_resources_destroy();
 }
 
 /* --------------------------------------------------------------------
@@ -601,12 +590,12 @@ bool BlenderDisplayDriver::update_begin(const Params &params,
   /* Note that it's the responsibility of BlenderDisplayDriver to ensure updating and drawing
    * the texture does not happen at the same time. This is achieved indirectly.
    *
-   * When enabling the OpenGL context, it uses an internal mutex lock DST.gl_context_lock.
+   * When enabling the OpenGL context, it uses an internal mutex lock DST.gpu_context_lock.
    * This same lock is also held when do_draw() is called, which together ensure mutual
    * exclusion.
    *
    * This locking is not performed on the Cycles side, because that would cause lock inversion. */
-  if (!gl_context_enable()) {
+  if (!gpu_context_enable()) {
     return false;
   }
 
@@ -627,13 +616,13 @@ bool BlenderDisplayDriver::update_begin(const Params &params,
 
   if (!tiles_->gl_resources_ensure()) {
     tiles_->gl_resources_destroy();
-    gl_context_disable();
+    gpu_context_disable();
     return false;
   }
 
   if (!tiles_->current_tile.gl_resources_ensure()) {
     tiles_->current_tile.gl_resources_destroy();
-    gl_context_disable();
+    gpu_context_disable();
     return false;
   }
 
@@ -712,7 +701,7 @@ void BlenderDisplayDriver::update_end()
    * On some older GPUs on macOS, there is a driver crash when updating the texture for viewport
    * renders while Blender is drawing. As a workaround update texture during draw, under assumption
    * that there is no graphics interop on macOS and viewport render has a single tile. */
-  if (use_gl_context_ &&
+  if (!background_ &&
       GPU_type_matches_ex(GPU_DEVICE_NVIDIA, GPU_OS_MAC, GPU_DRIVER_ANY, GPU_BACKEND_ANY)) {
     tiles_->current_tile.need_update_texture_pixels = true;
   }
@@ -723,7 +712,7 @@ void BlenderDisplayDriver::update_end()
   gl_upload_sync_ = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
   glFlush();
 
-  gl_context_disable();
+  gpu_context_disable();
 }
 
 /* --------------------------------------------------------------------
@@ -771,12 +760,12 @@ BlenderDisplayDriver::GraphicsInterop BlenderDisplayDriver::graphics_interop_get
 
 void BlenderDisplayDriver::graphics_interop_activate()
 {
-  gl_context_enable();
+  gpu_context_enable();
 }
 
 void BlenderDisplayDriver::graphics_interop_deactivate()
 {
-  gl_context_disable();
+  gpu_context_disable();
 }
 
 /* --------------------------------------------------------------------
@@ -910,7 +899,7 @@ void BlenderDisplayDriver::flush()
    * If we don't do this, the NVIDIA driver hangs for a few seconds for when ending 3D viewport
    * rendering, for unknown reasons. This was found with NVIDIA driver version 470.73 and a Quadro
    * RTX 6000 on Linux. */
-  if (!gl_context_enable()) {
+  if (!gpu_context_enable()) {
     return;
   }
 
@@ -922,15 +911,12 @@ void BlenderDisplayDriver::flush()
     glWaitSync((GLsync)gl_render_sync_, 0, GL_TIMEOUT_IGNORED);
   }
 
-  gl_context_disable();
+  gpu_context_disable();
 }
 
 void BlenderDisplayDriver::draw(const Params &params)
 {
-  /* See do_update_begin() for why no locking is required here. */
-  if (use_gl_context_) {
-    gl_context_mutex_.lock();
-  }
+  gpu_context_lock();
 
   if (need_clear_) {
     /* Texture is requested to be cleared and was not yet cleared.
@@ -938,9 +924,7 @@ void BlenderDisplayDriver::draw(const Params &params)
      * Do early return which should be equivalent of drawing all-zero texture.
      * Watch out for the lock though so that the clear happening during update is properly
      * synchronized here. */
-    if (use_gl_context_) {
-      gl_context_mutex_.unlock();
-    }
+    gpu_context_unlock();
     return;
   }
 
@@ -996,94 +980,55 @@ void BlenderDisplayDriver::draw(const Params &params)
   gl_render_sync_ = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
   glFlush();
 
+  gpu_context_unlock();
+
   VLOG_DEVICE_STATS << "Display driver number of textures: " << GLTexture::num_used;
   VLOG_DEVICE_STATS << "Display driver number of PBOs: " << GLPixelBufferObject::num_used;
+}
 
-  if (use_gl_context_) {
-    gl_context_mutex_.unlock();
+void BlenderDisplayDriver::gpu_context_create()
+{
+  if (!RE_engine_gpu_context_create(reinterpret_cast<RenderEngine *>(b_engine_.ptr.data))) {
+    LOG(ERROR) << "Error creating OpenGL context.";
   }
 }
 
-void BlenderDisplayDriver::gl_context_create()
+bool BlenderDisplayDriver::gpu_context_enable()
 {
-  /* When rendering in viewport there is no render context available via engine.
-   * Check whether own context is to be created here.
-   *
-   * NOTE: If the `b_engine_`'s context is not available, we are expected to be on a main thread
-   * here. */
-  use_gl_context_ = !RE_engine_has_render_context(
-      reinterpret_cast<RenderEngine *>(b_engine_.ptr.data));
-
-  if (use_gl_context_) {
-    const bool drw_state = DRW_opengl_context_release();
-    gl_context_ = WM_opengl_context_create();
-    if (gl_context_) {
-      /* On Windows an old context is restored after creation, and subsequent release of context
-       * generates a Win32 error. Harmless for users, but annoying to have possible misleading
-       * error prints in the console. */
-#ifndef _WIN32
-      WM_opengl_context_release(gl_context_);
-#endif
-    }
-    else {
-      LOG(ERROR) << "Error creating OpenGL context.";
-    }
-
-    DRW_opengl_context_activate(drw_state);
-  }
+  return RE_engine_gpu_context_enable(reinterpret_cast<RenderEngine *>(b_engine_.ptr.data));
 }
 
-bool BlenderDisplayDriver::gl_context_enable()
+void BlenderDisplayDriver::gpu_context_disable()
 {
-  if (use_gl_context_) {
-    if (!gl_context_) {
-      return false;
-    }
-    gl_context_mutex_.lock();
-    WM_opengl_context_activate(gl_context_);
-    return true;
-  }
-
-  RE_engine_render_context_enable(reinterpret_cast<RenderEngine *>(b_engine_.ptr.data));
-  return true;
+  RE_engine_gpu_context_disable(reinterpret_cast<RenderEngine *>(b_engine_.ptr.data));
 }
 
-void BlenderDisplayDriver::gl_context_disable()
+void BlenderDisplayDriver::gpu_context_destroy()
 {
-  if (use_gl_context_) {
-    if (gl_context_) {
-      WM_opengl_context_release(gl_context_);
-      gl_context_mutex_.unlock();
-    }
-    return;
-  }
-
-  RE_engine_render_context_disable(reinterpret_cast<RenderEngine *>(b_engine_.ptr.data));
+  RE_engine_gpu_context_destroy(reinterpret_cast<RenderEngine *>(b_engine_.ptr.data));
 }
 
-void BlenderDisplayDriver::gl_context_dispose()
+void BlenderDisplayDriver::gpu_context_lock()
 {
-  if (gl_context_) {
-    const bool drw_state = DRW_opengl_context_release();
-
-    WM_opengl_context_activate(gl_context_);
-    WM_opengl_context_dispose(gl_context_);
-
-    DRW_opengl_context_activate(drw_state);
-  }
+  RE_engine_gpu_context_lock(reinterpret_cast<RenderEngine *>(b_engine_.ptr.data));
 }
 
-void BlenderDisplayDriver::gl_resources_destroy()
+void BlenderDisplayDriver::gpu_context_unlock()
 {
-  gl_context_enable();
+  RE_engine_gpu_context_unlock(reinterpret_cast<RenderEngine *>(b_engine_.ptr.data));
+}
+
+void BlenderDisplayDriver::gpu_resources_destroy()
+{
+  gpu_context_enable();
 
   tiles_->current_tile.gl_resources_destroy();
   tiles_->finished_tiles.gl_resources_destroy_and_clear();
   tiles_->gl_resources_destroy();
 
-  gl_context_disable();
+  gpu_context_disable();
 
-  gl_context_dispose();
+  gpu_context_destroy();
 }
 
 CCL_NAMESPACE_END

intern/cycles/blender/display_driver.h

@@ -89,7 +89,7 @@ class BlenderDisplaySpaceShader : public BlenderDisplayShader {
 /* Display driver implementation which is specific for Blender viewport integration. */
 class BlenderDisplayDriver : public DisplayDriver {
  public:
-  BlenderDisplayDriver(BL::RenderEngine &b_engine, BL::Scene &b_scene);
+  BlenderDisplayDriver(BL::RenderEngine &b_engine, BL::Scene &b_scene, const bool background);
   ~BlenderDisplayDriver();
 
   virtual void graphics_interop_activate() override;
@@ -115,23 +115,18 @@ class BlenderDisplayDriver : public DisplayDriver {
   virtual void flush() override;
 
   /* Helper function which allocates new GPU context. */
-  void gl_context_create();
-  bool gl_context_enable();
-  void gl_context_disable();
-  void gl_context_dispose();
+  void gpu_context_create();
+  bool gpu_context_enable();
+  void gpu_context_disable();
+  void gpu_context_destroy();
+  void gpu_context_lock();
+  void gpu_context_unlock();
 
   /* Destroy all GPU resources which are being used by this object. */
-  void gl_resources_destroy();
+  void gpu_resources_destroy();
 
   BL::RenderEngine b_engine_;
-
-  /* OpenGL context which is used the render engine doesn't have its own. */
-  void *gl_context_ = nullptr;
-  /* The when Blender RenderEngine side context is not available and the DisplayDriver is to create
-   * its own context. */
-  bool use_gl_context_ = false;
-  /* Mutex used to guard the `gl_context_`. */
-  thread_mutex gl_context_mutex_;
+  bool background_;
 
   /* Content of the display is to be filled with zeroes. */
   std::atomic<bool> need_clear_ = true;

intern/cycles/blender/session.cpp

@@ -1059,8 +1059,8 @@ void BlenderSession::ensure_display_driver_if_needed()
     return;
   }
 
-  unique_ptr<BlenderDisplayDriver> display_driver = make_unique<BlenderDisplayDriver>(b_engine,
-                                                                                      b_scene);
+  unique_ptr<BlenderDisplayDriver> display_driver = make_unique<BlenderDisplayDriver>(
+      b_engine, b_scene, background);
   display_driver_ = display_driver.get();
   session->set_display_driver(move(display_driver));
 }

source/blender/render/CMakeLists.txt

@@ -17,6 +17,7 @@ set(INC
   ../nodes
   ../sequencer
   ../simulation
+  ../windowmanager
   ../../../intern/atomic
   ../../../intern/guardedalloc
   ../../../intern/mikktspace

source/blender/render/RE_engine.h

@@ -154,6 +154,11 @@ typedef struct RenderEngine {
   ThreadMutex update_render_passes_mutex;
   update_render_passes_cb_t update_render_passes_cb;
   void *update_render_passes_data;
+
+  /* GPU context. */
+  void *gpu_context;
+  ThreadMutex gpu_context_mutex;
+  bool use_drw_render_context;
 } RenderEngine;
 
 RenderEngine *RE_engine_create(RenderEngineType *type);
@@ -239,11 +244,17 @@ struct RenderEngine *RE_engine_get(const struct Render *re);
 bool RE_engine_draw_acquire(struct Render *re);
 void RE_engine_draw_release(struct Render *re);
 
-/* NOTE: Only used for Cycles's BLenderGPUDisplay integration with the draw manager. A subject
- * for re-consideration. Do not use this functionality. */
-bool RE_engine_has_render_context(struct RenderEngine *engine);
-void RE_engine_render_context_enable(struct RenderEngine *engine);
-void RE_engine_render_context_disable(struct RenderEngine *engine);
+/* GPU context for engine to create and update GPU resources in its own thread,
+ * without blocking the main thread. Used by Cycles' display driver to create
+ * display textures. */
+bool RE_engine_gpu_context_create(struct RenderEngine *engine);
+void RE_engine_gpu_context_destroy(struct RenderEngine *engine);
+
+bool RE_engine_gpu_context_enable(struct RenderEngine *engine);
+void RE_engine_gpu_context_disable(struct RenderEngine *engine);
+
+void RE_engine_gpu_context_lock(struct RenderEngine *engine);
+void RE_engine_gpu_context_unlock(struct RenderEngine *engine);
 
 /* Engine Types */
 

source/blender/render/intern/engine.cc

@@ -47,6 +47,7 @@
 #include "DRW_engine.h"
 
 #include "GPU_context.h"
+#include "WM_api.h"
 
 #include "pipeline.h"
 #include "render_result.h"
@@ -140,6 +141,7 @@ RenderEngine *RE_engine_create(RenderEngineType *type)
   engine->type = type;
 
   BLI_mutex_init(&engine->update_render_passes_mutex);
+  BLI_mutex_init(&engine->gpu_context_mutex);
 
   return engine;
 }
@@ -172,6 +174,7 @@ void RE_engine_free(RenderEngine *engine)
 
   engine_depsgraph_free(engine);
 
+  BLI_mutex_end(&engine->gpu_context_mutex);
   BLI_mutex_end(&engine->update_render_passes_mutex);
 
   MEM_freeN(engine);
@@ -1199,27 +1202,110 @@ void RE_engine_tile_highlight_clear_all(RenderEngine *engine)
 /* -------------------------------------------------------------------- */
 /** \name OpenGL context manipulation.
  *
- * NOTE: Only used for Cycles's BLenderGPUDisplay integration with the draw manager. A subject
- * for re-consideration. Do not use this functionality.
+ * GPU context for engine to create and update GPU resources in its own thread,
+ * without blocking the main thread. Used by Cycles' display driver to create
+ * display textures.
+ *
  * \{ */
 
-bool RE_engine_has_render_context(RenderEngine *engine)
+bool RE_engine_gpu_context_create(RenderEngine *engine)
 {
-  if (engine->re == nullptr) {
-    return false;
+  /* If the there already is a draw manager render context available, reuse it. */
+  engine->use_drw_render_context = (engine->re && RE_gl_context_get(engine->re));
+  if (engine->use_drw_render_context) {
+    return true;
   }
 
-  return RE_gl_context_get(engine->re) != nullptr;
+  /* Viewport render case where no render context is available. We are expected to be on
+   * the main thread here to safely create a context. */
+  BLI_assert(BLI_thread_is_main());
+
+  const bool drw_state = DRW_opengl_context_release();
+  engine->gpu_context = WM_opengl_context_create();
+
+  /* On Windows an old context is restored after creation, and subsequent release of context
+   * generates a Win32 error. Harmless for users, but annoying to have possible misleading
+   * error prints in the console. */
+#ifndef _WIN32
+  if (engine->gpu_context) {
+    WM_opengl_context_release(engine->gpu_context);
+  }
+#endif
+
+  DRW_opengl_context_activate(drw_state);
+
+  return engine->gpu_context != nullptr;
 }
 
-void RE_engine_render_context_enable(RenderEngine *engine)
+void RE_engine_gpu_context_destroy(RenderEngine *engine)
 {
-  DRW_render_context_enable(engine->re);
+  if (!engine->gpu_context) {
+    return;
+  }
+
+  BLI_assert(BLI_thread_is_main());
+
+  const bool drw_state = DRW_opengl_context_release();
+
+  WM_opengl_context_activate(engine->gpu_context);
+  WM_opengl_context_dispose(engine->gpu_context);
+
+  DRW_opengl_context_activate(drw_state);
 }
 
-void RE_engine_render_context_disable(RenderEngine *engine)
+bool RE_engine_gpu_context_enable(RenderEngine *engine)
 {
-  DRW_render_context_disable(engine->re);
+  if (engine->use_drw_render_context) {
+    DRW_render_context_enable(engine->re);
+    return true;
+  }
+  else {
+    if (engine->gpu_context) {
+      BLI_mutex_lock(&engine->gpu_context_mutex);
+      WM_opengl_context_activate(engine->gpu_context);
+      return true;
+    }
+    else {
+      return false;
+    }
+  }
+}
+
+void RE_engine_gpu_context_disable(RenderEngine *engine)
+{
+  if (engine->use_drw_render_context) {
+    DRW_render_context_disable(engine->re);
+  }
+  else {
+    if (engine->gpu_context) {
+      WM_opengl_context_release(engine->gpu_context);
+      BLI_mutex_unlock(&engine->gpu_context_mutex);
+    }
+  }
+}
+
+void RE_engine_gpu_context_lock(RenderEngine *engine)
+{
+  if (engine->use_drw_render_context) {
+    /* Locking already handled by the draw manager. */
+  }
+  else {
+    if (engine->gpu_context) {
+      BLI_mutex_lock(&engine->gpu_context_mutex);
+    }
+  }
+}
+
+void RE_engine_gpu_context_unlock(RenderEngine *engine)
+{
+  if (engine->use_drw_render_context) {
+    /* Locking already handled by the draw manager. */
+  }
+  else {
+    if (engine->gpu_context) {
+      BLI_mutex_unlock(&engine->gpu_context_mutex);
+    }
+  }
 }
 
 /** \} */

source/blender/render/intern/pipeline.cc

@@ -80,9 +80,9 @@
 #include "SEQ_relations.h"
 #include "SEQ_render.h"
 
-#include "../../windowmanager/WM_api.h"    /* XXX */
-#include "../../windowmanager/wm_window.h" /* XXX */
 #include "GPU_context.h"
+#include "WM_api.h"
+#include "wm_window.h"
 
 #ifdef WITH_FREESTYLE
 #  include "FRS_freestyle.h"