From 3ba3fa72ae2fd4102cff22b947d124f72ce0f880 Mon Sep 17 00:00:00 2001 From: Florin Malita Date: Mon, 22 Jan 2018 10:19:28 -0500 Subject: [PATCH] [sksg] Refactor inval registration ... to avoid having too many Node friends. TBR= Change-Id: I8f8ff570d94ea48017935066a3d51cd8265ec120 Reviewed-on: https://skia-review.googlesource.com/97980 Reviewed-by: Florin Malita Commit-Queue: Florin Malita --- experimental/sksg/SkSGDraw.cpp | 8 +-- experimental/sksg/SkSGEffectNode.cpp | 4 +- experimental/sksg/SkSGGroup.cpp | 6 +- experimental/sksg/SkSGNode.cpp | 64 ++++++++++--------- experimental/sksg/SkSGNode.h | 26 +++----- experimental/sksg/effects/SkSGMaskEffect.cpp | 4 +- experimental/sksg/effects/SkSGTransform.cpp | 8 +-- .../sksg/geometry/SkSGGeometryTransform.cpp | 8 +-- experimental/sksg/geometry/SkSGMerge.cpp | 4 +- experimental/sksg/geometry/SkSGTrimEffect.cpp | 4 +- 10 files changed, 64 insertions(+), 72 deletions(-) diff --git a/experimental/sksg/SkSGDraw.cpp b/experimental/sksg/SkSGDraw.cpp index 33c73a5b2b..b73bf3b577 100644 --- a/experimental/sksg/SkSGDraw.cpp +++ b/experimental/sksg/SkSGDraw.cpp @@ -16,13 +16,13 @@ namespace sksg { Draw::Draw(sk_sp geometry, sk_sp paint) : fGeometry(std::move(geometry)) , fPaint(std::move(paint)) { - fGeometry->addInvalReceiver(this); - fPaint->addInvalReceiver(this); + this->observeInval(fGeometry); + this->observeInval(fPaint); } Draw::~Draw() { - fGeometry->removeInvalReceiver(this); - fPaint->removeInvalReceiver(this); + this->unobserveInval(fGeometry); + this->unobserveInval(fPaint); } void Draw::onRender(SkCanvas* canvas) const { diff --git a/experimental/sksg/SkSGEffectNode.cpp b/experimental/sksg/SkSGEffectNode.cpp index 1ecf7c7a92..70050ccb70 100644 --- a/experimental/sksg/SkSGEffectNode.cpp +++ b/experimental/sksg/SkSGEffectNode.cpp @@ -11,11 +11,11 @@ namespace sksg { EffectNode::EffectNode(sk_sp child) : fChild(std::move(child)) { - fChild->addInvalReceiver(this); + this->observeInval(fChild); } EffectNode::~EffectNode() { - fChild->removeInvalReceiver(this); + this->unobserveInval(fChild); } void EffectNode::onRender(SkCanvas* canvas) const { diff --git a/experimental/sksg/SkSGGroup.cpp b/experimental/sksg/SkSGGroup.cpp index b71837a8bf..aeccf233f7 100644 --- a/experimental/sksg/SkSGGroup.cpp +++ b/experimental/sksg/SkSGGroup.cpp @@ -13,7 +13,7 @@ Group::Group() {} Group::~Group() { for (const auto& child : fChildren) { - child->removeInvalReceiver(this); + this->unobserveInval(child); } } @@ -25,7 +25,7 @@ void Group::addChild(sk_sp node) { } } - node->addInvalReceiver(this); + this->observeInval(node); fChildren.push_back(std::move(node)); this->invalidate(); @@ -36,7 +36,7 @@ void Group::removeChild(const sk_sp& node) { for (int i = 0; i < origCount; ++i) { if (fChildren[i] == node) { fChildren.removeShuffle(i); - node->removeInvalReceiver(this); + this->unobserveInval(node); break; } } diff --git a/experimental/sksg/SkSGNode.cpp b/experimental/sksg/SkSGNode.cpp index 74867ef819..35b2640dbb 100644 --- a/experimental/sksg/SkSGNode.cpp +++ b/experimental/sksg/SkSGNode.cpp @@ -39,64 +39,66 @@ private: return Node::Node(uint32_t invalTraits) - : fInvalReceiver(nullptr) + : fInvalObserver(nullptr) , fBounds(SkRectPriv::MakeLargeS32()) , fInvalTraits(invalTraits) , fFlags(kInvalidated_Flag) {} Node::~Node() { - if (fFlags & kReceiverArray_Flag) { - SkASSERT(fInvalReceiverArray->isEmpty()); - delete fInvalReceiverArray; + if (fFlags & kObserverArray_Flag) { + SkASSERT(fInvalObserverArray->isEmpty()); + delete fInvalObserverArray; } else { - SkASSERT(!fInvalReceiver); + SkASSERT(!fInvalObserver); } } -void Node::addInvalReceiver(Node* receiver) { - if (!(fFlags & kReceiverArray_Flag)) { - if (!fInvalReceiver) { - fInvalReceiver = receiver; +void Node::observeInval(const sk_sp& node) { + SkASSERT(node); + if (!(node->fFlags & kObserverArray_Flag)) { + if (!node->fInvalObserver) { + node->fInvalObserver = this; return; } - auto receivers = new SkTDArray(); - receivers->setReserve(2); - receivers->push(fInvalReceiver); + auto observers = new SkTDArray(); + observers->setReserve(2); + observers->push(node->fInvalObserver); - fInvalReceiverArray = receivers; - fFlags |= kReceiverArray_Flag; + node->fInvalObserverArray = observers; + node->fFlags |= kObserverArray_Flag; } - // No duplicate receivers. - SkASSERT(fInvalReceiverArray->find(receiver) < 0); + // No duplicate observers. + SkASSERT(node->fInvalObserverArray->find(this) < 0); - fInvalReceiverArray->push(receiver); + node->fInvalObserverArray->push(this); } -void Node::removeInvalReceiver(Node* receiver) { - if (!(fFlags & kReceiverArray_Flag)) { - SkASSERT(fInvalReceiver == receiver); - fInvalReceiver = nullptr; +void Node::unobserveInval(const sk_sp& node) { + SkASSERT(node); + if (!(node->fFlags & kObserverArray_Flag)) { + SkASSERT(node->fInvalObserver == this); + node->fInvalObserver = nullptr; return; } - const auto idx = fInvalReceiverArray->find(receiver); + const auto idx = node->fInvalObserverArray->find(this); SkASSERT(idx >= 0); - fInvalReceiverArray->remove(idx); + node->fInvalObserverArray->remove(idx); } template -void Node::forEachInvalReceiver(Func&& func) const { - if (fFlags & kReceiverArray_Flag) { - for (const auto& parent : *fInvalReceiverArray) { +void Node::forEachInvalObserver(Func&& func) const { + if (fFlags & kObserverArray_Flag) { + for (const auto& parent : *fInvalObserverArray) { func(parent); } return; } - if (fInvalReceiver) { - func(fInvalReceiver); + if (fInvalObserver) { + func(fInvalObserver); } } @@ -109,15 +111,15 @@ void Node::invalidate(bool damageBubbling) { } if (damageBubbling && !(fInvalTraits & kBubbleDamage_Trait)) { - // Found a damage receiver. + // Found a damage observer. fFlags |= kDamage_Flag; damageBubbling = false; } fFlags |= kInvalidated_Flag; - forEachInvalReceiver([&](Node* receiver) { - receiver->invalidate(damageBubbling); + forEachInvalObserver([&](Node* observer) { + observer->invalidate(damageBubbling); }); } diff --git a/experimental/sksg/SkSGNode.h b/experimental/sksg/SkSGNode.h index 5518984e51..6396e4d33e 100644 --- a/experimental/sksg/SkSGNode.h +++ b/experimental/sksg/SkSGNode.h @@ -57,36 +57,26 @@ protected: // and return their bounding box in local coordinates. virtual SkRect onRevalidate(InvalidationController*, const SkMatrix& ctm) = 0; + // Register/unregister |this| to receive invalidation events from a descendant. + void observeInval(const sk_sp&); + void unobserveInval(const sk_sp&); + 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 + kObserverArray_Flag = 1 << 2, // the node has more than one inval observer 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. - friend class Draw; - friend class EffectNode; - friend class GeometryTransform; - friend class Group; - friend class MaskEffect; - friend class Matrix; - friend class Merge; - friend class Stroke; - friend class Transform; - friend class TrimEffect; - template - void forEachInvalReceiver(Func&&) const; + void forEachInvalObserver(Func&&) const; class ScopedFlag; union { - Node* fInvalReceiver; - SkTDArray* fInvalReceiverArray; + Node* fInvalObserver; + SkTDArray* fInvalObserverArray; }; SkRect fBounds; const uint32_t fInvalTraits : 16; diff --git a/experimental/sksg/effects/SkSGMaskEffect.cpp b/experimental/sksg/effects/SkSGMaskEffect.cpp index d4ce6df4db..17b4da7037 100644 --- a/experimental/sksg/effects/SkSGMaskEffect.cpp +++ b/experimental/sksg/effects/SkSGMaskEffect.cpp @@ -14,11 +14,11 @@ namespace sksg { MaskEffect::MaskEffect(sk_sp child, sk_sp mask) : INHERITED(std::move(child)) , fMaskNode(std::move(mask)) { - fMaskNode->addInvalReceiver(this); + this->observeInval(fMaskNode); } MaskEffect::~MaskEffect() { - fMaskNode->removeInvalReceiver(this); + this->unobserveInval(fMaskNode); } void MaskEffect::onRender(SkCanvas* canvas) const { diff --git a/experimental/sksg/effects/SkSGTransform.cpp b/experimental/sksg/effects/SkSGTransform.cpp index a5731b1a3f..6a985a971e 100644 --- a/experimental/sksg/effects/SkSGTransform.cpp +++ b/experimental/sksg/effects/SkSGTransform.cpp @@ -16,13 +16,13 @@ Matrix::Matrix(const SkMatrix& m, sk_sp parent) , fParent(std::move(parent)) , fLocalMatrix(m) { if (fParent) { - fParent->addInvalReceiver(this); + this->observeInval(fParent); } } Matrix::~Matrix() { if (fParent) { - fParent->removeInvalReceiver(this); + this->unobserveInval(fParent); } } @@ -40,11 +40,11 @@ SkRect Matrix::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) { Transform::Transform(sk_sp child, sk_sp matrix) : INHERITED(std::move(child)) , fMatrix(std::move(matrix)) { - fMatrix->addInvalReceiver(this); + this->observeInval(fMatrix); } Transform::~Transform() { - fMatrix->removeInvalReceiver(this); + this->unobserveInval(fMatrix); } void Transform::onRender(SkCanvas* canvas) const { diff --git a/experimental/sksg/geometry/SkSGGeometryTransform.cpp b/experimental/sksg/geometry/SkSGGeometryTransform.cpp index 14c37d98b9..91367d48d6 100644 --- a/experimental/sksg/geometry/SkSGGeometryTransform.cpp +++ b/experimental/sksg/geometry/SkSGGeometryTransform.cpp @@ -14,13 +14,13 @@ namespace sksg { GeometryTransform::GeometryTransform(sk_sp child, sk_sp matrix) : fChild(std::move(child)) , fMatrix(std::move(matrix)) { - fChild->addInvalReceiver(this); - fMatrix->addInvalReceiver(this); + this->observeInval(fChild); + this->observeInval(fMatrix); } GeometryTransform::~GeometryTransform() { - fChild->removeInvalReceiver(this); - fMatrix->removeInvalReceiver(this); + this->unobserveInval(fChild); + this->unobserveInval(fMatrix); } SkRect GeometryTransform::onRevalidate(InvalidationController* ic, const SkMatrix& ctm) { diff --git a/experimental/sksg/geometry/SkSGMerge.cpp b/experimental/sksg/geometry/SkSGMerge.cpp index 9c22e75a81..49e780413d 100644 --- a/experimental/sksg/geometry/SkSGMerge.cpp +++ b/experimental/sksg/geometry/SkSGMerge.cpp @@ -16,13 +16,13 @@ Merge::Merge(std::vector>&& geos, Mode mode) : fGeos(std::move(geos)) , fMode(mode) { for (const auto& geo : fGeos) { - geo->addInvalReceiver(this); + this->observeInval(geo); } } Merge::~Merge() { for (const auto& geo : fGeos) { - geo->removeInvalReceiver(this); + this->unobserveInval(geo); } } diff --git a/experimental/sksg/geometry/SkSGTrimEffect.cpp b/experimental/sksg/geometry/SkSGTrimEffect.cpp index 4ee85c8a73..f3118221d5 100644 --- a/experimental/sksg/geometry/SkSGTrimEffect.cpp +++ b/experimental/sksg/geometry/SkSGTrimEffect.cpp @@ -15,11 +15,11 @@ namespace sksg { TrimEffect::TrimEffect(sk_sp child) : fChild(std::move(child)) { - fChild->addInvalReceiver(this); + this->observeInval(fChild); } TrimEffect::~TrimEffect() { - fChild->removeInvalReceiver(this); + this->unobserveInval(fChild); } // TODO