From b16c22a8f38e77f969fffcc5901e004e341d8eca Mon Sep 17 00:00:00 2001 From: Emil Sjolander Date: Sun, 20 Nov 2016 04:59:55 -0800 Subject: [PATCH] Only skip updating computed flex basis within the same generation Summary: computed flex basis was incorrectly being reused in some circumstances between calls to caluclateLayout also run format script. Reviewed By: dshahidehpour Differential Revision: D4207106 fbshipit-source-id: fc1063ca79ecf75f6101aadded53bca96cb0585d --- CSSLayout/CSSLayout.c | 16 ++++++++++------ CSSLayout/CSSLayout.h | 5 +++-- java/jni/CSSJNI.cpp | 15 ++++++++++----- tests/CSSLayoutRelayoutTest.cpp | 27 +++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 13 deletions(-) create mode 100644 tests/CSSLayoutRelayoutTest.cpp diff --git a/CSSLayout/CSSLayout.c b/CSSLayout/CSSLayout.c index 81795b72..9fa990b4 100644 --- a/CSSLayout/CSSLayout.c +++ b/CSSLayout/CSSLayout.c @@ -49,6 +49,7 @@ typedef struct CSSLayout { float dimensions[2]; CSSDirection direction; + uint32_t computedFlexBasisGeneration; float computedFlexBasis; // Instead of recomputing the entire layout every single time, we @@ -967,7 +968,8 @@ static void computeChildFlexBasis(const CSSNodeRef node, if (!CSSValueIsUndefined(CSSNodeStyleGetFlexBasis(child)) && !CSSValueIsUndefined(isMainAxisRow ? width : height)) { - if (CSSValueIsUndefined(child->layout.computedFlexBasis)) { + if (CSSValueIsUndefined(child->layout.computedFlexBasis) || + child->layout.computedFlexBasisGeneration != gCurrentGenerationCount) { child->layout.computedFlexBasis = fmaxf(CSSNodeStyleGetFlexBasis(child), getPaddingAndBorderAxis(child, mainAxis)); } @@ -1052,6 +1054,8 @@ static void computeChildFlexBasis(const CSSNodeRef node, : child->layout.measuredDimensions[CSSDimensionHeight], getPaddingAndBorderAxis(child, mainAxis)); } + + child->layout.computedFlexBasisGeneration = gCurrentGenerationCount; } static void absoluteLayoutChild(const CSSNodeRef node, @@ -1477,6 +1481,7 @@ static void layoutNodeImpl(const CSSNodeRef node, child->nextChild = NULL; } else { if (child == singleFlexChild) { + child->layout.computedFlexBasisGeneration = gCurrentGenerationCount; child->layout.computedFlexBasis = 0; } else { computeChildFlexBasis(node, @@ -1813,8 +1818,8 @@ static void layoutNodeImpl(const CSSNodeRef node, if (!CSSValueIsUndefined(node->style.minDimensions[dim[mainAxis]]) && node->style.minDimensions[dim[mainAxis]] >= 0) { remainingFreeSpace = fmaxf(0, - node->style.minDimensions[dim[mainAxis]] - - (availableInnerMainDim - remainingFreeSpace)); + node->style.minDimensions[dim[mainAxis]] - + (availableInnerMainDim - remainingFreeSpace)); } else { remainingFreeSpace = 0; } @@ -2542,10 +2547,9 @@ void CSSLayoutSetMemoryFuncs(CSSMalloc cssMalloc, CSSCalloc cssCalloc, CSSRealloc cssRealloc, CSSFree cssFree) { - CSS_ASSERT(gNodeInstanceCount == 0, - "Cannot set memory functions: all node must be freed first"); + CSS_ASSERT(gNodeInstanceCount == 0, "Cannot set memory functions: all node must be freed first"); CSS_ASSERT((cssMalloc == NULL && cssCalloc == NULL && cssRealloc == NULL && cssFree == NULL) || - (cssMalloc != NULL && cssCalloc != NULL && cssRealloc != NULL && cssFree != NULL), + (cssMalloc != NULL && cssCalloc != NULL && cssRealloc != NULL && cssFree != NULL), "Cannot set memory functions: functions must be all NULL or Non-NULL"); if (cssMalloc == NULL || cssCalloc == NULL || cssRealloc == NULL || cssFree == NULL) { diff --git a/CSSLayout/CSSLayout.h b/CSSLayout/CSSLayout.h index 8e28e4f7..e052b07e 100644 --- a/CSSLayout/CSSLayout.h +++ b/CSSLayout/CSSLayout.h @@ -28,8 +28,8 @@ static const unsigned long __nan[2] = {0xffffffff, 0x7fffffff}; #define CSSUndefined NAN -#include "CSSMacros.h" #include "CSSEnums.h" +#include "CSSMacros.h" CSS_EXTERN_C_BEGIN @@ -160,7 +160,8 @@ CSS_NODE_LAYOUT_PROPERTY(CSSDirection, Direction); WIN_EXPORT void CSSLayoutSetLogger(CSSLogger logger); WIN_EXPORT void CSSLog(CSSLogLevel level, const char *message, ...); -WIN_EXPORT void CSSLayoutSetExperimentalFeatureEnabled(CSSExperimentalFeature feature, bool enabled); +WIN_EXPORT void CSSLayoutSetExperimentalFeatureEnabled(CSSExperimentalFeature feature, + bool enabled); WIN_EXPORT bool CSSLayoutIsExperimentalFeatureEnabled(CSSExperimentalFeature feature); WIN_EXPORT void CSSLayoutSetMemoryFuncs(CSSMalloc cssMalloc, diff --git a/java/jni/CSSJNI.cpp b/java/jni/CSSJNI.cpp index 9c3e3959..2c6ef3af 100644 --- a/java/jni/CSSJNI.cpp +++ b/java/jni/CSSJNI.cpp @@ -77,12 +77,15 @@ static int _jniLog(CSSLogLevel level, const char *format, va_list args) { char buffer[256]; int result = vsnprintf(buffer, sizeof(buffer), format, args); - static auto logFunc = - findClassLocal("com/facebook/csslayout/CSSLogger")->getMethod, jstring)>("log"); + static auto logFunc = findClassLocal("com/facebook/csslayout/CSSLogger") + ->getMethod, jstring)>("log"); - static auto logLevelFromInt = JCSSLogLevel::javaClassStatic()->getStaticMethod("fromInt"); + static auto logLevelFromInt = + JCSSLogLevel::javaClassStatic()->getStaticMethod("fromInt"); - logFunc(jLogger->get(), logLevelFromInt(JCSSLogLevel::javaClassStatic(), static_cast(level)), Environment::current()->NewStringUTF(buffer)); + logFunc(jLogger->get(), + logLevelFromInt(JCSSLogLevel::javaClassStatic(), static_cast(level)), + Environment::current()->NewStringUTF(buffer)); return result; } @@ -112,7 +115,9 @@ void jni_CSSLog(alias_ref clazz, jint level, jstring message) { Environment::current()->ReleaseStringUTFChars(message, nMessage); } -void jni_CSSLayoutSetExperimentalFeatureEnabled(alias_ref clazz, jint feature, jboolean enabled) { +void jni_CSSLayoutSetExperimentalFeatureEnabled(alias_ref clazz, + jint feature, + jboolean enabled) { CSSLayoutSetExperimentalFeatureEnabled(static_cast(feature), enabled); } diff --git a/tests/CSSLayoutRelayoutTest.cpp b/tests/CSSLayoutRelayoutTest.cpp new file mode 100644 index 00000000..e30965db --- /dev/null +++ b/tests/CSSLayoutRelayoutTest.cpp @@ -0,0 +1,27 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#include +#include + +TEST(CSSLayoutTest, dont_cache_computed_flex_basis_between_layouts) { + const CSSNodeRef root = CSSNodeNew(); + + const CSSNodeRef root_child0 = CSSNodeNew(); + CSSNodeStyleSetHeight(root_child0, 10); + CSSNodeStyleSetFlexBasis(root_child0, 20); + CSSNodeInsertChild(root, root_child0, 0); + + CSSNodeCalculateLayout(root, 100, CSSUndefined, CSSDirectionLTR); + CSSNodeCalculateLayout(root, 100, 100, CSSDirectionLTR); + + ASSERT_EQ(20, CSSNodeLayoutGetHeight(root_child0)); + + CSSNodeFreeRecursive(root); +}