Closed
Bug 947781
Opened 11 years ago
Closed 11 years ago
Implement ReadbackLayer for OMTC
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: mattwoodrow, Assigned: bas.schouten)
References
Details
Attachments
(4 files)
10.48 KB,
patch
|
mattwoodrow
:
review+
|
Details | Diff | Splinter Review |
3.38 KB,
patch
|
nical
:
review+
|
Details | Diff | Splinter Review |
18.36 KB,
patch
|
nical
:
review+
|
Details | Diff | Splinter Review |
15.04 KB,
patch
|
mattwoodrow
:
review+
|
Details | Diff | Splinter Review |
I think we'll need this before shipping OMTC on windows to get good enough performance for windowless transparent plugins.
I think we need to add new API to TextureClient (or one of the helper interfaces) that is similar to:
typedef void (* LockCallback)(gfxASurface* aContents, void *aClosure);
void AsyncLock(LockCallback aCallback, void *aClosure);
We can do a default implementation that just calls the callback synchronously using GetAsSurface().
TextureClients using d2d can be implemented like D3D10 does (in gfx/layers/d3d10/ReadbackManagerD3D10.cpp) to do a proper asynchronous callback once the drawing has completed.
We can then implement a LockCallback that uses PluginBackgroundSink to copy the TextureClient's data into the plugin's background surface.
Does that seem sane?
Flags: needinfo?(nical.bugzilla)
Reporter | ||
Comment 1•11 years ago
|
||
The plugin background API is still using thebes, and that's our own use case for this API, so I'm planning on sticking with that for now.
Comment 2•11 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #0)
> I think we need to add new API to TextureClient (or one of the helper
> interfaces) that is similar to:
>
> typedef void (* LockCallback)(gfxASurface* aContents, void *aClosure);
> void AsyncLock(LockCallback aCallback, void *aClosure);
>
> We can do a default implementation that just calls the callback
> synchronously using GetAsSurface().
>
> TextureClients using d2d can be implemented like D3D10 does (in
> gfx/layers/d3d10/ReadbackManagerD3D10.cpp) to do a proper asynchronous
> callback once the drawing has completed.
>
> We can then implement a LockCallback that uses PluginBackgroundSink to copy
> the TextureClient's data into the plugin's background surface.
>
> Does that seem sane?
I am not familiar at all with readback layers so I don't really understand what this implies. So from very very far away this doesn't look insane (too bad that we need to add thebes stuff but if that's only going to be used for the backround API I suppose that will do). You should be extra careful about lifetimes whenever you want something to interact with TextureClient/Host.
Flags: needinfo?(nical.bugzilla)
Assignee | ||
Comment 3•11 years ago
|
||
I don't think we need this to turn it on on nightly, we need it before we go to aurora though. I will do this work.
Assignee: nobody → bas
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•11 years ago
|
||
I'm not convinced anymore that we need this to ride the trains? I'll write the patch and see if we can uplift it though. Do you actually still think we need this Matt?
Flags: needinfo?(matt.woodrow)
Reporter | ||
Comment 5•11 years ago
|
||
Given the lack of complaints, maybe we don't. It would still be really good to have though, I'm sure some people care about plugin performance.
Flags: needinfo?(matt.woodrow)
Assignee | ||
Comment 6•11 years ago
|
||
Attachment #8463366 -
Flags: review?(matt.woodrow)
Assignee | ||
Updated•11 years ago
|
Attachment #8463366 -
Attachment description: Refactor RotatedBuffer code to allow more broad reusal → Part 1: Refactor RotatedBuffer code to allow more broad reusal
Assignee | ||
Updated•11 years ago
|
Attachment #8463366 -
Attachment description: Part 1: Refactor RotatedBuffer code to allow more broad reusal → Part 1: Refactor RotatedBuffer code to allow easier reuse
Assignee | ||
Comment 7•11 years ago
|
||
Attachment #8463406 -
Flags: review?(nical.bugzilla)
Updated•11 years ago
|
Attachment #8463406 -
Flags: review?(nical.bugzilla) → review+
Reporter | ||
Updated•11 years ago
|
Attachment #8463366 -
Flags: review?(matt.woodrow) → review+
Assignee | ||
Comment 8•11 years ago
|
||
Attachment #8464100 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 9•11 years ago
|
||
Attachment #8464104 -
Flags: review?(matt.woodrow)
Reporter | ||
Comment 10•11 years ago
|
||
Comment on attachment 8464104 [details] [diff] [review]
Part 4: Implement ReadbackLayers for OMTC.
Review of attachment 8464104 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/layers/client/ContentClient.cpp
@@ +168,5 @@
> + {
> + // Manual AddRef to keep these layers alive! Very important these get killed
> + // again in the destructor!
> + for (uint32_t i = 0; i < mReadbackUpdates.Length(); ++i) {
> + mReadbackUpdates[i].mLayer->AddRef();
This is a bit sad, couldn't we make ReadbackProcessor::Update always hold refs using nsRefPtr?
Attachment #8464104 -
Flags: review?(matt.woodrow) → review+
Comment 11•11 years ago
|
||
Comment on attachment 8464100 [details] [diff] [review]
Part 2: Add readback functionality to TextureClients
Review of attachment 8464100 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good, what will call SetReadbackSink on the textures?
::: gfx/layers/d3d11/ReadbackManagerD3D11.h
@@ +43,5 @@
> +
> + void ProcessTasks();
> +
> + // The invariant maintained by |mTaskSemaphore| is that the readback thread
> + // will awaken from WaitForMultipleObjects() at least once per readback
nit: trailing space
Attachment #8464100 -
Flags: review?(nical.bugzilla) → review+
Assignee | ||
Comment 12•11 years ago
|
||
Comment 13•11 years ago
|
||
Backed out for:
https://tbpl.mozilla.org/php/getParsedLog.php?id=44891771&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=44891768&tree=Mozilla-Inbound
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/df0091e830db
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/ba00ca45db46
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/90d1352dbd19
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/1528900bd8cf
Comment 14•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/436cc8abfda3
https://hg.mozilla.org/mozilla-central/rev/6346e96ceac0
https://hg.mozilla.org/mozilla-central/rev/881f358bc39e
https://hg.mozilla.org/mozilla-central/rev/9b44cf686aba
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in
before you can comment on or make changes to this bug.
Description
•