1 files changed, 160 insertions, 49 deletions

diff --git a/source/blender/gpu/intern/gpu_viewport.c b/source/blender/gpu/intern/gpu_viewport.c
index 57efaf99b8b..753da8544ea 100644
--- a/source/blender/gpu/intern/gpu_viewport.c
+++ b/source/blender/gpu/intern/gpu_viewport.c
@@ -65,6 +65,24 @@ typedef struct ViewportTempTexture {
   GPUTexture *texture;
 } ViewportTempTexture;
 
+/* Struct storing a viewport specific GPUBatch.
+ * The end-goal is to have a single batch shared across viewport and use a model matrix to place
+ * the batch. Due to OCIO and Image/UV editor we are not able to use an model matrix yet. */
+struct GPUViewportBatch {
+  GPUBatch *batch;
+  struct {
+    rctf rect_pos;
+    rctf rect_uv;
+  } last_used_parameters;
+};
+
+static struct {
+  GPUVertFormat format;
+  struct {
+    uint pos, tex_coord;
+  } attr_id;
+} g_viewport = {{0}};
+
 struct GPUViewport {
   int size[2];
   int flag;
@@ -93,10 +111,12 @@ struct GPUViewport {
   /* Color management. */
   ColorManagedViewSettings view_settings;
   ColorManagedDisplaySettings display_settings;
+  CurveMapping *orig_curve_mapping;
   float dither;
   /* TODO(fclem) the uvimage display use the viewport but do not set any view transform for the
    * moment. The end goal would be to let the GPUViewport do the color management. */
   bool do_color_management;
+  struct GPUViewportBatch batch;
 };
 
 enum {
@@ -197,18 +217,6 @@ static void gpu_viewport_framebuffer_view_set(GPUViewport *viewport, int view)
                                     GPU_ATTACHMENT_TEXTURE(dtxl->color_overlay),
                                 });
 
-  if (((viewport->flag & GPU_VIEWPORT_STEREO) != 0)) {
-    GPU_framebuffer_ensure_config(&dfbl->stereo_comp_fb,
-                                  {
-                                      GPU_ATTACHMENT_NONE,
-                                      GPU_ATTACHMENT_TEXTURE(dtxl->color),
-                                      GPU_ATTACHMENT_TEXTURE(dtxl->color_overlay),
-                                  });
-  }
-  else {
-    dfbl->stereo_comp_fb = NULL;
-  }
-
   viewport->active_view = view;
 }
 
@@ -355,7 +363,7 @@ GPUTexture *GPU_viewport_texture_pool_query(
 {
   GPUTexture *tex;
 
-  for (ViewportTempTexture *tmp_tex = viewport->tex_pool.first; tmp_tex; tmp_tex = tmp_tex->next) {
+  LISTBASE_FOREACH (ViewportTempTexture *, tmp_tex, &viewport->tex_pool) {
     if ((GPU_texture_format(tmp_tex->texture) == format) &&
         (GPU_texture_width(tmp_tex->texture) == width) &&
         (GPU_texture_height(tmp_tex->texture) == height)) {
@@ -374,12 +382,10 @@ GPUTexture *GPU_viewport_texture_pool_query(
   }
 
   tex = GPU_texture_create_2d(width, height, format, NULL, NULL);
-  GPU_texture_bind(tex, 0);
   /* Doing filtering for depth does not make sense when not doing shadow mapping,
    * and enabling texture filtering on integer texture make them unreadable. */
   bool do_filter = !GPU_texture_depth(tex) && !GPU_texture_integer(tex);
   GPU_texture_filter_mode(tex, do_filter);
-  GPU_texture_unbind(tex);
 
   ViewportTempTexture *tmp_tex = MEM_callocN(sizeof(ViewportTempTexture), "ViewportTempTexture");
   tmp_tex->texture = tex;
@@ -412,7 +418,7 @@ static void gpu_viewport_texture_pool_clear_users(GPUViewport *viewport)
 
 static void gpu_viewport_texture_pool_free(GPUViewport *viewport)
 {
-  for (ViewportTempTexture *tmp_tex = viewport->tex_pool.first; tmp_tex; tmp_tex = tmp_tex->next) {
+  LISTBASE_FOREACH (ViewportTempTexture *, tmp_tex, &viewport->tex_pool) {
     GPU_texture_free(tmp_tex->texture);
   }
 
@@ -472,9 +478,6 @@ static void gpu_viewport_default_fb_create(GPUViewport *viewport)
   ok = ok && GPU_framebuffer_check_valid(dfbl->color_only_fb, NULL);
   ok = ok && GPU_framebuffer_check_valid(dfbl->depth_only_fb, NULL);
   ok = ok && GPU_framebuffer_check_valid(dfbl->overlay_only_fb, NULL);
-  if (((viewport->flag & GPU_VIEWPORT_STEREO) != 0)) {
-    ok = ok && GPU_framebuffer_check_valid(dfbl->stereo_comp_fb, NULL);
-  }
 cleanup:
   if (!ok) {
     GPU_viewport_free(viewport);
@@ -552,8 +555,43 @@ void GPU_viewport_colorspace_set(GPUViewport *viewport,
                                  ColorManagedDisplaySettings *display_settings,
                                  float dither)
 {
-  memcpy(&viewport->view_settings, view_settings, sizeof(*view_settings));
-  memcpy(&viewport->display_settings, display_settings, sizeof(*display_settings));
+  /**
+   * HACK(fclem): We copy the settings here to avoid use after free if an update frees the scene
+   * and the viewport stays cached (see T75443). But this means the OCIO curve-mapping caching
+   * (which is based on #CurveMap pointer address) cannot operate correctly and it will create
+   * a different OCIO processor for each viewport. We try to only reallocate the curve-map copy
+   * if needed to avoid unneeded cache invalidation.
+   */
+  if (view_settings->curve_mapping) {
+    if (viewport->view_settings.curve_mapping) {
+      if (view_settings->curve_mapping->changed_timestamp !=
+          viewport->view_settings.curve_mapping->changed_timestamp) {
+        BKE_color_managed_view_settings_free(&viewport->view_settings);
+      }
+    }
+  }
+
+  if (viewport->orig_curve_mapping != view_settings->curve_mapping) {
+    viewport->orig_curve_mapping = view_settings->curve_mapping;
+    BKE_color_managed_view_settings_free(&viewport->view_settings);
+  }
+  /* Don't copy the curve mapping already. */
+  CurveMapping *tmp_curve_mapping = view_settings->curve_mapping;
+  CurveMapping *tmp_curve_mapping_vp = viewport->view_settings.curve_mapping;
+  view_settings->curve_mapping = NULL;
+  viewport->view_settings.curve_mapping = NULL;
+
+  BKE_color_managed_view_settings_copy(&viewport->view_settings, view_settings);
+  /* Restore. */
+  view_settings->curve_mapping = tmp_curve_mapping;
+  viewport->view_settings.curve_mapping = tmp_curve_mapping_vp;
+  /* Only copy curvemapping if needed. Avoid uneeded OCIO cache miss. */
+  if (tmp_curve_mapping && viewport->view_settings.curve_mapping == NULL) {
+    BKE_color_managed_view_settings_free(&viewport->view_settings);
+    viewport->view_settings.curve_mapping = BKE_curvemapping_copy(tmp_curve_mapping);
+  }
+
+  BKE_color_managed_display_settings_copy(&viewport->display_settings, display_settings);
   viewport->dither = dither;
   viewport->do_color_management = true;
 }
@@ -570,6 +608,14 @@ void GPU_viewport_stereo_composite(GPUViewport *viewport, Stereo3dFormat *stereo
   DefaultTextureList *dtxl = viewport->txl;
   DefaultFramebufferList *dfbl = viewport->fbl;
 
+  /* The composite framebuffer object needs to be created in the window context. */
+  GPU_framebuffer_ensure_config(&dfbl->stereo_comp_fb,
+                                {
+                                    GPU_ATTACHMENT_NONE,
+                                    GPU_ATTACHMENT_TEXTURE(dtxl->color),
+                                    GPU_ATTACHMENT_TEXTURE(dtxl->color_overlay),
+                                });
+
   GPUVertFormat *vert_format = immVertexFormat();
   uint pos = GPU_vertformat_attr_add(vert_format, "pos", GPU_COMP_F32, 2, GPU_FETCH_FLOAT);
   GPU_framebuffer_bind(dfbl->stereo_comp_fb);
@@ -625,6 +671,76 @@ void GPU_viewport_stereo_composite(GPUViewport *viewport, Stereo3dFormat *stereo
 
   GPU_framebuffer_restore();
 }
+/* -------------------------------------------------------------------- */
+/** \name Viewport Batches
+ * \{ */
+
+static GPUVertFormat *gpu_viewport_batch_format(void)
+{
+  if (g_viewport.format.attr_len == 0) {
+    GPUVertFormat *format = &g_viewport.format;
+    g_viewport.attr_id.pos = GPU_vertformat_attr_add(
+        format, "pos", GPU_COMP_F32, 2, GPU_FETCH_FLOAT);
+    g_viewport.attr_id.tex_coord = GPU_vertformat_attr_add(
+        format, "texCoord", GPU_COMP_F32, 2, GPU_FETCH_FLOAT);
+  }
+  return &g_viewport.format;
+}
+
+static GPUBatch *gpu_viewport_batch_create(const rctf *rect_pos, const rctf *rect_uv)
+{
+  GPUVertBuf *vbo = GPU_vertbuf_create_with_format(gpu_viewport_batch_format());
+  const uint vbo_len = 4;
+  GPU_vertbuf_data_alloc(vbo, vbo_len);
+
+  GPUVertBufRaw pos_step, tex_coord_step;
+  GPU_vertbuf_attr_get_raw_data(vbo, g_viewport.attr_id.pos, &pos_step);
+  GPU_vertbuf_attr_get_raw_data(vbo, g_viewport.attr_id.tex_coord, &tex_coord_step);
+
+  copy_v2_fl2(GPU_vertbuf_raw_step(&pos_step), rect_pos->xmin, rect_pos->ymin);
+  copy_v2_fl2(GPU_vertbuf_raw_step(&tex_coord_step), rect_uv->xmin, rect_uv->ymin);
+  copy_v2_fl2(GPU_vertbuf_raw_step(&pos_step), rect_pos->xmax, rect_pos->ymin);
+  copy_v2_fl2(GPU_vertbuf_raw_step(&tex_coord_step), rect_uv->xmax, rect_uv->ymin);
+  copy_v2_fl2(GPU_vertbuf_raw_step(&pos_step), rect_pos->xmin, rect_pos->ymax);
+  copy_v2_fl2(GPU_vertbuf_raw_step(&tex_coord_step), rect_uv->xmin, rect_uv->ymax);
+  copy_v2_fl2(GPU_vertbuf_raw_step(&pos_step), rect_pos->xmax, rect_pos->ymax);
+  copy_v2_fl2(GPU_vertbuf_raw_step(&tex_coord_step), rect_uv->xmax, rect_uv->ymax);
+
+  return GPU_batch_create_ex(GPU_PRIM_TRI_STRIP, vbo, NULL, GPU_BATCH_OWNS_VBO);
+}
+
+static GPUBatch *gpu_viewport_batch_get(GPUViewport *viewport,
+                                        const rctf *rect_pos,
+                                        const rctf *rect_uv)
+{
+  const float compare_limit = 0.0001f;
+  const bool parameters_changed =
+      (!BLI_rctf_compare(
+           &viewport->batch.last_used_parameters.rect_pos, rect_pos, compare_limit) ||
+       !BLI_rctf_compare(&viewport->batch.last_used_parameters.rect_uv, rect_uv, compare_limit));
+
+  if (viewport->batch.batch && parameters_changed) {
+    GPU_batch_discard(viewport->batch.batch);
+    viewport->batch.batch = NULL;
+  }
+
+  if (!viewport->batch.batch) {
+    viewport->batch.batch = gpu_viewport_batch_create(rect_pos, rect_uv);
+    viewport->batch.last_used_parameters.rect_pos = *rect_pos;
+    viewport->batch.last_used_parameters.rect_uv = *rect_uv;
+  }
+  return viewport->batch.batch;
+}
+
+static void gpu_viewport_batch_free(GPUViewport *viewport)
+{
+  if (viewport->batch.batch) {
+    GPU_batch_discard(viewport->batch.batch);
+    viewport->batch.batch = NULL;
+  }
+}
+
+/** \} */
 
 static void gpu_viewport_draw_colormanaged(GPUViewport *viewport,
                                            const rctf *rect_pos,
@@ -635,13 +751,17 @@ static void gpu_viewport_draw_colormanaged(GPUViewport *viewport,
   GPUTexture *color = dtxl->color;
   GPUTexture *color_overlay = dtxl->color_overlay;
 
-  GPUVertFormat *vert_format = immVertexFormat();
-  uint pos = GPU_vertformat_attr_add(vert_format, "pos", GPU_COMP_F32, 2, GPU_FETCH_FLOAT);
-  uint texco = GPU_vertformat_attr_add(vert_format, "texCoord", GPU_COMP_F32, 2, GPU_FETCH_FLOAT);
-
   bool use_ocio = false;
 
   if (viewport->do_color_management && display_colorspace) {
+    /* During the binding process the last used VertexFormat is tested and can assert as it is not
+     * valid. By calling the `immVertexFormat` the last used VertexFormat is reset and the assert
+     * does not happen. This solves a chicken and egg problem when using GPUBatches. GPUBatches
+     * contain the correct vertex format, but can only bind after the shader is bound.
+     *
+     * Image/UV editor still uses imm, after that has been changed we could move this fix to the
+     * OCIO. */
+    immVertexFormat();
     use_ocio = IMB_colormanagement_setup_glsl_draw_from_space(&viewport->view_settings,
                                                               &viewport->display_settings,
                                                               NULL,
@@ -650,38 +770,26 @@ static void gpu_viewport_draw_colormanaged(GPUViewport *viewport,
                                                               true);
   }
 
-  if (!use_ocio) {
-    immBindBuiltinProgram(GPU_SHADER_2D_IMAGE_OVERLAYS_MERGE);
-    immUniform1i("display_transform", display_colorspace);
-    immUniform1i("image_texture", 0);
-    immUniform1i("overlays_texture", 1);
+  GPUBatch *batch = gpu_viewport_batch_get(viewport, rect_pos, rect_uv);
+  if (use_ocio) {
+    GPU_batch_program_set_imm_shader(batch);
+  }
+  else {
+    GPU_batch_program_set_builtin(batch, GPU_SHADER_2D_IMAGE_OVERLAYS_MERGE);
+    GPU_batch_uniform_1i(batch, "display_transform", display_colorspace);
+    GPU_batch_uniform_1i(batch, "image_texture", 0);
+    GPU_batch_uniform_1i(batch, "overlays_texture", 1);
   }
 
   GPU_texture_bind(color, 0);
   GPU_texture_bind(color_overlay, 1);
-
-  immBegin(GPU_PRIM_TRI_STRIP, 4);
-
-  immAttr2f(texco, rect_uv->xmin, rect_uv->ymin);
-  immVertex2f(pos, rect_pos->xmin, rect_pos->ymin);
-  immAttr2f(texco, rect_uv->xmax, rect_uv->ymin);
-  immVertex2f(pos, rect_pos->xmax, rect_pos->ymin);
-  immAttr2f(texco, rect_uv->xmin, rect_uv->ymax);
-  immVertex2f(pos, rect_pos->xmin, rect_pos->ymax);
-  immAttr2f(texco, rect_uv->xmax, rect_uv->ymax);
-  immVertex2f(pos, rect_pos->xmax, rect_pos->ymax);
-
-  immEnd();
-
+  GPU_batch_draw(batch);
   GPU_texture_unbind(color);
   GPU_texture_unbind(color_overlay);
 
   if (use_ocio) {
     IMB_colormanagement_finish_glsl_draw();
   }
-  else {
-    immUnbindProgram();
-  }
 }
 
 /**
@@ -745,8 +853,8 @@ void GPU_viewport_draw_to_screen_ex(GPUViewport *viewport,
  * Merge and draw the buffers of \a viewport into the currently active framebuffer, performing
  * color transform to display space.
  *
- * \param rect: Coordinates to draw into. By swapping min and max values, drawing can be done with
- *              inversed axis coordinates (upside down or sideways).
+ * \param rect: Coordinates to draw into. By swapping min and max values, drawing can be done
+ * with inversed axis coordinates (upside down or sideways).
  */
 void GPU_viewport_draw_to_screen(GPUViewport *viewport, int view, const rcti *rect)
 {
@@ -923,5 +1031,8 @@ void GPU_viewport_free(GPUViewport *viewport)
   DRW_instance_data_list_free(viewport->idatalist);
   MEM_freeN(viewport->idatalist);
 
+  BKE_color_managed_view_settings_free(&viewport->view_settings);
+  gpu_viewport_batch_free(viewport);
+
   MEM_freeN(viewport);
 }