[WIP] AnimatedLineString Support

[EricPKerr]

Hello,

I'd like to add support for an AnimatedLineString (via refactoring AnimatedPoint into AnimatedCoordinates to be reusable by other GeoJSON shapes).

This is usable by:

 <MapboxGL.Animated.ShapeSource id={`shape`} shape={this.state.shape}>
    <MapboxGL.Animated.LineLayer
        id={`line`}
        style={...}
    />
</MapboxGL.Animated.ShapeSource>

The biggest thing I'd like feedback on is the __attach and __addChild implementations.

Tagging a few folks tied to the original AnimatedPoint code for feedback: @ferdicus @kristfal @mfazekas

[ferdicus]

Hey @EricPKerr, thanks for opening this - I didn't really have time to look over this in detail.
However, what would really help would be to have tests for the AnimatedPoint before the changes get implemented.

Do you think you could get those up and running?

Thanks in advance

[kristfal]

Thanks a lot for your contribution, this is awesome. I'll have a proper look this weekend or early next week.

I haven't used any of the animated features yet nor reviewed the initial implementation. At initial glance, this seems straightforward and well implemented.

My only thought is that we may want to hard crash if props are not set properly instead of putting coordinates at [0,0] because the latter approach may lead people to think this is not working when they in reality have an issue on their own end.

Tests and potentially a simple example would be really nice for something like this. We can do this in a followup PR though. I'd also imagine animated polygons would be pretty cool.

[EricPKerr]

@kristfal thanks for spending the time to review this.

The biggest issue I'm having with this is adding and removing coordinates from the array and having those work properly in the MapboxGL.Animated.ShapeSource/LineLayer. Removing items from the coordinates array doesn't work, as the removed coordinate is still referenced in the ShapeSource/LineLayer. I think there's something wrong with the attach/detach and addChild/removeChild logic, because I also get a yellowbox "trying to remove a child that doesn't exist".

Once we can get arrays of coordinates working, the other shape types (polygons, etc) should be very doable.

What i'd like to accomplish is being able to:

coordinates: [[0,0], [1,1]] ⇒ coordinates: [[0,0],[1,1],[2,2]]

And have the last coordinate go from [1, 1] to [2, 2] so the animation is smooth and the line looks like it's growing (editing the existing coordinate pairs should work too).

Code for above:

this.state = {
  shape: new AnimatedLineString({
    coordinates: [
      [lon, lat],
      [lon + delta, lat + delta]
    ]
  })
};

//

setTimeout(() => {
  this.state.shape
    .timing({
      coordinates: [
        [lon + delta, lat],
        [lon, lat],
        [lon, lat + delta]
      ],
      duration: 300,
      easing: Easing.linear
    })
    .start();
}, 2000);

setTimeout(() => {
  this.state.shape
    .timing({
      coordinates: [
        [lon + delta, lat],
        [lon, lat],
        [lon + delta, lat + delta]
      ],
      duration: 300,
      easing: Easing.linear
    })
    .start();
}, 4000);

setTimeout(() => {
  this.state.shape
    .timing({
      coordinates: [
        [lon, lat],
        [lon, lat + delta]
      ],
      duration: 300,
      easing: Easing.linear
    })
    .start();
}, 6000);

[kristfal]

@EricPKerr you happy with this going into next release?

[EricPKerr]

Hi @kristfal - everything seems to currently work except removing coordinates from the LineString.coordinates array. The array is being modified with the removed coordinate, but it's not propagating properly in the Animated ShapeSource. I think there is an issue with the removeChild or detach logic. Any feedback on how that's set up or what might be breaking?

[kristfal]

Hey, sorry I haven't been able to investigate. I suggest we hold off until we have that sorted. I'll push out release now with updated Android native SDK, and hope we can get this into the release after.

[EricPKerr]

Thanks @kristfal - a review would be helpful and then i'd be glad to add a few additional GeoJSON shapes (Polygon would be awesome).

this.state.shape = new AnimatedLineString({
  coordinates: [...]
});

// and then
this.state.shape.coordinates.pop(); // works

But removing coordinates from within the AnimatedLineString doesn't seem to propagate up to the ShapeSource correctly - seems like either an issue with the array object reference or detach/removeChild and any memory pointers. Figuring this out would enable all of the other shapes, as it would be nice to have the shape fully animate into whatever coordinate array you pass in (same with the immediate setValue and setOffset). This would be an awesome addition I think, and helpful for route line animations vs the current setTimeout iteration approach.

[noway]

Off topic:

I wish AnimatedPoint were using native UI thread animation. 😔

[EricPKerr]

This is being superseded by https://github.com/react-native-mapbox-gl/maps/pull/702 🙂