Skip to content

Commit

Permalink
Add support for display: contents style (facebook#47035)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#47035

This PR adds support for `display: contents` style by effectively skipping nodes with `display: contents` set during layout.

This required changes in the logic related to children traversal - before this PR a node would be always laid out in the context of its direct parent. After this PR that assumption is no longer true - `display: contents` allows nodes to be skipped, i.e.:

```html
<div id="node1">
  <div id="node2" style="display: contents;">
    <div id="node3" />
  </div>
</div>
```

`node3` will be laid out as if it were a child of `node1`.

Because of this, iterating over direct children of a node is no longer correct to achieve the correct layout. This PR introduces `LayoutableChildren::Iterator` which can traverse the subtree of a given node in a way that nodes with `display: contents` are replaced with their concrete children.

A tree like this:
```mermaid
flowchart TD
    A((A))
    B((B))
    C((C))
    D((D))
    E((E))
    F((F))
    G((G))
    H((H))
    I((I))
    J((J))

    A --> B
    A --> C
    B --> D
    B --> E
    C --> F
    D --> G
    F --> H
    G --> I
    H --> J

    style B fill:facebook/yoga#50
    style C fill:facebook/yoga#50
    style D fill:facebook/yoga#50
    style H fill:facebook/yoga#50
    style I fill:facebook/yoga#50
```

would be laid out as if the green nodes (ones with `display: contents`) did not exist. It also changes the logic where children were accessed by index to use the iterator instead as random access would be non-trivial to implement and it's not really necessary - the iteration was always sequential and indices were only used as boundaries.

There's one place where knowledge of layoutable children is required to calculate the gap. An optimization for this is for a node to keep a counter of how many `display: contents` nodes are its children. If there are none, a short path of just returning the size of the children vector can be taken, otherwise it needs to iterate over layoutable children and count them, since the structure may be complex.

One more major change this PR introduces is `cleanupContentsNodesRecursively`. Since nodes with `display: contents` would be entirely skipped during the layout pass, they would keep previous metrics, would be kept as dirty, and, in the case of nested `contents` nodes, would not be cloned, breaking `doesOwn` relation. All of this is handled in the new method which clones `contents` nodes recursively, sets empty layout, and marks them as clean and having a new layout so that it can be used on the React Native side.

Relies on facebook/yoga#1725

Changelog: [Internal]

X-link: facebook/yoga#1726

Test Plan: Added tests for `display: contents` based on existing tests for `display: none` and ensured that all the tests were passing.

Reviewed By: joevilches

Differential Revision: D64404340

Pulled By: NickGerleman

fbshipit-source-id: f6f6e9a6fad82873f18c8a0ead58aad897df5d09
  • Loading branch information
j-piasecki authored and facebook-github-bot committed Oct 19, 2024
1 parent f3e37e2 commit e5296cf
Show file tree
Hide file tree
Showing 18 changed files with 283 additions and 53 deletions.
3 changes: 3 additions & 0 deletions packages/react-native/React/Views/RCTLayout.m
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,8 @@ RCTDisplayType RCTReactDisplayTypeFromYogaDisplayType(YGDisplay displayType)
return RCTDisplayTypeFlex;
case YGDisplayNone:
return RCTDisplayTypeNone;
case YGDisplayContents:
RCTAssert(NO, @"YGDisplayContents cannot be converted to RCTDisplayType value.");
return RCTDisplayTypeNone;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@

public enum YogaDisplay {
FLEX(0),
NONE(1);
NONE(1),
CONTENTS(2);

private final int mIntValue;

Expand All @@ -27,6 +28,7 @@ public static YogaDisplay fromInt(int value) {
switch (value) {
case 0: return FLEX;
case 1: return NONE;
case 2: return CONTENTS;
default: throw new IllegalArgumentException("Unknown enum value: " + value);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,17 @@ static inline PositionType positionTypeFromYogaPositionType(
}
}

inline DisplayType displayTypeFromYGDisplay(YGDisplay display) {
switch (display) {
case YGDisplayNone:
return DisplayType::None;
case YGDisplayContents:
return DisplayType::Contents;
case YGDisplayFlex:
return DisplayType::Flex;
}
}

inline LayoutMetrics layoutMetricsFromYogaNode(yoga::Node& yogaNode) {
auto layoutMetrics = LayoutMetrics{};

Expand Down Expand Up @@ -146,9 +157,8 @@ inline LayoutMetrics layoutMetricsFromYogaNode(yoga::Node& yogaNode) {
layoutMetrics.borderWidth.bottom +
floatFromYogaFloat(YGNodeLayoutGetPadding(&yogaNode, YGEdgeBottom))};

layoutMetrics.displayType = yogaNode.style().display() == yoga::Display::None
? DisplayType::None
: DisplayType::Flex;
layoutMetrics.displayType =
displayTypeFromYGDisplay(YGNodeStyleGetDisplay(&yogaNode));

layoutMetrics.positionType =
positionTypeFromYogaPositionType(yogaNode.style().positionType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace facebook::react {
enum class DisplayType {
None = 0,
Flex = 1,
Inline = 2,
Contents = 2,
};

enum class PositionType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ inline int toInt(const DisplayType& displayType) {
return 0;
case DisplayType::Flex:
return 1;
case DisplayType::Inline:
case DisplayType::Contents:
return 2;
}
}
Expand All @@ -50,8 +50,8 @@ inline std::string toString(const DisplayType& displayType) {
return "none";
case DisplayType::Flex:
return "flex";
case DisplayType::Inline:
return "inline";
case DisplayType::Contents:
return "contents";
}
}

Expand Down
9 changes: 3 additions & 6 deletions packages/react-native/ReactCommon/react/renderer/dom/DOM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,7 @@ DOMSizeRounded getScrollSize(
*shadowNodeInCurrentRevision,
{.includeTransform = false});

if (layoutMetrics == EmptyLayoutMetrics ||
layoutMetrics.displayType == DisplayType::Inline) {
if (layoutMetrics == EmptyLayoutMetrics) {
return DOMSizeRounded{};
}

Expand Down Expand Up @@ -417,8 +416,7 @@ DOMSizeRounded getInnerSize(
*shadowNodeInCurrentRevision,
{.includeTransform = false});

if (layoutMetrics == EmptyLayoutMetrics ||
layoutMetrics.displayType == DisplayType::Inline) {
if (layoutMetrics == EmptyLayoutMetrics) {
return DOMSizeRounded{};
}

Expand All @@ -445,8 +443,7 @@ DOMBorderWidthRounded getBorderWidth(
*shadowNodeInCurrentRevision,
{.includeTransform = false});

if (layoutMetrics == EmptyLayoutMetrics ||
layoutMetrics.displayType == DisplayType::Inline) {
if (layoutMetrics == EmptyLayoutMetrics) {
return DOMBorderWidthRounded{};
}

Expand Down
2 changes: 2 additions & 0 deletions packages/react-native/ReactCommon/yoga/yoga/YGEnums.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ const char* YGDisplayToString(const YGDisplay value) {
return "flex";
case YGDisplayNone:
return "none";
case YGDisplayContents:
return "contents";
}
return "unknown";
}
Expand Down
3 changes: 2 additions & 1 deletion packages/react-native/ReactCommon/yoga/yoga/YGEnums.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ YG_ENUM_DECL(
YG_ENUM_DECL(
YGDisplay,
YGDisplayFlex,
YGDisplayNone)
YGDisplayNone,
YGDisplayContents)

YG_ENUM_DECL(
YGEdge,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ bool layoutAbsoluteDescendants(
float containingNodeAvailableInnerWidth,
float containingNodeAvailableInnerHeight) {
bool hasNewLayout = false;
for (auto child : currentNode->getChildren()) {
for (auto child : currentNode->getLayoutChildren()) {
if (child->style().display() == Display::None) {
continue;
} else if (child->style().positionType() == PositionType::Absolute) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ float calculateBaseline(const yoga::Node* node) {
}

yoga::Node* baselineChild = nullptr;
const size_t childCount = node->getChildCount();
for (size_t i = 0; i < childCount; i++) {
auto child = node->getChild(i);
for (auto child : node->getLayoutChildren()) {
if (child->getLineIndex() > 0) {
break;
}
Expand Down Expand Up @@ -67,9 +65,7 @@ bool isBaselineLayout(const yoga::Node* node) {
if (node->style().alignItems() == Align::Baseline) {
return true;
}
const auto childCount = node->getChildCount();
for (size_t i = 0; i < childCount; i++) {
auto child = node->getChild(i);
for (auto child : node->getLayoutChildren()) {
if (child->style().positionType() != PositionType::Absolute &&
child->style().alignSelf() == Align::Baseline) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,21 @@ static void zeroOutLayoutRecursively(yoga::Node* const node) {
}
}

static void cleanupContentsNodesRecursively(yoga::Node* const node) {
for (auto child : node->getChildren()) {
if (child->style().display() == Display::Contents) {
child->getLayout() = {};
child->setLayoutDimension(0, Dimension::Width);
child->setLayoutDimension(0, Dimension::Height);
child->setHasNewLayout(true);
child->setDirty(false);
child->cloneChildrenIfNeeded();

cleanupContentsNodesRecursively(child);
}
}
}

static float calculateAvailableInnerDimension(
const yoga::Node* const node,
const Direction direction,
Expand Down Expand Up @@ -525,7 +540,7 @@ static float computeFlexBasisForChildren(
const uint32_t generationCount) {
float totalOuterFlexBasis = 0.0f;
YGNodeRef singleFlexChild = nullptr;
const auto& children = node->getChildren();
const auto& children = node->getLayoutChildren();
SizingMode sizingModeMainDim =
isRow(mainAxis) ? widthSizingMode : heightSizingMode;
// If there is only one child with flexGrow + flexShrink it means we can set
Expand Down Expand Up @@ -1316,7 +1331,7 @@ static void calculateLayoutImpl(
return;
}

const auto childCount = node->getChildCount();
const auto childCount = node->getLayoutChildCount();
if (childCount == 0) {
measureNodeWithoutChildren(
node,
Expand Down Expand Up @@ -1351,6 +1366,9 @@ static void calculateLayoutImpl(
// Reset layout flags, as they could have changed.
node->setLayoutHadOverflow(false);

// Clean and update all display: contents nodes with a direct path to the
// current node as they will not be traversed
cleanupContentsNodesRecursively(node);
// STEP 1: CALCULATE VALUES FOR REMAINDER OF ALGORITHM
const FlexDirection mainAxis =
resolveDirection(node->style().flexDirection(), direction);
Expand Down Expand Up @@ -1436,9 +1454,9 @@ static void calculateLayoutImpl(
}
// STEP 4: COLLECT FLEX ITEMS INTO FLEX LINES

// Indexes of children that represent the first and last items in the line.
size_t startOfLineIndex = 0;
size_t endOfLineIndex = 0;
// Iterator representing the beginning of the current line
Node::LayoutableChildren::Iterator startOfLineIterator =
node->getLayoutChildren().begin();

// Number of lines.
size_t lineCount = 0;
Expand All @@ -1451,20 +1469,17 @@ static void calculateLayoutImpl(

// Max main dimension of all the lines.
float maxLineMainDim = 0;
for (; endOfLineIndex < childCount;
lineCount++, startOfLineIndex = endOfLineIndex) {
for (; startOfLineIterator != node->getLayoutChildren().end(); lineCount++) {
auto flexLine = calculateFlexLine(
node,
ownerDirection,
ownerWidth,
mainAxisOwnerSize,
availableInnerWidth,
availableInnerMainDim,
startOfLineIndex,
startOfLineIterator,
lineCount);

endOfLineIndex = flexLine.endOfLineIndex;

// If we don't need to measure the cross axis, we can skip the entire flex
// step.
const bool canSkipFlex =
Expand Down Expand Up @@ -1816,17 +1831,18 @@ static void calculateLayoutImpl(
case Align::Baseline:
break;
}
size_t endIndex = 0;
Node::LayoutableChildren::Iterator endIterator =
node->getLayoutChildren().begin();
for (size_t i = 0; i < lineCount; i++) {
const size_t startIndex = endIndex;
size_t ii = startIndex;
const Node::LayoutableChildren::Iterator startIterator = endIterator;
auto iterator = startIterator;

// compute the line's height and find the endIndex
float lineHeight = 0;
float maxAscentForCurrentLine = 0;
float maxDescentForCurrentLine = 0;
for (; ii < childCount; ii++) {
const auto child = node->getChild(ii);
for (; iterator != node->getLayoutChildren().end(); iterator++) {
const auto child = *iterator;
if (child->style().display() == Display::None) {
continue;
}
Expand Down Expand Up @@ -1859,11 +1875,11 @@ static void calculateLayoutImpl(
}
}
}
endIndex = ii;
endIterator = iterator;
currentLead += i != 0 ? crossAxisGap : 0;

for (ii = startIndex; ii < endIndex; ii++) {
const auto child = node->getChild(ii);
for (iterator = startIterator; iterator != endIterator; iterator++) {
const auto child = *iterator;
if (child->style().display() == Display::None) {
continue;
}
Expand Down Expand Up @@ -2066,8 +2082,7 @@ static void calculateLayoutImpl(
// As we only wrapped in normal direction yet, we need to reverse the
// positions on wrap-reverse.
if (performLayout && node->style().flexWrap() == Wrap::WrapReverse) {
for (size_t i = 0; i < childCount; i++) {
const auto child = node->getChild(i);
for (auto child : node->getLayoutChildren()) {
if (child->style().positionType() != PositionType::Absolute) {
child->setLayoutPosition(
node->getLayout().measuredDimension(dimension(crossAxis)) -
Expand All @@ -2084,8 +2099,7 @@ static void calculateLayoutImpl(
const bool needsCrossTrailingPos = needsTrailingPosition(crossAxis);

if (needsMainTrailingPos || needsCrossTrailingPos) {
for (size_t i = 0; i < childCount; i++) {
const auto child = node->getChild(i);
for (auto child : node->getLayoutChildren()) {
// Absolute children will be handled by their containing block since we
// cannot guarantee that their positions are set when their parents are
// done with layout.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@ FlexLine calculateFlexLine(
const float mainAxisownerSize,
const float availableInnerWidth,
const float availableInnerMainDim,
const size_t startOfLineIndex,
Node::LayoutableChildren::Iterator& iterator,
const size_t lineCount) {
std::vector<yoga::Node*> itemsInFlow;
itemsInFlow.reserve(node->getChildren().size());
itemsInFlow.reserve(node->getChildCount());

float sizeConsumed = 0.0f;
float totalFlexGrowFactors = 0.0f;
float totalFlexShrinkScaledFactors = 0.0f;
size_t numberOfAutoMargins = 0;
size_t endOfLineIndex = startOfLineIndex;
size_t firstElementInLineIndex = startOfLineIndex;
size_t endOfLineIndex = iterator.index();
size_t firstElementInLineIndex = iterator.index();

float sizeConsumedIncludingMinConstraint = 0;
const Direction direction = node->resolveDirection(ownerDirection);
Expand All @@ -41,8 +41,9 @@ FlexLine calculateFlexLine(
node->style().computeGapForAxis(mainAxis, availableInnerMainDim);

// Add items to the current line until it's full or we run out of items.
for (; endOfLineIndex < node->getChildren().size(); endOfLineIndex++) {
auto child = node->getChild(endOfLineIndex);
for (; iterator != node->getLayoutChildren().end();
iterator++, endOfLineIndex = iterator.index()) {
auto child = *iterator;
if (child->style().display() == Display::None ||
child->style().positionType() == PositionType::Absolute) {
if (firstElementInLineIndex == endOfLineIndex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ FlexLine calculateFlexLine(
float mainAxisownerSize,
float availableInnerWidth,
float availableInnerMainDim,
size_t startOfLineIndex,
Node::LayoutableChildren::Iterator& iterator,
size_t lineCount);

} // namespace facebook::yoga
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ void roundLayoutResultsToPixelGrid(
Dimension::Height);
}

for (yoga::Node* child : node->getChildren()) {
for (yoga::Node* child : node->getLayoutChildren()) {
roundLayoutResultsToPixelGrid(child, absoluteNodeLeft, absoluteNodeTop);
}
}
Expand Down
3 changes: 2 additions & 1 deletion packages/react-native/ReactCommon/yoga/yoga/enums/Display.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ namespace facebook::yoga {
enum class Display : uint8_t {
Flex = YGDisplayFlex,
None = YGDisplayNone,
Contents = YGDisplayContents,
};

template <>
constexpr int32_t ordinalCount<Display>() {
return 2;
return 3;
}

constexpr Display scopedEnum(YGDisplay unscoped) {
Expand Down
Loading

0 comments on commit e5296cf

Please sign in to comment.