From 992e37c8bcdb13c89cb680e385a0fb344137d0fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20W=C3=B6hrl?= Date: Fri, 28 Apr 2017 06:13:51 -0700 Subject: [PATCH] Fix sizing of non strech items Summary: Fixes the sizing of items so that under most scenarios it calcultes its height by it's content for non exact measurings. This introduces a new useLegacyStretchBehaviour flag on the config to opt out of this change as it is breaking. See facebook/yoga#505 Closes https://github.com/facebook/yoga/pull/506 Reviewed By: astreet Differential Revision: D4954016 Pulled By: emilsjolander fbshipit-source-id: d28bd5d174cd76951fb94df85e3b0cfab7f81ff7 --- React/Views/RCTShadowView.m | 2 +- .../react/uimanager/ReactShadowNode.java | 1 + .../java/com/facebook/yoga/YogaConfig.java | 11 ++++++++++ .../yoga/YogaExperimentalFeature.java | 4 +--- .../jni/first-party/yogajni/jni/YGJNI.cpp | 6 +++++ ReactCommon/yoga/yoga/YGEnums.c | 2 -- ReactCommon/yoga/yoga/YGEnums.h | 3 +-- ReactCommon/yoga/yoga/Yoga.c | 22 ++++++++++++------- ReactCommon/yoga/yoga/Yoga.h | 5 +++++ 9 files changed, 40 insertions(+), 16 deletions(-) diff --git a/React/Views/RCTShadowView.m b/React/Views/RCTShadowView.m index c8e926ca1..15d2a9f27 100644 --- a/React/Views/RCTShadowView.m +++ b/React/Views/RCTShadowView.m @@ -54,6 +54,7 @@ typedef NS_ENUM(unsigned int, meta_prop_t) { yogaConfig = YGConfigNew(); // Turnig off pixel rounding. YGConfigSetPointScaleFactor(yogaConfig, 0.0); + YGConfigSetUseLegacyStretchBehaviour(yogaConfig, true); }); return yogaConfig; } @@ -347,7 +348,6 @@ static void RCTProcessMetaPropsBorder(const YGValue metaProps[META_PROP_COUNT], _reactSubviews = [NSMutableArray array]; _yogaNode = YGNodeNewWithConfig([[self class] yogaConfig]); - YGNodeSetContext(_yogaNode, (__bridge void *)self); YGNodeSetPrintFunc(_yogaNode, RCTPrint); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNode.java index 77603073e..afbf01ef3 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNode.java @@ -87,6 +87,7 @@ public class ReactShadowNode { if (sYogaConfig == null) { sYogaConfig = new YogaConfig(); sYogaConfig.setPointScaleFactor(0f); + sYogaConfig.setUseLegacyStretchBehaviour(true); } if (node == null) { node = new YogaNode(sYogaConfig); diff --git a/ReactAndroid/src/main/java/com/facebook/yoga/YogaConfig.java b/ReactAndroid/src/main/java/com/facebook/yoga/YogaConfig.java index 8f86b0ad4..6cad34bae 100644 --- a/ReactAndroid/src/main/java/com/facebook/yoga/YogaConfig.java +++ b/ReactAndroid/src/main/java/com/facebook/yoga/YogaConfig.java @@ -56,4 +56,15 @@ public class YogaConfig { public void setPointScaleFactor(float pixelsInPoint) { jni_YGConfigSetPointScaleFactor(mNativePointer, pixelsInPoint); } + + private native void jni_YGConfigSetUseLegacyStretchBehaviour(long nativePointer, boolean useLegacyStretchBehaviour); + + /** + * Yoga previously had an error where containers would take the maximum space possible instead of the minimum + * like they are supposed to. In practice this resulted in implicit behaviour similar to align-self: stretch; + * Because this was such a long-standing bug we must allow legacy users to switch back to this behaviour. + */ + public void setUseLegacyStretchBehaviour(boolean useLegacyStretchBehaviour) { + jni_YGConfigSetUseLegacyStretchBehaviour(mNativePointer, useLegacyStretchBehaviour); + } } diff --git a/ReactAndroid/src/main/java/com/facebook/yoga/YogaExperimentalFeature.java b/ReactAndroid/src/main/java/com/facebook/yoga/YogaExperimentalFeature.java index 178ece402..3d3885156 100644 --- a/ReactAndroid/src/main/java/com/facebook/yoga/YogaExperimentalFeature.java +++ b/ReactAndroid/src/main/java/com/facebook/yoga/YogaExperimentalFeature.java @@ -13,8 +13,7 @@ import com.facebook.proguard.annotations.DoNotStrip; @DoNotStrip public enum YogaExperimentalFeature { - WEB_FLEX_BASIS(0), - MIN_FLEX_FIX(1); + WEB_FLEX_BASIS(0); private int mIntValue; @@ -29,7 +28,6 @@ public enum YogaExperimentalFeature { public static YogaExperimentalFeature fromInt(int value) { switch (value) { case 0: return WEB_FLEX_BASIS; - case 1: return MIN_FLEX_FIX; default: throw new IllegalArgumentException("Unknown enum value: " + value); } } diff --git a/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.cpp b/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.cpp index fd9e60812..ca0af8c02 100644 --- a/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.cpp +++ b/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.cpp @@ -408,6 +408,11 @@ void jni_YGConfigSetPointScaleFactor(alias_ref, jlong nativePointer, jf YGConfigSetPointScaleFactor(config, pixelsInPoint); } +void jni_YGConfigSetUseLegacyStretchBehaviour(alias_ref, jlong nativePointer, jboolean useLegacyStretchBehaviour) { + const YGConfigRef config = _jlong2YGConfigRef(nativePointer); + YGConfigSetUseLegacyStretchBehaviour(config, useLegacyStretchBehaviour); +} + jint jni_YGNodeGetInstanceCount(alias_ref clazz) { return YGNodeGetInstanceCount(); } @@ -504,6 +509,7 @@ jint JNI_OnLoad(JavaVM *vm, void *) { YGMakeNativeMethod(jni_YGConfigSetExperimentalFeatureEnabled), YGMakeNativeMethod(jni_YGConfigSetUseWebDefaults), YGMakeNativeMethod(jni_YGConfigSetPointScaleFactor), + YGMakeNativeMethod(jni_YGConfigSetUseLegacyStretchBehaviour), }); }); } diff --git a/ReactCommon/yoga/yoga/YGEnums.c b/ReactCommon/yoga/yoga/YGEnums.c index c3e4521c0..85e8fcf0f 100644 --- a/ReactCommon/yoga/yoga/YGEnums.c +++ b/ReactCommon/yoga/yoga/YGEnums.c @@ -91,8 +91,6 @@ const char *YGExperimentalFeatureToString(const YGExperimentalFeature value){ switch(value){ case YGExperimentalFeatureWebFlexBasis: return "web-flex-basis"; - case YGExperimentalFeatureMinFlexFix: - return "min-flex-fix"; } return "unknown"; } diff --git a/ReactCommon/yoga/yoga/YGEnums.h b/ReactCommon/yoga/yoga/YGEnums.h index 972c5210f..db64e3d73 100644 --- a/ReactCommon/yoga/yoga/YGEnums.h +++ b/ReactCommon/yoga/yoga/YGEnums.h @@ -62,10 +62,9 @@ typedef YG_ENUM_BEGIN(YGEdge) { } YG_ENUM_END(YGEdge); WIN_EXPORT const char *YGEdgeToString(const YGEdge value); -#define YGExperimentalFeatureCount 2 +#define YGExperimentalFeatureCount 1 typedef YG_ENUM_BEGIN(YGExperimentalFeature) { YGExperimentalFeatureWebFlexBasis, - YGExperimentalFeatureMinFlexFix, } YG_ENUM_END(YGExperimentalFeature); WIN_EXPORT const char *YGExperimentalFeatureToString(const YGExperimentalFeature value); diff --git a/ReactCommon/yoga/yoga/Yoga.c b/ReactCommon/yoga/yoga/Yoga.c index 2ec842875..06bbc952c 100644 --- a/ReactCommon/yoga/yoga/Yoga.c +++ b/ReactCommon/yoga/yoga/Yoga.c @@ -97,6 +97,7 @@ typedef struct YGStyle { typedef struct YGConfig { bool experimentalFeatures[YGExperimentalFeatureCount + 1]; bool useWebDefaults; + bool useLegacyStretchBehaviour; float pointScaleFactor; } YGConfig; @@ -202,7 +203,6 @@ static YGNode gYGNodeDefaults = { static YGConfig gYGConfigDefaults = { .experimentalFeatures = { - [YGExperimentalFeatureMinFlexFix] = false, [YGExperimentalFeatureWebFlexBasis] = false, }, .useWebDefaults = false, @@ -2199,23 +2199,25 @@ static void YGNodelayoutImpl(const YGNodeRef node, // If the main dimension size isn't known, it is computed based on // the line length, so there's no more space left to distribute. + bool sizeBasedOnContent = false; // If we don't measure with exact main dimension we want to ensure we don't violate min and max if (measureModeMainDim != YGMeasureModeExactly) { if (!YGFloatIsUndefined(minInnerMainDim) && sizeConsumedOnCurrentLine < minInnerMainDim) { availableInnerMainDim = minInnerMainDim; } else if (!YGFloatIsUndefined(maxInnerMainDim) && sizeConsumedOnCurrentLine > maxInnerMainDim) { availableInnerMainDim = maxInnerMainDim; - } else if (YGConfigIsExperimentalFeatureEnabled(node->config, YGExperimentalFeatureMinFlexFix) && - (totalFlexGrowFactors == 0 || YGResolveFlexGrow(node) == 0)) { - // TODO: this needs to be moved out of experimental feature, as this is legitimate fix - // If we don't have any children to flex or we can't flex the node itself, - // space we've used is all space we need - availableInnerMainDim = sizeConsumedOnCurrentLine; + } else { + if (!node->config->useLegacyStretchBehaviour && (totalFlexGrowFactors == 0 || YGResolveFlexGrow(node) == 0)) { + // If we don't have any children to flex or we can't flex the node itself, + // space we've used is all space we need + availableInnerMainDim = sizeConsumedOnCurrentLine; + } + sizeBasedOnContent = true; } } float remainingFreeSpace = 0; - if (!YGFloatIsUndefined(availableInnerMainDim)) { + if ((!sizeBasedOnContent || node->config->useLegacyStretchBehaviour) && !YGFloatIsUndefined(availableInnerMainDim)) { remainingFreeSpace = availableInnerMainDim - sizeConsumedOnCurrentLine; } else if (sizeConsumedOnCurrentLine < 0) { // availableInnerMainDim is indefinite which means the node is being sized @@ -3427,6 +3429,10 @@ void YGConfigSetUseWebDefaults(const YGConfigRef config, const bool enabled) { config->useWebDefaults = enabled; } +void YGConfigSetUseLegacyStretchBehaviour(const YGConfigRef config, const bool useLegacyStretchBehaviour) { + config->useLegacyStretchBehaviour = useLegacyStretchBehaviour; +} + bool YGConfigGetUseWebDefaults(const YGConfigRef config) { return config->useWebDefaults; } diff --git a/ReactCommon/yoga/yoga/Yoga.h b/ReactCommon/yoga/yoga/Yoga.h index 4a6de72bc..a26a1e9ad 100644 --- a/ReactCommon/yoga/yoga/Yoga.h +++ b/ReactCommon/yoga/yoga/Yoga.h @@ -225,6 +225,11 @@ WIN_EXPORT void YGLog(YGLogLevel level, const char *message, ...); // If you want to avoid rounding - set PointScaleFactor to 0 WIN_EXPORT void YGConfigSetPointScaleFactor(const YGConfigRef config, const float pixelsInPoint); +// Yoga previously had an error where containers would take the maximum space possible instead of the minimum +// like they are supposed to. In practice this resulted in implicit behaviour similar to align-self: stretch; +// Because this was such a long-standing bug we must allow legacy users to switch back to this behaviour. +WIN_EXPORT void YGConfigSetUseLegacyStretchBehaviour(const YGConfigRef config, const bool useLegacyStretchBehaviour); + // YGConfig WIN_EXPORT YGConfigRef YGConfigNew(void); WIN_EXPORT void YGConfigFree(const YGConfigRef config);