react-native-skia icon indicating copy to clipboard operation
react-native-skia copied to clipboard

runSpring() with Group component's transform inconsistency

Open Aashu-Dubey opened this issue 2 years ago • 3 comments

I was trying to replicate this demo in React Native using this Skia package, had a success with the UI and simple transform animation overall, see the demo here.

I'm not able to properly apply spring animation to the Group component using runSpring, though whatever I tried seems to working as it is a list of around ~250+ image items being rendered, which takes some time and the spring effect seems to be working till all the images are loading, but as soon as all the images are rendered it is having inconsistency, see the video here for more details.

So according to me this seems to be probably more a bug from the package than the demo implementation, I'm attaching the sample's code to let you judge that.

Here is the code sample:- Grid Magnification Skia The spring related code is commented here

Thank you!

Aashu-Dubey avatar Aug 06 '22 21:08 Aashu-Dubey

Hi @Aashu-Dubey - thanks so much for reporting this, and also for trying to create such an astounding example! We'll take a look at this to see what's happening.

chrfalch avatar Aug 07 '22 09:08 chrfalch

We think that the spring issue is due to the fact so much time is spent on each frame.

This is a fantastic example and there is a lot of ground to cover here:

  • You need some sort of z-index (aka sorting of the siblings). We've also hit a similar use-case with more advanced examples such as the liquid swipe example
  • The transform + origin is slightly slower than using the matrix property. When using so many components, it can make a real difference.
  • We have a PR where instead of having two useComputeValue per component, you can calculate the data just in one pass: https://github.com/Shopify/react-native-skia/pull/704

Give us a bit of time, we will continue to investigate this example

wcandillon avatar Aug 08 '22 07:08 wcandillon

Hey, Thanks for updating on this.

  • Yes, I was going to mention that, though I'm using zIndex in the original non-skia implementation (hope this is the use case you mentioned this point for), but I wasn't able to find any alternative for Skia.
  • Thanks for letting me know that, I guess I need to spend more time playing with matrix animations to get more idea about their use cases.
  • Great. I wasn't aware of a more better and optimised way of using useComputedValue for multiple components, for every component though I was initially using a single useComputedValue per component for the calculations (distance, translate, scale etc.), but separated them thinking it wouldn't make much difference. Also as far as this example is concerned, the touch point will update constantly so I guess it will also require constant calculations for every component right?

Thank You!

Aashu-Dubey avatar Aug 08 '22 10:08 Aashu-Dubey

I'm closing this issue, now RN Skia offers the capabilities to run such an example (tons of drawings commands + ui thread animations) super smoothly. For the animations, please have a look at the Reanimated 3 integration: https://shopify.github.io/react-native-skia/docs/animations/reanimated

Please let us know if this helps

wcandillon avatar Apr 20 '23 07:04 wcandillon

Hey, thanks for the update, I again gave it a try with latest v0.1.185 and RN v0.71.7 and I gotta say it's way way better and performant than it was when I created this issue, so good job with that 👍🏼

That being said, I still find some little issues and inconsistency with the animations. There are two solutions, first is the original one using multiple useComputedValue on item level and second is the new one that I tried based on your comment having single useComputedValue at root component then using that through Selector, and both are using matrix property instead of transform as you suggested.

  1. First Solution
  • This one is comparably slow and as you can see in the video animation is not in sync with touch gesture, this I assume is mainly because of multiple computed values at items level (behaviour is same even if I don't use spring animation)
  • At 00:40 if I drag quickly it's skipping the spring effect and also flickers (not the case with non-skia solutions, if you check the application)

https://user-images.githubusercontent.com/46301285/233866802-0883c8d3-22a9-4816-b377-fcacb0180271.mp4

  1. Second Solution
  • Though it does have little delays, but it's way faster and more in sync with touch movement compared to the first one.
  • At 00:23, better to handle the quick drag but behaviour is same.

https://user-images.githubusercontent.com/46301285/233866942-f8e22414-bbd0-4172-95a4-ffa09cc88089.mp4

I also felt little delay in both cases in animation when you first touch on screen (pretty much same sometimes if not using spring effect).


Also recently I made the implementation responsive, so that it works fine on different dimensions and rotations, so for that I'm updating the icons array which kind of recreates the layout, but one issue I noticed is that there's an issue with this recreation in Skia, though It was not the case in last version I tried v0.1.156, below is the video attached for both

https://user-images.githubusercontent.com/46301285/233867050-1d1143d2-f277-4c94-a210-9fda5433dd45.mp4

https://user-images.githubusercontent.com/46301285/233867062-7f9c58ac-ee8a-4261-81da-9e0e40ba6b9c.mov


Lastly I also usually get crashes on iOS, I guess mainly cause of so many elements

https://user-images.githubusercontent.com/46301285/233867140-0f01c6dd-fc0d-4928-8dd4-9ffb47457bb6.mov


I haven't tested it on Android, so these were only related to iOS. And for the animation I'm using the built-in ones, haven't tried the reanimated integration, will also try that someday if I get the time.

Thanks!

Aashu-Dubey avatar Apr 23 '23 21:04 Aashu-Dubey

Tested on Android, the same implementation is wayyyy slower compare to iOS, for real device I tried it on a low-end device, where it's almost non-reactive there, I had to wait around 3-4 minutes for it to even react to touch.

So I instead ran it on a Android emulator (second solution using selector), here the issue is pretty clear in the video (ignore the quality)

  1. For initial touch it's working, but big delay once start dragging
  2. For second touch as you can see I almost have to wait ~2 minutes for it to even react to the touch

https://user-images.githubusercontent.com/46301285/233937889-5c4ac5a3-21d4-4058-b720-1830a5837b37.mp4


For Web, I couldn't make it run because of version limitation on Expo snack

Aashu-Dubey avatar Apr 24 '23 08:04 Aashu-Dubey

if you use reanimated 3 there, life should be good: https://shopify.github.io/react-native-skia/docs/animations/reanimated :)

william-candillon avatar Apr 24 '23 08:04 william-candillon

Alright, though I'm not really sure it's you, because the username is different, but I gave it a try and it does fixes almost all animation related issues like delay and the one with Android.

But since you're just recommending to use reanimated 3 instead, does that mean the built-in ones are not that in priority, cause I did mention some problems with that, especially with Android?

Though reanimated fixes them, what about the layout recreation on device rotation (which was comparably fine on v0.1.156), it's not able to load all the images, and also the crash on iOS?

Overall, it's an awesome library, no doubt.

Aashu-Dubey avatar Apr 24 '23 19:04 Aashu-Dubey

@Aashu-Dubey thanks for taking the time to rewrite it, it looks really cool, I will try to investigate the performance a bit more and will report back

wcandillon avatar Apr 24 '23 19:04 wcandillon

I just had a look at your example. You are building something quite special, super nice :) For the reanimated example to work, you need to upgrade to v3. In your particular example, Skia should have had thrown a warning, I'm not sure why it didn't happen, I will take a look

william-candillon avatar Apr 25 '23 06:04 william-candillon

Thanks, and reanimated implementation was done using reanimated v3, I on purpose changed it back to v2 because it is causing some issue with another sample using rn-svg (rope UI), that's why I had the skia-reanimated implementation commented for now.

Aashu-Dubey avatar Apr 25 '23 06:04 Aashu-Dubey

@Aashu-Dubey I just tried the example with Reanimated 3 on a real device and I must say that I am somewhat impressed by how well it performs (even on android). I'm not familiar how the grid magnification is done yet but let me share some assumptions. I have 3 notes.

  1. First there is a the image loading, every images should be loaded upfront, even before the component is mounted.
  2. At runtime, each box is running substantial code on every frame and is containing 3 animations values. This can surely be simplified. The top level component store the gesture state and position in one value, and each box derives the transformation from this one value. Now using the matrix instead of the transform and origin prop should be faster too, you can see an example at: https://github.com/Shopify/react-native-skia/pull/1399/files#diff-136c71d770a73012e26e72cb9c93095f12b80e9410df3ea28f9aa35bfb79db76R28
  3. Skia has way to deal with image sprites which would help with the loading and also the rendering somewhat if 2. is applied because we could give the sprite transformation directly too. But right now the biggest bang for your buck is 2.

I bet you're having tons of fun with this, Happy Hacking!

william-candillon avatar Apr 25 '23 06:04 william-candillon

It looks like you built the same example in Flutter. How does it perform? I would love to see the implementation and compare it.

william-candillon avatar Apr 25 '23 06:04 william-candillon

Thanks for the feedback, I will sure look into the points you mentioned whenever I get time.

And it's working pretty good on Flutter, here's the Flutter implementation and other details.

You can try it on Web here, and I also shared the demo on twitter.

Or you can download the build from actions, if you wanna try on real device.

Though I also had image loading issue with Flutter, as it was usually crashing rendering so many images, so that part is commented.

Aashu-Dubey avatar Apr 25 '23 06:04 Aashu-Dubey