Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1781
This new image removed preinstalled libc++, and the ability to install old version of Clang Format we were installing.
This manually installs libc++ when setting up Clang jobs, and fully removes the clang-format validation job, since it wasn't correctly running before, and it's probably more a pain to ask people to run this than to just run Arcanist when importing a change.
Changelog: [Internal]
Reviewed By: joevilches
Differential Revision: D68455129
fbshipit-source-id: b5767be832b2b5d46db7e60e18b66823819ba15a
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1776
# Changelog:
[Internal] -
I was looking into some profiling results and noticed that Yoga has some quirky custom logic when it comes to rounding to the pixel grid, especially for the values that are halfway through.
There are already unit tests in place, but those cases weren't covered, so I am adding these extra corner cases here, to make sure they are covered in case that the actual rounding function may get modified (which it might make sense to, as it's currently can be more expensive than it could have been in some scenarios).
Reviewed By: christophpurrer
Differential Revision: D67712673
fbshipit-source-id: da1b7339a8939ced8d91f15441bc7d1f8af768c8
Summary:
X-link: https://github.com/facebook/litho/pull/1036
Pull Request resolved: https://github.com/facebook/yoga/pull/1775
X-link: https://github.com/facebook/react-native/pull/48404
## Changelog:
[Internal] -
This popped up when profiling some heavy UI performance, calling `fmod` operation in Yoga's `roundLayoutResultsToPixelGrid` in `PixelGrid.cpp` can be expensive, furthermore it turns out that some of the calls were redundant.
This replaces the duplicate calls to fmod with an equivalent single round operation, which for e.g. clang compiler on Windows brings the code in question from ~50 instructions (including 4 call instructions to the fmod function) down to ~30 instructions (without any external calls), and the layout operation being **~1% more efficient** for the particular benchmark I was looking into.
Reviewed By: christophpurrer
Differential Revision: D67689065
fbshipit-source-id: 2a074a1cb81bd7f7a3c414050b9ddda2ba90180f
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1768
Depending on the flex direction text will either be capped to the measured size or to the longest word, so I added that functionality
Reviewed By: mlord93
Differential Revision: D67106199
fbshipit-source-id: 0b4691768809004043a847f3fc5f7b94e92f1575
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1766
While we added support for 16K pages for yoga through React Native,
the standalone version we upload to Maven Central wasn't handled.
This fixes it.
Reviewed By: NickGerleman
Differential Revision: D66975933
fbshipit-source-id: 36a8404e2f2a60a44b9a39e478b23d1829bc542e
Summary:
X-link: https://github.com/facebook/react-native/pull/47973
Gap can be styled using both `points` and `percentages`, but YGNodeStyleGetGap currently returns a float value.
To maintain alignment with the `padding` and `margin` functionalities and allow it to be handled in bridging code, this function has been updated to return YGValue.
Pull Request resolved: https://github.com/facebook/yoga/pull/1753
Reviewed By: joevilches
Differential Revision: D66513236
Pulled By: NickGerleman
fbshipit-source-id: b7110855c037f20780f031f22a945bde4446687d
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1763
X-link: https://github.com/facebook/react-native/pull/48080
Small bug that I noticed while doing intrinsic sizing. We have the ownerHeight as the axis size despite bounding the length of the cross axis. This should therefore be the crossAxisOwnerSize, which might be the width in some cases
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D66736539
fbshipit-source-id: 528fc438b3327cd6f7890ea0ba408e4ce7b0f02c
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1762
X-link: https://github.com/facebook/react-native/pull/48077
OCD strikes again. Grepped this time to make sure we didn't miss any cases for this specific param name
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D66715777
fbshipit-source-id: 3e881a15b3b2836a4a55b11d7ec621541b92a05d
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1759
We just have block based tests right now. Intrinsic sizing is commonly used with text so lets add a few there.
Reviewed By: NickGerleman
Differential Revision: D66662940
fbshipit-source-id: f8b91419c89d22d79a91d3bd8c7da70429c827fb
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1755
X-link: https://github.com/facebook/react-native/pull/47975
I've been working with callsites here and its annoying if you switch these that you need to move these params around too. Let's just make them the same order
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D66519836
fbshipit-source-id: 2e98e671270a053c6e62372e2003f1ca67774ec9
Summary:
X-link: https://github.com/facebook/react-native/pull/47895
Pull Request resolved: https://github.com/facebook/yoga/pull/1750
These APIs were only added so that we could do TDD as we work on intrinsic sizing functionality. As of right now they do nothing. We are aiming on publishing a new version of Yoga soon so for the time being we are going to back these out so as not to confuse anyone with this new functionality. Ideally we get to a point where we have some temporary experimental header to stage these in but this is a bit time sensitive so just backing out for now
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D66332307
fbshipit-source-id: 1d596964e0c893091c541988506e8b80fa6d1957
Summary:
X-link: https://github.com/facebook/react-native/pull/47896
Pull Request resolved: https://github.com/facebook/yoga/pull/1752
These APIs were only added so that we could do TDD as we work on intrinsic sizing functionality. As of right now they do nothing. We are aiming on publishing a new version of Yoga soon so for the time being we are going to back these out so as not to confuse anyone with this new functionality. Ideally we get to a point where we have some temporary experimental header to stage these in but this is a bit time sensitive so just backing out for now
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D66332309
fbshipit-source-id: 793f77dad021fa5e57b52c36ae954307636bcbf0
Summary:
X-link: https://github.com/facebook/react-native/pull/47733
In https://github.com/facebook/yoga/pull/1729 I moved the cleanup of `display: contents` nodes to happen before all the early returns, but that change also made it be called **before** `cloneChildrenIfNeeded`. It actually needs to be called after `cloneChildrenIfNeeded` to make sure that children of `display: contents` nodes are properly owned.
It also needs to be called in every short-path, so it's being called in four places in this PR. Please let me know whether it's ok or not.
Pull Request resolved: https://github.com/facebook/yoga/pull/1743
Reviewed By: NickGerleman
Differential Revision: D65953902
Pulled By: zeyap
fbshipit-source-id: 0b18a5651f19c23564f5b3aa2a50833426e9ca5f
Summary:
X-link: https://github.com/facebook/react-native/pull/47732
This pull request addresses the issue where combining align-content and align-items properties resulted in incorrect layout behavior in Yoga version 3.1.0, as reported in [Issue https://github.com/facebook/yoga/issues/1739](https://github.com/facebook/yoga/issues/1739).
# Changes Made:
Alignment Logic Update: Modified the alignment calculations to ensure that the combination of align-content and align-items properties produces the expected layout, consistent with CSS Flexbox standards and previous Yoga versions.
Test Cases Added: Introduced new test cases to cover scenarios involving various combinations of align-content and align-items properties to prevent future regressions.
# Testing:
All existing tests pass successfully.
New test cases confirm that the layout behaves as expected when align-content and align-items are used together.
# Impact:
This fix ensures that layouts using both align-content and align-items properties render correctly, aligning with the behavior observed in Yoga version 1.19.0 and standard web browsers.
Pull Request resolved: https://github.com/facebook/yoga/pull/1742
Reviewed By: joevilches
Differential Revision: D65953882
Pulled By: zeyap
fbshipit-source-id: 7e12a21b1d442b35c3f3536cad32dc4b82130d15
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1746
Chrome made some changes for how overflowed row-reverse containers are laid out which was causing some issues on CI. I updated them here and skipped the new failing tests which we would want to followup on.
For LTR, the differences are seen below
|Before|After|
|--|
|{F1962694149} | {F1962694151}|
The extra space is now extending past the flex start edge vs flex end. RTL is the opposite. NickGerleman had deviated from the spec back in the day to match Chrome and it seems they made the adjustment recently. T208209388 is tracking the followup to align with the spec again. Basically, there is a notion of fallback alignment when certain justification/alignment values cannot actually apply. Right now we are falling back to flex start in all cases but we should fallback to start sometimes.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D66138361
fbshipit-source-id: c46d2e9b0cd297069b9cc544e3bded995e4867a6
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1736
X-link: https://github.com/facebook/react-native/pull/47358
`LayoutableChildren<yoga::Node>::Iterator` showed up to a surprising extent on a recent trace. Part of this was during pixel grid rounding, which does full tree traversal (we should fix that...), where the iterator is the first thing to read from the node.
I ran Yoga microbenchmark with Yoga compiled with `-O2`, where we saw a regression of synthetic performance by ~10%, but it turns out this build also had ASAN and some other heavy bits enabled, so the real impact was quite lower (~6%).
I was able to make some optimizations in the meantime against that, which still show some minor wins, reducing that overhead to ~4% in the properly optimized build (and a bit more before that). This is still measurable on the beefy server, and the code is a bit cleaner, so let's commit these!
Note that, in real scenarios, measure functions may dominate layout time, so display: contents does not mean end-to-end 4% regression, even after this change.
This change makes a few different optimizations
1. Removes redundant copies
2. Removes redundant index keeping
3. Mark which branches are likely vs unlikely
4. Shrink iterator size from 6 pointers to 3 pointers
5. Avoid usage in pixel grid rounding (so we don't need to have cache read for style)
In "Huge nested layout" example
| Before display: contents support | After display: contents support | After optimizations |
| 9.77ms | 10.39ms | 10.17ms |
Changelog: [Internal]
Reviewed By: rozele
Differential Revision: D65336148
fbshipit-source-id: 01c592771ed7accf2d87dddd5a3a9e0225098b56
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1723
Adds gentest support for intrinsic sizing keywords and creates an initial batch of somewhat interesting tests for these keywords on `width`, `height`, `min-width`, `min-height`, `max-width`, `max-height`, and `flex-basis`
Reviewed By: NickGerleman
Differential Revision: D64145117
fbshipit-source-id: 1e3e7214fab062ab6f260cfe7bdfaf3c0aca3bf7
Summary:
X-link: https://github.com/facebook/react-native/pull/46939
Pull Request resolved: https://github.com/facebook/yoga/pull/1722
tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D64002837
fbshipit-source-id: f15fae9fc0103175e1d85850fc9aa68579989fd3
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1721
X-link: https://github.com/facebook/react-native/pull/46938
The private internals of how we store styles needed to change a bit to support 3 new keyword values. Right now the only other keyword that can be stored is `auto`. As a result there isn't much fancy logic to support storing this and its just stored as a specific type inside of `StyleValueHandle`. There are only 3 bits for types (8 values), so it is not sustainable to just stuff every keyword in there. So the change writes the keyword as a value with a new `keyword` `Type`.
I chose not to put `auto` in there even though it is a keyword since it is a hot path, I did not want to regress perf when I did not need to.
I also make a new `StyleSizeValue` class to store size values - so values for `width`, `height`, etc. This way these new keywords are kept specific to sizes and we will not be able to create, for example, a margin: `max-content`.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63927512
fbshipit-source-id: 7285469d37ac4b05226183b56275c77f0c06996c
Summary:
Adds unit tests that directly cover the order in which `LayoutableChildren` iterator goes over the descendant nodes. The covered cases are as follows (nodes with `display: contents` are marked green):
### Single `display: contents` node
```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))
B --> D((D))
B --> E((E))
style B fill:https://github.com/facebook/yoga/issues/090
```
Correct order: `A, D, E, C`
### Multiple `display: contents` nodes
```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))
A --> D((D))
A --> E((E))
B --> F((F))
B --> G((G))
C --> H((H))
C --> I((I))
style A fill:https://github.com/facebook/yoga/issues/090
style B fill:https://github.com/facebook/yoga/issues/090
style C fill:https://github.com/facebook/yoga/issues/090
```
Correct order: `D, E, F, G, H, I`
### Nested `display: contents` nodes
```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))
B --> D((D))
B --> E((E))
E --> F((F))
E --> G((G))
style B fill:https://github.com/facebook/yoga/issues/090
style E fill:https://github.com/facebook/yoga/issues/090
```
Correct order: `A, D, F, G, C`
### Leaf `display: contents` node
```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))
style B fill:https://github.com/facebook/yoga/issues/090
```
Correct order: `A, C`
### Root `display: contents` node
```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))
style R fill:https://github.com/facebook/yoga/issues/090
```
Correct order: `A, B, C` - `LayoutableChildren` goes over the children with `display: contents` property, setting it on the root node should have no effect.
Changelog: [Internal]
Pull Request resolved: https://github.com/facebook/yoga/pull/1731
Reviewed By: joevilches
Differential Revision: D64981779
Pulled By: NickGerleman
fbshipit-source-id: ee39759c663a40f96ad313f1b775d53ab68fb442
Summary:
X-link: https://github.com/facebook/react-native/pull/47194
Fixes a case where a node with `display: contents` would not be cleaned up in some cases. This was caused by it being called after some early returns handling different quick paths. This PR moves the call to `cleanupContentsNodesRecursively` earlier so that it's always called.
The problem here wasn't mutating before cloning, but leaving a node marked as dirty after the layout has finished.
The exact case in which I found this was a node with a single `display: contents` child which needs to be a leaf. Then in the parent node [this](b0b842d5e7/yoga/algorithm/CalculateLayout.cpp (L1339)) condition is true, so `cleanupContentsNodesRecursively` doesn't get called and the child node is never visited and cleaned. I assume the same will happen in the other paths with an early return here.
Changelog:
[General][Fixed] - Fix for nodes with `display: contents` not being cleaned in some cases
Pull Request resolved: https://github.com/facebook/yoga/pull/1729
Reviewed By: rozele
Differential Revision: D64910099
Pulled By: NickGerleman
fbshipit-source-id: 6d56f8fbf687b7ee5af889c0b868406213c9cee8
Summary:
Bumping Gradle to 8.9 and NDK to 27.1.12297006 in prep to build native libraries with 16KB page size support
See:
- Changelog r27: https://github.com/android/ndk/wiki/Changelog-r27
- 16KB page sizes: https://developer.android.com/guide/practices/page-sizes
Changelog:
[Android][Updated] - Bump Gradle to 8.9, AGP to 8.7.1 and NDK to 27
Reviewed By: cortinico
Differential Revision: D64381441
fbshipit-source-id: 5e4236c166568a3990deabfd628f0f34e52ea855
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:
X-link: https://github.com/facebook/react-native/pull/46984
Pull Request resolved: https://github.com/facebook/yoga/pull/1725
The legacy (wrong) absolute positioning path positions in two places, including work that is definitely always overwritten in the new absolute layout path.
This came up before for position: static, but we didn't clean this up at the time. This code is also now leading display: contents impl being more annoying.
This diff tries to converge to the more spec correct implementation of positioning here, that also only happens in one place.
Previous path would potentially also incorrectly justify when `justify-content` was non-default, but not handled in the previous few cases? We don't have access to the flexLine at this point later, and apart from the existing tests now passing I reused the new correct logic for justification (spec says we should position child as if its the only child in the container https://www.w3.org/TR/css-flexbox-1/#abspos-items).
I added a new, more scoped errata `AbsolutePositionWithoutInsetsExcludesPadding` to preserve some of the legacy behavior that showed as very breaking.
I also did not try removing `AbsolutePercentAgainstInnerSize` which I suspect would be more breaking than this change.
Changelog:
[General][Breaking] - More spec compliant absolute positioning
Reviewed By: joevilches
Differential Revision: D64244949
fbshipit-source-id: ca97570e0de82e8f0424a0912adfd0b05254559e
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1719
Right now there is no way to test fixtures with `auto` widths, heights, or flex basis - even though we expose those functions. I updated gentest to generate those functions. Notably, position and margin (the other auto-able props) already account for this.
I also created `YGAutoTest.html` to test this. Not really testing the capabilities of `auto` here, just if we can create a test about it.
Reviewed By: NickGerleman
Differential Revision: D64125522
fbshipit-source-id: 291ec82003cf53f99c21943142a63e2ef30402a5
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1718
Our gentest works by console.logging the contents of the test it is generating to the browser powered by the driver. The driver then reads the logs and writes it to a file. An unfortunate side effect here is that we cannot console.log to debug how the gentest logic actually works since the driver is expecting formatted code. To get around this I had the driver filter out logs with a certain prefix and add that a helper that logs a message with this prefix to the scripts.
Reviewed By: NickGerleman
Differential Revision: D64011035
fbshipit-source-id: 1f113fde425d1d7db1c16ab85f4bb16b0e18f41d
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/46800
Pull Request resolved: https://github.com/facebook/yoga/pull/1716
Had a mini heart attack thinking I set the default to content box. Wrote this to double check and it passed. Might as well check it in
Technically the default to BoxSizing.h is ContentBox, but in the style we override that. Regardless I switched that around so border box was the default.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63802722
fbshipit-source-id: 49ed29657c964bc12a2bf70988061ab4599267ec
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1717
CocoaPods is EOL (though we will still support it for a while), and Yoga has SPM support, so let's add a badge!
SPM does not have a central registry (it instead uses our Git branch), so there isn't a badge to show the current vesion, we instead just mention that we support it, which is what I found other projects do.
Reviewed By: nmn
Differential Revision: D63845470
fbshipit-source-id: d033700fa04b8d708890b1c51d0a37fab8251a43
Summary:
X-link: https://github.com/facebook/react-native/pull/46741
Pull Request resolved: https://github.com/facebook/yoga/pull/1711
box sizing is really just a reinterpretation of what length properties (like `width`, `height`, `max-width`, etc) mean. So to implement this I just add the border and padding if we are in content box when we ask for any of these properties. All the math that gets done by the algorithm is still in border box land, and the layout we return is to be interpreted as the border box (this is actually the expected behavior per https://drafts.csswg.org/css-sizing/#box-sizing). This makes this implementation pretty simple actually.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63416833
fbshipit-source-id: fd76132cf51e8a5092129802c3a12ab24023018b
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:
Pull Request resolved: https://github.com/facebook/yoga/pull/1701
X-link: https://github.com/facebook/react-native/pull/46630
I would like to write some tests for box sizing that will drive a lot of my development as I implement content box. To do that, I need this publicly exposed. Obviously not that ideal since this currently does not do anything. Maybe we can name the value in such a way that its clear it is in development?
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D63135970
fbshipit-source-id: 7520823bf925364eae45341531e012e80ec92284
