Closed Bug 1328602 Opened 3 years ago Closed 3 years ago

Separate the compositor thread and the render thread (Volume one).

Categories

(Core :: Graphics: WebRender, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54

People

(Reporter: nical, Assigned: nical)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

WrWindowState holds both the api and the renderer which are supposed to be on different threads.
Assignee: nobody → nical.bugzilla
Summary: Split WrWindowState → Separate the compositor thread and the render thread.
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/3694137dfa2b
Expose Renderer through the c bindings. r=gfx?
https://hg.mozilla.org/projects/graphics/rev/a7f20d481858
Expose some more WebRender initialization functions through the C bindings. r=gfx?
https://hg.mozilla.org/projects/graphics/rev/178137a72f3d
Start implementing the render thread. r=gfx?
https://hg.mozilla.org/projects/graphics/rev/7b211ca0a5cb
Add a few comments explaining where some of the pieces are meant to fit. r=gfx?
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/f9c041db5cc4
Follow-up to fix build bustage. r=gfx?
There were actually multiple types of build bustage in that push - the missing |explicit| keyword (comment 5), various instances of unified build bustage. Most importantly, though, the QR reftests started failing as well, which is the real reason for the backout (I could have fixed the bustage in-place, but I don't know what to do about the reftest failures). Example log from reftest failure:

https://treeherder.mozilla.org/logviewer.html#?job_id=66853853&repo=graphics&lineNumber=1633

shows stuff like "assertion failed: unsafe { is_in_render_thread() }"
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/af09a50796fc
Expose Renderer through the c bindings. r=gfx?
https://hg.mozilla.org/projects/graphics/rev/b80f1041fdc7
Expose some more WebRender initialization functions through the C bindings. r=gfx?
https://hg.mozilla.org/projects/graphics/rev/fcbcf320b8c1
Start implementing the render thread. r=gfx?
https://hg.mozilla.org/projects/graphics/rev/7b65e5dc976b
Add a few comments explaining where some of the pieces are meant to fit. r=gfx?
https://hg.mozilla.org/projects/graphics/rev/a61edc75cfb5
Begin implementing a C++ wrapper around WebRender's RenderApi. r=gfx?
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/a4d644d5881f
Fix unified build bustage. r=gfx?
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/efc949b8dd94
Have RendererOGL talk to CompositorBridgeParentBase instead of WebRenderBridgeParent. r=gfx?
https://hg.mozilla.org/projects/graphics/rev/ea4621db7b6a
Begin implementing creating/destroying WebRenderAPI objects. r=gfx?
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/0be9aa8742dc
Provide alternative bindings that don't depend on WrWindowState. r=gfx?
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/1d71945a1e74
Start replacing wrstate with DisplayListBuilder. r=gfx?
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/a56293fb8c1c
Implement more WebRenderAPI methods. r=gfx?
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/2483a6d1670d
Begin making it possible to not have a Compositor object. r=gfx?
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/68f28c9cae2d
Make it possible to enable the work-in-progress render thread code using MOZ_USE_RENDER_THREAD and ugly branching, hopefully temporarily. r=gfx?
Depends on: 1332249
There has been a lot of progress on this front, here are some of remaining things to do off the top of my head:

- make external images work (even sub-optimally), Sotaro has a patch for this
- implement synchronous snapshots
- make the TransactionId and epoch work properly
- synchronous widget resizing
- figure out whether video still works and maybe fix it
- make sure the WebRenderAPI/RendererOGL stuff shuts down properly without leaking
Closing this bug since we're stopping use of Phabricator and it would be cleaner to put follow-up work in a new bug with patches attached in the usual way. The patches in this bug will be audited via Phabricator and we can file follow-up bugs for issues encountered.
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Keywords: leave-open
Resolution: --- → FIXED
I finished a first pass of reviewing these patches. Not very thoroughly, and for the code that was modified multiple times I just looked at what the current state is in the tree rather than looking at each patch. I filed bug 1333503 and bug 1333505 for some follow-up work, and added a comment at https://bugzilla.mozilla.org/show_bug.cgi?id=1332737#c2 about an issue that was exacerbated by these patches. I'll try to do another pass through the code soon.
Summary: Separate the compositor thread and the render thread. → Separate the compositor thread and the render thread (Volume one).
Depends on: 1335658
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/b55c1f67df7b
Fix incorrect return type in ffi signature. r=gfx?
Target Milestone: --- → mozilla54
The push in comment 22 seems like it landed with the wrong bug number, and isn't really associated with this bug.

kats, looks like you're the author on that patch -- do you know which bug that patch was actually associated with?
Flags: needinfo?(bugmail)
(Or maybe it was indeed associated with this bug and it was just a followup that landed after this bug was closed?)
(In reply to Daniel Holbert [:dholbert] from comment #24)
> (Or maybe it was indeed associated with this bug and it was just a followup
> that landed after this bug was closed?)

This. But also for some reason pulsebot never reported the original landing of the patch on the graphics branch, but only reported it once graphics got merged to m-c.
Flags: needinfo?(bugmail)
Depends on: 1432309
You need to log in before you can comment on or make changes to this bug.