-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move configuration to new YGConfig and pass them down to CalculateLayout #432
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments. Looks very good! Thanks a ton for this
yoga/Yoga.h
Outdated
WIN_EXPORT YGConfigRef YGConfigNew(void); | ||
WIN_EXPORT void YGConfigFree(const YGConfigRef config); | ||
|
||
WIN_EXPORT void YGSetExperimentalFeatureEnabled(YGConfigRef config, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const for params please
yoga/Yoga.c
Outdated
YGConfigRef YGConfigNew(void) { | ||
const YGConfigRef config = gYGMalloc(sizeof(YGConfig)); | ||
YG_ASSERT(config, "Could not allocate memory for config"); | ||
gNodeInstanceCount++; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove gNodeInstanceCount++ as this is not allocating a node. I don't think we care about tracking these allocations right now.
yoga/Yoga.c
Outdated
static YGConfig gYGConfigDefaults = { | ||
.experimentalFeatures = | ||
{ | ||
[YGExperimentalFeatureRounding] = false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please uses spaces for indentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those are spaces, the formater is layouting it like that.
yoga/Yoga.c
Outdated
@@ -188,6 +190,14 @@ static YGNode gYGNodeDefaults = { | |||
}, | |||
}; | |||
|
|||
static YGConfig gYGConfigDefaults = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const
@@ -94,6 +94,8 @@ typedef struct YGStyle { | |||
float aspectRatio; | |||
} YGStyle; | |||
|
|||
typedef struct YGConfig { bool experimentalFeatures[YGExperimentalFeatureCount + 1]; } YGConfig; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you put this on multiple lines / run formatter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, but the layouter is formating it like that, I think we need to add a second value to layout it differently. As soon as you add float pointScale
this should be layouted as desired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hehe ok. The format config still needs some tweaking i guess. No problem
yoga/Yoga.h
Outdated
WIN_EXPORT void YGConfigFree(const YGConfigRef config); | ||
|
||
WIN_EXPORT void YGSetExperimentalFeatureEnabled(const YGConfigRef config, | ||
YGExperimentalFeature feature, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feature and enabled const please :)
yoga/Yoga.c
Outdated
inline bool YGIsExperimentalFeatureEnabled(YGExperimentalFeature feature) { | ||
return experimentalFeatures[feature]; | ||
inline bool YGIsExperimentalFeatureEnabled(const YGConfigRef config, | ||
YGExperimentalFeature feature) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const
yoga/Yoga.c
Outdated
void YGSetExperimentalFeatureEnabled(YGExperimentalFeature feature, bool enabled) { | ||
experimentalFeatures[feature] = enabled; | ||
void YGSetExperimentalFeatureEnabled(const YGConfigRef config, | ||
YGExperimentalFeature feature, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const
@emilsjolander has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
I'll most likely land this next week. It takes a while to integrate bigger changes like this into internal usages 👍 Thanks for the hard work! |
@emilsjolander fully reasonable! Do you also add the other missing languages? |
Yup! |
Awesome! |
Summary: Move configuration to new ```YGConfig``` and pass them down to CalculateLayout. See #418 . Adds ```YGConfigNew()``` + ```YGConfigFree```, and changed ```YGSetExperimentalFeatureEnabled``` to use the config. New function for calculation is ```YGNodeCalculateLayoutWithConfig```. Closes facebook/yoga#432 Reviewed By: astreet Differential Revision: D4611359 Pulled By: emilsjolander fbshipit-source-id: a1332f0e1b21cec02129dd021ee57408449e10b0
Summary: Move configuration to new ```YGConfig``` and pass them down to CalculateLayout. See #418 . Adds ```YGConfigNew()``` + ```YGConfigFree```, and changed ```YGSetExperimentalFeatureEnabled``` to use the config. New function for calculation is ```YGNodeCalculateLayoutWithConfig```. Closes facebook/yoga#432 Reviewed By: astreet Differential Revision: D4611359 Pulled By: emilsjolander fbshipit-source-id: a1332f0e1b21cec02129dd021ee57408449e10b0
Summary: Move configuration to new ```YGConfig``` and pass them down to CalculateLayout. See #418 . Adds ```YGConfigNew()``` + ```YGConfigFree```, and changed ```YGSetExperimentalFeatureEnabled``` to use the config. New function for calculation is ```YGNodeCalculateLayoutWithConfig```. Closes facebook/yoga#432 Reviewed By: astreet Differential Revision: D4611359 Pulled By: emilsjolander fbshipit-source-id: a1332f0e1b21cec02129dd021ee57408449e10b0
Move configuration to new
YGConfig
and pass them down to CalculateLayout. See #418 .Adds
YGConfigNew()
+YGConfigFree
, and changedYGSetExperimentalFeatureEnabled
to use the config.New function for calculation is
YGNodeCalculateLayoutWithConfig
.