Standardize around "coverage" terminology; make coverage optional everywhere (#118)

bdero · dnfield · commit 8075d383f4bd · 2022-04-27T15:57:33.000-07:00
diff --git a/impeller/entity/contents/contents.cc b/impeller/entity/contents/contents.cc
@@ -29,24 +29,27 @@ Contents::Contents() = default;
 
 Contents::~Contents() = default;
 
-Rect Contents::GetBounds(const Entity& entity) const {
-  return entity.GetTransformedPathBounds();
+std::optional<Rect> Contents::GetCoverage(const Entity& entity) const {
+  return entity.GetPathCoverage();
 }
 
 std::optional<Snapshot> Contents::RenderToTexture(
     const ContentContext& renderer,
     const Entity& entity) const {
-  auto bounds = GetBounds(entity);
+  auto bounds = GetCoverage(entity);
+  if (!bounds.has_value()) {
+    return std::nullopt;
+  }
 
   auto texture = renderer.MakeSubpass(
-      ISize::Ceil(bounds.size),
+      ISize::Ceil(bounds->size),
       [&contents = *this, &entity, &bounds](const ContentContext& renderer,
                                             RenderPass& pass) -> bool {
         Entity sub_entity;
         sub_entity.SetPath(entity.GetPath());
         sub_entity.SetBlendMode(Entity::BlendMode::kSource);
         sub_entity.SetTransformation(
-            Matrix::MakeTranslation(Vector3(-bounds.origin)) *
+            Matrix::MakeTranslation(Vector3(-bounds->origin)) *
             entity.GetTransformation());
         return contents.Render(renderer, sub_entity, pass);
       });
@@ -55,7 +58,7 @@ std::optional<Snapshot> Contents::RenderToTexture(
     return std::nullopt;
   }
 
-  return Snapshot{.texture = texture, .position = bounds.origin};
+  return Snapshot{.texture = texture, .position = bounds->origin};
 }
 
 }  // namespace impeller
diff --git a/impeller/entity/contents/contents.h b/impeller/entity/contents/contents.h
@@ -36,13 +36,13 @@ class Contents {
                       const Entity& entity,
                       RenderPass& pass) const = 0;
 
-  /// @brief Get the bounding rectangle that this contents modifies in screen
-  ///        space.
-  virtual Rect GetBounds(const Entity& entity) const;
+  /// @brief Get the screen space bounding rectangle that this contents affects.
+  virtual std::optional<Rect> GetCoverage(const Entity& entity) const;
 
   /// @brief Render this contents to a texture, respecting the entity's
   ///        transform, path, stencil depth, blend mode, etc.
-  ///        The result texture size is always the size of `GetBounds(entity)`.
+  ///        The result texture size is always the size of
+  ///        `GetCoverage(entity)`.
   virtual std::optional<Snapshot> RenderToTexture(
       const ContentContext& renderer,
       const Entity& entity) const;
diff --git a/impeller/entity/contents/filters/filter_contents.cc b/impeller/entity/contents/filters/filter_contents.cc
@@ -95,6 +95,11 @@ void FilterContents::SetInputs(FilterInput::Vector inputs) {
 bool FilterContents::Render(const ContentContext& renderer,
                             const Entity& entity,
                             RenderPass& pass) const {
+  auto filter_coverage = GetCoverage(entity);
+  if (!filter_coverage.has_value()) {
+    return true;
+  }
+
   // Run the filter.
 
   auto maybe_snapshot = RenderToTexture(renderer, entity);
@@ -110,49 +115,56 @@ bool FilterContents::Render(const ContentContext& renderer,
   contents->SetSourceRect(Rect::MakeSize(Size(snapshot.texture->GetSize())));
 
   Entity e;
-  e.SetPath(PathBuilder{}.AddRect(GetBounds(entity)).GetCurrentPath());
+  e.SetPath(PathBuilder{}.AddRect(filter_coverage.value()).GetCurrentPath());
   e.SetBlendMode(entity.GetBlendMode());
   e.SetStencilDepth(entity.GetStencilDepth());
   return contents->Render(renderer, e, pass);
 }
 
-Rect FilterContents::GetBounds(const Entity& entity) const {
-  // The default bounds of FilterContents is just the union of its inputs.
-  // FilterContents implementations may choose to increase the bounds in any
-  // direction, but it should never
+std::optional<Rect> FilterContents::GetCoverage(const Entity& entity) const {
+  // The default coverage of FilterContents is just the union of its inputs'
+  // coverage. FilterContents implementations may choose to adjust this
+  // coverage depending on the use case.
 
   if (inputs_.empty()) {
-    return Rect();
+    return std::nullopt;
   }
 
-  Rect result = inputs_.front()->GetBounds(entity);
-  for (auto input_i = inputs_.begin() + 1; input_i < inputs_.end(); input_i++) {
-    result.Union(input_i->get()->GetBounds(entity));
+  std::optional<Rect> result;
+  for (const auto& input : inputs_) {
+    auto coverage = input->GetCoverage(entity);
+    if (!coverage.has_value()) {
+      continue;
+    }
+    if (!result.has_value()) {
+      result = coverage;
+      continue;
+    }
+    result = result->Union(result.value());
   }
-
   return result;
 }
 
 std::optional<Snapshot> FilterContents::RenderToTexture(
     const ContentContext& renderer,
     const Entity& entity) const {
-  auto bounds = GetBounds(entity);
-  if (bounds.IsZero()) {
+  auto bounds = GetCoverage(entity);
+  if (!bounds.has_value() || bounds->IsEmpty()) {
     return std::nullopt;
   }
 
   // Render the filter into a new texture.
   auto texture = renderer.MakeSubpass(
-      ISize(GetBounds(entity).size),
+      ISize(bounds->size),
       [=](const ContentContext& renderer, RenderPass& pass) -> bool {
-        return RenderFilter(inputs_, renderer, entity, pass, bounds);
+        return RenderFilter(inputs_, renderer, entity, pass, bounds.value());
       });
 
   if (!texture) {
     return std::nullopt;
   }
 
-  return Snapshot{.texture = texture, .position = bounds.origin};
+  return Snapshot{.texture = texture, .position = bounds->origin};
 }
 
 }  // namespace impeller
diff --git a/impeller/entity/contents/filters/filter_contents.h b/impeller/entity/contents/filters/filter_contents.h
@@ -61,7 +61,7 @@ class FilterContents : public Contents {
               RenderPass& pass) const override;
 
   // |Contents|
-  Rect GetBounds(const Entity& entity) const override;
+  std::optional<Rect> GetCoverage(const Entity& entity) const override;
 
   // |Contents|
   virtual std::optional<Snapshot> RenderToTexture(
diff --git a/impeller/entity/contents/filters/filter_input.cc b/impeller/entity/contents/filters/filter_input.cc
@@ -30,17 +30,17 @@ FilterInput::Variant FilterInput::GetInput() const {
   return input_;
 }
 
-Rect FilterInput::GetBounds(const Entity& entity) const {
+std::optional<Rect> FilterInput::GetCoverage(const Entity& entity) const {
   if (snapshot_) {
     return Rect(snapshot_->position, Size(snapshot_->texture->GetSize()));
   }
 
   if (auto contents = std::get_if<std::shared_ptr<Contents>>(&input_)) {
-    return contents->get()->GetBounds(entity);
+    return contents->get()->GetCoverage(entity);
   }
 
   if (auto texture = std::get_if<std::shared_ptr<Texture>>(&input_)) {
-    return entity.GetTransformedPathBounds();
+    return entity.GetPathCoverage();
   }
 
   FML_UNREACHABLE();
diff --git a/impeller/entity/contents/filters/filter_input.h b/impeller/entity/contents/filters/filter_input.h
@@ -42,7 +42,7 @@ class FilterInput final {
 
   Variant GetInput() const;
 
-  Rect GetBounds(const Entity& entity) const;
+  std::optional<Rect> GetCoverage(const Entity& entity) const;
 
   std::optional<Snapshot> GetSnapshot(const ContentContext& renderer,
                                       const Entity& entity) const;
diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc
@@ -78,38 +78,55 @@ bool DirectionalGaussianBlurFilterContents::RenderFilter(
   auto& host_buffer = pass.GetTransientsBuffer();
 
   auto input = inputs[0]->GetSnapshot(renderer, entity);
-  auto input_bounds = inputs[0]->GetBounds(entity);
-  auto filter_bounds = GetBounds(entity);
+  if (!input.has_value()) {
+    return true;
+  }
+
+  auto input_bounds = inputs[0]->GetCoverage(entity);
+  if (!input_bounds.has_value() || input_bounds->IsEmpty()) {
+    return true;
+  }
+  auto filter_bounds = GetCoverage(entity);
+  if (!filter_bounds.has_value() || filter_bounds->IsEmpty()) {
+    FML_LOG(ERROR) << "The gaussian blur filter coverage is missing or empty "
+                      "even though the filter's input has coverage.";
+    return false;
+  }
 
   auto transformed_blur =
       entity.GetTransformation().TransformDirection(blur_vector_);
 
   // LTRB
   Scalar uv[4] = {
-      (filter_bounds.GetLeft() - input_bounds.GetLeft()) /
-          input_bounds.size.width,
-      (filter_bounds.GetTop() - input_bounds.GetTop()) /
-          input_bounds.size.height,
-      1 + (filter_bounds.GetRight() - input_bounds.GetRight()) /
-              input_bounds.size.width,
-      1 + (filter_bounds.GetBottom() - input_bounds.GetBottom()) /
-              input_bounds.size.height,
+      (filter_bounds->GetLeft() - input_bounds->GetLeft()) /
+          input_bounds->size.width,
+      (filter_bounds->GetTop() - input_bounds->GetTop()) /
+          input_bounds->size.height,
+      1 + (filter_bounds->GetRight() - input_bounds->GetRight()) /
+              input_bounds->size.width,
+      1 + (filter_bounds->GetBottom() - input_bounds->GetBottom()) /
+              input_bounds->size.height,
   };
 
   auto source = source_override_ ? source_override_ : inputs[0];
   auto source_texture = source->GetSnapshot(renderer, entity);
-  auto source_bounds = source->GetBounds(entity);
+  auto source_bounds = source->GetCoverage(entity);
+  if (!source_texture.has_value() || !source_bounds.has_value() ||
+      source_bounds->IsEmpty()) {
+    VALIDATION_LOG << "The gaussian blur source override has no coverage.";
+    return false;
+  }
 
   // LTRB
   Scalar uv_src[4] = {
-      (filter_bounds.GetLeft() - source_bounds.GetLeft()) /
-          source_bounds.size.width,
-      (filter_bounds.GetTop() - source_bounds.GetTop()) /
-          source_bounds.size.height,
-      1 + (filter_bounds.GetRight() - source_bounds.GetRight()) /
-              source_bounds.size.width,
-      1 + (filter_bounds.GetBottom() - source_bounds.GetBottom()) /
-              source_bounds.size.height,
+      (filter_bounds->GetLeft() - source_bounds->GetLeft()) /
+          source_bounds->size.width,
+      (filter_bounds->GetTop() - source_bounds->GetTop()) /
+          source_bounds->size.height,
+      1 + (filter_bounds->GetRight() - source_bounds->GetRight()) /
+              source_bounds->size.width,
+      1 + (filter_bounds->GetBottom() - source_bounds->GetBottom()) /
+              source_bounds->size.height,
   };
 
   VertexBufferBuilder<VS::PerVertexData> vtx_builder;
@@ -124,7 +141,7 @@ bool DirectionalGaussianBlurFilterContents::RenderFilter(
   auto vtx_buffer = vtx_builder.CreateVertexBuffer(host_buffer);
 
   VS::FrameInfo frame_info;
-  frame_info.texture_size = Point(input_bounds.size);
+  frame_info.texture_size = Point(input_bounds->size);
   frame_info.blur_radius = transformed_blur.GetLength();
   frame_info.blur_direction = transformed_blur.Normalize();
   frame_info.src_factor = src_color_factor_;
@@ -150,13 +167,17 @@ bool DirectionalGaussianBlurFilterContents::RenderFilter(
   return pass.AddCommand(cmd);
 }
 
-Rect DirectionalGaussianBlurFilterContents::GetBounds(
+std::optional<Rect> DirectionalGaussianBlurFilterContents::GetCoverage(
     const Entity& entity) const {
-  auto bounds = FilterContents::GetBounds(entity);
+  auto bounds = FilterContents::GetCoverage(entity);
+  if (!bounds.has_value()) {
+    return std::nullopt;
+  }
+
   auto transformed_blur =
       entity.GetTransformation().TransformDirection(blur_vector_).Abs();
-  auto extent = bounds.size + transformed_blur * 2;
-  return Rect(bounds.origin - transformed_blur, Size(extent.x, extent.y));
+  auto extent = bounds->size + transformed_blur * 2;
+  return Rect(bounds->origin - transformed_blur, Size(extent.x, extent.y));
 }
 
 }  // namespace impeller
diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h
@@ -25,7 +25,7 @@ class DirectionalGaussianBlurFilterContents final : public FilterContents {
   void SetSourceOverride(FilterInput::Ref alpha_mask);
 
   // |Contents|
-  Rect GetBounds(const Entity& entity) const override;
+  std::optional<Rect> GetCoverage(const Entity& entity) const override;
 
  private:
   // |FilterContents|
diff --git a/impeller/entity/contents/snapshot.cc b/impeller/entity/contents/snapshot.cc
@@ -4,6 +4,8 @@
 
 #include "impeller/entity/contents/snapshot.h"
 
+#include <optional>
+
 #include "impeller/entity/contents/content_context.h"
 #include "impeller/entity/contents/texture_contents.h"
 
@@ -13,28 +15,31 @@ std::optional<Snapshot> Snapshot::FromTransformedTexture(
     const ContentContext& renderer,
     const Entity& entity,
     std::shared_ptr<Texture> texture) {
-  Rect bounds = entity.GetTransformedPathBounds();
+  auto bounds = entity.GetPathCoverage();
+  if (!bounds.has_value()) {
+    return std::nullopt;
+  }
 
   auto result = renderer.MakeSubpass(
-      ISize(bounds.size),
-      [&texture, &entity, bounds](const ContentContext& renderer,
-                                  RenderPass& pass) -> bool {
+      ISize(bounds->size),
+      [&texture, &entity, &bounds](const ContentContext& renderer,
+                                   RenderPass& pass) -> bool {
         TextureContents contents;
         contents.SetTexture(texture);
         contents.SetSourceRect(Rect::MakeSize(Size(texture->GetSize())));
         Entity sub_entity;
         sub_entity.SetPath(entity.GetPath());
         sub_entity.SetBlendMode(Entity::BlendMode::kSource);
         sub_entity.SetTransformation(
-            Matrix::MakeTranslation(Vector3(-bounds.origin)) *
+            Matrix::MakeTranslation(Vector3(-bounds->origin)) *
             entity.GetTransformation());
         return contents.Render(renderer, sub_entity, pass);
       });
   if (!result) {
     return std::nullopt;
   }
 
-  return Snapshot{.texture = result, .position = bounds.origin};
+  return Snapshot{.texture = result, .position = bounds->origin};
 }
 
 }  // namespace impeller
diff --git a/impeller/entity/entity.cc b/impeller/entity/entity.cc
@@ -4,6 +4,8 @@
 
 #include "impeller/entity/entity.h"
 
+#include <optional>
+
 #include "impeller/base/validation.h"
 #include "impeller/entity/contents/content_context.h"
 #include "impeller/renderer/render_pass.h"
@@ -30,10 +32,10 @@ void Entity::SetPath(Path path) {
   path_ = std::move(path);
 }
 
-Rect Entity::GetTransformedPathBounds() const {
+std::optional<Rect> Entity::GetPathCoverage() const {
   auto bounds = GetPath().GetBoundingBox();
   if (!bounds.has_value()) {
-    return Rect();
+    return std::nullopt;
   }
   return bounds->TransformBounds(GetTransformation());
 }
@@ -51,7 +53,7 @@ std::optional<Rect> Entity::GetCoverage() const {
     return std::nullopt;
   }
 
-  return contents_->GetBounds(*this);
+  return contents_->GetCoverage(*this);
 }
 
 void Entity::SetContents(std::shared_ptr<Contents> contents) {
diff --git a/impeller/entity/entity.h b/impeller/entity/entity.h
@@ -65,7 +65,7 @@ class Entity {
 
   void SetPath(Path path);
 
-  Rect GetTransformedPathBounds() const;
+  std::optional<Rect> GetPathCoverage() const;
 
   void SetAddsToCoverage(bool adds);
 
diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc

Original file line number	Diff line number	Diff line change
`@@ -30,17 +30,17 @@ FilterInput::Variant FilterInput::GetInput() const {`
`30`	`30`	`return input_;`
`31`	`31`	`}`
`32`	`32`
`33`		`-Rect FilterInput::GetBounds(const Entity& entity) const {`
	`33`	`+std::optional<Rect> FilterInput::GetCoverage(const Entity& entity) const {`
`34`	`34`	`if (snapshot_) {`
`35`	`35`	`return Rect(snapshot_->position, Size(snapshot_->texture->GetSize()));`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`if (auto contents = std::get_if<std::shared_ptr<Contents>>(&input_)) {`
`39`		`- return contents->get()->GetBounds(entity);`
	`39`	`+ return contents->get()->GetCoverage(entity);`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`if (auto texture = std::get_if<std::shared_ptr<Texture>>(&input_)) {`
`43`		`- return entity.GetTransformedPathBounds();`
	`43`	`+ return entity.GetPathCoverage();`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`FML_UNREACHABLE();`
Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,8 @@`
`4`	`4`
`5`	`5`	`#include "impeller/entity/entity.h"`
`6`	`6`
	`7`	`+#include <optional>`
	`8`	`+`
`7`	`9`	`#include "impeller/base/validation.h"`
`8`	`10`	`#include "impeller/entity/contents/content_context.h"`
`9`	`11`	`#include "impeller/renderer/render_pass.h"`
`@@ -30,10 +32,10 @@ void Entity::SetPath(Path path) {`
`30`	`32`	`path_ = std::move(path);`
`31`	`33`	`}`
`32`	`34`
`33`		`-Rect Entity::GetTransformedPathBounds() const {`
	`35`	`+std::optional<Rect> Entity::GetPathCoverage() const {`
`34`	`36`	`auto bounds = GetPath().GetBoundingBox();`
`35`	`37`	`if (!bounds.has_value()) {`
`36`		`- return Rect();`
	`38`	`+ return std::nullopt;`
`37`	`39`	`}`
`38`	`40`	`return bounds->TransformBounds(GetTransformation());`
`39`	`41`	`}`
`@@ -51,7 +53,7 @@ std::optional<Rect> Entity::GetCoverage() const {`
`51`	`53`	`return std::nullopt;`
`52`	`54`	`}`
`53`	`55`
`54`		`- return contents_->GetBounds(*this);`
	`56`	`+ return contents_->GetCoverage(*this);`
`55`	`57`	`}`
`56`	`58`
`57`	`59`	`void Entity::SetContents(std::shared_ptr<Contents> contents) {`