1170229 - Investigate making the layers preference a "live" preference

Status: RESOLVED WONTFIX

(Core :: Graphics, defect)

Description

[Mason Chang [Inactive] [:mchang]]

+++ This bug was initially created as a clone of Bug #1156135 +++

From https://bugzilla.mozilla.org/show_bug.cgi?id=1156135#c78

Currently, the layers acceleration preference is a "Once" preference, in that it is set at Gecko startup and assumed to be the same value the whole lifetime of the process. Investigate / change the preference to be a "Live" preference so that we can dynamically test layers acceleration in bug 1156135.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

This doesn't sound easy. Others, might have more concrete reasons why.

In the olden days we sort of did this on startup on Windows, but that was somewhat restricted in scope. Perhaps this is achievable with some restrictions.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

My idea was to have a semi-live pref. It would only affect widgets that haven't yet created a layer manager. If the pixel sanity test check runs before the main window is painted, that should be good enough.

Comment 3

[Robert Kaiser]

I remember we once had this thing that launched a separate process, tried to init stuff there and if it crashed, we turned off the feature in the main process (as a crash there would be fatal to the whole application). Could we do a test-in-separate-process approach here as well?

Comment 4

[Mason Chang [Inactive] [:mchang]]

(In reply to Robert Kaiser (:[e-mail]) from comment #3)
> I remember we once had this thing that launched a separate process, tried to
> init stuff there and if it crashed, we turned off the feature in the main
> process (as a crash there would be fatal to the whole application). Could we
> do a test-in-separate-process approach here as well?

I'd like to try that, maybe also during installation time if having an actual "live" preference doesn't work out.

Comment 5

[Mason Chang [Inactive] [:mchang]]

I've been testing just having layers acceleration be a life preference. It actually seems to work most of the time on Windows and Mac, especially since the LayerManager is only associated with the widget. The only reliable crash I get is this - https://crash-stats.mozilla.com/report/index/c9c60c61-d9ce-4c36-9975-5c6f72150602

STR (with layers.acceleration.disabled as a live pref locally)
1) Disable layers acceleration
2) Ensure basic compositor
3) Start playing a youtube video (has to be the actual video, not the ad interestingly enough)
4) Enable layers acceleration
5) Open a new Firefox window, crash

Maybe for runtime testing, we can create a new widget explicitly with layers acceleration and test only that widget instead of fixing all the "live" bugs as having separate widgets w/ and w/o acceleration outside of video applications seems to be ok. Still digging into the crash.

Comment 6

[(Away)]

> https://crash-stats.mozilla.com/report/index/c9c60c61-d9ce-4c36-9975-5c6f72150602

For anyone following along, here's the abort message (since crash-stats doesn't have symbols for private builds):

0c52e874  "[4288] ###!!! ABORT: Can't (easi"
0c52e894  "ly) support component alpha with"
0c52e8b4  " BasicCompositor!: file c:/Users"
0c52e8d4  "/mozilla/Projects/gecko-dev/gfx/"
0c52e8f4  "layers/basic/BasicCompositor.cpp"
0c52e914  ", line 419"

Comment 7

[Nicolas Silva [:nical]]

We have some code all over gfx/layers that relies on the backends never changing, so I fear that things work more out of luck than proper design and engineering :) I also fear that when we ask bug reporters to try to disable acceleration we'll generate crashes that are specific to changing the prefs.

It's probably silly but maybe we could start by making it live only in nightly and have it NOT ride the trains? I'd rather not have to deal with the possibility that some crash reports are caused by changing the backend at runtime (vs actual bugs in the backends).

Comment 8

[Mason Chang [Inactive] [:mchang]]

When we've had layers acceleration problems, what specifically caused the problems? Was it just using the GPU at all caused crashes, so any GPU utilization would crash and that's what we need to test? Or was it a specific feature set of the GPU? It seems like making layers live is not only risky and dangerous, but it's because we just want to test something and I'm having a hard time imagining why a user would want a live preference. Nor do I think having mixed backends for different windows actually wins anything but can cause lots of headaches.

@nical - What kind of bugs has layer acceleration caused before and do we know why? Is it possible to narrow it down to a few specific GPU features we could test instead? Thanks!

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

I'd like to make sure we avoid "tail wagging the dog".  If we have a valid reason for layers acceleration changing during the session, without the "test if things work" scenario that restarted this conversation, than I'm all for it, and we should figure out how to do it.  If, on the other hand, the only reason we'd want this is for that test on start up, that happens only in limited situations (e.g., driver or version update), then it seems a wrong thing to burden the system with, over-complicate the code, and only use it in those situations.

Comment 10

[Nicolas Silva [:nical]]

(In reply to Mason Chang [:mchang] from comment #8)
> When we've had layers acceleration problems, what specifically caused the
> problems? Was it just using the GPU at all caused crashes, so any GPU
> utilization would crash and that's what we need to test? Or was it a
> specific feature set of the GPU? It seems like making layers live is not
> only risky and dangerous, but it's because we just want to test something
> and I'm having a hard time imagining why a user would want a live
> preference. Nor do I think having mixed backends for different windows
> actually wins anything but can cause lots of headaches.
> 
> @nical - What kind of bugs has layer acceleration caused before and do we
> know why? Is it possible to narrow it down to a few specific GPU features we
> could test instead? Thanks!

We have loads of driver issues, and when they are reported we usually ask reporters to try different drawing and compositing backends, to see in which configurations problems go away.

Ideally we'd figure out all of the driver issues and once the blacklist is updated this would be a concern of the past, but new drivers keep being released and they come up with new and exciting bugs each time. In particular, OMTC on Windows seems to exercise a lot of poorly tested features of D3D (loads of fun around cross-thread texture sharing and synchronization).

The problem with a live pref for the backend is that if we don't have some logic to carefully destroy all resources of the previous backend before starting to use the new one, we can easily end up with backend B trying to read or write into resources created by backend A and that'll blow up in interesting ways.

I am not saying that we should not support this, in fact we have some rare but worrying cases where a backend stops working and fails to be reinitialized, and for these cases the only sensible things to do are either crash (sad) or switch to another backend at runtime. When I looked into how to do the latter in a safe way, it appeared to be tricky.

Comment 11

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Nicolas Silva [:nical] from comment #10)
> (In reply to Mason Chang [:mchang] from comment #8)
> > When we've had layers acceleration problems, what specifically caused the
> > problems? Was it just using the GPU at all caused crashes, so any GPU
> > utilization would crash and that's what we need to test? Or was it a
> > specific feature set of the GPU? It seems like making layers live is not
> > only risky and dangerous, but it's because we just want to test something
> > and I'm having a hard time imagining why a user would want a live
> > preference. Nor do I think having mixed backends for different windows
> > actually wins anything but can cause lots of headaches.
> > 
> > @nical - What kind of bugs has layer acceleration caused before and do we
> > know why? Is it possible to narrow it down to a few specific GPU features we
> > could test instead? Thanks!
> 
> We have loads of driver issues, and when they are reported we usually ask
> reporters to try different drawing and compositing backends, to see in which
> configurations problems go away.
> 
> Ideally we'd figure out all of the driver issues and once the blacklist is
> updated this would be a concern of the past, but new drivers keep being
> released and they come up with new and exciting bugs each time. In
> particular, OMTC on Windows seems to exercise a lot of poorly tested
> features of D3D (loads of fun around cross-thread texture sharing and
> synchronization).
> 
> The problem with a live pref for the backend is that if we don't have some
> logic to carefully destroy all resources of the previous backend before
> starting to use the new one, we can easily end up with backend B trying to
> read or write into resources created by backend A and that'll blow up in
> interesting ways.
> 
> I am not saying that we should not support this, in fact we have some rare
> but worrying cases where a backend stops working and fails to be
> reinitialized, and for these cases the only sensible things to do are either
> crash (sad) or switch to another backend at runtime. When I looked into how
> to do the latter in a safe way, it appeared to be tricky.

We are not proposing that flipping the pref change rendering modes for existing widgets - only new widgets that have not yet asked for a layer manager. Are there system-wide places that depend on the value of LayersAccelerationDisabled not changing?

Comment 12

[Nicolas Silva [:nical]]

Ok that's less scary all of a sudden.
There is Compositor::sBackend which we really need to remove. Also we have to go through the places where we read from the pref and make sure it happens only once at widget creation, and that we always ask the widget afterwards. There is also the problem of incompatible drawing/compositing backend pairs. The choice of the drawing backend is global. It's not the end of the world if we let users choose incompatible backends but it  generates a some crashes every now and then. I had a patch that prevented that but it never got reviewed. It'll be a bit harder to prevent users from using, say, D2D if they are using the basic compositor once the compositor backend can change while the drawing backend is set in stone until restart.
But at this point I am nit picking. I am fine with having a compositor backend per widget if we support it properly.

Comment 13

[Mason Chang [Inactive] [:mchang]]

After some thought, I don't think we want to make layers live. The only strong use case is so that we don't have to restart Firefox in the cases where layers acceleration did not work with the start up test. Having multiple different backends in a normal configuration adds a lot of engineering complexity and no strong use case for why the user would want this. Resolving as WONTFIX for now.