diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java index f03cc8a460fbd5..0591f5dbb7e14c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java @@ -17,17 +17,20 @@ import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.LifecycleEventListener; -import com.facebook.react.bridge.OnBatchCompleteListener; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReactContextBaseJavaModule; import com.facebook.react.bridge.ReactMethod; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.WritableMap; +import com.facebook.react.common.annotations.VisibleForTesting; import com.facebook.react.module.annotations.ReactModule; import com.facebook.react.modules.core.DeviceEventManagerModule; import com.facebook.react.modules.core.ReactChoreographer; import com.facebook.react.uimanager.GuardedFrameCallback; +import com.facebook.react.uimanager.NativeViewHierarchyManager; +import com.facebook.react.uimanager.UIBlock; import com.facebook.react.uimanager.UIManagerModule; +import com.facebook.react.uimanager.UIManagerModuleListener; /** * Module that exposes interface for creating and managing animated nodes on the "native" side. @@ -72,7 +75,7 @@ */ @ReactModule(name = NativeAnimatedModule.NAME) public class NativeAnimatedModule extends ReactContextBaseJavaModule implements - OnBatchCompleteListener, LifecycleEventListener { + LifecycleEventListener, UIManagerModuleListener { protected static final String NAME = "NativeAnimatedModule"; @@ -80,11 +83,10 @@ private interface UIThreadOperation { void execute(NativeAnimatedNodesManager animatedNodesManager); } - private final Object mOperationsCopyLock = new Object(); private final GuardedFrameCallback mAnimatedFrameCallback; private final ReactChoreographer mReactChoreographer; private ArrayList mOperations = new ArrayList<>(); - private volatile @Nullable ArrayList mReadyOperations = null; + private ArrayList mPreOperations = new ArrayList<>(); private @Nullable NativeAnimatedNodesManager mNodesManager; @@ -95,26 +97,9 @@ public NativeAnimatedModule(ReactApplicationContext reactContext) { mAnimatedFrameCallback = new GuardedFrameCallback(reactContext) { @Override protected void doFrameGuarded(final long frameTimeNanos) { - if (mNodesManager == null) { - UIManagerModule uiManager = getReactApplicationContext() - .getNativeModule(UIManagerModule.class); - mNodesManager = new NativeAnimatedNodesManager(uiManager); - } - - ArrayList operations; - synchronized (mOperationsCopyLock) { - operations = mReadyOperations; - mReadyOperations = null; - } - - if (operations != null) { - for (int i = 0, size = operations.size(); i < size; i++) { - operations.get(i).execute(mNodesManager); - } - } - - if (mNodesManager.hasActiveAnimations()) { - mNodesManager.runUpdates(frameTimeNanos); + NativeAnimatedNodesManager nodesManager = getNodesManager(); + if (nodesManager.hasActiveAnimations()) { + nodesManager.runUpdates(frameTimeNanos); } // TODO: Would be great to avoid adding this callback in case there are no active animations @@ -130,7 +115,10 @@ protected void doFrameGuarded(final long frameTimeNanos) { @Override public void initialize() { - getReactApplicationContext().addLifecycleEventListener(this); + ReactApplicationContext reactCtx = getReactApplicationContext(); + UIManagerModule uiManager = reactCtx.getNativeModule(UIManagerModule.class); + reactCtx.addLifecycleEventListener(this); + uiManager.addUIManagerListener(this); } @Override @@ -139,24 +127,32 @@ public void onHostResume() { } @Override - public void onBatchComplete() { - // Note: The order of executing onBatchComplete handler (especially in terms of onBatchComplete - // from the UIManagerModule) doesn't matter as we only enqueue operations for the UI thread to - // be executed from here. Thanks to ReactChoreographer all the operations from here are going - // to be executed *after* all the operations enqueued by UIManager as the callback type that we - // use for ReactChoreographer (CallbackType.NATIVE_ANIMATED_MODULE) is run after callbacks that - // UIManager uses. - ArrayList operations = mOperations.isEmpty() ? null : mOperations; - if (operations != null) { - mOperations = new ArrayList<>(); - synchronized (mOperationsCopyLock) { - if (mReadyOperations == null) { - mReadyOperations = operations; - } else { - mReadyOperations.addAll(operations); + public void willDispatchViewUpdates(final UIManagerModule uiManager) { + if (mOperations.isEmpty() && mPreOperations.isEmpty()) { + return; + } + final ArrayList preOperations = mPreOperations; + final ArrayList operations = mOperations; + mPreOperations = new ArrayList<>(); + mOperations = new ArrayList<>(); + uiManager.prependUIBlock(new UIBlock() { + @Override + public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) { + NativeAnimatedNodesManager nodesManager = getNodesManager(); + for (UIThreadOperation operation : preOperations) { + operation.execute(nodesManager); } } - } + }); + uiManager.addUIBlock(new UIBlock() { + @Override + public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) { + NativeAnimatedNodesManager nodesManager = getNodesManager(); + for (UIThreadOperation operation : operations) { + operation.execute(nodesManager); + } + } + }); } @Override @@ -174,6 +170,15 @@ public String getName() { return NAME; } + private NativeAnimatedNodesManager getNodesManager() { + if (mNodesManager == null) { + UIManagerModule uiManager = getReactApplicationContext().getNativeModule(UIManagerModule.class); + mNodesManager = new NativeAnimatedNodesManager(uiManager); + } + + return mNodesManager; + } + private void clearFrameCallback() { Assertions.assertNotNull(mReactChoreographer).removeFrameCallback( ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE, @@ -186,6 +191,11 @@ private void enqueueFrameCallback() { mAnimatedFrameCallback); } + @VisibleForTesting + public void setNodesManager(NativeAnimatedNodesManager nodesManager) { + mNodesManager = nodesManager; + } + @ReactMethod public void createAnimatedNode(final int tag, final ReadableMap config) { mOperations.add(new UIThreadOperation() { @@ -336,6 +346,12 @@ public void execute(NativeAnimatedNodesManager animatedNodesManager) { @ReactMethod public void disconnectAnimatedNodeFromView(final int animatedNodeTag, final int viewTag) { + mPreOperations.add(new UIThreadOperation() { + @Override + public void execute(NativeAnimatedNodesManager animatedNodesManager) { + animatedNodesManager.restoreDefaultValues(animatedNodeTag, viewTag); + } + }); mOperations.add(new UIThreadOperation() { @Override public void execute(NativeAnimatedNodesManager animatedNodesManager) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java index 9ed40b82368147..8f9694c445c13e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java @@ -92,7 +92,7 @@ public void createAnimatedNode(int tag, ReadableMap config) { } else if ("value".equals(type)) { node = new ValueAnimatedNode(config); } else if ("props".equals(type)) { - node = new PropsAnimatedNode(config, this); + node = new PropsAnimatedNode(config, this, mUIImplementation); } else if ("interpolation".equals(type)) { node = new InterpolationAnimatedNode(config); } else if ("addition".equals(type)) { @@ -289,11 +289,7 @@ public void connectAnimatedNodeToView(int animatedNodeTag, int viewTag) { "of type " + PropsAnimatedNode.class.getName()); } PropsAnimatedNode propsAnimatedNode = (PropsAnimatedNode) node; - if (propsAnimatedNode.mConnectedViewTag != -1) { - throw new JSApplicationIllegalArgumentException("Animated node " + animatedNodeTag + " is " + - "already attached to a view"); - } - propsAnimatedNode.mConnectedViewTag = viewTag; + propsAnimatedNode.connectToView(viewTag); mUpdatedNodes.put(animatedNodeTag, node); } @@ -308,11 +304,24 @@ public void disconnectAnimatedNodeFromView(int animatedNodeTag, int viewTag) { "of type " + PropsAnimatedNode.class.getName()); } PropsAnimatedNode propsAnimatedNode = (PropsAnimatedNode) node; - if (propsAnimatedNode.mConnectedViewTag != viewTag) { - throw new JSApplicationIllegalArgumentException("Attempting to disconnect view that has " + - "not been connected with the given animated node"); + propsAnimatedNode.disconnectFromView(viewTag); + } + + public void restoreDefaultValues(int animatedNodeTag, int viewTag) { + AnimatedNode node = mAnimatedNodes.get(animatedNodeTag); + // Restoring default values needs to happen before UIManager operations so it is + // possible the node hasn't been created yet if it is being connected and + // disconnected in the same batch. In that case we don't need to restore + // default values since it will never actually update the view. + if (node == null) { + return; } - propsAnimatedNode.mConnectedViewTag = -1; + if (!(node instanceof PropsAnimatedNode)) { + throw new JSApplicationIllegalArgumentException("Animated node connected to view should be" + + "of type " + PropsAnimatedNode.class.getName()); + } + PropsAnimatedNode propsAnimatedNode = (PropsAnimatedNode) node; + propsAnimatedNode.restoreDefaultValues(); } public void addAnimatedEventToView(int viewTag, String eventName, ReadableMap eventMapping) { @@ -513,7 +522,7 @@ private void updateNodes(List nodes) { if (nextNode instanceof PropsAnimatedNode) { // Send property updates to native view manager try { - ((PropsAnimatedNode) nextNode).updateView(mUIImplementation); + ((PropsAnimatedNode) nextNode).updateView(); } catch (IllegalViewOperationException e) { // An exception is thrown if the view hasn't been created yet. This can happen because views are // created in batches. If this particular view didn't make it into a batch yet, the view won't diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/PropsAnimatedNode.java b/ReactAndroid/src/main/java/com/facebook/react/animated/PropsAnimatedNode.java index de5c3e6f0278e9..036f88435cc21e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/PropsAnimatedNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/PropsAnimatedNode.java @@ -9,6 +9,7 @@ package com.facebook.react.animated; +import com.facebook.react.bridge.JSApplicationIllegalArgumentException; import com.facebook.react.bridge.JavaOnlyMap; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.ReadableMapKeySetIterator; @@ -27,47 +28,78 @@ */ /*package*/ class PropsAnimatedNode extends AnimatedNode { - /*package*/ int mConnectedViewTag = -1; - + private int mConnectedViewTag = -1; private final NativeAnimatedNodesManager mNativeAnimatedNodesManager; - private final Map mPropMapping; + private final UIImplementation mUIImplementation; + private final Map mPropNodeMapping; + // This is the backing map for `mDiffMap` we can mutate this to update it instead of having to + // create a new one for each update. + private final JavaOnlyMap mPropMap; + private final ReactStylesDiffMap mDiffMap; - PropsAnimatedNode(ReadableMap config, NativeAnimatedNodesManager nativeAnimatedNodesManager) { + PropsAnimatedNode(ReadableMap config, NativeAnimatedNodesManager nativeAnimatedNodesManager, UIImplementation uiImplementation) { ReadableMap props = config.getMap("props"); ReadableMapKeySetIterator iter = props.keySetIterator(); - mPropMapping = new HashMap<>(); + mPropNodeMapping = new HashMap<>(); while (iter.hasNextKey()) { String propKey = iter.nextKey(); int nodeIndex = props.getInt(propKey); - mPropMapping.put(propKey, nodeIndex); + mPropNodeMapping.put(propKey, nodeIndex); } + mPropMap = new JavaOnlyMap(); + mDiffMap = new ReactStylesDiffMap(mPropMap); mNativeAnimatedNodesManager = nativeAnimatedNodesManager; + mUIImplementation = uiImplementation; + } + + public void connectToView(int viewTag) { + if (mConnectedViewTag != -1) { + throw new JSApplicationIllegalArgumentException("Animated node " + mTag + " is " + + "already attached to a view"); + } + mConnectedViewTag = viewTag; + } + + public void disconnectFromView(int viewTag) { + if (mConnectedViewTag != viewTag) { + throw new JSApplicationIllegalArgumentException("Attempting to disconnect view that has " + + "not been connected with the given animated node"); + } + + mConnectedViewTag = -1; } - public final void updateView(UIImplementation uiImplementation) { + public void restoreDefaultValues() { + ReadableMapKeySetIterator it = mPropMap.keySetIterator(); + while(it.hasNextKey()) { + mPropMap.putNull(it.nextKey()); + } + + mUIImplementation.synchronouslyUpdateViewOnUIThread( + mConnectedViewTag, + mDiffMap); + } + + public final void updateView() { if (mConnectedViewTag == -1) { - throw new IllegalStateException("Node has not been attached to a view"); + return; } - JavaOnlyMap propsMap = new JavaOnlyMap(); - for (Map.Entry entry : mPropMapping.entrySet()) { + for (Map.Entry entry : mPropNodeMapping.entrySet()) { @Nullable AnimatedNode node = mNativeAnimatedNodesManager.getNodeById(entry.getValue()); if (node == null) { throw new IllegalArgumentException("Mapped property node does not exists"); } else if (node instanceof StyleAnimatedNode) { - ((StyleAnimatedNode) node).collectViewUpdates(propsMap); + ((StyleAnimatedNode) node).collectViewUpdates(mPropMap); } else if (node instanceof ValueAnimatedNode) { - propsMap.putDouble(entry.getKey(), ((ValueAnimatedNode) node).getValue()); + mPropMap.putDouble(entry.getKey(), ((ValueAnimatedNode) node).getValue()); } else { throw new IllegalArgumentException("Unsupported type of node used in property node " + node.getClass()); } } - // TODO: Reuse propsMap and stylesDiffMap objects - note that in subsequent animation steps - // for a given node most of the time we will be creating the same set of props (just with - // different values). We can take advantage on that and optimize the way we allocate property - // maps (we also know that updating view props doesn't retain a reference to the styles object). - uiImplementation.synchronouslyUpdateViewOnUIThread( + + mUIImplementation.synchronouslyUpdateViewOnUIThread( mConnectedViewTag, - new ReactStylesDiffMap(propsMap)); + mDiffMap); } } 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 0a90afd383f8db..95c3732c9b23ce 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java @@ -946,6 +946,10 @@ public void addUIBlock(UIBlock block) { mOperationsQueue.enqueueUIBlock(block); } + public void prependUIBlock(UIBlock block) { + mOperationsQueue.prependUIBlock(block); + } + public int resolveRootTagFromReactTag(int reactTag) { if (mShadowNodeRegistry.isRootNode(reactTag)) { return reactTag; 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 7fc14a632716ff..c7e7ab0f4e999e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java @@ -33,6 +33,8 @@ import com.facebook.react.uimanager.events.EventDispatcher; import com.facebook.systrace.Systrace; import com.facebook.systrace.SystraceMessage; + +import java.util.ArrayList; import java.util.List; import java.util.Map; import javax.annotation.Nullable; @@ -78,6 +80,7 @@ public class UIManagerModule extends ReactContextBaseJavaModule implements private final Map mModuleConstants; private final UIImplementation mUIImplementation; private final MemoryTrimCallback mMemoryTrimCallback = new MemoryTrimCallback(); + private final List mListeners = new ArrayList<>(); private int mBatchId = 0; @@ -513,6 +516,9 @@ public void onBatchComplete() { SystraceMessage.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "onBatchCompleteUI") .arg("BatchId", batchId) .flush(); + for (UIManagerModuleListener listener : mListeners) { + listener.willDispatchViewUpdates(this); + } try { mUIImplementation.dispatchViewUpdates(batchId); } finally { @@ -550,10 +556,28 @@ public void execute (NativeViewHierarchyManager nvhm) { } }); */ - public void addUIBlock (UIBlock block) { + public void addUIBlock(UIBlock block) { mUIImplementation.addUIBlock(block); } + /** + * Schedule a block to be executed on the UI thread. Useful if you need to execute + * view logic before all currently queued view updates have completed. + * + * @param block that contains UI logic you want to execute. + */ + public void prependUIBlock(UIBlock block) { + mUIImplementation.prependUIBlock(block); + } + + public void addUIManagerListener(UIManagerModuleListener listener) { + mListeners.add(listener); + } + + public void removeUIManagerListener(UIManagerModuleListener listener) { + mListeners.remove(listener); + } + /** * Given a reactTag from a component, find its root node tag, if possible. * Otherwise, this will return 0. If the reactTag belongs to a root node, this diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleListener.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleListener.java new file mode 100644 index 00000000000000..1dda859eb3f459 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleListener.java @@ -0,0 +1,21 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +package com.facebook.react.uimanager; + +/** + * Listener used to hook into the UIManager update process. + */ +public interface UIManagerModuleListener { + /** + * Called right before view updates are dispatched at the end of a batch. This is useful if a + * module needs to add UIBlocks to the queue before it is flushed. + */ + void willDispatchViewUpdates(UIManagerModule uiManager); +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java index 62b231a2a679a1..00f6c42977c9e1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java @@ -758,6 +758,10 @@ public void enqueueUIBlock(UIBlock block) { mOperations.add(new UIBlockOperation(block)); } + public void prependUIBlock(UIBlock block) { + mOperations.add(0, new UIBlockOperation(block)); + } + /* package */ void dispatchViewUpdates( final int batchId, final long commitStartTime, final long layoutTime) { SystraceMessage.beginSection( diff --git a/ReactAndroid/src/test/java/com/facebook/react/animated/BUCK b/ReactAndroid/src/test/java/com/facebook/react/animated/BUCK index a81a1ab62bf4ff..4a175cc5a52c00 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/animated/BUCK +++ b/ReactAndroid/src/test/java/com/facebook/react/animated/BUCK @@ -19,6 +19,7 @@ rn_robolectric_test( react_native_target("java/com/facebook/react/animated:animated"), react_native_target("java/com/facebook/react/bridge:bridge"), react_native_target("java/com/facebook/react/common:common"), + react_native_target("java/com/facebook/react/modules/core:core"), react_native_target("java/com/facebook/react/uimanager:uimanager"), react_native_tests_target("java/com/facebook/react/bridge:testhelpers"), ], diff --git a/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java b/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java index ced79811c36960..91dd104bcee776 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java @@ -992,4 +992,51 @@ public Object answer(InvocationOnMock invocation) throws Throwable { verify(mUIImplementationMock).synchronouslyUpdateViewOnUIThread(eq(viewTag), stylesCaptor.capture()); assertThat(stylesCaptor.getValue().getDouble("opacity", Double.NaN)).isEqualTo(10); } + + @Test + public void testRestoreDefaultProps() { + int viewTag = 1000; + int propsNodeTag = 3; + mNativeAnimatedNodesManager.createAnimatedNode( + 1, + JavaOnlyMap.of("type", "value", "value", 1d, "offset", 0d)); + mNativeAnimatedNodesManager.createAnimatedNode( + 2, + JavaOnlyMap.of("type", "style", "style", JavaOnlyMap.of("opacity", 1))); + mNativeAnimatedNodesManager.createAnimatedNode( + propsNodeTag, + JavaOnlyMap.of("type", "props", "props", JavaOnlyMap.of("style", 2))); + mNativeAnimatedNodesManager.connectAnimatedNodes(1, 2); + mNativeAnimatedNodesManager.connectAnimatedNodes(2, propsNodeTag); + mNativeAnimatedNodesManager.connectAnimatedNodeToView(propsNodeTag, viewTag); + + JavaOnlyArray frames = JavaOnlyArray.of(0d, 0.5d, 1d); + Callback animationCallback = mock(Callback.class); + mNativeAnimatedNodesManager.startAnimatingNode( + 1, + 1, + JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 0d), + animationCallback); + + ArgumentCaptor stylesCaptor = + ArgumentCaptor.forClass(ReactStylesDiffMap.class); + + reset(mUIImplementationMock); + mNativeAnimatedNodesManager.runUpdates(nextFrameTime()); + verify(mUIImplementationMock).synchronouslyUpdateViewOnUIThread(eq(viewTag), stylesCaptor.capture()); + assertThat(stylesCaptor.getValue().getDouble("opacity", Double.NaN)).isEqualTo(1); + + for (int i = 0; i < frames.size(); i++) { + reset(mUIImplementationMock); + mNativeAnimatedNodesManager.runUpdates(nextFrameTime()); + } + + verify(mUIImplementationMock).synchronouslyUpdateViewOnUIThread(eq(viewTag), stylesCaptor.capture()); + assertThat(stylesCaptor.getValue().getDouble("opacity", Double.NaN)).isEqualTo(0); + + reset(mUIImplementationMock); + mNativeAnimatedNodesManager.restoreDefaultValues(propsNodeTag, viewTag); + verify(mUIImplementationMock).synchronouslyUpdateViewOnUIThread(eq(viewTag), stylesCaptor.capture()); + assertThat(stylesCaptor.getValue().isNull("opacity")); + } }