DaddyFrosty/yoga
1
0
Fork 0
Code Issues 96 Pull Requests 31 Actions Packages Projects Releases 33 Wiki Activity

Fix bug in canUseCachedMeasurement causing unneeded double measure

Browse Source 
Summary: canUseCachedMeasurement function was not handling CSSMeasureModeAtMost correctly so a bunch of measurements that could have been reused were not. When a previous measurement used AtMost and the new one as an AtMost with a smaller constraint but still bigger than the previous computed size it should be OK to re-use the previously computed size.

Reviewed By: gkassabli

Differential Revision: D4071621

fbshipit-source-id: 19d87d939051ddf8ee2e1c6e60efee22d173bbb9
This commit is contained in:
Emil Sjolander
2016-10-25 07:33:58 -07:00
committed by Facebook Github Bot
parent 4452c7be5c
commit 4d0e657653
2 changed files with 192 additions and 126 deletions
Download Patch File Download Diff File Expand all files Collapse all files

184
CSSLayout/CSSLayout.c
View File

@@ -230,6 +230,8 @@ void CSSNodeInit(const CSSNodeRef node) {
node->layout.measuredDimensions[CSSDimensionHeight] = CSSUndefined; node->layout.measuredDimensions[CSSDimensionHeight] = CSSUndefined;
node->layout.cachedLayout.widthMeasureMode = (CSSMeasureMode) -1; node->layout.cachedLayout.widthMeasureMode = (CSSMeasureMode) -1;
node->layout.cachedLayout.heightMeasureMode = (CSSMeasureMode) -1; node->layout.cachedLayout.heightMeasureMode = (CSSMeasureMode) -1;
node->layout.cachedLayout.computedWidth = -1;
node->layout.cachedLayout.computedHeight = -1;
} }
static void _CSSNodeMarkDirty(const CSSNodeRef node) { static void _CSSNodeMarkDirty(const CSSNodeRef node) {
@@ -2084,90 +2086,74 @@ static const char *getModeName(const CSSMeasureMode mode, const bool performLayo
return performLayout ? kLayoutModeNames[mode] : kMeasureModeNames[mode]; return performLayout ? kLayoutModeNames[mode] : kMeasureModeNames[mode];
} }
static bool canUseCachedMeasurement(const bool isTextNode, static inline bool newSizeIsExactAndMatchesOldMeasuredSize(CSSMeasureMode sizeMode,
const float availableWidth, float size,
const float availableHeight, float lastComputedSize) {
const float marginRow, return sizeMode == CSSMeasureModeExactly && eq(size, lastComputedSize);
const float marginColumn, }
const CSSMeasureMode widthMeasureMode,
const CSSMeasureMode heightMeasureMode,
CSSCachedMeasurement cachedLayout) {
const bool isHeightSame = (cachedLayout.heightMeasureMode == CSSMeasureModeUndefined &&
heightMeasureMode == CSSMeasureModeUndefined) ||
(cachedLayout.heightMeasureMode == heightMeasureMode &&
eq(cachedLayout.availableHeight, availableHeight));
const bool isWidthSame = (cachedLayout.widthMeasureMode == CSSMeasureModeUndefined && static inline bool oldSizeIsUnspecifiedAndStillFits(CSSMeasureMode sizeMode,
widthMeasureMode == CSSMeasureModeUndefined) || float size,
(cachedLayout.widthMeasureMode == widthMeasureMode && CSSMeasureMode lastSizeMode,
eq(cachedLayout.availableWidth, availableWidth)); float lastComputedSize) {
return sizeMode == CSSMeasureModeAtMost && lastSizeMode == CSSMeasureModeUndefined &&
size >= lastComputedSize;
}
if (isHeightSame && isWidthSame) { static inline bool newMeasureSizeIsStricterAndStillValid(CSSMeasureMode sizeMode,
return true; float size,
CSSMeasureMode lastSizeMode,
float lastSize,
float lastComputedSize) {
return lastSizeMode == CSSMeasureModeAtMost && sizeMode == CSSMeasureModeAtMost &&
lastSize > size && lastComputedSize <= size;
}
static bool CSSNodeCanUseCachedMeasurement(const bool isTextNode,
const CSSMeasureMode widthMode,
const float width,
const CSSMeasureMode heightMode,
const float height,
const CSSMeasureMode lastWidthMode,
const float lastWidth,
const CSSMeasureMode lastHeightMode,
const float lastHeight,
const float lastComputedWidth,
const float lastComputedHeight,
const float marginRow,
const float marginColumn) {
if (lastComputedHeight < 0 || lastComputedWidth < 0) {
return false;
} }
const bool isHeightValid = (cachedLayout.heightMeasureMode == CSSMeasureModeUndefined && const bool hasSameWidthSpec = lastWidthMode == widthMode && eq(lastWidth, width);
heightMeasureMode == CSSMeasureModeAtMost && const bool hasSameHeightSpec = lastHeightMode == heightMode && eq(lastHeight, height);
cachedLayout.computedHeight <= (availableHeight - marginColumn)) ||
(heightMeasureMode == CSSMeasureModeExactly &&
eq(cachedLayout.computedHeight, availableHeight - marginColumn));
if (isWidthSame && isHeightValid) { const bool widthIsCompatible =
return true; hasSameWidthSpec ||
} newSizeIsExactAndMatchesOldMeasuredSize(widthMode, width - marginRow, lastComputedWidth) ||
oldSizeIsUnspecifiedAndStillFits(widthMode,
width - marginRow,
lastWidthMode,
lastComputedWidth) ||
newMeasureSizeIsStricterAndStillValid(
widthMode, width - marginRow, lastWidthMode, lastWidth, lastComputedWidth);
const bool isWidthValid = (cachedLayout.widthMeasureMode == CSSMeasureModeUndefined && const bool heightIsCompatible = isTextNode || hasSameHeightSpec ||
widthMeasureMode == CSSMeasureModeAtMost && newSizeIsExactAndMatchesOldMeasuredSize(heightMode,
cachedLayout.computedWidth <= (availableWidth - marginRow)) || height - marginColumn,
(widthMeasureMode == CSSMeasureModeExactly && lastComputedHeight) ||
eq(cachedLayout.computedWidth, availableWidth - marginRow)); oldSizeIsUnspecifiedAndStillFits(heightMode,
height - marginColumn,
lastHeightMode,
lastComputedHeight) ||
newMeasureSizeIsStricterAndStillValid(heightMode,
height - marginColumn,
lastHeightMode,
lastHeight,
lastComputedHeight);
if (isHeightSame && isWidthValid) { return widthIsCompatible && heightIsCompatible;
return true;
}
if (isHeightValid && isWidthValid) {
return true;
}
// We know this to be text so we can apply some more specialized heuristics.
if (isTextNode) {
if (isWidthSame) {
if (heightMeasureMode == CSSMeasureModeUndefined) {
// Width is the same and height is not restricted. Re-use cahced value.
return true;
}
if (heightMeasureMode == CSSMeasureModeAtMost &&
cachedLayout.computedHeight < (availableHeight - marginColumn)) {
// Width is the same and height restriction is greater than the cached
// height. Re-use cached
// value.
return true;
}
// Width is the same but height restriction imposes smaller height than
// previously measured.
// Update the cached value to respect the new height restriction.
cachedLayout.computedHeight = availableHeight - marginColumn;
return true;
}
if (cachedLayout.widthMeasureMode == CSSMeasureModeUndefined) {
if (widthMeasureMode == CSSMeasureModeUndefined ||
(widthMeasureMode == CSSMeasureModeAtMost &&
cachedLayout.computedWidth <= (availableWidth - marginRow))) {
// Previsouly this text was measured with no width restriction, if width
// is now restricted
// but to a larger value than the previsouly measured width we can
// re-use the measurement
// as we know it will fit.
return true;
}
}
}
return false;
} }
// //
@@ -2199,6 +2185,8 @@ bool layoutNodeInternal(const CSSNodeRef node,
layout->nextCachedMeasurementsIndex = 0; layout->nextCachedMeasurementsIndex = 0;
layout->cachedLayout.widthMeasureMode = (CSSMeasureMode) -1; layout->cachedLayout.widthMeasureMode = (CSSMeasureMode) -1;
layout->cachedLayout.heightMeasureMode = (CSSMeasureMode) -1; layout->cachedLayout.heightMeasureMode = (CSSMeasureMode) -1;
layout->cachedLayout.computedWidth = -1;
layout->cachedLayout.computedHeight = -1;
} }
CSSCachedMeasurement *cachedResults = NULL; CSSCachedMeasurement *cachedResults = NULL;
@@ -2220,26 +2208,36 @@ bool layoutNodeInternal(const CSSNodeRef node,
const float marginAxisColumn = getMarginAxis(node, CSSFlexDirectionColumn); const float marginAxisColumn = getMarginAxis(node, CSSFlexDirectionColumn);
// First, try to use the layout cache. // First, try to use the layout cache.
if (canUseCachedMeasurement(node->isTextNode, if (CSSNodeCanUseCachedMeasurement(node->isTextNode,
availableWidth, widthMeasureMode,
availableHeight, availableWidth,
marginAxisRow, heightMeasureMode,
marginAxisColumn, availableHeight,
widthMeasureMode, layout->cachedLayout.widthMeasureMode,
heightMeasureMode, layout->cachedLayout.availableWidth,
layout->cachedLayout)) { layout->cachedLayout.heightMeasureMode,
layout->cachedLayout.availableHeight,
layout->cachedLayout.computedWidth,
layout->cachedLayout.computedHeight,
marginAxisRow,
marginAxisColumn)) {
cachedResults = &layout->cachedLayout; cachedResults = &layout->cachedLayout;
} else { } else {
// Try to use the measurement cache. // Try to use the measurement cache.
for (uint32_t i = 0; i < layout->nextCachedMeasurementsIndex; i++) { for (uint32_t i = 0; i < layout->nextCachedMeasurementsIndex; i++) {
if (canUseCachedMeasurement(node->isTextNode, if (CSSNodeCanUseCachedMeasurement(node->isTextNode,
availableWidth, widthMeasureMode,
availableHeight, availableWidth,
marginAxisRow, heightMeasureMode,
marginAxisColumn, availableHeight,
widthMeasureMode, layout->cachedMeasurements[i].widthMeasureMode,
heightMeasureMode, layout->cachedMeasurements[i].availableWidth,
layout->cachedMeasurements[i])) { layout->cachedMeasurements[i].heightMeasureMode,
layout->cachedMeasurements[i].availableHeight,
layout->cachedMeasurements[i].computedWidth,
layout->cachedMeasurements[i].computedHeight,
marginAxisRow,
marginAxisColumn)) {
cachedResults = &layout->cachedMeasurements[i]; cachedResults = &layout->cachedMeasurements[i];
break; break;
} }

134
tests/CSSLayoutMeasureCacheTest.cpp
View File

@@ -10,16 +10,31 @@
#include <CSSLayout/CSSLayout.h> #include <CSSLayout/CSSLayout.h>
#include <gtest/gtest.h> #include <gtest/gtest.h>
static CSSSize _measure(void *context, static CSSSize _measureMax(void *context,
float width, float width,
CSSMeasureMode widthMode, CSSMeasureMode widthMode,
float height, float height,
CSSMeasureMode heightMode) { CSSMeasureMode heightMode) {
int *measureCount = (int *)context; int *measureCount = (int *)context;
*measureCount = *measureCount + 1; *measureCount = *measureCount + 1;
return CSSSize { return CSSSize {
.width = widthMode == CSSMeasureModeUndefined ? 10 : width, .width = widthMode == CSSMeasureModeUndefined ? 10 : width,
.height = heightMode == CSSMeasureModeUndefined ? 10 : width, .height = heightMode == CSSMeasureModeUndefined ? 10 : height,
};
}
static CSSSize _measureMin(void *context,
float width,
CSSMeasureMode widthMode,
float height,
CSSMeasureMode heightMode) {
int *measureCount = (int *)context;
*measureCount = *measureCount + 1;
return CSSSize {
.width = widthMode == CSSMeasureModeUndefined || (widthMode == CSSMeasureModeAtMost && width > 10) ? 10 : width,
.height = heightMode == CSSMeasureModeUndefined || (heightMode == CSSMeasureModeAtMost && height > 10) ? 10 : height,
}; };
} }
@@ -31,19 +46,11 @@ TEST(CSSLayoutTest, measure_once_single_flexible_child) {
CSSNodeStyleSetHeight(root, 100); CSSNodeStyleSetHeight(root, 100);
const CSSNodeRef root_child0 = CSSNodeNew(); const CSSNodeRef root_child0 = CSSNodeNew();
CSSNodeStyleSetWidth(root_child0, 50);
CSSNodeInsertChild(root, root_child0, 0);
const CSSNodeRef root_child1 = CSSNodeNew();
int measureCount = 0; int measureCount = 0;
CSSNodeSetContext(root_child1, &measureCount); CSSNodeSetContext(root_child0, &measureCount);
CSSNodeSetMeasureFunc(root_child1, _measure); CSSNodeSetMeasureFunc(root_child0, _measureMax);
CSSNodeStyleSetFlexGrow(root_child1, 1); CSSNodeStyleSetFlexGrow(root_child0, 1);
CSSNodeInsertChild(root, root_child1, 1); CSSNodeInsertChild(root, root_child0, 0);
const CSSNodeRef root_child2 = CSSNodeNew();
CSSNodeStyleSetWidth(root_child2, 50);
CSSNodeInsertChild(root, root_child2, 2);
CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionLTR); CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionLTR);
@@ -52,31 +59,92 @@ TEST(CSSLayoutTest, measure_once_single_flexible_child) {
CSSNodeFreeRecursive(root); CSSNodeFreeRecursive(root);
} }
TEST(CSSLayoutTest, dont_remeasure_text_node_height_change) { TEST(CSSLayoutTest, remeasure_text_node_height_change) {
const CSSNodeRef root = CSSNodeNew(); const CSSNodeRef root = CSSNodeNew();
CSSNodeStyleSetAlignItems(root, CSSAlignFlexStart); CSSNodeStyleSetAlignItems(root, CSSAlignFlexStart);
CSSNodeStyleSetWidth(root, 100);
CSSNodeStyleSetHeight(root, 100);
const CSSNodeRef root_child0 = CSSNodeNew(); const CSSNodeRef root_child0 = CSSNodeNew();
CSSNodeStyleSetWidth(root_child0, 50); CSSNodeSetIsTextnode(root_child0, true);
CSSNodeStyleSetHeight(root_child0, 20); CSSNodeStyleSetFlexGrow(root_child0, 1);
int measureCount = 0;
CSSNodeSetContext(root_child0, &measureCount);
CSSNodeSetMeasureFunc(root_child0, _measureMax);
CSSNodeInsertChild(root, root_child0, 0); CSSNodeInsertChild(root, root_child0, 0);
const CSSNodeRef root_child1 = CSSNodeNew(); CSSNodeCalculateLayout(root, 100, 10, CSSDirectionLTR);
int measureCount = 0; CSSNodeCalculateLayout(root, 100, 20, CSSDirectionLTR);
CSSNodeSetContext(root_child1, &measureCount);
CSSNodeSetMeasureFunc(root_child1, _measure); ASSERT_EQ(1, measureCount);
CSSNodeSetIsTextnode(root_child1, true);
CSSNodeStyleSetFlexGrow(root_child1, 1); CSSNodeFreeRecursive(root);
CSSNodeInsertChild(root, root_child1, 1); }
const CSSNodeRef root_child2 = CSSNodeNew(); TEST(CSSLayoutTest, remeasure_with_same_exact_width_larger_than_needed_height) {
CSSNodeStyleSetWidth(root_child2, 50); const CSSNodeRef root = CSSNodeNew();
CSSNodeStyleSetHeight(root_child2, 20);
CSSNodeInsertChild(root, root_child2, 2); const CSSNodeRef root_child0 = CSSNodeNew();
int measureCount = 0;
CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionLTR); CSSNodeSetContext(root_child0, &measureCount);
CSSNodeSetMeasureFunc(root_child0, _measureMin);
CSSNodeInsertChild(root, root_child0, 0);
CSSNodeCalculateLayout(root, 100, 100, CSSDirectionLTR);
CSSNodeCalculateLayout(root, 100, 50, CSSDirectionLTR);
ASSERT_EQ(1, measureCount);
CSSNodeFreeRecursive(root);
}
TEST(CSSLayoutTest, remeasure_with_same_atmost_width_larger_than_needed_height) {
const CSSNodeRef root = CSSNodeNew();
CSSNodeStyleSetAlignItems(root, CSSAlignFlexStart);
const CSSNodeRef root_child0 = CSSNodeNew();
int measureCount = 0;
CSSNodeSetContext(root_child0, &measureCount);
CSSNodeSetMeasureFunc(root_child0, _measureMin);
CSSNodeInsertChild(root, root_child0, 0);
CSSNodeCalculateLayout(root, 100, 100, CSSDirectionLTR);
CSSNodeCalculateLayout(root, 100, 50, CSSDirectionLTR);
ASSERT_EQ(1, measureCount);
CSSNodeFreeRecursive(root);
}
TEST(CSSLayoutTest, remeasure_with_computed_width_larger_than_needed_height) {
const CSSNodeRef root = CSSNodeNew();
CSSNodeStyleSetAlignItems(root, CSSAlignFlexStart);
const CSSNodeRef root_child0 = CSSNodeNew();
int measureCount = 0;
CSSNodeSetContext(root_child0, &measureCount);
CSSNodeSetMeasureFunc(root_child0, _measureMin);
CSSNodeInsertChild(root, root_child0, 0);
CSSNodeCalculateLayout(root, 100, 100, CSSDirectionLTR);
CSSNodeStyleSetAlignItems(root, CSSAlignStretch);
CSSNodeCalculateLayout(root, 10, 50, CSSDirectionLTR);
ASSERT_EQ(1, measureCount);
CSSNodeFreeRecursive(root);
}
TEST(CSSLayoutTest, remeasure_with_atmost_computed_width_undefined_height) {
const CSSNodeRef root = CSSNodeNew();
CSSNodeStyleSetAlignItems(root, CSSAlignFlexStart);
const CSSNodeRef root_child0 = CSSNodeNew();
int measureCount = 0;
CSSNodeSetContext(root_child0, &measureCount);
CSSNodeSetMeasureFunc(root_child0, _measureMin);
CSSNodeInsertChild(root, root_child0, 0);
CSSNodeCalculateLayout(root, 100, CSSUndefined, CSSDirectionLTR);
CSSNodeCalculateLayout(root, 10, CSSUndefined, CSSDirectionLTR);
ASSERT_EQ(1, measureCount); ASSERT_EQ(1, measureCount);