GNOME Bugzilla – Bug 732442
Calibration with a rotated Thinkpad Tablet screen results in offset
Last modified: 2014-07-01 16:39:35 UTC
I've been testing wacom screen calibration with a X201T and when rotating the screen 90 degrees (with the physical button on the screen) and then calibrating with gnome-control-center, the result after calibration is that the cursor is lower by a few milimeters from the stylus tip, even if I hit all the targets "bull's eye". Could it be that there's something going on with GNOME Shell's top panel in rotated mode?
Created attachment 279626 [details] [review] wacom: Fix x/y ranges' swapping when calibrating on rotated screens The previous axis swapping code would oddly rely on the input coordinates remaining on untransformed device coordinates, while those are gotten on screen coordinates (hence rotated), this would cause swapping not to kick in, and result in swapped X/Y scaling to be applied to X/Y ranges. So make the axis swapping code be calculated upon screen coordinates, as the collected points already are, and swap the applied scalings if necessary. Also, the code that swapped coordinates before returning to the caller would scramble min/max values when swapping x/y. Fix that too, minimum and maximum should stay like that when swapping axes.
Review of attachment 279626 [details] [review]: ::: panels/wacom/calibrator/calibrator.c @@ +27,3 @@ +#define SWAP(valtype,x,y) \ + G_STMT_START { \ Separate commit for this please. @@ +144,2 @@ /* Should x and y be swapped? */ + swap_xy = (c->geometry.width < c->geometry.height); Is this really sufficient to assert that swapping is needed? What about devices for which portrait is the default rotation? (not that there are very many Wacom tablets like that...)
(In reply to comment #2) > Review of attachment 279626 [details] [review]: > > ::: panels/wacom/calibrator/calibrator.c > @@ +27,3 @@ > > +#define SWAP(valtype,x,y) \ > + G_STMT_START { \ > > Separate commit for this please. Sure, doing that > > @@ +144,2 @@ > /* Should x and y be swapped? */ > + swap_xy = (c->geometry.width < c->geometry.height); > > Is this really sufficient to assert that swapping is needed? What about devices > for which portrait is the default rotation? (not that there are very many Wacom > tablets like that...) Good point, we can check whether the geometries of the tablet and output are wider towards the same axis. I guess if we're super picky, also rotation matters here, so we don't calculate one side/corner based on possibly opposite click points.
Created attachment 279696 [details] [review] calibrator: Use G_STMT_START/END on SWAP define And let it pass the type, so it can be used on other than integers.
Created attachment 279697 [details] [review] wacom: Fix x/y ranges' swapping when calibrating on rotated screens The previous axis swapping code would oddly rely on the input coordinates remaining on untransformed device coordinates, while those are gotten on screen coordinates (hence rotated), this would cause swapping not to kick in, and result in swapped X/Y scaling to be applied to X/Y ranges. So make the axis swapping code be calculated upon screen coordinates, as the collected points already are, and swap the applied scalings if necessary. Also, the code that swapped coordinates before returning to the caller would scramble min/max values when swapping x/y. Fix that too, minimum and maximum should stay like that when swapping axes.
Review of attachment 279696 [details] [review]: ++
Review of attachment 279697 [details] [review]: Looks good otherwise. ::: panels/wacom/calibrator/calibrator.c @@ +180,3 @@ if (swap_xy) { + SWAP (int, axis.x_min, axis.y_min); The only difference here seems to be the space before the parenthesis. No need to add it.
Attachment 279696 [details] pushed as 4a9bde0 - calibrator: Use G_STMT_START/END on SWAP define Attachment 279697 [details] pushed as b1ae5c7 - wacom: Fix x/y ranges' swapping when calibrating on rotated screens