1347062 - Use texture handle directly for video with webrender

Status: RESOLVED FIXED

Description

[Jerry Shih[:jerry] (UTC+8) (inactive)]

This bug try to pass the hardware video decoded texture handle to webrender.

Comment 1

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: add WrExternalImage binding utility function.

MozReview-Commit-ID: G10ju70yGSS

Comment 2

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: use WrImageDescriptor for wr_api_add_external_image_handle().

MozReview-Commit-ID: GQxQZDrqUR9

Comment 3

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: create RenderBufferTextureHost and RenderIacIOSurfaceTextureHostOGL.

MozReview-Commit-ID: KjdLfYyBByf

Comment 4

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: use texture handle directly with webrender.

MozReview-Commit-ID: 7TGxbwGn2fo

Comment 5

[Jerry Shih[:jerry] (UTC+8) (inactive)]

I haven't finished the shader updating in WR. So, we still can't use a hardware decoder for video.

Comment 6

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: add WrExternalImage binding utility function.

MozReview-Commit-ID: G10ju70yGSS

Comment 7

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: use WrImageDescriptor for wr_api_add_external_image_handle().

MozReview-Commit-ID: GQxQZDrqUR9

Comment 8

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: create RenderBufferTextureHost and RenderMacIOSurfaceTextureHostOGL.

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

Comment 9

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: use texture handle directly with webrender.

MozReview-Commit-ID: KxesNcPWsFu

Comment 10

[Nicolas Silva [:nical]]

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).

Comment 11

[Nicolas Silva [:nical]]

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?

Comment 12

[Sotaro Ikeda [:sotaro]]

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?

Comment 13

[Sotaro Ikeda [:sotaro]]

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?

Comment 14

[Jerry Shih[:jerry] (UTC+8) (inactive)]

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

Comment 15

[Jerry Shih[:jerry] (UTC+8) (inactive)]

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();
	
  }	
}

Comment 16

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P1: add WrExternalImage binding utility function. r=nical,sotaro

rebase.

Comment 17

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P2: use WrImageDescriptor for wr_api_add_external_image_handle(). r=nical,sotaro

rebase

Comment 18

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P3: rename from RenderTextureHost to RenderBufferTextureHost for further updating.

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.

Comment 19

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P4: create RenderBufferTextureHost and RenderMacIOSurfaceTextureHostOGL.

Update for comment 11.

Comment 20

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P5: use texture handle directly with webrender. r=nical,sotaro

rebase

Comment 21

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P4: create RenderBufferTextureHost and RenderMacIOSurfaceTextureHostOGL. v2. r=nical,sotaro

Fix build break on try server.

Comment 22

[Jerry Shih[:jerry] (UTC+8) (inactive)]

try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=96ca7e72b820e218c87f62dad43c5108468c6bec&selectedJob=87817658

Comment 23

[Pulsebot]

Pushed by [e-mail]:
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

Comment 24

[Kartikaya Gupta (email:[e-mail]) (away until Feb-2020)]

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