drm: Save scaling and position of displays at shutdown
ClosedPublic

Authored by apol on Jul 5 2019, 5:38 PM.

Details

Summary

Position and most importantly scaling will likely be the same on the next run,
This saves a flicker round at startup when scaling is different of 1 initialising the view at 1 then jumping at whatever the user requested.

Test Plan

Restarted my system several times

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
apol created this revision.Jul 5 2019, 5:38 PM
Restricted Application added a project: KWin. · View Herald TranscriptJul 5 2019, 5:38 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
apol requested review of this revision.Jul 5 2019, 5:38 PM
zzag added subscribers: romangg, zzag.Jul 8 2019, 8:09 AM

Hmm, is this correct though? I thought output config is meant to be modified by the user.

@romangg Can you weigh in on this?

In the Drm backend still a code path exists to read output configuration from a KWin specific configuration file (for Plasma Mobile I believe). That is being used here. However the main way to configure outputs is through KScreen interface and the idea was to more to remove the alternative code path instead of expanding its usage.

An alternative approach to solving the flicker would be to delay showing the first frame until KScreen has provided the config (or a timeout occurs). This would then also work if a screen has been disconnected in between. However we could still use this patch here as a flicker fix in between.

apol added a comment.Jul 8 2019, 1:30 PM

This is correct as in we initialise DrmOutput and then KScreen does its thing. What this patch does is to actually try to initialise at a more likely state.

I don't think we want to wait for KScreen. As we can see nowadays, it takes quite a bit longer to arrive and we probably don't want to have al KWin or the session on hold.

zzag added a comment.Jul 9 2019, 6:28 PM

If Roman is okay with this change, feel free to push it.

Though please change drm: in the subject line to [platforms/drm].

zzag added a comment.Jul 9 2019, 6:33 PM

In the Drm backend still a code path exists to read output configuration from a KWin specific configuration file (for Plasma Mobile I believe). That is being used here. However the main way to configure outputs is through KScreen interface and the idea was to more to remove the alternative code path instead of expanding its usage.

An alternative approach to solving the flicker would be to delay showing the first frame until KScreen has provided the config (or a timeout occurs). This would then also work if a screen has been disconnected in between. However we could still use this patch here as a flicker fix in between.

All this KScreen stuff is a mess to be honest. KWin/Wayland needs to be in charge, not KScreen.

In D22292#493096, @zzag wrote:

All this KScreen stuff is a mess to be honest. KWin/Wayland needs to be in charge, not KScreen.

I might agree with that. It was done quite some time ago. The basic idea was to put all config handling outside of KWin and independent of the compositor/windowing system I believe. What's your argument against it? Might want to create a task to brainstorm it?

In D22292#492297, @apol wrote:

This is correct as in we initialise DrmOutput and then KScreen does its thing. What this patch does is to actually try to initialise at a more likely state.

I don't think we want to wait for KScreen. As we can see nowadays, it takes quite a bit longer to arrive and we probably don't want to have al KWin or the session on hold.

I can't imagine that it's such a long delay tbh. But let's do this here anyway as a quick fix. Please only remember the scale value though. There might be problems when the position gets saved relative to a display being removed in between.

apol updated this revision to Diff 61472.Jul 9 2019, 10:42 PM

Only store the scale

All this KScreen stuff is a mess to be honest. KWin/Wayland needs to be in charge, not KScreen.

Certainly having two running processes try and manipulate the same thing at once storing the same data in two places is a mess.

What the correct fix is up for discussion. I don't think there's anything inherently wrong with the majority of kscreen.

I don't think this patch in itself is the right ultimate solution, and I don't want to see it expanded upon to recreate a second kscreen.
As a temporary cache - meh, no objections either.

Go for it.

romangg accepted this revision.Jul 10 2019, 4:51 PM
This revision is now accepted and ready to land.Jul 10 2019, 4:51 PM
This revision was automatically updated to reflect the committed changes.