feat(kcm): add option for auto rotation
ClosedPublic

Authored by romangg on Dec 16 2019, 2:05 AM.

Details

Summary

This makes auto rotation configurable from the KScreen KCM. The auto rotation
feature is enabled when a rotation sensor is active and the output is internal,
otherwise the controls are disabled and we fall back to manual configuration.

An open question at this point is if there should be an additional option to
only rotate automatically when tablet mode is active or always. Currently it
always rotates automatically.

Test Plan

Tested on laptops with and without rotation sensor.

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 requested review of this revision.Dec 16 2019, 2:05 AM
romangg created this revision.
romangg edited the summary of this revision. (Show Details)
davidedmundson added inline comments.
kcm/config_handler.cpp
39

why here?

The point of your control config is that it's independent of the config.

romangg added inline comments.Dec 17 2019, 1:48 PM
kcm/config_handler.cpp
39

When the initial config is reset, the initial control should be reset as well at some point. Do you mean that question purely on a semantic level?

ngraham added inline comments.
kcm/package/contents/ui/Orientation.qml
48

There's enough horizontal space in the window to put this row next to the "Manual" label so it would be like so:

Orientation : (o) Automatic
              ( ) Manual: [0] [90] [180] [270]
romangg updated this revision to Diff 72353.Sun, Dec 29, 11:49 PM
  • Get auto rotation being supported
  • Add tablet mode checkbox
sasch added a subscriber: sasch.Mon, Dec 30, 11:04 AM

Does this need testing on hardware at this point or has that been done?

Does this need testing on hardware at this point or has that been done?

I tested it on my XPS 2-in-1. This laptop does have an orientation sensor. So this was tested. But it does not emit any signal when the display is snapped around (to hold it in "tablet-mode"). So I could test here only what happens when there is no signal but only the orientation sensor (in which case the checkbox for tablet mode only rotation must be unchecked to activate auto rotation --- i.e. an applet control would be nice for that).

So if you have a device which does have an orientation sensor and signals when tablet mode is activated (either by sending out the specific libinput event or deactivating the touchpad while having a touchscreen), this would be a very important test case. Thanks!

Note: For testing you need the patches in KWin applied and add in libkscreen in the KWayland backend the features flags for auto rotation and tablet mode detection.

sasch added a comment.Wed, Jan 8, 9:05 PM

Never compiled anything, but I have a Lenovo Yoga 900s convertible that deactivates the keyboard in tent mode or when the lid is closed (unfortunately not the touchpad until the platform x86 guys offer an kernel update, https://bugzilla.kernel.org/show_bug.cgi?id=110011 ).

Nevertheless, if anyone can provide a wayland ISO including the fixes, I'd be happy to test it and provide some feedback ;)

apol added a subscriber: apol.Thu, Jan 9, 1:16 AM

Never compiled anything, but I have a Lenovo Yoga 900s convertible that deactivates the keyboard in tent mode or when the lid is closed (unfortunately not the touchpad until the platform x86 guys offer an kernel update, https://bugzilla.kernel.org/show_bug.cgi?id=110011 ).

Nevertheless, if anyone can provide a wayland ISO including the fixes, I'd be happy to test it and provide some feedback ;)

I'd say as is it's too hard to set up a system to test these changes. I'd suggest reviewing these patches on code merits and do a good testing push before Plasma is released from Neon git unstable.

romangg updated this revision to Diff 73147.Thu, Jan 9, 6:33 PM

Rebase on master.

romangg updated this revision to Diff 73149.Thu, Jan 9, 6:57 PM

Rebase on series changes.

This revision was not accepted when it landed; it landed in state Needs Review.Wed, Jan 15, 2:38 PM
This revision was automatically updated to reflect the committed changes.
Restricted Application added a project: Plasma. · View Herald TranscriptWed, Jan 15, 2:38 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript