Firefox 48 on Windows 10 hangs on an SVG file with hardware acceleration enabled

RESOLVED FIXED in Firefox 50

Status

()

Core
Graphics
--
critical
RESOLVED FIXED
2 years ago
8 months ago

People

(Reporter: zoltan, Assigned: bas, NeedInfo)

Tracking

(Depends on: 1 bug, {hang, regression})

48 Branch
mozilla51
hang, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 wontfix, firefox49 wontfix, firefox-esr45 unaffected, firefox50 fixed, firefox51 fixed)

Details

(Whiteboard: [gfx-noted])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Description

2 years ago
Created attachment 8779234 [details]
poli-brand-logo.svg

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20160804004004

Steps to reproduce:

A web page I developed caused Firefox 48 on Windows 10 to hang. I removed lines of code until I arrived at a specific SVG file that caused it. Loading the SVG file by itself also caused the hang. Firefox 48 on Mac OS X renders the SVG. With hardware acceleration off, the SVG renders. It happened with this specific SVG (attached), other SVG files loaded fine.


Actual results:

Firefox didn't load the page, displayed (Not responding) in the window title, kept using CPU.


Expected results:

The SVG should have rendered.
sounds similar issue to bug 1293134.
Component: Untriaged → SVG

Comment 2

2 years ago
Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=25d4e570f1d983ecc112e835d8a23eeafa8cd684&tochange=a9ec5effe35f607793514fe3688efb3ba8096689

Suspect: Bug 1248913
Blocks: 1248913
Severity: normal → critical
Status: UNCONFIRMED → NEW
status-firefox48: --- → wontfix
status-firefox49: --- → affected
status-firefox50: --- → affected
status-firefox51: --- → affected
status-firefox-esr45: --- → unaffected
Ever confirmed: true
Flags: needinfo?(mstange)
Keywords: hang, regression
zoltan, can you paste your graphics section from about:supports into this bug?
Flags: needinfo?(mstange)
I can reproduce this on Windows 10 with Direct2D 1.1. I don't need the about:supports information any more.
Summary: Firefox 48 on Windows 10 crashes on an SVG file with hardware acceleration enabled → Firefox 48 on Windows 10 hangs on an SVG file with hardware acceleration enabled
Component: SVG → Graphics

Updated

2 years ago
See Also: → bug 1293134

Updated

2 years ago
Duplicate of this bug: 1293134
So this just does a lot of waiting in Direct2D, for multiple minutes. Bas, can you take a look?

Before bug 1238496, we were doing blend modes using BasicLayers always, which would have used D2D. Then after bug 1238496, we were doing blend modes on the compositor always. Then bug 1248913 made us do blend modes on the compositor only when necessary. There are no active layers being blended in the testcase in this bug, so we do the blending using BasicLayers with Direct2D.
My guess is that something changed in how we render blend modes with Direct2D between the landings of bug 1238496 and bug 1248913, i.e. between 2016-02-03 and 2016-04-07.
Flags: needinfo?(bas)
(Assignee)

Comment 7

2 years ago
(In reply to Markus Stange [:mstange] from comment #6)
> So this just does a lot of waiting in Direct2D, for multiple minutes. Bas,
> can you take a look?
> 
> Before bug 1238496, we were doing blend modes using BasicLayers always,
> which would have used D2D. Then after bug 1238496, we were doing blend modes
> on the compositor always. Then bug 1248913 made us do blend modes on the
> compositor only when necessary. There are no active layers being blended in
> the testcase in this bug, so we do the blending using BasicLayers with
> Direct2D.
> My guess is that something changed in how we render blend modes with
> Direct2D between the landings of bug 1238496 and bug 1248913, i.e. between
> 2016-02-03 and 2016-04-07.

Nothing really happened in there except the removal of D2D 1.0, but that didn't really affect how we do blending. I'll have a look.
Flags: needinfo?(bas)
(Assignee)

Comment 8

2 years ago
(In reply to Markus Stange [:mstange] from comment #6)
> So this just does a lot of waiting in Direct2D, for multiple minutes. Bas,
> can you take a look?
> 
> Before bug 1238496, we were doing blend modes using BasicLayers always,
> which would have used D2D. Then after bug 1238496, we were doing blend modes
> on the compositor always. Then bug 1248913 made us do blend modes on the
> compositor only when necessary. There are no active layers being blended in
> the testcase in this bug, so we do the blending using BasicLayers with
> Direct2D.
> My guess is that something changed in how we render blend modes with
> Direct2D between the landings of bug 1238496 and bug 1248913, i.e. between
> 2016-02-03 and 2016-04-07.

This is related to the PushGroup/PopGroup work that we did. But this landed in November 2015 already.
Comment hidden (mozreview-request)

Comment 10

2 years ago
mozreview-review
Comment on attachment 8781007 [details]
Bug 1293586: Don't use command lists for an effect when that command list already has an effect with a command list used inside of it.

https://reviewboard.mozilla.org/r/71510/#review69170

I'm not the best person to review this code, but the logic seems sound.

::: gfx/2d/DrawTargetD2D1.cpp:1098
(Diff revision 1)
>  #ifdef _DEBUG
>    options.debugLevel = D2D1_DEBUG_LEVEL_WARNING;
>  #else
>    options.debugLevel = D2D1_DEBUG_LEVEL_NONE;
>  #endif
> -  //options.debugLevel = D2D1_DEBUG_LEVEL_INFORMATION;
> +  options.debugLevel = D2D1_DEBUG_LEVEL_INFORMATION;

Left over from debugging?
Attachment #8781007 - Flags: review?(mstange) → review+
Comment hidden (mozreview-request)

Updated

2 years ago
Whiteboard: [gfx-noted]
Comment hidden (mozreview-request)
(Assignee)

Comment 13

2 years ago
Created attachment 8782121 [details]
Minimal testcase
Assignee: nobody → bas
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 18

2 years ago
Pushed by bschouten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f0e64326ce2b
Don't use command lists for an effect when that command list already has an effect with a command list used inside of it. r=mstange

Comment 19

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f0e64326ce2b
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Bas please consider uplift to 50 and 49.
Flags: needinfo?(bas)
(Assignee)

Comment 21

2 years ago
Comment on attachment 8781007 [details]
Bug 1293586: Don't use command lists for an effect when that command list already has an effect with a command list used inside of it.

Approval Request Comment
[Feature/regressing bug #]: 1220629
[User impact if declined]: Hang when certain blending features are used.
[Describe test coverage new/current, TreeHerder]: Nightly test coverage
[Risks and why]: Fairly low, some small internal changes theoretically leading to slightly higher memory usage and small performance hit in some specific usecases.
[String/UUID change made/needed]: None

I think this patch is too risky to uplift to beta in this stage of the cycle.
Flags: needinfo?(bas)
Attachment #8781007 - Flags: approval-mozilla-aurora?
Thanks for your careful consideration Bas. Marking wontfix for 49.
status-firefox49: affected → wontfix

Comment 23

2 years ago
Am not sure how to read "status: resolved fixed // wontfix" on this one. Does it mean that the instance I reported at http://cs.sru.edu/~ddailey/svg/modifier3RBlendSnapMode.svg won't be fixed?
Hello Zoltan, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(zoltan)
(In reply to david.dailey from comment #23)
> Am not sure how to read "status: resolved fixed // wontfix" on this one.
> Does it mean that the instance I reported at
> http://cs.sru.edu/~ddailey/svg/modifier3RBlendSnapMode.svg won't be fixed?

Hello David, this issue will not be backported to Fx48 (current release version) and neither will it be fixed in Fx49 (next release version, supposed to go live in ~10 days). However, this issue should be fixed in Firefox version 50 and on wards. Hope that helps!
Comment on attachment 8781007 [details]
Bug 1293586: Don't use command lists for an effect when that command list already has an effect with a command list used inside of it.

Fixed a hang in specific scenarios, Aurora50+
Attachment #8781007 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 27

2 years ago
(In reply to Ritu Kothari (:ritu) from comment #25)
> (In reply to david.dailey from comment #23)
> > Am not sure how to read "status: resolved fixed // wontfix" on this one.
> > Does it mean that the instance I reported at
> > http://cs.sru.edu/~ddailey/svg/modifier3RBlendSnapMode.svg won't be fixed?
> 
> Hello David, this issue will not be backported to Fx48 (current release
> version) and neither will it be fixed in Fx49 (next release version,
> supposed to go live in ~10 days). However, this issue should be fixed in
> Firefox version 50 and on wards. Hope that helps!

Absolutely! Thanks!

Comment 28

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/8a65412e524b
status-firefox50: affected → fixed
(Assignee)

Updated

2 years ago
See Also: → bug 1300338

Updated

a year ago
Depends on: 1332558
See Also: → bug 1392453
You need to log in before you can comment on or make changes to this bug.