[Bf-blender-cvs] [3d9f4012dc6] master: Cycles: Fixes for viewport render on Metal drawing backend

[Sergey Sharybin]

Commit: 3d9f4012dc6da72f616cff3ed6afb91f88c84405
Author: Sergey Sharybin
Date:   Fri Dec 2 15:56:12 2022 +0100
Branches: master
https://developer.blender.org/rB3d9f4012dc6da72f616cff3ed6afb91f88c84405

Cycles: Fixes for viewport render on Metal drawing backend

This change fixes issues with viewport rendering when Metal
GPU backend is used for drawing. This is not a default build
configuration and requires the following tweaks:

- Enable WITH_METAL_BACKEND CMake option (set it to on)
- Use `--gpu-backend metal` command line arguments

It also helps using the `--factory-startup` command line
argument to ensure Eevee is not used (it is not ported and
will crash).

The root of the problem was in the use of glViewport().
It is replaced with the GPU_viewport_size_get_i() which
is supposed to be portable equivalent form the GPU module.
Without this change the viewport size is detected to be 0
which backfired in few places.

The rest of the changes were to make the code more robust
in the extreme conditions instead of asserting or crashing.

Simplified and streamlined GPU resources creation in the
display driver. It was a bit convoluted mix of creation of
the GPU resources and resizing them to the proper size. It
even seemed to be done in the reverse order. Now it is as
simple as "just ensure GPU resources are there for the
given texture or buffer size".

Also avoid division by zero in the tile manager.

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

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

M	intern/cycles/blender/display_driver.cpp
M	intern/cycles/blender/python.cpp
M	intern/cycles/session/tile.cpp

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

diff --git a/intern/cycles/blender/display_driver.cpp b/intern/cycles/blender/display_driver.cpp
index 883310e13ae..f07d8b41838 100644
--- a/intern/cycles/blender/display_driver.cpp
+++ b/intern/cycles/blender/display_driver.cpp
@@ -238,12 +238,19 @@ class DisplayGPUTexture {
     return *this;
   }
 
-  bool gpu_resources_ensure()
+  bool gpu_resources_ensure(const uint texture_width, const uint texture_height)
   {
+    if (width != texture_width || height != texture_height) {
+      gpu_resources_destroy();
+    }
+
     if (gpu_texture) {
       return true;
     }
 
+    width = texture_width;
+    height = texture_height;
+
     /* Texture must have a minimum size of 1x1. */
     gpu_texture = GPU_texture_create_2d(
         "CyclesBlitTexture", max(width, 1), max(height, 1), 1, GPU_RGBA16F, nullptr);
@@ -274,16 +281,6 @@ class DisplayGPUTexture {
     --num_used;
   }
 
-  void ensure_size(uint texture_width, uint texture_height)
-  {
-    if (width != texture_width || height != texture_height) {
-      gpu_resources_destroy();
-      width = texture_width;
-      height = texture_height;
-      gpu_resources_ensure();
-    }
-  }
-
   /* Texture resource allocated by the GPU module.
    *
    * NOTE: Allocated on the render engine's context. */
@@ -339,15 +336,15 @@ class DisplayGPUPixelBuffer {
     return *this;
   }
 
-  void ensure_size(uint new_width, uint new_height)
+  bool gpu_resources_ensure(const uint new_width, const uint new_height)
   {
-    size_t required_size = sizeof(half4) * new_width * new_height * 4;
+    const size_t required_size = sizeof(half4) * new_width * new_height * 4;
 
+    /* Try to re-use the existing PBO if it has usable size. */
     if (gpu_pixel_buffer) {
       if (new_width != width || new_height != height ||
           GPU_pixel_buffer_size(gpu_pixel_buffer) < required_size) {
-        GPU_pixel_buffer_free(gpu_pixel_buffer);
-        gpu_pixel_buffer = nullptr;
+        gpu_resources_destroy();
       }
     }
 
@@ -359,12 +356,6 @@ class DisplayGPUPixelBuffer {
     if (!gpu_pixel_buffer) {
       gpu_pixel_buffer = GPU_pixel_buffer_create(required_size);
     }
-  }
-
-  bool gpu_resources_ensure()
-  {
-    /* Create pixel buffer using current size. */
-    ensure_size(width, height);
 
     if (gpu_pixel_buffer == nullptr) {
       LOG(ERROR) << "Error creating texture pixel buffer object.";
@@ -420,16 +411,6 @@ class DrawTile {
 
   DrawTile &operator=(DrawTile &&other) = default;
 
-  bool gpu_resources_ensure()
-  {
-    if (!texture.gpu_resources_ensure()) {
-      gpu_resources_destroy();
-      return false;
-    }
-
-    return true;
-  }
-
   void gpu_resources_destroy()
   {
     texture.gpu_resources_destroy();
@@ -449,16 +430,6 @@ class DrawTile {
 
 class DrawTileAndPBO {
  public:
-  bool gpu_resources_ensure()
-  {
-    if (!tile.gpu_resources_ensure() || !buffer_object.gpu_resources_ensure()) {
-      gpu_resources_destroy();
-      return false;
-    }
-
-    return true;
-  }
-
   void gpu_resources_destroy()
   {
     tile.gpu_resources_destroy();
@@ -560,15 +531,6 @@ bool BlenderDisplayDriver::update_begin(const Params &params,
     need_clear_ = false;
   }
 
-  if (!tiles_->current_tile.gpu_resources_ensure()) {
-    tiles_->current_tile.gpu_resources_destroy();
-    gpu_context_disable();
-    return false;
-  }
-
-  /* Update texture dimensions if needed. */
-  current_tile.texture.ensure_size(texture_width, texture_height);
-
   /* Update PBO dimensions if needed.
    *
    * NOTE: Allocate the PBO for the size which will fit the final render resolution (as in,
@@ -580,7 +542,13 @@ bool BlenderDisplayDriver::update_begin(const Params &params,
    * mode faster. */
   const int buffer_width = params.size.x;
   const int buffer_height = params.size.y;
-  current_tile_buffer_object.ensure_size(buffer_width, buffer_height);
+
+  if (!current_tile_buffer_object.gpu_resources_ensure(buffer_width, buffer_height) ||
+      !current_tile.texture.gpu_resources_ensure(texture_width, texture_height)) {
+    tiles_->current_tile.gpu_resources_destroy();
+    gpu_context_disable();
+    return false;
+  }
 
   /* Store an updated parameters of the current tile.
    * In theory it is only needed once per update of the tile, but doing it on every update is
diff --git a/intern/cycles/blender/python.cpp b/intern/cycles/blender/python.cpp
index 1e9478ea32d..d27bd26f3dc 100644
--- a/intern/cycles/blender/python.cpp
+++ b/intern/cycles/blender/python.cpp
@@ -26,6 +26,8 @@
 #include "util/tbb.h"
 #include "util/types.h"
 
+#include "GPU_state.h"
+
 #ifdef WITH_OSL
 #  include "scene/osl.h"
 
@@ -337,7 +339,7 @@ static PyObject *view_draw_func(PyObject * /*self*/, PyObject *args)
   if (PyLong_AsVoidPtr(pyrv3d)) {
     /* 3d view drawing */
     int viewport[4];
-    glGetIntegerv(GL_VIEWPORT, viewport);
+    GPU_viewport_size_get_i(viewport);
 
     session->view_draw(viewport[2], viewport[3]);
   }
diff --git a/intern/cycles/session/tile.cpp b/intern/cycles/session/tile.cpp
index 362372f1d7b..071c72a2c17 100644
--- a/intern/cycles/session/tile.cpp
+++ b/intern/cycles/session/tile.cpp
@@ -342,8 +342,8 @@ void TileManager::reset_scheduling(const BufferParams &params, int2 tile_size)
 
   tile_size_ = tile_size;
 
-  tile_state_.num_tiles_x = divide_up(params.width, tile_size_.x);
-  tile_state_.num_tiles_y = divide_up(params.height, tile_size_.y);
+  tile_state_.num_tiles_x = tile_size_.x ? divide_up(params.width, tile_size_.x) : 0;
+  tile_state_.num_tiles_y = tile_size_.y ? divide_up(params.height, tile_size_.y) : 0;
   tile_state_.num_tiles = tile_state_.num_tiles_x * tile_state_.num_tiles_y;
 
   tile_state_.next_tile_index = 0;