1391481 - Restore texture binding state before exiting RenderDXGITextureHostOGL::EnsureLockable()

Status: RESOLVED INVALID

(Core :: Graphics: WebRender, enhancement)

Description

[Sotaro Ikeda [:sotaro]]

RenderDXGITextureHostOGL::EnsureLockable() updates inding of GL_TEXTURE_2D. But WebRender keeps texture binding state and uses the state to update texture binding. EnsureLockable() breaks how to bind texture by webrender.

Therefore EnsureLockable() have to restore texture binding state of GL_TEXTURE_2D when exiting the function.
 https://dxr.mozilla.org/mozilla-central/source/gfx/webrender/src/device.rs#945

Comment 1

[Sotaro Ikeda [:sotaro]]

The following is a call stack when the problem happened

> libGLESv2.dll!gl::Context::handleError(const gl::Error & error) Line 1945	C++
> libGLESv2.dll!gl::ValidateTexImageFormatCombination() Line 46	C++
> libGLESv2.dll!gl::ValidateES3TexImageParametersBase() Line 211	C++
> libGLESv2.dll!gl::ValidateTexSubImage2D() Line 2021	C++
> libGLESv2.dll!gl::TexSubImage2D() Line 2330	C++
> xul.dll!gleam::gl::{{impl}}::tex_sub_image_2d(a) Line 487	Unknown
> xul.dll!webrender::device::Device::update_texture_from_pbo() Line 1628	Unknown
> xul.dll!webrender::renderer::{{impl}}::render::{{closure}}(closure) Line 1521	Unknown
> xul.dll!webrender::renderer::Renderer::render() Line 1506	Unknown
> xul.dll!mozilla::wr::RendererOGL::Render() Line 162	C++
> xul.dll!mozilla::wr::RenderThread::UpdateAndRender(mozilla::wr::WrWindowId aWindowId) Line 223	C++
> xul.dll!mozilla::wr::RenderThread::NewFrameReady(mozilla::wr::WrWindowId aWindowId) Line 170	C++
> xul.dll!mozilla::detail::RunnableMethodImpl<RefPtr<mozilla::GetUserMediaWindowListener>,bool>::Run() Line 1195	C++

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Restore texture binding state before exiting RenderDXGITextureHostOGL::EnsureLockable()

Comment 3

[Nicolas Silva [:nical]]

Comment on attachment 8898543 [details] [diff] [review]
patch - Restore texture binding state before exiting RenderDXGITextureHostOGL::EnsureLockable()

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

I filed https://github.com/servo/webrender/issues/1593 on the webrender side. I would feel a lot better about fixing this in webrender because ScopedBindTexture incorrectly assumes that the texture target to be restored is the same as the current one which is not necessarily true. Since webrender uses several targets, we would run into bugs if the previously bound texture target was different from GL_TEXTURE_2D.

I am giving this a soft non-definitive r- because I think we should try to fix this in webrender first. But we can revisit this approach if for whatever reason we decide not to do it in webrender (but then we'd still have to fix the texture target issue).

::: gfx/thebes/gfxPlatform.cpp
@@ +2462,5 @@
> +//      FeatureStatus::Unavailable,
> +//      "GPU Process is disabled",
> +//      NS_LITERAL_CSTRING("FEATURE_FAILURE_GPU_PROCESS_DISABLED"));
> +//  }
> +//#endif

I assume this would be removed from the patch before landing.

Comment 4

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Restore texture bindings after handling external images

Comment 5

[Sotaro Ikeda [:sotaro]]

This bug is going to be addressed by https://github.com/servo/webrender/pull/1609.