D21082: Fix lid open/closed configurations, OSD actions, more than 2 screens

[Robert Hoffmann]

hoffmannrobert added a comment.


  I tested the new code:
  Tests 1-5 of the test plan behave like previously "without patch", in tests 6 
and 7 the integrated laptop screen isn't reacitivated after opening.
  
  After adapting my changes to the new code, all tests work as described "with 
patch".
  
  The additional fixes are not independent of the first fix, so they are not 
separated.

REPOSITORY
  R104 KScreen

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

To: hoffmannrobert, #plasma, romangg
Cc: ngraham, romangg, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21082: Fix lid open/closed configurations, OSD actions, more than 2 screens

[Robert Hoffmann]

hoffmannrobert updated this revision to Diff 60637.
hoffmannrobert edited the summary of this revision.
hoffmannrobert edited the test plan for this revision.
hoffmannrobert added a comment.


  Rebase and adapt to new upstream code

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21082?vs=58471&id=60637

BRANCH
  fix_lid_open_closed_configs_new_upstream_code

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

AFFECTED FILES
  kded/config.cpp
  kded/config.h
  kded/daemon.cpp
  kded/generator.cpp
  kded/generator.h
  kded/osdmanager.cpp
  kded/osdmanager.h
  tests/kded/configtest.cpp
  tests/osd/osdtest.cpp

To: hoffmannrobert, #plasma, romangg
Cc: ngraham, romangg, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21082: Fix lid open/closed configurations, OSD actions, more than 2 screens

[Roman Gilg]

romangg added a comment.


  I added your diff to a task I created for tracking some improvements to 
KScreen.
  
  Please test out how lid open/closed configs behave with the new code on 
master branch. Also rebase on current master.
  
  Put the additional two fixes you did in separate diffs if they are 
independent of open/closed configs question. It sounds like that.
  
  Thank you!

REPOSITORY
  R104 KScreen

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

To: hoffmannrobert, #plasma, romangg
Cc: ngraham, romangg, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21082: Fix lid open/closed configurations

[Robert Hoffmann]

hoffmannrobert added a comment.


  I added some more fixes:
  
  - The OSD was displayed even on a closed laptop panel.
  - With more than two screens, using the OSD always generated an "extend to 
right" configuration, even if "extend to left", "clone", or "disable 
external/internal" was selected. Now these options do what they say with more 
than two screens, too.

REPOSITORY
  R104 KScreen

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

To: hoffmannrobert, #plasma, romangg
Cc: ngraham, romangg, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D21082: Fix lid open/closed configurations

[Robert Hoffmann]

hoffmannrobert added a comment.


  In D21082#463848 , @romangg wrote:
  
  > This is the real issue. Why are the screen positions scrambled around 
although there should be a config from before shutdown/suspend (not the 
lidOpened one), that has the current positions stored. Apparently this one has 
the wrong positions saved as well. You could check this in your config files.
  
  
  The problem is, there is no existing config anymore. If you configure screens 
with the lid closed, as soon as you open it again, the previous config is gone.
  
  I added this scenario as Test 7 in the test plan.
  
  > If between yesterday and in the morning they never opened the lid, there 
should still be a lidOpened-config from the last time yesterday they closed the 
lid.
  
  I guess they opened it some time after configuring and before logging out.

REPOSITORY
  R104 KScreen

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

To: hoffmannrobert, #plasma, romangg
Cc: ngraham, romangg, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D21082: Fix lid open/closed configurations

[Robert Hoffmann]

hoffmannrobert updated this revision to Diff 58471.
hoffmannrobert added a comment.


  - Fix OSD displayed on closed laptop panel.
  - Fix generated configurations if there are more than two screens.

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21082?vs=57763&id=58471

BRANCH
  fix_lid_open_closed_configs

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

AFFECTED FILES
  kded/daemon.cpp
  kded/generator.cpp
  kded/generator.h
  kded/osdmanager.cpp
  kded/osdmanager.h
  kded/serializer.cpp
  tests/osd/osdtest.cpp

To: hoffmannrobert, #plasma, romangg
Cc: ngraham, romangg, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D21082: Fix lid open/closed configurations

[Roman Gilg]

romangg added a comment.


  In D21082#463700 , @hoffmannrobert 
wrote:
  
  > My goal is to make the screen configurations work reliably with notebooks, 
not to support a new use case. The use case is: work with a notebook docked and 
undocked, with and without external screen(s), lid open or closed.
  >
  > The configurations work fine, if no notebook with a lid that can be opened 
or closed is involved.
  >
  > But several users report, that they switch on their notebook (in docking 
station) in the morning, lid is closed, log in, and the external screens are 
swapped left and right again. Or only the empty background of the second screen 
is shown on the external monitor while the primary screen is on the closed 
integrated notebook screen.
  
  
  This is the real issue. Why are the screen positions scrambled around 
although there should be a config from before shutdown/suspend (not the 
lidOpened one), that has the current positions stored. Apparently this one has 
the wrong positions saved as well. You could check this in your config files.
  
  > So they configure it again as needed, while the lid is closed. But there is 
already a lidOpened-configuration lurking around, waiting for the lid to be 
opened and to destroy the configuration previously made.
  > 
  > Where does the wrong lidOpened-configuration come from? I guess it's 
created automatically when the lid is open and a button on the OSD ("Extend to 
right" or so) is clicked. After closing the lid, this configuration is copied 
to "lidOpened".
  
  If between yesterday and in the morning they never opened the lid, there 
should still be a lidOpened-config from the last time yesterday they closed the 
lid.
  
  > The situation described in the test plan is just a quick way to show what 
can go wrong and if that is fixed.

REPOSITORY
  R104 KScreen

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

To: hoffmannrobert, #plasma, romangg
Cc: ngraham, romangg, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D21082: Fix lid open/closed configurations

[Robert Hoffmann]

hoffmannrobert added a comment.


  My goal is to make the screen configurations work reliably with notebooks, 
not to support a new use case. The use case is: work with a notebook docked and 
undocked, with and without external screen(s), lid open or closed.
  
  The configurations work fine, if no notebook with a lid that can be opened or 
closed is involved.
  
  But several users report, that they switch on their notebook (in docking 
station) in the morning, lid is closed, log in, and the external screens are 
swapped left and right again. Or only the empty background of the second screen 
is shown on the external monitor while the primary screen is on the closed 
integrated notebook screen.
  
  So they configure it again as needed, while the lid is closed. But there is 
already a lidOpened-configuration lurking around, waiting for the lid to be 
opened and to destroy the configuration previously made.
  
  Where does the wrong lidOpened-configuration come from? I guess it's created 
automatically when the lid is open and a button on the OSD ("Extend to right" 
or so) is clicked. After closing the lid, this configuration is copied to 
"lidOpened".
  
  The situation described in the test plan is just a quick way to show what can 
go wrong and if that is fixed.

REPOSITORY
  R104 KScreen

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

To: hoffmannrobert, #plasma, romangg
Cc: ngraham, romangg, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D21082: Fix lid open/closed configurations

[Roman Gilg]

romangg added a comment.


  So your goal is to support the following use case: User wants to set 
different screens properties (for example positions) on the same screen 
arrangement for when the lid is closed and when it is opened.
  
  Correct? Had the lidOpenend mechanic this goal from the beginning or is this 
a new feature? To my understanding the lidOpenend mechanic does the following 
at the moment:
  
  1. Disable the internal display on closing the lid and reenable it when the 
lid is opened again.
  2. Restore displays values active, which were active, before the lid was 
closed when the lid is opened again.
  
  Function 2. is questionable. What if I move my external screens around while 
the lid is closed? Why should this become different for when the lid is opened 
again? With this logic the same holds the other way around. Why should the 
display positions of external screens become different just when I close the 
lid? It would make more sense to keep these display positions consistent 
independently of lid closed/opened and just reinsert the internal screen at a 
sensible / the old position. Can you explain why, as described in your test 
plan, it is sensible to have different display positions of external monitors 
for when having the lid opened and closed?

REPOSITORY
  R104 KScreen

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

To: hoffmannrobert, broulik, #plasma
Cc: romangg, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D21082: Fix lid open/closed configurations

[Robert Hoffmann]

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

REVISION SUMMARY
  Screen configurations on notebooks couldn't distinguish
  properly between open lid and closed lid configurations. The
  open lid config was only created if the lid was closed and
  deleted if the lid was opened again. Known configurations
  were ignored.
  This is fixed by keeping open and closed lid configurations
  permanently in different config files.

TEST PLAN
  Use a notebook in a docking station with two external screens
  connected. Configure the screens differently for open and closed
  lid. Particularly for closed lid configure the screen connected to
  the higher output number to the left (e.g. DP-1-2), the lower to
  the right (e.g. DP-1-1). Open and close the lid.
  
  With this patch applied screen configurations remain as previously
  configured. Without, the higher output number is shown on the
  right and the lower on the left.

REPOSITORY
  R104 KScreen

BRANCH
  fix_lid_open_closed_configs

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

AFFECTED FILES
  kded/daemon.cpp
  kded/serializer.cpp

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