search

davidjerleke / embla-carousel

A lightweight carousel library with fluid motion and great swipe precision.
https://www.embla-carousel.com
MIT License
5.35k stars 163 forks source link

[Bug]: iOS Stutter #890

Open WilliamWelsh opened 4 weeks ago

WilliamWelsh commented 4 weeks ago

Which variants of Embla Carousel are you using?

Steps to reproduce

The bug occurs when I use caoursel.scrollTo(index, false) to smoothly scroll to the selected item.

Expected Behavior

It should scroll smoothly on iOS browsers as it does as non-Safari browsers.

Additional Context

I believe it has something to do with Safari/webkit.

On Android Chrome, it is perfectly smooth. On MacOS Chrome, it is perfectly smooth. On MacOS Safari, it stutters. On iOS Safari, it stutters. On iOS Chrome (which is still WebKit), it stutters.

What browsers are you seeing the problem on?

Safari

Version

8.1.3

CodeSandbox

https://codesandbox.io/p/sandbox/silly-haze-3evud?file=%2Findex.html

Before submitting

davidjerleke commented 4 weeks ago

@WilliamWelsh thanks. While I appreciate the effort, I get issues or questions like this once in a while where the dev in question claims that the scroll animations stutters. It’s really hard for me to do anything about this when I don’t see any stuttering on Safari whether I test it on my iOS or OSX devices. Anything could cause this including a performance heavy implementation of a carousel on top of the Embla library.

Do you have anything else to add, something more substantial that could support your claim and/or help me reproduce this?

WilliamWelsh commented 3 weeks ago

If you increase the duration prop, it's easier to notice. Chrome (excluding Chrome iOS) it feels like butter, but every WebKit device/browser I've tried (even on embla-carousel.com) I can notice stutter.

davidjerleke commented 3 weeks ago

@WilliamWelsh have you tried well known CSS tricks to promote the container (the element that is scrolled with transform: translate) to a separate layer? Example:

.embla__container {
  backface-visibility: hidden;
}

/* If that doesn’t work, try this instead (not both): */

.embla__container {
  will-change: transform;
}

Does that affect the animation in any way?

WilliamWelsh commented 3 weeks ago

It seems to be a lot better with the backface-visibility setting, combined with changing the duration to 10 the animation is so fast that you can't really notice it anymore

davidjerleke commented 3 weeks ago

It seems to be a lot better with the backface-visibility setting, combined with changing the duration to 10 the animation is so fast that you can't really notice it anymore

@WilliamWelsh so basically no, it didn't help then. I've put together a little test with a simple scroll animation for both JS and CSS here:

If you wouldn't mind, test both of them on webkit and Chrome browsers respectively, and let me know the results. Which ones stutter if any?

Thanks in advance! David

sarussss commented 2 weeks ago

Hi @davidjerleke It seems that when using css animation, in low power mode on iphone there is no stutter like with js animation.

davidjerleke commented 2 weeks ago

@sarussss when did you check? Right now or a while ago?

sarussss commented 2 weeks ago

@davidjerleke I just checked about 30 minutes ago

davidjerleke commented 2 weeks ago

@sarussss sorry to ask you but could you try Test once more? Both default and low battery mode and see if there's stuttering going on? I made some changes to the test sandbox like 20 minutes ago.

sarussss commented 2 weeks ago

@davidjerleke I just checked it. Default mode: There is no difference between css and js animation. Low battery mode: js animation is still a bit slower than css, but it seems to reduce stuttering.

davidjerleke commented 2 weeks ago

@sarussss thanks. What device did you test on?

sarussss commented 2 weeks ago

@davidjerleke I use Iphone 15 Pro to test.

davidjerleke commented 2 weeks ago

@sarussss thanks! I also tested both on iPhone and MacBook Pro and I couldn’t reproduce any stutter when triggering the JS example, even with low power mode active. However, the JS animation is slightly longer/slower (but still not stuttering). When you say this:

Low battery mode: js animation is still a bit slower than css, but it seems to reduce stuttering.

Do you mean that the JS animation is slower only or also stuttering?

sarussss commented 2 weeks ago

@davidjerleke I think is still stuttering, but the stuttering has improved. Video: https://youtube.com/shorts/anKVGukcSNQ?si=hEyE2H7-3Uohlyaz Demo: https://y682vc.csb.app/

davidjerleke commented 1 week ago

@sarussss thanks for sharing the screen recording. Watching it, I think both stutter (which is expected and makes sense because it’s low battery mode after all), and it seems like the JS animation only stutters slightly more than the CSS animation (the difference is barely noticeable). Do you agree?

WilliamWelsh commented 1 week ago

It's definitely noticeable by a lot of people, which is why I opened the issue

davidjerleke commented 1 week ago

@WilliamWelsh, first off, the Test sandbox has changed since you were here the last time because I've updated it. The JS animation there doesn't use the same animation technique as the current version of Embla Carousel.

Secondly, @sarussss wrote this:

Default mode: There is no difference between css and js animation. Low battery mode: js animation is still a bit slower than css, but it seems to reduce stuttering.

My test results are aligned with what @sarussss wrote above, with the exception that I can barely even notice a difference between the CSS and JS animation when low battery mode is activated. But watching @sarussss screen recording, the CSS animation stutters too. So the difference isn't big by any means.

And the whole point of low power mode is to save battery. Even native apps can stutter. So if the CSS and JS animation is just as smooth in this Test when the battery is loaded, and there's just a slight difference of how much they stutter between the CSS and JS animation in low power mode, I think the approach could be a candidate which could make it to the main library.

@WilliamWelsh I don't understand how this is helping:

It's definitely noticeable by a lot of people, which is why I opened the issue

You didn't answer my following question before:

Do you have anything else to add, something more substantial that could support your claim and/or help me reproduce this?

As I mentioned, Test sandbox has changed since you were here the last time. The stuttering @sarussss mentioned is in low power mode so that doesn't even support your claim about stuttering on webkit browsers. At least not when the battery is loaded.

I'm maintaining this on my spare time so if you don't respect that and you don't have anything constructive to say just don't say anything? What do you mean when you say "a lot of people"? The whole point of this conversation is for me to outline what choices I have moving forward, and the pros and cons of the options I have to solve/improve the animation smoothness. The point is not to read repeating comments that doesn't add any value to this conversation.

sarussss commented 1 week ago

Hi @davidjerleke Totally agree with you, hope you bring these improvements to Embla, so I can test on Embla when swiping, when swiping it will often be more jerky.

davidjerleke commented 1 week ago

Thanks @sarussss!

I will definitely see how I can improve the animation engine. Thanks for taking the time to discuss, test and sharing useful information like screen recordings and similar.