Closed
Bug 1276811
Opened 8 years ago
Closed 8 years ago
[webvr] Bug 1276811 - Enable TextureClient to be used without CompositableForwarder
Categories
(Core :: Graphics: Layers, defect)
Core
Graphics: Layers
Tracking
()
RESOLVED
FIXED
mozilla50
People
(Reporter: kip, Assigned: kip)
References
Details
Attachments
(1 file, 1 obsolete file)
TextureForwarder is a base class that fits between CompositableForwarder and ISurfaceAllocator, enabling the texture forwarding functionality to be shared by subclasses that do not wish to bring in the layerization and compositor dependencies from CompositableForwarder.
This is used the WebVR 1.0 implementation in Bug 1250244, which implements IPC for texture forwarding outside of the compositor.
Assignee | ||
Comment 1•8 years ago
|
||
- TextureForwarder is a base class that fits between
CompositableForwarder and ISurfaceAllocator, enabling
the texture forwarding functionality to be shared
by subclasses that do not wish to bring in the
layerization and compositor dependencies in
CompositableForwarder.
Review commit: https://reviewboard.mozilla.org/r/56458/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/56458/
Assignee | ||
Updated•8 years ago
|
Attachment #8758077 -
Flags: review?(m)
Assignee | ||
Updated•8 years ago
|
Attachment #8758077 -
Flags: review?(m) → review?(jmuizelaar)
Updated•8 years ago
|
Attachment #8758077 -
Flags: review?(jmuizelaar) → review?(nical.bugzilla)
Comment 2•8 years ago
|
||
(In reply to :kip (Kearwood Gilbert) from comment #0)
> TextureForwarder is a base class that fits between CompositableForwarder and
> ISurfaceAllocator, enabling the texture forwarding functionality to be
> shared by subclasses that do not wish to bring in the layerization and
> compositor dependencies from CompositableForwarder.
Was this coordinated with George (bug 1176011) or is it a pure coincidence?
Anyway, most of this patch does exactly the same as the TextureForarder from bug 1176011 already r+'ed which is good. Could you you rebase your patch on top of Georges's? There should not be much left of it after that.
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Nicolas Silva [:nical] from comment #2)
> (In reply to :kip (Kearwood Gilbert) from comment #0)
> > TextureForwarder is a base class that fits between CompositableForwarder and
> > ISurfaceAllocator, enabling the texture forwarding functionality to be
> > shared by subclasses that do not wish to bring in the layerization and
> > compositor dependencies from CompositableForwarder.
>
> Was this coordinated with George (bug 1176011) or is it a pure coincidence?
>
> Anyway, most of this patch does exactly the same as the TextureForarder from
> bug 1176011 already r+'ed which is good. Could you you rebase your patch on
> top of Georges's? There should not be much left of it after that.
This was a coincidence :-)
I am rebasing and testing my patchset over the Bug 1176011 patch.
The remaining changes after rebasing are mostly concerned with allowing TextureClient to be used without a CompositableForwarder. I'll update and rename this bug to match.
Depends on: 1176011
Assignee | ||
Comment 4•8 years ago
|
||
I have rebased this over Bug 1176011, which adds TextureForwarder.
The remaining patch enables TextureClient to be used with TextureForwarder rather than CompositableForwarder.
Summary: [webvr] Implement TextureForwarder → [webvr] IBug 1276811 - Enable TextureClient to be used without CompositableForwarder
Assignee | ||
Comment 5•8 years ago
|
||
Bug 1276811 - Enable TextureClient to be used without CompositableForwarder
- Refactoring to make TextureClient use the higher-level
TextureForwarder interface.
(If desired, I'll move this patch to reviewboard once Bug 1176011 lands)
Attachment #8758077 -
Attachment is obsolete: true
Attachment #8758077 -
Flags: review?(nical.bugzilla)
Attachment #8760963 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•8 years ago
|
Summary: [webvr] IBug 1276811 - Enable TextureClient to be used without CompositableForwarder → [webvr] Bug 1276811 - Enable TextureClient to be used without CompositableForwarder
Updated•8 years ago
|
Attachment #8760963 -
Flags: review?(nical.bugzilla) → review+
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8758077 [details]
Bug 1276811 - Enable TextureClient to be used without CompositableForwarder,
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56458/diff/1-2/
Attachment #8758077 -
Attachment description: MozReview Request: Bug 1276811 - Implement TextureForwarder → Bug 1276811 - Implement TextureForwarder
Attachment #8758077 -
Attachment is obsolete: false
Attachment #8758077 -
Flags: review?(jmuizelaar)
Assignee | ||
Updated•8 years ago
|
Attachment #8758077 -
Attachment is obsolete: true
Attachment #8758077 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 7•8 years ago
|
||
Comment on attachment 8758077 [details]
Bug 1276811 - Enable TextureClient to be used without CompositableForwarder,
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56458/diff/2-3/
Attachment #8758077 -
Attachment description: Bug 1276811 - Implement TextureForwarder → Bug 1276811 - Enable TextureClient to be used without CompositableForwarder,
Attachment #8758077 -
Attachment is obsolete: false
Attachment #8758077 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•8 years ago
|
Attachment #8760963 -
Attachment is obsolete: true
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8758077 [details]
Bug 1276811 - Enable TextureClient to be used without CompositableForwarder,
https://reviewboard.mozilla.org/r/56458/#review57958
nical had r+'ed this before I moved the patch to MozReview.
Attachment #8758077 -
Flags: review+
Assignee | ||
Comment 9•8 years ago
|
||
Pushed to try through mozreview. If passes, will commit to unblock Bug 1250244
Updated•8 years ago
|
Attachment #8758077 -
Flags: review?(nical.bugzilla) → review+
Comment 10•8 years ago
|
||
Comment on attachment 8758077 [details]
Bug 1276811 - Enable TextureClient to be used without CompositableForwarder,
https://reviewboard.mozilla.org/r/56458/#review58078
There's some brokenness that landed with the initial TextureForwarder patch. The fix I landed yesterday got backed out so I'll try to get something asap, in the mean time there are some really buggy assumptions about TextureForwarder and CompositableForwarder. Your patch doesn't make it strictly worse I think but whatever I'll come up with to fix the situation will probably involve changing parts of it.
::: gfx/layers/client/TextureClient.cpp:868
(Diff revision 3)
> ClearRecycleCallback();
> }
> }
>
> bool
> -TextureClient::InitIPDLActor(CompositableForwarder* aForwarder)
> +TextureClient::InitIPDLActor(TextureForwarder* aForwarder)
There's something hat will need to be figured out here. Right now ShadowLayerForwarder (our CompositableForwarder) inherits TextureForwarder, but it is confusing because the TextureForwarder we want is in fact CompositorBridgeChild (which is not a CompositableForwarder). At some point we need to clean this up and make CompositableForwarder not inherit TextureForwarder.
If a texture is used in a transaction (with layers and/or with ImageBridge) it needs to have a CompositableForwarder (I assume it won't need a CompositableForwarder for the WebVR usecase)
We probably need a separate InitIPDLActor method. One that takes a CompositableForwarder, and one that takes a TextureForwarder.
Assignee | ||
Comment 11•8 years ago
|
||
Assignee | ||
Comment 12•8 years ago
|
||
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 8758077 [details]
Bug 1276811 - Enable TextureClient to be used without CompositableForwarder,
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56458/diff/3-4/
Attachment #8758077 -
Flags: review+
Assignee | ||
Comment 14•8 years ago
|
||
(In reply to Nicolas Silva [:nical] from comment #10)
Some functions have been moved from TextureForwarder back to CompositableForwarder, such as GetCompositorBackendType, so I had to update the patch once again.
This time, I have left the InitIPDLActor method alone, and added an overloaded version that takes a TextureForwarder* and a LayersBackend.
This has passed a try run. Could you verify that the updated patch is okay with you before I commit?
Flags: needinfo?(nical.bugzilla)
Comment 15•8 years ago
|
||
(In reply to :kip (Kearwood Gilbert) from comment #14)
> Could you verify that the updated patch is okay with you before I commit?
Looks good!
Flags: needinfo?(nical.bugzilla)
Comment 16•8 years ago
|
||
Pushed by kgilbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/039447407fcc
Enable TextureClient to be used without CompositableForwarder,r=nical
Comment 17•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•