From 4a1d53267465ca642db74ad6df69744fdd431b08 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Fri, 3 May 2019 12:24:41 -0700 Subject: [PATCH] Convert FabricUIManager.measure params to floats Summary: Convert FabricUIManager.measure params to floats. Currently we convert parameters to ints across the JNI boundary, and then back to floats several times in Java. This is unnecessary and actually makes measurements trickier. The new implementation uses floats across the JNI boundary and uses Float.POSITIVE_INFINITY to represent unconstrained values, which is consistent with Fabric C++ as well. Reviewed By: shergin, mdvacca Differential Revision: D15176108 fbshipit-source-id: cf849b3773007637f059279460163872f300a4aa --- .../react/fabric/FabricUIManager.java | 8 +++--- .../mounting/LayoutMetricsConversions.java | 26 +++++++------------ .../android/SliderMeasurementsManager.cpp | 20 ++++++-------- .../platform/android/TextLayoutManager.cpp | 20 ++++++-------- 4 files changed, 30 insertions(+), 44 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index cd0850b8c75..55419a95b08 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -280,10 +280,10 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { String componentName, ReadableMap localData, ReadableMap props, - int minWidth, - int maxWidth, - int minHeight, - int maxHeight) { + float minWidth, + float maxWidth, + float minHeight, + float maxHeight) { return mMountingManager.measure( mReactApplicationContext, diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/LayoutMetricsConversions.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/LayoutMetricsConversions.java index 01365f878f1..c8641f0cccc 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/LayoutMetricsConversions.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/LayoutMetricsConversions.java @@ -14,9 +14,6 @@ import com.facebook.yoga.YogaMeasureMode; public class LayoutMetricsConversions { - // Represents the Layout constraint mode "undefined" from React side. - public static final int REACT_CONSTRAINT_UNDEFINED = -2147483648; - public static float getMinSize(int viewMeasureSpec) { int mode = View.MeasureSpec.getMode(viewMeasureSpec); int size = View.MeasureSpec.getSize(viewMeasureSpec); @@ -28,30 +25,27 @@ public class LayoutMetricsConversions { int mode = View.MeasureSpec.getMode(viewMeasureSpec); int size = View.MeasureSpec.getSize(viewMeasureSpec); - return mode == View.MeasureSpec.UNSPECIFIED ? REACT_CONSTRAINT_UNDEFINED : size; + // Infinity represents an "unconstrained" size + return mode == View.MeasureSpec.UNSPECIFIED ? Float.POSITIVE_INFINITY : size; } public static float getYogaSize(float minSize, float maxSize) { - float yogaSize; if (minSize == maxSize) { - yogaSize = PixelUtil.toPixelFromDIP(maxSize); - } else if (maxSize == REACT_CONSTRAINT_UNDEFINED) { - yogaSize = 0; + return PixelUtil.toPixelFromDIP(maxSize); + } else if (Float.isInfinite(maxSize)) { + return Float.POSITIVE_INFINITY; } else { - yogaSize = PixelUtil.toPixelFromDIP(maxSize); + return PixelUtil.toPixelFromDIP(maxSize); } - return yogaSize; } public static YogaMeasureMode getYogaMeasureMode(float minSize, float maxSize) { - YogaMeasureMode yogaMeasureMode; if (minSize == maxSize) { - yogaMeasureMode = YogaMeasureMode.EXACTLY; - } else if (maxSize == REACT_CONSTRAINT_UNDEFINED) { - yogaMeasureMode = YogaMeasureMode.UNDEFINED; + return YogaMeasureMode.EXACTLY; + } else if (Float.isInfinite(maxSize)) { + return YogaMeasureMode.UNDEFINED; } else { - yogaMeasureMode = YogaMeasureMode.AT_MOST; + return YogaMeasureMode.AT_MOST; } - return yogaMeasureMode; } } diff --git a/ReactCommon/fabric/components/slider/platform/android/SliderMeasurementsManager.cpp b/ReactCommon/fabric/components/slider/platform/android/SliderMeasurementsManager.cpp index 9f7cf72cc04..a087b29f1ad 100644 --- a/ReactCommon/fabric/components/slider/platform/android/SliderMeasurementsManager.cpp +++ b/ReactCommon/fabric/components/slider/platform/android/SliderMeasurementsManager.cpp @@ -35,17 +35,13 @@ Size SliderMeasurementsManager::measure( jstring, ReadableMap::javaobject, ReadableMap::javaobject, - jint, - jint, - jint, - jint)>("measure"); + jfloat, + jfloat, + jfloat, + jfloat)>("measure"); auto minimumSize = layoutConstraints.minimumSize; auto maximumSize = layoutConstraints.maximumSize; - int minWidth = (int)minimumSize.width; - int minHeight = (int)minimumSize.height; - int maxWidth = (int)maximumSize.width; - int maxHeight = (int)maximumSize.height; local_ref componentName = make_jstring("RCTSlider"); @@ -54,10 +50,10 @@ Size SliderMeasurementsManager::measure( componentName.get(), nullptr, nullptr, - minWidth, - maxWidth, - minHeight, - maxHeight)); + minimumSize.width, + maximumSize.width, + minimumSize.height, + maximumSize.height)); std::lock_guard lock(mutex_); cachedMeasurement_ = measurement; diff --git a/ReactCommon/fabric/textlayoutmanager/platform/android/TextLayoutManager.cpp b/ReactCommon/fabric/textlayoutmanager/platform/android/TextLayoutManager.cpp index 32977538ca2..f7a28f58aea 100644 --- a/ReactCommon/fabric/textlayoutmanager/platform/android/TextLayoutManager.cpp +++ b/ReactCommon/fabric/textlayoutmanager/platform/android/TextLayoutManager.cpp @@ -36,17 +36,13 @@ Size TextLayoutManager::measure( jstring, ReadableMap::javaobject, ReadableMap::javaobject, - jint, - jint, - jint, - jint)>("measure"); + jfloat, + jfloat, + jfloat, + jfloat)>("measure"); auto minimumSize = layoutConstraints.minimumSize; auto maximumSize = layoutConstraints.maximumSize; - int minWidth = (int)minimumSize.width; - int minHeight = (int)minimumSize.height; - int maxWidth = (int)maximumSize.width; - int maxHeight = (int)maximumSize.height; local_ref componentName = make_jstring("RCTText"); local_ref attributedStringRNM = @@ -63,10 +59,10 @@ Size TextLayoutManager::measure( componentName.get(), attributedStringRM.get(), paragraphAttributesRM.get(), - minWidth, - maxWidth, - minHeight, - maxHeight)); + minimumSize.width, + maximumSize.width, + minimumSize.height, + maximumSize.height)); } } // namespace react