1088414 - Use a single synchronized texture for d3d11

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Matt Woodrow (:mattwoodrow)]

Attachment: sync-hack

Calling AcquireSync on textures created with D3D11_RESOURCE_MISC_SHARED_KEYEDMUTEX appears to be really slow, this really bites us with the number of textures involved when tiling.

I've also tested using ID3D11Query to check when work is completed, this also appears to be pretty slow.

This instead just uses a single texture with a keyedmutex, and makes sure we draw to it last and lock it first. It's pretty hacky, but seems to work really well so far.

Comment 1

[Bas Schouten (:bas.schouten)]

Comment on attachment 8510725 [details] [diff] [review]
sync-hack

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

This is really really dangerous. But let's do it :P.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8510725 [details] [diff] [review]
sync-hack

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

I guess we'll get more information having this in the tree than having it as a patch...

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8510725 [details] [diff] [review]
sync-hack

Actually, I'd be a lot happier taking this if we hand a stand alone test case showing the performance problem and this fixing it.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

There's perhaps a starting point here:
https://github.com/jrmuizel/d3d-11-shared-surfaces

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Let's do it!

Comment 6

[Jeff Muizelaar [:jrmuizel]]

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

Comment 7

[Phil Ringnalda (:philor)]

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

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8510725 [details] [diff] [review]
sync-hack

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

::: gfx/thebes/gfxWindowsPlatform.cpp
@@ +1488,5 @@
> +  RefPtr<IDXGIKeyedMutex> mutex;
> +  sentinelTexture->QueryInterface((IDXGIKeyedMutex**)byRef(mutex));
> +  mutex->AcquireSync(0, INFINITE);
> +  mutex->ReleaseSync(0);
> +}

I'm not really convinced that this code is thread safe....

Also, FenceContentDrawing looks pretty sketchy to me. Shouldn't we AcquireSync at the beginning of drawing and ReleaseSync at the end instead of allocating a texture?

Comment 9

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8510725 [details] [diff] [review]
sync-hack

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

::: gfx/thebes/gfxWindowsPlatform.cpp
@@ +1488,5 @@
> +  RefPtr<IDXGIKeyedMutex> mutex;
> +  sentinelTexture->QueryInterface((IDXGIKeyedMutex**)byRef(mutex));
> +  mutex->AcquireSync(0, INFINITE);
> +  mutex->ReleaseSync(0);
> +}

I misread CreateDrawTargetForD3D11Texture as CreateDrawTarget.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8510725 [details] [diff] [review]
sync-hack

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

This also seems broken with e10s

Comment 11

[Jeff Muizelaar [:jrmuizel]]

I backed this out because of its brokeness:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4c4897f9ffb

Comment 12

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #11)
> I backed this out because of its brokeness:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/d4c4897f9ffb

Hopefully, we'll be able to fix it up and reland soon.

Comment 13

[Carsten Book [:Tomcat]]

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

Comment 14

[Jeff Muizelaar [:jrmuizel]]

We also get crashes here: https://crash-stats.mozilla.com/report/index/df756757-95df-4315-b64e-559572141116. Presumably because the draw target is no good.

Comment 15

[Bas Schouten (:bas.schouten)]

Attachment: Use a single sync texture for D3D11

Comment 16

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8533916 [details] [diff] [review]
Use a single sync texture for D3D11

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

Looks good. Hopefully it works out.

::: gfx/layers/d3d11/TextureD3D11.cpp
@@ +223,5 @@
> +
> +  MOZ_ASSERT(aSyncObject->GetSyncType() == SyncObject::SyncType::D3D11);
> +
> +  SyncObjectD3D11* sync = static_cast<SyncObjectD3D11*>(aSyncObject);
> +

Add a comment like:

// We copy into the Sync texture as a precaution to ensure ordering in the queue. Perhaps we can drop this if we can become confident of the queue ordering.

Comment 17

[Bas Schouten (:bas.schouten)]

Attachment: Use a single sync texture for D3D11

Comment 18

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

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/dadec018f807 for breaking all non-Windows builds:

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4614692&repo=mozilla-inbound

I know Bas mentioned a fix in #developers, but it was taking too long and I need to step out for the night. Feel free to immediately reland with the fix applied, though.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6d1f7a6e90a4