Commit Graph

25 Commits

Author SHA1 Message Date
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
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
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
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
85352c4e45 Automatic lint fixes
Summary:
@public
A round of automatic lint fixes.

Reviewed By: SidharthGuglani

Differential Revision: D14590396

fbshipit-source-id: f0b4a0ce503a1d9d46ea7ae788f9f2eac09c2ac7
2019-03-25 05:41:30 -07:00
David Aurelio
8ab01fde6e Back out "[yoga][PR] Fix aspect ratio when stretching with main axis margin"
Summary: Original commit changeset: 6970e6072e79

Reviewed By: mdvacca

Differential Revision: D13437552

fbshipit-source-id: 65a55d716da6b6582e38efa906d3f540e58418bf
2018-12-12 16:18:56 -08:00
Emil Sjölander
e522b2dee4 Fix aspect ratio when stretching with main axis margin (#834)
Summary:
I've noticed that when a child's size is determined by `align-items: stretch` in combination with `aspect-ratio` its size is wrongly calculated to account for margin in the main axis when there is more than enough space.

See playground: https://goo.gl/tgW6cD

I've yet to figure out exactly how to solve this but i've started by writing a failing test when can be seen in the first commit here.

I assumed I had found the bug here https://github.com/facebook/yoga/blob/master/yoga/Yoga.cpp#L1838 where margin is being subtracted from the desired width even though the measure mode tells it to measure to exactly that size. However, if we don't remove this margin from the available width then 15 tests fail (including the one I just added) not quite figured out why yet. I'm also a bit confused at to why this would only happen for nodes with `aspect-ratio` and not for nodes where an explicit height and width is set.
Pull Request resolved: https://github.com/facebook/yoga/pull/834

Reviewed By: astreet

Differential Revision: D13223579

Pulled By: davidaurelio

fbshipit-source-id: 6970e6072e79f3bb6f9097355ab6e441441bfd88
2018-12-10 09:23:03 -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
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
Yuichi ONO
44622912a2 Fix main size calculation from the aspect ratio
Summary:
When the following conditions are met, the main size become smaller by the margins in the main axis.
* The aspect ratio is defined
* The main size is not defined
* The cross size is defined
* The main margin is defined

This is because the main margin size is not included when calculating the main size from the aspect ratio.
Closes https://github.com/facebook/yoga/pull/715

Reviewed By: emilsjolander

Differential Revision: D6998988

Pulled By: priteshrnandgaonkar

fbshipit-source-id: f6f69c47ece17bd7c5e41517b96032bf0c149356
2018-02-15 07:40:34 -08:00
Pritesh Nandgaonkar
fbd332dee8 Make YGNode as c++ struct with properties exposed through accessors
Summary: Moved c implementation of `YGNode` to C++ struct. Not moving to C++ class as the React Classes dependent on `Yoga.h` assume it to be C. Thats why keeping `Yoga.h` C compatible. Sorry for the long diff, didn't thought that it will turn out to be this much big.Will keep an eye on number of lines next time 😉

Reviewed By: emilsjolander

Differential Revision: D6592257

fbshipit-source-id: 641e8b9462ad00731a094511f9f5608b23a6bb21
2017-12-19 11:33:07 -08:00
Emil Sjolander
35a9f33abb BREAKING: Change aspect ratio behavior
Summary:
@public

== Before ==
- Aspect ratio would do its best to fit within it's parent constraints
- Aspect ratio would prioritize `alignItems: stretch` over other sizing properties.

== After ==
- Aspect ratio is allowed to make a node grow past its parent constraints. This matches many other aspects of flexbox where parent constraints are not treated as hard constraints but rather as suggestions.
- Aspect ratio only takes `alignItems: stretch` into account if no other size definition is defined. This matches the interaction of other properties with `alignItems: stretch`.

== Updating your code ==

**You probably don't need to do anything** but in case something does break in your product it should be as easy as  adding `{width: '100%', height: '100%', flexShrink: 1}` to the style declaring the `aspectRatio`.

Reviewed By: gkassabli

Differential Revision: D5639187

fbshipit-source-id: 603e8fcc3373f0b7f2461da2dad1625ab59dcb19
2017-08-21 03:12:00 -07:00
Emil Sjolander
85c2e406e4 Fix flex basis not accounting for max size constraint
Summary: Fix flex basis not being constraint to the max size in the main direction. Previously this caused the added test to fail due to NaN in child dimensions.

Reviewed By: gkassabli

Differential Revision: D5044314

fbshipit-source-id: d9f9db832e4943a57a89c9d162ff6077b709795a
2017-05-12 09:12:51 -07:00
Pascal Hartig
326ae15532 Format C/C++ files
Summary: Run new `format.sh`.

Reviewed By: emilsjolander

Differential Revision: D4481501

fbshipit-source-id: 5791bee9919b44282d1549029ff8e078f253ddf8
2017-01-31 09:40:19 -08:00
Emil Sjolander
7fa4adb0d9 Fix aspectratio with margins
Summary: aspect ratio did not account for the widths and heights being including padding. This diff fixes that.

Reviewed By: astreet

Differential Revision: D4473024

fbshipit-source-id: 5a747e2f267b077203bb3b63e4c152847dc30774
2017-01-27 10:09:48 -08:00
Emil Sjolander
e39f13a8ea Allow aspect ratio to expand beyond bounds of parent
Summary: Allow aspect ratio to expand beyond bounds of parent as is generally accepted in css

Reviewed By: passy

Differential Revision: D4397547

fbshipit-source-id: d2b1ca7b096f2f17b3efbd8f47a50678bfe7bb5f
2017-01-10 08:39:36 -08:00
Emil Sjolander
1b5eb7da5e BREAKING - Increase priority of AspectRatio to override flex, align stretch, and fixed sizes if specified.
Summary:
@public
AspectRatio is a new addition and soon after introduction we noticed use cases which is did not support. Specifically we wanted to support a node being as large as possible within a container while maintaining an arbitrary aspect ratio. This was not possible due to the low priority of AspectRatio, by increasing the priority of AspectRatio this is now possible as FlexGrow will grow an item to fit its parent unless the AspectRatio makes it too big in the cross axis, the AspectRatio will now override the FlexGrow in the main axis in that case.

Differential Revision: D4346720

fbshipit-source-id: 1f15613604190e3ad5ff4a467ba57db4bcfd2741
2016-12-22 03:10:06 -08:00
Emil Sjolander
071f576db9 BREAKING - Change aspect ratio to always be width/height
Summary:
@public

Aspect ratio being defined as width/height or height/width depending on the situation it was used in turned out to be very confusing. This diff makes aspect ratio always be defined as width/height irregardless of the usage.

Differential Revision: D4339132

fbshipit-source-id: e5da32750b55ddaf6acaf1cbd7662d86f2b480c3
2016-12-22 03:10:06 -08:00
Emil Sjolander
74fb205083 Rename test files to match other test file names
Summary: Rename test files to match other test file names

Differential Revision: D4335198

fbshipit-source-id: b8f38162c3094231476059eb1f54326d8ba84848
2016-12-16 09:09:37 -08:00