Closed Bug 1391481 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Graphics: WebRender, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: sotaro, Unassigned)

References

Details

Attachments

(2 obsolete files)

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
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++
Blocks: 1357299
Assignee: nobody → sotaro.ikeda.g
Summary: Restore texture binding state at RenderDXGITextureHostOGL::EnsureLockable() → Restore texture binding state before exiting RenderDXGITextureHostOGL::EnsureLockable()
Attachment #8898543 - Flags: review?(nical.bugzilla)
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.
Attachment #8898543 - Flags: review?(nical.bugzilla) → review-
Attachment #8898543 - Attachment is obsolete: true
Attachment #8900599 - Attachment is obsolete: true
Assignee: sotaro.ikeda.g → nobody
This bug is going to be addressed by https://github.com/servo/webrender/pull/1609.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.