1136242 - http://phoboslab.org/xibalba flickers black or is all black on NVIDIA GPUs with D3D11 ANGLE

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Comment 1

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

Could be a duplicate of bug 1136172, but too early to tell.

Comment 2

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

Note from xibalba developers:

The flickering of the WebGL Context only occurs when there are other (2D) Canvas Contexts on the same page. I guess some internal OpenGL state in Firefox is not properly reset when switching to and from the WebGL Context to the 2D Contexts.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

It looks like this is caused by preserveDrawingBuffer = true being broken on Nvidia with D3D11 ANGLE

Comment 4

[Jeff Muizelaar [:jrmuizel]]

I think we're not properly acquiring the mutex in this case.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Attachment: Make sure we acquire the mutexes when copying the surfaces.

Comment 6

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

Comment on attachment 8568720 [details] [diff] [review]
Make sure we acquire the mutexes when copying the surfaces.

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

::: gfx/gl/SharedSurface.cpp
@@ +87,2 @@
>              src->UnlockProd();
> +            src->ProducerRelease();

I haven't even looked to see what the functions do, but is the order of these two calls relevant?  Lock + Acquire is OK to be handled by Unlock + Release, rather than Release + Unlock?

Comment 7

[Jeff Muizelaar [:jrmuizel]]

[Tracking Requested - why for this release]: This is a regression from bug 1079398

Comment 8

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8568720 [details] [diff] [review]
Make sure we acquire the mutexes when copying the surfaces.

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

::: gfx/gl/SharedSurface.cpp
@@ +59,4 @@
>              srcNeedsUnlock = true;
>          }
>  
> +        dest->ProducerAcquire();

You don't want to acquire this if it's already required. Is this possible?

@@ +87,2 @@
>              src->UnlockProd();
> +            src->ProducerRelease();

I don't think it matters in practice, but Acquire before Lock, Unlock before Release makes sense.

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

Tracking 37 gfx regression.

Jeff- Assigning to you as it looks like you're on this one.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Attachment: An alternative approach

This version is much simpler and leaves the locking up to the caller. It removes the 'Fence before copying.' comment and does the fencing after the copy. I can't see why the fencing would need to happen before the copy. The regular texture read barrier should be sufficient.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

The try server results look good:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e84325cccb2c

Comment 12

[Jeff Muizelaar [:jrmuizel]]

Review ping.

Comment 13

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8570639 [details] [diff] [review]
An alternative approach

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

This will regress performance on non-windows platforms, but it's probably our best to regain correctness on beta.

This only regresses perf with preserveDrawingBuffer:true, so it's not that big a deal. (pDB:false will always have better perf, and is the default)

To regain this perf, we need to rethink the API a bit, because it doesn't quite handle both per-buffer locking (d3d) and fencesync creation (other GL). On per-buffer locking (d3d) we need to re-acquire our lock during the copy. On fence/sync platforms, we want to fence before the copy, since we don't care whether or not the copy (from the front buffer back onto the back buffer) has finished before we use the front buffer.

Comment 14

[Jeff Muizelaar [:jrmuizel]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad32a8ae2631

Comment 15

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8570639 [details] [diff] [review]
An alternative approach

Approval Request Comment
[Feature/regressing bug #]: bug 1079398
[User impact if declined]: Broken WebGL content
[Describe test coverage new/current, TreeHerder]: limited at this point, I want to get as much as possible on all the trees
[Risks and why]: Should be pretty low risk. Super easy to back out.

Comment 16

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

(In reply to Jeff Gilbert [:jgilbert] from comment #13)
> 
> To regain this perf, we need to rethink the API a bit...

I like it - can we have a bug for it?

Comment 17

[Jeff Muizelaar [:jrmuizel]]

I've filed bug 1141730

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ad32a8ae2631

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/28738a551bd0

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/71e45360880e

Comment 21

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Are there any reliable steps to reproduce this issue? I can't reproduce this on Windows 7 x64 with NVIDIA GeForce GT 610.

Comment 22

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

Florin, I imagine you mean you can't reproduce it before the fix?  Is your about:support showing D3D11 ANGLE?

Comment 23

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

(In reply to Milan Sreckovic [:milan] from comment #22)
> Florin, I imagine you mean you can't reproduce it before the fix?  Is your
> about:support showing D3D11 ANGLE?

I've tried to reproduce the issue with Firefox 37 Beta 4 in hopes that tomorrow I would be able to verify the fix in 37 Beta 5. I loaded the page and played the game for a couple of minutes without any issues.

The full Graphics info is:
Adapter Description: NVIDIA GeForce GT 610
Adapter Drivers: nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um
Adapter RAM: 2048
Device ID: 0x104a
Direct2D Enabled: true
DirectWrite Enabled: true (6.2.9200.16571)
Driver Date: 2-5-2015
Driver Version: 9.18.13.4752
GPU #2 Active: false
GPU Accelerated Windows: 1/1 Direct3D 11 (OMTC)
Subsys ID: 104a10b0
Vendor ID: 0x10de
WebGL Renderer: Google Inc. -- ANGLE (NVIDIA GeForce GT 610 Direct3D11 vs_5_0 ps_5_0)
windowLayerManagerRemote: true
AzureCanvasBackend: direct2d 1.1
AzureContentBackend: direct2d 1.1
AzureFallbackCanvasBackend:cairo
AzureSkiaAccelerated: 0

Comment 24

[Jeff Muizelaar [:jrmuizel]]

I'm not sure what is need to reproduce. I was only able to reproduce on Milan's machine. We can verify on Milan's machine.

Comment 25

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

Release Note Request (optional, but appreciated)
[Why is this notable]: Regression from Firefox 37.
[Suggested wording]: A flickering problem on Windows when WebGL and Canvas 2D existed on the same page has been resolved.
[Links (documentation, blog post, etc)]:

Comment 26

[Liz Henry (:lizzard) (relman/hg->git project)]

Changed the wording a bit to be more concise. "WebGL and Canvas 2D content on the same page displays without flickering"