From 449fce24e1fee33e9403a592d100c3be438081df Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 5 Apr 2023 17:03:48 -0700
Subject: [PATCH 01/18] [Impeller] partial repaint for Impeller/iOS.

---
 shell/gpu/gpu_surface_metal_impeller.h  |  4 ++
 shell/gpu/gpu_surface_metal_impeller.mm | 56 +++++++++++++++++++++----
 2 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/shell/gpu/gpu_surface_metal_impeller.h b/shell/gpu/gpu_surface_metal_impeller.h
index 6254d851a5062..d17b7a8dbf376 100644
--- a/shell/gpu/gpu_surface_metal_impeller.h
+++ b/shell/gpu/gpu_surface_metal_impeller.h
@@ -34,6 +34,10 @@ class SK_API_AVAILABLE_CA_METAL_LAYER GPUSurfaceMetalImpeller : public Surface {
   std::shared_ptr<impeller::Renderer> impeller_renderer_;
   std::shared_ptr<impeller::AiksContext> aiks_context_;
   fml::scoped_nsprotocol<id<MTLDrawable>> last_drawable_;
+  bool disable_partial_repaint_ = false;
+  // Accumulated damage for each framebuffer; Key is address of underlying
+  // MTLTexture for each drawable
+  std::map<uintptr_t, SkIRect> damage_;
 
   // |Surface|
   std::unique_ptr<SurfaceFrame> AcquireFrame(const SkISize& size) override;
diff --git a/shell/gpu/gpu_surface_metal_impeller.mm b/shell/gpu/gpu_surface_metal_impeller.mm
index bad9f26ca780d..cfff491f2d342 100644
--- a/shell/gpu/gpu_surface_metal_impeller.mm
+++ b/shell/gpu/gpu_surface_metal_impeller.mm
@@ -33,7 +33,14 @@
     : delegate_(delegate),
       impeller_renderer_(CreateImpellerRenderer(context)),
       aiks_context_(
-          std::make_shared<impeller::AiksContext>(impeller_renderer_ ? context : nullptr)) {}
+          std::make_shared<impeller::AiksContext>(impeller_renderer_ ? context : nullptr)) {
+  // If this preference is explicitly set, we allow for disabling partial repaint.
+  NSNumber* disablePartialRepaint =
+      [[NSBundle mainBundle] objectForInfoDictionaryKey:@"FLTDisablePartialRepaint"];
+  if (disablePartialRepaint != nil) {
+    disable_partial_repaint_ = disablePartialRepaint.boolValue;
+  }
+}
 
 GPUSurfaceMetalImpeller::~GPUSurfaceMetalImpeller() = default;
 
@@ -65,10 +72,13 @@
     last_drawable_.reset([surface->drawable() retain]);
   }
 
+  id<CAMetalDrawable> metal_drawable = static_cast<id<CAMetalDrawable>>(last_drawable_);
   SurfaceFrame::SubmitCallback submit_callback =
-      fml::MakeCopyable([renderer = impeller_renderer_,  //
+      fml::MakeCopyable([this,                           //
+                         renderer = impeller_renderer_,  //
                          aiks_context = aiks_context_,   //
-                         surface = std::move(surface)    //
+                         surface = std::move(surface),   //
+                         metal_drawable                  //
   ](SurfaceFrame& surface_frame, DlCanvas* canvas) mutable -> bool {
         if (!aiks_context) {
           return false;
@@ -80,6 +90,20 @@
           return false;
         }
 
+        if (!disable_partial_repaint_) {
+          uintptr_t texture = reinterpret_cast<uintptr_t>(metal_drawable.texture);
+          for (auto& entry : damage_) {
+            if (entry.first != texture) {
+              // Accumulate damage for other framebuffers
+              if (surface_frame.submit_info().frame_damage) {
+                entry.second.join(*surface_frame.submit_info().frame_damage);
+              }
+            }
+          }
+          // Reset accumulated damage for current framebuffer
+          damage_[texture] = SkIRect::MakeEmpty();
+        }
+
         impeller::DisplayListDispatcher impeller_dispatcher;
         display_list->Dispatch(impeller_dispatcher);
         auto picture = impeller_dispatcher.EndRecordingAsPicture();
@@ -92,12 +116,26 @@
             }));
       });
 
-  return std::make_unique<SurfaceFrame>(nullptr,                          // surface
-                                        SurfaceFrame::FramebufferInfo{},  // framebuffer info
-                                        submit_callback,                  // submit callback
-                                        frame_info,                       // frame size
-                                        nullptr,                          // context result
-                                        true                              // display list fallback
+  SurfaceFrame::FramebufferInfo framebuffer_info;
+  framebuffer_info.supports_readback = true;
+
+  if (!disable_partial_repaint_) {
+    // Provide accumulated damage to rasterizer (area in current framebuffer that lags behind
+    // front buffer)
+    uintptr_t texture = reinterpret_cast<uintptr_t>(metal_drawable.texture);
+    auto i = damage_.find(texture);
+    if (i != damage_.end()) {
+      framebuffer_info.existing_damage = i->second;
+    }
+    framebuffer_info.supports_partial_repaint = true;
+  }
+
+  return std::make_unique<SurfaceFrame>(nullptr,           // surface
+                                        framebuffer_info,  // framebuffer info
+                                        submit_callback,   // submit callback
+                                        frame_info,        // frame size
+                                        nullptr,           // context result
+                                        true               // display list fallback
   );
 }
 

From c0c227b364168deef9abbc7d24a2a3f44e43858d Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Mon, 10 Apr 2023 18:18:13 -0700
Subject: [PATCH 02/18] use blit to preserve previous frame contents

---
 flow/compositor_context.cc                    | 21 ++++++-----
 impeller/renderer/backend/metal/surface_mtl.h | 17 +++++++--
 .../renderer/backend/metal/surface_mtl.mm     | 36 +++++++++++++++----
 shell/gpu/gpu_surface_metal_impeller.mm       |  6 ++++
 4 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/flow/compositor_context.cc b/flow/compositor_context.cc
index 12e3ed9751ea8..561f8efb3bb17 100644
--- a/flow/compositor_context.cc
+++ b/flow/compositor_context.cc
@@ -130,9 +130,8 @@ RasterStatus CompositorContext::ScopedFrame::Raster(
           ? frame_damage->ComputeClipRect(layer_tree, !ignore_raster_cache)
           : std::nullopt;
 
-  bool root_needs_readback = layer_tree.Preroll(
+  layer_tree.Preroll(
       *this, ignore_raster_cache, clip_rect ? *clip_rect : kGiantRect);
-  bool needs_save_layer = root_needs_readback && !surface_supports_readback();
   PostPrerollResult post_preroll_result = PostPrerollResult::kSuccess;
   if (view_embedder_ && raster_thread_merger_) {
     post_preroll_result =
@@ -146,7 +145,7 @@ RasterStatus CompositorContext::ScopedFrame::Raster(
     return RasterStatus::kSkipAndRetry;
   }
 
-  DlAutoCanvasRestore restore(canvas(), clip_rect.has_value());
+  //DlAutoCanvasRestore restore(canvas(), clip_rect.has_value());
 
   // Clearing canvas after preroll reduces one render target switch when preroll
   // paints some raster cache.
@@ -155,14 +154,14 @@ RasterStatus CompositorContext::ScopedFrame::Raster(
       canvas()->ClipRect(*clip_rect);
     }
 
-    if (needs_save_layer) {
-      TRACE_EVENT0("flutter", "Canvas::saveLayer");
-      SkRect bounds = SkRect::Make(layer_tree.frame_size());
-      DlPaint paint;
-      paint.setBlendMode(DlBlendMode::kSrc);
-      canvas()->SaveLayer(&bounds, &paint);
-    }
-    canvas()->Clear(DlColor::kTransparent());
+    // if (needs_save_layer) {
+    //   TRACE_EVENT0("flutter", "Canvas::saveLayer");
+    //   SkRect bounds = SkRect::Make(layer_tree.frame_size());
+    //   DlPaint paint;
+    //   paint.setBlendMode(DlBlendMode::kSrc);
+    //   canvas()->SaveLayer(&bounds, &paint);
+    // }
+    // canvas()->Clear(DlColor::kTransparent());
   }
   layer_tree.Paint(*this, ignore_raster_cache);
   // The canvas()->Restore() is taken care of by the DlAutoCanvasRestore
diff --git a/impeller/renderer/backend/metal/surface_mtl.h b/impeller/renderer/backend/metal/surface_mtl.h
index e641dd6c23299..7c19467692f90 100644
--- a/impeller/renderer/backend/metal/surface_mtl.h
+++ b/impeller/renderer/backend/metal/surface_mtl.h
@@ -9,6 +9,7 @@
 #include "flutter/fml/macros.h"
 #include "impeller/renderer/context.h"
 #include "impeller/renderer/surface.h"
+#include "impeller/geometry/rect.h"
 
 namespace impeller {
 
@@ -33,7 +34,7 @@ class SurfaceMTL final : public Surface {
   /// @return     A pointer to the wrapped surface or null.
   ///
   static std::unique_ptr<SurfaceMTL> WrapCurrentMetalLayerDrawable(
-      const std::shared_ptr<Context>& context,
+      std::shared_ptr<Context> context,
       CAMetalLayer* layer);
 #pragma GCC diagnostic pop
 
@@ -42,10 +43,20 @@ class SurfaceMTL final : public Surface {
 
   id<MTLDrawable> drawable() const { return drawable_; }
 
+  void SetDamageRect(Scalar x, Scalar y, Scalar width, Scalar height) {
+    damage_rect_ = IRect::MakeXYWH(x,y, width, height);
+  }
+
  private:
-  id<MTLDrawable> drawable_ = nil;
+  std::shared_ptr<Context> context_;
+  std::shared_ptr<Texture> resolve_texture_;
+  id<CAMetalDrawable> drawable_ = nil;
+  std::optional<IRect> damage_rect_ = std::nullopt;
 
-  SurfaceMTL(const RenderTarget& target, id<MTLDrawable> drawable);
+  SurfaceMTL(std::shared_ptr<Context> context,
+             const RenderTarget& target,
+             std::shared_ptr<Texture> resolve_texture,
+             id<CAMetalDrawable> drawable);
 
   // |Surface|
   bool Present() const override;
diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index dccdac147f4e9..ccf7cbb9ad5bd 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -5,6 +5,7 @@
 #include "impeller/renderer/backend/metal/surface_mtl.h"
 
 #include "flutter/fml/trace_event.h"
+#include "flutter/impeller/renderer/command_buffer.h"
 #include "impeller/base/validation.h"
 #include "impeller/renderer/backend/metal/formats_mtl.h"
 #include "impeller/renderer/backend/metal/texture_mtl.h"
@@ -16,7 +17,7 @@
 #pragma GCC diagnostic ignored "-Wunguarded-availability-new"
 
 std::unique_ptr<SurfaceMTL> SurfaceMTL::WrapCurrentMetalLayerDrawable(
-    const std::shared_ptr<Context>& context,
+    std::shared_ptr<Context> context,
     CAMetalLayer* layer) {
   TRACE_EVENT0("impeller", "SurfaceMTL::WrapCurrentMetalLayerDrawable");
 
@@ -67,8 +68,12 @@
   resolve_tex_desc.sample_count = SampleCount::kCount1;
   resolve_tex_desc.storage_mode = StorageMode::kDevicePrivate;
 
+  // Create color resolve texture.
   std::shared_ptr<Texture> resolve_tex =
-      std::make_shared<TextureMTL>(resolve_tex_desc, current_drawable.texture);
+      context->GetResourceAllocator()->CreateTexture(resolve_tex_desc);
+  // std::shared_ptr<Texture> resolve_tex =
+  //     std::make_shared<TextureMTL>(resolve_tex_desc,
+  //     current_drawable.texture);
   if (!resolve_tex) {
     VALIDATION_LOG << "Could not wrap resolve texture.";
     return nullptr;
@@ -111,12 +116,18 @@
   render_target_desc.SetStencilAttachment(stencil0);
 
   // The constructor is private. So make_unique may not be used.
-  return std::unique_ptr<SurfaceMTL>(
-      new SurfaceMTL(render_target_desc, current_drawable));
+  return std::unique_ptr<SurfaceMTL>(new SurfaceMTL(
+      context, render_target_desc, resolve_tex, current_drawable));
 }
 
-SurfaceMTL::SurfaceMTL(const RenderTarget& target, id<MTLDrawable> drawable)
-    : Surface(target), drawable_(drawable) {}
+SurfaceMTL::SurfaceMTL(std::shared_ptr<Context> context,
+                       const RenderTarget& target,
+                       std::shared_ptr<Texture> resolve_texture,
+                       id<CAMetalDrawable> drawable)
+    : Surface(target),
+      context_(context),
+      resolve_texture_(resolve_texture),
+      drawable_(drawable) {}
 
 // |Surface|
 SurfaceMTL::~SurfaceMTL() = default;
@@ -126,6 +137,19 @@
   if (drawable_ == nil) {
     return false;
   }
+  auto command_buffer = context_->CreateCommandBuffer();
+  if (!command_buffer) {
+    return false;
+  }
+
+  auto blit_pass = command_buffer->CreateBlitPass();
+  auto current = TextureMTL::Wrapper({}, drawable_.texture);
+  blit_pass->AddCopy(resolve_texture_, current, damage_rect_.value(),
+                     damage_rect_->origin);
+  blit_pass->EncodeCommands(context_->GetResourceAllocator());
+  if (!command_buffer->SubmitCommands()) {
+    return false;
+  }
 
   [drawable_ present];
   return true;
diff --git a/shell/gpu/gpu_surface_metal_impeller.mm b/shell/gpu/gpu_surface_metal_impeller.mm
index cfff491f2d342..2491f02f15223 100644
--- a/shell/gpu/gpu_surface_metal_impeller.mm
+++ b/shell/gpu/gpu_surface_metal_impeller.mm
@@ -91,7 +91,13 @@
         }
 
         if (!disable_partial_repaint_) {
+          // this isn't correct.
+
           uintptr_t texture = reinterpret_cast<uintptr_t>(metal_drawable.texture);
+          auto damage_rect = damage_[texture];
+          surface->SetDamageRect(damage_rect.x(), damage_rect.y(), damage_rect.width(),
+                                 damage_rect.height());
+
           for (auto& entry : damage_) {
             if (entry.first != texture) {
               // Accumulate damage for other framebuffers

From 4f9686425b6aefb02b039c1791d70ee19fcae683 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Mon, 17 Apr 2023 16:18:20 -0700
Subject: [PATCH 03/18] temp disable playground

---
 flow/compositor_context.cc                    | 12 +--
 flow/compositor_context.h                     |  3 +
 flow/surface_frame.h                          |  2 +
 .../backend/metal/playground_impl_mtl.mm      |  2 +-
 impeller/renderer/backend/metal/surface_mtl.h | 26 +++++--
 .../renderer/backend/metal/surface_mtl.mm     | 75 ++++++++++++++-----
 shell/common/rasterizer.cc                    |  1 +
 shell/gpu/gpu_surface_metal_impeller.mm       | 24 ++++--
 8 files changed, 105 insertions(+), 40 deletions(-)

diff --git a/flow/compositor_context.cc b/flow/compositor_context.cc
index 561f8efb3bb17..cfcbcaac4ced4 100644
--- a/flow/compositor_context.cc
+++ b/flow/compositor_context.cc
@@ -4,6 +4,7 @@
 
 #include "flutter/flow/compositor_context.h"
 
+#include <iostream>
 #include <optional>
 #include <utility>
 #include "flutter/flow/layers/layer_tree.h"
@@ -42,10 +43,11 @@ std::optional<SkRect> FrameDamage::ComputeClipRect(
     damage_ =
         context.ComputeDamage(additional_damage_, horizontal_clip_alignment_,
                               vertical_clip_alignment_);
-    return SkRect::Make(damage_->buffer_damage);
+    cached_clip_rect_ = SkRect::Make(damage_->buffer_damage);
   } else {
-    return std::nullopt;
+    cached_clip_rect_ = std::nullopt;
   }
+  return cached_clip_rect_;
 }
 
 CompositorContext::CompositorContext()
@@ -130,8 +132,8 @@ RasterStatus CompositorContext::ScopedFrame::Raster(
           ? frame_damage->ComputeClipRect(layer_tree, !ignore_raster_cache)
           : std::nullopt;
 
-  layer_tree.Preroll(
-      *this, ignore_raster_cache, clip_rect ? *clip_rect : kGiantRect);
+  layer_tree.Preroll(*this, ignore_raster_cache,
+                     clip_rect ? *clip_rect : kGiantRect);
   PostPrerollResult post_preroll_result = PostPrerollResult::kSuccess;
   if (view_embedder_ && raster_thread_merger_) {
     post_preroll_result =
@@ -145,7 +147,7 @@ RasterStatus CompositorContext::ScopedFrame::Raster(
     return RasterStatus::kSkipAndRetry;
   }
 
-  //DlAutoCanvasRestore restore(canvas(), clip_rect.has_value());
+  // DlAutoCanvasRestore restore(canvas(), clip_rect.has_value());
 
   // Clearing canvas after preroll reduces one render target switch when preroll
   // paints some raster cache.
diff --git a/flow/compositor_context.h b/flow/compositor_context.h
index dbc053590a319..f2159b62d9a79 100644
--- a/flow/compositor_context.h
+++ b/flow/compositor_context.h
@@ -94,12 +94,15 @@ class FrameDamage {
     return damage_ ? std::make_optional(damage_->buffer_damage) : std::nullopt;
   }
 
+  std::optional<SkRect> GetClipRect() const { return cached_clip_rect_; }
+
  private:
   SkIRect additional_damage_ = SkIRect::MakeEmpty();
   std::optional<Damage> damage_;
   const LayerTree* prev_layer_tree_ = nullptr;
   int vertical_clip_alignment_ = 1;
   int horizontal_clip_alignment_ = 1;
+  std::optional<SkRect> cached_clip_rect_;
 };
 
 class CompositorContext {
diff --git a/flow/surface_frame.h b/flow/surface_frame.h
index 31a89d807e989..446955070edcd 100644
--- a/flow/surface_frame.h
+++ b/flow/surface_frame.h
@@ -80,6 +80,8 @@ class SurfaceFrame {
     // Time at which this frame is scheduled to be presented. This is a hint
     // that can be passed to the platform to drop queued frames.
     std::optional<fml::TimePoint> presentation_time;
+
+    std::optional<SkRect> clip_rect;
   };
 
   bool Submit();
diff --git a/impeller/playground/backend/metal/playground_impl_mtl.mm b/impeller/playground/backend/metal/playground_impl_mtl.mm
index 594f8683c72b1..864831e6181c5 100644
--- a/impeller/playground/backend/metal/playground_impl_mtl.mm
+++ b/impeller/playground/backend/metal/playground_impl_mtl.mm
@@ -115,7 +115,7 @@
   data_->metal_layer.drawableSize =
       CGSizeMake(layer_size.width * scale.x, layer_size.height * scale.y);
 
-  return SurfaceMTL::WrapCurrentMetalLayerDrawable(context, data_->metal_layer);
+  return nullptr;
 }
 
 }  // namespace impeller
diff --git a/impeller/renderer/backend/metal/surface_mtl.h b/impeller/renderer/backend/metal/surface_mtl.h
index 7c19467692f90..a237147f1db5a 100644
--- a/impeller/renderer/backend/metal/surface_mtl.h
+++ b/impeller/renderer/backend/metal/surface_mtl.h
@@ -7,9 +7,9 @@
 #include <QuartzCore/CAMetalLayer.h>
 
 #include "flutter/fml/macros.h"
+#include "impeller/geometry/rect.h"
 #include "impeller/renderer/context.h"
 #include "impeller/renderer/surface.h"
-#include "impeller/geometry/rect.h"
 
 namespace impeller {
 
@@ -33,9 +33,14 @@ class SurfaceMTL final : public Surface {
   ///
   /// @return     A pointer to the wrapped surface or null.
   ///
-  static std::unique_ptr<SurfaceMTL> WrapCurrentMetalLayerDrawable(
+  static id<CAMetalDrawable> GetMetalDrawableAndValidate(
       std::shared_ptr<Context> context,
       CAMetalLayer* layer);
+
+  static std::unique_ptr<SurfaceMTL> WrapCurrentMetalLayerDrawable(
+      std::shared_ptr<Context> context,
+      id<CAMetalDrawable> drawable,
+      std::optional<IRect> clip_rect);
 #pragma GCC diagnostic pop
 
   // |Surface|
@@ -43,20 +48,25 @@ class SurfaceMTL final : public Surface {
 
   id<MTLDrawable> drawable() const { return drawable_; }
 
-  void SetDamageRect(Scalar x, Scalar y, Scalar width, Scalar height) {
-    damage_rect_ = IRect::MakeXYWH(x,y, width, height);
-  }
-
  private:
   std::shared_ptr<Context> context_;
   std::shared_ptr<Texture> resolve_texture_;
   id<CAMetalDrawable> drawable_ = nil;
-  std::optional<IRect> damage_rect_ = std::nullopt;
+  bool requires_blit_ = false;
+  std::optional<IRect> clip_rect_;
+
+  /// @brief  If the damage rect is larger than a fixed percentage of the
+  ///         screen. This is used to determine whether or not the additional
+  ///         blit pass required by partial repaint is worthwhile.
+  static bool ShouldPerformPartialRepaint(std::optional<IRect> damage_rect,
+                                          ISize texture_size);
 
   SurfaceMTL(std::shared_ptr<Context> context,
              const RenderTarget& target,
              std::shared_ptr<Texture> resolve_texture,
-             id<CAMetalDrawable> drawable);
+             id<CAMetalDrawable> drawable,
+             bool requires_blit,
+             std::optional<IRect> clip_rect);
 
   // |Surface|
   bool Present() const override;
diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index ccf7cbb9ad5bd..d3fcb98122fe8 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "impeller/renderer/backend/metal/surface_mtl.h"
+#include <iostream>
 
 #include "flutter/fml/trace_event.h"
 #include "flutter/impeller/renderer/command_buffer.h"
@@ -16,10 +17,10 @@
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunguarded-availability-new"
 
-std::unique_ptr<SurfaceMTL> SurfaceMTL::WrapCurrentMetalLayerDrawable(
+id<CAMetalDrawable> SurfaceMTL::GetMetalDrawableAndValidate(
     std::shared_ptr<Context> context,
     CAMetalLayer* layer) {
-  TRACE_EVENT0("impeller", "SurfaceMTL::WrapCurrentMetalLayerDrawable");
+  TRACE_EVENT0("impeller", "SurfaceMTL::GetMetalDrawableAndValidate");
 
   if (context == nullptr || !context->IsValid() || layer == nil) {
     return nullptr;
@@ -35,9 +36,19 @@
     VALIDATION_LOG << "Could not acquire current drawable.";
     return nullptr;
   }
+  return current_drawable;
+}
 
-  const auto color_format =
-      FromMTLPixelFormat(current_drawable.texture.pixelFormat);
+std::unique_ptr<SurfaceMTL> SurfaceMTL::WrapCurrentMetalLayerDrawable(
+    std::shared_ptr<Context> context,
+    id<CAMetalDrawable> drawable,
+    std::optional<IRect> clip_rect) {
+  TRACE_EVENT0("impeller", "SurfaceMTL::WrapCurrentMetalLayerDrawable");
+
+  ISize root_size = {static_cast<ISize::Type>(drawable.texture.width),
+                     static_cast<ISize::Type>(drawable.texture.height)};
+  bool requires_blit = ShouldPerformPartialRepaint(clip_rect, root_size);
+  const auto color_format = FromMTLPixelFormat(drawable.texture.pixelFormat);
 
   if (color_format == PixelFormat::kUnknown) {
     VALIDATION_LOG << "Unknown drawable color format.";
@@ -49,9 +60,7 @@
   msaa_tex_desc.type = TextureType::kTexture2DMultisample;
   msaa_tex_desc.sample_count = SampleCount::kCount4;
   msaa_tex_desc.format = color_format;
-  msaa_tex_desc.size = {
-      static_cast<ISize::Type>(current_drawable.texture.width),
-      static_cast<ISize::Type>(current_drawable.texture.height)};
+  msaa_tex_desc.size = root_size;
   msaa_tex_desc.usage = static_cast<uint64_t>(TextureUsage::kRenderTarget);
 
   auto msaa_tex = context->GetResourceAllocator()->CreateTexture(msaa_tex_desc);
@@ -69,11 +78,15 @@
   resolve_tex_desc.storage_mode = StorageMode::kDevicePrivate;
 
   // Create color resolve texture.
-  std::shared_ptr<Texture> resolve_tex =
-      context->GetResourceAllocator()->CreateTexture(resolve_tex_desc);
-  // std::shared_ptr<Texture> resolve_tex =
-  //     std::make_shared<TextureMTL>(resolve_tex_desc,
-  //     current_drawable.texture);
+  std::shared_ptr<Texture> resolve_tex;
+  if (requires_blit) {
+    resolve_tex =
+        context->GetResourceAllocator()->CreateTexture(resolve_tex_desc);
+  } else {
+    resolve_tex =
+        std::make_shared<TextureMTL>(resolve_tex_desc, drawable.texture);
+  }
+
   if (!resolve_tex) {
     VALIDATION_LOG << "Could not wrap resolve texture.";
     return nullptr;
@@ -116,27 +129,53 @@
   render_target_desc.SetStencilAttachment(stencil0);
 
   // The constructor is private. So make_unique may not be used.
-  return std::unique_ptr<SurfaceMTL>(new SurfaceMTL(
-      context, render_target_desc, resolve_tex, current_drawable));
+  return std::unique_ptr<SurfaceMTL>(new SurfaceMTL(context, render_target_desc,
+                                                    resolve_tex, drawable,
+                                                    requires_blit, clip_rect));
 }
 
 SurfaceMTL::SurfaceMTL(std::shared_ptr<Context> context,
                        const RenderTarget& target,
                        std::shared_ptr<Texture> resolve_texture,
-                       id<CAMetalDrawable> drawable)
+                       id<CAMetalDrawable> drawable,
+                       bool requires_blit,
+                       std::optional<IRect> clip_rect)
     : Surface(target),
       context_(context),
       resolve_texture_(resolve_texture),
-      drawable_(drawable) {}
+      drawable_(drawable),
+      requires_blit_(requires_blit),
+      clip_rect_(clip_rect) {}
 
 // |Surface|
 SurfaceMTL::~SurfaceMTL() = default;
 
+// constexpr Scalar kPartiaRepaintThreshold = 1.0;
+
+bool SurfaceMTL::ShouldPerformPartialRepaint(std::optional<IRect> damage_rect,
+                                             ISize texture_size) {
+  if (!damage_rect.has_value()) {
+    return false;
+  }
+  return true;
+  // return ((damage_rect->size.width / texture_size.width <
+  //          kPartiaRepaintThreshold) ||
+  //         (damage_rect->size.height / texture_size.height <
+  //          kPartiaRepaintThreshold));
+}
+
 // |Surface|
 bool SurfaceMTL::Present() const {
   if (drawable_ == nil) {
     return false;
   }
+  // If there is no partial repaint, then immediately present without a blit
+  // pass.
+  if (!requires_blit_) {
+    [drawable_ present];
+    return true;
+  }
+
   auto command_buffer = context_->CreateCommandBuffer();
   if (!command_buffer) {
     return false;
@@ -144,8 +183,8 @@
 
   auto blit_pass = command_buffer->CreateBlitPass();
   auto current = TextureMTL::Wrapper({}, drawable_.texture);
-  blit_pass->AddCopy(resolve_texture_, current, damage_rect_.value(),
-                     damage_rect_->origin);
+  blit_pass->AddCopy(resolve_texture_, current, clip_rect_.value(),
+                     clip_rect_->origin);
   blit_pass->EncodeCommands(context_->GetResourceAllocator());
   if (!command_buffer->SubmitCommands()) {
     return false;
diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc
index 3b01c5c9bb0a9..5524046ee790f 100644
--- a/shell/common/rasterizer.cc
+++ b/shell/common/rasterizer.cc
@@ -605,6 +605,7 @@ RasterStatus Rasterizer::DrawToSurfaceUnsafe(
     if (damage) {
       submit_info.frame_damage = damage->GetFrameDamage();
       submit_info.buffer_damage = damage->GetBufferDamage();
+      submit_info.clip_rect = damage->GetClipRect();
     }
 
     frame->set_submit_info(submit_info);
diff --git a/shell/gpu/gpu_surface_metal_impeller.mm b/shell/gpu/gpu_surface_metal_impeller.mm
index 297638c9c7044..0feb355e410f3 100644
--- a/shell/gpu/gpu_surface_metal_impeller.mm
+++ b/shell/gpu/gpu_surface_metal_impeller.mm
@@ -66,10 +66,10 @@
 
   auto* mtl_layer = (CAMetalLayer*)layer;
 
-  auto surface = impeller::SurfaceMTL::WrapCurrentMetalLayerDrawable(
+  auto drawable = impeller::SurfaceMTL::GetMetalDrawableAndValidate(
       impeller_renderer_->GetContext(), mtl_layer);
   if (Settings::kSurfaceDataAccessible) {
-    last_drawable_.reset([surface->drawable() retain]);
+    last_drawable_.reset([drawable retain]);
   }
 
   id<CAMetalDrawable> metal_drawable = static_cast<id<CAMetalDrawable>>(last_drawable_);
@@ -77,7 +77,6 @@
       fml::MakeCopyable([this,                           //
                          renderer = impeller_renderer_,  //
                          aiks_context = aiks_context_,   //
-                         surface = std::move(surface),   //
                          metal_drawable                  //
   ](SurfaceFrame& surface_frame, DlCanvas* canvas) mutable -> bool {
         if (!aiks_context) {
@@ -91,12 +90,7 @@
         }
 
         if (!disable_partial_repaint_) {
-          // this isn't correct.
-
           uintptr_t texture = reinterpret_cast<uintptr_t>(metal_drawable.texture);
-          auto damage_rect = damage_[texture];
-          surface->SetDamageRect(damage_rect.x(), damage_rect.y(), damage_rect.width(),
-                                 damage_rect.height());
 
           for (auto& entry : damage_) {
             if (entry.first != texture) {
@@ -110,6 +104,20 @@
           damage_[texture] = SkIRect::MakeEmpty();
         }
 
+        std::optional<impeller::IRect> clip_rect;
+        auto damage_rect = surface_frame.submit_info().clip_rect;
+        if (damage_rect.has_value()) {
+          if (damage_rect->width() == 0 || damage_rect->height() == 0) {
+            return true;
+          }
+
+          clip_rect = impeller::IRect::MakeXYWH(damage_rect->x(), damage_rect->y(),
+                                                damage_rect->width(), damage_rect->height());
+        }
+
+        auto surface = impeller::SurfaceMTL::WrapCurrentMetalLayerDrawable(
+            impeller_renderer_->GetContext(), metal_drawable, clip_rect);
+
         impeller::DisplayListDispatcher impeller_dispatcher;
         display_list->Dispatch(impeller_dispatcher);
         auto picture = impeller_dispatcher.EndRecordingAsPicture();

From 5477505eed6b5e37886f9472904ce1fb1e48ce8d Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Mon, 17 Apr 2023 21:44:40 -0700
Subject: [PATCH 04/18] ++

---
 impeller/renderer/backend/metal/surface_mtl.mm |  5 ++++-
 impeller/renderer/blit_pass.cc                 | 12 ++++++------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index d3fcb98122fe8..73a6b469b4028 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include "impeller/renderer/backend/metal/surface_mtl.h"
-#include <iostream>
 
 #include "flutter/fml/trace_event.h"
 #include "flutter/impeller/renderer/command_buffer.h"
@@ -54,6 +53,10 @@
     VALIDATION_LOG << "Unknown drawable color format.";
     return nullptr;
   }
+  if (requires_blit) {
+    root_size = ISize(clip_rect->size.width + clip_rect->origin.x,
+                      clip_rect->size.height + clip_rect->origin.y);
+  }
 
   TextureDescriptor msaa_tex_desc;
   msaa_tex_desc.storage_mode = StorageMode::kDeviceTransient;
diff --git a/impeller/renderer/blit_pass.cc b/impeller/renderer/blit_pass.cc
index 36ef50f40eecc..15b0c82f6cc5c 100644
--- a/impeller/renderer/blit_pass.cc
+++ b/impeller/renderer/blit_pass.cc
@@ -64,12 +64,12 @@ bool BlitPass::AddCopy(std::shared_ptr<Texture> source,
     return true;  // Nothing to blit.
   }
 
-  // Clip the destination image.
-  source_region = source_region->Intersection(
-      IRect(-destination_origin, destination->GetSize()));
-  if (!source_region.has_value()) {
-    return true;  // Nothing to blit.
-  }
+  // // Clip the destination image.
+  // source_region = source_region->Intersection(
+  //     IRect(-destination_origin, destination->GetSize()));
+  // if (!source_region.has_value()) {
+  //   return true;  // Nothing to blit.
+  // }
 
   return OnCopyTextureToTextureCommand(
       std::move(source), std::move(destination), source_region.value(),

From 503c7e1a319861b96cba4e8f22bbcfc010c93ba2 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Tue, 18 Apr 2023 15:06:37 -0700
Subject: [PATCH 05/18] ++

---
 flow/compositor_context.cc                    | 60 ++++++++++++++-----
 flow/compositor_context.h                     |  9 +++
 impeller/renderer/backend/metal/surface_mtl.h | 12 ++--
 .../renderer/backend/metal/surface_mtl.mm     | 34 ++++++-----
 shell/gpu/gpu_surface_metal_impeller.mm       | 18 +++---
 5 files changed, 87 insertions(+), 46 deletions(-)

diff --git a/flow/compositor_context.cc b/flow/compositor_context.cc
index cfcbcaac4ced4..2a9a3c7a8f242 100644
--- a/flow/compositor_context.cc
+++ b/flow/compositor_context.cc
@@ -132,8 +132,15 @@ RasterStatus CompositorContext::ScopedFrame::Raster(
           ? frame_damage->ComputeClipRect(layer_tree, !ignore_raster_cache)
           : std::nullopt;
 
-  layer_tree.Preroll(*this, ignore_raster_cache,
-                     clip_rect ? *clip_rect : kGiantRect);
+  if (aiks_context_ &&
+      !ShouldPerformPartialRepaint(clip_rect, layer_tree.frame_size())) {
+    clip_rect = std::nullopt;
+    frame_damage->Reset();
+  }
+
+  bool root_needs_readback = layer_tree.Preroll(
+      *this, ignore_raster_cache, clip_rect ? *clip_rect : kGiantRect);
+  bool needs_save_layer = root_needs_readback && !surface_supports_readback();
   PostPrerollResult post_preroll_result = PostPrerollResult::kSuccess;
   if (view_embedder_ && raster_thread_merger_) {
     post_preroll_result =
@@ -147,29 +154,50 @@ RasterStatus CompositorContext::ScopedFrame::Raster(
     return RasterStatus::kSkipAndRetry;
   }
 
-  // DlAutoCanvasRestore restore(canvas(), clip_rect.has_value());
+  DlAutoCanvasRestore restore(canvas(), clip_rect.has_value());
 
-  // Clearing canvas after preroll reduces one render target switch when preroll
-  // paints some raster cache.
   if (canvas()) {
-    if (clip_rect) {
-      canvas()->ClipRect(*clip_rect);
-    }
+    if (aiks_context_) {
+      if (clip_rect) {
+        canvas()->Translate(-clip_rect->x(), -clip_rect->y());
+      }
+    } else {
+      if (clip_rect) {
+        canvas()->ClipRect(*clip_rect);
+      }
 
-    // if (needs_save_layer) {
-    //   TRACE_EVENT0("flutter", "Canvas::saveLayer");
-    //   SkRect bounds = SkRect::Make(layer_tree.frame_size());
-    //   DlPaint paint;
-    //   paint.setBlendMode(DlBlendMode::kSrc);
-    //   canvas()->SaveLayer(&bounds, &paint);
-    // }
-    // canvas()->Clear(DlColor::kTransparent());
+      if (needs_save_layer) {
+        TRACE_EVENT0("flutter", "Canvas::saveLayer");
+        SkRect bounds = SkRect::Make(layer_tree.frame_size());
+        DlPaint paint;
+        paint.setBlendMode(DlBlendMode::kSrc);
+        canvas()->SaveLayer(&bounds, &paint);
+      }
+      canvas()->Clear(DlColor::kTransparent());
+    }
   }
   layer_tree.Paint(*this, ignore_raster_cache);
   // The canvas()->Restore() is taken care of by the DlAutoCanvasRestore
   return RasterStatus::kSuccess;
 }
 
+constexpr float kImpellerRepaintRatio = 0.7f;
+
+bool CompositorContext::ShouldPerformPartialRepaint(
+    std::optional<SkRect> damage_rect,
+    SkISize layer_tree_size) {
+  if (!damage_rect.has_value()) {
+    return false;
+  }
+  if (damage_rect->width() >= layer_tree_size.width() &&
+      damage_rect->height() >= layer_tree_size.height()) {
+    return false;
+  }
+  auto rx = damage_rect->width() / layer_tree_size.width();
+  auto ry = damage_rect->height() / layer_tree_size.height();
+  return rx <= kImpellerRepaintRatio || ry <= kImpellerRepaintRatio;
+}
+
 void CompositorContext::OnGrContextCreated() {
   texture_registry_->OnGrContextCreated();
   raster_cache_.Clear();
diff --git a/flow/compositor_context.h b/flow/compositor_context.h
index f2159b62d9a79..d32b8beb30738 100644
--- a/flow/compositor_context.h
+++ b/flow/compositor_context.h
@@ -96,6 +96,8 @@ class FrameDamage {
 
   std::optional<SkRect> GetClipRect() const { return cached_clip_rect_; }
 
+  void Reset() { cached_clip_rect_ = std::nullopt; }
+
  private:
   SkIRect additional_damage_ = SkIRect::MakeEmpty();
   std::optional<Damage> damage_;
@@ -208,6 +210,13 @@ class CompositorContext {
 
   void EndFrame(ScopedFrame& frame, bool enable_instrumentation);
 
+  /// @brief  If the damage rect is larger than a fixed percentage of the
+  ///         screen. This is used to determine whether or not the additional
+  ///         blit pass required by partial repaint is worthwhile.
+  ///         This is only used for impeller.
+  static bool ShouldPerformPartialRepaint(std::optional<SkRect> damage_rect,
+                                          SkISize layer_tree_size);
+
   FML_DISALLOW_COPY_AND_ASSIGN(CompositorContext);
 };
 
diff --git a/impeller/renderer/backend/metal/surface_mtl.h b/impeller/renderer/backend/metal/surface_mtl.h
index a237147f1db5a..2f99510345f94 100644
--- a/impeller/renderer/backend/metal/surface_mtl.h
+++ b/impeller/renderer/backend/metal/surface_mtl.h
@@ -48,6 +48,9 @@ class SurfaceMTL final : public Surface {
 
   id<MTLDrawable> drawable() const { return drawable_; }
 
+  // |Surface|
+  bool Present() const override;
+
  private:
   std::shared_ptr<Context> context_;
   std::shared_ptr<Texture> resolve_texture_;
@@ -55,11 +58,7 @@ class SurfaceMTL final : public Surface {
   bool requires_blit_ = false;
   std::optional<IRect> clip_rect_;
 
-  /// @brief  If the damage rect is larger than a fixed percentage of the
-  ///         screen. This is used to determine whether or not the additional
-  ///         blit pass required by partial repaint is worthwhile.
-  static bool ShouldPerformPartialRepaint(std::optional<IRect> damage_rect,
-                                          ISize texture_size);
+  static bool ShouldPerformPartialRepaint(std::optional<IRect> damage_rect);
 
   SurfaceMTL(std::shared_ptr<Context> context,
              const RenderTarget& target,
@@ -68,9 +67,6 @@ class SurfaceMTL final : public Surface {
              bool requires_blit,
              std::optional<IRect> clip_rect);
 
-  // |Surface|
-  bool Present() const override;
-
   FML_DISALLOW_COPY_AND_ASSIGN(SurfaceMTL);
 };
 
diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index 73a6b469b4028..3ca3eb941284a 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -44,18 +44,22 @@
     std::optional<IRect> clip_rect) {
   TRACE_EVENT0("impeller", "SurfaceMTL::WrapCurrentMetalLayerDrawable");
 
-  ISize root_size = {static_cast<ISize::Type>(drawable.texture.width),
-                     static_cast<ISize::Type>(drawable.texture.height)};
-  bool requires_blit = ShouldPerformPartialRepaint(clip_rect, root_size);
+  bool requires_blit = ShouldPerformPartialRepaint(clip_rect);
   const auto color_format = FromMTLPixelFormat(drawable.texture.pixelFormat);
 
   if (color_format == PixelFormat::kUnknown) {
     VALIDATION_LOG << "Unknown drawable color format.";
     return nullptr;
   }
+  // compositor_context.cc will offset the rendering by the clip origin. Here we
+  // shrink to the size of the clip. This has the same effect as clipping the
+  // rendering but also creates smaller intermediate passes.
+  ISize root_size;
   if (requires_blit) {
-    root_size = ISize(clip_rect->size.width + clip_rect->origin.x,
-                      clip_rect->size.height + clip_rect->origin.y);
+    root_size = ISize(clip_rect->size.width, clip_rect->size.height);
+  } else {
+    root_size = {static_cast<ISize::Type>(drawable.texture.width),
+                 static_cast<ISize::Type>(drawable.texture.height)};
   }
 
   TextureDescriptor msaa_tex_desc;
@@ -83,6 +87,7 @@
   // Create color resolve texture.
   std::shared_ptr<Texture> resolve_tex;
   if (requires_blit) {
+    resolve_tex_desc.compression_type = CompressionType::kLossy;
     resolve_tex =
         context->GetResourceAllocator()->CreateTexture(resolve_tex_desc);
   } else {
@@ -153,18 +158,19 @@
 // |Surface|
 SurfaceMTL::~SurfaceMTL() = default;
 
-// constexpr Scalar kPartiaRepaintThreshold = 1.0;
-
-bool SurfaceMTL::ShouldPerformPartialRepaint(std::optional<IRect> damage_rect,
-                                             ISize texture_size) {
+bool SurfaceMTL::ShouldPerformPartialRepaint(std::optional<IRect> damage_rect) {
+  // compositor_context.cc will conditionally disable partial repaint if the
+  // damage region is large. If that happened, then a nullopt damage rect
+  // will be provided here.
   if (!damage_rect.has_value()) {
     return false;
   }
+  // If the damage rect is 0 in at least one dimension, partial repaint isn't
+  // performed as we skip right to present.
+  if (damage_rect->size.width <= 0 || damage_rect->size.height <= 0) {
+    return false;
+  }
   return true;
-  // return ((damage_rect->size.width / texture_size.width <
-  //          kPartiaRepaintThreshold) ||
-  //         (damage_rect->size.height / texture_size.height <
-  //          kPartiaRepaintThreshold));
 }
 
 // |Surface|
@@ -186,7 +192,7 @@
 
   auto blit_pass = command_buffer->CreateBlitPass();
   auto current = TextureMTL::Wrapper({}, drawable_.texture);
-  blit_pass->AddCopy(resolve_texture_, current, clip_rect_.value(),
+  blit_pass->AddCopy(resolve_texture_, current, std::nullopt,
                      clip_rect_->origin);
   blit_pass->EncodeCommands(context_->GetResourceAllocator());
   if (!command_buffer->SubmitCommands()) {
diff --git a/shell/gpu/gpu_surface_metal_impeller.mm b/shell/gpu/gpu_surface_metal_impeller.mm
index 0feb355e410f3..c66b4b262fa4b 100644
--- a/shell/gpu/gpu_surface_metal_impeller.mm
+++ b/shell/gpu/gpu_surface_metal_impeller.mm
@@ -7,6 +7,7 @@
 #import <Metal/Metal.h>
 #import <QuartzCore/QuartzCore.h>
 
+#include <iostream>
 #include "flutter/common/settings.h"
 #include "flutter/fml/make_copyable.h"
 #include "flutter/fml/mapping.h"
@@ -105,19 +106,20 @@
         }
 
         std::optional<impeller::IRect> clip_rect;
-        auto damage_rect = surface_frame.submit_info().clip_rect;
-        if (damage_rect.has_value()) {
-          if (damage_rect->width() == 0 || damage_rect->height() == 0) {
-            return true;
-          }
-
-          clip_rect = impeller::IRect::MakeXYWH(damage_rect->x(), damage_rect->y(),
-                                                damage_rect->width(), damage_rect->height());
+        if (surface_frame.submit_info().clip_rect.has_value()) {
+          auto submit_info_rect = surface_frame.submit_info().clip_rect;
+          clip_rect =
+              impeller::IRect::MakeXYWH(submit_info_rect->x(), submit_info_rect->y(),
+                                        submit_info_rect->width(), submit_info_rect->height());
         }
 
         auto surface = impeller::SurfaceMTL::WrapCurrentMetalLayerDrawable(
             impeller_renderer_->GetContext(), metal_drawable, clip_rect);
 
+        if (clip_rect && (clip_rect->size.width == 0 || clip_rect->size.height == 0)) {
+          return surface->Present();
+        }
+
         impeller::DisplayListDispatcher impeller_dispatcher;
         display_list->Dispatch(impeller_dispatcher);
         auto picture = impeller_dispatcher.EndRecordingAsPicture();

From 3488b5912c87548d58bc9944b040275c40bd6f01 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Tue, 18 Apr 2023 15:18:24 -0700
Subject: [PATCH 06/18] ++

---
 flow/compositor_context.cc                               | 1 -
 impeller/playground/backend/metal/playground_impl_mtl.mm | 4 +++-
 impeller/renderer/backend/metal/surface_mtl.h            | 2 +-
 shell/gpu/gpu_surface_metal_impeller.mm                  | 1 -
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/flow/compositor_context.cc b/flow/compositor_context.cc
index 2a9a3c7a8f242..684aa1824d374 100644
--- a/flow/compositor_context.cc
+++ b/flow/compositor_context.cc
@@ -4,7 +4,6 @@
 
 #include "flutter/flow/compositor_context.h"
 
-#include <iostream>
 #include <optional>
 #include <utility>
 #include "flutter/flow/layers/layer_tree.h"
diff --git a/impeller/playground/backend/metal/playground_impl_mtl.mm b/impeller/playground/backend/metal/playground_impl_mtl.mm
index 864831e6181c5..033824b11383b 100644
--- a/impeller/playground/backend/metal/playground_impl_mtl.mm
+++ b/impeller/playground/backend/metal/playground_impl_mtl.mm
@@ -115,7 +115,9 @@
   data_->metal_layer.drawableSize =
       CGSizeMake(layer_size.width * scale.x, layer_size.height * scale.y);
 
-  return nullptr;
+  auto drawable =
+      SurfaceMTL::GetMetalDrawableAndValidate(context, data_->metal_layer);
+  return SurfaceMTL::WrapCurrentMetalLayerDrawable(context, drawable);
 }
 
 }  // namespace impeller
diff --git a/impeller/renderer/backend/metal/surface_mtl.h b/impeller/renderer/backend/metal/surface_mtl.h
index 2f99510345f94..f9ba7675648d0 100644
--- a/impeller/renderer/backend/metal/surface_mtl.h
+++ b/impeller/renderer/backend/metal/surface_mtl.h
@@ -40,7 +40,7 @@ class SurfaceMTL final : public Surface {
   static std::unique_ptr<SurfaceMTL> WrapCurrentMetalLayerDrawable(
       std::shared_ptr<Context> context,
       id<CAMetalDrawable> drawable,
-      std::optional<IRect> clip_rect);
+      std::optional<IRect> clip_rect = std::nullopt);
 #pragma GCC diagnostic pop
 
   // |Surface|
diff --git a/shell/gpu/gpu_surface_metal_impeller.mm b/shell/gpu/gpu_surface_metal_impeller.mm
index c66b4b262fa4b..af239513d2efe 100644
--- a/shell/gpu/gpu_surface_metal_impeller.mm
+++ b/shell/gpu/gpu_surface_metal_impeller.mm
@@ -7,7 +7,6 @@
 #import <Metal/Metal.h>
 #import <QuartzCore/QuartzCore.h>
 
-#include <iostream>
 #include "flutter/common/settings.h"
 #include "flutter/fml/make_copyable.h"
 #include "flutter/fml/mapping.h"

From 5081bb70d96dd1246a3c7e97291c965f04a5092d Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Tue, 18 Apr 2023 16:45:55 -0700
Subject: [PATCH 07/18] tidy

---
 impeller/renderer/backend/metal/surface_mtl.h  | 8 ++++----
 impeller/renderer/backend/metal/surface_mtl.mm | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/impeller/renderer/backend/metal/surface_mtl.h b/impeller/renderer/backend/metal/surface_mtl.h
index f9ba7675648d0..23cb134e8fb82 100644
--- a/impeller/renderer/backend/metal/surface_mtl.h
+++ b/impeller/renderer/backend/metal/surface_mtl.h
@@ -34,11 +34,11 @@ class SurfaceMTL final : public Surface {
   /// @return     A pointer to the wrapped surface or null.
   ///
   static id<CAMetalDrawable> GetMetalDrawableAndValidate(
-      std::shared_ptr<Context> context,
+      const std::shared_ptr<Context>& context,
       CAMetalLayer* layer);
 
   static std::unique_ptr<SurfaceMTL> WrapCurrentMetalLayerDrawable(
-      std::shared_ptr<Context> context,
+      const std::shared_ptr<Context>& context,
       id<CAMetalDrawable> drawable,
       std::optional<IRect> clip_rect = std::nullopt);
 #pragma GCC diagnostic pop
@@ -52,7 +52,7 @@ class SurfaceMTL final : public Surface {
   bool Present() const override;
 
  private:
-  std::shared_ptr<Context> context_;
+  const std::shared_ptr<Context>& context_;
   std::shared_ptr<Texture> resolve_texture_;
   id<CAMetalDrawable> drawable_ = nil;
   bool requires_blit_ = false;
@@ -60,7 +60,7 @@ class SurfaceMTL final : public Surface {
 
   static bool ShouldPerformPartialRepaint(std::optional<IRect> damage_rect);
 
-  SurfaceMTL(std::shared_ptr<Context> context,
+  SurfaceMTL(const std::shared_ptr<Context>& context,
              const RenderTarget& target,
              std::shared_ptr<Texture> resolve_texture,
              id<CAMetalDrawable> drawable,
diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index 3ca3eb941284a..20542d453c4db 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -17,7 +17,7 @@
 #pragma GCC diagnostic ignored "-Wunguarded-availability-new"
 
 id<CAMetalDrawable> SurfaceMTL::GetMetalDrawableAndValidate(
-    std::shared_ptr<Context> context,
+    const std::shared_ptr<Context>& context,
     CAMetalLayer* layer) {
   TRACE_EVENT0("impeller", "SurfaceMTL::GetMetalDrawableAndValidate");
 
@@ -39,7 +39,7 @@
 }
 
 std::unique_ptr<SurfaceMTL> SurfaceMTL::WrapCurrentMetalLayerDrawable(
-    std::shared_ptr<Context> context,
+    const std::shared_ptr<Context>& context,
     id<CAMetalDrawable> drawable,
     std::optional<IRect> clip_rect) {
   TRACE_EVENT0("impeller", "SurfaceMTL::WrapCurrentMetalLayerDrawable");
@@ -142,7 +142,7 @@
                                                     requires_blit, clip_rect));
 }
 
-SurfaceMTL::SurfaceMTL(std::shared_ptr<Context> context,
+SurfaceMTL::SurfaceMTL(const std::shared_ptr<Context>& context,
                        const RenderTarget& target,
                        std::shared_ptr<Texture> resolve_texture,
                        id<CAMetalDrawable> drawable,
@@ -150,7 +150,7 @@
                        std::optional<IRect> clip_rect)
     : Surface(target),
       context_(context),
-      resolve_texture_(resolve_texture),
+      resolve_texture_(std::move(resolve_texture)),
       drawable_(drawable),
       requires_blit_(requires_blit),
       clip_rect_(clip_rect) {}

From fc3ef297728f82e44ca8945056ba33a964a34e8c Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 19 Apr 2023 12:03:30 -0700
Subject: [PATCH 08/18] remove debug feature

---
 flow/compositor_context.cc                    | 55 +++++++++++++------
 flow/compositor_context.h                     | 16 ++++--
 flow/surface_frame.h                          |  2 +
 .../renderer/backend/metal/surface_mtl.mm     |  1 -
 4 files changed, 51 insertions(+), 23 deletions(-)

diff --git a/flow/compositor_context.cc b/flow/compositor_context.cc
index 684aa1824d374..79044daab00e1 100644
--- a/flow/compositor_context.cc
+++ b/flow/compositor_context.cc
@@ -153,31 +153,50 @@ RasterStatus CompositorContext::ScopedFrame::Raster(
     return RasterStatus::kSkipAndRetry;
   }
 
+  if (aiks_context_) {
+    PaintLayerTreeImpeller(layer_tree, clip_rect, ignore_raster_cache);
+  } else {
+    PaintLayerTreeSkia(layer_tree, clip_rect, needs_save_layer,
+                       ignore_raster_cache);
+  }
+  return RasterStatus::kSuccess;
+}
+
+void CompositorContext::ScopedFrame::PaintLayerTreeSkia(
+    flutter::LayerTree& layer_tree,
+    std::optional<SkRect> clip_rect,
+    bool needs_save_layer,
+    bool ignore_raster_cache) {
   DlAutoCanvasRestore restore(canvas(), clip_rect.has_value());
 
   if (canvas()) {
-    if (aiks_context_) {
-      if (clip_rect) {
-        canvas()->Translate(-clip_rect->x(), -clip_rect->y());
-      }
-    } else {
-      if (clip_rect) {
-        canvas()->ClipRect(*clip_rect);
-      }
+    if (clip_rect) {
+      canvas()->ClipRect(*clip_rect);
+    }
 
-      if (needs_save_layer) {
-        TRACE_EVENT0("flutter", "Canvas::saveLayer");
-        SkRect bounds = SkRect::Make(layer_tree.frame_size());
-        DlPaint paint;
-        paint.setBlendMode(DlBlendMode::kSrc);
-        canvas()->SaveLayer(&bounds, &paint);
-      }
-      canvas()->Clear(DlColor::kTransparent());
+    if (needs_save_layer) {
+      TRACE_EVENT0("flutter", "Canvas::saveLayer");
+      SkRect bounds = SkRect::Make(layer_tree.frame_size());
+      DlPaint paint;
+      paint.setBlendMode(DlBlendMode::kSrc);
+      canvas()->SaveLayer(&bounds, &paint);
     }
+    canvas()->Clear(DlColor::kTransparent());
   }
-  layer_tree.Paint(*this, ignore_raster_cache);
+
   // The canvas()->Restore() is taken care of by the DlAutoCanvasRestore
-  return RasterStatus::kSuccess;
+  layer_tree.Paint(*this, ignore_raster_cache);
+}
+
+void CompositorContext::ScopedFrame::PaintLayerTreeImpeller(
+    flutter::LayerTree& layer_tree,
+    std::optional<SkRect> clip_rect,
+    bool ignore_raster_cache) {
+  if (canvas() && clip_rect) {
+    canvas()->Translate(-clip_rect->x(), -clip_rect->y());
+  }
+
+  layer_tree.Paint(*this, ignore_raster_cache);
 }
 
 constexpr float kImpellerRepaintRatio = 0.7f;
diff --git a/flow/compositor_context.h b/flow/compositor_context.h
index d32b8beb30738..2b298cf9da558 100644
--- a/flow/compositor_context.h
+++ b/flow/compositor_context.h
@@ -149,6 +149,15 @@ class CompositorContext {
                                 FrameDamage* frame_damage);
 
    private:
+    void PaintLayerTreeSkia(flutter::LayerTree& layer_tree,
+                            std::optional<SkRect> clip_rect,
+                            bool needs_save_layer,
+                            bool ignore_raster_cache);
+
+    void PaintLayerTreeImpeller(flutter::LayerTree& layer_tree,
+                                std::optional<SkRect> clip_rect,
+                                bool ignore_raster_cache);
+
     CompositorContext& context_;
     GrDirectContext* gr_context_;
     DlCanvas* canvas_;
@@ -210,10 +219,9 @@ class CompositorContext {
 
   void EndFrame(ScopedFrame& frame, bool enable_instrumentation);
 
-  /// @brief  If the damage rect is larger than a fixed percentage of the
-  ///         screen. This is used to determine whether or not the additional
-  ///         blit pass required by partial repaint is worthwhile.
-  ///         This is only used for impeller.
+  /// @brief  Whether Impeller shouild attempt a partial repaint.
+  ///         The Impeller backend requires an additional blit pass, which may
+  ///         not be worthwhile if the damage region is large.
   static bool ShouldPerformPartialRepaint(std::optional<SkRect> damage_rect,
                                           SkISize layer_tree_size);
 
diff --git a/flow/surface_frame.h b/flow/surface_frame.h
index 446955070edcd..877b0ba48d075 100644
--- a/flow/surface_frame.h
+++ b/flow/surface_frame.h
@@ -81,6 +81,8 @@ class SurfaceFrame {
     // that can be passed to the platform to drop queued frames.
     std::optional<fml::TimePoint> presentation_time;
 
+    // The clip rect applied to the layer tree for partial repaint. This is
+    // used by the Impeller backend to determine the correct blit command.
     std::optional<SkRect> clip_rect;
   };
 
diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index 20542d453c4db..7b83591ddc3ec 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -189,7 +189,6 @@
   if (!command_buffer) {
     return false;
   }
-
   auto blit_pass = command_buffer->CreateBlitPass();
   auto current = TextureMTL::Wrapper({}, drawable_.texture);
   blit_pass->AddCopy(resolve_texture_, current, std::nullopt,

From 00dcdbaae67ca5a77669842bbbf0762bbb870d1b Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 19 Apr 2023 12:53:17 -0700
Subject: [PATCH 09/18] ++

---
 impeller/renderer/backend/metal/surface_mtl.h  |  6 +++---
 impeller/renderer/backend/metal/surface_mtl.mm | 12 ++++++------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/impeller/renderer/backend/metal/surface_mtl.h b/impeller/renderer/backend/metal/surface_mtl.h
index 23cb134e8fb82..b15beb7fc1e28 100644
--- a/impeller/renderer/backend/metal/surface_mtl.h
+++ b/impeller/renderer/backend/metal/surface_mtl.h
@@ -38,7 +38,7 @@ class SurfaceMTL final : public Surface {
       CAMetalLayer* layer);
 
   static std::unique_ptr<SurfaceMTL> WrapCurrentMetalLayerDrawable(
-      const std::shared_ptr<Context>& context,
+      const std::shared_ptr<Context> context,
       id<CAMetalDrawable> drawable,
       std::optional<IRect> clip_rect = std::nullopt);
 #pragma GCC diagnostic pop
@@ -52,7 +52,7 @@ class SurfaceMTL final : public Surface {
   bool Present() const override;
 
  private:
-  const std::shared_ptr<Context>& context_;
+  const std::shared_ptr<Context> context_;
   std::shared_ptr<Texture> resolve_texture_;
   id<CAMetalDrawable> drawable_ = nil;
   bool requires_blit_ = false;
@@ -60,7 +60,7 @@ class SurfaceMTL final : public Surface {
 
   static bool ShouldPerformPartialRepaint(std::optional<IRect> damage_rect);
 
-  SurfaceMTL(const std::shared_ptr<Context>& context,
+  SurfaceMTL(const std::shared_ptr<Context> context,
              const RenderTarget& target,
              std::shared_ptr<Texture> resolve_texture,
              id<CAMetalDrawable> drawable,
diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index 7b83591ddc3ec..f5855746adcde 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -39,7 +39,7 @@
 }
 
 std::unique_ptr<SurfaceMTL> SurfaceMTL::WrapCurrentMetalLayerDrawable(
-    const std::shared_ptr<Context>& context,
+    const std::shared_ptr<Context> context,
     id<CAMetalDrawable> drawable,
     std::optional<IRect> clip_rect) {
   TRACE_EVENT0("impeller", "SurfaceMTL::WrapCurrentMetalLayerDrawable");
@@ -137,19 +137,19 @@
   render_target_desc.SetStencilAttachment(stencil0);
 
   // The constructor is private. So make_unique may not be used.
-  return std::unique_ptr<SurfaceMTL>(new SurfaceMTL(context, render_target_desc,
-                                                    resolve_tex, drawable,
-                                                    requires_blit, clip_rect));
+  return std::unique_ptr<SurfaceMTL>(
+      new SurfaceMTL(std::move(context), render_target_desc, resolve_tex,
+                     drawable, requires_blit, clip_rect));
 }
 
-SurfaceMTL::SurfaceMTL(const std::shared_ptr<Context>& context,
+SurfaceMTL::SurfaceMTL(const std::shared_ptr<Context> context,
                        const RenderTarget& target,
                        std::shared_ptr<Texture> resolve_texture,
                        id<CAMetalDrawable> drawable,
                        bool requires_blit,
                        std::optional<IRect> clip_rect)
     : Surface(target),
-      context_(context),
+      context_(std::move(context)),
       resolve_texture_(std::move(resolve_texture)),
       drawable_(drawable),
       requires_blit_(requires_blit),

From a76d650cbc7679908e0d08622a6933afc7bb3900 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 19 Apr 2023 13:02:47 -0700
Subject: [PATCH 10/18] ++

---
 impeller/renderer/blit_pass.cc | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/impeller/renderer/blit_pass.cc b/impeller/renderer/blit_pass.cc
index 15b0c82f6cc5c..36ef50f40eecc 100644
--- a/impeller/renderer/blit_pass.cc
+++ b/impeller/renderer/blit_pass.cc
@@ -64,12 +64,12 @@ bool BlitPass::AddCopy(std::shared_ptr<Texture> source,
     return true;  // Nothing to blit.
   }
 
-  // // Clip the destination image.
-  // source_region = source_region->Intersection(
-  //     IRect(-destination_origin, destination->GetSize()));
-  // if (!source_region.has_value()) {
-  //   return true;  // Nothing to blit.
-  // }
+  // Clip the destination image.
+  source_region = source_region->Intersection(
+      IRect(-destination_origin, destination->GetSize()));
+  if (!source_region.has_value()) {
+    return true;  // Nothing to blit.
+  }
 
   return OnCopyTextureToTextureCommand(
       std::move(source), std::move(destination), source_region.value(),

From 2bde543fb0799485eb0c244141ab5917351bea42 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 19 Apr 2023 14:05:31 -0700
Subject: [PATCH 11/18] ++

---
 impeller/renderer/backend/metal/surface_mtl.h  | 6 +++---
 impeller/renderer/backend/metal/surface_mtl.mm | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/impeller/renderer/backend/metal/surface_mtl.h b/impeller/renderer/backend/metal/surface_mtl.h
index b15beb7fc1e28..bdb79dfc92e13 100644
--- a/impeller/renderer/backend/metal/surface_mtl.h
+++ b/impeller/renderer/backend/metal/surface_mtl.h
@@ -38,7 +38,7 @@ class SurfaceMTL final : public Surface {
       CAMetalLayer* layer);
 
   static std::unique_ptr<SurfaceMTL> WrapCurrentMetalLayerDrawable(
-      const std::shared_ptr<Context> context,
+      std::shared_ptr<Context> context,
       id<CAMetalDrawable> drawable,
       std::optional<IRect> clip_rect = std::nullopt);
 #pragma GCC diagnostic pop
@@ -52,7 +52,7 @@ class SurfaceMTL final : public Surface {
   bool Present() const override;
 
  private:
-  const std::shared_ptr<Context> context_;
+  std::shared_ptr<Context> context_;
   std::shared_ptr<Texture> resolve_texture_;
   id<CAMetalDrawable> drawable_ = nil;
   bool requires_blit_ = false;
@@ -60,7 +60,7 @@ class SurfaceMTL final : public Surface {
 
   static bool ShouldPerformPartialRepaint(std::optional<IRect> damage_rect);
 
-  SurfaceMTL(const std::shared_ptr<Context> context,
+  SurfaceMTL(std::shared_ptr<Context> context,
              const RenderTarget& target,
              std::shared_ptr<Texture> resolve_texture,
              id<CAMetalDrawable> drawable,
diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index f5855746adcde..859bb3bdf4e39 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -39,7 +39,7 @@
 }
 
 std::unique_ptr<SurfaceMTL> SurfaceMTL::WrapCurrentMetalLayerDrawable(
-    const std::shared_ptr<Context> context,
+    std::shared_ptr<Context> context,
     id<CAMetalDrawable> drawable,
     std::optional<IRect> clip_rect) {
   TRACE_EVENT0("impeller", "SurfaceMTL::WrapCurrentMetalLayerDrawable");
@@ -142,7 +142,7 @@ new SurfaceMTL(std::move(context), render_target_desc, resolve_tex,
                      drawable, requires_blit, clip_rect));
 }
 
-SurfaceMTL::SurfaceMTL(const std::shared_ptr<Context> context,
+SurfaceMTL::SurfaceMTL(std::shared_ptr<Context> context,
                        const RenderTarget& target,
                        std::shared_ptr<Texture> resolve_texture,
                        id<CAMetalDrawable> drawable,

From a1c4caa5e4a2616cc11cd52b23ab221718117d16 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 21 Apr 2023 09:25:04 -0700
Subject: [PATCH 12/18] remove separate clip rect field

---
 flow/compositor_context.cc                            |  6 ++----
 flow/compositor_context.h                             | 10 +++++-----
 flow/surface_frame.h                                  |  4 ----
 shell/common/rasterizer.cc                            |  1 -
 shell/gpu/gpu_surface_metal_impeller.mm               |  9 ++++-----
 .../darwin/ios/framework/Source/FlutterView.mm        |  6 +-----
 .../platform/darwin/ios/ios_surface_metal_impeller.mm | 11 +----------
 7 files changed, 13 insertions(+), 34 deletions(-)

diff --git a/flow/compositor_context.cc b/flow/compositor_context.cc
index 79044daab00e1..cf7ca5895c8be 100644
--- a/flow/compositor_context.cc
+++ b/flow/compositor_context.cc
@@ -42,11 +42,9 @@ std::optional<SkRect> FrameDamage::ComputeClipRect(
     damage_ =
         context.ComputeDamage(additional_damage_, horizontal_clip_alignment_,
                               vertical_clip_alignment_);
-    cached_clip_rect_ = SkRect::Make(damage_->buffer_damage);
-  } else {
-    cached_clip_rect_ = std::nullopt;
+    return SkRect::Make(damage_->buffer_damage);
   }
-  return cached_clip_rect_;
+  return std::nullopt;
 }
 
 CompositorContext::CompositorContext()
diff --git a/flow/compositor_context.h b/flow/compositor_context.h
index 2b298cf9da558..2985805966d35 100644
--- a/flow/compositor_context.h
+++ b/flow/compositor_context.h
@@ -91,12 +91,12 @@ class FrameDamage {
 
   // See Damage::buffer_damage.
   std::optional<SkIRect> GetBufferDamage() {
-    return damage_ ? std::make_optional(damage_->buffer_damage) : std::nullopt;
+    return (damage_ && !ignore_damage_)
+               ? std::make_optional(damage_->buffer_damage)
+               : std::nullopt;
   }
 
-  std::optional<SkRect> GetClipRect() const { return cached_clip_rect_; }
-
-  void Reset() { cached_clip_rect_ = std::nullopt; }
+  void Reset() { ignore_damage_ = true; }
 
  private:
   SkIRect additional_damage_ = SkIRect::MakeEmpty();
@@ -104,7 +104,7 @@ class FrameDamage {
   const LayerTree* prev_layer_tree_ = nullptr;
   int vertical_clip_alignment_ = 1;
   int horizontal_clip_alignment_ = 1;
-  std::optional<SkRect> cached_clip_rect_;
+  bool ignore_damage_ = false;
 };
 
 class CompositorContext {
diff --git a/flow/surface_frame.h b/flow/surface_frame.h
index 877b0ba48d075..31a89d807e989 100644
--- a/flow/surface_frame.h
+++ b/flow/surface_frame.h
@@ -80,10 +80,6 @@ class SurfaceFrame {
     // Time at which this frame is scheduled to be presented. This is a hint
     // that can be passed to the platform to drop queued frames.
     std::optional<fml::TimePoint> presentation_time;
-
-    // The clip rect applied to the layer tree for partial repaint. This is
-    // used by the Impeller backend to determine the correct blit command.
-    std::optional<SkRect> clip_rect;
   };
 
   bool Submit();
diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc
index 5524046ee790f..3b01c5c9bb0a9 100644
--- a/shell/common/rasterizer.cc
+++ b/shell/common/rasterizer.cc
@@ -605,7 +605,6 @@ RasterStatus Rasterizer::DrawToSurfaceUnsafe(
     if (damage) {
       submit_info.frame_damage = damage->GetFrameDamage();
       submit_info.buffer_damage = damage->GetBufferDamage();
-      submit_info.clip_rect = damage->GetClipRect();
     }
 
     frame->set_submit_info(submit_info);
diff --git a/shell/gpu/gpu_surface_metal_impeller.mm b/shell/gpu/gpu_surface_metal_impeller.mm
index 593c4c79393be..ee503202c6de1 100644
--- a/shell/gpu/gpu_surface_metal_impeller.mm
+++ b/shell/gpu/gpu_surface_metal_impeller.mm
@@ -105,11 +105,10 @@
         }
 
         std::optional<impeller::IRect> clip_rect;
-        if (surface_frame.submit_info().clip_rect.has_value()) {
-          auto submit_info_rect = surface_frame.submit_info().clip_rect;
-          clip_rect =
-              impeller::IRect::MakeXYWH(submit_info_rect->x(), submit_info_rect->y(),
-                                        submit_info_rect->width(), submit_info_rect->height());
+        if (surface_frame.submit_info().buffer_damage.has_value()) {
+          auto buffer_damage = surface_frame.submit_info().buffer_damage;
+          clip_rect = impeller::IRect::MakeXYWH(buffer_damage->x(), buffer_damage->y(),
+                                                buffer_damage->width(), buffer_damage->height());
         }
 
         auto surface = impeller::SurfaceMTL::WrapCurrentMetalLayerDrawable(
diff --git a/shell/platform/darwin/ios/framework/Source/FlutterView.mm b/shell/platform/darwin/ios/framework/Source/FlutterView.mm
index c441883999ad4..aa95fb844591d 100644
--- a/shell/platform/darwin/ios/framework/Source/FlutterView.mm
+++ b/shell/platform/darwin/ios/framework/Source/FlutterView.mm
@@ -96,11 +96,7 @@ - (void)layoutSubviews {
       CGColorSpaceRef srgb = CGColorSpaceCreateWithName(kCGColorSpaceExtendedSRGB);
       layer.colorspace = srgb;
       CFRelease(srgb);
-      if (self.opaque) {
-        layer.pixelFormat = MTLPixelFormatBGR10_XR;
-      } else {
-        layer.pixelFormat = MTLPixelFormatBGRA10_XR;
-      }
+      layer.pixelFormat = MTLPixelFormatBGRA10_XR;
     }
   }
 
diff --git a/shell/platform/darwin/ios/ios_surface_metal_impeller.mm b/shell/platform/darwin/ios/ios_surface_metal_impeller.mm
index b4a7436885daa..427b91e0fa6ba 100644
--- a/shell/platform/darwin/ios/ios_surface_metal_impeller.mm
+++ b/shell/platform/darwin/ios/ios_surface_metal_impeller.mm
@@ -10,15 +10,6 @@
 
 namespace flutter {
 
-static impeller::PixelFormat InferOffscreenLayerPixelFormat(impeller::PixelFormat pixel_format) {
-  switch (pixel_format) {
-    case impeller::PixelFormat::kB10G10R10XR:
-      return impeller::PixelFormat::kB10G10R10A10XR;
-    default:
-      return pixel_format;
-  }
-}
-
 IOSSurfaceMetalImpeller::IOSSurfaceMetalImpeller(const fml::scoped_nsobject<CAMetalLayer>& layer,
                                                  const std::shared_ptr<IOSContext>& context)
     : IOSSurface(context),
@@ -47,7 +38,7 @@
 // |IOSSurface|
 std::unique_ptr<Surface> IOSSurfaceMetalImpeller::CreateGPUSurface(GrDirectContext*) {
   impeller_context_->UpdateOffscreenLayerPixelFormat(
-      InferOffscreenLayerPixelFormat(impeller::FromMTLPixelFormat(layer_.get().pixelFormat)));
+      impeller::FromMTLPixelFormat(layer_.get().pixelFormat));
   return std::make_unique<GPUSurfaceMetalImpeller>(this,              //
                                                    impeller_context_  //
   );

From 353bd2ff4569bead46d7b4407c82adbd360a656b Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Mon, 24 Apr 2023 10:20:50 -0700
Subject: [PATCH 13/18] null check frame damage

---
 flow/compositor_context.cc | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/flow/compositor_context.cc b/flow/compositor_context.cc
index cf7ca5895c8be..b1d046c88eedf 100644
--- a/flow/compositor_context.cc
+++ b/flow/compositor_context.cc
@@ -124,15 +124,15 @@ RasterStatus CompositorContext::ScopedFrame::Raster(
     FrameDamage* frame_damage) {
   TRACE_EVENT0("flutter", "CompositorContext::ScopedFrame::Raster");
 
-  std::optional<SkRect> clip_rect =
-      frame_damage
-          ? frame_damage->ComputeClipRect(layer_tree, !ignore_raster_cache)
-          : std::nullopt;
-
-  if (aiks_context_ &&
-      !ShouldPerformPartialRepaint(clip_rect, layer_tree.frame_size())) {
-    clip_rect = std::nullopt;
-    frame_damage->Reset();
+  std::optional<SkRect> clip_rect;
+  if (frame_damage) {
+    clip_rect = frame_damage->ComputeClipRect(layer_tree, !ignore_raster_cache);
+
+    if (aiks_context_ &&
+        !ShouldPerformPartialRepaint(clip_rect, layer_tree.frame_size())) {
+      clip_rect = std::nullopt;
+      frame_damage->Reset();
+    }
   }
 
   bool root_needs_readback = layer_tree.Preroll(

From c701ac016ead4266e1a9d19d09d70349d6a77535 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Tue, 25 Apr 2023 11:26:13 -0700
Subject: [PATCH 14/18] add comment on why frame damage is not reset

---
 flow/compositor_context.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/flow/compositor_context.h b/flow/compositor_context.h
index 2985805966d35..a82466ca91d01 100644
--- a/flow/compositor_context.h
+++ b/flow/compositor_context.h
@@ -96,6 +96,10 @@ class FrameDamage {
                : std::nullopt;
   }
 
+  // Remove reported buffer_damage to inform clients that a partial repaint
+  // should not be performed on this frame.
+  // frame_damage is required to correctly track accumulated damage for
+  // subsequent frames.
   void Reset() { ignore_damage_ = true; }
 
  private:

From 4c84d0fa948d50b8941a64b9deb5a5faa6b066f1 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 26 Apr 2023 11:30:51 -0700
Subject: [PATCH 15/18] ++

---
 impeller/renderer/backend/metal/surface_mtl.mm | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index f00d5157926e2..142262cd45a83 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -201,16 +201,16 @@ new SurfaceMTL(context->weak_from_this(), render_target_desc, resolve_tex,
     return true;
   }
 
-  auto command_buffer = context_->CreateCommandBuffer();
-  if (!command_buffer) {
+  auto blit_command_buffer = context->CreateCommandBuffer();
+  if (!blit_command_buffer) {
     return false;
   }
-  auto blit_pass = command_buffer->CreateBlitPass();
+  auto blit_pass = blit_command_buffer->CreateBlitPass();
   auto current = TextureMTL::Wrapper({}, drawable_.texture);
   blit_pass->AddCopy(resolve_texture_, current, std::nullopt,
                      clip_rect_->origin);
-  blit_pass->EncodeCommands(context_->GetResourceAllocator());
-  if (!command_buffer->SubmitCommands()) {
+  blit_pass->EncodeCommands(context->GetResourceAllocator());
+  if (!blit_command_buffer->SubmitCommands()) {
     return false;
   }
 

From 1a0de9a3c35454c23d5c478ce53fc427ec995a41 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 26 Apr 2023 11:57:42 -0700
Subject: [PATCH 16/18] ++

---
 .../renderer/backend/metal/surface_mtl.mm     | 36 ++++++++-----------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index 142262cd45a83..9053fb1e1323d 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -185,6 +185,21 @@ new SurfaceMTL(context->weak_from_this(), render_target_desc, resolve_tex,
     return false;
   }
 
+  if (requires_blit_) {
+    auto blit_command_buffer = context->CreateCommandBuffer();
+    if (!blit_command_buffer) {
+      return false;
+    }
+    auto blit_pass = blit_command_buffer->CreateBlitPass();
+    auto current = TextureMTL::Wrapper({}, drawable_.texture);
+    blit_pass->AddCopy(resolve_texture_, current, std::nullopt,
+                       clip_rect_->origin);
+    blit_pass->EncodeCommands(context->GetResourceAllocator());
+    if (!blit_command_buffer->SubmitCommands()) {
+      return false;
+    }
+  }
+
   // If a transaction is present, `presentDrawable` will present too early. And
   // so we wait on an empty command buffer to get scheduled instead, which
   // forces us to also wait for all of the previous command buffers in the queue
@@ -193,27 +208,6 @@ new SurfaceMTL(context->weak_from_this(), render_target_desc, resolve_tex,
       ContextMTL::Cast(context.get())->CreateMTLCommandBuffer();
   [command_buffer commit];
   [command_buffer waitUntilScheduled];
-
-  // If there is no partial repaint, then immediately present without a blit
-  // pass.
-  if (!requires_blit_) {
-    [drawable_ present];
-    return true;
-  }
-
-  auto blit_command_buffer = context->CreateCommandBuffer();
-  if (!blit_command_buffer) {
-    return false;
-  }
-  auto blit_pass = blit_command_buffer->CreateBlitPass();
-  auto current = TextureMTL::Wrapper({}, drawable_.texture);
-  blit_pass->AddCopy(resolve_texture_, current, std::nullopt,
-                     clip_rect_->origin);
-  blit_pass->EncodeCommands(context->GetResourceAllocator());
-  if (!blit_command_buffer->SubmitCommands()) {
-    return false;
-  }
-
   [drawable_ present];
 
   return true;

From b91f7392b45f0021b2498d8c690d07c54971c4cd Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 26 Apr 2023 12:29:44 -0700
Subject: [PATCH 17/18] tidy

---
 impeller/renderer/backend/metal/surface_mtl.h  | 2 +-
 impeller/renderer/backend/metal/surface_mtl.mm | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/impeller/renderer/backend/metal/surface_mtl.h b/impeller/renderer/backend/metal/surface_mtl.h
index b21ab35f3c04e..72bdd3e70e3c1 100644
--- a/impeller/renderer/backend/metal/surface_mtl.h
+++ b/impeller/renderer/backend/metal/surface_mtl.h
@@ -38,7 +38,7 @@ class SurfaceMTL final : public Surface {
       CAMetalLayer* layer);
 
   static std::unique_ptr<SurfaceMTL> WrapCurrentMetalLayerDrawable(
-      std::shared_ptr<Context> context,
+      const std::shared_ptr<Context>& context,
       id<CAMetalDrawable> drawable,
       std::optional<IRect> clip_rect = std::nullopt);
 #pragma GCC diagnostic pop
diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index 9053fb1e1323d..5a0073144ef6f 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -40,7 +40,7 @@
 }
 
 std::unique_ptr<SurfaceMTL> SurfaceMTL::WrapCurrentMetalLayerDrawable(
-    std::shared_ptr<Context> context,
+    const std::shared_ptr<Context>& context,
     id<CAMetalDrawable> drawable,
     std::optional<IRect> clip_rect) {
   TRACE_EVENT0("impeller", "SurfaceMTL::WrapCurrentMetalLayerDrawable");
@@ -150,7 +150,7 @@ new SurfaceMTL(context->weak_from_this(), render_target_desc, resolve_tex,
                        bool requires_blit,
                        std::optional<IRect> clip_rect)
     : Surface(target),
-      context_(std::move(context)),
+      context_(context),
       resolve_texture_(std::move(resolve_texture)),
       drawable_(drawable),
       requires_blit_(requires_blit),

From 0fdbd2cea50c8558f7884b9d9ae7e5db0dcc58b5 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 26 Apr 2023 13:24:51 -0700
Subject: [PATCH 18/18] dnfield review

---
 flow/compositor_context.cc                     | 11 +++++++++++
 impeller/renderer/backend/metal/surface_mtl.mm |  4 +---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/flow/compositor_context.cc b/flow/compositor_context.cc
index b1d046c88eedf..cb119669d6430 100644
--- a/flow/compositor_context.cc
+++ b/flow/compositor_context.cc
@@ -197,6 +197,17 @@ void CompositorContext::ScopedFrame::PaintLayerTreeImpeller(
   layer_tree.Paint(*this, ignore_raster_cache);
 }
 
+/// @brief The max ratio of pixel width or height to size that is dirty which
+///        results in a partial repaint.
+///
+///        Performing a partial repaint has a small overhead - Impeller needs to
+///        allocate a fairly large resolve texture for the root pass instead of
+///        using the drawable texture, and a final blit must be performed. At a
+///        minimum, if the damage rect is the entire buffer, we must not perform
+///        a partial repaint. Beyond that, we could only experimentally
+///        determine what this value should be. From looking at the Flutter
+///        Gallery, we noticed that there are occassionally small partial
+///        repaints which shave off trivial numbers of pixels.
 constexpr float kImpellerRepaintRatio = 0.7f;
 
 bool CompositorContext::ShouldPerformPartialRepaint(
diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index 5a0073144ef6f..c6d6a265472e2 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -20,7 +20,7 @@
 id<CAMetalDrawable> SurfaceMTL::GetMetalDrawableAndValidate(
     const std::shared_ptr<Context>& context,
     CAMetalLayer* layer) {
-  TRACE_EVENT0("impeller", "SurfaceMTL::GetMetalDrawableAndValidate");
+  TRACE_EVENT0("impeller", "SurfaceMTL::WrapCurrentMetalLayerDrawable");
 
   if (context == nullptr || !context->IsValid() || layer == nil) {
     return nullptr;
@@ -43,8 +43,6 @@
     const std::shared_ptr<Context>& context,
     id<CAMetalDrawable> drawable,
     std::optional<IRect> clip_rect) {
-  TRACE_EVENT0("impeller", "SurfaceMTL::WrapCurrentMetalLayerDrawable");
-
   bool requires_blit = ShouldPerformPartialRepaint(clip_rect);
   const auto color_format = FromMTLPixelFormat(drawable.texture.pixelFormat);