1696025 - Assertion failure: !mLockedBits, at /builds/worker/checkouts/gecko/gfx/2d/DrawTargetCairo.cpp:582

Status: RESOLVED FIXED

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

Description

[Andrew Osmond [:aosmond] (he/him)]

This was uncovered trying to turn on SW-WR popups in CI.

https://treeherder.mozilla.org/logviewer?job_id=331779098&repo=try&lineNumber=9671

[task 2021-03-02T17:31:16.285Z] 17:31:16     INFO - GECKO(2888) | Assertion failure: !mLockedBits, at /builds/worker/checkouts/gecko/gfx/2d/DrawTargetCairo.cpp:582
[task 2021-03-02T17:31:16.414Z] 17:31:16     INFO - GECKO(2888) | #01: mozilla::wr::RenderCompositorSWGL::BeginFrame() [gfx/webrender_bindings/RenderCompositorSWGL.cpp:53]
[task 2021-03-02T17:31:16.415Z] 17:31:16     INFO - GECKO(2888) | #02: mozilla::wr::RendererOGL::UpdateAndRender(mozilla::Maybe<mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> > const&, mozilla::Maybe<mozilla::wr::ImageFormat> const&, mozilla::Maybe<mozilla::Range<unsigned char> > const&, bool*, mozilla::wr::RendererStats*) [gfx/webrender_bindings/RendererOGL.cpp:164]
[task 2021-03-02T17:31:16.415Z] 17:31:16     INFO - GECKO(2888) | #03: mozilla::wr::RenderThread::UpdateAndRender(mozilla::wr::WrWindowId, mozilla::layers::BaseTransactionId<mozilla::VsyncIdType> const&, mozilla::TimeStamp const&, bool, mozilla::Maybe<mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> > const&, mozilla::Maybe<mozilla::wr::ImageFormat> const&, mozilla::Maybe<mozilla::Range<unsigned char> > const&, bool*) [gfx/webrender_bindings/RenderThread.cpp:482]
[task 2021-03-02T17:31:16.416Z] 17:31:16     INFO - GECKO(2888) | #04: mozilla::wr::RenderThread::HandleFrameOneDoc(mozilla::wr::WrWindowId, bool) [gfx/webrender_bindings/RenderThread.cpp:343]
[task 2021-03-02T17:31:16.416Z] 17:31:16     INFO - GECKO(2888) | #05: mozilla::detail::RunnableMethodImpl<mozilla::wr::RenderThread *,void (mozilla::wr::RenderThread::*)(mozilla::wr::WrWindowId, bool),1,mozilla::RunnableKind::Standard,mozilla::wr::WrWindowId,bool>::Run() [xpcom/threads/nsThreadUtils.h:1204]
[task 2021-03-02T17:31:16.416Z] 17:31:16     INFO - GECKO(2888) | #06: MessageLoop::RunTask(already_AddRefed<nsIRunnable>) [ipc/chromium/src/base/message_loop.cc:469]
[task 2021-03-02T17:31:16.417Z] 17:31:16     INFO - GECKO(2888) | #07: MessageLoop::DoWork() [ipc/chromium/src/base/message_loop.cc:554]
[task 2021-03-02T17:31:16.417Z] 17:31:16     INFO - GECKO(2888) | #08: base::MessagePumpDefault::Run(base::MessagePump::Delegate*) [ipc/chromium/src/base/message_pump_default.cc:35]
[task 2021-03-02T17:31:16.417Z] 17:31:16     INFO - GECKO(2888) | #09: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:329]
[task 2021-03-02T17:31:16.417Z] 17:31:16     INFO - GECKO(2888) | #10: base::Thread::ThreadMain() [ipc/chromium/src/base/thread.cc:191]
[task 2021-03-02T17:31:16.417Z] 17:31:16     INFO - GECKO(2888) | #11: `anonymous namespace'::ThreadFunc(void*) [ipc/chromium/src/base/platform_thread_win.cc:20]
[task 2021-03-02T17:31:16.418Z] 17:31:16     INFO - GECKO(2888) | #12: BaseThreadInitThunk [C:\Windows\System32\KERNEL32.DLL + 0x13034]
[task 2021-03-02T17:31:16.477Z] 17:31:16     INFO - GECKO(2888) | #13: patched_BaseThreadInitThunk(int, void*, void*) [mozglue/dllservices/WindowsDllBlocklist.cpp:587]
[task 2021-03-02T17:31:16.477Z] 17:31:16     INFO - GECKO(2888) | #14: RtlUserThreadStart [C:\Windows\SYSTEM32\ntdll.dll + 0x71461]

If RenderCompositorSWGL::mSurface is not null, we will always try to draw to the target with it and skip calling ReleaseBits here:

https://searchfox.org/mozilla-central/rev/c24ecdc6f5025ea7e60d0691673de030bd5bf411/gfx/webrender_bindings/RenderCompositorSWGL.cpp#202

If we successfully lock after creating the surface, we should probably clear mSurface here:

https://searchfox.org/mozilla-central/rev/c24ecdc6f5025ea7e60d0691673de030bd5bf411/gfx/webrender_bindings/RenderCompositorSWGL.cpp#86

Otherwise we will draw to one buffer, and present another.

Comment 1

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1696025 - Ensure we release our intermediate surface if we switch back to the DrawTarget's buffer in SWGL.

When mapping a buffer for SWGL to draw to, we first attempt to lock the
draw target's buffer directly. If this fails we create an intermediate
surface for drawing and copy that to the draw target when finished. If
at a later point we successfully manage to lock the DT's buffer, we
should release the intermediate surface because we use its existence to
decide if we need to copy back into the DT or just release our lock.

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1696025 - Avoid allocating a draw target when not presenting frames.

Internally WebRender may try to redraw a frame to update its picture
cache textures, but the frame will not be presented. This patch prevents
us from allocating a draw target in that case, avoiding an assert when
said draw target is freed without releasing our buffer lock (done when
we present.)

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/40abe6235712
Avoid allocating a draw target when not presenting frames. r=lsalzman

Comment 4

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/40abe6235712