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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox49 --- affected
firefox50 --- fixed

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.
- 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/
Attachment #8758077 - Flags: review?(m)
Attachment #8758077 - Flags: review?(m) → review?(jmuizelaar)
Attachment #8758077 - Flags: review?(jmuizelaar) → review?(nical.bugzilla)
(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.
(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
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
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)
Summary: [webvr] IBug 1276811 - Enable TextureClient to be used without CompositableForwarder → [webvr] Bug 1276811 - Enable TextureClient to be used without CompositableForwarder
Attachment #8760963 - Flags: review?(nical.bugzilla) → review+
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)
Attachment #8758077 - Attachment is obsolete: true
Attachment #8758077 - Flags: review?(jmuizelaar)
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)
Attachment #8760963 - Attachment is obsolete: true
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+
Pushed to try through mozreview. If passes, will commit to unblock Bug 1250244
Attachment #8758077 - Flags: review?(nical.bugzilla) → review+
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.
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+
(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)
(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)
Pushed by kgilbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/039447407fcc Enable TextureClient to be used without CompositableForwarder,r=nical
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: