Summary:
X-link: https://github.com/facebook/react-native/pull/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:https://github.com/facebook/yoga/issues/050
style C fill:https://github.com/facebook/yoga/issues/050
style D fill:https://github.com/facebook/yoga/issues/050
style H fill:https://github.com/facebook/yoga/issues/050
style I fill:https://github.com/facebook/yoga/issues/050
```
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 https://github.com/facebook/yoga/pull/1725
Changelog: [Internal]
Pull Request resolved: https://github.com/facebook/yoga/pull/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
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1715
X-link: https://github.com/facebook/react-native/pull/46799
Content box impl had a bug where we resolved padding % against the same reference length as the dimensions. Padding should always be against containing block's width. This is also true for width, but not for height, which should be against containing block's height.
This just pipes the width into our box sizing functions.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63787577
fbshipit-source-id: e512338770f25b66506cabab5a7cde8f04397ea0
Summary:
X-link: https://github.com/facebook/react-native/pull/46649
Pull Request resolved: https://github.com/facebook/yoga/pull/1705
To get the height and width we call a function currently named `getResolvedDimension`. This returns a `Style::Length`, which in most cases we "resolve" immediately by calling `resolve`. This is a bit confusing that you need to `resolve` something that is already `resolved`.
I plan on adding a new function soon for `contentBox` which would resolve the length for you, so I think this should be renamed.
Also deleted unused `getResolvedDimensions`
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63407730
fbshipit-source-id: e855c17d9c99817be308b7263fcb5d43559ede14
Summary:
X-link: https://github.com/facebook/react-native/pull/46216
Regarding [issue](https://github.com/facebook/react-native/issues/45817) with incorrect layout when `left` is set to `auto`. This PR introduces handling `auto` whenever inline or flex position is checked to be defined and it fixes above issue.
Changelog:
[General][Fixed] - Fix handling 'auto' checks in absolute layout
## Tests:
I have run the provided unit tests and everything passes.
Pull Request resolved: https://github.com/facebook/yoga/pull/1689
Reviewed By: cipolleschi
Differential Revision: D61737876
Pulled By: NickGerleman
fbshipit-source-id: 531199a91c5e122b930b49725ea567cbb1d592ce
Summary:
X-link: https://github.com/facebook/react-native/pull/44792
Pull Request resolved: https://github.com/facebook/yoga/pull/1663
Fixing https://github.com/facebook/yoga/issues/1658. We had a problem where if a child had a different flex direction than its parent, and it also set a position as a percent, it would look at the wrong axis to evaluate the percent. What was happening was we were passing in the container's mainAxis size and crossAxis size to use to evaluate the position size if it was a percent. However, we matched these sizes with the main/cross axis of the child - which is wrong if the flex direction is different.
I changed it so that the function just takes in ownerWidth and ownerHeight then calls isRow to determine which one to use for the main/cross axis position. This reduces the ambiguity quite a bit imo.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D58172416
fbshipit-source-id: eafd8069e03493fc56c41a76879d1ad9b7e9236d
Summary:
D57285584 was reverted because we have service code with a faulty measure function, and adding logging to Yoga when invalid measurements were received was enough to spike error rate to elevated levels and block release.
This is a reland of the below change, with a couple modifications:
1. We log warnings instead of errors, which from what I heard, shouldn't block release, but should still make signal
2. We only zero the dimension which was NaN, to preserve exact behavior
## Original
We've started seeing assertion failures in Yoga where a `NaN` value makes its way to an `availableHeight` constraint when measuring Litho tree.
Because it's only happening on Litho, I have some suspicion this might be originating from a Litho-specific measure function. This adds sanitization in Yoga to measure function results, where we will log an error, and set size to zero, if either dimension ends up being negative of `NaN`.
This doesn't really help track down where the error was happening, but Yoga doesn't have great context to show this to begin with. If we see this is issue, next steps would be Litho internal intrumentation to find culprit.
Changelog: [Internal]
Reviewed By: sbuggay
Differential Revision: D57473295
fbshipit-source-id: 979f1b9a51f5550a8d3ca534276ec191a3cb7b9e
Summary:
X-link: https://github.com/facebook/react-native/pull/44557
We've started seeing assertion failures in Yoga where a `NaN` value makes its way to an `availableHeight` constraint when measuring Litho tree.
Because it's only happening on Litho, I have some suspicion this might be originating from a Litho-specific measure function. This adds sanitization in Yoga to measure function results, where we will log an error, and set size to zero, if either dimension ends up being negative of `NaN`.
This doesn't really help track down where the error was happening, but Yoga doesn't have great context to show this to begin with. If we see this is issue, next steps would be Litho internal intrumentation to find culprit.
Changelog: [Internal]
Reviewed By: joevilches
Differential Revision: D57285584
fbshipit-source-id: 935fcdd28c05bbac0d73e1c7654ae11a74898537
Summary:
X-link: https://github.com/facebook/litho/pull/976
Pull Request resolved: https://github.com/facebook/yoga/pull/1586
X-link: https://github.com/facebook/react-native/pull/43299
Add the React Clang Tidy config to Yoga, run the auto fixes, and make some manual mechanical tweaks.
Notably, the automatic changes to the infra for generating a Yoga tree from JSON capture make it 70% faster.
Before:
{F1463947076}
After:
{F1463946802}
This also cleans up all the no-op shallow const parameters in headers.
{F1463943386}
Not all checks are available in all environments, but that is okay, as Clang Tidy will gracefully skip them.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D54461054
fbshipit-source-id: dbd2d9ce51afd3174d1f2c6d439fa7d08baff46f
Summary:
X-link: https://github.com/facebook/react-native/pull/42254
Pull Request resolved: https://github.com/facebook/yoga/pull/1550
This change aims to simplify how we resolve edges. This operation happens many, many times, and has gotten complex and slow when paired with StyleValuePool.
This starts reshaping so that `yoga::Style` can resolve a style prop for a given edge. This is closer to the ideal computed style API to avoid recalcing this so many times, but doesn't address that.
This relies on removing the errata related to row-reverse, and cleans up the removal started in the last change.
This has no measurable perf effect under CompactValue, but has a >10% uplift in perf when using StyleValueHandle, where we can trivially check if a handle points to a defined value without resolving it, but only within `yoga::Style` since we don't expose the handle outside of it.
More quantifiably, we go from 2.35 million StyleValuePool reads to 993k. The rest are checks on the handle.
Reviewed By: joevilches
Differential Revision: D52605596
fbshipit-source-id: 0b366963a899e376f99ce3d75cd5f14a25d60cec
Summary:
X-link: https://github.com/facebook/react-native/pull/42314
Pull Request resolved: https://github.com/facebook/yoga/pull/1555
The next diff moves a bunch of methods to `yoga::Style`. This renames the function to be a tad bit shorter, for more readable callsites. It also makes it more consistent with style property getters.
Changelog: [Internal]
Reviewed By: rozele
Differential Revision: D52803393
fbshipit-source-id: 557df34a9f0fb0ee42ad23b1fda99c1e0eb1d4e3
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1556
X-link: https://github.com/facebook/react-native/pull/42315
Since we aim to ship static to all users of yoga (not just XPR), we need to remove the errata that is gating most of the features. This should be a non breaking change. To ensure that, I added a new errata which, if on, will use the inner size of the containing node as the containing block. This is how it has been for a while and resolving this is risky and time consuming so for the time being we will stick with that.
Reviewed By: NickGerleman
Differential Revision: D52706161
fbshipit-source-id: 30a93f29cb0d97b20b2947eaa21f36cdc78c4961
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1553
X-link: https://github.com/facebook/react-native/pull/42274
Separate from `YGConfigSetPrintTreeFlag` we have a public API `YGNodeSetPrintFunc` which sets a function called, if you manually change a constant in source code during debugging.
This is not debug-only, is exposed as part of the public API (without a way to turn it on from the public API), and takes up a pointer per node doing nothing.
I'm not aware of anyone recently using the capability, and the tracing/event related work done since then would be more powerful for this anyway.
Remove the API.
Changelog: [Internal]
Reviewed By: rozele
Differential Revision: D52767445
fbshipit-source-id: f72927b47cffa4fe6fe886b42f07cc1ba55f141e
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1547
X-link: https://github.com/facebook/react-native/pull/42251
Yoga has an odd behavior, where `start`/`end` edges under row-reverse are relative to flex-direction, instead of writing direction.
While Yoga doesn't actually document what this behavior is supposed to be, it goes against CK documentation, historic RN documentation, and the behavior valid on the web. It is also applied inconsistently (e.g. sometimes only on container, sometimes on child). It really is a bug, instead of an intended behavior.
We changed the default behavior for Yoga, but left the existing one behind an errata (so existing fbsource users got old behavior). We have previously seen this behavior show up in product code, including CK when running on FlexLayout.
`row-reverse` is surprisingly uncommon though:
1. Litho has <40 usages
2. RN has ~40 usages in `RKJSModules`,~30 in `arvr/js`, ~6 in `xplat/archon`
3. CK has ~80 usages
4. NT has ~40 usages
There are few enough, mostly simple components, that we can inspect through each of them, looking for signs they will hit the issue (at the potential chance of missing some).
CK accounts for 10/14 usages that I could tell would trigger the issue, since it only exposes start/end edge, and not left/right. It might make sense to make it preserve behavior instead, to reduce risk a bit.
FlexLayout is now separately powering Bloks, which wasn't surveyed, so I didn't touch CK behavior under Bloks.
There could also be other usages in other frameworks/bespoke usages, and this has implications for OSS users. But based on our own usage, of many, many components, this seems rare.
Changelog:
[General][Breaking] - Make `start/end` in styles always refer to writing direction
Reviewed By: pentiumao, joevilches
Differential Revision: D52698130
fbshipit-source-id: 2a9ac47e177469f30dc988d916b6c0ad95d53461
Summary:
X-link: https://github.com/facebook/react-native/pull/41995
Pull Request resolved: https://github.com/facebook/yoga/pull/1526
This function has made quite the journey from something that originally made more sense. This renames, refactors, and adds documentation for what it actually does.
This should eventually make its way into `yoga::Style` once computed style is moved into that structure.
bypass-github-export-checks
Reviewed By: joevilches
Differential Revision: D52105718
fbshipit-source-id: 6492224dd2e10cef3c5fc6a139323ad189a0925c
Summary:
X-link: https://github.com/facebook/react-native/pull/41939
Pull Request resolved: https://github.com/facebook/yoga/pull/1520
This code originates as `YGValueResolve`, used to compute a YGValue to a length in points, using a reference for 100%.
This moves it to `Style::Length`, so we can encapsulate parts of it (for style value functions), and make the API more cohesive now that we can do C++ style OOP with it.
Changelog: [Internal]
Reviewed By: joevilches
Differential Revision: D51796973
fbshipit-source-id: a7c359c7544f4bd2066a80d976dde67a0d16f1dd
Summary:
X-link: https://github.com/facebook/react-native/pull/41776
Pull Request resolved: https://github.com/facebook/yoga/pull/1492
# Summary
In preparation to replace `CompactValue`, this fully encapsulates it as an implementation detail of `yoga::Style`.
The internal API now always operates on `Style::Length`, converted to `YGValue` at the public API boundary.
In the next step, we can plug in a new representation within `Style`, which should enable 64 bit values, and lower memory usage.
# Test Plan
1. Existing tests (inc for style, invalidation, CompactValue) pass
2. Check that constexpr `yoga::isinf()` produces same assembly under Clang as `std::isinf()`
3. Fabric Android builds
4. Yoga benchmark does style reads
# Performance
Checking whether a style is defined, then reading after, is a hot path, and we are doubling any space style lengths take in the stack (but not long-term on the node). After a naive move, on one system, the Yoga benchmark creating, laying out, and destroying a tree, ran about 8-10% slower in the "Huge nested flex" example. We are converting in many more cases instead of doing undefined check, but operating on accessed style values no longer needs to do the conversion multiple times.
I changed the `CompactValue` conversion to YGValue/StyleLength path to check for undefined as the common case (since we always convert, instead of calling `isUndefined` directly on CompactValue. That seemed to get the difference down to ~5-6% when I was playing with it then. We can optimistically make some of this up with ValuePool giving better locality, and fix this more holistically if we reduce edge and value resolution.
On another machine where I tested this, the new revision went the opposite direction, and was about 5% faster, so this isn't really a cut and dry regression, but we see different characteristics than before.
# Changelog
[Internal]
Reviewed By: rozele
Differential Revision: D51775346
fbshipit-source-id: c618af41b4882b4a227c917fcad07375806faf78
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1485
X-link: https://github.com/facebook/react-native/pull/41686
The size of the containing block is the size of the padding box of the containing node for absolute nodes. We were looking at `containingNode->getLayout().measuredDimension(Dimension::Width)` which is the border box. So we need to subtract the border from this.
Added a test that was failing before this change as well
Reviewed By: NickGerleman
Differential Revision: D51330526
fbshipit-source-id: adc448dfb71b54f1bbed0d9d61c5553bda4b106c
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1495
X-link: https://github.com/facebook/react-native/pull/41794
This is a copy of D51369722 to make it so that it preserves the file history
CalculateLayout.cpp is massive and approaching 3k lines. I added a few large functions dealing with layout of absolute nodes and was thinking it would be nice if that logic was just in its own file so it was more isolated and easier to reason about. So I made AbsoluteLayout.cpp and AbsoluteLayout.h to house this logic. In order for this to work I had to expose calculateLayoutInternal in CalculateLayout.h as layoutAbsoluteChild calls it. This is unideal and I would like to find a better way...
I also make LayoutUtils.h to house misc small helper methods as they are called in AbsoluteLayout.cpp and CalculateLayout.cpp
Reviewed By: NickGerleman
Differential Revision: D51824115
fbshipit-source-id: 9b27449e3c1516492c01e6167a6b2c4568a33807
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1479
X-link: https://github.com/facebook/react-native/pull/41682
There are two ways to get the value of a style for a specific edge right now:
1) From the inline start/end edge which is determined via the writing direction (ltr or rtl), assuming you do not have errata on
2) From the flex start/end edge which is determined via the flex direction (row, row-reverse, column, column-reverse)
There is a weird curiosity in the second case: you can define a style to be on the "start" or "end" edge when writing the stylex/css. The physical edge that this refers to is dependent on the writing direction. So `start` would be `left` in `ltr` and `right` in `rtl`, with `end` the opposite. It is **never** determined via the flex direction. Additionally, `start`/`end` takes precedence over the physical edge it corresponds to in the case both are defined.
So, all of this means that to actually get the value of a style from the flex start/end edges, we need to account for the case that one of these relative edges was defined and would overwrite any physical edge. Since this mapping is solely determined by the writing direction, we need to pass that in to all the flex start/end getters and do that logic. This is done in `flexStartRelativeEdge`/`flexEndRelativeEdge` which was added earlier but for some reason only being used on border.
Reviewed By: NickGerleman
Differential Revision: D51293315
fbshipit-source-id: 26fafff54827134e7c5b10354ff9bfdf67096f5b
Summary:
X-link: https://github.com/facebook/react-native/pull/41391
Pull Request resolved: https://github.com/facebook/yoga/pull/1461
Converts usages of `YGEdge` within internal APIs to `yoga::Edge` scoped enum.
With the exception of YGUnit which is in its own state of transition, this is the last public yoga enum to need to be moved to scoped enum form for usages internal to the Yoga public API.
Changelog: [internal]
Reviewed By: rshest
Differential Revision: D51152779
fbshipit-source-id: 06554f67bfd7709cbc24fdd9a5474e897e9e95d8
Summary:
X-link: https://github.com/facebook/react-native/pull/41392
Pull Request resolved: https://github.com/facebook/yoga/pull/1458
We're moving `CompactValue` to be an internal detail of `yoga::Style`, where users outside of the style will be dealing with a resolved/non-compact representation.
This change renames usages of `CompactValue` to `Style::Length`, which will be Yoga's representation for CSS input lengths. Right now one is just a type alias of the other, but this will let us change the internals of CompactValue with the rest of the world looking the same.
A few factory functions are added to `yoga::value` for creating CSS values. There are some shenanigans around how we want to represent CSS pixels (one YGUnitPoint), when we also end up adding CSS points (slightly larger than one YGUnitPoint). For now, I reused `point` until making other changes.
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D51000389
fbshipit-source-id: 00f55e72bfb8aa291b53308f8a62ac8797be490f
Summary:
X-link: https://github.com/facebook/react-native/pull/41347
Pull Request resolved: https://github.com/facebook/yoga/pull/1453
This follows the previous patterns used for `Gutters` and `Dimension`, where we hide CompactValue array implementation from `yoga::Style` callers.
This allows a single read of a style to only need access to the resolved values of a single edge, vs all edges. This is cheap now because the interface is the representation, but gets expensive if `StyleValuePool` is the actual implementation.
This prevents us from needing to resolve nine dimensions, in order to read a single value like `marginLeft`. Doing this, in the new style, also lets us remove `IdxRef` from the API.
We unroll the structure dependent parts in the props parsing code, for something more verbose, but also a bit clearer.
Changelog: [Internal]
Reviewed By: joevilches
Differential Revision: D50998164
fbshipit-source-id: 248396f9587e29d62cde05ae7512d8194f60c809
Summary:
X-link: https://github.com/facebook/react-native/pull/41305
Pull Request resolved: https://github.com/facebook/yoga/pull/1448
This should not be part of Yoga's API. If benchmarks want to do this, they still can (though I don't know the ones we have for it are super valuable).
Reviewed By: javache
Differential Revision: D50963933
fbshipit-source-id: 6482bd269928188b6469a358ffde5c4f9f5f9527
Summary:
X-link: https://github.com/facebook/react-native/pull/41293
Pull Request resolved: https://github.com/facebook/yoga/pull/1446
NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right.
Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here.
Reviewed By: NickGerleman
Differential Revision: D50945475
fbshipit-source-id: 290de06dcc04e8e644a3a32c127af12fdabb2f75
Summary:
X-link: https://github.com/facebook/react-native/pull/41209
Pull Request resolved: https://github.com/facebook/yoga/pull/1439
There are so many instances in this code base where we use the double negative of `!yoga::isUndefined(<something>)`. This is not as easy to read since because of this double negative imo. Additionally, sometimes we have really long chains like `!longVariableName.longFunctionName(longArgumentName).isUndefined()` and it is hard to see that this undefined is inverted.
This just replaces all instances of inverted `isUndefined()` with `isDefined()` so its easier to read.
Reviewed By: NickGerleman
Differential Revision: D50705523
fbshipit-source-id: edc7d3f2cbbae38ddaeb2030a419320caf73feff
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1431
X-link: https://github.com/facebook/react-native/pull/41041
The last of the row-reverse issues hurray!
The position insets were broken with row-reverse since we were using the main-start/main-end edges to inset from and NOT the inline-start/inline-end edges as we should. This made it so that inset in left and right were swapped and same with top and bottom (with column-reverse). The solution here is the same as the previous ones were we are migrating to using inline-start/end as the leading/trailing edge now.
Reviewed By: NickGerleman
Differential Revision: D50390543
fbshipit-source-id: b714deab8489fbe11f7f6db21e4aad3b3aa314b3
Summary:
X-link: https://github.com/facebook/react-native/pull/41023
Pull Request resolved: https://github.com/facebook/yoga/pull/1426
Just like D50140503 where marginStart and marginEnd were not working with row reverse, paddingStart and paddingEnd are not working either with row reverse either. The solution is similar - we were checking the flex item layout starting/ending edges and not the general layout starting/ending edges. This change makes it so that we look at the proper edge according to what direction is set.
One caveat is that in the case of padding (and also border) there is a callsite that actually wants to get the flex item layout's leading/trailing padding and not the one dictated by direction. So, I made a new function to accommodate this and just swapped that callsite out.
Reviewed By: NickGerleman
Differential Revision: D50348995
fbshipit-source-id: 85717df23de7cf5f66b38d3ff28435b053a4e68e
Summary:
X-link: https://github.com/facebook/react-native/pull/41022
Pull Request resolved: https://github.com/facebook/yoga/pull/1425
Just like D50140503 where marginStart and marginEnd were not working with row reverse, borderStart and borderEnd are not working either with row reverse either. The solution is similar - we were checking the flex item layout starting/ending edges and not the general layout starting/ending edges. This change makes it so that we look at the proper edge according to what direction is set.
One caveat is that in the case of border (and also padding) there is a callsite that actually wants to get the flex item layout's leading/trailing border and not the one dictated by direction. So, I made a new function to accommodate this and just swapped that callsite out.
Reviewed By: NickGerleman
Differential Revision: D50348085
fbshipit-source-id: eca2702c1753dbebb503034e2f0732684ad6c56e
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1423
X-link: https://github.com/facebook/react-native/pull/41017
Before resolving https://github.com/facebook/yoga/issues/1208 yoga was in a state where "leading" and "trailing" only referred to the main-start and main-end directions ([definition in spec](https://drafts.csswg.org/css-flexbox/#box-model)). That is, the start/end of the layout of flex items in a container. This is distinct from something like inline-start/inline-end which is the [start of text layout as defined by direction](https://drafts.csswg.org/css-writing-modes-3/#inline-start).
The bug linked above happened because "leading" and "trailing" functions are referring to the wrong directions in certain cases. So in order to fix this we added a new set of functions to get the "leading" and "trailing" edges according to what inline-start/inline-end would refer to - i.e. those defined by the direction (ltr | rtl). In this state I think it is confusing to understand which function refers to which direction and more specific names could help that.
This diff just renames the following 4 FlexDirection.h functions:
* **leadingEdge** -> **flexStartEdge**
* **trailingEdge** -> **flexEndEdge**
* **leadingLayoutEdge** -> **inlineStartEdge**
* **trailingLayoutEdge** -> **inlineEndEdge**
The spec calls the start/end directions as dictated by the flex-direction attribute "main-start" and "main-end" respectively, but mainStartEdge might be a bit confusing given it will be compared to a non-flexbox-specific name in inlineStartEdge. As a result I landed on flexStart/flexEnd similar to what values are used with alignment attributes (justify-content, align-content).
I chose to get rid of the "leading" and "trailing" descriptors to be more in line with what terminology the spec uses.
Next diff will be to rename the functions in Node.cpp to adhere to the above patterns.
Reviewed By: NickGerleman
Differential Revision: D50342254
fbshipit-source-id: 1e83a885876af9cf363822ebdbb64537f4784520
Summary:
X-link: https://github.com/facebook/litho/pull/962
X-link: https://github.com/facebook/react-native/pull/40804
Pull Request resolved: https://github.com/facebook/yoga/pull/1420
This stack is ultimately aiming to solve https://github.com/facebook/yoga/issues/1208
**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.
This means that if you do something like
```
export default function Playground(props: Props): React.Node {
return (
<View style={styles.container}>
<View style={styles.item} />
</View>
);
}
const styles = StyleSheet.create({
container: {
marginEnd: 100,
flexDirection: 'row-reverse',
backgroundColor: 'red',
display: 'flex',
width: 100,
height: 100,
},
item: {
backgroundColor: 'blue',
width: 10,
},
});
```
You get {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.
**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.
You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.
Reviewed By: NickGerleman
Differential Revision: D50140503
fbshipit-source-id: 5b580c7570f6ae1e2d031971926ac4e8f52dd362
Summary:
X-link: https://github.com/facebook/react-native/pull/39595
Pull Request resolved: https://github.com/facebook/yoga/pull/1404
These functions all ensure their returns are defined, but return FloatOptional anyway, making their callers have to deal with that possibility. Return `float` instead of `FloatOptional`, and do some additional cleanup.
Reviewed By: rshest
Differential Revision: D49531421
fbshipit-source-id: 95b21cade74e501dd54c7b6ca667c8c3859c5dae
Summary:
X-link: https://github.com/facebook/react-native/pull/39597
Pull Request resolved: https://github.com/facebook/yoga/pull/1406
Similar in vain to D49362819, we want to stop exposing pre-resolved CompactValue, and allow enum class usage without becoming annoying.
This also simplifies gap resolution a bit. I moved this to Style, to make it clear we aren't relying on any node state. I plan to do some similar cleanup for other resolution later.
Reviewed By: rshest
Differential Revision: D49530923
fbshipit-source-id: 47b06a7301fb283acc493dba159f496159d59580
Summary:
X-link: https://github.com/facebook/react-native/pull/39598
Pull Request resolved: https://github.com/facebook/yoga/pull/1403
Replaces all usages of YGDimension with Dimension.
Adds `yoga::to_underlying` to act like `std::to_underlying`, added in C++ 23.
This enum is oddly only used internally, and is never an input to the public API, but it handled as any other public generated enum. Potentially some more cleanup to do there.
Changelog: [Internal]
Reviewed By: rshest
Differential Revision: D49475409
fbshipit-source-id: 7d4c31e8a84485baea0dab50b5cf16b86769fa07
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1402
X-link: https://github.com/facebook/react-native/pull/39567
This change hides away most usages of YGDimension as an index. We do this for a couple reasons:
1. Right now the style interface may return a full array of resolved edge or dimension values, as a CompactValue. As we abstract away from CompactValue, and move towards ValuePool, this will no longer be the internal interface, and cheap to return. We instead change the interface to return a single value at once, which lets us resolve values lazily.
2. As we move internal usage to scoped enums, enums are not implicitly convertible to intergers (broadly a good thing). Hiding the enum as index prevents the need for callers to cast or convert to underlying.
Instead of making a new version of `IdxRef` for this, I converted to a more traditional setter. I will be making similar changes later for other styles, when I hide CompactValue from the public interface.
To review I would recommend filtering to changes in `xplat`, or viewing this in a single one of the OSS PRs exported. Everything apart from the below 20 files is a mirror.
{F1096792573}
Changelog: [Internal]
Reviewed By: javache
Differential Revision: D49362819
fbshipit-source-id: 30d730d78e62f36597d43f477120f65694e51ea3
Summary:
X-link: https://github.com/facebook/react-native/pull/39485
Pull Request resolved: https://github.com/facebook/yoga/pull/1393
These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.
Setting enum values is a bit jank right now, due to relying on C enums which are effectively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).
bypass-github-export-checks
Changelog:
[General][Breaking] - Require C++ 20 when including renderer headers
Reviewed By: sammy-SC
Differential Revision: D49265967
fbshipit-source-id: 6ab935a866196df06e742c821f3af88eb4d18e1a
