From 58a7ddd55dab8d5e63704dc90e4ec0d69600be21 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Fri, 22 May 2020 01:17:26 -0700 Subject: [PATCH] Implement equality of ARTElements and use it in ARTState Summary: Here I'm implementing equality methods for ARTGroup, ARTShape and ARTText and I'm using these methods to update the state only when it is necessary. This will improve perf in rendering of ART changelog: [Internal] Reviewed By: JoshuaGross Differential Revision: D21695127 fbshipit-source-id: b438ddea4c34bd7a0bdf26a6aac4fd62a9f78b49 --- .../fabric/components/art/state/ARTElement.h | 31 ++++++++++++++++ .../fabric/components/art/state/ARTGroup.cpp | 18 +++++++++ .../fabric/components/art/state/ARTGroup.h | 7 +++- .../fabric/components/art/state/ARTShape.cpp | 33 +++++++++++++++++ .../fabric/components/art/state/ARTShape.h | 3 ++ .../fabric/components/art/state/ARTText.cpp | 37 +++++++++++++++++++ .../fabric/components/art/state/ARTText.h | 9 +++-- .../fabric/components/art/state/primitives.h | 21 +++++++++++ .../surfaceview/ARTSurfaceViewShadowNode.cpp | 12 ++---- 9 files changed, 158 insertions(+), 13 deletions(-) diff --git a/ReactCommon/fabric/components/art/state/ARTElement.h b/ReactCommon/fabric/components/art/state/ARTElement.h index 51a29de1e6a..1b3b82b1685 100644 --- a/ReactCommon/fabric/components/art/state/ARTElement.h +++ b/ReactCommon/fabric/components/art/state/ARTElement.h @@ -41,6 +41,37 @@ class ARTElement { Float opacity; std::vector transform; + virtual bool operator==(const ARTElement &rhs) const = 0; + virtual bool operator!=(const ARTElement &rhs) const = 0; + friend bool operator==(ListOfShared e1, ListOfShared e2) { + bool equals = e1.size() == e2.size(); + for (int i = 0; i < equals && e1.size(); i++) { + // Pointer equality - this will work if both are pointing at the same + // object, or both are nullptr + if (e1[i] == e2[i]) { + continue; + } + + // Get pointers from both + // If one is null, we know they can't both be null because of the above + // check + auto ptr1 = e1[i].get(); + auto ptr2 = e2[i].get(); + if (ptr1 == nullptr || ptr2 == nullptr) { + equals = false; + break; + } + + // Dereference and compare objects + if (*ptr1 != *ptr2) { + equals = false; + break; + } + } + + return equals; + }; + #ifdef ANDROID virtual folly::dynamic getDynamic() const = 0; #endif diff --git a/ReactCommon/fabric/components/art/state/ARTGroup.cpp b/ReactCommon/fabric/components/art/state/ARTGroup.cpp index 99fc1026836..f3a8c6ee59b 100644 --- a/ReactCommon/fabric/components/art/state/ARTGroup.cpp +++ b/ReactCommon/fabric/components/art/state/ARTGroup.cpp @@ -12,6 +12,24 @@ namespace facebook { namespace react { +bool ARTGroup::operator==(const ARTElement &rhs) const { + if (rhs.elementType != ARTElementType::Group) { + return false; + } + auto group = (const ARTGroup &)(rhs); + return std::tie(elementType, opacity, transform, clipping) == + std::tie( + group.elementType, + group.opacity, + group.transform, + group.clipping) && + elements == group.elements; +} + +bool ARTGroup::operator!=(const ARTElement &rhs) const { + return !(*this == rhs); +} + #ifdef ANDROID folly::dynamic ARTGroup::getDynamic() const { return toDynamic(*this); diff --git a/ReactCommon/fabric/components/art/state/ARTGroup.h b/ReactCommon/fabric/components/art/state/ARTGroup.h index 11198f701eb..fa86e792365 100644 --- a/ReactCommon/fabric/components/art/state/ARTGroup.h +++ b/ReactCommon/fabric/components/art/state/ARTGroup.h @@ -21,11 +21,11 @@ namespace react { */ class ARTGroup : public ARTElement { public: - using Shared = std::shared_ptr; + using Shared = std::shared_ptr; ARTGroup( Float opacity, std::vector transform, - ARTElement::ListOfShared elements, + ARTGroup::ListOfShared elements, std::vector clipping) : ARTElement(ARTElementType::Group, opacity, transform), elements(elements), @@ -37,6 +37,9 @@ class ARTGroup : public ARTElement { std::vector clipping{}; + bool operator==(const ARTElement &rhs) const override; + bool operator!=(const ARTElement &rhs) const override; + #ifdef ANDROID folly::dynamic getDynamic() const override; #endif diff --git a/ReactCommon/fabric/components/art/state/ARTShape.cpp b/ReactCommon/fabric/components/art/state/ARTShape.cpp index 393be9a272d..31fac9ffeb3 100644 --- a/ReactCommon/fabric/components/art/state/ARTShape.cpp +++ b/ReactCommon/fabric/components/art/state/ARTShape.cpp @@ -12,6 +12,39 @@ namespace facebook { namespace react { +bool ARTShape::operator==(const ARTElement &rhs) const { + if (rhs.elementType != ARTElementType::Shape) { + return false; + } + auto shape = (const ARTShape &)(rhs); + return std::tie( + elementType, + opacity, + transform, + d, + stroke, + strokeDash, + fill, + strokeWidth, + strokeCap, + strokeJoin) == + std::tie( + shape.elementType, + shape.opacity, + shape.transform, + shape.d, + shape.stroke, + shape.strokeDash, + shape.fill, + shape.strokeWidth, + shape.strokeCap, + shape.strokeJoin); +} + +bool ARTShape::operator!=(const ARTElement &rhs) const { + return !(*this == rhs); +} + #ifdef ANDROID folly::dynamic ARTShape::getDynamic() const { return toDynamic(*this); diff --git a/ReactCommon/fabric/components/art/state/ARTShape.h b/ReactCommon/fabric/components/art/state/ARTShape.h index 42438dd1a70..e50cf966a06 100644 --- a/ReactCommon/fabric/components/art/state/ARTShape.h +++ b/ReactCommon/fabric/components/art/state/ARTShape.h @@ -51,6 +51,9 @@ class ARTShape : public ARTElement { int strokeCap{1}; int strokeJoin{1}; + bool operator==(const ARTElement &rhs) const override; + bool operator!=(const ARTElement &rhs) const override; + #ifdef ANDROID folly::dynamic getDynamic() const override; #endif diff --git a/ReactCommon/fabric/components/art/state/ARTText.cpp b/ReactCommon/fabric/components/art/state/ARTText.cpp index 9ed21b0d10f..1a061554134 100644 --- a/ReactCommon/fabric/components/art/state/ARTText.cpp +++ b/ReactCommon/fabric/components/art/state/ARTText.cpp @@ -12,6 +12,43 @@ namespace facebook { namespace react { +bool ARTText::operator==(const ARTElement &rhs) const { + if (rhs.elementType != ARTElementType::Text) { + return false; + } + auto text = (const ARTText &)(rhs); + return std::tie( + elementType, + opacity, + transform, + d, + stroke, + strokeDash, + fill, + strokeWidth, + strokeCap, + strokeJoin, + alignment, + frame) == + std::tie( + text.elementType, + text.opacity, + text.transform, + text.d, + text.stroke, + text.strokeDash, + text.fill, + text.strokeWidth, + text.strokeCap, + text.strokeJoin, + text.alignment, + text.frame); +} + +bool ARTText::operator!=(const ARTElement &rhs) const { + return !(*this == rhs); +} + #ifdef ANDROID folly::dynamic ARTText::getDynamic() const { return toDynamic(*this); diff --git a/ReactCommon/fabric/components/art/state/ARTText.h b/ReactCommon/fabric/components/art/state/ARTText.h index ddda9790379..b01da694a37 100644 --- a/ReactCommon/fabric/components/art/state/ARTText.h +++ b/ReactCommon/fabric/components/art/state/ARTText.h @@ -47,12 +47,15 @@ class ARTText : public ARTShape { strokeCap, strokeJoin), alignment(alignment), - frame(frame){ - // elementType = ARTElementType::Text; - }; + frame(frame) { + elementType = ARTElementType::Text; + }; ARTText() = default; virtual ~ARTText(){}; + bool operator==(const ARTElement &rhs) const override; + bool operator!=(const ARTElement &rhs) const override; + ARTTextAlignment alignment{ARTTextAlignment::Default}; ARTTextFrame frame{}; diff --git a/ReactCommon/fabric/components/art/state/primitives.h b/ReactCommon/fabric/components/art/state/primitives.h index 88a039cbbb1..677fcf9ce11 100644 --- a/ReactCommon/fabric/components/art/state/primitives.h +++ b/ReactCommon/fabric/components/art/state/primitives.h @@ -24,11 +24,32 @@ struct ARTTextFrameFont { std::string fontStyle; std::string fontFamily; std::string fontWeight; + + bool operator==(const ARTTextFrameFont &rhs) const { + return std::tie( + this->fontSize, + this->fontStyle, + this->fontFamily, + this->fontWeight) == + std::tie(rhs.fontSize, rhs.fontStyle, rhs.fontFamily, rhs.fontWeight); + } + + bool operator!=(const ARTTextFrameFont &rhs) const { + return !(*this == rhs); + } }; struct ARTTextFrame { std::vector lines; ARTTextFrameFont font; + + bool operator==(const ARTTextFrame &rhs) const { + return std::tie(this->lines, this->font) == std::tie(rhs.lines, rhs.font); + } + + bool operator!=(const ARTTextFrame &rhs) const { + return !(*this == rhs); + } }; } // namespace react diff --git a/ReactCommon/fabric/components/art/surfaceview/ARTSurfaceViewShadowNode.cpp b/ReactCommon/fabric/components/art/surfaceview/ARTSurfaceViewShadowNode.cpp index a83b506a12a..5be367ba3b6 100644 --- a/ReactCommon/fabric/components/art/surfaceview/ARTSurfaceViewShadowNode.cpp +++ b/ReactCommon/fabric/components/art/surfaceview/ARTSurfaceViewShadowNode.cpp @@ -36,14 +36,10 @@ Content const &ARTSurfaceViewShadowNode::getContent() const { void ARTSurfaceViewShadowNode::updateStateIfNeeded(Content const &content) { ensureUnsealed(); - - // TODO T64130144: Compare to make sure it is needed. - // auto &state = getStateData(); - // if (state.attributedString == content.attributedString && - // state.layoutManager == textLayoutManager_) { - // return; - // } - + auto &state = getStateData(); + if (content.elements == state.elements) { + return; + } setStateData(ARTSurfaceViewState{content.elements}); }