Skip to content

Commit 502f3c7

Browse files
jingcFacebook Github Bot
jingc
authored and
Facebook Github Bot
committed
Reverted commit D4147298
Summary: If there is a single child which is flex grow and flex shrink then instead of measuring and then shrinking we can just set the flex basis to zero as we know the final result will be that the child take up all remaining space. Reviewed By: gkassabli Differential Revision: D4147298 fbshipit-source-id: 8416508a31e8d317bf59d956abdbe5760b55bb6d
1 parent 12ebaa5 commit 502f3c7

4 files changed

Lines changed: 38 additions & 88 deletions

File tree

‎CSSLayout/CSSLayout.c‎

Lines changed: 31 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,7 @@ static void _CSSNodeMarkDirty(const CSSNodeRef node) {
259259
}
260260

261261
void CSSNodeSetMeasureFunc(const CSSNodeRef node, CSSMeasureFunc measureFunc) {
262-
CSS_ASSERT(CSSNodeChildCount(node) == 0,
263-
"Cannot set measure function: Nodes with measure functions cannot have children.");
262+
CSS_ASSERT(CSSNodeChildCount(node) == 0, "Cannot set measure function: Nodes with measure functions cannot have children.");
264263
node->measure = measureFunc;
265264
}
266265

@@ -270,8 +269,7 @@ CSSMeasureFunc CSSNodeGetMeasureFunc(const CSSNodeRef node) {
270269

271270
void CSSNodeInsertChild(const CSSNodeRef node, const CSSNodeRef child, const uint32_t index) {
272271
CSS_ASSERT(child->parent == NULL, "Child already has a parent, it must be removed first.");
273-
CSS_ASSERT(node->measure == NULL,
274-
"Cannot add child: Nodes with measure functions cannot have children.");
272+
CSS_ASSERT(node->measure == NULL, "Cannot add child: Nodes with measure functions cannot have children.");
275273
CSSNodeListInsert(&node->children, child, index);
276274
child->parent = node;
277275
_CSSNodeMarkDirty(node);
@@ -1369,26 +1367,6 @@ static void layoutNodeImpl(const CSSNodeRef node,
13691367
const float availableInnerMainDim = isMainAxisRow ? availableInnerWidth : availableInnerHeight;
13701368
const float availableInnerCrossDim = isMainAxisRow ? availableInnerHeight : availableInnerWidth;
13711369

1372-
// If there is only one child with flexGrow + flexShrink it means we can set the
1373-
// computedFlexBasis to 0 instead of measuring and shrinking / flexing the child to exactly
1374-
// match the remaining space
1375-
CSSNodeRef singleFlexChild = NULL;
1376-
if ((isMainAxisRow && widthMeasureMode != CSSMeasureModeUndefined) ||
1377-
(!isMainAxisRow && heightMeasureMode != CSSMeasureModeUndefined)) {
1378-
for (uint32_t i = 0; i < childCount; i++) {
1379-
const CSSNodeRef child = CSSNodeGetChild(node, i);
1380-
if (singleFlexChild) {
1381-
if (isFlex(child)) {
1382-
// There is already a flexible child, abort.
1383-
singleFlexChild = NULL;
1384-
break;
1385-
}
1386-
} else if (CSSNodeStyleGetFlexGrow(child) > 0 && CSSNodeStyleGetFlexShrink(child) > 0) {
1387-
singleFlexChild = child;
1388-
}
1389-
}
1390-
}
1391-
13921370
// STEP 3: DETERMINE FLEX BASIS FOR EACH ITEM
13931371
for (uint32_t i = 0; i < childCount; i++) {
13941372
const CSSNodeRef child = CSSNodeListGet(node->children, i);
@@ -1413,17 +1391,13 @@ static void layoutNodeImpl(const CSSNodeRef node,
14131391
currentAbsoluteChild = child;
14141392
child->nextChild = NULL;
14151393
} else {
1416-
if (child == singleFlexChild) {
1417-
child->layout.computedFlexBasis = 0;
1418-
} else {
1419-
computeChildFlexBasis(node,
1420-
child,
1421-
availableInnerWidth,
1422-
widthMeasureMode,
1423-
availableInnerHeight,
1424-
heightMeasureMode,
1425-
direction);
1426-
}
1394+
computeChildFlexBasis(node,
1395+
child,
1396+
availableInnerWidth,
1397+
widthMeasureMode,
1398+
availableInnerHeight,
1399+
heightMeasureMode,
1400+
direction);
14271401
}
14281402
}
14291403

@@ -2159,17 +2133,17 @@ static inline bool newMeasureSizeIsStricterAndStillValid(CSSMeasureMode sizeMode
21592133
}
21602134

21612135
bool CSSNodeCanUseCachedMeasurement(const CSSMeasureMode widthMode,
2162-
const float width,
2163-
const CSSMeasureMode heightMode,
2164-
const float height,
2165-
const CSSMeasureMode lastWidthMode,
2166-
const float lastWidth,
2167-
const CSSMeasureMode lastHeightMode,
2168-
const float lastHeight,
2169-
const float lastComputedWidth,
2170-
const float lastComputedHeight,
2171-
const float marginRow,
2172-
const float marginColumn) {
2136+
const float width,
2137+
const CSSMeasureMode heightMode,
2138+
const float height,
2139+
const CSSMeasureMode lastWidthMode,
2140+
const float lastWidth,
2141+
const CSSMeasureMode lastHeightMode,
2142+
const float lastHeight,
2143+
const float lastComputedWidth,
2144+
const float lastComputedHeight,
2145+
const float marginRow,
2146+
const float marginColumn) {
21732147
if (lastComputedHeight < 0 || lastComputedWidth < 0) {
21742148
return false;
21752149
}
@@ -2187,16 +2161,19 @@ bool CSSNodeCanUseCachedMeasurement(const CSSMeasureMode widthMode,
21872161
newMeasureSizeIsStricterAndStillValid(
21882162
widthMode, width - marginRow, lastWidthMode, lastWidth, lastComputedWidth);
21892163

2190-
const bool heightIsCompatible =
2191-
hasSameHeightSpec || newSizeIsExactAndMatchesOldMeasuredSize(heightMode,
2164+
const bool heightIsCompatible = hasSameHeightSpec ||
2165+
newSizeIsExactAndMatchesOldMeasuredSize(heightMode,
2166+
height - marginColumn,
2167+
lastComputedHeight) ||
2168+
oldSizeIsUnspecifiedAndStillFits(heightMode,
21922169
height - marginColumn,
2170+
lastHeightMode,
21932171
lastComputedHeight) ||
2194-
oldSizeIsUnspecifiedAndStillFits(heightMode,
2195-
height - marginColumn,
2196-
lastHeightMode,
2197-
lastComputedHeight) ||
2198-
newMeasureSizeIsStricterAndStillValid(
2199-
heightMode, height - marginColumn, lastHeightMode, lastHeight, lastComputedHeight);
2172+
newMeasureSizeIsStricterAndStillValid(heightMode,
2173+
height - marginColumn,
2174+
lastHeightMode,
2175+
lastHeight,
2176+
lastComputedHeight);
22002177

22012178
return widthIsCompatible && heightIsCompatible;
22022179
}

‎java/jni/CSSJNI.cpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ static void _jniTransferLayoutOutputsRecursive(CSSNodeRef root) {
4040
}
4141

4242
static void _jniPrint(CSSNodeRef node) {
43-
auto obj = adopt_local(
44-
Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(node))));
43+
auto obj = adopt_local(Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(node))));
4544
cout << obj->toString() << endl;
4645
}
4746

@@ -50,11 +49,10 @@ static CSSSize _jniMeasureFunc(CSSNodeRef node,
5049
CSSMeasureMode widthMode,
5150
float height,
5251
CSSMeasureMode heightMode) {
53-
auto obj = adopt_local(
54-
Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(node))));
52+
auto obj = adopt_local(Environment::current()->NewLocalRef(reinterpret_cast<jweak>(CSSNodeGetContext(node))));
5553

5654
static auto measureFunc = findClassLocal("com/facebook/csslayout/CSSNode")
57-
->getMethod<jlong(jfloat, jint, jfloat, jint)>("measure");
55+
->getMethod<jlong(jfloat, jint, jfloat, jint)>("measure");
5856

5957
_jniTransferLayoutDirection(node, obj);
6058
const auto measureResult = measureFunc(obj, width, widthMode, height, heightMode);

‎tests/CSSLayoutMeasureCacheTest.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ static CSSSize _measureMax(CSSNodeRef node,
1717
CSSMeasureMode heightMode) {
1818

1919
int *measureCount = (int *)CSSNodeGetContext(node);
20-
(*measureCount)++;
21-
20+
*measureCount = *measureCount + 1;
2221
return CSSSize {
2322
.width = widthMode == CSSMeasureModeUndefined ? 10 : width,
2423
.height = heightMode == CSSMeasureModeUndefined ? 10 : height,

‎tests/CSSLayoutMeasureTest.cpp‎

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,42 +10,18 @@
1010
#include <CSSLayout/CSSLayout.h>
1111
#include <gtest/gtest.h>
1212

13+
#if GTEST_HAS_DEATH_TEST
1314
static CSSSize _measure(CSSNodeRef node,
1415
float width,
1516
CSSMeasureMode widthMode,
1617
float height,
1718
CSSMeasureMode heightMode) {
18-
int *measureCount = (int*) CSSNodeGetContext(node);
19-
if (measureCount) {
20-
(*measureCount)++;
21-
}
22-
2319
return CSSSize {
24-
.width = 10,
25-
.height = 10,
20+
.width = 0,
21+
.height = 0,
2622
};
2723
}
2824

29-
TEST(CSSLayoutTest, dont_measure_single_grow_shrink_child) {
30-
const CSSNodeRef root = CSSNodeNew();
31-
CSSNodeStyleSetWidth(root, 100);
32-
CSSNodeStyleSetHeight(root, 100);
33-
34-
int measureCount = 0;
35-
36-
const CSSNodeRef root_child0 = CSSNodeNew();
37-
CSSNodeSetContext(root_child0, &measureCount);
38-
CSSNodeSetMeasureFunc(root_child0, _measure);
39-
CSSNodeStyleSetFlexGrow(root_child0, 1);
40-
CSSNodeStyleSetFlexShrink(root_child0, 1);
41-
CSSNodeInsertChild(root, root_child0, 0);
42-
43-
CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionLTR);
44-
45-
ASSERT_EQ(0, measureCount);
46-
}
47-
48-
#if GTEST_HAS_DEATH_TEST
4925
TEST(CSSLayoutTest, cannot_add_child_to_node_with_measure_func) {
5026
const CSSNodeRef root = CSSNodeNew();
5127
CSSNodeSetMeasureFunc(root, _measure);

0 commit comments

Comments
 (0)