Closed Bug 1008211 Opened 6 years ago Closed 5 years ago

Don't use BufferTextureClient with 2D canvases with D3D11 OMTC

Categories

(Core :: Graphics: Layers, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
firefox32 + fixed
firefox33 --- fixed

People

(Reporter: nical, Assigned: nical)

References

Details

Attachments

(1 file, 2 obsolete files)

I think that we were using BufferTextureClient originally because TextureClient::GetAsDrawTarget was always using the Moz2D backend for content drawing (which wouldn't always work with canvases), but that's fixed now. Unless we specifically want to access the Texture's pixel through pointers, there is no good reason to use CreateBufferTextureClient over CreateTextureClientForDrawing.
The two remaining users of CreateBufferTextureClient are:
* SharedPlanarYCbCrImage, which is legit since it packs its channels in one buffer
* SharedRGBImage, which should probably not use it since CreateTextureClientForDrawing would save one copy in some cases (like D3D11 and Gralloc). But to get there we need to make the media code use Moz2D to write into the texture, instead of using raw pointers and memcpy.
Attachment #8420168 - Flags: review?(bas)
try push https://tbpl.mozilla.org/?tree=Try&rev=24e5757a5936 (with OMTC turned on everywhere)
Attachment #8420168 - Flags: review?(bas) → review+
(In reply to Nicolas Silva [:nical] from comment #2)
> try push https://tbpl.mozilla.org/?tree=Try&rev=24e5757a5936 (with OMTC
> turned on everywhere)

The build failures on that try push make no sense since this patch doesn't touch any #include. Here is a new try push (after a rebase).

https://tbpl.mozilla.org/?tree=Try&rev=8a3020de9372
https://hg.mozilla.org/mozilla-central/rev/4e1e052624c2
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Depends on: 1010146
Backed out because of a regression on B2G. When using gralloc, CanvasClient goes into a sort of single-buffering" mode which is bad with direct texturing because of lock contension.

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

I'll need to make CanvasClient use double buffering or something similar before relanding.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 1008221
Pretty much the same patch but only on windows, since it single buffering with gralloc creates too much lock contention.
Attachment #8420168 - Attachment is obsolete: true
Attachment #8423935 - Flags: review?(bas)
Comment on attachment 8423935 [details] [diff] [review]
Don't use CreateBufferTextureClient with CanvasClient2D on Windows.

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

Let's create a CreateTextureClientForCanvas and use that.
Attachment #8423935 - Flags: review?(bas) → review-
https://hg.mozilla.org/mozilla-central/rev/dc9bcd6653f0
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Reopening since this was a backout.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The logic to choose between the different TextureClient types is tied to CanvasClient2D's quirks so it doesn't make sense to expose it to the rest of the world. I placed the code in a method (as requested), only accessible to CanvasClient2D.
Attachment #8423935 - Attachment is obsolete: true
Attachment #8435798 - Flags: review?(bas)
Comment on attachment 8435798 [details] [diff] [review]
Don't use CreateBufferTextureClient with CanvasClient2D on Windows.

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

::: gfx/layers/client/CanvasClient.cpp
@@ +121,5 @@
> +    return CreateBufferTextureClient(aFormat, aFlags, BackendType::CAIRO);
> +  }
> +
> +  gfx::BackendType backend = gfxPlatform::GetPlatform()->GetPreferredCanvasBackend();
> +#ifdef XP_WIN

This is pretty ugly, but it will do for now.
Attachment #8435798 - Flags: review?(bas) → review+
We may have to uplift this to Aurora. Let's get it landed soon.
https://hg.mozilla.org/mozilla-central/rev/ffc3c599d28e
Status: REOPENED → RESOLVED
Closed: 6 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: mozilla32 → mozilla33
Comment on attachment 8435798 [details] [diff] [review]
Don't use CreateBufferTextureClient with CanvasClient2D on Windows.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): OMTC
User impact if declined: Sloooow canvas2D on windows
Testing completed (on m-c, etc.): a few days
Risk to taking this patch (and alternatives if risky): not risky, trivial to back-out
String or IDL/UUID changes made by this patch:
Attachment #8435798 - Flags: approval-mozilla-aurora?
Comment on attachment 8435798 [details] [diff] [review]
Don't use CreateBufferTextureClient with CanvasClient2D on Windows.

Aurora approval granted. Please confirm this performance fix once it lands on Aurora.
Attachment #8435798 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.