[sksg] More inval fixes

Backpedal on node/reval-time-determined damage: nodes cannot control
the invalidation order, and shared descendants may be revalidated before
a particular ancestor gets to query their state - thus making any
decisions based on that invalid.

Instead, apply damage suppression at invalidation time, based on node
type/traits.  Node types which don't generate direct damage are marked
as such, and the invalidation logic bubbles damage past them, until it
finds a valid damage receiver.

Nodes which currently suppress damage:

 - PaintNode    (and subclasses)
 - GeometryNode (and subclasses)
 - Matrix

TBR=
Change-Id: I843e683e64cb6253d8c26d8397c44d02a7d6026f
Reviewed-on: https://skia-review.googlesource.com/91421
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>

21 files changed, 106 insertions, 117 deletions

diff --git a/experimental/sksg/SkSGDraw.cpp b/experimental/sksg/SkSGDraw.cpp
index 56415fc86e..efeee0577d 100644
--- a/experimental/sksg/SkSGDraw.cpp
+++ b/experimental/sksg/SkSGDraw.cpp
@@ -29,16 +29,14 @@ void Draw::onRender(SkCanvas* canvas) const {
     fGeometry->draw(canvas, fPaint->makePaint());
 }
 
-Node::RevalidationResult Draw::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) {
+SkRect Draw::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) {
     SkASSERT(this->hasInval());
 
     // TODO: adjust bounds for paint
     const auto bounds = fGeometry->revalidate(ic, ctm);
     fPaint->revalidate(ic, ctm);
 
-    // Neither paint nor geometry contribute to damage directly; instead we generate
-    // damage here, at the binding point.
-    return { bounds, Damage::kForceSelf };
+    return bounds;
 }
 
 } // namespace sksg
diff --git a/experimental/sksg/SkSGDraw.h b/experimental/sksg/SkSGDraw.h
index ad2b8fd602..20ead3d5f6 100644
--- a/experimental/sksg/SkSGDraw.h
+++ b/experimental/sksg/SkSGDraw.h
@@ -34,7 +34,7 @@ protected:
 
     void onRender(SkCanvas*) const override;
 
-    RevalidationResult onRevalidate(InvalidationController*, const SkMatrix&) override;
+    SkRect onRevalidate(InvalidationController*, const SkMatrix&) override;
 
 private:
     sk_sp<GeometryNode> fGeometry;
diff --git a/experimental/sksg/SkSGEffectNode.cpp b/experimental/sksg/SkSGEffectNode.cpp
index 4a1c8add8f..1ecf7c7a92 100644
--- a/experimental/sksg/SkSGEffectNode.cpp
+++ b/experimental/sksg/SkSGEffectNode.cpp
@@ -22,10 +22,10 @@ void EffectNode::onRender(SkCanvas* canvas) const {
     fChild->render(canvas);
 }
 
-Node::RevalidationResult EffectNode::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) {
+SkRect EffectNode::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) {
     SkASSERT(this->hasInval());
 
-    return { fChild->revalidate(ic, ctm), Damage::kDefault };
+    return fChild->revalidate(ic, ctm);
 }
 
 } // namespace sksg
diff --git a/experimental/sksg/SkSGEffectNode.h b/experimental/sksg/SkSGEffectNode.h
index 0fd71bc996..ab0968e96c 100644
--- a/experimental/sksg/SkSGEffectNode.h
+++ b/experimental/sksg/SkSGEffectNode.h
@@ -25,7 +25,7 @@ protected:
 
     void onRender(SkCanvas*) const override;
 
-    RevalidationResult onRevalidate(InvalidationController*, const SkMatrix&) override;
+    SkRect onRevalidate(InvalidationController*, const SkMatrix&) override;
 
 private:
     sk_sp<RenderNode> fChild;
diff --git a/experimental/sksg/SkSGGeometryNode.cpp b/experimental/sksg/SkSGGeometryNode.cpp
index cbc0d558e2..98022ad811 100644
--- a/experimental/sksg/SkSGGeometryNode.cpp
+++ b/experimental/sksg/SkSGGeometryNode.cpp
@@ -11,6 +11,9 @@
 
 namespace sksg {
 
+// Geometry nodes don't generate damage on their own, but via their aggregation ancestor Draw nodes.
+GeometryNode::GeometryNode() : INHERITED(kBubbleDamage_Trait) {}
+
 void GeometryNode::draw(SkCanvas* canvas, const SkPaint& paint) const {
     SkASSERT(!this->hasInval());
     this->onDraw(canvas, paint);
diff --git a/experimental/sksg/SkSGGeometryNode.h b/experimental/sksg/SkSGGeometryNode.h
index a898b06519..b436d27570 100644
--- a/experimental/sksg/SkSGGeometryNode.h
+++ b/experimental/sksg/SkSGGeometryNode.h
@@ -29,7 +29,7 @@ public:
     SkPath asPath() const;
 
 protected:
-    GeometryNode() = default;
+    GeometryNode();
 
     virtual void onDraw(SkCanvas*, const SkPaint&) const = 0;
 
diff --git a/experimental/sksg/SkSGGroup.cpp b/experimental/sksg/SkSGGroup.cpp
index 9f40d6ad48..b71837a8bf 100644
--- a/experimental/sksg/SkSGGroup.cpp
+++ b/experimental/sksg/SkSGGroup.cpp
@@ -28,7 +28,7 @@ void Group::addChild(sk_sp<RenderNode> node) {
     node->addInvalReceiver(this);
     fChildren.push_back(std::move(node));
 
-    this->invalidateSelf();
+    this->invalidate();
 }
 
 void Group::removeChild(const sk_sp<RenderNode>& node) {
@@ -42,7 +42,7 @@ void Group::removeChild(const sk_sp<RenderNode>& node) {
     }
     SkASSERT(fChildren.count() == origCount - 1);
 
-    this->invalidateSelf();
+    this->invalidate();
 }
 
 void Group::onRender(SkCanvas* canvas) const {
@@ -51,16 +51,16 @@ void Group::onRender(SkCanvas* canvas) const {
     }
 }
 
-Node::RevalidationResult Group::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) {
+SkRect Group::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) {
     SkASSERT(this->hasInval());
 
-    RevalidationResult result =  { SkRect::MakeEmpty(), Damage::kDefault };
+    SkRect bounds = SkRect::MakeEmpty();
 
     for (const auto& child : fChildren) {
-        result.fBounds.join(child->revalidate(ic, ctm));
+        bounds.join(child->revalidate(ic, ctm));
     }
 
-    return result;
+    return bounds;
 }
 
 } // namespace sksg
diff --git a/experimental/sksg/SkSGGroup.h b/experimental/sksg/SkSGGroup.h
index 3d8b1ad3c4..f9126ea37f 100644
--- a/experimental/sksg/SkSGGroup.h
+++ b/experimental/sksg/SkSGGroup.h
@@ -31,7 +31,7 @@ protected:
     ~Group() override;
 
     void onRender(SkCanvas*) const override;
-    RevalidationResult onRevalidate(InvalidationController*, const SkMatrix&) override;
+    SkRect onRevalidate(InvalidationController*, const SkMatrix&) override;
 
 private:
     SkTArray<sk_sp<RenderNode>, true> fChildren;
diff --git a/experimental/sksg/SkSGNode.cpp b/experimental/sksg/SkSGNode.cpp
index fc4d278580..768fdfa7b6 100644
--- a/experimental/sksg/SkSGNode.cpp
+++ b/experimental/sksg/SkSGNode.cpp
@@ -38,10 +38,11 @@ private:
     if (traversal_guard.wasSet())                        \
         return
 
-Node::Node()
+Node::Node(uint32_t invalTraits)
     : fInvalReceiver(nullptr)
     , fBounds(SkRect::MakeLargestS32())
-    , fFlags(kInvalSelf_Flag | kInvalDescendant_Flag) {}
+    , fInvalTraits(invalTraits)
+    , fFlags(kInvalidated_Flag) {}
 
 Node::~Node() {
     if (fFlags & kReceiverArray_Flag) {
@@ -99,24 +100,24 @@ void Node::forEachInvalReceiver(Func&& func) const {
     }
 }
 
-void Node::invalidateSelf() {
-    if (this->hasSelfInval()) {
+void Node::invalidate(bool damageBubbling) {
+    TRAVERSAL_GUARD;
+
+    if (this->hasInval() && (!damageBubbling || (fFlags & kDamage_Flag))) {
+        // All done.
         return;
     }
 
-    fFlags |= kInvalSelf_Flag;
-    this->invalidateAncestors();
-}
+    if (damageBubbling && !(fInvalTraits & kBubbleDamage_Trait)) {
+        // Found a damage receiver.
+        fFlags |= kDamage_Flag;
+        damageBubbling = false;
+    }
 
-void Node::invalidateAncestors() {
-    TRAVERSAL_GUARD;
+    fFlags |= kInvalidated_Flag;
 
     forEachInvalReceiver([&](Node* receiver) {
-        if (receiver->hasDescendantInval()) {
-            return;
-        }
-        receiver->fFlags |= kInvalDescendant_Flag;
-        receiver->invalidateAncestors();
+        receiver->invalidate(damageBubbling);
     });
 }
 
@@ -127,21 +128,21 @@ const SkRect& Node::revalidate(InvalidationController* ic, const SkMatrix& ctm)
         return fBounds;
     }
 
-    const auto result     = this->onRevalidate(ic, ctm);
-    const auto selfDamage = result.fDamage == Damage::kForceSelf ||
-                            (this->hasSelfInval() && result.fDamage != Damage::kBlockSelf);
+    SkRect prevBounds;
+    if (fFlags & kDamage_Flag) {
+        prevBounds = fBounds;
+    }
+
+    fBounds = this->onRevalidate(ic, ctm);
 
-    if (selfDamage) {
-        // old bounds
-        ic->inval(fBounds, ctm);
-        if (result.fBounds != fBounds) {
-            // new bounds
-            ic->inval(result.fBounds, ctm);
+    if (fFlags & kDamage_Flag) {
+        ic->inval(prevBounds, ctm);
+        if (fBounds != prevBounds) {
+            ic->inval(fBounds, ctm);
         }
     }
 
-    fBounds = result.fBounds;
-    fFlags &= ~(kInvalSelf_Flag | kInvalDescendant_Flag);
+    fFlags &= ~(kInvalidated_Flag | kDamage_Flag);
 
     return fBounds;
 }
diff --git a/experimental/sksg/SkSGNode.h b/experimental/sksg/SkSGNode.h
index 1a7560684c..769e2a4e68 100644
--- a/experimental/sksg/SkSGNode.h
+++ b/experimental/sksg/SkSGNode.h
@@ -35,30 +35,31 @@ public:
     const SkRect& revalidate(InvalidationController*, const SkMatrix&);
 
 protected:
-    Node();
-    ~Node() override;
+    enum InvalTraits {
+        // Nodes with this trait never generate direct damage -- instead,
+        // the damage bubbles up to ancestors.
+        kBubbleDamage_Trait = 1 << 0,
+    };
 
-    void invalidateSelf();
-    void invalidateAncestors();
+    explicit Node(uint32_t invalTraits);
+    ~Node() override;
 
-    bool hasSelfInval()       const { return fFlags & kInvalSelf_Flag; }
-    bool hasDescendantInval() const { return fFlags & kInvalDescendant_Flag; }
-    bool hasInval()           const { return this->hasSelfInval() || this->hasDescendantInval(); }
+    // Tag this node for invalidation and optional damage.
+    void invalidate(bool damage = true);
+    bool hasInval() const { return fFlags & kInvalidated_Flag; }
 
     // Dispatched on revalidation.  Subclasses are expected to recompute/cache their properties
     // and return their bounding box in local coordinates.
-    enum class Damage {
-        kDefault,    // respects the local kInvalSelf_Flag
-        kForceSelf,  // forces self revalidation regardless of kInvalSelf_Flag
-        kBlockSelf,  // blocks self revalidation regardless of kInvalSelf_Flag
-    };
-    struct RevalidationResult {
-        SkRect  fBounds;
-        Damage  fDamage;
-    };
-    virtual RevalidationResult onRevalidate(InvalidationController*, const SkMatrix& ctm) = 0;
+    virtual SkRect onRevalidate(InvalidationController*, const SkMatrix& ctm) = 0;
 
 private:
+    enum Flags {
+        kInvalidated_Flag   = 1 << 0, // the node or its descendants require revalidation
+        kDamage_Flag        = 1 << 1, // the node contributes damage during revalidation
+        kReceiverArray_Flag = 1 << 2, // the node has more than one inval receiver
+        kInTraversal_Flag   = 1 << 3, // the node is part of a traversal (cycle detection)
+    };
+
     void addInvalReceiver(Node*);
     void removeInvalReceiver(Node*);
     // TODO: too friendly, find another way.
@@ -73,13 +74,6 @@ private:
     template <typename Func>
     void forEachInvalReceiver(Func&&) const;
 
-    enum Flags {
-        kInvalSelf_Flag       = 1 << 0, // the node requires revalidation
-        kInvalDescendant_Flag = 1 << 1, // the node's descendents require invalidation
-        kReceiverArray_Flag   = 1 << 2, // the node has more than one inval receiver
-        kInTraversal_Flag     = 1 << 3, // the node is part of a traversal (cycle detection)
-    };
-
     class ScopedFlag;
 
     union {
@@ -87,7 +81,8 @@ private:
         SkTDArray<Node*>* fInvalReceiverArray;
     };
     SkRect                fBounds;
-    uint32_t              fFlags;
+    const uint32_t        fInvalTraits : 16;
+    uint32_t              fFlags       : 16;
 
     typedef SkRefCnt INHERITED;
 };
@@ -98,7 +93,7 @@ private:
     void set##attr_name(attr_type v) {                          \
         if (attr_container == v) return;                        \
         attr_container = v;                                     \
-        this->invalidateSelf();                                 \
+        this->invalidate();                                 \
    }
 
 } // namespace sksg
diff --git a/experimental/sksg/SkSGPaintNode.cpp b/experimental/sksg/SkSGPaintNode.cpp
index be8edac7c5..146d248b8e 100644
--- a/experimental/sksg/SkSGPaintNode.cpp
+++ b/experimental/sksg/SkSGPaintNode.cpp
@@ -9,7 +9,8 @@
 
 namespace sksg {
 
-PaintNode::PaintNode() {}
+// Paint nodes don't generate damage on their own, but via their aggregation ancestor Draw nodes.
+PaintNode::PaintNode() : INHERITED(kBubbleDamage_Trait) {}
 
 const SkPaint& PaintNode::makePaint() {
     SkASSERT(!this->hasInval());
@@ -17,23 +18,20 @@ const SkPaint& PaintNode::makePaint() {
     return fPaint;
 }
 
-Node::RevalidationResult PaintNode::onRevalidate(InvalidationController*, const SkMatrix&) {
+SkRect PaintNode::onRevalidate(InvalidationController*, const SkMatrix&) {
     SkASSERT(this->hasInval());
 
-    if (this->hasSelfInval()) {
-        fPaint.reset();
-        fPaint.setAntiAlias(fAntiAlias);
-        fPaint.setStyle(fStyle);
-        fPaint.setStrokeWidth(fStrokeWidth);
-        fPaint.setStrokeMiter(fStrokeMiter);
-        fPaint.setStrokeJoin(fStrokeJoin);
-        fPaint.setStrokeCap(fStrokeCap);
+    fPaint.reset();
+    fPaint.setAntiAlias(fAntiAlias);
+    fPaint.setStyle(fStyle);
+    fPaint.setStrokeWidth(fStrokeWidth);
+    fPaint.setStrokeMiter(fStrokeMiter);
+    fPaint.setStrokeJoin(fStrokeJoin);
+    fPaint.setStrokeCap(fStrokeCap);
 
-        this->onApplyToPaint(&fPaint);
-    }
+    this->onApplyToPaint(&fPaint);
 
-    // Paints have no bounds and don't contribute to damage.
-    return { SkRect::MakeEmpty(), Damage::kBlockSelf };
+    return SkRect::MakeEmpty();
 }
 
 } // namespace sksg
diff --git a/experimental/sksg/SkSGPaintNode.h b/experimental/sksg/SkSGPaintNode.h
index 0dac92e444..a2fbada065 100644
--- a/experimental/sksg/SkSGPaintNode.h
+++ b/experimental/sksg/SkSGPaintNode.h
@@ -36,7 +36,7 @@ protected:
 
     virtual void onApplyToPaint(SkPaint*) const = 0;
 
-    RevalidationResult onRevalidate(InvalidationController*, const SkMatrix&) final;
+    SkRect onRevalidate(InvalidationController*, const SkMatrix&) final;
 
 private:
     SkPaint        fPaint;
diff --git a/experimental/sksg/SkSGRenderNode.cpp b/experimental/sksg/SkSGRenderNode.cpp
index 19e4d0ea5a..e952c69f20 100644
--- a/experimental/sksg/SkSGRenderNode.cpp
+++ b/experimental/sksg/SkSGRenderNode.cpp
@@ -9,7 +9,7 @@
 
 namespace sksg {
 
-RenderNode::RenderNode() {}
+RenderNode::RenderNode() : INHERITED(0) {}
 
 void RenderNode::render(SkCanvas* canvas) const {
     SkASSERT(!this->hasInval());
diff --git a/experimental/sksg/effects/SkSGTransform.cpp b/experimental/sksg/effects/SkSGTransform.cpp
index 1ea1e619a8..a5731b1a3f 100644
--- a/experimental/sksg/effects/SkSGTransform.cpp
+++ b/experimental/sksg/effects/SkSGTransform.cpp
@@ -10,9 +10,10 @@
 #include "SkCanvas.h"
 
 namespace sksg {
-
+// Matrix nodes don't generate damage on their own, but via aggregation ancestor Transform nodes.
 Matrix::Matrix(const SkMatrix& m, sk_sp<Matrix> parent)
-    : fParent(std::move(parent))
+    : INHERITED(kBubbleDamage_Trait)
+    , fParent(std::move(parent))
     , fLocalMatrix(m) {
     if (fParent) {
         fParent->addInvalReceiver(this);
@@ -25,7 +26,7 @@ Matrix::~Matrix() {
     }
 }
 
-Node::RevalidationResult Matrix::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) {
+SkRect Matrix::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) {
     fTotalMatrix = fLocalMatrix;
 
     if (fParent) {
@@ -33,8 +34,7 @@ Node::RevalidationResult Matrix::onRevalidate(InvalidationController* ic, const
         fTotalMatrix.postConcat(fParent->getTotalMatrix());
     }
 
-    // A free-floating matrix contributes no damage.
-    return { SkRect::MakeEmpty(), Damage::kBlockSelf };
+    return SkRect::MakeEmpty();
 }
 
 Transform::Transform(sk_sp<RenderNode> child, sk_sp<Matrix> matrix)
@@ -54,19 +54,17 @@ void Transform::onRender(SkCanvas* canvas) const {
     this->INHERITED::onRender(canvas);
 }
 
-Node::RevalidationResult Transform::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) {
+SkRect Transform::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) {
     SkASSERT(this->hasInval());
 
-    // We don't care about matrix reval results, but we do care whether it was invalidated.
-    const auto localDamage = fMatrix->hasInval() ? Damage::kForceSelf : Damage::kDefault;
+    // We don't care about matrix reval results.
     fMatrix->revalidate(ic, ctm);
 
     const auto& m = fMatrix->getTotalMatrix();
-    auto result = this->INHERITED::onRevalidate(ic, SkMatrix::Concat(ctm, m));
-    m.mapRect(&result.fBounds);
-    result.fDamage = localDamage;
+    auto bounds = this->INHERITED::onRevalidate(ic, SkMatrix::Concat(ctm, m));
+    m.mapRect(&bounds);
 
-    return result;
+    return bounds;
 }
 
 } // namespace sksg
diff --git a/experimental/sksg/effects/SkSGTransform.h b/experimental/sksg/effects/SkSGTransform.h
index 0d11739f30..0694117a2a 100644
--- a/experimental/sksg/effects/SkSGTransform.h
+++ b/experimental/sksg/effects/SkSGTransform.h
@@ -32,9 +32,9 @@ public:
     const SkMatrix& getTotalMatrix() const { return fTotalMatrix; }
 
 protected:
-    explicit Matrix(const SkMatrix&, sk_sp<Matrix>);
+    Matrix(const SkMatrix&, sk_sp<Matrix>);
 
-    RevalidationResult onRevalidate(InvalidationController*, const SkMatrix&) override;
+    SkRect onRevalidate(InvalidationController*, const SkMatrix&) override;
 
 private:
     sk_sp<Matrix> fParent;
@@ -68,7 +68,7 @@ protected:
 
     void onRender(SkCanvas*) const override;
 
-    RevalidationResult onRevalidate(InvalidationController*, const SkMatrix&) override;
+    SkRect onRevalidate(InvalidationController*, const SkMatrix&) override;
 
 private:
     sk_sp<Matrix> fMatrix;
diff --git a/experimental/sksg/geometry/SkSGMerge.cpp b/experimental/sksg/geometry/SkSGMerge.cpp
index 47429a5444..a9f06d464f 100644
--- a/experimental/sksg/geometry/SkSGMerge.cpp
+++ b/experimental/sksg/geometry/SkSGMerge.cpp
@@ -54,7 +54,7 @@ static SkPathOp mode_to_op(Merge::Mode mode) {
     return kUnion_SkPathOp;
 }
 
-Node::RevalidationResult Merge::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) {
+SkRect Merge::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) {
     SkASSERT(this->hasInval());
 
     const auto op = mode_to_op(fMode);
@@ -75,8 +75,7 @@ Node::RevalidationResult Merge::onRevalidate(InvalidationController* ic, const S
         builder.resolve(&fMerged);
     }
 
-    // Geometry does not contribute damage directly.
-    return { fMerged.computeTightBounds(), Damage::kBlockSelf };
+    return fMerged.computeTightBounds();
 }
 
 } // namespace skotty
diff --git a/experimental/sksg/geometry/SkSGMerge.h b/experimental/sksg/geometry/SkSGMerge.h
index f3e20877fa..b0cb40de9c 100644
--- a/experimental/sksg/geometry/SkSGMerge.h
+++ b/experimental/sksg/geometry/SkSGMerge.h
@@ -45,7 +45,7 @@ public:
 protected:
     void onDraw(SkCanvas*, const SkPaint&) const override;
 
-    RevalidationResult onRevalidate(InvalidationController*, const SkMatrix&) override;
+    SkRect onRevalidate(InvalidationController*, const SkMatrix&) override;
     SkPath onAsPath() const override;
 
 private:
diff --git a/experimental/sksg/geometry/SkSGPath.cpp b/experimental/sksg/geometry/SkSGPath.cpp
index ce1ff392ac..2b7dc94952 100644
--- a/experimental/sksg/geometry/SkSGPath.cpp
+++ b/experimental/sksg/geometry/SkSGPath.cpp
@@ -18,11 +18,10 @@ void Path::onDraw(SkCanvas* canvas, const SkPaint& paint) const {
     canvas->drawPath(fPath, paint);
 }
 
-Node::RevalidationResult Path::onRevalidate(InvalidationController*, const SkMatrix&) {
-    SkASSERT(this->hasSelfInval());
+SkRect Path::onRevalidate(InvalidationController*, const SkMatrix&) {
+    SkASSERT(this->hasInval());
 
-    // Geometry does not contribute damage directly.
-    return { fPath.computeTightBounds(), Damage::kBlockSelf };
+    return fPath.computeTightBounds();
 }
 
 SkPath Path::onAsPath() const {
diff --git a/experimental/sksg/geometry/SkSGPath.h b/experimental/sksg/geometry/SkSGPath.h
index 32388c988b..18caa10f2a 100644
--- a/experimental/sksg/geometry/SkSGPath.h
+++ b/experimental/sksg/geometry/SkSGPath.h
@@ -30,7 +30,7 @@ public:
 protected:
     void onDraw(SkCanvas*, const SkPaint&) const override;
 
-    RevalidationResult onRevalidate(InvalidationController*, const SkMatrix&) override;
+    SkRect onRevalidate(InvalidationController*, const SkMatrix&) override;
     SkPath onAsPath() const override;
 
 private:
diff --git a/experimental/sksg/geometry/SkSGRect.cpp b/experimental/sksg/geometry/SkSGRect.cpp
index 0184dace90..532a604af4 100644
--- a/experimental/sksg/geometry/SkSGRect.cpp
+++ b/experimental/sksg/geometry/SkSGRect.cpp
@@ -19,11 +19,10 @@ void Rect::onDraw(SkCanvas* canvas, const SkPaint& paint) const {
     canvas->drawRect(fRect, paint);
 }
 
-Node::RevalidationResult Rect::onRevalidate(InvalidationController*, const SkMatrix&) {
-    SkASSERT(this->hasSelfInval());
+SkRect Rect::onRevalidate(InvalidationController*, const SkMatrix&) {
+    SkASSERT(this->hasInval());
 
-    // Geometry does not contribute damage directly.
-    return { fRect, Damage::kBlockSelf };
+    return fRect;
 }
 
 SkPath Rect::onAsPath() const {
@@ -38,11 +37,10 @@ void RRect::onDraw(SkCanvas* canvas, const SkPaint& paint) const {
     canvas->drawRRect(fRRect, paint);
 }
 
-Node::RevalidationResult RRect::onRevalidate(InvalidationController*, const SkMatrix&) {
-    SkASSERT(this->hasSelfInval());
+SkRect RRect::onRevalidate(InvalidationController*, const SkMatrix&) {
+    SkASSERT(this->hasInval());
 
-    // Geometry does not contribute damage directly.
-    return { fRRect.getBounds(), Damage::kBlockSelf };
+    return fRRect.getBounds();
 }
 
 SkPath RRect::onAsPath() const {
diff --git a/experimental/sksg/geometry/SkSGRect.h b/experimental/sksg/geometry/SkSGRect.h
index 4667b6b42d..ad27910da9 100644
--- a/experimental/sksg/geometry/SkSGRect.h
+++ b/experimental/sksg/geometry/SkSGRect.h
@@ -34,7 +34,7 @@ public:
 protected:
     void onDraw(SkCanvas*, const SkPaint&) const override;
 
-    RevalidationResult onRevalidate(InvalidationController*, const SkMatrix&) override;
+    SkRect onRevalidate(InvalidationController*, const SkMatrix&) override;
     SkPath onAsPath() const override;
 
 private:
@@ -56,7 +56,7 @@ public:
 protected:
     void onDraw(SkCanvas*, const SkPaint&) const override;
 
-    RevalidationResult onRevalidate(InvalidationController*, const SkMatrix&) override;
+    SkRect onRevalidate(InvalidationController*, const SkMatrix&) override;
     SkPath onAsPath() const override;
 
 private: