1407748 - Windows 2012 x64 debug build with MOZ_WEBRENDER=1 has an assertion failure on startup

Status: RESOLVED FIXED

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

Description

[Kartikaya Gupta (email:[e-mail])]

1. Download a Windows 2012 x64 debug build from TreeHerder and run it with webrender enabled (MOZ_WEBRENDER=1 in the env, or set gfx.webrender.enabled=true and restart)
2. Observe an assertion failure/crash on startup. In some cases this goes into an infinite loop of crashiness.

I bisected this using builds from TreeHerder, and it started with the ANGLE update in bug 1371190.

This is what the stack of the failure looks like (taken from a try push that tries to run reftests with WR enabled):

Assertion failure: mSurface, at z:/build/build/src/gfx/webrender_bindings/RenderD3D11TextureHostOGL.cpp:99
#01: mozilla::wr::RenderDXGITextureHostOGL::Lock() [gfx/webrender_bindings/RenderD3D11TextureHostOGL.cpp:184]
#02: mozilla::wr::LockExternalImage(void *,mozilla::wr::WrExternalImageId,unsigned char) [gfx/webrender_bindings/RendererOGL.cpp:46]
#03: webrender_bindings::bindings::{{impl}}::lock [gfx/webrender_bindings/src/bindings.rs:295]
#04: webrender::renderer::Renderer::update_deferred_resolves [gfx/webrender/src/renderer.rs:2948]
#05: webrender::renderer::Renderer::update_gpu_cache [gfx/webrender/src/renderer.rs:2183]
#06: webrender::renderer::{{impl}}::render::{{closure}} [gfx/webrender/src/renderer.rs:2098]
#07: webrender::profiler::TimeProfileCounter::profile<webrender::device::FrameId,closure> [gfx/webrender/src/profiler.rs:198]
#08: webrender::renderer::Renderer::render [gfx/webrender/src/renderer.rs:2083]
#09: webrender_bindings::bindings::wr_renderer_render [gfx/webrender_bindings/src/bindings.rs:467]
#10: mozilla::wr::RendererOGL::Render() [gfx/webrender_bindings/RendererOGL.cpp:177]
#11: mozilla::wr::RenderThread::UpdateAndRender(mozilla::wr::WrWindowId) [gfx/webrender_bindings/RenderThread.cpp:223]
#12: mozilla::wr::RenderThread::NewFrameReady(mozilla::wr::WrWindowId) [gfx/webrender_bindings/RenderThread.cpp:170]
...

Comment 1

[Sotaro Ikeda [:sotaro]]

egl->fCreatePbufferFromClientBuffer() was failed at the following in ValidateCreatePbufferFromClientBuffer()

  >    if ((textureFormat == EGL_TEXTURE_RGB  && config->bindToTextureRGB  != EGL_TRUE) ||
  >        (textureFormat == EGL_TEXTURE_RGBA && config->bindToTextureRGBA != EGL_TRUE))

  https://dxr.mozilla.org/mozilla-central/source/gfx/angle/src/libANGLE/validationEGL.cpp#1100

With current config, config->bindToTextureRGB = EGL_TRUE and config->bindToTextureRGBA != EGL_TRUE. But textureFormat == EGL_TEXTURE_RGBA.

Comment 2

[Sotaro Ikeda [:sotaro]]

If we want to create Pbuffer with EGL_TEXTURE_RGBA. config->bindToTextureRGBA needs to be EGL_TRUE.

Comment 3

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #2)
> If we want to create Pbuffer with EGL_TEXTURE_RGBA.
> config->bindToTextureRGBA needs to be EGL_TRUE.

I confirmed that if config->bindToTextureRGBA == EGL_TRUE was true, the problem was addressed.

Comment 4

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #3)
> (In reply to Sotaro Ikeda [:sotaro] from comment #2)
> > If we want to create Pbuffer with EGL_TEXTURE_RGBA.
> > config->bindToTextureRGBA needs to be EGL_TRUE.
> 
> I confirmed that if config->bindToTextureRGBA == EGL_TRUE was true, the
> problem was addressed.

bug 1371190 also did similar thing
  https://hg.mozilla.org/mozilla-central/rev/162ddc0d22f6

Comment 5

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Force enable alpha channel to make sure ANGLE use correct framebuffer formart

Comment 6

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5621c7f95b0bef6ffb9d9e22cff579a17288e19a

Comment 7

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Force enable alpha channel to make sure ANGLE use correct framebuffer formart

Comment 8

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8917740 [details] [diff] [review]
patch - Force enable alpha channel to make sure ANGLE use correct framebuffer formart

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

::: gfx/gl/GLContextProviderEGL.cpp
@@ +179,5 @@
>  
>      bool doubleBuffered = true;
>  
>      EGLConfig config;
> +#ifdef XP_WIN

Why #ifdef this?

@@ +182,5 @@
>      EGLConfig config;
> +#ifdef XP_WIN
> +    if (aWebRender && sEGLLibrary.IsANGLE()) {
> +        // Force enable alpha channel to make sure ANGLE use correct framebuffer formart
> +        if (!CreateConfig(&config, /* depth */ 32, /* aEnableDepthBuffer */ true)) {

It's cleaner to declare locals with the values than inlining comments.
Consider:
const int bpp = 32;
const bool withDepth = true;
if (!CreateConfig(&config, bpp, withDepth))

Comment 9

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8917740 [details] [diff] [review]
patch - Force enable alpha channel to make sure ANGLE use correct framebuffer formart

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

::: gfx/gl/GLContextProviderEGL.cpp
@@ +179,5 @@
>  
>      bool doubleBuffered = true;
>  
>      EGLConfig config;
> +#ifdef XP_WIN

Yea, it could  be removed. I'll update it in a next patch.

@@ +182,5 @@
>      EGLConfig config;
> +#ifdef XP_WIN
> +    if (aWebRender && sEGLLibrary.IsANGLE()) {
> +        // Force enable alpha channel to make sure ANGLE use correct framebuffer formart
> +        if (!CreateConfig(&config, /* depth */ 32, /* aEnableDepthBuffer */ true)) {

I'll update it in a next patch.

Comment 10

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Force enable alpha channel to make sure ANGLE use correct framebuffer formart

Applied the comments.

Comment 11

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8918041 [details] [diff] [review]
patch - Force enable alpha channel to make sure ANGLE use correct framebuffer formart

:jgilbert, can you review a patch again?

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5278d6756359
Force enable alpha channel to make sure ANGLE use correct framebuffer formart r=jgilbert

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5278d6756359