Closed Bug 1434522 Opened 6 years ago Closed 6 years ago

Fix D3D Texture fallback handling when NV12 is not supported

Categories

(Core :: Graphics: WebRender, defect, P1)

defect

Tracking

()

RESOLVED DUPLICATE of bug 1440849

People

(Reporter: sotaro, Assigned: sotaro)

References

Details

Attachments

(1 file, 1 obsolete file)

This bug is created based on Bug 1433696 comment 19.
A copy of Bug 1433696 comment 19.

> My case was caused by regression of Bug 1415754. When
> RenderDXGITextureHostOGL and RenderDXGIYCbCrTextureHostOGL does not support
> ANGLE_stream_producer_d3d_texture_nv12(NV12 format), they does not render
> d3d texture. Bug 1415754 addressed the crash, but the way of fixing was not
> good :(
Assignee: nobody → sotaro.ikeda.g
Blocks: 1433696
Depends on: 1434524
Bug 1415754 added ANGLE_stream_producer_d3d_texture_nv12 support checks to RenderDXGITextureHostOGL::EnsureLockable() and RenderDXGIYCbCrTextureHostOGL::EnsureLockable(), since they use eglCreateStreamProducerD3DTextureNV12ANGLE() eglStreamPostD3DTextureNV12ANGLE().

ANGLE makes "ANGLE_stream_producer_d3d_texture_nv12 support" to true, when ID3D11Device supports NV12 format.
  https://dxr.mozilla.org/mozilla-central/source/gfx/angle/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp#2324
eglCreateStreamProducerD3DTextureNV12ANGLE() and eglStreamPostD3DTextureNV12ANGLE() are always used by RenderDXGITextureHostOGL and RenderDXGIYCbCrTextureHostOGL to bind to gl textures even when color format is not NV12.
One easy fix is to disable WebRender when ID3D11Device does not support NV12 format. I am not sure if there are another way to address the problem.
:jgilbert, is there another way to address the problem when ID3D11Device does not support NV12 format.
Flags: needinfo?(jgilbert)
(In reply to Sotaro Ikeda [:sotaro] from comment #5)
> :jgilbert, is there another way to address the problem when ID3D11Device
> does not support NV12 format.

It is nice if we could bind d3d Texture to gl texture even when ID3D11Device does not support NV12 format. RenderDXGITextureHostOGL and RenderDXGIYCbCrTextureHostOGL use eglCreateStreamProducerD3DTextureNV12ANGLE() and eglStreamPostD3DTextureNV12ANGLE() even for non NV12 use case. They could not be used when ID3D11Device does not support NV12 format :(
Summary: Fix D3D Texrure fallback handling when NV12 is not supported → Fix D3D Texture fallback handling when NV12 is not supported
(In reply to Sotaro Ikeda [:sotaro] from comment #6)
> (In reply to Sotaro Ikeda [:sotaro] from comment #5)
> > :jgilbert, is there another way to address the problem when ID3D11Device
> > does not support NV12 format.
> 
> It is nice if we could bind d3d Texture to gl texture even when ID3D11Device
> does not support NV12 format. RenderDXGITextureHostOGL and
> RenderDXGIYCbCrTextureHostOGL use
> eglCreateStreamProducerD3DTextureNV12ANGLE() and
> eglStreamPostD3DTextureNV12ANGLE() even for non NV12 use case. They could
> not be used when ID3D11Device does not support NV12 format :(

They definitely should be usable without NV12 support. The names only have NV12 in them for historical reasons.
Flags: needinfo?(jgilbert)
By Comment 7, ANGLE_stream_producer_d3d_texture_nv12 could be usable even when D3DDevice does not support NV12 format. Instead, the patch add NV12 format check to StreamProducerNV12::validateD3DNV12Texture().
Attachment #8947727 - Flags: review?(jgilbert)
Comment on attachment 8947727 [details] [diff] [review]
patch - Relieve ANGLE_stream_producer_d3d_texture_nv12 support check

Review of attachment 8947727 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/angle/src/libANGLE/renderer/d3d/d3d11/Renderer11.h
@@ +54,5 @@
>      bool supportsMultisampledDepthStencilSRVs;  // D3D feature level 10.0 no longer allows creation
>                                                  // of textures with both the bind SRV and DSV flags
>                                                  // when multisampled.  Textures will need to be
>                                                  // resolved before reading. crbug.com/656989
> +    bool supportsNV12;     // Support for NV12 Texture

No need for this. Just check the display extension.

::: gfx/angle/src/libANGLE/renderer/d3d/d3d11/StreamProducerNV12.cpp
@@ +117,5 @@
>  
> +    // Check that the texture format is supported by our device.
> +    D3D11_TEXTURE2D_DESC desc;
> +    textureD3D->GetDesc(&desc);
> +    if (desc.Format == DXGI_FORMAT_NV12 && !mRenderer->getRenderer11DeviceCaps().supportsNV12)

!mRenderer->mDisplay->mDisplayExtensions->streamProducerD3DTextureNV12
Attachment #8947727 - Flags: review?(jgilbert) → review-
(In reply to Jeff Gilbert [:jgilbert] from comment #9)
> Comment on attachment 8947727 [details] [diff] [review]
> patch - Relieve ANGLE_stream_producer_d3d_texture_nv12 support check
> 
> Review of attachment 8947727 [details] [diff] [review]:
> 
> ::: gfx/angle/src/libANGLE/renderer/d3d/d3d11/StreamProducerNV12.cpp
> @@ +117,5 @@
> >  
> > +    // Check that the texture format is supported by our device.
> > +    D3D11_TEXTURE2D_DESC desc;
> > +    textureD3D->GetDesc(&desc);
> > +    if (desc.Format == DXGI_FORMAT_NV12 && !mRenderer->getRenderer11DeviceCaps().supportsNV12)
> 
> !mRenderer->mDisplay->mDisplayExtensions->streamProducerD3DTextureNV12

:jgilbert, if we use mDisplayExtensions->streamProducerD3DTextureNV12 for checking NV12 texture format support. How could we know if eglCreateStreamProducerD3DTextureNV12ANGLE() and eglStreamPostD3DTextureNV12ANGLE() functions are usable? If D3D Device does not support NV12 format, the streamProducerD3DTextureNV12 becomes false.

ANGLE expose the streamProducerD3DTextureNV12 as "EGL_ANGLE_stream_producer_d3d_texture_nv12".
 https://dxr.mozilla.org/mozilla-central/source/gfx/angle/src/libANGLE/Caps.cpp#1132

Then GLLibraryEGL uses "EGL_ANGLE_stream_producer_d3d_texture_nv12" to determine whether to load eglCreateStreamProducerD3DTextureNV12ANGLE() and eglStreamPostD3DTextureNV12ANGLE().
 https://dxr.mozilla.org/mozilla-central/source/gfx/gl/GLLibraryEGL.cpp#700
Flags: needinfo?(jgilbert)
Blocks: 1422298
Ah, ok. In that case the call needs to fail without generating an error. An error is a failure where the caller did something illegal. (except for OOM) In this case, Post() should fail, since that's the first time we are handed the d3d texture.
Flags: needinfo?(jgilbert)
:jgilbert, we could not use eglCreateStreamProducerD3DTextureNV12ANGLE() and eglStreamPostD3DTextureNV12ANGLE() if mDisplayExtensions->streamProducerD3DTextureNV12 is false(D3D device does not support NV12) because of the followings.

-[1] ValidateCreateStreamProducerD3DTextureNV12ANGLE() and ValidateCreateStreamProducerD3DTextureNV12ANGLE() check the streamProducerD3DTextureNV12.
  https://dxr.mozilla.org/mozilla-central/source/gfx/angle/src/libANGLE/validationEGL.cpp#1958
  https://dxr.mozilla.org/mozilla-central/source/gfx/angle/src/libANGLE/validationEGL.cpp#2005

-[2] GLLibraryEGL loads symbols of CreateStreamProducerD3DTextureNV12ANGLE and StreamPostD3DTextureNV12ANGLE only when "EGL_ANGLE_stream_producer_d3d_texture_nv12" is supported.
  https://dxr.mozilla.org/mozilla-central/source/gfx/gl/GLLibraryEGL.cpp#700

But Comment 7 says that eglCreateStreamProducerD3DTextureNV12ANGLE() and eglStreamPostD3DTextureNV12ANGLE() should be usable without NV12 support. Can you clarify more about how this could be addressed?

Your comment seems to say that the problem could be addressed just by the following change. but if mDisplayExtensions->streamProducerD3DTextureNV12 is false, we could not use ValidateCreateStreamProducerD3DTextureNV12ANGLE() and ValidateCreateStreamProducerD3DTextureNV12ANGLE() because of [1] and [2].
 > if (desc.Format == DXGI_FORMAT_NV12 && !mRenderer->mDisplay->mDisplayExtensions->streamProducerD3DTextureNV12)
Flags: needinfo?(jgilbert)
(In reply to Sotaro Ikeda [:sotaro] from comment #12)
> :jgilbert, we could not use eglCreateStreamProducerD3DTextureNV12ANGLE() and
> eglStreamPostD3DTextureNV12ANGLE() if
> mDisplayExtensions->streamProducerD3DTextureNV12 is false(D3D device does
> not support NV12) because of the followings.
> 
> -[1] ValidateCreateStreamProducerD3DTextureNV12ANGLE() and
> ValidateCreateStreamProducerD3DTextureNV12ANGLE() check the
> streamProducerD3DTextureNV12.
>  
> https://dxr.mozilla.org/mozilla-central/source/gfx/angle/src/libANGLE/
> validationEGL.cpp#1958
>  
> https://dxr.mozilla.org/mozilla-central/source/gfx/angle/src/libANGLE/
> validationEGL.cpp#2005
> 
> -[2] GLLibraryEGL loads symbols of CreateStreamProducerD3DTextureNV12ANGLE
> and StreamPostD3DTextureNV12ANGLE only when
> "EGL_ANGLE_stream_producer_d3d_texture_nv12" is supported.
>   https://dxr.mozilla.org/mozilla-central/source/gfx/gl/GLLibraryEGL.cpp#700
> 
> But Comment 7 says that eglCreateStreamProducerD3DTextureNV12ANGLE() and
> eglStreamPostD3DTextureNV12ANGLE() should be usable without NV12 support.
> Can you clarify more about how this could be addressed?
> 
> Your comment seems to say that the problem could be addressed just by the
> following change. but if mDisplayExtensions->streamProducerD3DTextureNV12 is
> false, we could not use ValidateCreateStreamProducerD3DTextureNV12ANGLE()
> and ValidateCreateStreamProducerD3DTextureNV12ANGLE() because of [1] and [2].
>  > if (desc.Format == DXGI_FORMAT_NV12 &&
> !mRenderer->mDisplay->mDisplayExtensions->streamProducerD3DTextureNV12)

mDisplayExtensions->streamProducerD3DTextureNV12 should presently be supported even without nv12 support. It is poorly named right now. We use the streamProducerD3DTextureNV12 extension for non-nv12 blitting of rgba8 (and other) d3d11textures.
Flags: needinfo?(jgilbert)
:jgilbert, Comment 13 seems to said that mDisplayExtensions->streamProducerD3DTextureNV12 is always true with D3D11. Is it correct? If it is correct, I am not sure about Comment 9. The following is copied comment. It seemed say that "if (desc.Format == DXGI_FORMAT_NV12 && !mRenderer->mDisplay->mDisplayExtensions->streamProducerD3DTextureNV12" should be used for checking. But the streamProducerD3DTextureNV12 becomes always true. It could not catch the case that D3D Device does not support NV12 format. Can you explain more about it? 

----------------------------------------------------------------------------------

::: gfx/angle/src/libANGLE/renderer/d3d/d3d11/StreamProducerNV12.cpp
@@ +117,5 @@
>  
> +    // Check that the texture format is supported by our device.
> +    D3D11_TEXTURE2D_DESC desc;
> +    textureD3D->GetDesc(&desc);
> +    if (desc.Format == DXGI_FORMAT_NV12 && !mRenderer->getRenderer11DeviceCaps().supportsNV12)

!mRenderer->mDisplay->mDisplayExtensions->streamProducerD3DTextureNV12
Flags: needinfo?(jgilbert)
The extension should always be available since we need it even if nv12 is not supported by the d3d device.
eglStreamPostD3DTextureANGLE should return false without generating an error if the d3d11 texture isn't supported (because nv12 isn't supported on ANGLE's d3d device).

Add asserts to ensure that this keeps working, because I'm about to update ANGLE and upstream ANGLE has my patches to these entrypoints that stop them from being labelled with nv12. nv12-unsupported ANGLE might be broken again after we take the new ANGLE update.
Flags: needinfo?(jgilbert)
(In reply to Jeff Gilbert [:jgilbert] from comment #15)
> The extension should always be available since we need it even if nv12 is
> not supported by the d3d device.
> eglStreamPostD3DTextureANGLE should return false without generating an error
> if the d3d11 texture isn't supported (because nv12 isn't supported on
> ANGLE's d3d device).

Thanks. I am going to set mDisplayExtensions->streamProducerD3DTextureNV12 to true. Then we could not use the streamProducerD3DTextureNV12 for nv12 support checking. Then I am going to use Renderer11::getNV12TextureSupport().

> 
> Add asserts to ensure that this keeps working, because I'm about to update
> ANGLE and upstream ANGLE has my patches to these entrypoints that stop them
> from being labelled with nv12. nv12-unsupported ANGLE might be broken again
> after we take the new ANGLE update.

RenderDXGITextureHostOGL::EnsureLockable() and RenderDXGIYCbCrTextureHostOGL::EnsureLockable() already have the asserts.
https://dxr.mozilla.org/mozilla-central/source/gfx/webrender_bindings/RenderD3D11TextureHostOGL.cpp#64
https://dxr.mozilla.org/mozilla-central/source/gfx/webrender_bindings/RenderD3D11TextureHostOGL.cpp#268
Attachment #8949651 - Flags: review?(jgilbert)
:jgilbert, I just noticed your comment on IRC:( We could have vidyo.
Flags: needinfo?(jgilbert)
With irc, :jgilbert is going to update ANGLE on 60. It will address the problem.
Flags: needinfo?(jgilbert)
Attachment #8949651 - Flags: review?(jgilbert)
Depends on: 1426596
Depends on: angle-60
With Bug 1440849 fix, the problem was addressed for me.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: