refactor: let Control be a QObject
ClosedPublic

Authored by romangg on Dec 15 2019, 8:51 PM.

Details

Reviewers
None
Group Reviewers
Plasma
Commits
R104:e5abdd26fd27: refactor: let Control be a QObject
Summary

This is a preparatory step to add signals later for watching file changes.

Test Plan

Tested in Wayland session.

Diff Detail

Repository
R104 KScreen
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
romangg created this revision.Dec 15 2019, 8:51 PM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 15 2019, 8:51 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
romangg requested review of this revision.Dec 15 2019, 8:51 PM

This is a preparatory step to add signals later for watching file changes.

I don't think I understand. What for?
If it's full output configuration going via the disk that would go against the main part of kscreen architecture.

common/control.h
38–39

-> override

This is a preparatory step to add signals later for watching file changes.

I don't think I understand. What for?
If it's full output configuration going via the disk that would go against the main part of kscreen architecture.

It's not for the full output configuration but only certain settings communicated from KScreen KCM to its daemon and which are not communicated through KWayland / XCB. For now I do/plan it for:

  • Output retention
  • If the refresh rate is automatically chosen or set by user explicitly.
  • If the resolution is automatically chosen or set by user explicitly.
  • If the rotation value is automatically chosen or set by user explicitly.

Example rotation: Default is "Automatic". That means if KScreen daemon detects orientation change via sensor it will send new configuration with updated rotation value for internal screen to KWin via KWayland. User now selects "Manual", then KScreen daemon ignores future orientation changes and not send new configuration via KWayland.

That being said thinking about it some more having a file watcher can lead to races for example in the following case:

  • User changes auto rotation and resolution at the same time while device is held in a position unequal to current rotation
  • KWin receives updated configuration (I)
  • KScreen daemon receives auto rotation change through file watcher before resolution change through KCM -> KWin -> daemon
  • KScreen sends updated configuration (II) with different rotation to KWin
  • KWin sends new configuration (i) and afterwards receives configuration (II)
  • KWin applies (II) and sends (II)
  • KScreen receives first new configuration (I) and then (II)

In this case the resolution would not be set.

I'm unsure how to solve this best. I really don't want to send all auxiliary KScreen data through the KWin connection. Simple solution would be a small delay on control changes waiting for potential KWin changes. A likely better solution might be to introduce this libkscreen dbus out of process service found on RandR backend to the KWayland backend and then send auxiliary data through there but not to the compositor.

romangg added inline comments.Dec 15 2019, 11:27 PM
kded/config.cpp
45

Caching it means we don't read in new retention value when it changed. So this patch without a file watcher yet or other system to communicate changes breaks the runtime. Solution without it: do not cache but in writeFile always create new object for now.

romangg marked 2 inline comments as done.Dec 25 2019, 2:54 PM

I move forward with this approach and revisit later to work on a better solution. Maybe with the libkscreen dbus process.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 25 2019, 2:54 PM
This revision was automatically updated to reflect the committed changes.