Commit Graph

17 Commits

Author SHA1 Message Date
Nick Gerleman
5009f5c1ac Fix misc-misplaced-const warnings in Yoga tests (#1677)
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1677

We have Clang Tidy warnings enabled internally, that will flag `const YG*Ref` as misleading, as a const pointer to non-const, instead of non-const pointer to const.

Let's remove all the misleading const in existing tests, and generated tests.

Reviewed By: joevilches

Differential Revision: D59335968

fbshipit-source-id: c66af878904ba7900f8ffcd99162968d90f8e5c7
2024-07-03 17:30:10 -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
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
08eaae7223 Reorgnaize C++ tests
Summary:
This does some preprataion for the Yoga CMake Build. The main change is removing the dedicated testutil top-level-directory and static library. This contains a method to count nodes using the event functions exposed to C++, along with a Java binding for the test utility (since the events don't have a Java binding). It is only used in a single place in a way that isn't very useful, so it simplifies things to treat is as source in the existing C++ test library.

This also separates the hand-written and generated UTs, like we are doing in the JS directory in D42207782.

Reviewed By: christophpurrer

Differential Revision: D42247762

fbshipit-source-id: f8a270e99d0315ba7fc608f2471333e7a7be9d79
2022-12-28 01:21:52 -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
James Burnett
1c8e8d3aec Compile Issues with Recent GCC (#895)
Summary:
GCC 8.3.0 (and possibly all gcc 7+) identified several warnings for signed unsigned integer comparison. With `-Werror` enabled this broke compiling tests. I suspect the warning is related to google/googletest#683. This diff updates those `ASSERT_EQ` calls that attempt to compare signed and unsigned errors by specifically declaring the literals to be unsigned.

There is also an issue with Buck where it will not link to pthreads. facebook/buck#1443. Adding a `prebuilt_cxx_library` for pthread fixes that issue and the tests will compile and run.

Finally, there was a warning about a missing return after a switch in `InstrumentationTest.cpp`. I added a `return ""` as a default, but it might be better to throw something. Thoughts?
Pull Request resolved: https://github.com/facebook/yoga/pull/895

Reviewed By: davidaurelio

Differential Revision: D15393082

Pulled By: davidaurelio

fbshipit-source-id: 4f13ec2f016af39537c08fb591b188a6a0ed55ce
2019-07-04 19:08:18 -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
9e20dfeea1 Switch tests to test utility for counting nodes
Summary:
@public
replaces the global node counter with the event-based one for all tests.

Reviewed By: SidharthGuglani

Differential Revision: D15174856

fbshipit-source-id: f4401d502bdbaf3b6e4632a4d985aac260cb35a8
2019-05-09 04:31:24 -07:00
David Aurelio
b1c749075d Clone children only during layout, allow mixing shared + owned children
Summary:
@public

Limit child cloning to layout calculation. This also allows for mixing shared and owned children.

Rationale:
We do allow for shared children if the caller manages themselves. The single known use case is React Native.

So far, we have cloned children eagerly whenever child lists are mutated, or layout is run. This was to allow for a quick check of the owner of any first child, assuming that either *all* or *no* child of a node are shared.

For Yoga/Java, we want to get rid of global weak JNI refs, and these are also used to invoke clone callbacks. We can achieve that goal by switching to an alternative approach, passing additional data to the layout pass. This additional data has to be passed to any configured cloning callback. Therefore, it is desirable to **only call cloning functions during the layout pass.**

The obvious solution seems to be to not uphold the invariant of the first child determining shared/owned state of all siblings, and allow for a mix of shared and own children.

Reviewed By: shergin

Differential Revision: D14136223

fbshipit-source-id: 34490cfeeb2170c99d6ed1b9bdcbcedb316813af
2019-02-20 12:01:14 -08:00
David Aurelio
b9972cee6e Adjust yearless format for MIT license
Summary:
@public

Adjust license headers throughout the project

Reviewed By: SidharthGuglani

Differential Revision: D13255691

fbshipit-source-id: 98be2aa372a94e7a54a65e3d64e5c6a436b18083
2018-11-29 11:37:52 -08:00
Héctor Ramos
e224a29d1c Update copyright headers to yearless format
Summary: This change drops the year from the copyright headers and the LICENSE file.

Reviewed By: yungsters

Differential Revision: D9727774

fbshipit-source-id: df4fc1e4390733fe774b1a160dd41b4a3d83302a
2018-09-11 15:53:35 -07:00
Valentin Shergin
89ba4f282d Removed misleading comment in YGPersistenceTest
Summary:
Trivial... but quite confusing. :)

Created from Diffusion's 'Open in Editor' feature.

Reviewed By: priteshrnandgaonkar

Differential Revision: D7151891

fbshipit-source-id: d09a6434d6327736c2d5ce672b3e9c5b5573508a
2018-07-11 20:12:37 -07:00
Sophie Alpert
a2b6ddb7b1 Update license headers for MIT license
Summary:
Includes React Native and its dependencies Fresco, Metro, and Yoga. Excludes samples/examples/docs.

find: ^(?:( *)|( *(?:[\*~#]|::))( )? *)?Copyright (?:\(c\) )?(\d{4})\b.+Facebook[\s\S]+?BSD[\s\S]+?(?:this source tree|the same directory)\.$
replace: $1$2$3Copyright (c) $4-present, Facebook, Inc.\n$2\n$1$2$3This source code is licensed under the MIT license found in the\n$1$2$3LICENSE file in the root directory of this source tree.

Reviewed By: TheSavior, yungsters

Differential Revision: D7007050

fbshipit-source-id: 37dd6bf0ffec0923bfc99c260bb330683f35553e
2018-02-16 18:27:33 -08:00
Pritesh Nandgaonkar
03dd1d23fa vector instead of YGNodeList
Summary: Replaced YGNodeList with std::vector

Reviewed By: jonathandann, emilsjolander

Differential Revision: D6442379

fbshipit-source-id: d2d48ef0676351d2eeaa2d427dcd72e082cd15a1
2017-12-05 08:28:54 -08:00
Sebastian Markbage
4785e95a0c Persistent Yoga
Summary:
This is meant to show a possible route format for a persistent form of Yoga. Where previous layouts can remain intact while still taking advantage of incremental layout by reusing previous subtrees.

```c
YGNodeRef YGNodeClone(const YGNodeRef node);
```

The core of this functionality is a new API to clone an existing node. This makes a new detached node with all the same values as the previous one. Conceptually this makes the original node "frozen" from that point on. It's now immutable. (This is not yet enforced at runtime in this PR but something we should add.)

Since the original is frozen, we reuse the children set from the original node. Their parent pointers still point back to the original tree though.

The cloned node is still mutable. It can have its styles updated, and nodes can be inserted or deleted. If an insertion/deletion happens on a cloned node whose children were reused, it'll first shallow clone its children automatically.

As a convenience I also added an API to clear all children:

```c
void YGNodeRemoveAllChildren(const YGNodeRef node);
```

During insert/delete, or as a result of layout a set of reused children may need to be first cloned. A kind of copy-on-write. When that happens, the host may want to respond. E.g. by updating the `context` such as by cloning any wrapper objects and attaching them to the new node.

```c
typedef void (*YGNodeClonedFunc)(YGNodeRef oldNode,
                                 YGNodeRef newNode,
                                 YGNodeRef parent,
                                 int childIndex);

void YGConfigSetNodeClonedFunc(YGConfigRef config,
                               YGNodeClonedFunc callback);
```

This PR doesn't change any existing semantics for trees that are not first cloned.

It's possible for a single node to exist in two trees at once and be used by multiple threads. Therefore it's not safe to recursively free a whole tree when you use persistence. To solve this, any user of the library has to manually manage ref counting or tracing GC. E.g. by replicating the tree structure in a wrapper.

In a follow up we could consider moving ref counting into Yoga.
Closes https://github.com/facebook/yoga/pull/636

Reviewed By: emilsjolander

Differential Revision: D5941921

Pulled By: sebmarkbage

fbshipit-source-id: c8e93421824c112d09c4773bed4e3141b6491ccf
2017-10-17 01:11:11 -07:00