compositor: When WebRender is in use, only composite on new WebRender frames.

[pcwalton]

Scheduling composition on scroll and so forth is unnecessary and can
cause us to miss frames if the code happens to start the composition
while the WebRender backend is still in the process of preparing the
frame. This is most easily seen when scrolling in full-screen mode in
release builds on Mac.

Closes #9879.

r? @glennw
cc @paulrouget
cc @tschneidereit

---

This change is

[pcwalton]

The reason why syscalls affected how often we ran into this bug is presumably that entering the kernel would tend to induce the scheduler to switch to the WebRender thread before composition occurred, which would let the WebRender backend win the race more often.

[samlh]

I think this code needs a time-series diagram showing the messages sent and received by the different threads over the life of a single frame. It would help me understand whether my questions below even make sense :).

1. Does this cause servo to render immediately after any scroll event, or does it wait for a vsync or timer?
2. Does the desired behavior change based on webrender on/off?
3. How does the actual behavior change based on webrender on/off and with/without this change?
4. Do other threads change behavior based on webrender on/off in a way that requires differences here?

I'm just a curious bystander so feel free to ignore these questions, but I do think making webrender act differently here is making me even more confused :/.

[pcwalton]

Does this cause servo to render immediately after any scroll event, or does it wait for a vsync or timer?

It always is implicitly sync'd to vsync because the swap interval is 1.

Does the desired behavior change based on webrender on/off?

Sort of. There shouldn't be any user-visible difference. But in the non-WR case, the compositor is responsible for redisplaying tiles after a scroll, while in the WR case the WR backend thread is responsible for that.

How does the actual behavior change based on webrender on/off and with/without this change?

Without this change, with WebRender on, when a scroll happens causing a previously rendered frame N to transition to N+1, sometimes frame N will be rendered (CompositingReason::Scroll) and sometimes frame N+1 (the correct one) will be rendered (CompositingReason::NewWebRenderFrame).

Do other threads change behavior based on webrender on/off in a way that requires differences here?

Not really.

[samlh]

Ok, thanks for the explanation. I've reread the code, and I'm summarizing the new state as I understand it - please let me know if it makes sense. If so, I'll try to put together a PR with some comments around this :).

If Webrender is enabled and an OS event occurs:

1. Event handling is triggered when an OS message occurs.
2. This handles both OS and inter-thread messages, during which webrender_api.set_root_pipeline may be called, indirectly triggering a Recomposite message
3. If a Recomposite is received from webrender, or a LoadComplete or IsReadyToSaveImageReply message is received in headless mode, composition_request is set to CompositeNow.
4. Msg::CreatePng triggers composition immediately - this looks unavoidable.
5. WindowEvent::Refresh triggers an composition immediately - is this intended?
6. Once the messages are handled, merged scroll updates are sent via webrender_api.scroll
7. If composition_request is set to CompositeNow, then composition occurs. Note that DelayedComposite does nothing.
8. Composition consists of webrender.update(), webrender.render(), and window.present()
9. After composition, start_scrolling_bounce_if_necessary may trigger webrender to later send a new Recomposite message. Also, process_animations may cause layout to send a new display list to webrender which will also cause a Recomposite message.

If webrender is enabled and a resize occurs on OS X:

1. Event handling is triggered by an callback method (Apple, why?)
2. This handles inter-thread messages only, in a loop, until a Recomposite is triggered by webrender (this will not handle scrolling, unlike step 6 above)

Anyhow, other than the question in step 5 above, I think this all makes sense, and is good improvement for the webrender case. I feel like the non-webrender case could use some cleanup, but I'll look into that myself.

[pcwalton]

Refresh means that the OS wants us to redraw the window. So yes, it should trigger a WR repaint.

[samlh]

Right, but why not just set composition_request to CompositeNow, and let the check at the end of handle_events do the composition?

[pcwalton]

Yeah, I guess that would work too.

[glennw]

Nice find!

[glennw]

@bors-servo r+

[bors-servo]

📌 Commit ed90011 has been approved by glennw

[bors-servo]

⌛ Testing commit ed90011 with merge a868bb2...

[bors-servo]

💔 Test failed - windows

[cbrewster]

@bors-servo retry

• Github connection issue

[bors-servo]

⚡ Previous build results for arm32, linux-dev are reusable. Rebuilding only android, arm64, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows...

[bors-servo]

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows

[frewsxcv]

Just tried this out. Seems to be a lot smoother. Nice work!