602 Commits

Author SHA1 Message Date
Joe Vilches
7e91004b90 Remove skips on passing position tests and add errata tests
Summary: Now that the tests are passing let's not skip it anymore. Also adding errata tests to make sure most prod builds are still protected.

Reviewed By: NickGerleman

Differential Revision: D50390993

fbshipit-source-id: cb91a7a377e919eaca24fb25e3d73d3c92eb8931
2023-10-18 17:30:18 -07:00
Joe Vilches
2668e8e70c Fix row-reverse flex direction fixtures
Summary:
These tests were a bit weird for testing something with position. The gentest setup makes it so that the fixtures are wrapped in a absolutely positioned container with height and width bot 0. However, the generated yoga tests do NOT do this and instead have the root node as the fixture itself with no wrapping container.

This causes a problem when testing left/right/top/bottom position insets. Because left/right/top/bottom will position the element relative to its containing block when position is absolute, we will get different values on yoga and chrome even if the implementation is correct: https://developer.mozilla.org/en-US/docs/Web/CSS/right#description

To fix this, we just wrap the fixture in a set size div that is also absolutely positioned.

The file was also formatted.

Reviewed By: NickGerleman

Differential Revision: D50389229

fbshipit-source-id: ecd23939b973225cfb0611dc87f30c262952c5fc
2023-10-18 17:30:18 -07:00
Nico Burns
0d28b283e2 Support "align-content: space-evenly" (#1422)
Summary:
X-link: https://github.com/facebook/react-native/pull/41019

### Changes made
- Regenerated tests (as some aspect ratio tests seem to be out of date compared to the fixtures)
- Added SpaceEvenly variant to the "Align" enums (via enums.py)
- Implemented `align-content: space-evenly` alignment in CalculateLayout.cpp
- Added generated tests `align-content: space-evenly`
- Updated NumericBitfield test to account for the fact that the Align enum now requires more bits (this bit could do with being reviewed as I am not 100% certain that it's valid to just update the test like this).

### Changes not made
- Any attempt to improve the spec-compliance of content alignment in general (e.g. I think https://github.com/facebook/yoga/pull/1013 probably still needs to happen)

Pull Request resolved: https://github.com/facebook/yoga/pull/1422

Reviewed By: yungsters

Differential Revision: D50305438

Pulled By: NickGerleman

fbshipit-source-id: ef9f6f14220a0db066bc30db8dd690a4a82a0b00
2023-10-17 20:59:51 -07:00
Joe Vilches
2e2c124c28 Revamp tests for row reverse + padding after fix
Summary: no longer skip passing tests + add errata tests

Reviewed By: NickGerleman

Differential Revision: D50282417

fbshipit-source-id: 515600111c1ad7b15e40cfe5a3894e40c241c559
2023-10-17 20:30:16 -07:00
Joe Vilches
b558059b74 Revamp tests for row reverse + border after fix
Summary: this is fixed now so we can turn it on

Reviewed By: NickGerleman

Differential Revision: D50348206

fbshipit-source-id: 61c2a72164c6f0ee91b1b5b576d3f129e8cfbe40
2023-10-17 20:30:16 -07:00
Joe Vilches
3fbd92bc3b No longer skip margin_start and margin_end row reverse tests
Summary: These are fixed now!

Reviewed By: NickGerleman

Differential Revision: D50200030

fbshipit-source-id: ac3e80e33d8c35440e342a10cfd3246161ee9018
2023-10-12 16:22:27 -07:00
Joe Vilches
50dc5ae2d1 Add errata for fix to marginStart/End for row-reverse flex direction (#1419)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1419

X-link: https://github.com/facebook/litho/pull/961

X-link: https://github.com/facebook/react-native/pull/40803

This stack is ultimately aiming to solve https://github.com/facebook/yoga/issues/1208

This adds an value to the Errata enum. I will use this to gate this fix as there is potential for users to rely on this bug or have a hack in place to fix it and this would be a breaking change.

Reviewed By: NickGerleman

Differential Revision: D50145273

fbshipit-source-id: 913d2103cd31c1fa94cb39fc15d05b0c0b255920
2023-10-12 16:22:27 -07:00
Eric Rozell
50ecd98141 Add test fixture to reproduce bug in aspect-ratio (#1421)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1421

Adds repro of inconsistent implementation of aspect-ratio behavior across Web and React Native.

Reviewed By: NickGerleman

Differential Revision: D50225804

fbshipit-source-id: 0494e0373dcdebc789715b2ec19c002a47d69ce0
2023-10-12 15:02:00 -07:00
Joe Vilches
bac658b4f5 Add row reverse tests for position
Summary: after looking into the issue described in https://github.com/facebook/yoga/issues/1208 it seems to apply to position too, so adding tests to confirm

Reviewed By: NickGerleman

Differential Revision: D50154056

fbshipit-source-id: 64dd04ce3ad765526a547fe60b699b664f251c06
2023-10-11 14:02:39 -07:00
Joe Vilches
799624b9a5 Add row reverse tests for border
Summary: after looking into the issue described in https://github.com/facebook/yoga/issues/1208 it seems to apply to border too, so adding tests to confirm

Reviewed By: NickGerleman

Differential Revision: D50153472

fbshipit-source-id: a50f3e040153086b6a573924b513919dbb94f3c0
2023-10-11 14:02:39 -07:00
Joe Vilches
9d9b1f0874 Add row reverse tests for padding
Summary: after looking into the issue described in https://github.com/facebook/yoga/issues/1208 it seems to apply to padding too, so adding tests to confirm

Reviewed By: NickGerleman

Differential Revision: D50153085

fbshipit-source-id: bad0ef50389a71a45ec3a58d87c1dea0c2b26024
2023-10-11 14:02:39 -07:00
Nick Gerleman
98d2172e05 Use modern Android libc++ in Yoga and React Native OSS (#1412)
Summary:
X-link: https://github.com/facebook/react-native/pull/39795
Pull Request resolved: https://github.com/facebook/yoga/pull/1412

Android NDK 25 uses a version of libc++ that is more than three years old, missing a lot of basic features of C++ 20. This is rectified in NDK 26 (latest LTS NDK), which brings us up to date with latest Clang (17, released this year), and adds a new policy where future NDK versions will bump libc++ as part of bumping LLVM/Clang.

This requires an a beta AGP version (and corresponding Android Studio Preview). Based on how far we are historically, it wouldn't be a surprise if we see the stable release this month (well before the RN 0.74/Yoga 3.0 cut, even in the worse case).

Changelog:
[Android][Changed] - Use NDK 26

Reviewed By: yungsters

Differential Revision: D49895949

fbshipit-source-id: 37bb4d1fdf81137be7f14f6675b4e079c6f861e4
2023-10-04 19:57:14 -07:00
Joe Vilches
e60ae290ea Add fixtures to test proper margins with reverse flex directions
Summary:
Yoga has a known bug where marginStart and marginEnd will swap with row-reverse flex direction. This is not the intended behavior. On Paper this is also an issue with marginLeft and marginRight (at least we think Paper is the culprit, not exactly clear yet).

margin-start (and end) is not actually valid css. The gentest.rb script will just turn this into margin-left, but the cpp generated will properly test marginStart. This seems a bit weird to be since marginStart != marginLeft AFAIK. Things like RTL and LTR modes might make this test not exactly right. But given how many other tests depend on this quirk I think it is fine to add as is - the end result is the same after all. If not, a followup would be to add support for mapping margin-inline-start (valid css) to marginStart.

Anyway, this diff is to add test coverage for this scenario. Next stop is to actually try to fix this problem, which may be a bit harder :P

See https://github.com/facebook/yoga/issues/1208 for more info.

Reviewed By: NickGerleman

Differential Revision: D49744271

fbshipit-source-id: 75b8dd0cc5c53b2f338476fb70b60006aaa89054
2023-10-03 11:31:34 -07:00
jlmip
07cabca526 Fixed issue with first line element gap handling. (#1408)
Summary:
If the first element of a line is not contributing (e.g. position absolute), an additional gap will be added to the line, because the first gap element of the line is never identified (wrong start index).
Fix: raise the index of the first line element until we find an element that is contributing to the line.

Pull Request resolved: https://github.com/facebook/yoga/pull/1408

Reviewed By: yungsters

Differential Revision: D49722065

Pulled By: NickGerleman

fbshipit-source-id: 1068cb0b11ae4b04ec8d063e70540cce06181d5a
2023-10-03 06:24:48 -07:00
Nick Gerleman
557d2a76fe Breaking: Use C++ 20 (#1382)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1382

X-link: https://github.com/facebook/react-native/pull/39437

Have been running into places where C++ 20 makes life easier for use like `std::bit_cast` (that one is easy to polyfill), in-class member initializer support for bitfields, designated initializers, defaulted comparison operator, concepts instead of SFINAE, and probably more.

Our other infra is in the process of making this jump, or already has. This tests it out everywhere, across the various reference builds, to see if we have any issues.

This is a bit more aggressive than I had previously communicated, but n - 1 is going to be a better long term place than n - 2.

If we wanted to use `std::bit_cast` we would need one of:
1. GCC 11+ (~2.5 years old)
1. Clang 14 (~2.5 years old)
1. VS 16.11 (~2 years old)

For mobile this means:
1. NDK 26 (still in Beta 😭)
1. XCode 14.3.0 (~6 months old)

https://en.cppreference.com/w/cpp/compiler_support/20

That isn't quite doable yet, but we can start taking advantage of language features in the meantime. More of these will be supported in older toolchains.

Anyone needing support for older C++ versions can lag behind on more recent changes. E.g. Yoga 2.0 supports C++ 14.

bypass-github-export-checks

Changelog: [Internal]

Reviewed By: cortinico

Differential Revision: D49261607

fbshipit-source-id: ceb06eac20dfe93352d7b796d6847a7314069cf3
2023-09-19 01:28:35 -07:00
Nick Gerleman
c60050d0cb Fixup hack for flex line size calculation (#1380)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1380

X-link: https://github.com/facebook/react-native/pull/39433

Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.

`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on https://github.com/facebook/yoga/pull/1188

Reviewed By: javache

Differential Revision: D49260049

fbshipit-source-id: 218552c5ff938668b9f257df7a1493e13ded4d0d
2023-09-14 20:26:31 -07:00
Nick Gerleman
f9c2c27d33 Use fbsource clang-format config
Summary:
This mirrors the clang-format config used by fbsource to Yoga.

They are pretty similar, except for an annoying habit where Yoga's previous forced small functions in headers to be a a single line, so you would get a combination of multiline and single line functions next to each other which are hard to read. That is what motivated this change.

It also enforces header ordering (yay). I don't think we have any side-effect causing headers, so this should be safe.

Reviewed By: yungsters

Differential Revision: D49248994

fbshipit-source-id: 66998395e7c0158ff9d9fb1bee44e8401bdd8f21
2023-09-13 20:12:55 -07:00
Nick Gerleman
b1e0140aaa Remove JNI Binding usage of layoutContext (#1377)
Summary:
X-link: https://github.com/facebook/react-native/pull/39402

Pull Request resolved: https://github.com/facebook/yoga/pull/1377

To avoid keeping a per-node mapping on native Yoga nodes to Java nodes, a per-layout context was added, to be able to pass information from the start of the layout, to measure functions, log functions, etc.

The way this was done was super invasive, and added quite a few private APIs used only by the JNI functions.

This change removes the context-using functions from the JNI bindings in favor of it managing its own context. Next diff removes all the cruft.

Reviewed By: javache

Differential Revision: D49179243

fbshipit-source-id: 7e4944bead864e6b73fd2208a47c5725c18ff2b0
2023-09-12 19:08:55 -07:00
Nick Gerleman
c35f8819ae Breaking: Remove "UseLegacyStretchBehaviour" functions (#1368)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1368

X-link: https://github.com/facebook/react-native/pull/39372

These were marked as deprecated as part of the public Yoga 2.0 release, and were alredy emitting deprecation warnings. Remove them.

Reviewed By: javache

Differential Revision: D49131250

fbshipit-source-id: cc1d4e8b179697b9a11a685f4fc4e9d36e1a26a0
2023-09-11 19:51:40 -07:00
Nick Gerleman
776065d7c7 Breaking: size_t indices (#1366)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1366

X-link: https://github.com/facebook/react-native/pull/39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 6a004c160c4c50f68047b108508fd437156f5fac
2023-09-11 19:51:40 -07:00
Nick Gerleman
26f2b28eca Breaking: Fix callback const-correctness (#1369)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1369

X-link: https://github.com/facebook/react-native/pull/39370

This fixes const-correctness of callbacks (e.g. not letting a logger function modify nodes during layout). This helps us to continue to fix const-correctness issues inside of Yoga.

This change is breaking to the public API, since it requires a change in signature passed to Yoga.

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D49130714

fbshipit-source-id: 4305f8882d89f296e45b78497a51716a0dbb3b2d
2023-09-11 19:51:40 -07:00
Nick Gerleman
26d2a2682f yoga::bit_cast
Summary:
X-link: https://github.com/facebook/react-native/pull/39358

This adds a function polyfilling C++ 20's `std::bit_cast`, using `memcpy()` to be safe with strict aliasing rules.

This replaces the conditional code in CompactValue for type punning, an unsafe place in YGJNI where we do it unsafely, and is used in ValuePool. The polyfill can be switched to `std::bit_cast` whenever we adopt C++ 20.

Note that this doesn't actually call into `memcpy()`, as verified by Godbolt. Compilers are aware of the memcpy type punning pattern and optimize it, but it's ugly and confusing to folks who haven't seen it before.

Reviewed By: javache

Differential Revision: D49082997

fbshipit-source-id: b848775a68286bdb11b2a3a95bef8069364ac9b5
2023-09-08 13:03:48 -07:00
Nick Gerleman
f8e2bc0875 Add copyright header to ld version script (#1360)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1360

Fixes a ShipIt warning about this.

Reviewed By: yungsters

Differential Revision: D48992315

fbshipit-source-id: 20b6ba86abc27599e5f7dc12471344295151db66
2023-09-06 09:50:43 -07:00
Nick Gerleman
aee43a53bc Enable -Wconversion (#1359)
Summary:
X-link: https://github.com/facebook/react-native/pull/39291

Pull Request resolved: https://github.com/facebook/yoga/pull/1359

This enables clang warnings around potentially unsafe conversions, such as those with mismatched signedness, or ones which may lead to truncation.

This should catch issues in local development which create errors for MSVC (e.g. Dash), who's default `/W3` includes warnings akin to `-Wshorten-64-to-32`.

This full set of warnings here is a tad spammy, but probably more useful than not.

Changelog: [Internal]

Reviewed By: yungsters

Differential Revision: D48954777

fbshipit-source-id: 1ccc07b99d09d1c2d428158149698ffd04025605
2023-09-06 08:16:42 -07:00
Nick Gerleman
7be985d97c C++ Cleanup 8/N: Yoga-internal (#1355)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1355

X-link: https://github.com/facebook/react-native/pull/39198

## This diff

This splits up `Yoga-internal.h` which has become a grab bag. The actual header is left, with the purpose of being a private C ABI for bindings, but everything else is moved to a place more appropriate or removed.

A few notes:
1. `yoga::isUndefined` is replaced with `std::isnan` to avoid a layer of indirection (we will never be able to change its representation anyway). Internal usages of `YGFloatIsUndefined` are also replaced with `std::isnan` since the previous being at a library boundary means I'm not sure it can be inlined/.
2. `leading`, `trailing` arrays are factored into proper functions
3. `Values` is replaced entirely with `std::array`, since most of it was unused.

## This stack

The organization of the C++ internals of Yoga are in need of attention.
1. Some of the C++ internals are namespaced, but others not.
2. Some of the namespaces include `detail`, but are meant to be used outside of the translation unit (FB Clang Tidy rules warn on any usage of these)
2. Most of the files are in a flat hierarchy, except for event tracing in its own folder
3. Some files and functions begin with YG, others don’t
4. Some functions are uppercase, others are not
5. Almost all of the interesting logic is in Yoga.cpp, and the file is too large to reason about
6. There are multiple grab bag files where folks put random functions they need in (Utils, BitUtils, Yoga-Internal.h)
7. There is no clear indication from file structure or type naming what is private vs not
8. Handles like `YGNodeRef` and `YGConfigRef` can be used to access internals just by importing headers

This stack does some much needed spring cleaning:
1. All non-public headers and C++ implementation details are in separate folders from the root level `yoga`. This will give us room to split up logic and add more files without too large a flat hierarchy
3. All private C++ internals are under the `facebook::yoga` namespace. Details namespaces are only ever used within the same header, as they are intended
4. Utils files are split
5. Most C++ internals drop the YG prefix
6. Most C++ internal function names are all lower camel case
7. We start to split up Yoga.cpp
8. Every header beginning with YG or at the top-level directory is public and C only, with the exception of Yoga-Internal.h which has non-public functions for bindings
9. It is not possible to use private APIs without static casting handles to internal classes

This will give us more leeway to continue splitting monolithic files, and consistent guidelines for style in new files as well.

These changes should not be breaking to any project using only public Yoga headers. This includes every usage of Yoga in fbsource except for RN Fabric which is currently tied to internals. This refactor should make that boundary clearer.

Reviewed By: rshest

Differential Revision: D48769241

fbshipit-source-id: 5b8e2192309539e7c133c3b3b29b445b59dd5835
2023-09-04 11:20:17 -07:00
Nick Gerleman
992f073746 C++ Cleanup 3/N: Reorganize YGNode (#1350)
Summary:
X-link: https://github.com/facebook/react-native/pull/39219

Pull Request resolved: https://github.com/facebook/yoga/pull/1350

X-link: https://github.com/facebook/react-native/pull/39170

## This diff

This diff adds a top level `node` directory for code related to Yoga nodes and data structures on them (inc moving `YGLayout` to `LayoutResults`).

The public API for config handles is `YGNodeRef`, which is forward declared to be a pointer to a struct named `YGNode`. The existing `YGNode` is split into `yoga::Node`, as the private C++ implementation, inheriting from `YGNode`, a marker type represented as an empty struct. The public API continues to accept `YGNodeRef`, which continues to be `YGNode *`, but it must be cast to its concrete internal representation at the API boundary before doing work on it.

This change ends up needing to touch quite a bit, due to the amount of code that mixed and matched private and public APIs. Don't be scared though, because these changes are very mechanical, and Phabricator's line-count is 3x the actual amount due to mirrors and dirsyncs.

## This stack

The organization of the C++ internals of Yoga are in need of attention.
1. Some of the C++ internals are namespaced, but others not.
2. Some of the namespaces include `detail`, but are meant to be used outside of the translation unit (FB Clang Tidy rules warn on any usage of these)
2. Most of the files are in a flat hierarchy, except for event tracing in its own folder
3. Some files and functions begin with YG, others don’t
4. Some functions are uppercase, others are not
5. Almost all of the interesting logic is in Yoga.cpp, and the file is too large to reason about
6. There are multiple grab bag files where folks put random functions they need in (Utils, BitUtils, Yoga-Internal.h)
7. There is no clear indication from file structure or type naming what is private vs not
8. Handles like `YGNodeRef` and `YGConfigRef` can be used to access internals just by importing headers

This stack does some much needed spring cleaning:
1. All non-public headers and C++ implementation details are in separate folders from the root level `yoga`. This will give us room to split up logic and add more files without too large a flat hierarchy
3. All private C++ internals are under the `facebook::yoga` namespace. Details namespaces are only ever used within the same header, as they are intended
4. Utils files are split
5. Most C++ internals drop the YG prefix
6. Most C++ internal function names are all lower camel case
7. We start to split up Yoga.cpp
8. Every header beginning with YG or at the top-level directory is public and C only, with the exception of Yoga-Internal.h which has non-public functions for bindings
9. It is not possible to use private APIs without static casting handles to internal classes

This will give us more leeway to continue splitting monolithic files, and consistent guidelines for style in new files as well.

These changes should not be breaking to any project using only public Yoga headers. This includes every usage of Yoga in fbsource except for RN Fabric which is currently tied to internals. This refactor should make that boundary clearer.

Changelog: [Internal]

bypass-github-export-checks

Reviewed By: shwanton

Differential Revision: D48847258

fbshipit-source-id: fc560893533b55a5c2d52c37d8e9a59f7369f174
2023-08-30 19:57:16 -07:00
Nick Gerleman
f82babba8a C++ Cleanup 2/N: Reorganize YGConfig (#1348)
Summary:
X-link: https://github.com/facebook/react-native/pull/39218

X-link: https://github.com/facebook/react-native/pull/39169

Pull Request resolved: https://github.com/facebook/yoga/pull/1348

## This diff

This diff adds a top level `config` directory for code related to configuring Yoga and Yoga Nodes.

The public API for config handles is `YGConfigRef`, which is forward declared to be a pointer to a struct named `YGConfig`. The existing `YGConfig` is split into `yoga::Config`, as the private C++ implementation, inheriting from `YGConfig`, a marker type represented as an empty struct. The public API continues to accept `YGConfigRef`, which continues to be `YGConfig *`, but it must be cast to its concrete internal representation at the API boundary before doing work on it.

## This stack

The organization of the C++ internals of Yoga are in need of attention.
1. Some of the C++ internals are namespaced, but others not.
2. Some of the namespaces include `detail`, but are meant to be used outside of the translation unit (FB Clang Tidy rules warn on any usage of these)
2. Most of the files are in a flat hierarchy, except for event tracing in its own folder
3. Some files and functions begin with YG, others don’t
4. Some functions are uppercase, others are not
5. Almost all of the interesting logic is in Yoga.cpp, and the file is too large to reason about
6. There are multiple grab bag files where folks put random functions they need in (Utils, BitUtils, Yoga-Internal.h)
7. There is no clear indication from file structure or type naming what is private vs not
8. Handles like `YGNodeRef` and `YGConfigRef` can be used to access internals just by importing headers

This stack does some much needed spring cleaning:
1. All non-public headers and C++ implementation details are in separate folders from the root level `yoga`. This will give us room to split up logic and add more files without too large a flat hierarchy
3. All private C++ internals are under the `facebook::yoga` namespace. Details namespaces are only ever used within the same header, as they are intended
4. Utils files are split
5. Most C++ internals drop the YG prefix
6. Most C++ internal function names are all lower camel case
7. We start to split up Yoga.cpp
8. Every header beginning with YG or at the top-level directory is public and C only, with the exception of Yoga-Internal.h which has non-public functions for bindings
9. It is not possible to use private APIs without static casting handles to internal classes

This will give us more leeway to continue splitting monolithic files, and consistent guidelines for style in new files as well.

These changes should not be breaking to any project using only public Yoga headers. This includes every usage of Yoga in fbsource except for RN Fabric which is currently tied to internals. This refactor should make that boundary clearer.

Changelog: [Internal]

Reviewed By: shwanton

Differential Revision: D48847257

fbshipit-source-id: 7a2157d169ba80a6f79620693ae45bb10dfca5a3
2023-08-30 16:27:32 -07:00
Zhiyao Zhou
13c5ce2234 Revert D48710796: C++ Cleanup 2/N: Reorganize YGConfig
Differential Revision:
D48710796

Original commit changeset: d548553f7ce8

Original Phabricator Diff: D48710796

fbshipit-source-id: c8b2de245f3894f6a87c262ec70d313020aa228e
2023-08-29 23:27:25 -07:00
Zhiyao Zhou
ea7f61a3db Revert D48712710: C++ Cleanup 3/N: Reorganize YGNode
Differential Revision:
D48712710

Original commit changeset: d28eae38469a

Original Phabricator Diff: D48712710

fbshipit-source-id: 7a10b071edcf045ce98bbf8f9deca0d0e2e80a14
2023-08-29 23:27:25 -07:00
Nick Gerleman
59f75b0194 C++ Cleanup 3/N: Reorganize YGNode (#1350)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1350

X-link: https://github.com/facebook/react-native/pull/39170

## This diff

This diff adds a top level `node` directory for code related to Yoga nodes and data structures on them (inc moving `YGLayout` to `LayoutResults`).

The public API for config handles is `YGNodeRef`, which is forward declared to be a pointer to a struct named `YGNode`. The existing `YGNode` is split into `yoga::Node`, as the private C++ implementation, inheriting from `YGNode`, a marker type represented as an empty struct. The public API continues to accept `YGNodeRef`, which continues to be `YGNode *`, but it must be cast to its concrete internal representation at the API boundary before doing work on it.

This change ends up needing to touch quite a bit, due to the amount of code that mixed and matched private and public APIs. Don't be scared though, because these changes are very mechanical, and Phabricator's line-count is 3x the actual amount due to mirrors and dirsyncs.

## This stack

The organization of the C++ internals of Yoga are in need of attention.
1. Some of the C++ internals are namespaced, but others not.
2. Some of the namespaces include `detail`, but are meant to be used outside of the translation unit (FB Clang Tidy rules warn on any usage of these)
2. Most of the files are in a flat hierarchy, except for event tracing in its own folder
3. Some files and functions begin with YG, others don’t
4. Some functions are uppercase, others are not
5. Almost all of the interesting logic is in Yoga.cpp, and the file is too large to reason about
6. There are multiple grab bag files where folks put random functions they need in (Utils, BitUtils, Yoga-Internal.h)
7. There is no clear indication from file structure or type naming what is private vs not
8. Handles like `YGNodeRef` and `YGConfigRef` can be used to access internals just by importing headers

This stack does some much needed spring cleaning:
1. All non-public headers and C++ implementation details are in separate folders from the root level `yoga`. This will give us room to split up logic and add more files without too large a flat hierarchy
3. All private C++ internals are under the `facebook::yoga` namespace. Details namespaces are only ever used within the same header, as they are intended
4. Utils files are split
5. Most C++ internals drop the YG prefix
6. Most C++ internal function names are all lower camel case
7. We start to split up Yoga.cpp
8. Every header beginning with YG or at the top-level directory is public and C only, with the exception of Yoga-Internal.h which has non-public functions for bindings
9. It is not possible to use private APIs without static casting handles to internal classes

This will give us more leeway to continue splitting monolithic files, and consistent guidelines for style in new files as well.

These changes should not be breaking to any project using only public Yoga headers. This includes every usage of Yoga in fbsource except for RN Fabric which is currently tied to internals. This refactor should make that boundary clearer.

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D48712710

fbshipit-source-id: d28eae38469afa24a8cb03e4e75eeb8e431173c5
2023-08-29 21:32:56 -07:00
Nick Gerleman
5af3a5d1d9 C++ Cleanup 2/N: Reorganize YGConfig (#1348)
Summary:
X-link: https://github.com/facebook/react-native/pull/39169

Pull Request resolved: https://github.com/facebook/yoga/pull/1348

## This diff

This diff adds a top level `config` directory for code related to configuring Yoga and Yoga Nodes.

The public API for config handles is `YGConfigRef`, which is forward declared to be a pointer to a struct named `YGConfig`. The existing `YGConfig` is split into `yoga::Config`, as the private C++ implementation, inheriting from `YGConfig`, a marker type represented as an empty struct. The public API continues to accept `YGConfigRef`, which continues to be `YGConfig *`, but it must be cast to its concrete internal representation at the API boundary before doing work on it.

## This stack

The organization of the C++ internals of Yoga are in need of attention.
1. Some of the C++ internals are namespaced, but others not.
2. Some of the namespaces include `detail`, but are meant to be used outside of the translation unit (FB Clang Tidy rules warn on any usage of these)
2. Most of the files are in a flat hierarchy, except for event tracing in its own folder
3. Some files and functions begin with YG, others don’t
4. Some functions are uppercase, others are not
5. Almost all of the interesting logic is in Yoga.cpp, and the file is too large to reason about
6. There are multiple grab bag files where folks put random functions they need in (Utils, BitUtils, Yoga-Internal.h)
7. There is no clear indication from file structure or type naming what is private vs not
8. Handles like `YGNodeRef` and `YGConfigRef` can be used to access internals just by importing headers

This stack does some much needed spring cleaning:
1. All non-public headers and C++ implementation details are in separate folders from the root level `yoga`. This will give us room to split up logic and add more files without too large a flat hierarchy
3. All private C++ internals are under the `facebook::yoga` namespace. Details namespaces are only ever used within the same header, as they are intended
4. Utils files are split
5. Most C++ internals drop the YG prefix
6. Most C++ internal function names are all lower camel case
7. We start to split up Yoga.cpp
8. Every header beginning with YG or at the top-level directory is public and C only, with the exception of Yoga-Internal.h which has non-public functions for bindings
9. It is not possible to use private APIs without static casting handles to internal classes

This will give us more leeway to continue splitting monolithic files, and consistent guidelines for style in new files as well.

These changes should not be breaking to any project using only public Yoga headers. This includes every usage of Yoga in fbsource except for RN Fabric which is currently tied to internals. This refactor should make that boundary clearer.

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D48710796

fbshipit-source-id: d548553f7ce872488ebdd697e0aceaa9a625df62
2023-08-29 21:32:56 -07:00
Andrew Wang
49fbd406b6 Ship the fix for local reference overflow in Yoga (#1347)
Summary:
X-link: https://github.com/facebook/litho/pull/954

X-link: https://github.com/facebook/react-native/pull/39132

Pull Request resolved: https://github.com/facebook/yoga/pull/1347

# Context

Reviewed By: NickGerleman, astreet

Differential Revision: D48607502

fbshipit-source-id: 79552bc76879d1fc15341423ae6fbadeab2fb7af
2023-08-24 12:48:56 -07:00
Nick Gerleman
38ad93c87b Fix segfault calling YGJNILogFunc (#1344)
Summary:
X-link: https://github.com/facebook/react-native/pull/39051

Pull Request resolved: https://github.com/facebook/yoga/pull/1344

`YGJNILogFunc` has a bug where it uses a `va_list` to determine the length of a printf string, then reuses the same `va_list` later after it has already been iterated through. Even if no arguments are present, this may cause a crash looking something like:

```
C  [libsystem_platform.dylib+0xf12]  _platform_strlen+0x12
C  [libsystem_c.dylib+0x31bf]  __vfprintf+0x1339
C  [libsystem_c.dylib+0x307ce]  _vsnprintf+0x100
C  [libsystem_c.dylib+0x6965]  vsnprintf+0x44
C  [libyoga.dylib+0x5161]  YGJNILogFunc(YGConfig*, YGNode*, YGLogLevel, void*, char const*, __va_list_tag*)+0x59
```

Fixing this fixes crashing unit tests which are not explicitly disabled.

Reviewed By: yungsters

Differential Revision: D48388548

fbshipit-source-id: 492e7a89aeb5f9d15485ce31641875a295356bef
2023-08-18 00:07:51 -07:00
Nick Gerleman
660edcec20 C++ 17 style nested namespaces (#1326)
Summary:
X-link: https://github.com/facebook/react-native/pull/38304

Pull Request resolved: https://github.com/facebook/yoga/pull/1326

For better readability

Reviewed By: christophpurrer

Differential Revision: D47384926

fbshipit-source-id: 2f60d50a185331b3624d45d1fc45f98d504b3034
2023-07-12 09:38:40 -07:00
Nicola Corti
dbd8e915d5 Remove the build-logic module
Summary:
As now we have a single module to publish, I'm removing the build-logic folder and moving everything inside the java/build.gradle file.
I've also converted it to Kotlin,

Reviewed By: NickGerleman

Differential Revision: D47259204

fbshipit-source-id: 2378d9e9598d7816f230db5f763f2b0f4cdf01d0
2023-07-11 04:30:02 -07:00
Nick Gerleman
0aec0a1ac4 Remove YGExperimentalFeatureFixAbsoluteTrailingColumnMargin (#1317)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1317

X-link: https://github.com/facebook/react-native/pull/37374

This is edge-casey enough, and I actually broke this in D42282358 without us noticing (I changed height to width of the bottom usage, instead, copy/pasting the value of the top one).

Reviewed By: yungsters

Differential Revision: D45766764

fbshipit-source-id: b600b79b8436534fe48ef2acbfde8ba64068e593
2023-06-29 11:04:01 -07:00
Andrew Wang
f3e9b6bfb0 Fix the issue that local reference overflows in Yoga 1 (#1308)
Summary:
X-link: https://github.com/facebook/react-native/pull/37929

X-link: https://github.com/facebook/litho/pull/952

Pull Request resolved: https://github.com/facebook/yoga/pull/1308

Long story in short, we're trying to fix an issue with Yoga that could potentially lead to an overflow in the JNI local reference table.

Reviewed By: NickGerleman, astreet

Differential Revision: D46653732

fbshipit-source-id: 0bc34bd5a819037c046c62b651e414b249cbdcb8
2023-06-16 06:15:07 -07:00
Nicola Corti
36e491754f Setup publishing for Yoga stables
Summary:
This sets up publishing for stable of Yoga whenever a git tag gets published.
I also re-enabled Javadoc publishing as this is a requirement to hit Maven Central.

Reviewed By: NickGerleman

Differential Revision: D46522898

fbshipit-source-id: dc4b9139aed7aff27dce966bcee7b9b0cc4c6fe9
2023-06-08 02:23:48 -07:00
Nicola Corti
9a0ba05d13 Setup publishing of snapshots using io.github.gradle-nexus.publish-plugin
Summary:
This sets up publishing of -SNAPSHOT verison of Yoga using the
Gradle plugin `io.github.gradle-nexus.publish-plugin`

This plugin will take care of setting up the credentials for Sonatype and hitting the Maven repository.
I've cleaned up the setup and centralized it inside a script plugin in the `build-logic` folder so we can easily add more module and just use `id("publish")` to publish them as well.

Reviewed By: mdvacca

Differential Revision: D46330013

fbshipit-source-id: 7221b296b9955a257fc290a2d1ac1d9fedfb787d
2023-05-31 15:22:21 -07:00
Nicola Corti
186f4d318d Update project to use Gradle's plugins{} block
Summary:
This project was still using the legacy Gradle's "apply:" syntax
to apply Gradle Plugins. I'm changing it here to use instead the
`plugins{}` block as it make easier to apply other plugins for publishing later.

Reviewed By: mdvacca

Differential Revision: D46330014

fbshipit-source-id: 5483a717a62d5ab76749026c5203dc96a35d73e7
2023-05-31 14:48:05 -07:00
Nick Gerleman
f2b4e42ca2 Change how we set cmake policy (#1288)
Summary:
X-link: https://github.com/facebook/react-native/pull/37349

Pull Request resolved: https://github.com/facebook/yoga/pull/1288

Fixes https://github.com/facebook/yoga/issues/1283

New versions of CMake add "policies" which control how the build system acts wrt breaking changes. By default, CMake will emulate the behavior of the version specified in `cmake_minimum_required`.

Setting a policy to true (to opt into new behavior where `cmake_minimum_required` is lower than the current version) seems actually just error out on the old versions.

Googling around, apparently the way I should be doing this is to specify `<policy_max>` as part of `cmake_minimum_required `. https://gitlab.kitware.com/cmake/cmake/-/issues/20392

This should I think use new policies introduced up to 3.26 (what we test on right now), while letting 3.13 be the minimum.

Reviewed By: cortinico

Differential Revision: D45724864

fbshipit-source-id: 120cc2015a043605e7c07ef0459667643a4284b7
2023-05-15 15:21:56 -07:00
Nick Gerleman
3fb698c08b Remove pinned CMake version from Gradle build (#1292)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1292

AGP 8.0 will now bring in a new enough CMake version to avoid internal warnings without extra configuration.

Reviewed By: cortinico

Differential Revision: D45766980

fbshipit-source-id: 7834f723c87ec76c450a94958cd36b4930a8961a
2023-05-15 14:03:00 -07:00
Nick Gerleman
6db81656ac Make jni_YGNodeDeallocateJNI call YGNodeDeallocate
Summary:
X-link: https://github.com/facebook/react-native/pull/37388

Fixes https://github.com/facebook/yoga/issues/1271

Updating this glue was missed in D45556206 when moving from one revision to the other...

Reviewed By: yungsters

Differential Revision: D45780647

fbshipit-source-id: 4ca64bc9971d3e4697990e73b618a3dc91df259b
2023-05-11 13:30:49 -07:00
Nick Gerleman
e53d1ee28a Enable -Wextra in C++ builds (#1294)
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
2023-05-11 09:43:36 -07:00
Nick Gerleman
a97dbecb49 Apply fixes from clag-tidy misc-unused-parameters (#1293)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1293

X-link: https://github.com/facebook/react-native/pull/37384

Gets the project mostly clean of `-Wunused-parameter`, part of `-Wextra`.

Reviewed By: yungsters

Differential Revision: D45772554

fbshipit-source-id: db4c4d2bd222debef178c4c16f7b60fb6c8db2a2
2023-05-11 09:43:36 -07:00
Nick Gerleman
3b088c3383 Fix use-after-free if JNI Yoga nodes are garbage collected using multiple threads (#1279)
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
2023-05-10 22:52:48 -07:00
Nick Gerleman
e769dd97d8 Add data-disabled to test fixtures (#1286)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1286

This can be marked in fixtures to skip a test without commenting it out. We add one more usage of this.

The same functionality existed (unused) before for `experiments`, which I changed to `data-experiments`.

Formatting of JS tests changed to be closer to what Prettier would output, and to remove usage of `Yoga.UNDEFINED` which doesn't existi and just resolves to `undefined` (this is converted to NaN by the wrapper layer).

Reviewed By: yungsters

Differential Revision: D45723003

fbshipit-source-id: 337af319ab1c1c12047d6579da8c7e63b4f1537a
2023-05-10 22:46:39 -07:00
Nicola Corti
54d78926ce AGP to 8.0.1 (#1280)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1280

I'm refreshing some of the dependencies of the project,
here doing AGP to 8.0.1.
I'm also removing the older publishing plugin as we're most likely going to use
Gradle's default publishing + another plugin to manage the nexus interactions (the same we use on React Native).

I'm also doing some changes on the JDK side:
- Bumps the JDK version to 17 as that's required by AGP
- Bumps the source/target version to JDK 8. JDK 7 is long deprecated and we're getting a lot of warnings for it on console. Users should be on JDK 11 already by now, but 8 is also good enough.

Reviewed By: passy

Differential Revision: D45564575

fbshipit-source-id: ffe1cc15892659923177a2cad609d5d30f8249ac
2023-05-05 04:07:40 -07:00
Nick Gerleman
d8dec0f85a Don't export private headers from Buck target (#1269)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1269

X-link: https://github.com/facebook/react-native/pull/37127

This prevents targets which include Yoga from using its private APIs.

Instances of this have been mostly cleaned up in the past diffs, with the major exception of RN Fabric. To stage this without blocking on that, I added a `yoga-private-api` target for now to keep using these headers while making it unlikely new usages will show up.

Reviewed By: javache

Differential Revision: D45339425

fbshipit-source-id: eb7ef151ad2467d7c3370cd7c10d47e8db9496a0
2023-05-02 18:08:58 -07:00
Nick Gerleman
f32d37a3d5 Deprecate YGConfigSetUseLegacyStretchBehaviour (#37117)
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
2023-04-30 08:20:05 -07:00