Notify also if modes have changed
AbandonedPublic

Authored by amantia on Dec 18 2018, 3:56 PM.

Details

Summary

Will be needed in the KCM to correctly update the UI

Diff Detail

Repository
R110 KScreen Library
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6146
Build 6164: arc lint + arc unit
amantia created this revision.Dec 18 2018, 3:56 PM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 18 2018, 3:56 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
amantia requested review of this revision.Dec 18 2018, 3:56 PM
davidedmundson added inline comments.
src/output.cpp
582–585

isn't that what this is doing?

amantia added inline comments.Dec 18 2018, 6:50 PM
src/output.cpp
582–585

Good point, missed it. @dvratil any reason why it emits outputChanged and not modesChanged? Actually setModes emits both, so maybe indeed it is easier to just put modesChnaged there as well. I'm still curious why we need both.

amantia updated this revision to Diff 47789.Dec 18 2018, 6:55 PM

Simplify

amantia marked an inline comment as done.Dec 18 2018, 6:55 PM
dvratil added inline comments.Dec 19 2018, 4:54 PM
src/output.cpp
582–585

There are properties that are related to the overall output state and will never change individually (e.g. id, type, name). Technically modes /may/ change independently of the output (it is possible to add modes in XRandR), but historically we did not support that, so modes were always coupled with the output state as well. I think this is the correct approach to fix a bug, but longterm someone should probably look into what happens when the outputChanged() signal is removed from here and from setModes() as there might be some code listening to it instead of modesChanged().

amantia abandoned this revision.Dec 19 2018, 4:58 PM

Will fix first in 5.12 and merge to master, as per https://phabricator.kde.org/D17685