Skip to content

Commit

Permalink
make yoga threadsafe (facebook#852)
Browse files Browse the repository at this point in the history
Summary:
Continuing facebook/yoga#791
nokia6686 is a former member of our team, so we are trying to pick up what he left and carry out the pull request.
# Solution
Improved from previous solution with jpap's suggestions.
2. Passing ```gDepth``` and ```gCurrentGenerationCount``` (renamed to **_depth_** and **_generationCount_** respectively) between function calls that stem from ```YGNodeCalculateLayout```.
In ```YGNodeCalculateLayout```, pass ```depth``` as value 0, to indicate the root depth.
Pull Request resolved: facebook/yoga#852

Reviewed By: SidharthGuglani

Differential Revision: D15537450

Pulled By: davidaurelio

fbshipit-source-id: 338f51383591ba27702ebe759f6c47c2dede3530
  • Loading branch information
dattc2 authored and M-i-k-e-l committed Mar 10, 2020
1 parent f5c51c5 commit 0847155
Showing 1 changed file with 78 additions and 37 deletions.
115 changes: 78 additions & 37 deletions ReactCommon/yoga/yoga/Yoga.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,9 @@ bool YGLayoutNodeInternal(
const char* reason,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext);
void* const layoutContext,
const uint32_t depth,
const uint32_t generationCount);

#ifdef DEBUG
static void YGNodePrintInternal(
Expand Down Expand Up @@ -1197,7 +1199,9 @@ static void YGNodeComputeFlexBasisForChild(
const YGDirection direction,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
const uint32_t depth,
const uint32_t generationCount) {
const YGFlexDirection mainAxis =
YGResolveFlexDirection(node->getStyle().flexDirection(), direction);
const bool isMainAxisRow = YGFlexDirectionIsRow(mainAxis);
Expand All @@ -1220,8 +1224,7 @@ static void YGNodeComputeFlexBasisForChild(
if (child->getLayout().computedFlexBasis.isUndefined() ||
(YGConfigIsExperimentalFeatureEnabled(
child->getConfig(), YGExperimentalFeatureWebFlexBasis) &&
child->getLayout().computedFlexBasisGeneration !=
gCurrentGenerationCount)) {
child->getLayout().computedFlexBasisGeneration != generationCount)) {
const YGFloatOptional paddingAndBorder = YGFloatOptional(
YGNodePaddingAndBorderForAxis(child, mainAxis, ownerWidth));
child->setLayoutComputedFlexBasis(
Expand Down Expand Up @@ -1372,13 +1375,15 @@ static void YGNodeComputeFlexBasisForChild(
"measure",
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);

child->setLayoutComputedFlexBasis(YGFloatOptional(YGFloatMax(
child->getLayout().measuredDimensions[dim[mainAxis]],
YGNodePaddingAndBorderForAxis(child, mainAxis, ownerWidth))));
}
child->setLayoutComputedFlexBasisGeneration(gCurrentGenerationCount);
child->setLayoutComputedFlexBasisGeneration(generationCount);
}

static void YGNodeAbsoluteLayoutChild(
Expand All @@ -1390,7 +1395,9 @@ static void YGNodeAbsoluteLayoutChild(
const YGDirection direction,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
const uint32_t depth,
const uint32_t generationCount) {
const YGFlexDirection mainAxis =
YGResolveFlexDirection(node->getStyle().flexDirection(), direction);
const YGFlexDirection crossAxis = YGFlexDirectionCross(mainAxis, direction);
Expand Down Expand Up @@ -1496,7 +1503,9 @@ static void YGNodeAbsoluteLayoutChild(
"abs-measure",
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
childWidth = child->getLayout().measuredDimensions[YGDimensionWidth] +
child->getMarginForAxis(YGFlexDirectionRow, width).unwrap();
childHeight = child->getLayout().measuredDimensions[YGDimensionHeight] +
Expand All @@ -1516,7 +1525,9 @@ static void YGNodeAbsoluteLayoutChild(
"abs-layout",
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);

if (child->isTrailingPosDefined(mainAxis) &&
!child->isLeadingPositionDefined(mainAxis)) {
Expand Down Expand Up @@ -1844,7 +1855,9 @@ static float YGNodeComputeFlexBasisForChildren(
const YGConfigRef config,
bool performLayout,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
const uint32_t depth,
const uint32_t generationCount) {
float totalOuterFlexBasis = 0.0f;
YGNodeRef singleFlexChild = nullptr;
YGVector children = node->getChildren();
Expand Down Expand Up @@ -1895,7 +1908,7 @@ static float YGNodeComputeFlexBasisForChildren(
continue;
}
if (child == singleFlexChild) {
child->setLayoutComputedFlexBasisGeneration(gCurrentGenerationCount);
child->setLayoutComputedFlexBasisGeneration(generationCount);
child->setLayoutComputedFlexBasis(YGFloatOptional(0));
} else {
YGNodeComputeFlexBasisForChild(
Expand All @@ -1910,7 +1923,9 @@ static float YGNodeComputeFlexBasisForChildren(
direction,
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
}

totalOuterFlexBasis +=
Expand Down Expand Up @@ -2024,7 +2039,9 @@ static float YGDistributeFreeSpaceSecondPass(
const bool performLayout,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
const uint32_t depth,
const uint32_t generationCount) {
float childFlexBasis = 0;
float flexShrinkScaledFactor = 0;
float flexGrowFactor = 0;
Expand Down Expand Up @@ -2190,7 +2207,9 @@ static float YGDistributeFreeSpaceSecondPass(
"flex",
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
node->setLayoutHadOverflow(
node->getLayout().hadOverflow |
currentRelativeChild->getLayout().hadOverflow);
Expand Down Expand Up @@ -2321,7 +2340,9 @@ static void YGResolveFlexibleLength(
const bool performLayout,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
const uint32_t depth,
const uint32_t generationCount) {
const float originalFreeSpace = collectedFlexItemsValues.remainingFreeSpace;
// First pass: detect the flex items whose min/max constraints trigger
YGDistributeFreeSpaceFirstPass(
Expand All @@ -2347,7 +2368,9 @@ static void YGResolveFlexibleLength(
performLayout,
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);

collectedFlexItemsValues.remainingFreeSpace =
originalFreeSpace - distributedFreeSpace;
Expand Down Expand Up @@ -2647,7 +2670,9 @@ static void YGNodelayoutImpl(
const bool performLayout,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
const uint32_t depth,
const uint32_t generationCount) {
YGAssertWithNode(
node,
YGFloatIsUndefined(availableWidth)
Expand Down Expand Up @@ -2828,7 +2853,9 @@ static void YGNodelayoutImpl(
config,
performLayout,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);

const bool flexBasisOverflows = measureModeMainDim == YGMeasureModeUndefined
? false
Expand Down Expand Up @@ -2936,7 +2963,9 @@ static void YGNodelayoutImpl(
performLayout,
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
}

node->setLayoutHadOverflow(
Expand Down Expand Up @@ -3110,7 +3139,9 @@ static void YGNodelayoutImpl(
"stretch",
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
}
} else {
const float remainingCrossDim = containerCrossAxis -
Expand Down Expand Up @@ -3318,7 +3349,9 @@ static void YGNodelayoutImpl(
"multiline-stretch",
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
}
}
break;
Expand Down Expand Up @@ -3458,7 +3491,9 @@ static void YGNodelayoutImpl(
direction,
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
}

// STEP 11: SETTING TRAILING POSITIONS FOR CHILDREN
Expand Down Expand Up @@ -3486,7 +3521,6 @@ static void YGNodelayoutImpl(
}
}

uint32_t gDepth = 0;
bool gPrintChanges = false;
bool gPrintSkips = false;

Expand Down Expand Up @@ -3692,13 +3726,15 @@ bool YGLayoutNodeInternal(
const char* reason,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
uint32_t depth,
const uint32_t generationCount) {
YGLayout* layout = &node->getLayout();

gDepth++;
depth++;

const bool needToVisitNode =
(node->isDirty() && layout->generationCount != gCurrentGenerationCount) ||
(node->isDirty() && layout->generationCount != generationCount) ||
layout->lastOwnerDirection != ownerDirection;

if (needToVisitNode) {
Expand Down Expand Up @@ -3800,8 +3836,8 @@ bool YGLayoutNodeInternal(
YGLogLevelVerbose,
nullptr,
"%s%d.{[skipped] ",
YGSpacer(gDepth),
gDepth);
YGSpacer(depth),
depth);
node->print(layoutContext);
Log::log(
node,
Expand All @@ -3823,8 +3859,8 @@ bool YGLayoutNodeInternal(
YGLogLevelVerbose,
nullptr,
"%s%d.{%s",
YGSpacer(gDepth),
gDepth,
YGSpacer(depth),
depth,
needToVisitNode ? "*" : "");
node->print(layoutContext);
Log::log(
Expand All @@ -3851,16 +3887,18 @@ bool YGLayoutNodeInternal(
performLayout,
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);

if (gPrintChanges) {
Log::log(
node,
YGLogLevelVerbose,
nullptr,
"%s%d.}%s",
YGSpacer(gDepth),
gDepth,
YGSpacer(depth),
depth,
needToVisitNode ? "*" : "");
node->print(layoutContext);
Log::log(
Expand Down Expand Up @@ -3924,8 +3962,7 @@ bool YGLayoutNodeInternal(
node->setDirty(false);
}

gDepth--;
layout->generationCount = gCurrentGenerationCount;
layout->generationCount = generationCount;

#ifdef YG_ENABLE_EVENTS
LayoutType layoutType;
Expand Down Expand Up @@ -4110,7 +4147,9 @@ void YGNodeCalculateLayoutWithContext(
"initial",
node->getConfig(),
marker.data,
layoutContext)) {
layoutContext,
0, // tree root
gCurrentGenerationCount)) {
node->setPosition(
node->getLayout().direction, ownerWidth, ownerHeight, ownerWidth);
YGRoundToPixelGrid(node, node->getConfig()->pointScaleFactor, 0.0f, 0.0f);
Expand Down Expand Up @@ -4162,7 +4201,9 @@ void YGNodeCalculateLayoutWithContext(
"initial",
nodeWithoutLegacyFlag->getConfig(),
layoutMarkerData,
layoutContext)) {
layoutContext,
0, // tree root
gCurrentGenerationCount)) {
nodeWithoutLegacyFlag->setPosition(
nodeWithoutLegacyFlag->getLayout().direction,
ownerWidth,
Expand Down

0 comments on commit 0847155

Please sign in to comment.