Closed Bug 1030604 Opened 5 years ago Closed 5 years ago

SVG gradient opacity mask renders DIVElement blank when applied in some cases

Categories

(Core :: SVG, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
blocking-b2g 2.0+
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed
firefox33 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: mnjul, Assigned: dbaron)

References

Details

Attachments

(5 files)

I was doing bug 1023500 and hit this bug. Attached is the demo video using B2G "template" app in https://github.com/mnjul/gaia/tree/demo_svg_gradient_opacity_mask_div_empty (only the tip commit is relevant)

Synopsis:
when a SVG gradient mask is added onto a DIV element, it can make DIV element graphically blank under some scenario.

STR with Flame (B2G reference phone): (please refer to demo video)
1. On phone, start the "template" app provided in my github repository above
2. On desktop, start App Manager on Nightly and Debug the "template" app
3. On phone, in "template" app, click "Add an item" exactly 5 times.
3a. On the 5th click, you see that a gradient opacity mask is applied to the list container, and the container fades out to transparency at its bottom.
4. (Optional) Click "Add an item" 2 more times to make sure the mask renders things perfectly.
5. Click "Clear all items". The list should be cleared.
5a. At this moment, the mask is removed from the list container by my code.
6. Click "Add an item" exactly 5 times. At this moment, the gradient opacity mask is applied by the codes again.

Expected:
On step 6, on the 5th click, the gradient mask renders correctly and the list container fades out to transparency at its bottom.

Actual:
On step 6, on the 5th click, The list container becomes graphically blank.


Additional Info:
At step 6, when the container is blank, if you use App Manager's inspector to inspect the container element, it will suddenly re-appear. Also at step 6, when the container is blank, if you use click "Add an item" one more time, the container will suddenly re-appear too. So I think this is some graphics/rendering engine issue in gecko.

Environment:
latest gecko master and latest gaia master

This bug is also applicable to Nightly desktop browser. More info in some subsequent comment.
Attached file template_desktop.zip
This issue also happens on desktop Nightly (I am on Ubuntu 14.04 64-bit, and the nightly is 2014-06-25 33.0a1).

Check the attached file for a demo webpage and associated styles/codes. They contain almost the same codes as my demo B2G "template" app.

STR is almost the same, simplified as follows:
1. Click "Add an item" exactly 5 times.
2. Click "Clear all items".
3. Click "Add an item" exactly 5 times.

Expected:
On step 3, on the 5th click, the gradient mask renders correctly and the list container fades out to transparency at its bottom.

Actual:
On step 3, on the 5th click, The list container becomes graphically blank.
Hi Jerry,

I was told that you might be able to delegate this to somebody properly responsible for this bug. Could you check this out? Thanks.
Flags: needinfo?(hshih)
Hi Seth,
Could you give some comment about this issue? I'm not familiar with the svg rendering path.
Thanks.
Flags: needinfo?(hshih) → needinfo?(seth)
Hi David and Daniel, could you provide assistance here if you are the right person? Thank you very much! ni Ivan since the component is set to Graphics.
blocking-b2g: --- → 2.0?
Flags: needinfo?(itsay)
Flags: needinfo?(dholbert)
Flags: needinfo?(dbaron)
(In reply to John Lu [:mnjul] [Out-of-Office 7/11~7/20] [MoCoTPE] from comment #1)
> Created attachment 8446359 [details]
[...]
> STR is almost the same, simplified as follows:
> 1. Click "Add an item" exactly 5 times.

After this step (the first time I see some transparency, on the bottom-most item), I get two instances of this assertion, in my debug build:
###!!! ASSERTION: How did we getting here, then?: 'aFrame->GetParent()->StyleContext()->GetPseudo() == nsCSSAnonBoxes::mozAnonymousBlock', file layout/svg/nsSVGIntegrationUtils.cpp, line 111
http://mxr.mozilla.org/mozilla-central/source/layout/svg/nsSVGIntegrationUtils.cpp?rev=fb2ab91ddd34#109

It seems likely that any brokenness after that point is related to that.

(Bug 842114 is also filed on a testcase that triggers this assertion. May be related.)
See Also: → 842114
Summary: SVG gradient opacity mask renders DIVElement blank when applied in some cases → SVG gradient opacity mask renders DIVElement blank when applied in some cases, after "###!!! ASSERTION: How did we getting here, then?"
This is actually a blocker, without this being fixed, notifications disappear / appear when we have more than 4. Another demo: https://www.youtube.com/watch?v=hZzvkitPjXw&feature=youtu.be

So Daniel, Greg, could we try the patch in bug 842114 to see if it solves this issue? Thank you!
blocking-b2g: 2.0? → 2.0+
Flags: needinfo?(gweng)
Did this go through full triage of the release drivers to get 2.0+, or Howie, did you just assume this to be the case? I only saw the "use the template app and put together a test case", I don't see a shipping or a partner app getting in trouble.  Is there another test case?
blocking-b2g: 2.0+ → 2.0?
Component: Graphics → SVG
(In reply to howie [:howie] from comment #6)
> 
> So Daniel, Greg, could we try the patch in bug 842114 to see if it solves
> this issue? Thank you!

The patch in that build is not a fix, it's just diagnostics trying to provide more details when something goes wrong.
s/build/bug/
(In reply to Milan Sreckovic [:milan] from comment #7)
> Did this go through full triage of the release drivers to get 2.0+, or
> Howie, did you just assume this to be the case? I only saw the "use the
> template app and put together a test case", I don't see a shipping or a
> partner app getting in trouble.  Is there another test case?

Hi, while I understand a full triage is necessary for blocker+, I think Howie mark it because Bug 1023500 is a blocker, which is mainly for notification on LockScreen, and this bug would make it totally broken even with the visual update patch. This video would show after the Bug 1023500 got patched, the notification would still be broken since this bug is still there:

https://www.youtube.com/watch?v=ON_qVIkvCW4&feature=youtu.be

(Sorry for the annoying pan moving) 

You can see the notifications suddenly disappear as the first comment describing.
The symptom would disappear after rebooting. It would appear again if the screen got black out.
Flags: needinfo?(gweng)
blocking-b2g: 2.0? → 2.0+
Attached file testcase 1
Here's a somewhat-reduced standalone testcase.

(I've become less-convinced that the assertion is related to the root problem here, too, because while working on this testcase, I had some dead-ends where the assertion did fire, but everything painted correctly.)
Flags: needinfo?(dholbert)
Here's an automated testcase, which starts out with a masked child, removes the mask & child, and then adds them back (with some carefully-placed reflows via "offsetHeight").

This one doesn't trigger the assertion from comment 5, so that assertion is indeed unrelated & should no longer be considered here. (sorry for that red herring)
See Also: 842114
Summary: SVG gradient opacity mask renders DIVElement blank when applied in some cases, after "###!!! ASSERTION: How did we getting here, then?" → SVG gradient opacity mask renders DIVElement blank when applied in some cases
So one thing that seems suspicious:

ComputeEffectsRect in nsFrame.cpp (a helper function for nsIFrame::FinishAndStoreOverflow) stores a PreEffectsBBoxProperty and then adjusts the overflow rect for the effects.  Dynamic changes to the mask property don't trigger UpdateOverflow.  It seems to me that all properties for which nsSVGIntegrationUtils::UsingEffectsForFrame returns true should return nsChangeHint_UpdateOverflow.  Currently we do this for filters but not the others.
Yes, adding an UpdateOverflow hint fixes it; I'll clean up the patch.
Assignee: nobody → dbaron
Flags: needinfo?(seth)
Flags: needinfo?(dbaron)
(In reply to Daniel Holbert [:dholbert] from comment #12)
> Created attachment 8456312 [details]
> testcase 2 (automated; says "PASS" if passed)
> 
> Here's an automated testcase, which starts out with a masked child, removes
> the mask & child, and then adds them back (with some carefully-placed
> reflows via "offsetHeight").

Thanks; this testcase was useful.

> This one doesn't trigger the assertion from comment 5, so that assertion is
> indeed unrelated & should no longer be considered here. (sorry for that red
> herring)

I think the assertion was somewhat related; I think the assertion and the bug are both symptoms of the same problem, which is failure to keep the PreEffectsBBoxProperty up-to-date.
The testcase is a slight simplification of dholbert's testcase 2
(attachment 8456312 [details]) in the bug.  It fails in the reftest harness
without the patch, and passes in the reftest harness with the patch.
Attachment #8456519 - Flags: review?(roc)
Flags: needinfo?(itsay)
(In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from comment #15)
> I think the assertion was somewhat related; I think the assertion and the
> bug are both symptoms of the same problem, which is failure to keep the
> PreEffectsBBoxProperty up-to-date.

For the record: I can confirm that the attached patch fixes the assertion as well as the testcase on the attached "testcase 1" & "template_desktop.zip" (and it fixes the rendering bug in all testcases here), but it does not fix the assertion on bug 842114's testcase. (attachment 714873 [details])

So, that bug should remain open.
https://hg.mozilla.org/mozilla-central/rev/8dd8407fd66b
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.