Closed
Bug 1347062
Opened 6 years ago
Closed 6 years ago
Use texture handle directly for video with webrender
Categories
(Core :: Graphics: WebRender, enhancement)
Core
Graphics: WebRender
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jerry, Assigned: jerry)
References
Details
Attachments
(5 files, 9 obsolete files)
4.24 KB,
patch
|
Details | Diff | Splinter Review | |
6.47 KB,
patch
|
Details | Diff | Splinter Review | |
4.94 KB,
patch
|
sotaro
:
review+
|
Details | Diff | Splinter Review |
5.80 KB,
patch
|
Details | Diff | Splinter Review | |
34.31 KB,
patch
|
Details | Diff | Splinter Review |
This bug try to pass the hardware video decoded texture handle to webrender.
Assignee | ||
Comment 1•6 years ago
|
||
MozReview-Commit-ID: G10ju70yGSS
Assignee | ||
Comment 2•6 years ago
|
||
MozReview-Commit-ID: GQxQZDrqUR9
Assignee | ||
Comment 3•6 years ago
|
||
MozReview-Commit-ID: KjdLfYyBByf
Assignee | ||
Comment 4•6 years ago
|
||
MozReview-Commit-ID: 7TGxbwGn2fo
Assignee | ||
Comment 5•6 years ago
|
||
I haven't finished the shader updating in WR. So, we still can't use a hardware decoder for video.
Flags: needinfo?(sotaro.ikeda.g)
Flags: needinfo?(nical.bugzilla)
Assignee | ||
Comment 6•6 years ago
|
||
MozReview-Commit-ID: G10ju70yGSS
Attachment #8851333 -
Flags: review?(sotaro.ikeda.g)
Attachment #8851333 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•6 years ago
|
Attachment #8847018 -
Attachment is obsolete: true
Attachment #8847019 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8847020 -
Attachment is obsolete: true
Attachment #8847021 -
Attachment is obsolete: true
Assignee | ||
Comment 7•6 years ago
|
||
MozReview-Commit-ID: GQxQZDrqUR9
Attachment #8851334 -
Flags: review?(sotaro.ikeda.g)
Attachment #8851334 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 8•6 years ago
|
||
1) make RenderTextureHost into a abstract base class for all RenderXXXTextureHost. 2) create a base class RenderTextureHostOGL for all texture handle base texture. 3) create RenderBufferTextureHost for buffer texture at render thread. 4) create RenderMacIOSurfaceTextureHostOGL for MacIOSurface at render thread. MozReview-Commit-ID: 1t3NgXS8FIp
Attachment #8851335 -
Flags: review?(sotaro.ikeda.g)
Attachment #8851335 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 9•6 years ago
|
||
MozReview-Commit-ID: KxesNcPWsFu
Attachment #8851336 -
Flags: review?(sotaro.ikeda.g)
Attachment #8851336 -
Flags: review?(nical.bugzilla)
Updated•6 years ago
|
Attachment #8851334 -
Flags: review?(sotaro.ikeda.g) → review+
Updated•6 years ago
|
Flags: needinfo?(sotaro.ikeda.g)
Attachment #8851333 -
Flags: review?(sotaro.ikeda.g) → review+
Updated•6 years ago
|
Attachment #8851333 -
Flags: review?(nical.bugzilla) → review+
Updated•6 years ago
|
Attachment #8851334 -
Flags: review?(nical.bugzilla) → review+
Comment 10•6 years ago
|
||
Comment on attachment 8851336 [details] [diff] [review] use texture handle directly with webrender. Review of attachment 8851336 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/webrender_bindings/WebRenderTypes.h @@ +43,5 @@ > > inline Maybe<WrImageFormat> > SurfaceFormatToWrImageFormat(gfx::SurfaceFormat aFormat) { > switch (aFormat) { > + case gfx::SurfaceFormat::R8G8B8X8: Do we actually get RGBX images? WebRender doesn't have a RGB/BGR distinction (and it would be great if we never have to add this mess to WR).
Attachment #8851336 -
Flags: review?(nical.bugzilla) → review+
Comment 11•6 years ago
|
||
Comment on attachment 8851335 [details] [diff] [review] create RenderBufferTextureHost and RenderMacIOSurfaceTextureHostOGL. Review of attachment 8851335 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/wr/WebRenderTextureHost.h @@ +16,1 @@ > class WebRenderTextureHost : public TextureHost I know this class already exists, but could you add a comment here briefly explaining what WebRenderTextureHost represents and how it is used? (In this patch or in a followup if you prefer) @@ +30,5 @@ > virtual void Unlock() override; > > virtual gfx::SurfaceFormat GetFormat() const override; > > + virtual gfx::SurfaceFormat GetReadFormat() const override; A comment explaining the difference between GetFormat and GetReadFormat would be useful. @@ +50,5 @@ > uint64_t GetExternalImageKey() { return mExternalImageId; } > > int32_t GetRGBStride(); > + > + bool WrapNativeHandle() { return mWrapNativeHandle; } The name confuses me because it reads like this method does the work of wraping a native handle and returns true if it suceeded but it's in fact a simple getter. Could you rename it into "WrapsNativeHandle" or something along these lines?
Attachment #8851335 -
Flags: review?(nical.bugzilla) → review+
Comment 12•6 years ago
|
||
Comment on attachment 8851335 [details] [diff] [review] create RenderBufferTextureHost and RenderMacIOSurfaceTextureHostOGL. Review of attachment 8851335 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/composite/TextureHost.cpp @@ +194,5 @@ > TextureHost::Create(const SurfaceDescriptor& aDesc, > ISurfaceAllocator* aDeallocator, > LayersBackend aBackend, > TextureFlags aFlags) > { WebRenderTextureHost supports only BufferTextureHost and MacIOSurfaceTextureHost, but TextureHost::Create seems to wrap all types of TextureHosts, it there a reason to do it?
Updated•6 years ago
|
Attachment #8851336 -
Flags: review?(sotaro.ikeda.g) → review+
Comment 13•6 years ago
|
||
Comment on attachment 8851335 [details] [diff] [review] create RenderBufferTextureHost and RenderMacIOSurfaceTextureHostOGL. Review of attachment 8851335 [details] [diff] [review]: ----------------------------------------------------------------- Is it possible to create a patch by using |hg cp| for RenderBufferTextureHost like Bug 1316479 Comment 2?
Assignee | ||
Comment 14•6 years ago
|
||
Comment on attachment 8851336 [details] [diff] [review] use texture handle directly with webrender. Review of attachment 8851336 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/webrender_bindings/WebRenderTypes.h @@ +43,5 @@ > > inline Maybe<WrImageFormat> > SurfaceFormatToWrImageFormat(gfx::SurfaceFormat aFormat) { > switch (aFormat) { > + case gfx::SurfaceFormat::R8G8B8X8: Yes. wr::ImageDescriptor descriptor(wrTexture->GetSize(), wrTexture->GetReadFormat()); We will get R8G8B8X8 here. https://hg.mozilla.org/mozilla-central/annotate/3364cc17988c013c36f2a8123315db2855393011/gfx/layers/composite/TextureHost.h#l416
Assignee | ||
Comment 15•6 years ago
|
||
Comment on attachment 8851335 [details] [diff] [review] create RenderBufferTextureHost and RenderMacIOSurfaceTextureHostOGL. Review of attachment 8851335 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/composite/TextureHost.cpp @@ +194,5 @@ > TextureHost::Create(const SurfaceDescriptor& aDesc, > ISurfaceAllocator* aDeallocator, > LayersBackend aBackend, > TextureFlags aFlags) > { In WebRenderTextureHost::CreateRenderTextureHost(), the default behavior is a gfx critical error. If we turn on webrender, We should only receive BufferTextureHost and MacIOSurfaceTextureHost from client. It's much easier to catch the non-implemented texture type with this approach. And I will handle the directxYCbCr texture host at another bug. void WebRenderTextureHost::CreateRenderTextureHost(const layers::SurfaceDescriptor& aDesc, TextureHost* aTexture) { ... switch (aDesc.type()) { case SurfaceDescriptor::TSurfaceDescriptorBuffer: { ... break; } #ifdef XP_MACOSX case SurfaceDescriptor::TSurfaceDescriptorMacIOSurface: { ... break; } #endif default: gfxCriticalError() << "No WR implement for texture type:" << aDesc.type(); } }
Assignee | ||
Comment 16•6 years ago
|
||
rebase.
Assignee | ||
Comment 17•6 years ago
|
||
rebase
Assignee | ||
Comment 18•6 years ago
|
||
Update for comment 13. We will have a class RenderTextureHost as the base-class and its sub-class RenderBufferTextureHost in next patch. Use "rename" to preserve the file history.
Attachment #8853294 -
Flags: review?(sotaro.ikeda.g)
Assignee | ||
Comment 19•6 years ago
|
||
Update for comment 11.
Attachment #8853295 -
Flags: review?(sotaro.ikeda.g)
Assignee | ||
Comment 20•6 years ago
|
||
rebase
Assignee | ||
Updated•6 years ago
|
Attachment #8851333 -
Attachment is obsolete: true
Attachment #8851334 -
Attachment is obsolete: true
Attachment #8851335 -
Attachment is obsolete: true
Attachment #8851336 -
Attachment is obsolete: true
Attachment #8851335 -
Flags: review?(sotaro.ikeda.g)
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(nical.bugzilla)
Assignee | ||
Updated•6 years ago
|
Attachment #8853292 -
Attachment description: P1: add WrExternalImage binding utility function. → P1: add WrExternalImage binding utility function. r=nical,sotaro
Assignee | ||
Updated•6 years ago
|
Attachment #8853293 -
Attachment description: P2: use WrImageDescriptor for wr_api_add_external_image_handle(). → P2: use WrImageDescriptor for wr_api_add_external_image_handle(). r=nical,sotaro
Assignee | ||
Updated•6 years ago
|
Attachment #8853296 -
Attachment description: P5: use texture handle directly with webrender. → P5: use texture handle directly with webrender. r=nical,sotaro
Updated•6 years ago
|
Attachment #8853294 -
Flags: review?(sotaro.ikeda.g) → review+
Updated•6 years ago
|
Attachment #8853295 -
Flags: review?(sotaro.ikeda.g) → review+
Assignee | ||
Comment 21•6 years ago
|
||
Fix build break on try server.
Assignee | ||
Updated•6 years ago
|
Attachment #8853295 -
Attachment is obsolete: true
Assignee | ||
Comment 22•6 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=96ca7e72b820e218c87f62dad43c5108468c6bec&selectedJob=87817658
Comment 23•6 years ago
|
||
Pushed by hshih@mozilla.com: https://hg.mozilla.org/projects/graphics/rev/f9813e4134ae P1: add WrExternalImage binding utility function. r=nical,sotaro https://hg.mozilla.org/projects/graphics/rev/4b54605f1dcd P2: use WrImageDescriptor for wr_api_add_external_image_handle(). r=nical,sotaro https://hg.mozilla.org/projects/graphics/rev/5aea276a5283 P3: rename from RenderTextureHost to RenderBufferTextureHost for further updating. r=sotaro https://hg.mozilla.org/projects/graphics/rev/66e555be3167 P4: create RenderBufferTextureHost and RenderMacIOSurfaceTextureHostOGL. r=nical,sotaro https://hg.mozilla.org/projects/graphics/rev/34976aba2753 P5: use texture handle directly with webrender. r=nical,sotaro
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Comment 24•6 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f9813e4134ae https://hg.mozilla.org/mozilla-central/rev/4b54605f1dcd https://hg.mozilla.org/mozilla-central/rev/5aea276a5283 https://hg.mozilla.org/mozilla-central/rev/66e555be3167 https://hg.mozilla.org/mozilla-central/rev/34976aba2753
status-firefox55:
--- → fixed
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•