meteoric / meteor-ionic

Ionic components for Meteor. No Angular!
http://meteoric.github.io
MIT License
1.51k stars 219 forks source link

Animation performance #4

Open nickw opened 9 years ago

nickw commented 9 years ago

Ionic recently switched to custom JS animations https://github.com/driftyco/ionic/blob/master/js/angular/service/ionicConfig.js#L324

I hacked together a CSS version just to get things working: https://github.com/meteoric/ionic-sass/blob/master/stylesheets/_transitions.scss

We should investigate how to get better performance. Velocity.js? Is iron:router impacting performance at all?

dennisharrison commented 9 years ago

I would suggest GSAP for this - more than willing to help!

nickw commented 9 years ago

@dennisharrison any help we can get would be much appreciated! I'm definitely open to trying GSAP.

dennisharrison commented 9 years ago

@nickw OK, let's do this first for the side menu animation since we'll be touching that this week anyway. Then you can comment and we can figure the best refactor methodology.

nickw commented 9 years ago

@dennisharrison sounds good. For some background:

I haven't done any 'scientific' testing around frame rates or anything, but in my experience most of the animations are pretty smooth using CSS animations, the problem seems to be mostly with transitioning between routes using _uihooks - especially when a route relies on data from the server to be rendered.

For instance, the routes in https://github.com/meteoric/demo seem to slide back and forth pretty smoothly, but the templates are all static content.

Most of the complaints seem to come from https://github.com/meteoric/meteorhunt and https://github.com/meteoric/contacts where the routes are dynamic and the transitions can get pretty laggy. My assumption is that it is caused by Meteor and/or iron:router loading data at the same time the routes are being animated.

dennisharrison commented 9 years ago

@nikw Interesting. Can you link the line that kicks off the animation you see the choppiness with? I'll do a quick test with GSAP there first to see if there is any real gain.

nickw commented 9 years ago

ionNavView is responsible for sliding the "pages": https://github.com/meteoric/meteor-ionic/blob/master/components/ionNavView/ionNavView.js#L29

ionNavBar handles animating/fading the titles and buttons: https://github.com/meteoric/meteor-ionic/blob/master/components/ionNavBar/ionNavBar.js#L27

Both of these just add/remove CSS classes from the elements, the CSS framework does the heavy lifting: https://github.com/meteoric/ionic-sass/blob/master/stylesheets/_transitions.scss

nickw commented 9 years ago

This post might have some good info for us:

Animating the Mobile Web

dennisharrison commented 9 years ago

After some profiling it looks like the css 3d translation of the parent element of a container full of 3d translated (to get them hardware accelerated) causes the blinking I'm seeing. If we take that class off the child elements before doing the translate of the parent element (say sliding) then I don't see the 'jitter'. Working on making the toggle work without autoform and then can submit a pull request showing the difference for you to test.

nickw commented 9 years ago

@dennisharrison awesome - looking forward to it!

nickw commented 9 years ago

I think we can narrow this down to three issues:

1. Flickering

A known issue: http://nathanhoad.net/how-to-stop-css-animation-flicker-in-webkit. Adding this CSS seems to fix it, but I'd like to get a second pair of eyes on it:

.view {
  -webkit-backface-visibility: hidden;
  -webkit-transform-style: preserve-3d;
  backface-visibility: hidden;
  transform-style: preserve-3d;
}

2. Jittery / staggered transitions

I've been able to confirm that the occasional choppy or staggering transitions are definitely caused by Iron Router's waitOn and data. If the data property loads immediately, the transition is seamless, but if there's any delay at all, the initial view will transition out, then there will be a noticeable delay before the new view transitions in.

I'm looking into implementing template-level subscriptions, which appears to fix the issue, but this technique doesn't play well with block helpers. We may have to wait for the official template-level subscriptions or this PR to be released.

3. Low frame rate

I think this is primarily on Android. Not sure what the solution is yet.

rclai commented 9 years ago

Regarding number 2, these functions from this package might be able to help with getting the template instance from inside a content block. As a matter of fact, I just realized how incredibly useful that entire package is.

dennisharrison commented 9 years ago
  1. Great - this fixes it without re-classing anything. I'll toss that helper method out and use this instead!
  2. This is an interesting problem, https://github.com/meteor/meteor/pull/3546 looks like the solution for it though.
  3. I don't want to stray from your goals for this project, and create wasted time where we implement two separate solutions to this. Let's pick a method you want to go for and I'll concentrate there.
nickw commented 9 years ago

@dennisharrison re: # 3 it's my understanding that CSS animations are almost always going to be more performant (especially on mobile platforms) than JS-based animations, so while I'm skeptical, I'm certainly open to give it a try (I also don't have any better ideas at the moment).

So if either GSAP or Velocity fix the animation issues definitively I don't have a problem including them with this package.

dennisharrison commented 9 years ago

@nickw You're correct. Except on Android (which is pretty odd!), which handles redraw very poorly. GSAP and Velocity have performance parity with CSS transitions now in almost every case, but the performance on Android is better. I wish I understood why, since that shouldn't be the case.

Let's try and fix it without anything else first. On the road today but I'll finally be able to give this a day tomorrow.

Have you run into this one? http://stackoverflow.com/questions/10014461/why-does-enabling-hardware-acceleration-in-css3-slow-down-performance

dennisharrison commented 9 years ago

@nickw I've spent the last two days with this. We can do it with CSS, pull request coming - but I'm doing all this with 1.11 and Android still isn't as smooth as iOS, but I think that's just going to be the case.

Can you merge the PR from Digilord to fix up the data binding for the position of header buttons if it hasn't been fixed in 1.13?

Also I notice someone saying that 1.13 is not working them on iOS and Android? Is that reproducible on your side?

Thanks again for such an awesome package! :)

I'll try to update and then merge all changes and see where we are. We also added in hammer.js for gesture control (went through 6 methods before finding something simple and worthwhile), but I want your input on that before trying to package up for meteoric. It requires a little bit of manual setup in the template. I'm not sure of the overhead of doing it on 'everything' so I think we should figure out a way of conditionally turning it on an element or not, but it seems like events are getting gobbled up by being in the helpers inside of the templates. I'll start an issue with a link to example code once we get the css and layout changes in.

Also - just for anyone who runs across this issue in the future: Read this first: http://greensock.com/css-performance

Check this out to see a method for alleviating the majority of the issues the author mentions with CSS draw times: http://codepen.io/paulirish/full/9712e8fb6a451e0ee7393d01e7f59f53/

TL;DR version: https://developers.google.com/web/fundamentals/look-and-feel/animations/css-vs-javascript?hl=en

nickw commented 9 years ago

Can you merge the PR from Digilord to fix up the data binding for the position of header buttons if it hasn't been fixed in 1.13?

I'm not sure which PR that is - can you link to it?

Also I notice someone saying that 1.13 is not working them on iOS and Android? Is that reproducible on your side?

Nope, working fine for me - you have a link?

dennisharrison commented 9 years ago

I just saw the issue about 1.13 and mobile when browsing for this one on the laptop. Looks like it's likely a non-issue.

I can't find the PR off-hand. Hey @digilord do you have that available?

In the meantime here is the change that mattered.

https://github.com/digilord/meteor-ionic/commit/dd3248035b0dd1d5469c3386cd32cf499c19a4fd

nickw commented 9 years ago

Can you merge the PR from Digilord to fix up the data binding for the position of header buttons if it hasn't been fixed in 1.13?

Ah I see it now, that issue was fixed a while back.

Also I notice someone saying that 1.13 is not working them on iOS and Android? Is that reproducible on your side?

Ok yeah that's #78, I've heard it's an issue for several users but I can't reproduce. For some reason it isn't finding the Cordova plugin com.ionic.keyboard/1.0.3 - which definitely existed at one point. Upgrading the dependency to 1.0.4 should fix it.

We also added in hammer.js for gesture control (went through 6 methods before finding something simple and worthwhile), but I want your input on that before trying to package up for meteoric.

Let's move that discussion to #2

Let me know when you think you'll have a chance to create a PR for animations. If it's not going to be today I'd like to go ahead and push a new release to Atmosphere to fix some of these other issues.

Thanks!

dennisharrison commented 9 years ago

@nickw Go ahead and push release today, headed into Valentines Day territory here :)

dennisharrison commented 9 years ago

Coming soon, promise.

I'm going to update meteor-ionic after implementing more hammerjs gestures and refactoring some stuff on the sample project I'm using to learn all this - and then send along the PR for the css changes to animations.

I've been playing with hammerjs and gsap to handle the menu slide animation based on finger placement and rewind the animation, etc. Feels more native that way. I still think the package should stay default css though to not bring in that dependency.

teknologist commented 9 years ago

Can't wait.

Right now, performance on Android, on a flagship device, is horrible. On an iPhone 5 it is a lot better. On pure Ionic, it is very nice! In fact, back in December pure Ionic was very laggy too. Then they implemented AngularJS views caching and it started feeling native.

More infos here: http://forum.ionicframework.com/t/please-help-test-angular-1-3-improved-transitions-cached-views-etc/12528

rclai commented 9 years ago

@dennisharrison, are you going to use this package as mentioned in #2 to implement the hammer.js stuff?

dennisharrison commented 9 years ago

@rclai No I manually included hammer and the jquery plugin. Like this

Latest updates on everything has me see'ing much better Android performance. Also - Meteor Template based subscriptions is a huge help here. Display a spinner until template subscriptions are ready. Template subscription and Template conditional

Overall, I'm very happy with performance since this change.

guilhermedecampo commented 9 years ago

Hey @dennisharrison if you have iron-router in your app.. why not just use the hooks they already implement like the waitOn for subscriptions?

obs: forked your app to get some profit! haha

dennisharrison commented 9 years ago

@guilhermedecampo haha, thanks :)

Good question, and I wish I was good enough to give you a less anecdotal answer than "everything works better and faster with template based subscriptions." It's hugely helped the Android animation performance, among other issues - such as extra items in collections for short amounts of time due to subscriptions not being explicit enough before render.

guilhermedecampo commented 9 years ago

Awesome @dennisharrison, thanks for your time!

zleman1593 commented 9 years ago

Hey @guilhermedecampo and @dennisharrison. I am using the latest version of meteor iconic with template based subscriptions using subscription manager. This has not yielded any improvement in Android animation performance. It still flickers and looks laggy. What else do I need to do to fix this? Are we waiting on a PR? Thanks.

guilhermedecampo commented 9 years ago

hey @zleman1593 can you make a gist of your template? Would be easier to understand what is going on :smile: