Skip to content

Commit

Permalink
Fix connection of animated nodes and scroll offset with useNativeDriv…
Browse files Browse the repository at this point in the history
…er. (facebook#24177)

Summary:
Add example showing regression before this fix is applied.

facebook#18187 Was found to introduce a regression in some internal facebook code-base end to end test which couldn't be shared. I was able to create a reproducible demo of a regression I found, and made a fix for it. Hopefully this will fix the internal test, such that the pr can stay merged.

## Changelog

[GENERAL] [Fixed] - Fix connection of animated nodes and scroll offset with useNativeDriver.
Pull Request resolved: facebook#24177

Reviewed By: rickhanlonii

Differential Revision: D14845617

Pulled By: cpojer

fbshipit-source-id: 1f121dbe773b0cde2adf1ee5a8c3c0266034e50d
  • Loading branch information
msand authored and M-i-k-e-l committed Mar 10, 2020
1 parent d543a5e commit a64f857
Show file tree
Hide file tree
Showing 15 changed files with 366 additions and 21 deletions.
25 changes: 21 additions & 4 deletions Libraries/Animated/src/NativeAnimatedHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,25 @@ let __nativeAnimationIdCount = 1; /* used for started animations */

let nativeEventEmitter;

let queueConnections = false;
let queue = [];

/**
* Simple wrappers around NativeAnimatedModule to provide flow and autocmplete support for
* the native module methods
*/
const API = {
enableQueue: function(): void {
queueConnections = true;
},
disableQueue: function(): void {
invariant(NativeAnimatedModule, 'Native animated module is not available');
queueConnections = false;
while (queue.length) {
const args = queue.shift();
NativeAnimatedModule.connectAnimatedNodes(args[0], args[1]);
}
},
createAnimatedNode: function(tag: ?number, config: AnimatedNodeConfig): void {
invariant(NativeAnimatedModule, 'Native animated module is not available');
NativeAnimatedModule.createAnimatedNode(tag, config);
Expand All @@ -46,6 +60,10 @@ const API = {
},
connectAnimatedNodes: function(parentTag: ?number, childTag: ?number): void {
invariant(NativeAnimatedModule, 'Native animated module is not available');
if (queueConnections) {
queue.push([parentTag, childTag]);
return;
}
NativeAnimatedModule.connectAnimatedNodes(parentTag, childTag);
},
disconnectAnimatedNodes: function(
Expand Down Expand Up @@ -197,7 +215,7 @@ function addWhitelistedInterpolationParam(param: string): void {
function validateTransform(
configs: Array<
| {type: 'animated', property: string, nodeTag: ?number}
| {type: 'static', property: string, value: number},
| {type: 'static', property: string, value: number | string},
>,
): void {
configs.forEach(config => {
Expand Down Expand Up @@ -263,7 +281,7 @@ function shouldUseNativeDriver(config: AnimationConfig | EventConfig): boolean {
return config.useNativeDriver || false;
}

function transformDataType(value: number | string): number {
function transformDataType(value: number | string): number | string {
// Change the string type to number type so we can reuse the same logic in
// iOS and Android platform
if (typeof value !== 'string') {
Expand All @@ -274,8 +292,7 @@ function transformDataType(value: number | string): number {
const radians = (degrees * Math.PI) / 180.0;
return radians;
} else {
// Assume radians
return parseFloat(value) || 0;
return value;
}
}

Expand Down
2 changes: 2 additions & 0 deletions Libraries/Animated/src/animations/Animation.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ class Animation {
onEnd && onEnd(result);
}
__startNativeAnimation(animatedValue: AnimatedValue): void {
NativeAnimatedHelper.API.enableQueue();
animatedValue.__makeNative();
NativeAnimatedHelper.API.disableQueue();
this.__nativeId = NativeAnimatedHelper.generateNewAnimationId();
NativeAnimatedHelper.API.startAnimatingNode(
this.__nativeId,
Expand Down
11 changes: 6 additions & 5 deletions Libraries/Animated/src/nodes/AnimatedInterpolation.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ function colorToRgba(input: string): string {
return `rgba(${r}, ${g}, ${b}, ${a})`;
}

const stringShapeRegex = /[0-9\.-]+/g;
const stringShapeRegex = /[+-]?(?:\d+\.?\d*|\.\d+)(?:[eE][+-]?\d+)?/g;

/**
* Supports string shapes by extracting numbers so new values can be computed,
Expand Down Expand Up @@ -242,10 +242,11 @@ function createInterpolationFromStringOutputRange(
// ->
// 'rgba(${interpolations[0](input)}, ${interpolations[1](input)}, ...'
return outputRange[0].replace(stringShapeRegex, () => {
const val = +interpolations[i++](input);
const rounded =
shouldRound && i < 4 ? Math.round(val) : Math.round(val * 1000) / 1000;
return String(rounded);
let val = +interpolations[i++](input);
if (shouldRound) {
val = i < 4 ? Math.round(val) : Math.round(val * 1000) / 1000;
}
return String(val);
});
};
}
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Animated/src/nodes/AnimatedNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,11 @@ class AnimatedNode {
);
if (this.__nativeTag == null) {
const nativeTag: ?number = NativeAnimatedHelper.generateNewNodeTag();
this.__nativeTag = nativeTag;
NativeAnimatedHelper.API.createAnimatedNode(
nativeTag,
this.__getNativeConfig(),
);
this.__nativeTag = nativeTag;
this.__shouldUpdateListenersForNewNativeTag = true;
}
return this.__nativeTag;
Expand Down
1 change: 1 addition & 0 deletions Libraries/Animated/src/nodes/AnimatedProps.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ class AnimatedProps extends AnimatedNode {
for (const propKey in this._props) {
const value = this._props[propKey];
if (value instanceof AnimatedNode) {
value.__makeNative();
propsConfig[propKey] = value.__getNativeTag();
}
}
Expand Down
4 changes: 3 additions & 1 deletion Libraries/Animated/src/nodes/AnimatedStyle.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ class AnimatedStyle extends AnimatedWithChildren {
const styleConfig = {};
for (const styleKey in this._style) {
if (this._style[styleKey] instanceof AnimatedNode) {
styleConfig[styleKey] = this._style[styleKey].__getNativeTag();
const style = this._style[styleKey];
style.__makeNative();
styleConfig[styleKey] = style.__getNativeTag();
}
// Non-animated styles are set using `setNativeProps`, no need
// to pass those as a part of the node config
Expand Down
110 changes: 105 additions & 5 deletions Libraries/NativeAnimation/Nodes/RCTInterpolationAnimatedNode.m
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,90 @@

#import "RCTAnimationUtils.h"

static NSRegularExpression *regex;

@implementation RCTInterpolationAnimatedNode
{
__weak RCTValueAnimatedNode *_parentNode;
NSArray<NSNumber *> *_inputRange;
NSArray<NSNumber *> *_outputRange;
NSArray<NSArray<NSNumber *> *> *_outputs;
NSArray<NSString *> *_soutputRange;
NSString *_extrapolateLeft;
NSString *_extrapolateRight;
NSUInteger _numVals;
bool _hasStringOutput;
bool _shouldRound;
NSArray<NSTextCheckingResult*> *_matches;
}

- (instancetype)initWithTag:(NSNumber *)tag
config:(NSDictionary<NSString *, id> *)config
{
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
NSString *fpRegex = @"[+-]?(\\d+\\.?\\d*|\\.\\d+)([eE][+-]?\\d+)?";
regex = [NSRegularExpression regularExpressionWithPattern:fpRegex options:NSRegularExpressionCaseInsensitive error:nil];
});
if ((self = [super initWithTag:tag config:config])) {
_inputRange = [config[@"inputRange"] copy];
NSMutableArray *outputRange = [NSMutableArray array];
NSMutableArray *soutputRange = [NSMutableArray array];
NSMutableArray<NSMutableArray<NSNumber *> *> *_outputRanges = [NSMutableArray array];

_hasStringOutput = NO;
for (id value in config[@"outputRange"]) {
if ([value isKindOfClass:[NSNumber class]]) {
[outputRange addObject:value];
} else if ([value isKindOfClass:[NSString class]]) {
/**
* Supports string shapes by extracting numbers so new values can be computed,
* and recombines those values into new strings of the same shape. Supports
* things like:
*
* rgba(123, 42, 99, 0.36) // colors
* -45deg // values with units
*/
NSMutableArray *output = [NSMutableArray array];
[_outputRanges addObject:output];
[soutputRange addObject:value];

_matches = [regex matchesInString:value options:0 range:NSMakeRange(0, [value length])];
for (NSTextCheckingResult *match in _matches) {
NSString* strNumber = [value substringWithRange:match.range];
[output addObject:[NSNumber numberWithDouble:strNumber.doubleValue]];
}

_hasStringOutput = YES;
[outputRange addObject:[output objectAtIndex:0]];
}
}
if (_hasStringOutput) {
// ['rgba(0, 100, 200, 0)', 'rgba(50, 150, 250, 0.5)']
// ->
// [
// [0, 50],
// [100, 150],
// [200, 250],
// [0, 0.5],
// ]
_numVals = [_matches count];
NSString *value = [soutputRange objectAtIndex:0];
_shouldRound = [value containsString:@"rgb"];
_matches = [regex matchesInString:value options:0 range:NSMakeRange(0, [value length])];
NSMutableArray<NSMutableArray<NSNumber *> *> *outputs = [NSMutableArray arrayWithCapacity:_numVals];
NSUInteger size = [soutputRange count];
for (NSUInteger j = 0; j < _numVals; j++) {
NSMutableArray *output = [NSMutableArray arrayWithCapacity:size];
[outputs addObject:output];
for (int i = 0; i < size; i++) {
[output addObject:[[_outputRanges objectAtIndex:i] objectAtIndex:j]];
}
}
_outputs = [outputs copy];
}
_outputRange = [outputRange copy];
_soutputRange = [soutputRange copy];
_extrapolateLeft = config[@"extrapolateLeft"];
_extrapolateRight = config[@"extrapolateRight"];
}
Expand Down Expand Up @@ -61,11 +124,48 @@ - (void)performUpdate

CGFloat inputValue = _parentNode.value;

self.value = RCTInterpolateValueInRange(inputValue,
_inputRange,
_outputRange,
_extrapolateLeft,
_extrapolateRight);
CGFloat interpolated = RCTInterpolateValueInRange(inputValue,
_inputRange,
_outputRange,
_extrapolateLeft,
_extrapolateRight);
self.value = interpolated;
if (_hasStringOutput) {
// 'rgba(0, 100, 200, 0)'
// ->
// 'rgba(${interpolations[0](input)}, ${interpolations[1](input)}, ...'
if (_numVals > 1) {
NSString *text = _soutputRange[0];
NSMutableString *formattedText = [NSMutableString stringWithString:text];
NSUInteger i = _numVals;
for (NSTextCheckingResult *match in [_matches reverseObjectEnumerator]) {
CGFloat val = RCTInterpolateValueInRange(inputValue,
_inputRange,
_outputs[--i],
_extrapolateLeft,
_extrapolateRight);
NSString *str;
if (_shouldRound) {
// rgba requires that the r,g,b are integers.... so we want to round them, but we *dont* want to
// round the opacity (4th column).
bool isAlpha = i == 3;
CGFloat rounded = isAlpha ? round(val * 1000) / 1000 : round(val);
str = isAlpha ? [NSString stringWithFormat:@"%1.3f", rounded] : [NSString stringWithFormat:@"%1.0f", rounded];
} else {
NSNumber *numberValue = [NSNumber numberWithDouble:val];
str = [numberValue stringValue];
}

[formattedText replaceCharactersInRange:[match range] withString:str];
}
self.animatedObject = formattedText;
} else {
self.animatedObject = [regex stringByReplacingMatchesInString:_soutputRange[0]
options:0
range:NSMakeRange(0, _soutputRange[0].length)
withTemplate:[NSString stringWithFormat:@"%1f", interpolated]];
}
}
}

@end
9 changes: 7 additions & 2 deletions Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.m
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,13 @@ - (void)performUpdate

} else if ([parentNode isKindOfClass:[RCTValueAnimatedNode class]]) {
NSString *property = [self propertyNameForParentTag:parentTag];
CGFloat value = [(RCTValueAnimatedNode *)parentNode value];
self->_propsDictionary[property] = @(value);
id animatedObject = [(RCTValueAnimatedNode *)parentNode animatedObject];
if (animatedObject) {
self->_propsDictionary[property] = animatedObject;
} else {
CGFloat value = [(RCTValueAnimatedNode *)parentNode value];
self->_propsDictionary[property] = @(value);
}
}
}

Expand Down
1 change: 1 addition & 0 deletions Libraries/NativeAnimation/Nodes/RCTValueAnimatedNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
- (void)extractOffset;

@property (nonatomic, assign) CGFloat value;
@property (nonatomic, strong) id animatedObject;
@property (nonatomic, weak) id<RCTValueAnimatedNodeObserver> valueObserver;

@end

0 comments on commit a64f857

Please sign in to comment.