From 58d14ee557b8236f81a8ecc69c959923bda85da7 Mon Sep 17 00:00:00 2001 From: Pritesh Nandgaonkar Date: Tue, 31 Oct 2017 23:08:19 -0700 Subject: [PATCH] Reverting the dirty child optimization Summary: Reverting D6134754 Reviewed By: emilsjolander Differential Revision: D6203290 fbshipit-source-id: 8e42abb70e55f0fac90faaa21ecdbe0fbb76ce6b --- 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, 7 insertions(+), 101 deletions(-) diff --git a/java/com/facebook/yoga/YogaNode.java b/java/com/facebook/yoga/YogaNode.java index 91a0910a..815fd2c3 100644 --- a/java/com/facebook/yoga/YogaNode.java +++ b/java/com/facebook/yoga/YogaNode.java @@ -9,11 +9,13 @@ 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 { @@ -196,12 +198,6 @@ 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 cae5df64..df039883 100644 --- a/java/jni/YGJNI.cpp +++ b/java/jni/YGJNI.cpp @@ -249,10 +249,6 @@ 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); } @@ -457,7 +453,6 @@ 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 2c251046..953de26a 100644 --- a/tests/YGDirtyMarkingTest.cpp +++ b/tests/YGDirtyMarkingTest.cpp @@ -10,45 +10,6 @@ #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 a71a3f4c..d1ce158e 100644 --- a/yoga/Yoga.c +++ b/yoga/Yoga.c @@ -123,7 +123,6 @@ typedef struct YGNode { void *context; bool isDirty; - bool hasDirtyDescendants; bool hasNewLayout; YGNodeType nodeType; @@ -163,7 +162,6 @@ static const YGNode gYGNodeDefaults = { .children = NULL, .hasNewLayout = true, .isDirty = false, - .hasDirtyDescendants = false, .nodeType = YGNodeTypeDefault, .resolvedDimensions = {[YGDimensionWidth] = &YGValueUndefined, [YGDimensionHeight] = &YGValueUndefined}, @@ -240,8 +238,6 @@ 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; @@ -456,33 +452,16 @@ 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) { - 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); - } + 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; @@ -649,10 +628,6 @@ 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)); @@ -3431,6 +3406,7 @@ bool YGLayoutNodeInternal(const YGNodeRef node, const char *reason, const YGConfigRef config) { YGLayout *layout = &node->layout; + gDepth++; const bool needToVisitNode = @@ -3522,26 +3498,6 @@ 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); @@ -3629,7 +3585,6 @@ 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 33b714e8..ae954c51 100644 --- a/yoga/Yoga.h +++ b/yoga/Yoga.h @@ -101,7 +101,6 @@ 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);