From e75843516770cdc41ba02941af0264a036eb60e8 Mon Sep 17 00:00:00 2001 From: Luna Wei Date: Thu, 21 Feb 2019 19:51:45 -0800 Subject: [PATCH] Refactor the way UIManagerModule uses measure specs Summary: Instead of using measure specs to set certain yoga properties on the root node (like max width, auto width, specific width), use yoga's calculateLayout(width, height) instead. The measure specs will be stored in the shadow node. This allows us to remove duplicated code that processes the measure specs and allows us to remove other logic like the enableLayoutCalculation() method. This diff also removes MeasureSpecProvider. MeasureSpecProvider was originally introduced to pass previously measured view measure specs to the initial creation of the root shadow node, but it turns out that this is unnecessary. We can update the root layout specs from the root view instead. Reviewed By: mdvacca Differential Revision: D9729744 fbshipit-source-id: 79b0b27ca879758f5dc3fc7cc8a0d38856a6cc79 --- .../com/facebook/react/ReactRootView.java | 41 ++---------- .../com/facebook/react/bridge/UIManager.java | 3 +- .../react/fabric/FabricUIManager.java | 4 +- .../react/uimanager/ReactShadowNode.java | 8 +++ .../react/uimanager/ReactShadowNodeImpl.java | 24 ++++++- .../react/uimanager/UIImplementation.java | 63 ++++--------------- .../react/uimanager/UIManagerModule.java | 5 +- .../uimanager/common/MeasureSpecProvider.java | 20 ------ 8 files changed, 52 insertions(+), 116 deletions(-) delete mode 100644 ReactAndroid/src/main/java/com/facebook/react/uimanager/common/MeasureSpecProvider.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java b/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java index 9b25d19d50c..83bdf1d6e5b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java @@ -48,7 +48,6 @@ import com.facebook.react.uimanager.PixelUtil; import com.facebook.react.uimanager.RootView; import com.facebook.react.uimanager.UIManagerHelper; import com.facebook.react.uimanager.UIManagerModule; -import com.facebook.react.uimanager.common.MeasureSpecProvider; import com.facebook.react.uimanager.common.UIManagerType; import com.facebook.react.uimanager.events.EventDispatcher; import com.facebook.systrace.Systrace; @@ -66,7 +65,7 @@ import javax.annotation.Nullable; * subsequent touch events related to that gesture (in case when JS code wants to handle that * gesture). */ -public class ReactRootView extends FrameLayout implements RootView, MeasureSpecProvider { +public class ReactRootView extends FrameLayout implements RootView { /** * Listener interface for react root view events @@ -156,9 +155,7 @@ public class ReactRootView extends FrameLayout implements RootView, MeasureSpecP // Check if we were waiting for onMeasure to attach the root view. if (mReactInstanceManager != null && !mIsAttachedToInstance) { attachToReactInstanceManager(); - enableLayoutCalculation(); } else { - enableLayoutCalculation(); updateRootLayoutSpecs(mWidthMeasureSpec, mHeightMeasureSpec); } @@ -167,22 +164,6 @@ public class ReactRootView extends FrameLayout implements RootView, MeasureSpecP } } - @Override - public int getWidthMeasureSpec() { - if (!mWasMeasured && getLayoutParams() != null && getLayoutParams().width > 0) { - return MeasureSpec.makeMeasureSpec(getLayoutParams().width, MeasureSpec.EXACTLY); - } - return mWidthMeasureSpec; - } - - @Override - public int getHeightMeasureSpec() { - if (!mWasMeasured && getLayoutParams() != null && getLayoutParams().height > 0) { - return MeasureSpec.makeMeasureSpec(getLayoutParams().height, MeasureSpec.EXACTLY); - } - return mHeightMeasureSpec; - } - @Override public void onChildStartedNativeGesture(MotionEvent androidEvent) { if (mReactInstanceManager == null || !mIsAttachedToInstance || @@ -389,23 +370,6 @@ public class ReactRootView extends FrameLayout implements RootView, MeasureSpecP } } - private void enableLayoutCalculation() { - if (mReactInstanceManager == null) { - FLog.w( - ReactConstants.TAG, - "Unable to enable layout calculation for uninitialized ReactInstanceManager"); - return; - } - final ReactContext reactApplicationContext = mReactInstanceManager.getCurrentReactContext(); - if (reactApplicationContext != null) { - reactApplicationContext - .getCatalystInstance() - .getNativeModule(UIManagerModule.class) - .getUIImplementation() - .enableLayoutCalculationForRootNode(getRootViewTag()); - } - } - private void updateRootLayoutSpecs(final int widthMeasureSpec, final int heightMeasureSpec) { if (mReactInstanceManager == null) { FLog.w( @@ -487,6 +451,9 @@ public class ReactRootView extends FrameLayout implements RootView, MeasureSpecP return; } + if (mWasMeasured) { + updateRootLayoutSpecs(mWidthMeasureSpec, mHeightMeasureSpec); + } CatalystInstance catalystInstance = reactContext.getCatalystInstance(); WritableNativeMap appParams = new WritableNativeMap(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java index 15949bbce9f..f34ac2b2591 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java @@ -8,7 +8,6 @@ package com.facebook.react.bridge; import android.view.View; -import com.facebook.react.uimanager.common.MeasureSpecProvider; import javax.annotation.Nullable; public interface UIManager extends JSIModule, PerformanceCounter { @@ -16,7 +15,7 @@ public interface UIManager extends JSIModule, PerformanceCounter { /** * Registers a new root view. */ - int addRootView(final T rootView, WritableMap initialProps, @Nullable String initialUITemplate); + int addRootView(final T rootView, WritableMap initialProps, @Nullable String initialUITemplate); /** * Unregisters a new root view. 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 42fa686094c..2b545397275 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -56,7 +56,6 @@ import com.facebook.react.uimanager.ReactRootViewTagGenerator; import com.facebook.react.uimanager.ThemedReactContext; import com.facebook.react.uimanager.ViewManagerPropertyUpdater; import com.facebook.react.uimanager.ViewManagerRegistry; -import com.facebook.react.uimanager.common.MeasureSpecProvider; import com.facebook.react.uimanager.events.EventDispatcher; import com.facebook.systrace.Systrace; import java.util.ArrayList; @@ -131,7 +130,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { } @Override - public int addRootView( + public int addRootView( final T rootView, final WritableMap initialProps, final @Nullable String initialUITemplate) { final int rootTag = ReactRootViewTagGenerator.getNextRootViewTag(); ThemedReactContext reactContext = @@ -139,7 +138,6 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { mMountingManager.addRootView(rootTag, rootView); mReactContextForRootTag.put(rootTag, reactContext); mBinding.startSurface(rootTag, (NativeMap) initialProps); - updateRootLayoutSpecs(rootTag, rootView.getWidthMeasureSpec(), rootView.getHeightMeasureSpec()); if (initialUITemplate != null) { mBinding.renderTemplateToSurface(rootTag, initialUITemplate); } 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 885ab3dd2b3..7d3e43aac40 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNode.java @@ -148,6 +148,8 @@ public interface ReactShadowNode { void calculateLayout(); + void calculateLayout(float width, float height); + boolean hasNewLayout(); void markLayoutSeen(); @@ -346,4 +348,10 @@ public interface ReactShadowNode { boolean isMeasureDefined(); void dispose(); + + void setMeasureSpecs(int widthMeasureSpec, int heightMeasureSpec); + + Integer getWidthMeasureSpec(); + + Integer getHeightMeasureSpec(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNodeImpl.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNodeImpl.java index d1ba66c102f..5c44f6c4d97 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNodeImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNodeImpl.java @@ -81,6 +81,8 @@ public class ReactShadowNodeImpl implements ReactShadowNode private final float[] mPadding = new float[Spacing.ALL + 1]; private final boolean[] mPaddingIsPercent = new boolean[Spacing.ALL + 1]; private YogaNode mYogaNode; + private Integer mWidthMeasureSpec; + private Integer mHeightMeasureSpec; public ReactShadowNodeImpl() { mDefaultPadding = new Spacing(0); @@ -418,7 +420,12 @@ public class ReactShadowNodeImpl implements ReactShadowNode @Override public void calculateLayout() { - mYogaNode.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); + calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); + } + + @Override + public void calculateLayout(float width, float height) { + mYogaNode.calculateLayout(width, height); } @Override @@ -956,4 +963,19 @@ public class ReactShadowNodeImpl implements ReactShadowNode } } + @Override + public void setMeasureSpecs(int widthMeasureSpec, int heightMeasureSpec) { + mWidthMeasureSpec = widthMeasureSpec; + mHeightMeasureSpec = heightMeasureSpec; + } + + @Override + public Integer getWidthMeasureSpec() { + return mWidthMeasureSpec; + } + + @Override + public Integer getHeightMeasureSpec() { + return mHeightMeasureSpec; + } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java index d8fcc91a172..d9791957b70 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java @@ -6,10 +6,6 @@ */ package com.facebook.react.uimanager; -import static android.view.View.MeasureSpec.AT_MOST; -import static android.view.View.MeasureSpec.EXACTLY; -import static android.view.View.MeasureSpec.UNSPECIFIED; - import android.os.SystemClock; import android.view.View; import android.view.View.MeasureSpec; @@ -25,11 +21,11 @@ import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.bridge.WritableArray; import com.facebook.react.common.ReactConstants; import com.facebook.react.modules.i18nmanager.I18nUtil; -import com.facebook.react.uimanager.common.MeasureSpecProvider; import com.facebook.react.uimanager.debug.NotThreadSafeViewHierarchyUpdateDebugListener; import com.facebook.react.uimanager.events.EventDispatcher; import com.facebook.systrace.Systrace; import com.facebook.systrace.SystraceMessage; +import com.facebook.yoga.YogaConstants; import com.facebook.yoga.YogaDirection; import java.util.Arrays; import java.util.HashSet; @@ -47,7 +43,6 @@ public class UIImplementation { protected final EventDispatcher mEventDispatcher; protected final ReactApplicationContext mReactContext; protected final ShadowNodeRegistry mShadowNodeRegistry = new ShadowNodeRegistry(); - private final Set mMeasuredRootNodes = new HashSet<>(); private final ViewManagerRegistry mViewManagers; private final UIViewOperationQueue mOperationsQueue; private final NativeViewHierarchyOptimizer mNativeViewHierarchyOptimizer; @@ -162,49 +157,19 @@ public class UIImplementation { */ public void updateRootView( ReactShadowNode rootCSSNode, int widthMeasureSpec, int heightMeasureSpec) { - int widthMode = MeasureSpec.getMode(widthMeasureSpec); - int widthSize = MeasureSpec.getSize(widthMeasureSpec); - switch (widthMode) { - case EXACTLY: - rootCSSNode.setStyleWidth(widthSize); - break; - case AT_MOST: - rootCSSNode.setStyleMaxWidth(widthSize); - break; - case UNSPECIFIED: - rootCSSNode.setStyleWidthAuto(); - break; - } - - int heightMode = MeasureSpec.getMode(heightMeasureSpec); - int heightSize = MeasureSpec.getSize(heightMeasureSpec); - switch (heightMode) { - case EXACTLY: - rootCSSNode.setStyleHeight(heightSize); - break; - case AT_MOST: - rootCSSNode.setStyleMaxHeight(heightSize); - break; - case UNSPECIFIED: - rootCSSNode.setStyleHeightAuto(); - break; - } + rootCSSNode.setMeasureSpecs(widthMeasureSpec, heightMeasureSpec); } /** * Registers a root node with a given tag, size and ThemedReactContext and adds it to a node * registry. */ - public void registerRootView( + public void registerRootView( T rootView, int tag, ThemedReactContext context) { final ReactShadowNode rootCSSNode = createRootShadowNode(); rootCSSNode.setReactTag(tag); rootCSSNode.setThemedContext(context); - int widthMeasureSpec = rootView.getWidthMeasureSpec(); - int heightMeasureSpec = rootView.getHeightMeasureSpec(); - updateRootView(rootCSSNode, widthMeasureSpec, heightMeasureSpec); - context.runOnNativeModulesQueueThread(new Runnable() { @Override public void run() { @@ -685,7 +650,7 @@ public class UIImplementation { int tag = mShadowNodeRegistry.getRootTag(i); ReactShadowNode cssRoot = mShadowNodeRegistry.getNode(tag); - if (mMeasuredRootNodes.contains(tag)) { + if (cssRoot.getWidthMeasureSpec() != null && cssRoot.getHeightMeasureSpec() != null) { SystraceMessage.beginSection( Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "UIImplementation.notifyOnBeforeLayoutRecursive") @@ -846,7 +811,6 @@ public class UIImplementation { private void removeShadowNodeRecursive(ReactShadowNode nodeToRemove) { NativeViewHierarchyOptimizer.handleRemoveNode(nodeToRemove); mShadowNodeRegistry.removeNode(nodeToRemove.getReactTag()); - mMeasuredRootNodes.remove(nodeToRemove.getReactTag()); for (int i = nodeToRemove.getChildCount() - 1; i >= 0; i--) { removeShadowNodeRecursive(nodeToRemove.getChildAt(i)); } @@ -955,7 +919,15 @@ public class UIImplementation { .flush(); long startTime = SystemClock.uptimeMillis(); try { - cssRoot.calculateLayout(); + int widthSpec = cssRoot.getWidthMeasureSpec(); + int heightSpec = cssRoot.getHeightMeasureSpec(); + cssRoot.calculateLayout( + MeasureSpec.getMode(widthSpec) == MeasureSpec.UNSPECIFIED + ? YogaConstants.UNDEFINED + : MeasureSpec.getSize(widthSpec), + MeasureSpec.getMode(heightSpec) == MeasureSpec.UNSPECIFIED + ? YogaConstants.UNDEFINED + : MeasureSpec.getSize(heightSpec)); } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); mLastCalculateLayoutTime = SystemClock.uptimeMillis() - startTime; @@ -1029,15 +1001,6 @@ public class UIImplementation { return rootTag; } - /** - * Enables Layout calculation for a Root node that has been measured. - * - * @param rootViewTag {@link int} Tag of the root node - */ - public void enableLayoutCalculationForRootNode(int rootViewTag) { - this.mMeasuredRootNodes.add(rootViewTag); - } - public void setLayoutUpdateListener(LayoutUpdateListener listener) { mLayoutUpdateListener = listener; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java index 5af84191c36..cc51ec89a48 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java @@ -39,7 +39,6 @@ import com.facebook.react.common.MapBuilder; import com.facebook.react.common.ReactConstants; import com.facebook.react.common.annotations.VisibleForTesting; import com.facebook.react.module.annotations.ReactModule; -import com.facebook.react.uimanager.common.MeasureSpecProvider; import com.facebook.react.uimanager.common.ViewUtil; import com.facebook.react.uimanager.debug.NotThreadSafeViewHierarchyUpdateDebugListener; import com.facebook.react.uimanager.events.EventDispatcher; @@ -377,7 +376,7 @@ public class UIManagerModule extends ReactContextBaseJavaModule return mUIImplementation.getProfiledBatchPerfCounters(); } - public int addRootView( + public int addRootView( final T rootView) { return addRootView(rootView, null, null); } @@ -409,7 +408,7 @@ public class UIManagerModule extends ReactContextBaseJavaModule *

TODO(6242243): Make addRootView thread safe NB: this method is horribly not-thread-safe. */ @Override - public int addRootView( + public int addRootView( final T rootView, WritableMap initialProps, @Nullable String initialUITemplate) { Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "UIManagerModule.addRootView"); final int tag = ReactRootViewTagGenerator.getNextRootViewTag(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/common/MeasureSpecProvider.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/common/MeasureSpecProvider.java deleted file mode 100644 index fc9a8fb322c..00000000000 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/common/MeasureSpecProvider.java +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright (c) Facebook, Inc. and its affiliates. - -// This source code is licensed under the MIT license found in the -// LICENSE file in the root directory of this source tree. - -package com.facebook.react.uimanager.common; - -import android.view.View; - -/** - * Interface for a {@link View} subclass that provides the width and height measure specs from its - * measure pass. This is currently used to re-measure the root view by reusing the specs for yoga - * layout calculations. - */ -public interface MeasureSpecProvider { - - int getWidthMeasureSpec(); - - int getHeightMeasureSpec(); -}