[Bf-blender-cvs] [ed8f3dc9c79] master: Fix T103399: correctly apply SRGB framebuffer and shader conversion mode in Metal.

[Jason Fielder]

Commit: ed8f3dc9c79de72f3b8dd8bdcdf9b47cecb2a541
Author: Jason Fielder
Date:   Sun Jan 8 15:58:05 2023 +0100
Branches: master
https://developer.blender.org/rBed8f3dc9c79de72f3b8dd8bdcdf9b47cecb2a541

Fix T103399: correctly apply SRGB framebuffer and shader conversion mode in Metal.

First binding of a framebuffer lead to an incorrect SRGB conversion state being applied, as attachments, where presence of SRGB is determined, were processed after the SRGB check rather than before.
This DIFF also cleans up SRGB naming conventions and caching of fallback non-srgb texture view, for use when SRGB mode is disabled.

Authored by Apple: Michael Parkin-White

Ref T103399
Ref T96261

Reviewed By: fclem

Maniphest Tasks: T103399, T96261

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

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

M	source/blender/gpu/metal/mtl_framebuffer.hh
M	source/blender/gpu/metal/mtl_framebuffer.mm
M	source/blender/gpu/metal/mtl_texture.hh
M	source/blender/gpu/metal/mtl_texture.mm

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

diff --git a/source/blender/gpu/metal/mtl_framebuffer.hh b/source/blender/gpu/metal/mtl_framebuffer.hh
index 434d1a15b43..2d3d5fb0730 100644
--- a/source/blender/gpu/metal/mtl_framebuffer.hh
+++ b/source/blender/gpu/metal/mtl_framebuffer.hh
@@ -100,9 +100,9 @@ class MTLFrameBuffer : public FrameBuffer {
   /** Whether `MTLRenderPassDescriptor[N]` requires updating with latest state. */
   bool descriptor_dirty_[MTL_FB_CONFIG_MAX];
   /** Whether SRGB is enabled for this frame-buffer configuration. */
-  bool srgb_enabled_;
+  bool enabled_srgb_;
   /** Whether the primary Frame-buffer attachment is an SRGB target or not. */
-  bool is_srgb_;
+  bool srgb_;
 
  public:
   /**
@@ -223,12 +223,12 @@ class MTLFrameBuffer : public FrameBuffer {
 
   bool get_srgb_enabled()
   {
-    return srgb_enabled_;
+    return enabled_srgb_;
   }
 
   bool get_is_srgb()
   {
-    return is_srgb_;
+    return srgb_;
   }
 
  private:
diff --git a/source/blender/gpu/metal/mtl_framebuffer.mm b/source/blender/gpu/metal/mtl_framebuffer.mm
index 467a4a6f412..dc8741e377b 100644
--- a/source/blender/gpu/metal/mtl_framebuffer.mm
+++ b/source/blender/gpu/metal/mtl_framebuffer.mm
@@ -27,8 +27,8 @@ MTLFrameBuffer::MTLFrameBuffer(MTLContext *ctx, const char *name) : FrameBuffer(
   dirty_state_ctx_ = nullptr;
   has_pending_clear_ = false;
   colour_attachment_count_ = 0;
-  srgb_enabled_ = false;
-  is_srgb_ = false;
+  enabled_srgb_ = false;
+  srgb_ = false;
 
   for (int i = 0; i < GPU_FB_MAX_COLOR_ATTACHMENT; i++) {
     mtl_color_attachments_[i].used = false;
@@ -99,19 +99,19 @@ void MTLFrameBuffer::bind(bool enabled_srgb)
     return;
   }
 
+  /* Ensure local MTLAttachment data is up to date. */
+  this->update_attachments(true);
+
   /* Ensure SRGB state is up-to-date and valid. */
-  bool srgb_state_changed = srgb_enabled_ != enabled_srgb;
+  bool srgb_state_changed = enabled_srgb_ != enabled_srgb;
   if (context_->active_fb != this || srgb_state_changed) {
     if (srgb_state_changed) {
       this->mark_dirty();
     }
-    srgb_enabled_ = enabled_srgb;
-    GPU_shader_set_framebuffer_srgb_target(srgb_enabled_ && is_srgb_);
+    enabled_srgb_ = enabled_srgb;
+    GPU_shader_set_framebuffer_srgb_target(enabled_srgb && srgb_);
   }
 
-  /* Ensure local MTLAttachment data is up to date. */
-  this->update_attachments(true);
-
   /* Reset clear state on bind -- Clears and load/store ops are set after binding. */
   this->reset_clear_state();
 
@@ -740,7 +740,7 @@ void MTLFrameBuffer::update_attachments(bool update_viewport)
 
   /* Check whether the first attachment is SRGB. */
   if (first_attachment != GPU_FB_MAX_ATTACHMENT) {
-    is_srgb_ = (first_attachment_mtl.texture->format_get() == GPU_SRGB8_A8);
+    srgb_ = (first_attachment_mtl.texture->format_get() == GPU_SRGB8_A8);
   }
 
   /* Reset viewport and Scissor (If viewport is smaller or equal to the framebuffer size). */
@@ -1617,10 +1617,13 @@ MTLRenderPassDescriptor *MTLFrameBuffer::bake_render_pass_descriptor(bool load_c
         }
 
         /* IF SRGB is enabled, but we are rendering with SRGB disabled, sample texture view. */
-        /* TODO(Metal): Consider caching SRGB texture view. */
         id<MTLTexture> source_color_texture = texture;
-        if (this->get_is_srgb() && !this->get_srgb_enabled()) {
-          source_color_texture = [texture newTextureViewWithPixelFormat:MTLPixelFormatRGBA8Unorm];
+        if (this->get_is_srgb() &&
+            mtl_color_attachments_[attachment_ind].texture->is_format_srgb() &&
+            !this->get_srgb_enabled()) {
+          source_color_texture =
+              mtl_color_attachments_[attachment_ind].texture->get_non_srgb_handle();
+          BLI_assert(source_color_texture != nil);
         }
 
         /* Resolve appropriate load action -- IF force load, perform load.
diff --git a/source/blender/gpu/metal/mtl_texture.hh b/source/blender/gpu/metal/mtl_texture.hh
index 1237e2275b3..7a9cee97d24 100644
--- a/source/blender/gpu/metal/mtl_texture.hh
+++ b/source/blender/gpu/metal/mtl_texture.hh
@@ -180,6 +180,9 @@ class MTLTexture : public Texture {
   uint blit_fb_slice_ = 0;
   uint blit_fb_mip_ = 0;
 
+  /* Non-SRGB texture view, used for when a framebuffer is bound with SRGB disabled. */
+  id<MTLTexture> texture_no_srgb_ = nil;
+
   /* Texture view properties */
   /* In Metal, we use texture views to either limit mipmap ranges,
    * , apply a swizzle mask, or both.
@@ -251,6 +254,7 @@ class MTLTexture : public Texture {
   /* Remove once no longer required -- will just return 0 for now in MTL path. */
   uint gl_bindcode_get() const override;
 
+  bool is_format_srgb();
   bool texture_is_baked();
   const char *get_name()
   {
@@ -305,6 +309,7 @@ class MTLTexture : public Texture {
 
   id<MTLTexture> get_metal_handle();
   id<MTLTexture> get_metal_handle_base();
+  id<MTLTexture> get_non_srgb_handle();
   MTLSamplerState get_sampler_state();
   void blit(id<MTLBlitCommandEncoder> blit_encoder,
             uint src_x_offset,
diff --git a/source/blender/gpu/metal/mtl_texture.mm b/source/blender/gpu/metal/mtl_texture.mm
index 2d6ee9a8e6a..daf3fb8c36e 100644
--- a/source/blender/gpu/metal/mtl_texture.mm
+++ b/source/blender/gpu/metal/mtl_texture.mm
@@ -1933,6 +1933,11 @@ void gpu::MTLTexture::reset()
     is_dirty_ = true;
   }
 
+  if (texture_no_srgb_ != nil) {
+    [texture_no_srgb_ release];
+    texture_no_srgb_ = nil;
+  }
+
   if (mip_swizzle_view_ != nil) {
     [mip_swizzle_view_ release];
     mip_swizzle_view_ = nil;
@@ -1963,6 +1968,25 @@ void gpu::MTLTexture::reset()
 
 /** \} */
 
+/* -------------------------------------------------------------------- */
+/** \name SRGB Handling.
+ * \{ */
+bool MTLTexture::is_format_srgb()
+{
+  return (format_ == GPU_SRGB8_A8);
+}
+
+id<MTLTexture> MTLTexture::get_non_srgb_handle()
+{
+  id<MTLTexture> base_tex = get_metal_handle_base();
+  BLI_assert(base_tex != nil);
+  if (texture_no_srgb_ == nil) {
+    texture_no_srgb_ = [base_tex newTextureViewWithPixelFormat:MTLPixelFormatRGBA8Unorm];
+  }
+  return texture_no_srgb_;
+}
+
+/** \} */
 /* -------------------------------------------------------------------- */
 /** \name Pixel Buffer
  * \{ */