Support animating Svg Path d attribute on android

[msand]

software-mansion/react-native-svg#908

import * as React from 'react';
import { View, StyleSheet } from 'react-native';
import { Svg, Path } from 'react-native-svg';
import Animated from 'react-native-reanimated';

const { concat, multiply } = Animated;
const x = multiply(1, 2);
const y = multiply(2, 3);
const path = concat('M 0 0 L ', x, ' ', y);

const AnimatedPath = Animated.createAnimatedComponent(Path);

export default class App extends React.Component {
  render() {
    return (
      <View style={styles.container}>
        <Svg viewBox="0 0 10 10" width="100" height="100">
          <AnimatedPath d={path} stroke="black" />
        </Svg>
      </View>
    );
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
    backgroundColor: '#ecf0f1',
  },
});

[reneeichhorn]

Maybe a small suggestion:
Does it make sense use a type check in the else block instead of a new else if block specially for ConcatNode.
In the future the might be more Nodes that do string operations.

I'm thinking about doing it the same it is done in line#75

[msand]

Yeah most probably, I just made a quick patch to get it working. I'm thinking @kmagiera and @osdnk will probably have better insight into the rest of the codebase than me, perhaps they can take over from here? Otherwise, once they review it I can probably make any needed changes.

[osdnk]

Hi!
I'm sorry, but I could not understand how it should help. Firstly, you check if some node is concat and if yes, then it is UI prop (like translateX etc), which could be updated by RN itself.
I think that it might help only by some unexpected side effect.

However, I see a wider problem. I think that it might be sometimes possible to update some non-style prop of component without passing bridge. This PR appears to do the same thing #131 so maybe try to fix this solution?

Also, please add some examples to Example app and try do add some comments if needed :)

[msand]

The react-native-svg elements are native views and have property setters to handle the updates directly from the native animation driver, so, most of the props do not need to cross the bridge. Almost all props which take lengths/dimensions can be given either numbers or strings now, but path data (the d attribute of the Path element) needs to be a string, which this change allows to work. But, it might interfere with other use cases, I haven't used reanimated for anything else yet.

[osdnk]

I could imagine that it might break something.

[reneeichhorn]

@osdnk Any idea whats the best way to proceed with this issue? I'd gladly help and contribute. Maybe it should be possible to extend createAnimatedComponent to specify what is a UI Prop and doesn't need to go over the bridge to update.
I'm also wondering what exactly happens on ios because it seems to work fine there as well.

[kmagiera]

Hey @msand and @reneeichhorn – thank you for your work on this. We are motivated to help push this effort forward.

I understand that what is needed is for the props node to support string values as opposed to supporting only styles and numeric values. If so I agree with @reneeichhorn suggestion that instead of doing a node type check we should verify the type of the value that the node returns.

On top of that I'd appreciate if you could update you PR description to include a few words about the scope of this change. The example you provided is very nice but would like a few words on what changes and why.

[msand]

I've rebased my earlier PR to react-native to support string interpolation for use with e.g. react-native-svg: facebook/react-native#18187
@osdnk @kmagiera Would you mind having a look/review it? There's a few other examples there which could be replicated using reanimated as well. I can probably have a look at fixing this PR soon as well.

[msand]

@kmagiera Were you thinking something like this?

  @Override
  protected Double evaluate() {
    boolean hasUIProps = false;
    boolean hasNativeProps = false;
    boolean hasJSProps = false;
    WritableMap jsProps = Arguments.createMap();
    final WritableMap nativeProps = Arguments.createMap();

    for (Map.Entry<String, Integer> entry : mMapping.entrySet()) {
      Node node = mNodesManager.findNodeById(entry.getValue(), Node.class);
      if (node instanceof StyleNode) {
        WritableMap style = (WritableMap) node.value();
        ReadableMapKeySetIterator iter = style.keySetIterator();
        while (iter.hasNextKey()) {
          String key = iter.nextKey();
          WritableMap dest;
          if (mNodesManager.uiProps.contains(key)) {
            hasUIProps = true;
            dest = mPropMap;
          } else if (mNodesManager.nativeProps.contains(key)){
            hasNativeProps = true;
            dest = nativeProps;
          } else {
            hasJSProps = true;
            dest = jsProps;
          }
          ReadableType type = style.getType(key);
          switch (type) {
            case Number:
              dest.putDouble(key, style.getDouble(key));
              break;
            case String:
              dest.putString(key, style.getString(key));
              break;
            case Array:
              dest.putArray(key, (WritableArray) style.getArray(key));
              break;
            default:
              throw new IllegalArgumentException("Unexpected type " + type);
          }
        }
      } else {
        String key = entry.getKey();
        Object value = node.value();
        if (mNodesManager.uiProps.contains(key)) {
          hasUIProps = true;
          if (value == null) {
            mPropMap.putNull(key);
          } else if (value instanceof Double) {
            mPropMap.putDouble(key, (Double) value);
          } else if (value instanceof Integer) {
            mPropMap.putInt(key, (Integer) value);
          } else if (value instanceof Number) {
            mPropMap.putDouble(key, ((Number) value).doubleValue());
          } else if (value instanceof Boolean) {
            mPropMap.putBoolean(key, (Boolean) value);
          } else if (value instanceof String) {
            mPropMap.putString(key, (String) value);
          } else if (value instanceof WritableArray) {
            mPropMap.putArray(key, (WritableArray)value);
          } else if (value instanceof WritableMap) {
            mPropMap.putMap(key, (WritableMap)value);
          } else {
            throw new IllegalStateException("Unknown type of animated value");
          }
        } else {
          hasNativeProps = true;
          if (value == null) {
            nativeProps.putNull(key);
          } else if (value instanceof Double) {
            nativeProps.putDouble(key, (Double) value);
          } else if (value instanceof Integer) {
            nativeProps.putInt(key, (Integer) value);
          } else if (value instanceof Number) {
            nativeProps.putDouble(key, ((Number) value).doubleValue());
          } else if (value instanceof Boolean) {
            nativeProps.putBoolean(key, (Boolean) value);
          } else if (value instanceof String) {
            nativeProps.putString(key, (String) value);
          } else if (value instanceof WritableArray) {
            nativeProps.putArray(key, (WritableArray)value);
          } else if (value instanceof WritableMap) {
            nativeProps.putMap(key, (WritableMap)value);
          } else {
            throw new IllegalStateException("Unknown type of animated value");
          }
        }
      }
    }

    if (mConnectedViewTag != View.NO_ID) {
      if (hasUIProps) {
        mUIImplementation.synchronouslyUpdateViewOnUIThread(
                mConnectedViewTag,
                mDiffMap);
      }
      if (hasNativeProps) {
        mNodesManager.enqueueUpdateViewOnNativeThread(mConnectedViewTag, nativeProps);
      }
      if (hasJSProps) {
        WritableMap evt = Arguments.createMap();
        evt.putInt("viewTag", mConnectedViewTag);
        evt.putMap("props", jsProps);
        mNodesManager.sendEvent("onReanimatedPropsChange", evt);
      }
    }

    return ZERO;
  }

[msand]

This would require calling:
Animated.addWhitelistedUIProps({ d: true });
And similarly for almost all other react-native-svg properties (the ones which don't require any processing in javascript atm, ideally everything would support animation using purely native logic).
But, addWhitelistedUIProps isn't even listed in the types, and this comment exists instead:

// `addWhitelistedNativeProps` will likely be removed soon, and so is
// intentionally not exposed to TypeScript. If it is needed, it could be
// uncommented here, or just use
// `(Animated as any).addWhitelistedNativeProps({ myProp: true });`

// addWhitelistedNativeProps(props: { [key: string]: true }): void;

How are UIProps supposed to be registered once those are dropped?

[osdnk]

@msand It won't be dropped. Could we remove this comment @adamczyk777?

Please, update PR following changes you've suggested, @msand

[wcandillon]

@msand Thank you so much for doing this. Isn't the issue with string animated values on Android in general? I will try to take a look at it myself as well (#300). On iOS text values as well as SVG path can be animated, would be nice to have the same on Android of course.

[owinter86]

Is there anything blocking the release of this pull request (adjusted with the @msand comment updates)? I can confirm that it closes out issue #300 reported by @wcandillon and have tested on android with the following props.

text={concat("hello ", "world")}
text={new Animated.Value("hello")}

This TextInput setNativeProps solution is a really good way of tracking animated values without crossing the bridge, so it will be nice to include it in this package until RN core supports this functionality properly.

To allow the TextInput to track number values additional logic is required to convert the double values from new Animated.Value(10) to string as android TextInputs only accept string values.

That would fully close out #300 in my opinion, but I am unsure if this logic is suitable in this instance.

} else if (value instanceof Double) {
if (key.equals("text")) {
    // TextInput text prop only accepts string values
    // Convert animated values 'new Animated.Value(10)' to string
    nativeProps.putString(key, (String) Double.toString((Double) value));
} else {
    nativeProps.putDouble(key, (Double) value);
}
} else if (value instanceof Integer) {

[wcandillon]

@owinter86 I'm able to build the lib locally but then how can I link it to my RN project in order to test it? I'd be curious to know who does the development setup look like.

[wcandillon]

I can confirm that it solves the svg path animation issue. And it's very exciting. I lost overview on what needs to be done for this PR to be merge. Please let me know I would be happy to help.

[wcandillon]

@owinter86 I was not able to get the text input example working with RN 0.59.9. Did something change there in the TextInput component?

[owinter86]

@wcandillon it should be the same setup as you had in the #300 issue, I have forked this package and applied the patch so I can test it in my local setup.

If you modify your package json to include my commit like below, you should be able to test it out.

"react-native-reanimated": "https://github.com/owinter86/react-native-reanimated.git#bbbef93f146df86393a07cd23dde9006061fc2ae",

It includes @msand's above snippet and my small patch so that animated strings and animated values will work in android TextInput component.

[msand]

@wcandillon If you have a plain react-native project, you can make changes directly in the code in node_modules and rebuild the native code using: react-native run-ios / run-android, or using xcode / android studio. You don't need any special setup, except whats required to build a plain project:

react-native init CleanProject
cd CleanProject/
npm i react-native-svg react-native-reanimated
react-native link
# make changes in app.js and node_modules and then build & run the project
react-native run-ios

You can use e.g. https://www.npmjs.com/package/patch-package if you don't want to create a fork of some codebase to monkeypatch it in a single project 😄Making a fork in github and pushing commits there instead enables reusing it in several projects.

I would normally do development directly in node_modules, and once I'm ready to publish a change, I diff the folder in node_modules with a separate folder (Webstorm has quite a nice folder diff tool for these kinds of use cases) where I've checked out the git repository of my fork of the module I'm changing, then commit in there and push to my fork on github.

Then it's just a matter of clicking the open pr button in github to spread the fixes/changes to the maintainers/community.

[msand]

Your package.json can refer to git repos, and this way use forks of the module. E.g. to use the latest commit from this pr: https://github.com/msand/react-native-reanimated/tree/animate-svg-path-data-android
npm i react-native-reanimated@msand/react-native-reanimated#be87477
or
npm i react-native-reanimated@msand/react-native-reanimated#be87477ee75a8dbff68b14621203013bbc282585

[wcandillon]

I opened #311 which seems to work great on my Android. Let me know your thoughts.

[wcandillon]

This issue is fixed via #311, @msand can you close it?

[msand]

@wcandillon Ah, great. Thanks for helping out!

[wcandillon]

@msand no, no, no, no ☝🏻Thank YOU 😀