Commit Graph

11 Commits

Author SHA1 Message Date
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
61bf4d15ab Fixup Yoga UT compilation on non-clang compilers (#1333)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1333

Yoga is compiled on more than clang, both internally, and in OSS. D47345669 added unguarded usage of `
#pragma clang diagnostic push` to Yoga UTs which will cause unrecognized pragma errors if building the tests on other platforms.

The code here is flagged for implicitly converting enum to float. This is test code which is using the enum as an arbitrary differentiating input, so I just changed this to an explicit cast, which I assume should silence the warning.

Reviewed By: NuriAmari

Differential Revision: D47460301

fbshipit-source-id: 952ff61e0c26deb8bbff8b860faf30896753a3bd
2023-07-14 09:49:50 -07:00
Nuri Amari
6ae890dccd Pragma guard remaining -Wenum-conversion failures
Summary:
These incorrect enum conversions constitute compile errors
with Pika / Xcode 15. Pragma guard the current offenders so
we can make the warning fatal again.

Reviewed By: drodriguez

Differential Revision: D47345669

fbshipit-source-id: bb85a93e9f48236c9f8c54d59d55a86648c1c6fd
2023-07-13 09:53:47 -07:00
Nick Gerleman
83cef5b12f Fixup UTs for MSVC and Release Mode
Summary: This fixes incompatibility with MSVC in /W3 (designated initializers, precision loss) along with guarding tests which will only pass in DEBUG builds

Reviewed By: cortinico

Differential Revision: D42406531

fbshipit-source-id: 2c0d59678f76decf9b9b4d91a7c9ec12136ca1b9
2023-01-09 13:59:19 -08:00
Nick Gerleman
c96564d23d Fix License Headers and Whitespace
Summary: This change applies all Arcanist recommended lint changes, which amounts to changing copyright headers and some cases of whitespace changes.

Reviewed By: yungsters

Differential Revision: D40060899

fbshipit-source-id: b62f9472e6ef58a3fc3d22eed661578a2635cb1f
2022-10-04 13:59:32 -07:00
Andres Suarez
42bba10894 Tidy up license headers
Summary: Changelog: Tidy up license headers

Reviewed By: SidharthGuglani

Differential Revision: D17919414

fbshipit-source-id: 0501b495dc0a42256ca6ba3284a873da1ab175c0
2019-10-15 10:36:38 -07:00
Rain ⁣
a4bdd9cd9b standardize C-like MIT copyright headers throughout fbsource
Summary:
`/*` is the standard throughout open source code. For example, Firefox uses single /*: https://hg.mozilla.org/mozilla-central/file/21d22b2f541258d3d1cf96c7ba5ad73e96e616b5/gfx/ipc/CompositorWidgetVsyncObserver.cpp#l3

In addition, Rust considers `/**` to be a doc comment (similar to Javadoc) and having such a comment at the beginning of the file causes `rustc` to barf.

Note that some JavaScript tooling requires `/**`. This is OK since JavaScript files were not covered by the linter in the first place, but it would be good to have that tooling fixed too.

Reviewed By: zertosh

Differential Revision: D15640366

fbshipit-source-id: b4ed4599071516364d6109720750d6a43304c089
2019-06-06 19:44:16 -07:00
David Aurelio
7331de78bd setBaseLine -> setBaseline
Summary:
@public

`setBaseLine` was the only place where we used a capitalised *L.* Fixed here.

Reviewed By: SidharthGuglani

Differential Revision: D14152320

fbshipit-source-id: abf54fe7d6088e03775968baa8421c4bf43d6a6e
2019-02-21 05:38:45 -08:00
David Aurelio
e7fcf1ee65 Add support for context-aware measure and baseline functions
Summary:
@public

Context-aware measure and baseline functions are an internal Yoga feature that will be used for Yoga’s JNI code.

It will be possible to specify a context when calculating layout, which will be passed on to baseline and measure functions. This will be a private feature.

Reviewed By: SidharthGuglani

Differential Revision: D14100509

fbshipit-source-id: acf4a030549b2e38d5ce0cd5dbe837864e5ffd81
2019-02-19 09:58:42 -08:00
David Aurelio
f86c74ce7e Call measure and baseline fns within YGNode
Summary:
@public

Stricter encapsulation of baseline and measure callbacks withing `YGNode`.

Instead of invoking these callbacks directly (`node->getBaseline()(...)`), they are invoked via methods on `YGNode` (`node->baseline(...)`).

This change will allow us to add the concept of a *Layout Context,* where measure and baseline functions will be able to receive an additional `void *` argument if configured accordingly. This API will be used internally for Yoga’s JNI bindings, to avoid storing a weak JNI reference for each node, and avoid reference table overflows.

Changed API:

- `YGNodeGetMeasureFunc()` -> `YGNodeHasMeasureFunc()`
- `YGNodeGetBaselineFunc()` -> `YGNodeHasBaselineFunc()`
- `YGNode::getMeasure()` -> `YGNode::hasMeasureFunc()` + `YGNode::measure()`
- `YGNpde::getBaseline()` -> `YGNode::hasBaselineFunc()` + `YGNode::baseline()`

Reviewed By: SidharthGuglani

Differential Revision: D14099550

fbshipit-source-id: 2653ab36acc252a9747986bc88d21dac22d8c91b
2019-02-19 09:58:42 -08:00