Closed Bug 1281428 Opened 3 years ago Closed 3 years ago

SVG image with animated opacity and a fill filter is blank until hovered

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla50
Tracking Status
firefox47 --- unaffected
firefox48 --- unaffected
firefox49 --- unaffected
firefox50 + verified
firefox51 --- verified
firefox52 --- verified

People

(Reporter: florian, Assigned: mattwoodrow)

References

Details

(Keywords: regression)

Attachments

(4 files)

The animation I'm adding in bug 1275262 stopped working recently. In my fx-team build from 2016-06-10 it worked fine; in Gijs' fx-team build from 2016-06-13 he observed that the camera icon was just blank (I now reproduced the same issue on a current fx-team build), and reappeared after hovering it.
Summary: SVG image with animated opacity is blank until hovered → SVG image with animated opacity and a fill filter is blank until hovered
Attached image test.svg
Attached file test.xul
Steps to reproduce:
1. Download the 2 test files (test.svg and test.xul) into the same folder.
2. In about:config, set dom.allow_XUL_XBL_for_file to true.
3. Load file://.../test.xul

Expected result:
Animated (blinking/pulsing) camera icon.

Actual result:
Blank page until the mouse is moved above it. Once the mouse is moved, the animation starts working. Reloading the page (Cmd+R) makes the page look blank again.


Note: replacing the fill filter with fill="rgb(...)" directly on the SVG path element hides the bug.
Flags: needinfo?(jwatt)
It seems like dom.allow_XUL_XBL_for_file doesn't work any more. I get the same error message after loading the testcase locally as I do loading it from bmo:

  This page uses an unsupported technology that is no longer
  available by default in Firefox.

  Please contact the website owners to inform them of this problem

Can you create an HTML testcase, Florian?
Flags: needinfo?(jwatt) → needinfo?(florian)
(In reply to Jonathan Watt [:jwatt] (Away Jun. 27 - Jul. 13) from comment #4)
> It seems like dom.allow_XUL_XBL_for_file doesn't work any more.

I used it to get the regression range using mozregression, so I can guarantee you that it works.
Flags: needinfo?(florian)
That doesn't help me at all...
You may want to try with e10s disabled, as I'm not sure how about:config propagates preferences changes to the content process.
Attached file test.html
I managed to reproduce with an HTML testcase, but it isn't as reliable as the XUL one. To reproduce I click randomly once or twice on the blinking camera icon or outside of it (alternatively), and sometimes the animation stops (the icon disappears) after the click. Moving the mouse makes the animation restart.
The behavior has changed again recently. Now the animation doesn't stop, but the opacity is either 0% or 100%, we don't get anything in between.

mozregression shows https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=067521ccd17907843e1db644ca0f02a1f5a6520a&tochange=eac0c056235ee1174fc092fe159d2d5710331fbd
-> bug 1283827
Blocks: 1283827
Flags: needinfo?(matt.woodrow)
I managed to reproduce this a few times in my Nightly, but can't do so in a local build.

Does this fix the problem?: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e58a5372e62d
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(florian)
(In reply to Matt Woodrow (:mattwoodrow) from comment #10)
> I managed to reproduce this a few times in my Nightly, but can't do so in a
> local build.
> 
> Does this fix the problem?:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=e58a5372e62d

No, it doesn't (I applied the patch locally to have results quickly without waiting for try).
Flags: needinfo?(florian)
Blocks: 1263829
No longer depends on: 1263829
Version: unspecified → Trunk
Tracking 50+ for this SVG animation regression.
Blocks: 1206233
(In reply to Matt Woodrow (:mattwoodrow) from comment #10)
> I managed to reproduce this a few times in my Nightly, but can't do so in a
> local build.

Are you still unable to reproduce? Bug 1206233 will make this even more visible.
Flags: needinfo?(matt.woodrow)
Not yet, but I just added a new patch in bug 1287122 that is quite likely related. Does that patch (or more likely, both of them) fix this issue?
Flags: needinfo?(matt.woodrow)
(In reply to Matt Woodrow (:mattwoodrow) from comment #14)
> Not yet, but I just added a new patch in bug 1287122 that is quite likely
> related. Does that patch (or more likely, both of them) fix this issue?

attachment 8774197 [details] [diff] [review] from bug 1287122 fixes the regression from bug 1283827 that I observed in comment 9 here, and reverts the behavior to what was described in comment 1 to 8, ie. the regression from bug 1263829 remains.

When you said "both of them", I assume you meant attachment 8774197 [details] [diff] [review] + the patch in the try build at comment 10. Adding this second patch doesn't (visibly) change the behavior.
I managed to reproduce it with the XUL testcase, needed a 3rd fix :)
Assignee: nobody → matt.woodrow
Attachment #8775384 - Flags: review?(jwatt)
Comment on attachment 8775384 [details] [diff] [review]
Don't skip building layers if we're not actually supposed to consider the opacity

Review of attachment 8775384 [details] [diff] [review]:
-----------------------------------------------------------------

> I managed to reproduce it with the XUL testcase, needed a 3rd fix :)

\o/
Attachment #8775384 - Flags: review?(jwatt) → review+
Comment on attachment 8775384 [details] [diff] [review]
Don't skip building layers if we're not actually supposed to consider the opacity

Review of attachment 8775384 [details] [diff] [review]:
-----------------------------------------------------------------

I can confirm that applying attachment 8774197 [details] [diff] [review] from bug 1287122 + this patch fixes the animation issue :-). Thanks!

Now it would be great if all these fixes could land before 50 merges to aurora.

::: layout/svg/nsSVGIntegrationUtils.cpp
@@ +632,5 @@
>         aParams.callerPaintsOpacity)) {
>      opacity = 1.0f;
>    }
> +  if (opacity == 0.0f) {
> +    return;

This is |return DrawResult::SUCCESS;| since bug 1258510.
Attachment #8775384 - Flags: feedback+
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bd88c9c8068
Don't skip paint painting SVG effects due to 0 opacity if the opacity is being handled by a separate item. r=jwatt
https://hg.mozilla.org/mozilla-central/rev/2bd88c9c8068
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Depends on: 1287122
Depends on: 1291522
Verified as fixed on Nightly 52.0a1 -20160920030429, Aurora 51.0a2 - 20160920004004 and Beta 50.0b1-20160920155715.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.