From 01da845f16f8b238fff5bd01e2a109ef08d40e23 Mon Sep 17 00:00:00 2001
From: Chinmay Garde <chinmaygarde@google.com>
Date: Sun, 6 Jun 2021 15:34:52 -0700
Subject: [PATCH] Wire up surface from render pass descriptor.

---
 .../impeller/compositor/command_buffer.mm     |  7 +-
 .../impeller/compositor/formats_metal.h       | 31 +++++++++
 .../impeller/compositor/formats_metal.mm      | 69 +++++++++++++++++++
 .../flutter/impeller/compositor/render_pass.h |  2 +
 .../impeller/compositor/render_pass.mm        |  7 ++
 .../flutter/impeller/compositor/renderer.h    | 15 ++--
 .../flutter/impeller/compositor/renderer.mm   | 50 +++++++-------
 .../src/flutter/impeller/compositor/surface.h | 10 ++-
 .../flutter/impeller/compositor/surface.mm    | 31 +--------
 .../src/flutter/impeller/compositor/texture.h |  2 +-
 .../flutter/impeller/compositor/texture.mm    |  2 +-
 .../flutter/impeller/entity/entity_renderer.h |  2 -
 .../impeller/entity/entity_renderer.mm        |  8 +--
 engine/src/flutter/impeller/geometry/size.h   | 24 ++++---
 .../impeller/host/impeller_renderer.mm        | 14 ++--
 15 files changed, 178 insertions(+), 96 deletions(-)

diff --git a/engine/src/flutter/impeller/compositor/command_buffer.mm b/engine/src/flutter/impeller/compositor/command_buffer.mm
index cd6ffa29daa..c636bab2ca8 100644
--- a/engine/src/flutter/impeller/compositor/command_buffer.mm
+++ b/engine/src/flutter/impeller/compositor/command_buffer.mm
@@ -7,7 +7,12 @@
 namespace impeller {
 
 CommandBuffer::CommandBuffer(id<MTLCommandQueue> queue)
-    : buffer_([queue commandBuffer]) {}
+    : buffer_([queue commandBuffer]) {
+  if (!buffer_) {
+    return;
+  }
+  is_valid_ = true;
+}
 
 CommandBuffer::~CommandBuffer() = default;
 
diff --git a/engine/src/flutter/impeller/compositor/formats_metal.h b/engine/src/flutter/impeller/compositor/formats_metal.h
index a93ab76771e..ba99c92f2cb 100644
--- a/engine/src/flutter/impeller/compositor/formats_metal.h
+++ b/engine/src/flutter/impeller/compositor/formats_metal.h
@@ -14,6 +14,8 @@
 
 namespace impeller {
 
+class RenderPassDescriptor;
+
 constexpr MTLPixelFormat ToMTLPixelFormat(PixelFormat format) {
   switch (format) {
     case PixelFormat::kUnknown:
@@ -156,6 +158,21 @@ constexpr MTLLoadAction ToMTLLoadAction(LoadAction action) {
   return MTLLoadActionDontCare;
 }
 
+constexpr LoadAction FromMTLLoadAction(MTLLoadAction action) {
+  switch (action) {
+    case MTLLoadActionDontCare:
+      return LoadAction::kDontCare;
+    case MTLLoadActionLoad:
+      return LoadAction::kLoad;
+    case MTLLoadActionClear:
+      return LoadAction::kClear;
+    default:
+      break;
+  }
+
+  return LoadAction::kDontCare;
+}
+
 constexpr MTLStoreAction ToMTLStoreAction(StoreAction action) {
   switch (action) {
     case StoreAction::kDontCare:
@@ -166,10 +183,24 @@ constexpr MTLStoreAction ToMTLStoreAction(StoreAction action) {
   return MTLStoreActionDontCare;
 }
 
+constexpr StoreAction FromMTLStoreAction(MTLStoreAction action) {
+  switch (action) {
+    case MTLStoreActionDontCare:
+      return StoreAction::kDontCare;
+    case MTLStoreActionStore:
+      return StoreAction::kStore;
+    default:
+      break;
+  }
+  return StoreAction::kDontCare;
+}
+
 constexpr MTLClearColor ToMTLClearColor(const Color& color) {
   return MTLClearColorMake(color.red, color.green, color.blue, color.alpha);
 }
 
+RenderPassDescriptor FromMTLRenderPassDescriptor(MTLRenderPassDescriptor*);
+
 MTLRenderPipelineColorAttachmentDescriptor*
 ToMTLRenderPipelineColorAttachmentDescriptor(
     ColorAttachmentDescriptor descriptor);
diff --git a/engine/src/flutter/impeller/compositor/formats_metal.mm b/engine/src/flutter/impeller/compositor/formats_metal.mm
index d5411ba008c..f83a054a02b 100644
--- a/engine/src/flutter/impeller/compositor/formats_metal.mm
+++ b/engine/src/flutter/impeller/compositor/formats_metal.mm
@@ -4,6 +4,10 @@
 
 #include "impeller/compositor/formats_metal.h"
 
+#include <memory>
+
+#include "impeller/compositor/render_pass.h"
+
 namespace impeller {
 
 MTLRenderPipelineColorAttachmentDescriptor*
@@ -73,4 +77,69 @@ MTLDepthStencilDescriptor* ToMTLDepthStencilDescriptor(
   return des;
 }
 
+static bool ConfigureRenderPassAttachment(
+    RenderPassAttachment& attachment,
+    MTLRenderPassAttachmentDescriptor* desc) {
+  if (!attachment) {
+    return false;
+  }
+
+  if (!desc.texture) {
+    return false;
+  }
+
+  attachment.texture = std::make_shared<Texture>(desc.texture);
+  attachment.load_action = FromMTLLoadAction(desc.loadAction);
+  attachment.store_action = FromMTLStoreAction(desc.storeAction);
+
+  return true;
+}
+
+static ColorRenderPassAttachment FromMTLRenderPassColorAttachmentDescriptor(
+    MTLRenderPassColorAttachmentDescriptor* desc) {
+  ColorRenderPassAttachment attachment;
+  ConfigureRenderPassAttachment(attachment, desc);
+  auto clear = desc.clearColor;
+  attachment.clear_color =
+      Color{clear.red, clear.green, clear.blue, clear.alpha};
+  return attachment;
+}
+
+static DepthRenderPassAttachment FromMTLRenderPassDepthAttachmentDescriptor(
+    MTLRenderPassDepthAttachmentDescriptor* desc) {
+  DepthRenderPassAttachment attachment;
+  ConfigureRenderPassAttachment(attachment, desc);
+  attachment.clear_depth = desc.clearDepth;
+  return attachment;
+}
+
+static StencilRenderPassAttachment FromMTLRenderPassStencilAttachmentDescriptor(
+    MTLRenderPassStencilAttachmentDescriptor* desc) {
+  StencilRenderPassAttachment attachment;
+  ConfigureRenderPassAttachment(attachment, desc);
+  attachment.clear_stencil = desc.clearStencil;
+  return attachment;
+}
+
+RenderPassDescriptor FromMTLRenderPassDescriptor(
+    MTLRenderPassDescriptor* desc) {
+  RenderPassDescriptor result;
+  if (!desc) {
+    return result;
+  }
+
+  // From https://developer.apple.com/metal/Metal-Feature-Set-Tables.pdf
+  constexpr size_t kMaxPossibleColorAttachments = 8u;
+  for (size_t i = 0; i < kMaxPossibleColorAttachments; i++) {
+    result.SetColorAttachment(
+        FromMTLRenderPassColorAttachmentDescriptor(desc.colorAttachments[i]),
+        i);
+  }
+  result.SetDepthAttachment(
+      FromMTLRenderPassDepthAttachmentDescriptor(desc.depthAttachment));
+  result.SetStencilAttachment(
+      FromMTLRenderPassStencilAttachmentDescriptor(desc.stencilAttachment));
+  return result;
+}
+
 }  // namespace impeller
diff --git a/engine/src/flutter/impeller/compositor/render_pass.h b/engine/src/flutter/impeller/compositor/render_pass.h
index c01cbbb56dc..b5ab8e713f8 100644
--- a/engine/src/flutter/impeller/compositor/render_pass.h
+++ b/engine/src/flutter/impeller/compositor/render_pass.h
@@ -43,6 +43,8 @@ class RenderPassDescriptor {
 
   ~RenderPassDescriptor();
 
+  bool HasColorAttachment(size_t index) const;
+
   RenderPassDescriptor& SetColorAttachment(ColorRenderPassAttachment attachment,
                                            size_t index);
 
diff --git a/engine/src/flutter/impeller/compositor/render_pass.mm b/engine/src/flutter/impeller/compositor/render_pass.mm
index 06d9c81b98f..0850266035c 100644
--- a/engine/src/flutter/impeller/compositor/render_pass.mm
+++ b/engine/src/flutter/impeller/compositor/render_pass.mm
@@ -13,6 +13,13 @@ RenderPassDescriptor::RenderPassDescriptor() = default;
 
 RenderPassDescriptor::~RenderPassDescriptor() = default;
 
+bool RenderPassDescriptor::HasColorAttachment(size_t index) const {
+  if (auto found = colors_.find(index); found != colors_.end()) {
+    return true;
+  }
+  return false;
+}
+
 RenderPassDescriptor& RenderPassDescriptor::SetColorAttachment(
     ColorRenderPassAttachment attachment,
     size_t index) {
diff --git a/engine/src/flutter/impeller/compositor/renderer.h b/engine/src/flutter/impeller/compositor/renderer.h
index bf199e6fb73..7c1cc9df44f 100644
--- a/engine/src/flutter/impeller/compositor/renderer.h
+++ b/engine/src/flutter/impeller/compositor/renderer.h
@@ -4,24 +4,25 @@
 
 #pragma once
 
+#include <dispatch/dispatch.h>
+
 #include <memory>
 
 #include "flutter/fml/macros.h"
 #include "impeller/compositor/context.h"
-#include "impeller/compositor/surface.h"
 #include "impeller/geometry/size.h"
 
 namespace impeller {
 
+class Surface;
+
 class Renderer {
  public:
   virtual ~Renderer();
 
   bool IsValid() const;
 
-  bool SurfaceSizeDidChange(Size size);
-
-  bool Render();
+  bool Render(const Surface& surface);
 
   std::shared_ptr<Context> GetContext() const;
 
@@ -30,16 +31,12 @@ class Renderer {
 
   virtual bool OnRender() = 0;
 
-  virtual bool OnSurfaceSizeDidChange(Size size) = 0;
-
  private:
+  dispatch_semaphore_t frames_in_flight_sema_ = nullptr;
   std::shared_ptr<Context> context_;
-  std::unique_ptr<Surface> surface_;
   Size size_;
   bool is_valid_ = false;
 
-  bool ShouldRender() const;
-
   FML_DISALLOW_COPY_AND_ASSIGN(Renderer);
 };
 
diff --git a/engine/src/flutter/impeller/compositor/renderer.mm b/engine/src/flutter/impeller/compositor/renderer.mm
index 6cba3970752..6ca114d36a1 100644
--- a/engine/src/flutter/impeller/compositor/renderer.mm
+++ b/engine/src/flutter/impeller/compositor/renderer.mm
@@ -5,17 +5,17 @@
 #include "impeller/compositor/renderer.h"
 
 #include "flutter/fml/logging.h"
+#include "impeller/compositor/command_buffer.h"
+#include "impeller/compositor/surface.h"
 
 namespace impeller {
 
-Renderer::Renderer(std::string shaders_directory)
-    : context_(std::make_shared<Context>(std::move(shaders_directory))),
-      surface_(std::make_unique<Surface>(context_)) {
-  if (!context_->IsValid()) {
-    return;
-  }
+constexpr size_t kMaxFramesInFlight = 3u;
 
-  if (!surface_->IsValid()) {
+Renderer::Renderer(std::string shaders_directory)
+    : frames_in_flight_sema_(::dispatch_semaphore_create(kMaxFramesInFlight)),
+      context_(std::make_shared<Context>(std::move(shaders_directory))) {
+  if (!context_->IsValid()) {
     return;
   }
 
@@ -28,26 +28,28 @@ bool Renderer::IsValid() const {
   return is_valid_;
 }
 
-bool Renderer::ShouldRender() const {
-  return IsValid() && !size_.IsZero();
-}
-
-bool Renderer::SurfaceSizeDidChange(Size size) {
-  if (size_ == size) {
-    return true;
-  }
-
-  size_ = size;
-
-  return OnSurfaceSizeDidChange(size_);
-}
-
-bool Renderer::Render() {
-  if (!ShouldRender()) {
+bool Renderer::Render(const Surface& surface) {
+  if (!IsValid()) {
     return false;
   }
 
-  return surface_->Render();
+  if (!surface.IsValid()) {
+    return false;
+  }
+
+  auto command_buffer = context_->CreateRenderCommandBuffer();
+
+  if (!command_buffer) {
+    return false;
+  }
+
+  ::dispatch_semaphore_wait(frames_in_flight_sema_, DISPATCH_TIME_FOREVER);
+
+  command_buffer->Commit(
+      [sema = frames_in_flight_sema_](CommandBuffer::CommitResult) {
+        ::dispatch_semaphore_signal(sema);
+      });
+  return true;
 }
 
 std::shared_ptr<Context> Renderer::GetContext() const {
diff --git a/engine/src/flutter/impeller/compositor/surface.h b/engine/src/flutter/impeller/compositor/surface.h
index fed78ce1528..496443cbeed 100644
--- a/engine/src/flutter/impeller/compositor/surface.h
+++ b/engine/src/flutter/impeller/compositor/surface.h
@@ -4,28 +4,26 @@
 
 #pragma once
 
-#include <dispatch/dispatch.h>
+#include <Metal/Metal.h>
 
 #include <memory>
 
 #include "flutter/fml/macros.h"
 #include "impeller/compositor/context.h"
+#include "impeller/compositor/render_pass.h"
 
 namespace impeller {
 
 class Surface {
  public:
-  Surface(std::shared_ptr<Context> context);
+  Surface(RenderPassDescriptor desc);
 
   ~Surface();
 
   bool IsValid() const;
 
-  bool Render() const;
-
  private:
-  std::shared_ptr<Context> context_;
-  dispatch_semaphore_t frames_in_flight_sema_ = nullptr;
+  RenderPassDescriptor desc_;
   bool is_valid_ = false;
 
   FML_DISALLOW_COPY_AND_ASSIGN(Surface);
diff --git a/engine/src/flutter/impeller/compositor/surface.mm b/engine/src/flutter/impeller/compositor/surface.mm
index adbd20c1d51..009333a1373 100644
--- a/engine/src/flutter/impeller/compositor/surface.mm
+++ b/engine/src/flutter/impeller/compositor/surface.mm
@@ -5,19 +5,13 @@
 #include "impeller/compositor/surface.h"
 
 #include "flutter/fml/logging.h"
-#include "impeller/compositor/command_buffer.h"
 
 namespace impeller {
 
-constexpr size_t kMaxFramesInFlight = 3u;
-
-Surface::Surface(std::shared_ptr<Context> context)
-    : context_(std::move(context)),
-      frames_in_flight_sema_(::dispatch_semaphore_create(kMaxFramesInFlight)) {
-  if (!context_ || !context_->IsValid()) {
+Surface::Surface(RenderPassDescriptor desc) {
+  if (desc.HasColorAttachment(0)) {
     return;
   }
-
   is_valid_ = true;
 }
 
@@ -27,25 +21,4 @@ bool Surface::IsValid() const {
   return is_valid_;
 }
 
-bool Surface::Render() const {
-  if (!IsValid()) {
-    return false;
-  }
-
-  auto command_buffer = context_->CreateRenderCommandBuffer();
-
-  if (!command_buffer) {
-    return false;
-  }
-
-  ::dispatch_semaphore_wait(frames_in_flight_sema_, DISPATCH_TIME_FOREVER);
-
-  command_buffer->Commit(
-      [sema = frames_in_flight_sema_](CommandBuffer::CommitResult) {
-        ::dispatch_semaphore_signal(sema);
-      });
-
-  return true;
-}
-
 }  // namespace impeller
diff --git a/engine/src/flutter/impeller/compositor/texture.h b/engine/src/flutter/impeller/compositor/texture.h
index 8e7c55c5b62..7827c98c94a 100644
--- a/engine/src/flutter/impeller/compositor/texture.h
+++ b/engine/src/flutter/impeller/compositor/texture.h
@@ -12,7 +12,7 @@ namespace impeller {
 
 class Texture {
  public:
-  Texture();
+  Texture(id<MTLTexture> texture);
 
   ~Texture();
 
diff --git a/engine/src/flutter/impeller/compositor/texture.mm b/engine/src/flutter/impeller/compositor/texture.mm
index dc9ea9130cd..69e1d4a8d66 100644
--- a/engine/src/flutter/impeller/compositor/texture.mm
+++ b/engine/src/flutter/impeller/compositor/texture.mm
@@ -6,7 +6,7 @@
 
 namespace impeller {
 
-Texture::Texture() = default;
+Texture::Texture(id<MTLTexture> texture) : texture_(texture) {}
 
 Texture::~Texture() = default;
 
diff --git a/engine/src/flutter/impeller/entity/entity_renderer.h b/engine/src/flutter/impeller/entity/entity_renderer.h
index 07113c77865..629ef14ed61 100644
--- a/engine/src/flutter/impeller/entity/entity_renderer.h
+++ b/engine/src/flutter/impeller/entity/entity_renderer.h
@@ -21,8 +21,6 @@ class EntityRenderer final : public Renderer {
 
   bool OnRender() override;
 
-  bool OnSurfaceSizeDidChange(Size size) override;
-
   FML_DISALLOW_COPY_AND_ASSIGN(EntityRenderer);
 };
 
diff --git a/engine/src/flutter/impeller/entity/entity_renderer.mm b/engine/src/flutter/impeller/entity/entity_renderer.mm
index 914e364f20f..161a857dfe8 100644
--- a/engine/src/flutter/impeller/entity/entity_renderer.mm
+++ b/engine/src/flutter/impeller/entity/entity_renderer.mm
@@ -8,7 +8,9 @@ namespace impeller {
 
 EntityRenderer::EntityRenderer(std::string shaders_directory)
     : Renderer(std::move(shaders_directory)),
-      root_(std::make_shared<Entity>()) {}
+      root_(std::make_shared<Entity>()) {
+  root_->SetBackgroundColor(Color::DarkGray());
+}
 
 EntityRenderer::~EntityRenderer() = default;
 
@@ -16,8 +18,4 @@ bool EntityRenderer::OnRender() {
   return false;
 }
 
-bool EntityRenderer::OnSurfaceSizeDidChange(Size size) {
-  return false;
-}
-
 }  // namespace impeller
diff --git a/engine/src/flutter/impeller/geometry/size.h b/engine/src/flutter/impeller/geometry/size.h
index 25faa9600e7..0ff85993fa4 100644
--- a/engine/src/flutter/impeller/geometry/size.h
+++ b/engine/src/flutter/impeller/geometry/size.h
@@ -12,41 +12,45 @@ struct Size {
   double width = 0.0;
   double height = 0.0;
 
-  Size() {}
+  constexpr Size() {}
 
-  Size(double width, double height) : width(width), height(height) {}
+  constexpr Size(double width, double height) : width(width), height(height) {}
 
   /*
    *  Operator overloads
    */
-  Size operator*(double scale) const { return {width * scale, height * scale}; }
+  constexpr Size operator*(double scale) const {
+    return {width * scale, height * scale};
+  }
 
-  bool operator==(const Size& s) const {
+  constexpr bool operator==(const Size& s) const {
     return s.width == width && s.height == height;
   }
 
-  bool operator!=(const Size& s) const {
+  constexpr bool operator!=(const Size& s) const {
     return s.width != width || s.height != height;
   }
 
-  Size operator+(const Size& s) const {
+  constexpr Size operator+(const Size& s) const {
     return {width + s.width, height + s.height};
   }
 
-  Size operator-(const Size& s) const {
+  constexpr Size operator-(const Size& s) const {
     return {width - s.width, height - s.height};
   }
 
-  Size Union(const Size& o) const {
+  constexpr Size Union(const Size& o) const {
     return {
         std::max(width, o.width),
         std::max(height, o.height),
     };
   }
 
-  bool IsZero() const { return width * height == 0.0; }
+  constexpr bool IsZero() const { return width * height == 0.0; }
 
-  bool IsPositive() const { return width > 0.0 && height > 0.0; }
+  constexpr bool IsPositive() const { return width * height > 0.0; }
+
+  constexpr bool IsEmpty() { return !IsPositive(); }
 
   std::string ToString() const;
 
diff --git a/engine/src/flutter/impeller/host/impeller_renderer.mm b/engine/src/flutter/impeller/host/impeller_renderer.mm
index 1617193e863..c9e654f11d2 100644
--- a/engine/src/flutter/impeller/host/impeller_renderer.mm
+++ b/engine/src/flutter/impeller/host/impeller_renderer.mm
@@ -8,6 +8,8 @@
 
 #include "assets_location.h"
 #include "flutter/fml/logging.h"
+#include "impeller/compositor/formats_metal.h"
+#include "impeller/compositor/surface.h"
 #include "impeller/entity/entity_renderer.h"
 #include "impeller/primitives/box_primitive.h"
 #include "impeller_renderer.h"
@@ -53,11 +55,7 @@ static const size_t kAlignedUniformsSize = (sizeof(Uniforms) & ~0xFF) + 0x100;
 
   renderer_ = std::make_unique<impeller::EntityRenderer>(
       impeller::ImpellerShadersDirectory());
-
-  if (!renderer_->IsValid()) {
-    FML_LOG(ERROR) << "Impeller Renderer is not valid.";
-  }
-
+  FML_CHECK(renderer_->IsValid()) << "Impeller Renderer is not valid.";
   return self;
 }
 
@@ -227,7 +225,9 @@ static const size_t kAlignedUniformsSize = (sizeof(Uniforms) & ~0xFF) + 0x100;
 }
 
 - (void)drawInMTKView:(nonnull MTKView*)view {
-  if (!renderer_->Render()) {
+  impeller::Surface surface(
+      impeller::FromMTLRenderPassDescriptor(view.currentRenderPassDescriptor));
+  if (!renderer_->Render(surface)) {
     FML_LOG(ERROR) << "Could not render.";
   }
   /// Per frame updates here
@@ -304,8 +304,6 @@ static const size_t kAlignedUniformsSize = (sizeof(Uniforms) & ~0xFF) + 0x100;
 }
 
 - (void)mtkView:(nonnull MTKView*)view drawableSizeWillChange:(CGSize)size {
-  renderer_->SurfaceSizeDidChange({size.width, size.height});
-
   float aspect = size.width / (float)size.height;
   _projectionMatrix = matrix_perspective_right_hand(65.0f * (M_PI / 180.0f),
                                                     aspect, 0.1f, 100.0f);