From 921b9ac53d4ca0cabd55f4000b72d187179a01fd Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Thu, 9 Mar 2017 15:16:01 -0800 Subject: [PATCH] Native Animated - Support multiple events attached to the same prop Summary: Re-applying the diff that was reverted in D4659669 / https://github.com/facebook/react-native/commit/b87f4abf7815be4666719fb03c040efe3b0f9d81 because of some crashes with fixes from D4659708 merged in. --- Fixes a bug that happens when trying to use ScrollView with sticky headers and native `Animated.event` with `onScroll`. Made a few changes to the ListViewPaging UIExplorer example to repro https://gist.github.com/janicduplessis/17e2fcd99c6ea49ced2954d881011b09. What happens is we need to be able to add multiple events to the same prop + viewTag pair. To do that I simple changed the data structure to `Map>` and try to optimize for the case where there is only one item in the list since it will be the case 99% of the time. **Test plan** Tested by reproducing the bug with the above gist and made sure it was fixed after applying this diff. Closes https://github.com/facebook/react-native/pull/12697 Reviewed By: fkgozali Differential Revision: D4661105 Pulled By: sahrens fbshipit-source-id: c719dc85f45c1a142ef5b9ebfe0a82ae8ec66497 --- .../Animated/src/AnimatedImplementation.js | 8 ++- .../Animated/src/NativeAnimatedHelper.js | 4 +- .../NativeAnimation/RCTNativeAnimatedModule.m | 5 +- .../RCTNativeAnimatedNodesManager.h | 5 +- .../RCTNativeAnimatedNodesManager.m | 60 +++++++++++++------ .../react/animated/NativeAnimatedModule.java | 4 +- .../animated/NativeAnimatedNodesManager.java | 50 ++++++++++++---- 7 files changed, 98 insertions(+), 38 deletions(-) diff --git a/Libraries/Animated/src/AnimatedImplementation.js b/Libraries/Animated/src/AnimatedImplementation.js index 65190e608..e0860eced 100644 --- a/Libraries/Animated/src/AnimatedImplementation.js +++ b/Libraries/Animated/src/AnimatedImplementation.js @@ -2187,7 +2187,13 @@ function attachNativeEvent(viewRef: any, eventName: string, argMapping: Array { + NativeAnimatedAPI.removeAnimatedEventFromView( + viewTag, + eventName, + mapping.animatedValueTag, + ); + }); }, }; } diff --git a/Libraries/Animated/src/NativeAnimatedHelper.js b/Libraries/Animated/src/NativeAnimatedHelper.js index 74c71c7dc..72c22ef63 100644 --- a/Libraries/Animated/src/NativeAnimatedHelper.js +++ b/Libraries/Animated/src/NativeAnimatedHelper.js @@ -93,9 +93,9 @@ const API = { assertNativeAnimatedModule(); NativeAnimatedModule.addAnimatedEventToView(viewTag, eventName, eventMapping); }, - removeAnimatedEventFromView(viewTag: ?number, eventName: string) { + removeAnimatedEventFromView(viewTag: ?number, eventName: string, animatedNodeTag: ?number) { assertNativeAnimatedModule(); - NativeAnimatedModule.removeAnimatedEventFromView(viewTag, eventName); + NativeAnimatedModule.removeAnimatedEventFromView(viewTag, eventName, animatedNodeTag); } }; diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedModule.m b/Libraries/NativeAnimation/RCTNativeAnimatedModule.m index 55763bfe5..4b30c47d7 100644 --- a/Libraries/NativeAnimation/RCTNativeAnimatedModule.m +++ b/Libraries/NativeAnimation/RCTNativeAnimatedModule.m @@ -168,10 +168,11 @@ RCT_EXPORT_METHOD(addAnimatedEventToView:(nonnull NSNumber *)viewTag } RCT_EXPORT_METHOD(removeAnimatedEventFromView:(nonnull NSNumber *)viewTag - eventName:(nonnull NSString *)eventName) + eventName:(nonnull NSString *)eventName + animatedNodeTag:(nonnull NSNumber *)animatedNodeTag) { [_operations addObject:^(RCTNativeAnimatedNodesManager *nodesManager) { - [nodesManager removeAnimatedEventFromView:viewTag eventName:eventName]; + [nodesManager removeAnimatedEventFromView:viewTag eventName:eventName animatedNodeTag:animatedNodeTag]; }]; } diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h index e911f1b37..c4ffc652a 100644 --- a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h +++ b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h @@ -9,8 +9,8 @@ #import -#import #import +#import #import "RCTValueAnimatedNode.h" @@ -70,7 +70,8 @@ eventMapping:(NSDictionary *__nonnull)eventMapping; - (void)removeAnimatedEventFromView:(nonnull NSNumber *)viewTag - eventName:(nonnull NSString *)eventName; + eventName:(nonnull NSString *)eventName + animatedNodeTag:(nonnull NSNumber *)animatedNodeTag; - (void)handleAnimatedEvent:(nonnull id)event; diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m index ac15ad705..caeabba72 100644 --- a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m +++ b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m @@ -11,29 +11,29 @@ #import +#import "RCTAdditionAnimatedNode.h" #import "RCTAnimatedNode.h" #import "RCTAnimationDriver.h" -#import "RCTEventAnimation.h" - -#import "RCTAdditionAnimatedNode.h" -#import "RCTInterpolationAnimatedNode.h" #import "RCTDiffClampAnimatedNode.h" #import "RCTDivisionAnimatedNode.h" +#import "RCTEventAnimation.h" +#import "RCTFrameAnimation.h" +#import "RCTInterpolationAnimatedNode.h" #import "RCTModuloAnimatedNode.h" #import "RCTMultiplicationAnimatedNode.h" -#import "RCTModuloAnimatedNode.h" #import "RCTPropsAnimatedNode.h" +#import "RCTSpringAnimation.h" #import "RCTStyleAnimatedNode.h" #import "RCTTransformAnimatedNode.h" #import "RCTValueAnimatedNode.h" -#import "RCTFrameAnimation.h" -#import "RCTSpringAnimation.h" @implementation RCTNativeAnimatedNodesManager { RCTUIManager *_uiManager; NSMutableDictionary *_animationNodes; - NSMutableDictionary *_eventDrivers; + // Mapping of a view tag and an event name to a list of event animation drivers. 99% of the time + // there will be only one driver per mapping so all code code should be optimized around that. + NSMutableDictionary *> *_eventDrivers; NSMutableSet> *_activeAnimations; CADisplayLink *_displayLink; } @@ -207,7 +207,7 @@ RCTValueAnimatedNode *valueNode = (RCTValueAnimatedNode *)_animationNodes[nodeTag]; NSString *type = config[@"type"]; - idanimationDriver; + id animationDriver; if ([type isEqual:@"frames"]) { animationDriver = [[RCTFrameAnimation alloc] initWithId:animationId @@ -233,7 +233,7 @@ - (void)stopAnimation:(nonnull NSNumber *)animationId { - for (iddriver in _activeAnimations) { + for (id driver in _activeAnimations) { if ([driver.animationId isEqual:animationId]) { [driver removeAnimation]; [_activeAnimations removeObject:driver]; @@ -264,15 +264,36 @@ NSArray *eventPath = [RCTConvert NSStringArray:eventMapping[@"nativeEventPath"]]; RCTEventAnimation *driver = - [[RCTEventAnimation alloc] initWithEventPath:eventPath valueNode:(RCTValueAnimatedNode *)node]; + [[RCTEventAnimation alloc] initWithEventPath:eventPath valueNode:(RCTValueAnimatedNode *)node]; - _eventDrivers[[NSString stringWithFormat:@"%@%@", viewTag, eventName]] = driver; + NSString *key = [NSString stringWithFormat:@"%@%@", viewTag, eventName]; + if (_eventDrivers[key] != nil) { + [_eventDrivers[key] addObject:driver]; + } else { + NSMutableArray *drivers = [NSMutableArray new]; + [drivers addObject:driver]; + _eventDrivers[key] = drivers; + } } - (void)removeAnimatedEventFromView:(nonnull NSNumber *)viewTag eventName:(nonnull NSString *)eventName + animatedNodeTag:(nonnull NSNumber *)animatedNodeTag { - [_eventDrivers removeObjectForKey:[NSString stringWithFormat:@"%@%@", viewTag, eventName]]; + NSString *key = [NSString stringWithFormat:@"%@%@", viewTag, eventName]; + if (_eventDrivers[key] != nil) { + if (_eventDrivers[key].count == 1) { + [_eventDrivers removeObjectForKey:key]; + } else { + NSMutableArray *driversForKey = _eventDrivers[key]; + for (NSUInteger i = 0; i < driversForKey.count; i++) { + if (driversForKey[i].valueNode.nodeTag == animatedNodeTag) { + [driversForKey removeObjectAtIndex:i]; + break; + } + } + } + } } - (void)handleAnimatedEvent:(id)event @@ -282,9 +303,12 @@ } NSString *key = [NSString stringWithFormat:@"%@%@", event.viewTag, event.eventName]; - RCTEventAnimation *driver = _eventDrivers[key]; - if (driver) { - [driver updateWithEvent:event]; + NSMutableArray *driversForKey = _eventDrivers[key]; + if (driversForKey) { + for (RCTEventAnimation *driver in driversForKey) { + [driver updateWithEvent:event]; + } + [self updateAnimations]; } } @@ -337,13 +361,13 @@ - (void)stepAnimations { - for (idanimationDriver in _activeAnimations) { + for (id animationDriver in _activeAnimations) { [animationDriver stepAnimation]; } [self updateAnimations]; - for (idanimationDriver in [_activeAnimations copy]) { + for (id animationDriver in [_activeAnimations copy]) { if (animationDriver.animationHasFinished) { [animationDriver removeAnimation]; [_activeAnimations removeObject:animationDriver]; 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 8256e7b56..c16e2acd0 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java @@ -353,11 +353,11 @@ public class NativeAnimatedModule extends ReactContextBaseJavaModule implements } @ReactMethod - public void removeAnimatedEventFromView(final int viewTag, final String eventName) { + public void removeAnimatedEventFromView(final int viewTag, final String eventName, final int animatedValueTag) { mOperations.add(new UIThreadOperation() { @Override public void execute(NativeAnimatedNodesManager animatedNodesManager) { - animatedNodesManager.removeAnimatedEventFromView(viewTag, eventName); + animatedNodesManager.removeAnimatedEventFromView(viewTag, eventName, animatedValueTag); } }); } 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 12c7f640b..6251b8f4d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java @@ -31,7 +31,9 @@ import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedList; import java.util.List; +import java.util.ListIterator; import java.util.Map; import java.util.Queue; @@ -55,12 +57,14 @@ import javax.annotation.Nullable; private final SparseArray mAnimatedNodes = new SparseArray<>(); private final SparseArray mActiveAnimations = new SparseArray<>(); private final SparseArray mUpdatedNodes = new SparseArray<>(); - private final Map mEventDrivers = new HashMap<>(); + // Mapping of a view tag and an event name to a list of event animation drivers. 99% of the time + // there will be only one driver per mapping so all code code should be optimized around that. + private final Map> mEventDrivers = new HashMap<>(); private final Map> mCustomEventTypes; private final UIImplementation mUIImplementation; private int mAnimatedGraphBFSColor = 0; - // Used to avoid allocating a new array on every frame in runUpdates. - private final List mRunUpdateNodeList = new ArrayList<>(); + // Used to avoid allocating a new array on every frame in `runUpdates` and `onEventDispatch`. + private final List mRunUpdateNodeList = new LinkedList<>(); public NativeAnimatedNodesManager(UIManagerModule uiManager) { mUIImplementation = uiManager.getUIImplementation(); @@ -312,11 +316,32 @@ import javax.annotation.Nullable; } EventAnimationDriver event = new EventAnimationDriver(pathList, (ValueAnimatedNode) node); - mEventDrivers.put(viewTag + eventName, event); + String key = viewTag + eventName; + if (mEventDrivers.containsKey(key)) { + mEventDrivers.get(key).add(event); + } else { + List drivers = new ArrayList<>(1); + drivers.add(event); + mEventDrivers.put(key, drivers); + } } - public void removeAnimatedEventFromView(int viewTag, String eventName) { - mEventDrivers.remove(viewTag + eventName); + public void removeAnimatedEventFromView(int viewTag, String eventName, int animatedValueTag) { + String key = viewTag + eventName; + if (mEventDrivers.containsKey(key)) { + List driversForKey = mEventDrivers.get(key); + if (driversForKey.size() == 1) { + mEventDrivers.remove(viewTag + eventName); + } else { + ListIterator it = driversForKey.listIterator(); + while (it.hasNext()) { + if (it.next().mValueNode.mTag == animatedValueTag) { + it.remove(); + break; + } + } + } + } } @Override @@ -334,11 +359,14 @@ import javax.annotation.Nullable; eventName = customEventType.get("registrationName"); } - EventAnimationDriver eventDriver = mEventDrivers.get(event.getViewTag() + eventName); - if (eventDriver != null) { - event.dispatch(eventDriver); - - updateNodes(Collections.singletonList((AnimatedNode) eventDriver.mValueNode)); + List driversForKey = mEventDrivers.get(event.getViewTag() + eventName); + if (driversForKey != null) { + for (EventAnimationDriver driver : driversForKey) { + event.dispatch(driver); + mRunUpdateNodeList.add(driver.mValueNode); + } + updateNodes(mRunUpdateNodeList); + mRunUpdateNodeList.clear(); } } }