Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1294
X-link: https://github.com/facebook/react-native/pull/37383
Add -Wextra to the build, and fixup some more instances of -Wunused-parameter that it sufaces which were not automatically fixable.
Reviewed By: javache
Differential Revision: D45772846
fbshipit-source-id: 29bf71006f63161521fe5869c3a7d8bf7aae9c81
Summary:
X-link: https://github.com/facebook/react-native/pull/37375
Adds internal helpers to YGConfig to make bit manipulation more readable. We also expose `hasErrata()` to YGNode beacuse checking that will be a common pattern. We intentionally don't add mutating functions to the node, since current model is to inval a node on commiting whole config.
This is not exposed via the C ABI.
Reviewed By: yungsters
Differential Revision: D45765971
fbshipit-source-id: eadaee4b9cf5204ac4984ecc52cc08650d144a30
Summary:
X-link: https://github.com/facebook/react-native/pull/37243
X-link: https://github.com/facebook/litho/pull/944
Pull Request resolved: https://github.com/facebook/yoga/pull/1279
Java bindings for Yoga rely solely on garbage collection for memory management. Each Java `YogaNode` has references to its children and parent Java Nodes. This means, for a node to be garbage collected, it cannot be reachable from any user accessible node. Each node then has single ownership of a `YGNodeRef`. When the `YogaNode` is garbage collected, a finalizer is run to call `YGNodeFree` and free the underlying native Yoga Node.
This may cause a use-after-free if finalizers are run from multiple threads. This is because `YGNodeFree` does more than just freeing, but instead also interacts with its parent and children nodes to detach itself, and remove any dangling pointers. If multiple threads run finalizers at once, one may traverse and try to mutate a node which another is freeing.
Because we know the entire connected tree is dead, there is no need to remove dangling pointers, so I want to expose a way to just free a Yoga Node, without it mutating the tree as a side effect.
This adds a currently private `YGNodeDeallocate` that frees without traversal. Ideally from naming this is what `YGNodeFree` would do, but we think changing the behavior of that might be too disruptive to OSS. At the same time there may be other memory safety related API changes we would like to eventually make, so this isn't made public beyond the JNI bindings to prevent needing to transition more APIs.
Changelog: [Internal]
Reviewed By: rshest
Differential Revision: D45556206
fbshipit-source-id: 62a1394c6f6bdc2b437b388098ea362a0fbcd0f7
Summary:
X-link: https://github.com/facebook/react-native/pull/37316
In Fabric, a ShadowNode may re-initialize YGConfig several times throughout the lifetime of a perpetually cloned Yoga Node.
RN sets `pointScaleFactor` lazily, when laying out the rootview. So right now it initializes a config to `pointScaleFactor` of 1.0, sets it, sets a new `pointScaleFactor` on the config, then repeats. This cycles the config between two `pointScaleFactor` values and will excessively dirty the node now that `YGNodeSetConfig` dirties on config change (D45505089)
This change makes it so that we retain previously used `pointScaleFactor` when cloning the Yoga nodes.
Changelog: [Internal]
Reviewed By: rozele
Differential Revision: D45669878
fbshipit-source-id: bfd2e185d9264a1cda64e59132960060385e16f1
Summary:
X-link: https://github.com/facebook/react-native/pull/37207
Pull Request resolved: https://github.com/facebook/yoga/pull/1274
Yoga exposes public APIs for dirtying Nodes, but will itself perform dirty marking when changing bits which invalidate layout. E.g. changing the style of a Node will invalidate it along with every parent Node.
Because config setting is newly public to the C ABI, this makes a similar change so that replacing a Node's config will dirty the tree above the node if there is a layout impacting config change (I don't think children need to be invalidated since child output shouldn't change given the same owner dimensions).
One quirk of this is that configs may be changed independently of the node. So someone could attach a config to a Node, then change the live config after the fact. The config does not currently have a back pointer to the Node, so we do not invalidate in that case of live config edits. The future work to rectify this would be to make configs immutable once created.
There are also currently some experimental features here which should maybe be compared, but these should be moved to YGErrata anyway.
Reviewed By: javache
Differential Revision: D45505089
fbshipit-source-id: 72b2b84ba758679af081d92e7403750c9cc53cb5
Summary:
X-link: https://github.com/facebook/react-native/pull/37179
Pull Request resolved: https://github.com/facebook/yoga/pull/1273
1. Simplify nan handling a bit
2. Remove string literal operators which seem dead enough to remove. These are public, so anyone could be using them, but it seems like almost nobody is.
1. FB has no usages of `using namespace facebook::yoga::literals` (exposing the literal operators) outside of Yoga tests.
1. There is only [a single usage](6dfba905ea/SDLTest/UIKit.hpp (L19)) on GitHub.
Reviewed By: yungsters
Differential Revision: D45419970
fbshipit-source-id: 8121303e5ae66596132a848a711802081728f4fb
Summary:
X-link: https://github.com/facebook/react-native/pull/37117
Pull Request resolved: https://github.com/facebook/yoga/pull/1265
This deprecates `YGConfigSetUseLegacyStretchBehaviour` and `YGConfigGetUseLegacyStretchBehaviour`and points users to errata APIs instead. Using the C API will fire deprecation warnings, which should create errors in builds with `-Werror`, though they can be ignored if truly needed (like we do with the language bindings which need to expose their own deprecated interface).
Reviewed By: rshest
Differential Revision: D45337198
fbshipit-source-id: 7f069623e38834171f5702382bbf47c37a556a22
Summary:
X-link: https://github.com/facebook/react-native/pull/37075
Pull Request resolved: https://github.com/facebook/yoga/pull/1255
This diff wires up YGErrata to a public API, along with existing functions to set UseLegacyStretchBehaviour.
The `UseLegacyStretchBehaviour` functions will be removed after the world internally is transitioned to `YGConfigSetErrata`. This is intentionally breaking, since most users previously enabling `UseLegacyStretchBehaviour` will want to pick a new appropriate errata setting. Internally, users of the API will be moved to`YGErrataAll`.
The overall change looks like:
1. Clean up YGConfig to use accessors/setters
2. Change up YGconfig internal storage
1. Fabric has a config per ShadowNode, so it makes sense to do some size optimization before adding more (free-form bools to bitfield, `std::array<bool,>` to `std::bitset` since not specialized)
3. Wire accessor/setter of UseLegacyStretchBehaviour to errata while both APIs exist
4. Add errata APIs to C ABI
After this we will need to expose the ABI to more language projections, and (more involved), add usages of the API to internal consumption of Yoga before adding more errata and removing `UseLegacyStretchBehaviour`.
Note that this API representation is similar, but distinct to `YGExperimentalFeature`. I think that API may also have made sense as an enum bitset, like we explicitly want for the new API, but it's not really worth changing the existing API to make that happen.
Reviewed By: rshest
Differential Revision: D45254097
fbshipit-source-id: 5c725ce5a77b25c1356f753d11c468587dbd8ded
Summary:
X-link: https://github.com/facebook/react-native/pull/37091
Pull Request resolved: https://github.com/facebook/yoga/pull/1258
This private constructor was added specifically for Fabric when config setting was deprecated, but that is undeprecated now. Fbsource fabric was moved off of it, and the RN desktop for was in the last diff in the stack, so we can remove it now.
Reviewed By: yungsters
Differential Revision: D45292729
fbshipit-source-id: 87b2a1adaafaf817befe44dbc3ac178af59a6e68
Summary:
X-link: https://github.com/facebook/react-native/pull/37115
Pull Request resolved: https://github.com/facebook/yoga/pull/1267
This is an existing bitset enum in the API. Use the facility added in the last diff to add flag operators to it, to avoid the need for casting, and to make it clearer in the generated YGEnums.h that it is a bitset.
Reviewed By: rshest
Differential Revision: D45341504
fbshipit-source-id: 0b80588f3e2e167d4c1c722c6d6608408dd617ba
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1256
X-link: https://github.com/facebook/react-native/pull/37076
This adds a `YGErrata` bitset enum matching the API and guarantees described in https://github.com/facebook/yoga/issues/1247.
It is hooked up in later diffs. There are a couple of `YGExperimentalFeature` values that belong here, but keeping the current options means that the default `YGErrataNone` corresponds to existing default behavior, letting us stage the series of changes as:
1. Implement errata API
2. Update internal Yoga users we want to de-risk to `YGErrataClassic` or `YGErrataAll` (if setting `UseLegacyStretchBehaviour`)
3. Add new errata, changing Yoga defaults to be conformant, while letting internal apps opt into compatibility modes pending experimentation.
I also added a macro to let C++ users of Yoga perform bitwise operations on the enum without casting (already available for C users).
Reviewed By: rshest
Differential Revision: D45254098
fbshipit-source-id: d4b61271a8018f548f2d9d8c953db4b121a502d1
Summary:
X-link: https://github.com/facebook/litho/pull/940
Pull Request resolved: https://github.com/facebook/yoga/pull/1252
X-link: https://github.com/facebook/react-native/pull/36993
Fabric relies on the private C++ internals of Yoga. This creates a conundrum in the open source build due to how header creation in Cocoapods works.
1. The default mechanism of specifying public headers needs to include the private headers for them to be made usable by fabric (by default)
2. Cocoapods will roll up all of the public headers when importing a module
https://github.com/facebook/react-native/pull/33381 fixed the Fabric Cocoapods build which ran into this. React Native relies on FlipperKit which relies on YogaKit, which in turn finally imports the Yoga podspec. Because YogaKit may use Swift, we can only expose the public Yoga C ABI.
The first solution in that PR was to allow RN to access Yoga private headers, but this was changed to instead make all Yoga headers public, and to add ifdefs to all of them to no-op when included outside of a C++ environment.
Talking to Kudo, we should be able to change back to the earlier approach in the PR, to instead expose the private headers to only RN. This lets us avoid exposing headers that we ideally wouldn't be, and lets us avoid the messy ifdefs in every Yoga header.
Changelog: [Internal]
Reviewed By: rshest
Differential Revision: D45139075
fbshipit-source-id: 99152986a578f7aac8324dffe0e18c42a38cc6a5
Summary:
Cleans up some of the changes to UseWebDefaults that were made in the interest of moving it outside of YGConfig. It still exists in YGConfig, but also exists on the node.
We also assert on null config, or when someone tries to change UseWebDefaults after creating a node (since right now YGStyle does not know the difference between unset vs set explicitly to what would normally be default).
Removes a peculiar constructor which was added to avoid config setting.
Reviewed By: rshest
Differential Revision: D45133644
fbshipit-source-id: 2b5e2baeb826653133df9b1175cf5c194e342e3e
Summary:
X-link: https://github.com/facebook/react-native/pull/36991
D15296732 added a byte to each YGNode exposed via private API, to stash random junk in. At the time, not adding to node size because of how fields ended up aligning. I'm not sure if this is still the case, but this is subject to change (e.g. adding a single extra flag).
There is a per-node "context" already that can store arbitrary data, and this reserved space isn't public, so this API is already a bit suspect.
The only place it is used is in instrumentation in fbandroid, enabled only in benchmarks, to store an enum to forward to QPL for what framework it thinks created the Yoga Node.
This is already broken for React Native (worked for Paper only), and afaict isn't used anywhere. But it also has little reason to be caching more information on the node (beyond maybe saving a couple memory accesses) since it derives this information from the node config already.
This removes the field.
Changelog:
[Internal]
Reviewed By: rshest
Differential Revision: D45137133
fbshipit-source-id: 75755b21102f7928b3ad947051c35b1a6566ef40
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1250
X-link: https://github.com/facebook/react-native/pull/36988
BitUtils functions in Yoga are like bit fields, with more steps, and more error prone (you need to work with explicit offsets which can be tricky for anything variable length). Replace usage with a bitfield struct. Eventually I'd like to remove the BitUtils functions in general.
Changelog: [Internal]
Reviewed By: rshest
Differential Revision: D45133645
fbshipit-source-id: aa1430df5e2fb71ed9d2a5f5b1a35429b71c7069
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1251
X-link: https://github.com/facebook/react-native/pull/36990
YGNode::setConfig was deprecated as part of D15416474 in an apparent goal to remove config pointers per-Node.
While I don't know the history of the motivation here, these config pointers were never removed, and we will be doubling down on per-node configs for StrictLayout, so we will want to undeprecate this.
This also exposes functions to the public C ABI, but I didn't spend the effort to create language projections for it.
Changelog: [Internal]
Reviewed By: rshest
Differential Revision: D45133646
fbshipit-source-id: 2bb15c4825717793529cdad8542447d11e723e35
Summary:
add print yoga node AllEdge value when debug mode, the old logic just print four edgeValue including top left bottom and right
but when we set the same value to these edge, we use YGAllEdge, this will be more convenient debug code
so I open this pull request to add it
Pull Request resolved: https://github.com/facebook/yoga/pull/948
Reviewed By: cortinico
Differential Revision: D43525328
Pulled By: NickGerleman
fbshipit-source-id: 59dde363e6ee8433d10ecf27e440ea7d54000bc1
Summary:
This removes the null-check on Yoga config added (we think we root-caused the issue), and adds an assertion to the public API accepting a config that it is non-null.
There are more changes to config setting that will come later.
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D43273456
fbshipit-source-id: cba498352d114a3fa2694f3a144f5f01a83d3190
Summary:
See code comment. D42282358 (7e96b65790) added usage of `YGConfigIsExperimentalFeatureEnabled` during layout, in a place where we sometimes encounter a Yoga node from RN which has an unexpectedly null config.
This is a hack to stop the bleed while we add logging to figure out where the null config is coming from in RN.
Changelog: [Internal]
Reviewed By: rozele
Differential Revision: D43203521
fbshipit-source-id: 2a21143a45c712ca00d16172f734fb116d165926
Summary:
This removes some unused flags which will cause Yoga to layout every tree twice, then diffing the tree, reporting whether the whole tree is different. This is too expensive to run outside of local experimentation, but we have more nuanced ways to implement the `YGNodeLayoutAffectedByQuirk` I am wanting to add.
Changelog: [Internal]
Reviewed By: lunaleaps
Differential Revision: D42406917
fbshipit-source-id: b415ed02768f6b59de3a6fa90c60c750d56fd4b0
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1217
This updates the CMake build present for being able to share options, fixing up flags, etc. A GTest build is added as well, along with a script and VSCode debug target so that OSS contributors can very easily run and debug tests on any OS.
Note that this isn't completely done (need to revise Windows, Mac, documentation), but should be finished enough otherwise for review.
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D42406686
fbshipit-source-id: 95e7ba5e4751c496a171785490e85cf0097fa839
Summary:
This is always enabled internally, so we should just turn it on everywhere.
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D42406509
fbshipit-source-id: c9cdd4fcf907d66cd276e0aec608a2e7db7ca5fb
Summary:
Fixes https://github.com/facebook/yoga/issues/850https://github.com/facebook/yoga/issues/850 describes a conformance issue where positioning of an absolute child using percentages is not calculated against the correct box size.
This takes the fix for that in https://github.com/facebook/yoga/pull/1028, regenerates tests, and fixes tests so that the experimental feature can be enabled. Goal is to run this as an experiment internally to see if we can enable by default.
Changelog:
[Internal]
Pull Request resolved: https://github.com/facebook/yoga/pull/1201
Reviewed By: yungsters
Differential Revision: D42282358
Pulled By: NickGerleman
fbshipit-source-id: 57c0dd9b0f1c47cb9335ff6e13d44b4646e5fa58
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1195
These files are remnants of experimentation functions from D16687367 (dcfdb955b3). They do not seem to be included anywhere anymore, and we already have a YGExperimentalFeature setter in config we can use in the more common case.
Changelog:
[Internal]
Reviewed By: christophpurrer
Differential Revision: D42241980
fbshipit-source-id: 482984b9a619dba8042e9166aee02a5e75e761ee
Summary:
Fixes - https://github.com/facebook/react-native/issues/35553
## Approach
We're using `betweenMainDim` to add [gap between](bbeede82d3/yoga/Yoga.cpp (L2495)) items in main axis. This is resulting in increased [main axis](bbeede82d3/yoga/Yoga.cpp (L2598)) dimension of the container as it gets added even for the last element. One solution is to keep using it and subtract the gap when last element is reached.
## Aside
Mutating this value feels weird, but I think `betweenMainDim` gets initialized for every line so should be fine? I did some manual tests to verify. I tried running tests but I'll have to downgrade the java version. Let me know if anything fails. Thanks! 🙏
Pull Request resolved: https://github.com/facebook/yoga/pull/1188
Test Plan: Added fixtures which previously failed but now pass.
Reviewed By: necolas
Differential Revision: D42078162
Pulled By: NickGerleman
fbshipit-source-id: 0e535618350422e001141a8786a83fc81651afe9
Summary:
Yoga tries to use `std::bit_cast<>()` where it's available and falls back to
`std::memcpy()` everywhere else. Unfortunately, the feature-test macro
(`__cpp_lib_bit_cast`) is only defined if the feature is available *and you have
already included either `<version>` or `<bit>`* (or something else that includes
one of those). Since `CompactValue.h` checks `__cpp_lib_bit_cast` *first*, it's
not defined even if it *would be* defined, leading the header not to
`#include <bit>`, leaving `std::bit_cast<>()` undefined; later, other headers
from the STL are included, leading to `__cpp_lib_bit_cast` *becoming* defined
and causing later code to choose to use the undefind `std::bit_cast<>()`.
This diff fixes the problem by `#include`ing either `<version>` or `<ciso646>`
(depending on availability) before checking any feature-test macros.
Changelog: [Internal]
Reviewed By: smeenai
Differential Revision: D41641205
fbshipit-source-id: 7d7bc5791c902a45302d3707e6cbf21fc0493f0c
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1176
`Values` is a wrapper to story an array of YGValue's as CompactValues.
From https://github.com/facebook/yoga/issues/1174 we see a warning `Wdeprecated-copy` beacuse a user-defined copy constructor is not present, but a user-defined asignment operator is (the defaulted one). This adds an explicitly defaulted copy contructor which should silence the warning I think.
Changelog:
[Internal]
Reviewed By: christophpurrer
Differential Revision: D41447490
fbshipit-source-id: 8cc8f291cf12014d87cc71c4598bb84fdd6cd930
Summary:
Fix linter warning when pulling in some code into AR
Changelog: [Internal]
Reviewed By: NickGerleman, mdvacca
Differential Revision: D41269423
fbshipit-source-id: 4305d6c362a51e62b19b4d3590fb0823073dff9a
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1173
In https://github.com/facebook/react-native/issues/35351 we see incorrect child item height when the flex-wrap is enabled, the cross-axis is to be stretched, and main-axis overflow is caused by gap.
In YGDistributeFreeSpaceSecondPass, if we do not have overflow (determined by flexBasisOverflows), we have stretch cross-alignment, and we reason that nothing can add to main axis dimensions, we know we're a single line and want to take full cross dimensions. and can set YGMeasureModeExactly which uses parent dimensions. Guessing an optimization?
If we do have overflow, then we set YGMeasureModeAtMost to find minimum possible cross-axis dimensions instead.
`flexBasisOverflows` incorporates both computed flex basis, and margin, so it is more generally a flag for whether we will wrap. So we should incorporate gap spacing into it. E.g. it is also used for whether we should the match main axis parent dimension of the overall container. This change does just that, and renames the flag to `mainAxisOverflows`.
We will want to cherry-pick the fix for this into RN 0.71 since we have not yet introduced the community to the incorrect behavior, and we expect a lot of usage of flex-gap.
Changelog:
[General][Fixed] - Fix incorrect height when gap causes main axis to overflow and cross-axis is stretched
Reviewed By: yungsters
Differential Revision: D41311424
fbshipit-source-id: bd0c3b5aac478a56878703b6da84fc3993cc14da
Summary:
This change starts adding more coverage to GitHub Actions. Existing workflows are split up to be per-platform, and stale scripts, etc are removed.
We are currently limited a bit by issues with the build itself, but this still adds a good bit of coverage that readily works, and adds places to inject more.
Another option would have been to move these to CircleCI where we have more credits, or used docker images instead of manual setup steps. etc, The Yoga build and number of changes is very light though, so we don't really need the complexity yet.
Some TODOs:
1. Fix the Apple Builds (pod lint and pod install return errors seen by the community)
2. Add working Android UTs
3. Add C++ UTs
4. Add Apple Publish
5. Add version stamping
Changelog:
[Internal][Added] - Start Adding Yoga GitHub Actions
Pull Request resolved: https://github.com/facebook/yoga/pull/1165
Reviewed By: cortinico
Differential Revision: D40386426
Pulled By: NickGerleman
fbshipit-source-id: c540dd25bfec6ac8c05e461c1236ef7fe6cb8598
Summary:
This extracts the core changes from https://github.com/facebook/yoga/pull/1116, to support gap/row-gap/column-gap, mostly identical, apart from the rename of gaps -> gutters.
The core functionality in this PR looks to be well tested from the fixtures added. I am not an expert in the internals of Yoga, but I am seeing everything that I would expect to. The space for the gap is accounted for in line-breaking, and the accumulated gaps limit the available line-length, before sizing flexible children, so items are sized correctly as to accommodate the gap. Then the gap is used for spacing during main axis and cross-axis justification.
Changelog:
[Genral][Added] - Implement gap/row-gap/column-gap (within the yoga C ABI)
Reviewed By: javache
Differential Revision: D39922410
fbshipit-source-id: 5850f22032169028bd8383b49dd240b335c11d3d
Summary:
When building and using C # libraries,
EntryPointNotFoundException thrown from YGInteropSetLogger.
so, I added YOGA_EXPORT on YGInteropSetLogger.
Pull Request resolved: https://github.com/facebook/yoga/pull/960
Reviewed By: yungsters
Differential Revision: D40027238
Pulled By: yungsters
fbshipit-source-id: 6af584a16e66a31c91374a1bb64434888762e3c8
Summary:
When I use libyogacore.so in other programming languages, it crash with message as
> Could not obtain symbol from the library: dlsym(0x20b84d220, YGConfigIsExperimentalFeatureEnabled): symbol not found
This function is defined as `WIN_EXPORT bool YGConfigIsExperimentalFeatureEnabled` in yoga.h, but is not defined using `YOGA_EXPORT` in yoga.cpp.
Pull Request resolved: https://github.com/facebook/yoga/pull/1127
Reviewed By: yungsters
Differential Revision: D40024450
Pulled By: yungsters
fbshipit-source-id: f6f01eadccb13d593c68300059e96f4b0bbc9fb6
Summary:
This adds the YGGutter enum, used to choose between row/column gap variants (row-gap, column-gap, gap).
This used later in changes from https://github.com/facebook/yoga/pull/1116, in the APIs which deal with setting gap on style on yoga node.
Note the original PR called this `YGGap`, but this ending up leading to a couple public method signatures that could appear ambiguous:
1. `SetGap(YGGap gap, float gapLength)`: Enums like `YGAlign` are the vaues for an `align` prop. `YGGap` controls the variant of the gap (like `YGEdge` does for left/right/top/bottom variants). So the enum reads as if it is the `gapValue`, and it looks like we have two of the same parameter.
2. `SetGap(YGGap gapDirection, float gap)`: This is misleading, because the direction gaps flow is the cross-axis of flex-direction.
3. `GetGap(YGGap gap)`: `gap` is the variant, but looks like an out param.
The [CSS Box Alignment](https://www.w3.org/TR/css-align-3/#column-row-gap) spec refers to these gaps as "Gutters", which removes the ambiguity.
Changelog:
[General][Added] - Add YGGutter Enum
Reviewed By: yungsters
Differential Revision: D39922412
fbshipit-source-id: 4b0baf800fecb3d03560a4267c7fb4c4330fd39e
Summary:
https://github.com/facebook/yoga/pull/1116 adds a new enum. The enum generator is out of date with copyright header, and some codemods, but it also looks like there were manual changes, types added, etc since generation. I fixed up the script to incorporate generating the changes folks made manually, and also added an enum that was previously only added manually to the C ABI.
Changelog:
[General][Fixed] - Fixup Yoga Enum Generator
Reviewed By: yungsters
Differential Revision: D39922252
fbshipit-source-id: b678fa9a43a896873d8c434745bdaf3f16fd991f