After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778508 - x/y displacement of pen tip and stroke on HiDPI screen (Surface Book / MyPaint)
x/y displacement of pen tip and stroke on HiDPI screen (Surface Book / MyPaint)
Status: RESOLVED OBSOLETE
Product: gtk+
Classification: Platform
Component: Backend: Win32
unspecified
Other Windows
: Normal normal
: ---
Assigned To: gtk-win32 maintainers
gtk-bugs
: 778511 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-02-11 21:00 UTC by Windmaedchen
Modified: 2018-05-02 18:04 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Windmaedchen 2017-02-11 21:00:58 UTC
This one has been reported as part of https://bugzilla.gnome.org/show_bug.cgi?id=778328 , where the actual stroke appears about 5cm next to the pen tip.

Image: https://bug778328.bugzilla-attachments.gnome.org/attachment.cgi?id=345175 First attachement of bug mentioned above.

Carlos Garnacho mentioned:
> This is indeed a separate issue, I think down to 
> https://git.gnome.org//browse/gtk+/tree/gdk/win32/gdkdevicemanager-win32.c#n1014 
> not scaling down the x/y coordinates on hidpi setups.
Comment 1 Andrew Chadwick 2017-02-11 22:33:51 UTC
*** Bug 778511 has been marked as a duplicate of this bug. ***
Comment 2 Andrew Chadwick 2017-02-11 22:39:03 UTC
I kicked off a duplicate report (bug 778511) without having noticed this earlier one. Sorry about that. It has some steps to reproduce in it: see  no need for a HiDPI screen, just set GDK_SCALE to 2.0 or greater.

Further notes: if you're reproducing this, note that you may have to move the Event Axes window close to the top-left of the screen in order to even see any crosshairs or coloured blobs. Maximizing this little window helps too.

Also, this *might* be a duplicate of bug 776883, but that seems to be to do with multi-monitor setups. This bug can be readily replicated on a single-monitor system using either an inexpensive environment variable or a somewhat more expensive fancy HiDPI Surface tablet☺

Windmaedchen: does running GTK3-demo on your Surface with:

    $ GDK_SCALE=1.0 gtk3-demo --run=event_axes

Make the offset go back to normal (despite everything appearing tiny now)?
Comment 3 Windmaedchen 2017-02-12 09:06:38 UTC
(In reply to Andrew Chadwick from comment #2)
> Windmaedchen: does running GTK3-demo on your Surface with:
> 
>     $ GDK_SCALE=1.0 gtk3-demo --run=event_axes
> 
> Make the offset go back to normal (despite everything appearing tiny now)?

It does, indeed!
Comment 4 Carlos Garnacho 2017-02-12 17:36:56 UTC
Andrew, replying to https://bugzilla.gnome.org/show_bug.cgi?id=778328#c29 here:

hidpi and multimonitor/scale coordinates seem related, or at least the wrong coordinates come from the very same place. AFAICS, in:

https://git.gnome.org//browse/gtk+/tree/gdk/win32/gdkdevicemanager-win32.c#n1014

We do fill in x/y device axes, which after reading both bugs I can only think they come in full stage coordinates.

Shortly after, these x/y coordinates are used to fill in the event coords on motion/button events:

https://git.gnome.org//browse/gtk+/tree/gdk/win32/gdkdevicemanager-win32.c#n1116
https://git.gnome.org//browse/gtk+/tree/gdk/win32/gdkdevicemanager-win32.c#n1149

The _gdk_device_wintab_translate_axes() function internally relies on _gdk_device_translate_screen_coord() to get the x/y coordinates.

Now, this function (whose name always confuses me, this is gtk2-days' terminology for "translate coords to window-relative coordinates", whereas translate_window_coord() is "map entire device to this window") assumes scale=1 if min/max axis ranges are meaningless:

https://git.gnome.org//browse/gtk+/tree/gdk/gdkdevice.c?h=gtk-3-22#n1811

And this can basically only work if the x/y axes are already in pixel coordinates and no scaling applies. Actually all of GDK_MODE_SCREEN is ill defined if multiple monitor scales apply. Even on the paths where devices provide proper min/max axis values, as the calculations rely on pixels being uniform (screen width/height should already be downscaled though).

In the x11/wayland paths, this function is just avoided when translating x/y axes for events, the windowing event coordinates are used right away. And I think something similar should happen in win32. The window/screen GdkInputMode only matters for GDK_DEVICE_TYPE_FLOATING devices, and those just don't exist on win32 (or virtually nowhere, for that matter), GdkInputMode should probably go away altogether in gtk4.

However, I'm not sure if we can just use _gdk_device_query_state() on the wintab device to obtain the current event coordinates, AFAICT relies on GetCursorPos() and would  drop subpixel precision. Is there maybe a better way to obtain window-relative x/y coordinates from the wintab event?
Comment 5 LRN 2017-02-13 04:55:04 UTC
I think you've lost me at the "this can basically only work if the x/y axes are already in pixel coordinates". The way i read the source, all computations are done in gdouble and x/y axis are in some undefined gdouble units.
Comment 6 Carlos Garnacho 2017-02-13 10:28:18 UTC
yes, I'm taking into account the axis_width <= 0 paths, which happens basically if the device doesn't provide a proper axis range. There scale is set to 1 (meaning "don't scale"), and the following calculation happens.

axis_value = offset + scale * (value - axis_info.min_value);

If the window as its top-left corner in 100x100 and the tool is at 10x10 in there, that calculation would boil down to:

axis_value = -100 + 1 * (X - 0) = -100 + X

so if we want that to return the proper window-relative coordinates as the win32 backend expects of it, X can only be 110, that is, the screen-relative position of the tool, in pixels.

I honestly don't know if this path is taken, but the other path should have some scaling in mind for the one hidpi monitor case, as gdk_screen_get_width/height() return downscaled sizes.
Comment 7 Carlos Garnacho 2017-02-13 15:45:36 UTC
fwiw, if my hunch is right, it would probably suffice to make

  scale = 1 / gdk_monitor_get_scale_factor (window_monitor);

instead of "scale = 1" in _gdk_device_translate_screen_coord()
to fix the single-output case. 

However this still wouldn't fix this function being fundamentally broken for the multi-monitor, multi-scale case in bug #776883 mentioned by Andrew. Hence my suggestion to look for a more direct way to get x/y window-relative coordinates from the wintab event, or from the cursor which is currently being driven by the tablet tool.
Comment 8 LRN 2017-02-13 16:36:53 UTC
There are different contexts (WTI_DEFCONTEXT, WTI_DEFSYSCTX, WTI_DDCTXS and WTI_DSCTXS), which can be queried for things like CTX_INORGX, CTX_INEXTX, CTX_OUTORGX and CTX_OUTEXTX (and the Y and Z variants of the same). I am not sure what are these things. The documentation says things like "the origin of the context's output coordinate space in context output coordinates", which is not very enlightening. I would suggest hacking the GDK code to dump these values and see if they are meaningful in any way. Although, they seem to correspond to lcInOrgX and such in the debug output, so maybe we don't really need to alter the source code to see them.

Other than that, i don't think that wintab interacts with the windowing system much. At least, not according to the documentation.
Comment 9 LRN 2017-02-13 17:19:00 UTC
Mined the logs from bug 778328:

Windmaedchen:

Default context:
lcInOrg   {    0,     0,     0 }
lcInExt   { 9600,  7200,     0 }
lcOutOrg  {    0,     0,     0 }
lcOutExt  { 3000,  2000,     0 }
lcSysMode = 0 /* absolute */
lcSysOrg  {    0,     0 }
lcSysExt  { 3000,  2000 }

context for device 0 (where different from the default context):
lcOutExt  { 9600, -9600,    0 }

context for device 0 after WTOpen (where different from the default context):
lcOutExt  { 9600, -9600,    0 }

achadwick:

Default context:
lcInOrg   {    0,     0, -1023 }
lcInExt   {44704, 27940,  2047 }
lcOutOrg  {    0,     0, -1023 }
lcOutExt  { 1920,  1080,  2047 }
lcSysMode = 0 /* absolute */
lcSysOrg  {    0,     0 }
lcSysExt  { 1920,  1080 }

context for device 0 (where different from the default context):
lcOutExt  {44704,-27940, 2047 }

context for device 0 after WTOpen (where different from the default context):
lcOutExt  {44704,-27940, 2047 }

Does that mean anything to anyone?
Comment 10 LRN 2017-02-14 10:49:17 UTC
Anyway, i've read the docs a bit. I've also read the source code, and googled things up. It seems that wintab automatically takes the coordinates in the input area (presumably, it can be set to allow the table to have "margins" where input doesn't work or where input skirts the edge (optional "out of bounds" feature)) and maps them into the coordinates of the output area. Output area is specified by the application. GDK, for example, sets output area to be equal to the axis size (with a twist that it gives negative output extents in the Y direction, because tablet origin 0:0 is the bottom-left corner, not the top-left corner).

Another note: GDK W32 code uses axis_x/y.axMin/Max (which it gets from wintab) to initialize output area AND gives them to _gdk_device_add_axis(), which results in the generic GDK code later getting these values as min/max_value of each axis. My hypothesis is that settings output area to 0 (which is what happens when "axMax - axMin" is zero) will make the device unusable, as the cursor coordinates will always be zero no matter what. In that light the check for "max_value - min_value != 0" is meaningless - this should never happen with wintab (if it does, fallback scale = 1 is the *least* of our problems).

Anyway, point is, possible values in WT_PACKET lay in the area defined by lcOutOrg and lcOutExt. Given the way GDK initializes these fields, these values are, in our case, unmodified wintab input area coordinates, except that they are flipped in Y direction. They have no relation to the windowing system.
Comment 11 Carlos Garnacho 2017-02-14 11:41:02 UTC
(In reply to LRN from comment #10)
> Anyway, i've read the docs a bit. I've also read the source code, and
> googled things up. It seems that wintab automatically takes the coordinates
> in the input area (presumably, it can be set to allow the table to have
> "margins" where input doesn't work or where input skirts the edge (optional

Makes sense. This is probably the "letterbox" feature we also have in gnome-control-center, where input/output ratios are made to match by reducing the input sensitive area on one axis.

> "out of bounds" feature)) and maps them into the coordinates of the output
> area. Output area is specified by the application. GDK, for example, sets
> output area to be equal to the axis size (with a twist that it gives
> negative output extents in the Y direction, because tablet origin 0:0 is the
> bottom-left corner, not the top-left corner).
> 
> Another note: GDK W32 code uses axis_x/y.axMin/Max (which it gets from
> wintab) to initialize output area AND gives them to _gdk_device_add_axis(),
> which results in the generic GDK code later getting these values as
> min/max_value of each axis. My hypothesis is that settings output area to 0
> (which is what happens when "axMax - axMin" is zero) will make the device
> unusable, as the cursor coordinates will always be zero no matter what. In
> that light the check for "max_value - min_value != 0" is meaningless - this
> should never happen with wintab (if it does, fallback scale = 1 is the
> *least* of our problems).

Hmm, however it's usable, at least with window_scale=1. Knowing very little about wintab, it does seem troublesome that we could pass 0 as the output area, and I have no idea how would the driver deal with that. Where is this code passing the output size?

And I've actually got one uncertainty here: It does seem like Andrew is able to reproduce by forcing GDK_SCALE=2, but I would expect external (wacom) tablets to provide a proper axis range, and thus not fall in these paths. However afaict the other code path should be "fine".

> 
> Anyway, point is, possible values in WT_PACKET lay in the area defined by
> lcOutOrg and lcOutExt. Given the way GDK initializes these fields, these
> values are, in our case, unmodified wintab input area coordinates, except
> that they are flipped in Y direction. They have no relation to the windowing
> system.

And that doesn't sound good I'd say for this usecase :(. Taking into account that the windowing/driver can apply whatever mapping, we can't infer the actual pointer position.

Quickly reading the wintab api, it does seem like we want SysOrg/SysExt involved in the calculations, those seem to be the "origin/extent in screen coordinates of the screen mapping area for system cursor tracking". It looks like that can give us a way to map device coordinates to the pointer position.
Comment 12 LRN 2017-02-14 12:13:40 UTC
(In reply to Carlos Garnacho from comment #11)
> (In reply to LRN from comment #10)
> > "out of bounds" feature)) and maps them into the coordinates of the output
> > area. Output area is specified by the application. GDK, for example, sets
> > output area to be equal to the axis size (with a twist that it gives
> > negative output extents in the Y direction, because tablet origin 0:0 is the
> > bottom-left corner, not the top-left corner).
> > 
> > Another note: GDK W32 code uses axis_x/y.axMin/Max (which it gets from
> > wintab) to initialize output area AND gives them to _gdk_device_add_axis(),
> > which results in the generic GDK code later getting these values as
> > min/max_value of each axis. My hypothesis is that settings output area to 0
> > (which is what happens when "axMax - axMin" is zero) will make the device
> > unusable, as the cursor coordinates will always be zero no matter what. In
> > that light the check for "max_value - min_value != 0" is meaningless - this
> > should never happen with wintab (if it does, fallback scale = 1 is the
> > *least* of our problems).
> 
> Hmm, however it's usable, at least with window_scale=1. Knowing very little
> about wintab, it does seem troublesome that we could pass 0 as the output
> area, and I have no idea how would the driver deal with that. Where is this
> code passing the output size?

https://git.gnome.org//browse/gtk+/tree/gdk/win32/gdkdevicemanager-win32.c#n492

> 
> And I've actually got one uncertainty here: It does seem like Andrew is able
> to reproduce by forcing GDK_SCALE=2, but I would expect external (wacom)
> tablets to provide a proper axis range, and thus not fall in these paths.
> However afaict the other code path should be "fine".

Well, as you've said before, GDK does post-scaling operation on the raw tablet coordinates instead of passing them unmodified in the event. Depending on which values (screen resolution - real or scaled for HiDPI) go into scaling, you get different results.

> 
> > 
> > Anyway, point is, possible values in WT_PACKET lay in the area defined by
> > lcOutOrg and lcOutExt. Given the way GDK initializes these fields, these
> > values are, in our case, unmodified wintab input area coordinates, except
> > that they are flipped in Y direction. They have no relation to the windowing
> > system.
> 
> And that doesn't sound good I'd say for this usecase :(. Taking into account
> that the windowing/driver can apply whatever mapping, we can't infer the
> actual pointer position.
> 
> Quickly reading the wintab api, it does seem like we want SysOrg/SysExt
> involved in the calculations, those seem to be the "origin/extent in screen
> coordinates of the screen mapping area for system cursor tracking". It looks
> like that can give us a way to map device coordinates to the pointer
> position.

lcSysOrg/Ext are for the system cursor tracking feature (which GDK does try to use, and the spec says that this feature must be available). Basically, wintab moves the mouse cursor for us automatically (if we drop CXO_SYSTEM flag, this feature is disabled, and we'll be able to move the mouse by ourselves). lcSysOrg/Ext control the mapping the same way as lcOutOrg/Ext do, only lcOut results in mapped coordinates being sent in WT_PACKETs, while lcSys results in mouse cursor moving around. Default system context (which is what GDK queries - either that or per-device default system context) initializes lcSysExt with current display resolution and sets lcSysOrg to 0. For some reason it doesn't flip the Y axis. I guess it's done automatically (or, rather, it uses the Y direction that the OS used; i'm not sure, maybe Windows can change the coordinate system, i don't remember).
Comment 13 Andrew Chadwick 2017-02-17 16:41:48 UTC
Possible cause of this bug, as mentioned on IRC: bug 778835. I'll try to crank a build out this evening so we can test that patch against this bug.
Comment 14 Andrew Chadwick 2017-02-17 23:48:13 UTC
I've uploaded some time-limited debugging builds of the gtk-3-22 branch at d825c34ca3 plus attachment 346081 [details] [review] from bug 778832 for testers who are using current MSYS2. Please can everyone affected by this bug test with them?

    https://filebin.net/8wf4l3dl4nbdblu9

To install these .pkg.tar.xz files, download them and type:

    $ pacman -U /path/to/mingw-w64-*-gtk3-git-3.22.8.14.gd825c34ca3_patch346081-1-any.pkg.tar.xz

To revert to a stock MSYS2 GTK3 installation (to allow future upgrades to work):

    $ pacman -Syyuu   # upgrade the rest of the system as needed
    $ pacman -S mingw-w64-{x86_64,i686}-gtk3

Git packages are marked as conflicting with non-git ones, so you will be asked several times during installation whether you want to proceed. Enter "y" each time.
Comment 15 Andrew Chadwick 2017-02-17 23:53:15 UTC
I don't know if this is the fix that the developers want, but for me

    $ GDK_SCALE=2 gtk3-demo --run=event_axes

scales wintab input correctly with the build in comment 14. Stock gtk3 3.22.7-1 from MSYS2 is still broken. In other words, either the patch fixes this bug, or something else on the gtk-3-22 branch fixes this bug between 3.22.7 and d825c34ca3.

I do not have a fancy 4k monitor to test with, so I don't know if this fixes the problem for those: I need to force it with GDK_SCALE.
Comment 16 Andrew Chadwick 2017-02-17 23:55:50 UTC
(In reply to Andrew Chadwick from comment #14)
> [...] from bug 778832 [...]

Oh for an edit button. That should read bug 776635. Sorry.
Comment 17 Windmaedchen 2017-02-18 19:54:21 UTC
So I have tested Andrews patch of comment #14 on my Surface Book via MSYS2 with stable GTK3, and I am happy to say: it works! The position has no offset anymore. 

Would you like me to attach anything? Screenshot? Log?
Can I actually try and combine this solution with the other from Bug 778328?
Comment 18 Andrew Chadwick 2017-02-18 23:11:37 UTC
I only did a build. LRN is responsible for the patch itself ☺

You'll have to wait for the patches from bug 778835 and bug 778328 to be merged into GTK. I think they are both in a good state, and I have just reviewed both as "accepted, commit now" with references to your testing and some of mine in the hope of speeding things along. As I understand it:

* bug 778835 (wrong screen size returned when in HiDPI mode) is the root cause of:
  - bug 778508 (this bug) (wrong x & y with all wintab devices in HiDPI mode)
  - bug 778834 (menus can appear partly offscreen in HiDPI mode)
* bug 778328 (loss of pressure after proximity-out) is a separate bug.

I'm only making the builds available as a courtesy for testing right now.
Comment 19 GNOME Infrastructure Team 2018-05-02 18:04:27 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gtk/issues/753.