Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

ngAnimate causes serious layout thrashing #4124

Closed
mgol opened this issue Sep 23, 2013 · 16 comments
Closed

ngAnimate causes serious layout thrashing #4124

mgol opened this issue Sep 23, 2013 · 16 comments

Comments

@mgol
Copy link
Member

mgol commented Sep 23, 2013

Using ngAnimate can cause serious layout thrashing. Compare two videos:

  1. Our current app, ngAnimate included: http://youtu.be/kUf9vDYIlt0
  2. Our current app with just one difference - removing ngAnimate from the list of modules our app depends on: http://youtu.be/210VHxidY0o

I recorded Timeline profiles for an operation of switching the tab from the second to the third one; version with ngAnimate causes a massive amount of reflows:

  1. Timeline with ngAnimate included:
    with-ng-animate
  2. Timeline without ngAnimate:
    without-ng-animate

One of my colleagues, @jrencz, created videos of analogous operations on his 4-year-old MacBook, notice the slowness:

  1. ngAnimate included: http://www.youtube.com/watch?v=FEmSiWq1qAI
  2. ngAnimate excluded: http://www.youtube.com/watch?v=bshDaxUAzU4
@joegaudet
Copy link

This is the same issue as #4011, but definitely would love to have it addressed.

@mgol
Copy link
Member Author

mgol commented Sep 25, 2013

@joegaudet Probably. My issue has more data, though. ;)

@joegaudet
Copy link

Yeah, we experience the same thing our app. We have tables of 400 + rows, which basically makes the app unusable. I put a guard in the animation code, but that ends up thrashing the filtering on repeaters.... so I guess waiting for @matsko et al.

@bfricka
Copy link

bfricka commented Sep 26, 2013

These looks like exact screen grabs of the timeline profiles for EPG app we're building right now.

@matsko
Copy link
Contributor

matsko commented Sep 26, 2013

The plan is to performance-tune ngAnimate using caching. Getting to work on this today and tomorrow actually.

@joegaudet
Copy link

@matsko sweet man, can't wait. Thanks as always for your hard work man.

@matsko
Copy link
Contributor

matsko commented Sep 26, 2013

My pleasure :)

@matsko
Copy link
Contributor

matsko commented Oct 1, 2013

@mzgol @joegaudet please try testing out your code using the JS files mentioned in #4011 (comment)

@mgol
Copy link
Member Author

mgol commented Oct 1, 2013

@matsko Is this available as a commit somewhere? I use a patched AngularJS version to add support for pushState (see #3325) so I need it as a commit and not just built files.

@matsko
Copy link
Contributor

matsko commented Oct 1, 2013

@mzgol matsko@1022d56

@mgol
Copy link
Member Author

mgol commented Oct 1, 2013

@matsko Unfortunately, after applying your patch there's still severe layout trashing with ngAnimate present in app dependencies; all those repaints disappear if the dependency is removed.

@matsko
Copy link
Contributor

matsko commented Oct 10, 2013

@mzgol merging something in so I'll be closing this issue soon.

matsko added a commit to matsko/angular.js that referenced this issue Oct 11, 2013
…the performance of CSS3 transitions/animations

Closes angular#4011
Closes angular#4124
@matsko matsko closed this as completed in b1e604e Oct 11, 2013
@matsko
Copy link
Contributor

matsko commented Oct 11, 2013

Landed as ddcbf76

@mgol
Copy link
Member Author

mgol commented Oct 11, 2013

I confirm the issue is fixed on current master. Thanks again. Good work. :)

@bfricka
Copy link

bfricka commented Oct 11, 2013

Definitely. Congrats @matsko. 👍

@jeffbcross
Copy link
Contributor

👍 👍 👍

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
…the performance of CSS3 transitions/animations

Closes angular#4011
Closes angular#4124
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
…the performance of CSS3 transitions/animations

Closes angular#4011
Closes angular#4124
@mgol mgol changed the title ngAnimate causes serious layout trashing ngAnimate causes serious layout thrashing Aug 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants