Closed
Bug 1171966
Opened 9 years ago
Closed 9 years ago
animation performance degraded in version 38.0.5 (38+) slow bug (version 37.0.2 works fast)
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
People
(Reporter: Atom_linkwww, Assigned: birtles)
References
()
Details
(Keywords: perf, regression)
Attachments
(7 files)
(deleted),
application/x-rar
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
image/jpeg
|
Details | |
(deleted),
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.81 Safari/537.36
Steps to reproduce:
When page loaded
Actual results:
it behaves slowly
Expected results:
fast response, like in version
https://youtu.be/1RK3l6H0uDU
Summary: version 38.0.5 (38+) slow bug → version 38.0.5 (38+) slow bug (version 37.0.5 works fast)
Summary: version 38.0.5 (38+) slow bug (version 37.0.5 works fast) → version 38.0.5 (38+) slow bug (version 37.0.2 works fast)
Slow animation after version 38
The same (slow) situation in cases:
- safe mode
- when hardware acceleration disabled
- after updating the graphic drivers
Comment 2•9 years ago
|
||
could you type about:support into the address-bar, copy the graphics section of that page & provide that here in the bug report?
Flags: needinfo?(Atom_linkwww)
I ll try that, thanks.You got the same slow page (folder 2) in last version FF ?
The difference between folder 1 and folder 2 is that in folder 1 no active elements
Flags: needinfo?(Atom_linkwww)
https://youtu.be/1RK3l6H0uDU
This bug is seen for all users, making the work with the browser FF difficult
Source of problem is numerios active elements, look like next templates.
<!--switcher on -->
<g id="switcher.y" cursor="pointer" stroke-width="0.15">
<g transform="scale(1,1.375)">
<g>
<rect x="-0.5" y="-0.5" width="1" height="1" stroke="white" pointer-events="none"></rect>
<rect x="-0.5" y="-0.5" width="1" height="1" fill="white">
<!--it makes half-visible selecting effect -->
<set attributeName="stroke-opacity" begin="mouseover" end="mouseout" to="0.5"></set>
<!-- explicitly reverse the opacity animation on mouseout -->
<set attributeName="stroke-opacity" begin="mouseout" end="mouseover" to="1"></set>
</rect>
<line x1="0" y1="-0.25" x2="0" y2="0.25" stroke-width="0.17" stroke-linecap="round" pointer-events="none"></line><!-- vertical on -->
<!--animation-->
<!--it scales a few times after change committed to this element -->
<animateTransform attributeType="XML" attributeName="transform" type="scale" dur="0.5s" keyTimes="0;0.5;0.5;1" values="1;1.12;1.12;1" repeatCount="6" fill="freeze"></animateTransform>
<!--it animates scale up and scale down onclick -->
<animateTransform attributeName="transform" attributeType="XML" type="scale" from="1" to="1.15" repeatCount="1" begin="mousedown+0.2s" dur="0.2s" fill="freeze"></animateTransform>
<animateTransform attributeName="transform" attributeType="XML" type="scale" from="1.15" to="1" repeatCount="1" begin="mouseup+0.4s" dur="0.2s" fill="freeze"></animateTransform>
</g>
</g>
</g>
<!-- switched on -->
<g id="zn.y" cursor="pointer">
<g transform="rotate(-90 0 0)">
<g>
<rect x="-1.32" y="-0.63" width="3.64" height="1.26" fill="white" stroke-width="0.0" rx="0.12"></rect>
<rect x="-1.32" y="-0.63" width="3.64" height="1.26" opacity="0.15" stroke-width="0.0" rx="0.12">
<!--this animation makes half-visible selecting effect -->
<animate attributeType="CSS" attributeName="opacity" to="0.07" dur="0.5s" begin="mouseover" fill="freeze"></animate>
<animate attributeType="CSS" attributeName="opacity" to="0.15" dur="0.5s" begin="mouseout" fill="freeze"></animate>
</rect>
<!--brush-->
<line stroke-width="0.15" x1="1.995" y1="-0.05" x2="1.995" y2="0.05" stroke-linecap="round" stroke-miterlimit="10" pointer-events="none"></line>
<line stroke-width="0.15" x1="1.795" y1="-0.2" x2="1.795" y2="0.2" stroke-linecap="round" stroke-miterlimit="10" pointer-events="none"></line>
<line stroke-width="0.15" x1="1.595" y1="-0.35" x2="1.595" y2="0.35" stroke-linecap="round" stroke-miterlimit="10" pointer-events="none"></line>
<line stroke-width="0.15" x1="1.395" y1="-0.5" x2="1.395" y2="0.5" stroke-linecap="round" stroke-miterlimit="10" pointer-events="none"></line>
<line stroke-width="0.22" x1="1.39" y1="0.005" x2="0.79" y2="0.005" stroke-miterlimit="10" pointer-events="none"></line>
<!--clock-arrow-->
<line stroke-width="0.22" stroke-miterlimit="10" x1="0.37" y1="0.005" x2="-0.75" y2="0.005" pointer-events="none"></line>
<!--rotate-mechanism -->
<circle cx="0.595" cy="0" r="0.25" stroke-width="0" stroke-miterlimit="10" pointer-events="none"></circle>
<circle cx="0.595" cy="0" r="0.15" fill="white" fill-opacity="0.8" stroke-width="0" stroke-miterlimit="10" pointer-events="none"></circle>
<line stroke-width="0.22" stroke-linecap="round" stroke-miterlimit="10" x1="-0.805" y1="-0.5" x2="-0.805" y2="0.50" pointer-events="none"></line>
<!--it scales a few times after change committed to this element -->
<animateTransform attributeType="XML" attributeName="transform" type="scale" dur="0.5s" keyTimes="0;0.5;0.5;1" values="1;1.12;1.12;1" repeatCount="6" fill="freeze"></animateTransform>
<!--it animates scale up and scale down onclick -->
<animateTransform attributeName="transform" attributeType="XML" type="scale" from="1" to="1.15" repeatCount="1" begin="mousedown+0.2s" dur="0.2s" fill="freeze"></animateTransform>
<animateTransform attributeName="transform" attributeType="XML" type="scale" from="1.15" to="1" repeatCount="1" begin="mouseup+0.4s" dur="0.2s" fill="freeze"></animateTransform>
</g>
<!--unanimate contact to scheme-->
<line stroke-width="0.22" stroke-miterlimit="10" x1="-0.85" y1="0.005" x2="-2.23" y2="0.005" pointer-events="none"></line>
<circle cx="-2.23" cy="0" r="0.25" stroke-width="0" stroke-miterlimit="10" pointer-events="none"></circle>
<circle cx="-2.23" cy="0" r="0.15" fill="white" fill-opacity="0.8" stroke-width="0" stroke-miterlimit="10" pointer-events="none"></circle>
</g>
</g>
<!-- switched on -->
<g id="dscn.y" cursor="pointer">
<rect x="-0.63" y="-0.82" width="1.26" height="1.64" fill="white" stroke-width="0" rx="0.12" pointer-events="none"></rect>
<g>
<rect x="-0.63" y="-0.82" width="1.26" height="1.64" opacity="0.15" stroke-width="0.0" rx="0.12"/>
<!--it makes half-visible selecting effect -->
<animate attributeType="CSS" attributeName="opacity" to="0.1" dur="0.5s" begin="mouseover" fill="freeze"></animate>
<animate attributeType="CSS" attributeName="opacity" to="0.15" dur="0.5s" begin="mouseout" fill="freeze"></animate>
</rect>
<line x1="0" y1="-0.73" x2="0" y2="0.55" stroke-width="0.22" pointer-events="none"></line><!-- left from dot -->
<line x1="-0.5" y1="-0.73" x2="0.5" y2="-0.73" stroke-width="0.22" stroke-linecap="round" pointer-events="none"></line><!-- left vertical -->
<circle cx="0.0" cy="0.61" r="0.215" stroke-width="0" stroke-miterlimit="10" pointer-events="none"></circle><!-- dot -->
<circle cx="0.0" cy="0.61" r="0.13" fill="white" fill-opacity="0.8" stroke-width="0" stroke-miterlimit="10" pointer-events="none"></circle><!-- dot -->
<!--it scales a few times after change committed to this element -->
<animateTransform attributeType="XML" attributeName="transform" type="scale" dur="0.5s" keyTimes="0;0.5;0.5;1" values="1;1.12;1.12;1" repeatCount="6" fill="freeze"></animateTransform>
<!--it animates scale up and scale down onclick -->
<animateTransform attributeName="transform" attributeType="XML" type="scale" from="1" to="1.15" repeatCount="1" begin="mousedown+0.2s" dur="0.2s" fill="freeze"></animateTransform>
<animateTransform attributeName="transform" attributeType="XML" type="scale" from="1.15" to="1" repeatCount="1" begin="mouseup+0.4s" dur="0.2s" fill="freeze"></animateTransform>
</g>
</g>
If I try to explicity them and make look like this
<!--switcher on -->
<g id="switcher.y" cursor="pointer" stroke-width="0.15">
<g transform="scale(1,1.375)">
<rect x="-0.5" y="-0.5" width="1" height="1" stroke="white" pointer-events="none"></rect>
</g>
</g>
<!-- switched on -->
<g id="zn.y" cursor="pointer">
<g transform="rotate(-90 0 0)">
<rect x="-1.32" y="-0.63" width="3.64" height="1.26" fill="white" stroke-width="0.0" rx="0.12"></rect>
</g>
</g>
<!-- switched on -->
<g id="dscn.y" cursor="pointer">
<rect x="-0.63" y="-0.82" width="1.26" height="1.64" fill="white" stroke-width="0" rx="0.12" pointer-events="none"></rect>
</g>
Then spped is ok on 38.0.5, like version 37.0.2
I noticed that the reason for the slow behavior in versions 38+ is attribute
<animateTransform> ... </animateTransform>
Updated•9 years ago
|
Component: Untriaged → Graphics
Product: Firefox → Core
Comment 6•9 years ago
|
||
[Tracking Requested - why for this release]: Large performance regression in 38
tracking-firefox39:
--- → ?
tracking-firefox40:
--- → ?
tracking-firefox41:
--- → ?
tracking-firefox-esr38:
--- → ?
Updated•9 years ago
|
In 38+ If you hover mouse on element with animation effect the page Then starts to be slowly
In 38+ If you hover mouse on element with animation effect the page Then starts to be slowly
youtu.be/k1urnpoiO5E
Comment 9•9 years ago
|
||
Mention - if remove all <animate> / <animateTransform> tags the page not so slow any more
Reporter | ||
Comment 10•9 years ago
|
||
http://youtu.be/RgfikYEHc8Y
Hard case - more difficult tamplete with animate tags in defs block. Any browser load a little harder, but in versions 38+ we can see real differences in speed slowdown
Reporter | ||
Comment 11•9 years ago
|
||
Updated•9 years ago
|
Summary: version 38.0.5 (38+) slow bug (version 37.0.2 works fast) → animation performance degraded in version 38.0.5 (38+) slow bug (version 37.0.2 works fast)
Updated•9 years ago
|
Keywords: regressionwindow-wanted
Reporter | ||
Comment 12•9 years ago
|
||
Is this was meant ? Is there enough information, or try again with the inbound (Bisection) instead Nightlies
Reporter | ||
Comment 13•9 years ago
|
||
2015-02-17 is good
2015-02-18 is bad
Worked with attach 'DNES_CLEAR.rar'
I tried to select the desired diapason in url (end 17 day, full 18 day):
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b6c56fab513d&tochange=9696d1c4b3ba
Somewhere here bad changeset, how to find where, fix it?
It is real to find reason of bug, fix it, and wait until update new fixed version of Stable release through Preview release. How long it may take time, what's the chance of occurrence of this happy variant(find the cause of a bug, fix it, add correction in a new version of FF).
Or I should count more on my strength? Find a different direction for the solution of this problem (to bypass around - somehow not using animate tag) or adapt the project to another browser
Thank you
Comment 14•9 years ago
|
||
Updated•9 years ago
|
Component: Graphics → CSS Parsing and Computation
Comment 15•9 years ago
|
||
Via local build
Last Good: 0288ff191edf
First Bad: 7d16f2fd8329
Triggered by: 7d16f2fd8329 L. David Baron — Bug 960465 patch 6 - Add the new mechanism for avoiding starting spurious transitions as a result of animations: flush animation styles only before doing other restyle processing. r=birtles
Comment hidden (off-topic) |
Comment hidden (off-topic) |
Comment 18•9 years ago
|
||
This sounds like it's probably related to the first FIXME in RestyleManager::UpdateOnlyAnimationStyles, and affects only pages that use SMIL Animation. (I believe Chrome is removing support for SMIL Animation, and I wouldn't consider its performance a high priority.)
Comment 19•9 years ago
|
||
At the moment they intend (so they say) merely to deprecate it and not remove at least according to https://groups.google.com/a/chromium.org/d/msg/blink-dev/5o0yiO440LM/59rZqirUQNwJ
Reporter | ||
Comment 20•9 years ago
|
||
sounds highpromising xD
I'll try to adapt the project to ccs transiction
Reporter | ||
Comment 21•9 years ago
|
||
Mozilla SMIL animation performance is still degraded (the problem not resolved)
It is too late to get any fix for this into 39. It sounds like this performance problem started in 38.
Do we also intend to deprecate this as it's mentioned that Chrome is in comment 18?
status-firefox39:
--- → wontfix
Comment hidden (off-topic) |
Comment hidden (off-topic) |
Assignee | ||
Comment 25•9 years ago
|
||
(In reply to Liz Henry (:lizzard) from comment #22)
> Do we also intend to deprecate this as it's mentioned that Chrome is in
> comment 18?
We don't have any intention to deprecate SMIL at this stage.
Comment hidden (off-topic) |
Comment hidden (off-topic) |
Brian, is this something we should be fixing in FF40? We have ~2 weeks of time remaining to do so. Could you please help find an owner for this bug?
Flags: needinfo?(bbirtles)
Assignee | ||
Comment 29•9 years ago
|
||
I'll have a look at it:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f78a6f642ae
Assuming this patch works, I'm still not sure if this is critical enough to warrant landing on beta so late in the game, however.
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Flags: needinfo?(bbirtles)
Assignee | ||
Comment 30•9 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #29)
> I'll have a look at it:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f78a6f642ae
>
> Assuming this patch works, I'm still not sure if this is critical enough to
> warrant landing on beta so late in the game, however.
That patch fixes the performance problem but breaks some tests. I'm not going to be able to look at this properly in time for an uplift to beta.
Untracked for ESR38 as this does not meet the ESR triage bar which includes security related bug fixed and issue that have a significantly impact our end-users. Also see coment #18.
Assignee | ||
Comment 32•9 years ago
|
||
I've been trying to understand when we actually need to update SMIL and
I *think* the problem is mapped attributes. I *think* what's happening is
something like the following:
* Animations play normally with the nsSMILAnimationController getting ticks from
the refresh driver
* If anything happens out-of-band (e.g. a new animation element added, an
animation element's attribute changed etc. etc.) we tell the animation
controller a resample is needed
* On a resample (which can happen due to either of the above) we cycle through
all the timed elements and they mark their animation functions as having
changed (or not) and give them the required timing information
* Then we build up a compositor for each animated attribute that is changing
which contains a list of all the animation functions targeting it
* After building up the animation value by visiting each animation function we
then apply it to the target
* For the case of mapped attributes, this means we set a property of category
SMIL_MAPPED_ATTR_ANIMVAL, with name corresponding to the attribute name,
holding the string representing the animated value.
* Then we blow away the computed content-style-rule on the element, i.e. we do:
mElement->DeleteProperty(SMIL_MAPPED_ATTR_ANIMVAL, SMIL_MAPPED_ATTR_STYLERULE_ATOM);
* Then we request an animation restyle (to fill in the style rule)
shell->RestyleForAnimation(mElement, eRestyle_Self);
At this point, there's nothing in the nsSMILAnimationController or any of the
nsSMILAnimationFunctions etc. to indicate that there's more work to be done, but
there is.
I *think* this is different to the case for nsSMILCSSProperty where SetAnimValue
updates the SMIL override style immediately.
If that's right, it might explain why we need to update SMIL on every call to
UpdateOnlyAnimationStyles under the scheme introduced in bug 960465.
I'm not really sure about all those details but I wonder if we could better
detect when SMIL needs to be updated by iterating through all the
SMIL_MAPPED_ATTR_ANIMVAL properties in the document and looking for an element
that has a SMIL_MAPPED_ATTR_ANIMVAL attribute but no
SMIL_MAPPED_ATTR_STYLERULE_ATOM. If we find one we know we need to do work.
I don't *think* we also need to inspect the animation controller's "needs
resample state" since I don't think anything in the existing code called by
UpdateOnlyAnimationStyles triggers a SMIL resample and it appears to work so
simply checking of the style rule needs to be updated from the currently
recorded animation values should be enough.
Assignee | ||
Comment 33•9 years ago
|
||
Actually, fixing mapped attributes isn't enough. We have similar issues in other cases too. I'll try a few more ways of fixing this but I'm not sure how much more time this bug is worth.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e38f0885b3db
Assignee | ||
Comment 34•9 years ago
|
||
This hopefully fixes it (a previous version of the patch did but broke other tests; not sure if my opt build will finish before I go home so I'm putting this up for review now)
Attachment #8640329 -
Flags: review?(dholbert)
Assignee | ||
Comment 35•9 years ago
|
||
Assignee | ||
Comment 36•9 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #34)
> This hopefully fixes it (a previous version of the patch did but broke other
> tests; not sure if my opt build will finish before I go home so I'm putting
> this up for review now)
Verified that this still fixes the issue for me.
Comment 37•9 years ago
|
||
Comment on attachment 8640329 [details] [diff] [review]
Update SMIL animation styles only when there are pending changes
Review of attachment 8640329 [details] [diff] [review]:
-----------------------------------------------------------------
r=me, with nits below addressed as you see fit.
First, an extended-commit-message nit:
> Bug 960465 (specifically part 6, changeset 7d16f2fd8329) changed the way we
> process animation-only style changes. This causes us to update SMIL animations
> more often than is needed.
s/causes/caused/ (since this is talking about a cset in the past)
::: dom/smil/nsSMILAnimationController.cpp
@@ +439,5 @@
> }
>
> // Update last compositor table
> mLastCompositorTable = currentCompositorTable.forget();
> + mHasPendingStyleUpdates = true;
This variable (and its accessor) seem confusingly-named -- we don't *necessarily* have pending style updates when we get here. (For example, suppose we only have one <animate> node, and it's targeting a SVG rect's "x" attribute -- then, we definitely don't have any pending style updates. And yet, we'll set this flag which says we do.)
So: This should probably be renamed to something more tentative, like "mMightHavePendingStyleUpdates", and the accessor should be named something similarly-tentative, like "MightHavePendingStyleUpdates".
@@ +723,5 @@
> : eRestyle_SVGAttrAnimations;
> aTracker.AddPendingRestyle(key.mElement, rshint, nsChangeHint(0));
> }
> +
> + mHasPendingStyleUpdates = false;
We should assert that this flag is true, when we enter this function (nsSMILAnimationController::AddStyleUpdatesTo). If it's false, then we shouldn't be here.
Attachment #8640329 -
Flags: review?(dholbert) → review+
Comment 38•9 years ago
|
||
(BTW: I filed bug 1189185 after poking around in GDB to see what happens in AddStyleUpdatesTo for non-style-related SVG animations.)
Comment 39•9 years ago
|
||
It's too late to consider this fix for 40. We'll have to take the fix in a later release. Marking 40 as wontfix.
status-firefox40:
--- → wontfix
status-firefox41:
--- → affected
status-firefox42:
--- → affected
tracking-firefox42:
--- → +
Comment 40•9 years ago
|
||
Comment 41•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Assignee | ||
Comment 42•9 years ago
|
||
Assignee | ||
Comment 43•9 years ago
|
||
Comment on attachment 8644728 [details] [diff] [review]
Rebased patch for aurora
Approval Request Comment
[Feature/regressing bug #]: bug 960465
[User impact if declined]: Performance degradation on content with SVG (SMIL) animation including nvite.com (bug 1188522)
[Describe test coverage new/current, TreeHerder]: Baked on central for 1 week
[Risks and why]: Minimal
[String/UUID change made/needed]: None
Attachment #8644728 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(dbaron)
Comment 44•9 years ago
|
||
Is it possible to fix the bug earlier, in the Firefox 41?
[Firefox 42 2015-11-03]
[Firefox 41 2015-09-22]
Comment 45•9 years ago
|
||
Travis, comment 43 translated into English asks just that.
Reporter | ||
Comment 46•9 years ago
|
||
Understand you, thanks(that was my second account)
Artem, would you be able to verify the fix on the latest Nightly build? If it looks good, we can uplift the change to Firefox 41. Thanks!
Flags: needinfo?(Atom_linkwww)
Comment on attachment 8644728 [details] [diff] [review]
Rebased patch for aurora
[Triage Comment]
Patch has been in Nightly for a week, we don't know of any negative impact. Let's uplift to m-b.
Attachment #8644728 -
Flags: approval-mozilla-aurora? → approval-mozilla-beta+
Comment 49•9 years ago
|
||
Reporter | ||
Comment 50•9 years ago
|
||
FF Nightly 43.0a1 (2015-08-13) works just as well as the FF 37.0.2
Flags: needinfo?(Atom_linkwww)
Updated•9 years ago
|
Flags: qe-verify+
Comment 52•9 years ago
|
||
I was able to reproduce this issue on Firefox 38.0.5 (20150525141253) using Windows 7 64-bit.
Verified fixed on Firefox 41 Beta 3 (20150820142145) and Firefox 42.0a2 (2015-08-24) using Windows 7 64-bit and Ubuntu 14.04 32-bit.
You need to log in
before you can comment on or make changes to this bug.
Description
•