1058634 - animations in SVG-in-OpenType glyphs do not run

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Cameron McCormack (:heycam)]

Animations in OpenType SVG glyphs used to run, but now don't with Nightly builds.  It works in Firefox 31.

Test: http://people.mozilla.org/~jkew/opentype-svg/soccer.html

Comment 1

[Daniel Holbert [:dholbert]]

m-c regression range:
{
Last good revision: b227a707080f (2014-05-01)
First bad revision: e2e1b19fcffc (2014-05-02)
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b227a707080f&tochange=e2e1b19fcffc
}

No commit messages that mention "svg" or "anim" there, but there are a few small font-related changes which might be involved.

Comment 2

[Daniel Holbert [:dholbert]]

Inbound regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ed4c52e21a5f&tochange=9a6b434b34be
...which means this was regressed by bug 1002632.

Comment 3

[Daniel Holbert [:dholbert]]

So, in bug 1002632, the problem was that SVG images were spinning their refresh driver at 60fps even when their tab had been closed, because they can't directly tell if they're in a foreground tab vs. just sitting in the image cache.  So, we'd burn power and consume CPU even when nothing was visibly animating.

(Presumably we had this same problem for SVG in OpenType fonts, though we may not have realized it.)

The solution for SVG images (which I landed in that bug) is to use imgIContainer's preexisting-but-not-functional-for-SVG-images-at-that-point "RequestRefresh()" method, to let the viewing document's refresh driver update the image's animation state, and then the image can invalidate (or not) if necessary.  This way, when the viewing document (or documents) are closed or in background tabs, their refresh driver-slowdown will keep us from needlessly churning on the SVG image's animations.

I'm not sure if we have any sort of equivalent "RequestRefresh()" signal that the host document can send to OpenType glyphs, to get them to update their animation state.  The dead-simple solution here would be to add back an internal refresh driver for SVG OpenType glyphs (if we have a way of asking "are we an opentype-glyph document" -- not sure if we do?).  But that would presumably have the same performance issues that we had for images, so it's probably not ideal.

Comment 4

[Daniel Holbert [:dholbert]]

(Also: to keep this from breaking, we really need a test for this, with e.g. a SVG-in-OpenType glyph that animates a rect from red to green within the first ~100 milliseconds.  And we can use a generous timeout in the test to make sure it has enough time to load & start animating. Hacky, but I'm not sure we can do better, and this would be much better than just leaving this untested.)

Comment 5

[Daniel Holbert [:dholbert]]

Edwin (or jfkthame, or roc), do you know if there's a way for the host document's refresh driver to communicate the refresh-driver "heartbeat" through to the SVG opentype glyph's internal document, so that it can update its animation state?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We don't have anything for that at the moment. We need to add it, through gfxUserFontSet. Do you want this bug?

Comment 7

[Daniel Holbert [:dholbert]]

I could, thought it might be a day or so before I get to it.

Also: this is a new regression in Firefox 32 (which goes to release next week).  If we wanted, we might be able to back out bug 1002632, to fix this there, but it's probably too late in the game for that.  

I'm also not clear on how much this SVG-in-opentype feature is used (with animations), so I don't know how worrisome this breakage is. Given that we went this long before a bug was reported, and given that we're the only browser to support this feature (I think?), I suspect this regression won't break the web significantly in the short term, if we ship it in Firefox 32.

Might be worth considering a backout of bug 1002632 for Firefox 33, though.  (A "real" fix here would probably be too risky to land on a beta / nearly-beta channel.)

[Tracking Requested - why for this release (33)]: New regression in Firefox 32; may not have time to land a backout/fix to address it there, but could conceivably do so for Firefox 33.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Daniel Holbert [:dholbert] from comment #7)
> Might be worth considering a backout of bug 1002632 for Firefox 33, though. 
> (A "real" fix here would probably be too risky to land on a beta /
> nearly-beta channel.)

Yes please.

Comment 9

[Daniel Holbert [:dholbert]]

I backed bug 1002632 out on Trunk & current Aurora (versions 34 and 33), so those versions are now unaffected by this bug.

Let's use this bug to implement a communications channel for host-document-to-glyph refresh driver communications (per comment 5 & comment 6). Once we've got that, we can re-land bug 1002632.

Comment 10

[Sairus Patel]

Glad to see that Firefox 33 has SVG-in-OpenType animation working again. http://people.mozilla.org/~jkew/opentype-svg/soccer.html is one of very few examples that folks use to evangelize SVG-in-OpenType at during demos or at conferences, so it's important that the feature keep working. Thanks!

Sairus (CoreType, Adobe)

Comment 11

[Daniel Holbert [:dholbert]]

Sure! We should really add tests to make sure we don't break it again... Hopefully we'll do that here, when I (or someone) gets to adding the communications channel mentioned in comment 9. :)

Comment 12

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #9)
> I backed bug 1002632 out on Trunk & current Aurora (versions 34 and 33), so
> those versions are now unaffected by this bug.
> 
> Let's use this bug to implement a communications channel for
> host-document-to-glyph refresh driver communications (per comment 5 &
> comment 6). Once we've got that, we can re-land bug 1002632.

Actually, I'm spinning off bug 1107252 to cover that.  I'm going to re-land bug 1002632 with a tweak to keep it from breaking fonts, and then call this FIXED at that point, since this was a regression which will no longer be regressed.