From c5182c4bf5f8aca53ddbdcab10c78938970eed0e Mon Sep 17 00:00:00 2001 From: Pritesh Nandgaonkar Date: Tue, 31 Oct 2017 08:04:04 -0700 Subject: [PATCH] Used hasDirtyChildren tag for the optimization Summary: More fine grained dirty marking Currently a node's dirty flag propagates to the root of the tree ensuring that when any node is invalidated its whole subtree will be re-calculated. This is often times not needed. There are many properties which only effects a node's children and would not need to propagate all the way to the root such as align-items. Also in cases where the style does change layout it may not need to propagate all the way to the root but can often stop at the nearest position: absolute parent. This change has the potential of greatly improving performance of re-calculating a tree. This might require adding a second dirty flag named hasDirtyDescendants ensuring that traversal still works even though a parent is not marked as dirty. Reviewed By: emilsjolander Differential Revision: D6134754 fbshipit-source-id: bbcfee14058140b946401de756a3f130de0f51cd --- java/com/facebook/yoga/YogaNode.java | 14 +++++--- java/jni/YGJNI.cpp | 5 +++ tests/YGDirtyMarkingTest.cpp | 39 ++++++++++++++++++++++ yoga/Yoga.c | 49 ++++++++++++++++++++++++++-- yoga/Yoga.h | 1 + 5 files changed, 101 insertions(+), 7 deletions(-) diff --git a/java/com/facebook/yoga/YogaNode.java b/java/com/facebook/yoga/YogaNode.java index 815fd2c3..91a0910a 100644 --- a/java/com/facebook/yoga/YogaNode.java +++ b/java/com/facebook/yoga/YogaNode.java @@ -9,13 +9,11 @@ package com.facebook.yoga; -import javax.annotation.Nullable; - -import java.util.List; -import java.util.ArrayList; - import com.facebook.proguard.annotations.DoNotStrip; import com.facebook.soloader.SoLoader; +import java.util.ArrayList; +import java.util.List; +import javax.annotation.Nullable; @DoNotStrip public class YogaNode { @@ -198,6 +196,12 @@ public class YogaNode { return jni_YGNodeIsDirty(mNativePointer); } + private native boolean jni_YGNodeHasDirtyDescendants(long nativePointer); + + public boolean hasDirtyDescendants() { + return jni_YGNodeHasDirtyDescendants(mNativePointer); + } + private native void jni_YGNodeCopyStyle(long dstNativePointer, long srcNativePointer); public void copyStyle(YogaNode srcNode) { jni_YGNodeCopyStyle(mNativePointer, srcNode.mNativePointer); diff --git a/java/jni/YGJNI.cpp b/java/jni/YGJNI.cpp index df039883..cae5df64 100644 --- a/java/jni/YGJNI.cpp +++ b/java/jni/YGJNI.cpp @@ -249,6 +249,10 @@ jboolean jni_YGNodeIsDirty(alias_ref, jlong nativePointer) { return (jboolean) YGNodeIsDirty(_jlong2YGNodeRef(nativePointer)); } +jboolean jni_YGNodeHasDirtyDescendants(alias_ref, jlong nativePointer) { + return (jboolean) YGNodeHasDirtyDescendants(_jlong2YGNodeRef(nativePointer)); +} + void jni_YGNodeSetHasMeasureFunc(alias_ref, jlong nativePointer, jboolean hasMeasureFunc) { YGNodeSetMeasureFunc(_jlong2YGNodeRef(nativePointer), hasMeasureFunc ? YGJNIMeasureFunc : NULL); } @@ -453,6 +457,7 @@ jint JNI_OnLoad(JavaVM *vm, void *) { YGMakeNativeMethod(jni_YGNodeCalculateLayout), YGMakeNativeMethod(jni_YGNodeMarkDirty), YGMakeNativeMethod(jni_YGNodeIsDirty), + YGMakeNativeMethod(jni_YGNodeHasDirtyDescendants), YGMakeNativeMethod(jni_YGNodeSetHasMeasureFunc), YGMakeNativeMethod(jni_YGNodeSetHasBaselineFunc), YGMakeNativeMethod(jni_YGNodeCopyStyle), diff --git a/tests/YGDirtyMarkingTest.cpp b/tests/YGDirtyMarkingTest.cpp index 953de26a..2c251046 100644 --- a/tests/YGDirtyMarkingTest.cpp +++ b/tests/YGDirtyMarkingTest.cpp @@ -10,6 +10,45 @@ #include #include +TEST(YogaTest, dirty_child_propogation) { + const YGNodeRef root = YGNodeNew(); + YGNodeStyleSetAlignItems(root, YGAlignFlexStart); + YGNodeStyleSetHeight(root, 200); + YGNodeStyleSetWidth(root, 500); + + const YGNodeRef child0 = YGNodeNew(); + YGNodeStyleSetWidth(child0, 30); + YGNodeStyleSetHeight(child0, 30); + YGNodeInsertChild(root, child0, 0); + + const YGNodeRef child1 = YGNodeNew(); + YGNodeStyleSetPositionType(child1, YGPositionTypeAbsolute); + YGNodeStyleSetPosition(child1, YGEdgeStart, 10); + YGNodeStyleSetPosition(child1, YGEdgeTop, 10); + YGNodeStyleSetWidth(child1, 10); + YGNodeStyleSetHeight(child1, 10); + + YGNodeInsertChild(root, child1, 1); + + const YGNodeRef child1_child0 = YGNodeNew(); + YGNodeStyleSetWidth(child1_child0, 10); + YGNodeStyleSetHeight(child1_child0, 10); + YGNodeInsertChild(child1, child1_child0, 0); + + YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); + + YGNodeStyleSetWidth(child1_child0, 20); + + EXPECT_TRUE(YGNodeIsDirty(child1_child0)); + EXPECT_TRUE(YGNodeHasDirtyDescendants(child1)); + EXPECT_TRUE(YGNodeIsDirty(child1)); + EXPECT_FALSE(YGNodeIsDirty(root)); + EXPECT_TRUE(YGNodeHasDirtyDescendants(root)); + EXPECT_FALSE(YGNodeIsDirty(child0)); + EXPECT_FALSE(YGNodeHasDirtyDescendants(child0)); + +} + TEST(YogaTest, dirty_propagation) { const YGNodeRef root = YGNodeNew(); YGNodeStyleSetAlignItems(root, YGAlignFlexStart); diff --git a/yoga/Yoga.c b/yoga/Yoga.c index d1ce158e..a71a3f4c 100644 --- a/yoga/Yoga.c +++ b/yoga/Yoga.c @@ -123,6 +123,7 @@ typedef struct YGNode { void *context; bool isDirty; + bool hasDirtyDescendants; bool hasNewLayout; YGNodeType nodeType; @@ -162,6 +163,7 @@ static const YGNode gYGNodeDefaults = { .children = NULL, .hasNewLayout = true, .isDirty = false, + .hasDirtyDescendants = false, .nodeType = YGNodeTypeDefault, .resolvedDimensions = {[YGDimensionWidth] = &YGValueUndefined, [YGDimensionHeight] = &YGValueUndefined}, @@ -238,6 +240,8 @@ static YGConfig gYGConfigDefaults = { }; static void YGNodeMarkDirtyInternal(const YGNodeRef node); +static bool YGNodeIsLayoutBoundary(const YGNodeRef node); +static void YGNodeMarkHasDirtyDescendants(const YGNodeRef node); YGMalloc gYGMalloc = &malloc; YGCalloc gYGCalloc = &calloc; @@ -452,16 +456,33 @@ void YGConfigCopy(const YGConfigRef dest, const YGConfigRef src) { memcpy(dest, src, sizeof(YGConfig)); } +static void YGNodeMarkHasDirtyDescendants(const YGNodeRef node) { + if (node && !node->hasDirtyDescendants) { + node->hasDirtyDescendants = true; + YGNodeMarkHasDirtyDescendants(node->parent); + } +} + static void YGNodeMarkDirtyInternal(const YGNodeRef node) { if (!node->isDirty) { node->isDirty = true; node->layout.computedFlexBasis = YGUndefined; if (node->parent) { - YGNodeMarkDirtyInternal(node->parent); + if (YGNodeIsLayoutBoundary(node->parent)) { + node->parent->isDirty = true; // Because parent lays out the child + node->parent->layout.computedFlexBasis = YGUndefined; + YGNodeMarkHasDirtyDescendants(node->parent); + } else { + YGNodeMarkDirtyInternal(node->parent); + } } } } +static bool YGNodeIsLayoutBoundary(const YGNodeRef node) { + return YGNodeStyleGetPositionType(node) == YGPositionTypeAbsolute; +} + void YGNodeSetMeasureFunc(const YGNodeRef node, YGMeasureFunc measureFunc) { if (measureFunc == NULL) { node->measure = NULL; @@ -628,6 +649,10 @@ bool YGNodeIsDirty(const YGNodeRef node) { return node->isDirty; } +bool YGNodeHasDirtyDescendants(const YGNodeRef node) { + return node->hasDirtyDescendants; +} + void YGNodeCopyStyle(const YGNodeRef dstNode, const YGNodeRef srcNode) { if (memcmp(&dstNode->style, &srcNode->style, sizeof(YGStyle)) != 0) { memcpy(&dstNode->style, &srcNode->style, sizeof(YGStyle)); @@ -3406,7 +3431,6 @@ bool YGLayoutNodeInternal(const YGNodeRef node, const char *reason, const YGConfigRef config) { YGLayout *layout = &node->layout; - gDepth++; const bool needToVisitNode = @@ -3498,6 +3522,26 @@ bool YGLayoutNodeInternal(const YGNodeRef node, if (!needToVisitNode && cachedResults != NULL) { layout->measuredDimensions[YGDimensionWidth] = cachedResults->computedWidth; layout->measuredDimensions[YGDimensionHeight] = cachedResults->computedHeight; + if (YGNodeHasDirtyDescendants(node)) { + const uint32_t childCount = YGNodeListCount(node->children); + for (uint32_t i = 0; i < childCount; ++i) { + const YGNodeRef child = YGNodeListGet(node->children, i); + if (child->isDirty || YGNodeHasDirtyDescendants(child)) { + YGLayoutNodeInternal(child, + child->layout.cachedLayout.availableWidth, + child->layout.cachedLayout.availableHeight, + child->layout.direction, + child->layout.cachedLayout.widthMeasureMode, + child->layout.cachedLayout.heightMeasureMode, + cachedResults->computedWidth, + cachedResults->computedHeight, + performLayout, + "layout", + child->config + ); + } + } + } if (gPrintChanges && gPrintSkips) { printf("%s%d.{[skipped] ", YGSpacer(gDepth), gDepth); @@ -3585,6 +3629,7 @@ bool YGLayoutNodeInternal(const YGNodeRef node, node->layout.dimensions[YGDimensionHeight] = node->layout.measuredDimensions[YGDimensionHeight]; node->hasNewLayout = true; node->isDirty = false; + node->hasDirtyDescendants = false; } gDepth--; diff --git a/yoga/Yoga.h b/yoga/Yoga.h index ae954c51..33b714e8 100644 --- a/yoga/Yoga.h +++ b/yoga/Yoga.h @@ -101,6 +101,7 @@ WIN_EXPORT void YGNodeCalculateLayout(const YGNodeRef node, // marking manually. WIN_EXPORT void YGNodeMarkDirty(const YGNodeRef node); WIN_EXPORT bool YGNodeIsDirty(const YGNodeRef node); +WIN_EXPORT bool YGNodeHasDirtyDescendants(const YGNodeRef node); WIN_EXPORT void YGNodePrint(const YGNodeRef node, const YGPrintOptions options);