Address comments

Author: bdero

impeller/entity/contents/filters/filter_contents.cc

@@ -270,9 +270,9 @@ Matrix FilterContents::GetLocalTransform(const Matrix& parent_transform) const {
 Matrix FilterContents::GetTransform(const Matrix& parent_transform) const {
   return parent_transform * GetLocalTransform(parent_transform);
 }
-bool FilterContents::HasBasisTransformations() const {
+bool FilterContents::IsTranslationOnly() const {
   for (auto& input : inputs_) {
-    if (!input->HasBasisTransformations()) {
+    if (!input->IsTranslationOnly()) {
       return false;
     }
   }

impeller/entity/contents/filters/filter_contents.h

@@ -132,14 +132,15 @@ class FilterContents : public Contents {
 
   Matrix GetTransform(const Matrix& parent_transform) const;
 
-  /// @brief  Returns true of this filter graph performs basis transformations
-  ///         to the filtered content. For example: Rotating, scaling, and
-  ///         skewing are all basis transformations, but translating is not.
+  /// @brief  Returns true if this filter graph doesn't perform any basis
+  ///         transformations to the filtered content. For example: Rotating,
+  ///         scaling, and skewing are all basis transformations, but
+  ///         translating is not.
   ///
   ///         This is useful for determining whether a filtered object's space
   ///         is compatible enough with the parent pass space to perform certain
-  ///         subpass optimizations.
-  virtual bool HasBasisTransformations() const;
+  ///         subpass clipping optimizations.
+  virtual bool IsTranslationOnly() const;
 
   /// @brief  Returns `true` if this filter does not have any `FilterInput`
   ///         children.

impeller/entity/contents/filters/inputs/filter_contents_filter_input.cc

@@ -58,8 +58,8 @@ void FilterContentsFilterInput::PopulateGlyphAtlas(
   filter_->PopulateGlyphAtlas(lazy_glyph_atlas, scale);
 }
 
-bool FilterContentsFilterInput::HasBasisTransformations() const {
-  return filter_->HasBasisTransformations();
+bool FilterContentsFilterInput::IsTranslationOnly() const {
+  return filter_->IsTranslationOnly();
 }
 
 bool FilterContentsFilterInput::IsLeaf() const {

impeller/entity/contents/filters/inputs/filter_contents_filter_input.h

@@ -37,7 +37,7 @@ class FilterContentsFilterInput final : public FilterInput {
       Scalar scale) override;
 
   // |FilterInput|
-  bool HasBasisTransformations() const override;
+  bool IsTranslationOnly() const override;
 
   // |FilterInput|
   bool IsLeaf() const override;

impeller/entity/contents/filters/inputs/filter_input.cc

@@ -76,8 +76,8 @@ void FilterInput::PopulateGlyphAtlas(
 
 FilterInput::~FilterInput() = default;
 
-bool FilterInput::HasBasisTransformations() const {
-  return false;
+bool FilterInput::IsTranslationOnly() const {
+  return true;
 }
 
 bool FilterInput::IsLeaf() const {

impeller/entity/contents/filters/inputs/filter_input.h

@@ -70,7 +70,7 @@ class FilterInput {
       Scalar scale);
 
   /// @see  `FilterContents::HasBasisTransformations`
-  virtual bool HasBasisTransformations() const;
+  virtual bool IsTranslationOnly() const;
 
   /// @brief  Returns `true` unless this input is a `FilterInput`, which may
   ///         take other inputs.

impeller/entity/contents/filters/matrix_filter_contents.cc

@@ -19,9 +19,8 @@ void MatrixFilterContents::SetIsForSubpass(bool is_subpass) {
   FilterContents::SetIsForSubpass(is_subpass);
 }
 
-bool MatrixFilterContents::HasBasisTransformations() const {
-  return !matrix_.Basis().IsIdentity() ||
-         FilterContents::HasBasisTransformations();
+bool MatrixFilterContents::IsTranslationOnly() const {
+  return matrix_.Basis().IsIdentity() && FilterContents::IsTranslationOnly();
 }
 
 void MatrixFilterContents::SetSamplerDescriptor(SamplerDescriptor desc) {

impeller/entity/contents/filters/matrix_filter_contents.h

@@ -21,7 +21,7 @@ class MatrixFilterContents final : public FilterContents {
   void SetIsForSubpass(bool is_for_subpass) override;
 
   // |FilterContents|
-  bool HasBasisTransformations() const override;
+  bool IsTranslationOnly() const override;
 
   void SetSamplerDescriptor(SamplerDescriptor desc);
 

impeller/entity/entity_pass.cc

@@ -114,7 +114,7 @@ std::optional<Rect> EntityPass::GetElementsCoverage(
       // as opposed to empty coverage.
       if (coverage.has_value() && coverage_limit.has_value()) {
         const auto* filter = entity->GetContents()->AsFilter();
-        if (!filter || !filter->HasBasisTransformations()) {
+        if (!filter || filter->IsTranslationOnly()) {
           coverage = coverage->Intersection(coverage_limit.value());
         }
       }
@@ -140,7 +140,7 @@ std::optional<Rect> EntityPass::GetElementsCoverage(
       }
 
       if (coverage.has_value() && coverage_limit.has_value() &&
-          (!image_filter || !image_filter->HasBasisTransformations())) {
+          (!image_filter || image_filter->IsTranslationOnly())) {
         coverage = coverage->Intersection(coverage_limit.value());
       }
     } else {
@@ -168,9 +168,9 @@ std::optional<Rect> EntityPass::GetSubpassCoverage(
   // If the filter graph transforms the basis of the subpass, then its space
   // has deviated too much from the parent pass to safely intersect with the
   // pass coverage limit.
-  coverage_limit = (image_filter && image_filter->HasBasisTransformations()
-                        ? std::nullopt
-                        : coverage_limit);
+  coverage_limit =
+      (image_filter && image_filter->IsTranslationOnly() ? std::nullopt
+                                                         : coverage_limit);
 
   auto entities_coverage = subpass.GetElementsCoverage(coverage_limit);
   // The entities don't cover anything. There is nothing to do.