Closed Bug 1437949 Opened 3 years ago Closed 3 years ago

wake_up implementation should poke the renderer to check for messages

Categories

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

Other Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(2 files)

The WR capturing mechanism relies on the wake_up callback [1] to process the DebugOutput message in the renderer's queue, which writes external images to files. Right now the wake_up callback is implemented as a no-op [2] when really what it should do is trigger a call to wr_renderer_update on the Renderer thread.

[1] https://searchfox.org/mozilla-central/rev/9d47b5fb14152865ad1375eb3ee2e571d81ecdb9/gfx/webrender/src/render_backend.rs#644
[2] https://searchfox.org/mozilla-central/rev/9d47b5fb14152865ad1375eb3ee2e571d81ecdb9/gfx/webrender_bindings/RenderThread.cpp#468
Assignee: nobody → bugmail
Comment on attachment 8950662 [details]
Bug 1437949 - Make the wake_up WR notification poke the renderer.

https://reviewboard.mozilla.org/r/219916/#review225726

::: gfx/webrender_bindings/RenderThread.cpp:187
(Diff revision 1)
> +    return;
> +  }
> +
> +  if (!IsInRenderThread()) {
> +    Loop()->PostTask(
> +      NewRunnableMethod<wr::WindowId>("wr::RenderThread::WaekUp",

typo "WaekUp"
Attachment #8950662 - Flags: review?(kvark) → review+
(In reply to Dzmitry Malyshau [:kvark] from comment #3)
> typo "WaekUp"

Good catch! Fixed.
Don't we need to call MakeCurrent() in RendererOGL::Update(), do we?
Good point, Sotaro. WebRender may need to upload some texture data, so GL context is needed to be valid.
Yea, and GL context needs to be current if there are multiple windows(multiple gl contexts).
Good point. I already triggered autolanding of this patch (and there's no way to stop it) but after it lands I'll push a follow-up to add the MakeCurrent call.
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/37935fa7ffc2
Make the wake_up WR notification poke the renderer. r=kvark
https://hg.mozilla.org/mozilla-central/rev/37935fa7ffc2
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Attachment #8951166 - Flags: review?(sotaro.ikeda.g) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/16fab079284b
Follow-up to make the GL context current before poking the WR renderer. r=sotaro
You need to log in before you can comment on or make changes to this bug.