1083079 - SVG / CSS animation on firefox/desktop landing page broken in Nightly on OS X and other platforms where tiled layers are used

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Alex Gibson [:agibson]]

"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:36.0) Gecko/20100101 Firefox/36.0"

The SVG & CSS keyframe animation on https://www.mozilla.org/firefox/desktop/ is glitchy/broken in current Nightly 36.0a1 (2014-10-14).

It plays fine in Release/Beta/Aurora, and worked fine in Nightly too up until this week. So I think the regression window must be pretty small. The glitch seems to happen when animating SVG path/fill colors toward the end of the animation.

I have yet to properly dig into it, but it would be great to get this fixed if possible.

Not sure if this should be in SVG or CSS component, so I took a guess.

Comment 1

[Daniel Holbert [:dholbert]]

The animation looks pretty smooth to me, in yesterday's nightly (2014-10-14) and today's, on Linux. Maybe it's hardware/OS-specific?

Assuming you see a definite difference between working builds & broken builds, could you track down a regression range with http://mozilla.github.com/mozregression/ ?

Comment 2

[Alex Gibson [:agibson]]

Last good revision: 14665b1de5ee (2014-10-01)
First bad revision: 5d6ec4dddf14 (2014-10-02)

Comment 3

[Alex Gibson [:agibson]]

Attachment: svg-anim.mov

Attached a video of the broken animation for reference (sorry for the .mov format).

Comment 4

[Daniel Holbert [:dholbert]]

Thanks! So that yields this range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=14665b1de5ee&tochange=5d6ec4dddf14

The first thing to jump out there is bug 1074054, though I'm not 100% sure -- there are also several imagelib & graphics changes that could conceivably be involved.

Any chance you can narrow the range further? mozregression should automatically download & run mozilla-inbound builds for you, if they're available. This should do it, if you have mozregression installed:
{
mozregression --good-rev=14665b1de5ee --bad-rev=5d6ec4dddf14 --inbound -a "https://www.mozilla.org/firefox/desktop/"
}

Or this:
{
mozregression --good=2014-10-01 --bad=2014-10-02 -a "https://www.mozilla.org/firefox/desktop/"
}


(It might have to think for a bit before it starts popping up browser windows to test.)

Comment 5

[Daniel Holbert [:dholbert]]

Sorry, that pushlog should've been for mozilla-central, I think:
 https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=14665b1de5ee&tochange=5d6ec4dddf14
(There's a slight chance that it makes a difference, since m-c and m-i pushlogs can be out of order with respect to another)

And for the same reason, the latter mozregression command (with datestamps instead of revisions) in comment 4 is more likely to work than the earlier one.

Comment 6

[Daniel Holbert [:dholbert]]

I also shouldn't have singled out bug 1074054 in particular -- there were several animation-related bugs grouped together in that push, any of which might've also been the cause:
 https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=28519d825a23

Comment 7

[Alex Gibson [:agibson]]

(In reply to Daniel Holbert [:dholbert] from comment #5)
> And for the same reason, the latter mozregression command (with datestamps
> instead of revisions) in comment 4 is more likely to work than the earlier
> one.

Last good revision: 33a3fd4d1970
First bad revision: 28519d825a23
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=33a3fd4d1970&tochange=28519d825a23

Comment 8

[Daniel Holbert [:dholbert]]

Thanks! Suspicion confirmed, then.  birtles, do you see this regression locally, & can you take a look?

Comment 9

[Alex Gibson [:agibson]]

Thanks for digging into this one!

Btw, http://mozilla.github.com/mozregression/ is the awesome (will be using this again in the future).

Comment 10

[Daniel Holbert [:dholbert]]

(Yup, mozregression is great!)

Just to be confirm: I think the glitch that you're talking about here is at the very end of your video (6 seconds), where the wireframe browser turns 100%-solid-dark-blue -- is that right?  (This is right when the "Committed to you" text starts to fade into sight.)

(I can't reproduce that locally, on a Linux Nightly.)

Comment 11

[Alex Gibson [:agibson]]

(In reply to Daniel Holbert [:dholbert] from comment #10)
> (Yup, mozregression is great!)
> 
> Just to be confirm: I think the glitch that you're talking about here is at
> the very end of your video (6 seconds), where the wireframe browser turns
> 100%-solid-dark-blue -- is that right?  (This is right when the "Committed
> to you" text starts to fade into sight.)
> 
> (I can't reproduce that locally, on a Linux Nightly.)

Yes that's right, the icons flash and then the whole browser chrome image also flashes as the line paths fade out and the fill color fades in. I'm guessing it's something to do with the animation fill-mode.

If it helps, the CSS is here: https://github.com/mozilla/bedrock/blob/master/media/css/firefox/desktop/intro-anim.less

Comment 12

[Brian Birtles (:birtles)]

Sorry, I can't reproduce this on a Windows Nightly with or without OMTA.

None of those bugs are immediately suspicious but at a glance my guess is bug 1046055. That bug makes us demote animations from layers once they complete which is consistent with a glitch at the end of rendering. If that's the case, then it may just be a graphics bug (hence why we only see it on OSX too) although there may be a synchronization issue.

Do you have OMTA (config layers.offmainthreadcomposition.async-animations) turned on? It's off by default for Mac so if you haven't manually turned it on then I'd assume no.

If so, one possible theory is that we finish the animation on the compositor and remove the layer before bringing the animations on the main thread up to date.

Comment 13

[Daniel Holbert [:dholbert]]

(Given that agibson reproduced in mozregression's fresh profiles, I'm guessing he didn't have OMTA turned on.)

Also: I just tested on my mac mini, & I *can* reproduce there.  So, seems to be reproducible but  mac-specific (given comment 12 + comment 10).

Comment 14

[Daniel Holbert [:dholbert]]

At birtles' suggestion, I tried backing out this cset locally, on a (confirmed-to-be-affected) debug build on my mac mini:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/dedaffb8297a

That backout fixed the problem.  So, this is a regression from bug 1046055 (though it probably really indicates a mac-specific graphics issue).

Comment 15

[Alex Gibson [:agibson]]

Thanks, yes I can confirm `layers.offmainthreadcomposition.async-animations` is set to false.

Comment 16

[Brian Birtles (:birtles)]

I'm not going to be able to look at this in any depth soon. If you can make a reduced test case though, someone from graphics might be able to check what's going on.

Comment 17

[Alex Gibson [:agibson]]

I put together a reduced version here that features just a couple of elements, hope this helps: http://jsfiddle.net/alexgibson/pp2rbjm1/2/

Comment 18

[Jonathan Watt [:jwatt]]

(In reply to Alex Gibson [:agibson] from comment #17)
> I put together a reduced version here that features just a couple of
> elements, hope this helps: http://jsfiddle.net/alexgibson/pp2rbjm1/2/

Yes, definitely. Thank you.

I'll take a look at this and see how far I can get.

Comment 19

[Jonathan Watt [:jwatt]]

Attachment: reduced testcase

Comment 20

[Jonathan Watt [:jwatt]]

We somehow end up painting the animated div into a clipped away tile:

  ClientTiledPaintedLayer (0x13149b400) [clip=(x=0, y=0, w=0, h=0)] [not visible]

If I flip off layers.enable-tiles and restart the bug is gone. So it seems this is a bug in tiling.

Comment 21

[Jonathan Watt [:jwatt]]

Attachment: MOZ_DUMP_PAINT_LIST log output

I changed the color of the div to rgba(0%,0%,0%,0.2) which allows searching for "0.2" to find the relevant display list items/layers.

Comment 22

[Jonathan Watt [:jwatt]]

Attachment: patch

Got Matt to walk through the code today and it turns out this is because the TiledDrawTarget that is temporarily created during flattening at the end of the animation ends up with zero height because the tiles are at negative offsets.

Comment 23

[Jonathan Watt [:jwatt]]

Comment on attachment 8532737 [details] [diff] [review]
patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/d5af1ac09ada

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d5af1ac09ada

Comment 25

[Jeff Muizelaar [:jrmuizel]]

This fixes the broken rendering that has been on openstreemap.org (and probably mapbox) since tiling was turned on. We should probably land this on all of the places.

Comment 26

[Jeff Muizelaar [:jrmuizel]]

[Tracking Requested - why for this release]: scrolling the map is very glitchy on openstreetmap.org without this.

Comment 27

[Alex Gibson [:agibson]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #25)
> This fixes the broken rendering that has been on openstreemap.org (and
> probably mapbox) since tiling was turned on. We should probably land this on
> all of the places.

Oh, so this also fixes another bug I filed (Bug 1105762). Hooray :D

Comment 28

[Alex Gibson [:agibson]]

Thanks to all for helping to fix this \o/

Comment 29

[Jeff Muizelaar [:jrmuizel]]

Can we get a testcase for this checked into the tree?

Comment 30

[Sylvestre Ledru [:Sylvestre]]

Jonathan, could you fill the uplift requests? Thanks

Comment 31

[Jonathan Petto [:jpetto]]

We have a similar animation (SVG lines w/absolute positioning & other elements) being developed for the Hello product page (due to launch on 1/13). How likely is it for this fix to make it into Fx 35?

Comment 32

[Lukas Blakk [:lsblakk] use ?needinfo]

We're out of time for 35 landings, with only the RC build remaining on Mon Jan 5th.  This will have to ride from 36.

Comment 33

[Lukas Blakk [:lsblakk] use ?needinfo]

Jon - I hope you'll be able to find a workaround for the 35 release.

Comment 34

[Jonathan Petto [:jpetto]]

:lsblakk - So far, I've been able to work around the bug for the Hello animation. Looking like we'll be safe on that front for 35.

Thanks for the update!

Comment 35

[Sylvestre Ledru [:Sylvestre]]

Bas, could you fill the uplift request for aurora? Thanks

Comment 36

[Bas Schouten (:bas.schouten)]

Comment on attachment 8532737 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: DrawTargetTiled usage on OS X
[User impact if declined]: Rendering artifacts
[Describe test coverage new/current, TBPL]: Extensive nightly coverage
[Risks and why]: Relatively low, used everywhere and well tested
[String/UUID change made/needed]: None

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ce0ae0d141e3

Comment 38

[Vladimir Agafonkin]

(In reply to Jon Petto [:jpetto] from comment #34)
> :lsblakk - So far, I've been able to work around the bug for the Hello
> animation. Looking like we'll be safe on that front for 35.
> 
> Thanks for the update!

Jon, can you kindly share how you worked around the bug? We're facing the same bug in a widely popular interactive maps JS library (http://leafletjs.com/) and it's pretty severe. Reported separately in #1085024

Comment 40

[Jonathan Petto [:jpetto]]

(In reply to Vladimir Agafonkin from comment #38)
> Jon, can you kindly share how you worked around the bug? We're facing the
> same bug in a widely popular interactive maps JS library
> (http://leafletjs.com/) and it's pretty severe. Reported separately in
> #1085024

I wish I could give specifics, but I just never ran in to the bug for the Hello animation (https://www.mozilla.org/en-US/firefox/hello/). I'm doing both positive and negative transforming/translating, SVG line animation, absolute positioning, opacity changes, etc. I'm unsure of why the bug never came up for me.

Comment 41

[Lukas Blakk [:lsblakk] use ?needinfo]

Putting this back to 'affected' so it's in the queue if there is a driver for a dot release.  This issue on its own is most likely not going to be a driver given that the visual glitch is only during scrolling (ie: the site still works after you stop) and it's limited to Mac users.  We'll see what happens once we've gotten uptake on 35.0.

Comment 42

[Alex Muller]

I don't know how Mozilla decides what should be released as patches to major versions, but this is a serious enough rendering issue that I searched out the issue. It makes OpenStreetMap (among other sites) almost unusable in Firefox on OS X because the flickering is so annoying.

(I reached this thread by searching for "firefox leaflet map" -> https://github.com/Leaflet/Leaflet/issues/3133 -> bug 1085024 -> here)

Comment 43

[Lukas Blakk [:lsblakk] use ?needinfo]

Please nominate the patch for mozilla-release uplift in preparation for a dot release, so we can get it landed quickly if we go with one.

Comment 44

[Jonathan Watt [:jwatt]]

Comment on attachment 8532737 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: DrawTargetTiled usage on OS X
[User impact if declined]: Some bad rendering
[Describe test coverage new/current, TreeHerder]: rode the aurora36 to beta36 train without any issues being reported so far
[Risks and why]: Expected to be low
[String/UUID change made/needed]: none

Comment 45

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/a6e8c946f3e4

Comment 46

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

I've tested this fix today on Mac OS X 10.9.5 using the following pages:
1. https://www.mozilla.org/firefox/desktop/
2. http://jsfiddle.net/alexgibson/pp2rbjm1/2/
3. https://bugzilla.mozilla.org/attachment.cgi?id=8516184
4. http://leafletjs.com/
5. http://www.openstreetmap.org/#map=11/50.9377/1.7159

For pages #2 to #5 I was able to reproduce the flickering on Firefox 35 Nightly from October 3rd. 
No more issues were seen for:
- latest Nightly 38 (BuildID: 20150122030202) – e10s & non-e10s
- latest Aurora 37 (BuildID: 20150122004004)
- Firefox 36 Beta 3 (BuildID: 20150122214638)
- Firefox 35.0.1 (BuildID: 20150122214805)

Page #1 behaves a bit weird for me though, as there is NO animation executed for Firefox 35 Nightly from October 3rd, Firefox 35.0, or Firefox 35.0.1. The end of the animation is just displayed in place. I tried this on 2 different machines, but still no animation. Note that the animation shows fine with Beta, Aurora and Nightly, and it also shows fine for the other pages. I'm not quite sure what to make of this, but maybe someone should double check this.

Though this was not initially reported for Windows or Linux, I did a quick check on Windows 7 x64 and Ubuntu 13.04 x64, and encountered no issues on 35.0.1, 36 Beta 3, latest 37 Aurora, or latest 38 Nightly.

Jonathan, can you provide some insight on issue with no animation in 35.0.1 for page #1 above?

Comment 47

[Alex Gibson [:agibson]]

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #46)
Page #1 behaves a bit weird for me though, as there is NO animation executed for Firefox 35 Nightly from October 3rd, Firefox 35.0, or Firefox 35.0.1. The end of the animation is just displayed in place. I tried this on 2 different machines, but still no animation. Note that the animation shows fine with Beta, Aurora and Nightly, and it also shows fine for the other pages. I'm not quite sure what to make of this, but maybe someone should double check this.

Please note we disabled the animation in Firefox 35 on OSX while this bug was being fixed, which is why you only see the fallback image.

Comment 48

[Alex Gibson [:agibson]]

Clearing the needs info for jwatt

Comment 49

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

(In reply to Alex Gibson [:agibson] from comment #47)
> Please note we disabled the animation in Firefox 35 on OSX while this bug
> was being fixed, which is why you only see the fallback image.

Thank you Alex for clearing this up! This means everything works now as expected. Closing this bug.