GNOME Bugzilla – Bug 787836
patches to fix various rotation issues
Last modified: 2017-09-25 19:36:12 UTC
Created attachment 359977 [details] [review] [PATCH 1/4] monitor-config-manager: Fix 90/270 degree rotation not working I've been running some tests of gnome 3.26 on a tablet with an accelerometer. There are several issues causing accelerometer triggered rotation to not work well. Here are patches for most of the issues. See bug 787801 for one more issue, with which I need some help to fix it.
Created attachment 359978 [details] [review] [PATCH 2/4] monitor-config-manager-kms: Fix is_transform_handled
Created attachment 359979 [details] [review] [PATCH 3/4] meta-input-settings: Fix strv memleak in find_logical_monitor()
Created attachment 359981 [details] [review] [PATCH 4/4] meta-input-settings: Fix touchscreen coordinates not rotating with LCD panel
Ok, that is all of them, it would be good if we could get these into the 3.26.x branch.
Comment on attachment 359978 [details] [review] [PATCH 2/4] monitor-config-manager-kms: Fix is_transform_handled Looks right.
Comment on attachment 359979 [details] [review] [PATCH 3/4] meta-input-settings: Fix strv memleak in find_logical_monitor() Oops!
Comment on attachment 359981 [details] [review] [PATCH 4/4] meta-input-settings: Fix touchscreen coordinates not rotating with LCD panel Hmm, the policy to map touchscreens and other output-mappable devices is at: https://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/common/gsd-device-mapper.c#n272 And carried out by gsd-wacom (also for touchscreens, oddly). It would be nice to eventually fold this code into mutter, since g-s-d just updates the "display" setting for mutter to actually apply the mapping. However, IMO would be nicer to have this bit of policy right into g-s-d in the mean time (which AFAICT happens at https://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/common/gsd-device-mapper.c#n316, odd...)
Hi, (In reply to Carlos Garnacho from comment #7) > Comment on attachment 359981 [details] [review] [review] > [PATCH 4/4] meta-input-settings: Fix touchscreen coordinates not rotating > with LCD panel > > Hmm, the policy to map touchscreens and other output-mappable devices is at: > https://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/common/gsd- > device-mapper.c#n272 > And carried out by gsd-wacom (also for touchscreens, oddly). > > It would be nice to eventually fold this code into mutter, since g-s-d just > updates the "display" setting for mutter to actually apply the mapping. > However, IMO would be nicer to have this bit of policy right into g-s-d in > the mean time (which AFAICT happens at > https://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/common/gsd- > device-mapper.c#n316, odd...) So I guess that that means that the mutter patch is no good and I should instead figure out why g-s-d is not doing what it should do ? Also does g-s-d also run under gdm, because we need this under gdm too. Regards, Hans
(In reply to Hans de Goede from comment #8) > So I guess that that means that the mutter patch is no good and I should > instead figure out why g-s-d is not doing what it should do ? Yup, the fact that you're getting ['', '', ''] as edid seems to indicate GsdDeviceMapper/gsd-wacom is failing to do its job. > > Also does g-s-d also run under gdm, because we need this under gdm too. It does, for better or worse :).
Review of attachment 359977 [details] [review]: Looks good; just coding style issues. ::: src/backends/meta-monitor-config-manager.c @@ +770,3 @@ + logical_monitor_config->layout.width = logical_monitor_config->layout.height; + logical_monitor_config->layout.height = temp; + } Coding style is incorrect here: space between function name and (, "{" on its own line indented two spaces.
(In reply to Carlos Garnacho from comment #9) > (In reply to Hans de Goede from comment #8) > > So I guess that that means that the mutter patch is no good and I should > > instead figure out why g-s-d is not doing what it should do ? > > Yup, the fact that you're getting ['', '', ''] as edid seems to indicate > GsdDeviceMapper/gsd-wacom is failing to do its job. Ok, I've filed g-s-d bug 787884 for tracking this. I currently have a whole bunch of other stuff which I need to look into first. So feel free to beat me to debugging this. I will add a note to bug 787884 when I start looking into this to avoid us doing double work.
Pushed to master (incl. style fixes). Thanks Hans!
*** Bug 771901 has been marked as a duplicate of this bug. ***