[platforms/x11] Never block on retrace, always present after paint
Needs ReviewPublic

Authored by romangg on Aug 27 2019, 9:41 PM.

Details

Reviewers
None
Group Reviewers
KWin
Maniphest Tasks
T11071: Rework compositing pipeline
Summary

Compositing in X11 was done time shifted, meaning that we paint first, then
wait one vblank interval length and present on prepareRenderingFrame the
previous paint result. This is supposed to make sure we don't miss the vblank
and in case of block till retrace be able to continue issuing commands and
only shortly before next vblank present.

This is counter-intuitiv, not how we do it on Wayland or even on MESA with X.
The reason seems to be that the GLX backend was in the beginning written
against Nvidia proprietary driver which needed this but nowadays even this
driver defaults to non-blocking behavior on buffer swap.

Therefore remove this legacy anomaly fully and directly present after paint.
We then wait one refresh cycle and in the future can optimize this by delaying
the paint and present till shortly before vsync.

Test Plan

kwin_x11 tested on i915 and Nvidia proprietary driver.

Diff Detail

Branch
mvPresent
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 16829
Build 16847: arc lint + arc unit
romangg created this revision.Aug 27 2019, 9:41 PM
Restricted Application added a project: KWin. · View Herald TranscriptAug 27 2019, 9:41 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
romangg requested review of this revision.Aug 27 2019, 9:41 PM
romangg updated this revision to Diff 64781.Aug 27 2019, 9:59 PM

Cache refresh rate on Wayland. Otherwise it is calculated again on every present.

I tested now also on Nvidia proprietary driver and there were no issues.

romangg edited the test plan for this revision. (Show Details)Aug 28 2019, 11:02 AM

As usual, do you need help testing this on amdgpu? 😜

As usual, do you need help testing this on amdgpu? 😜

Sure. :) Can you check X and Wayland sessions?

alexeymin added a comment.EditedAug 28 2019, 7:18 PM

Sure. :) Can you check X and Wayland sessions?

I've tested both. Works fine, wayland one seem to work even smoother that x11. On X I feel some kind of slight fps drop when dragging window, hard to tell for sure. Wayland seems perfect (I only miss working keyboard repeat/delay setting and middle-click paste)

P.S. tested X again, no, all fine, maybe I'm just seeing things

P.P.S. X11 I am using all the time, I did make && sudo make install into my main system and living with that. Into Wayland I only reboot eventually for a smaller periods of time, to test something

zzag added a subscriber: zzag.Sep 5 2019, 8:56 AM

Given that the feature freeze is closer and closer, are you okay if your changes go to 5.18? That way we'll have more time to test these patches.

workspace.cpp
1611–1612

No, we don't need to print this line anymore because it will always say "removed."

In D23514#525852, @zzag wrote:

Given that the feature freeze is closer and closer, are you okay if your changes go to 5.18? That way we'll have more time to test these patches.

Sorry if I wasn't clear on that, my plan was anyway to merge it only after 5.17 release to have enough time testing them on master since the changes are fundamental to the compositing pipeline and in some way a bit experimental.

workspace.cpp
1611–1612

I thought I leave it in so it's clear that the feature got "removed". But maybe it's better to just not print the line.

romangg updated this revision to Diff 66536.Fri, Sep 20, 10:51 AM
romangg marked 2 inline comments as done.
  • Remove vertical retrace mention in supportInformation
  • Rebase on master and series changes
zzag added a comment.Fri, Sep 20, 4:48 PM
This comment was removed by zzag.