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

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:

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.

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,

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
  * \{ */