1555098 - Double buffering changes lead to failure to init D3D11

Status: RESOLVED FIXED

(Core :: Graphics, defect, P2)

Description

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: About support from a build with double buffering.

Bug 1547775 appears to regress D3D11 handling on my dev machine: a Lenovo P51 with NVIDIA Quadro M2200 (latest vendor drivers). This can be seen in about:support where we see graphics errors and a fall back to basic compositing.

Attaching an about support from an affected build. Will follow up with one from a happy build for comparison.

Comment 1

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: About support from a build without double buffering

Comment 2

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

NI Bas based on regressing bug.

Comment 3

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

This seems related to driving an external monitor from my laptop. If I unplug my external monitor and start FireFox I don't get the D3D11 issues and get D3D11 compositing.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

This seems more important than a P3 and somehow I doubt Bas will respond to the needinfo. Jeff, thoughts?

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

Presumably we would see a reduction in users getting D3D since this causes one to regress to basic. If it happens frequently enough, I would agree it is probably a P1. Setting to P2 for now.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

I'm inclined to disable double buffering in 68 until we figure out what's going on here.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #6)

I'm inclined to disable double buffering in 68 until we figure out what's going on here.

Assuming you meant exactly what you said (leave it enabled in 69 Nightly, but disable on 68) I'm going to generalize that a bit and only leave it on in Nightly builds. I don't want to have to deal with uplifting a disable patch to beta every train. Once we sort out the problems we can re-enable it on the trains.

Comment 8

[Bas Schouten (:bas.schouten)]

Attachment: Bug 1555098: Fall back to non-double buffering when double buffering swap chain creation fails in CompositorD3D11, the way we do in advanced layers. r=jrmuizel

Comment 9

[Bas Schouten (:bas.schouten)]

This patch should solve the issue.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c7c89a50c19
Fall back to non-double buffering when double buffering swap chain creation fails in CompositorD3D11, the way we do in advanced layers. r=jrmuizel

Comment 11

[Bas Schouten (:bas.schouten)]

Comment on attachment 9069857 [details]
Bug 1555098: Fall back to non-double buffering when double buffering swap chain creation fails in CompositorD3D11, the way we do in advanced layers. r=jrmuizel

Beta/Release Uplift Approval Request

• User impact if declined: Inability to re-enable double buffering causing increased power usage and reduced performance.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This causes us to use the same codepath in our blacklisted-for-advanced-layers codepath as we use in our regular, well-tested, more widely used codepath.
• String changes made/needed: None

Comment 12

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/3c7c89a50c19

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 9069857 [details]
Bug 1555098: Fall back to non-double buffering when double buffering swap chain creation fails in CompositorD3D11, the way we do in advanced layers. r=jrmuizel

aiui d3d11 double buffering is disabled in 68 so we shouldn't need this

Comment 14

[Bas Schouten (:bas.schouten)]

(In reply to Julien Cristau [:jcristau] from comment #13)

Comment on attachment 9069857 [details]
Bug 1555098: Fall back to non-double buffering when double buffering swap chain creation fails in CompositorD3D11, the way we do in advanced layers. r=jrmuizel

aiui d3d11 double buffering is disabled in 68 so we shouldn't need this

The intent would be to re-enable double buffering since it's a very large performance win on a wide variety of machine and a big power saver.

Comment 15

[Kartikaya Gupta (email:[e-mail])]

FWIW I would be opposed to enabling it on release without having somebody officially assigned to deal with any fallout. Given this bug is fixed it might be acceptable to turn it back on in EARLY_BETA_OR_EARLIER builds until we have somebody assigned.

Comment 16

[Julien Cristau [:jcristau]]

Enabling double buffering isn't something we should be doing 2 weeks before shipping, so it's not an option for 68 at this point anyway.

Comment 17

[Bas Schouten (:bas.schouten)]

(In reply to Julien Cristau [:jcristau] from comment #16)

Enabling double buffering isn't something we should be doing 2 weeks before shipping, so it's not an option for 68 at this point anyway.

This was enabled for most of the Beta. Just being disabled for a fairly small bug for a very small group of people.

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #15)

FWIW I would be opposed to enabling it on release without having somebody officially assigned to deal with any fallout. Given this bug is fixed it might be acceptable to turn it back on in EARLY_BETA_OR_EARLIER builds until we have somebody assigned.

That sounds very reasonable. In the interest of the quality of our product it would be great if an owner for this was found within Graphics. It would be sad if the entire platform team had rallied around pageload performance, herculean efforts have been made to get a couple of percent wins left and right and we have a very easy win lying around that not only makes a considerably improvement to pageload but also makes scrolling and video playback considerably better.

Comment 18

[Kartikaya Gupta (email:[e-mail])]

(In reply to Bas Schouten (:bas.schouten) from comment #17)

In the interest of the quality of our product it would be great if an owner for this was found within Graphics.

Agreed. I'm guessing Sotaro is the most appropriate person given that he's fixing most of our windows-specific bugs these days, but I don't know what else he has on his plate for the near future. I also don't know how to estimate the amount of fallout we might reasonably expect from this feature because I don't know the details about how it works and what kind of compatibility pitfalls it might have. Passing the buck to Jessie :)

Comment 19

[Kartikaya Gupta (email:[e-mail])]

(Also note that bug 1554610 still needs fixing one way or another)

Comment 20

[Jessie [:jbonisteel] pls NI]

Will discuss some options during All Hands and update everyone afterwards

Comment 21

[Bas Schouten (:bas.schouten)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #19)

(Also note that bug 1554610 still needs fixing one way or another)

The best thing to do is probably just to blacklist VirtualBox, maybe just double buffering but potentially just acceleration altogether, I'm highly skeptical of whether it would provide a lot of value on something like VirtualBox. I can help out with that if need be.

Comment 22

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Should the result of this patch be that if double buffering fails I should see a fallback where I still use D3D11 compositing (as reported in about:support)? I am still getting basic compositing unless I explicitly disable double buffering. If I disable double buffering I get 'Direct3D 11 (Advanced Layers)'.

Comment 23

[Sotaro Ikeda [:sotaro]]

On current gecko, double buffering is disabled when creation of IDXGISwapChain1 was failed. On NVIDIA Quadro M2200, IDXGISwapChain1 creation seemed to be succeeded, but it did not work and it failed to present with "D3D11 swap chain preset failed".

To address the problem, there are 2 choices
[1] black list "NVIDIA Quadro M2200" for double buffering
[2] Disable double buffering when device reset happens.

[2] seems reasonable in this use case.

Comment 24

[Sotaro Ikeda [:sotaro]]

Created Bug 1562847 by comment 23.

Comment 25

[Sotaro Ikeda [:sotaro]]

:bryce, does the problem still happens on latest nightly?

Comment 26

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

It does not. I am getting Direct3D 11 (Advanced Layers) without needing to set gfx.direct3d11.use-double-buffering to false.

Comment 27

[Sotaro Ikeda [:sotaro]]

Thanks for the confirmation!