D17657: Notify also if modes have changed

[Andras Mantia]

amantia abandoned this revision.
amantia added a comment.


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

REPOSITORY
  R110 KScreen Library

REVISION DETAIL
  https://phabricator.kde.org/D17657

To: amantia, dvratil
Cc: davidedmundson, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17657: Notify also if modes have changed

[Daniel Vrátil]

dvratil added inline comments.

INLINE COMMENTS

> amantia wrote in output.cpp:583-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.

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()`.

REPOSITORY
  R110 KScreen Library

REVISION DETAIL
  https://phabricator.kde.org/D17657

To: amantia, dvratil
Cc: davidedmundson, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17657: Notify also if modes have changed

[Andras Mantia]

amantia updated this revision to Diff 47789.
amantia added a comment.


  Simplify

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17657?vs=47778&id=47789

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D17657

AFFECTED FILES
  src/output.cpp

To: amantia, dvratil
Cc: davidedmundson, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17657: Notify also if modes have changed

[Andras Mantia]

amantia added inline comments.

INLINE COMMENTS

> davidedmundson wrote in output.cpp:583-585
> isn't that what this is doing?

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.

REPOSITORY
  R110 KScreen Library

REVISION DETAIL
  https://phabricator.kde.org/D17657

To: amantia, dvratil
Cc: davidedmundson, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17657: Notify also if modes have changed

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> output.cpp:583-585
>  if (!d->compareModeList(d->modeList, other->d->modeList)) {
>  changes << &Output::outputChanged;
>  }

isn't that what this is doing?

REPOSITORY
  R110 KScreen Library

REVISION DETAIL
  https://phabricator.kde.org/D17657

To: amantia, dvratil
Cc: davidedmundson, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17657: Notify also if modes have changed

[Andras Mantia]

amantia created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
amantia requested review of this revision.

REVISION SUMMARY
  Will be needed in the KCM to correctly update the UI

REPOSITORY
  R110 KScreen Library

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D17657

AFFECTED FILES
  src/output.cpp
  src/output.h

To: amantia
Cc: plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart