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 774265 - No tilt for wintab devices
No tilt for wintab devices
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Win32
3.22.x
Other Windows
: Normal normal
: ---
Assigned To: gtk-win32 maintainers
gtk-bugs
Depends on: 774379 774699
Blocks:
 
 
Reported: 2016-11-11 12:42 UTC by Andrew Chadwick
Modified: 2016-11-22 11:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
get-tilt-working.patch (636 bytes, patch)
2016-11-12 22:50 UTC, Andrew Chadwick
none Details | Review
0001-wintab-fix-absence-of-tilt.patch (1.37 KB, patch)
2016-11-12 23:23 UTC, Andrew Chadwick
none Details | Review
0001-win32-Fix-tilt-from-Wintab-devices.patch (1.60 KB, patch)
2016-11-13 18:22 UTC, Andrew Chadwick
none Details | Review
0001-win32-Fix-tilt-from-Wintab-devices.patch (3.00 KB, patch)
2016-11-19 19:12 UTC, Andrew Chadwick
none Details | Review
0001-win32-Fix-tilt-from-Wintab-devices.patch (3.00 KB, patch)
2016-11-22 10:19 UTC, Andrew Chadwick
committed Details | Review
0001-wintab-tilt-Check-return-location-for-validity.patch (1.14 KB, patch)
2016-11-22 10:31 UTC, Andrew Chadwick
none Details | Review
0001-wintab-tilt-Check-return-location-for-validity.patch (1.14 KB, patch)
2016-11-22 10:40 UTC, Andrew Chadwick
committed Details | Review

Description Andrew Chadwick 2016-11-11 12:42:42 UTC
We are getting reports that tilt is missing in Windows, as seen through the lens of recent MyPaint builds:

https://community.mypaint.org/t/tilt-does-not-work/515

I've also seen the same thing myself in MSYS2 with ~3.18 and ~3.20 builds and gtk3-demo's event axis tester, but it has been happening for as long as I've been paying attention to Windows stuff. I'm using a Wacom Intuos 5 M, which sends tilt just fine in X11 GNOME 3.22 and xinput-xf86-wacom ☺

I do not know yet if this is a regression since the the GTK2 era.


There's a test and a comment in the code which seem to be at odds with each other:

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

Also note that the block controlled by the conditional is the only place where _gdk_device_add_axis ever does stuff for GDK_AXIS_YTILT and GDK_AXIS_XTILT.

The test for the special ArtPad II resolution case seems to be incorrect to my eyes, and I think it might be causing axis setup to be skipped for all tablets that report nonzero orientation info.

I have the hardware and the knowledge needed to make debugging builds from git, so I'll try to investigate more later on today.
Comment 1 Andrew Chadwick 2016-11-12 22:50:22 UTC
Created attachment 339725 [details] [review]
get-tilt-working.patch

The quick fix is a oneliner, like I thought. Although now it seems that packets are being converted to GdkEvents with tilts that are 90 degrees out of phase. But that's clearly a separate bug.

https://twitter.com/MyPaintApp/status/797566033020289025
Comment 2 Ignacio Casal Quinteiro (nacho) 2016-11-12 23:02:19 UTC
I'll let Carlos to decide whether or not to get it in but in the meantime could you attach a git format path so you get the credit for it?
Comment 3 Andrew Chadwick 2016-11-12 23:23:14 UTC
Created attachment 339728 [details] [review]
0001-wintab-fix-absence-of-tilt.patch

Sure thing, attached.

GTK+ is failing to build from git right now on MSYS2 (see below), so this new patch is strictly speaking untested.

-------------------8<---------------------
==> Starting prepare()...
autoreconf: Entering directory `.'
autoreconf: running: autopoint --force
autoreconf: running: aclocal --force -I m4 ${ACLOCAL_FLAGS}
autoreconf: configure.ac: tracing
autoreconf: running: libtoolize --copy --force
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
libtoolize: copying file 'build-aux/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'.
libtoolize: copying file 'm4/libtool.m4'
libtoolize: copying file 'm4/ltoptions.m4'
libtoolize: copying file 'm4/ltsugar.m4'
libtoolize: copying file 'm4/ltversion.m4'
libtoolize: copying file 'm4/lt~obsolete.m4'
autoreconf: running: /usr/bin/autoconf --force
configure.ac:184: error: possibly undefined macro: dnl
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
autoreconf: /usr/bin/autoconf failed with exit status: 1
==> ERROR: A failure occurred in prepare().
    Aborting...

------------------>8----------------------
Comment 4 Andrew Chadwick 2016-11-13 18:22:07 UTC
Created attachment 339762 [details] [review]
0001-win32-Fix-tilt-from-Wintab-devices.patch

A better fix, this time correcting the angle bug too.

This updated patch is against branch gtk-3-22, and is tested in gtk3-demo. I can confirm that it fixes the missing and incorrect tilt on my Wacom Intuos 5 touch M (model PTH-650) for with drivers version 6.3.18-5 (latest).

I tried to confirm that this patch does not break a different tablet which doesn't support tilt, but GTK is broken for that in a different way.
gtk-3-22 HEAD's gtk3-demo does not start when my HUION H610PRO tablet is plugged in at startup with drivers V12.2.16. Since this misbehaviour is also present in unpatched GTK 3.22.1 from the msys2 repositories, I guess this patch can't be at fault! I'll investigate further, and send a separate bug.
Comment 5 Andrew Chadwick 2016-11-14 11:50:36 UTC
> [...] a separate bug.

bug 774379 for the Huion crash.
Comment 6 Ignacio Casal Quinteiro (nacho) 2016-11-16 16:37:14 UTC
Review of attachment 339762 [details] [review]:

Let's get this in then if you got the ok from Carlos
Comment 7 Andrew Chadwick 2016-11-19 01:09:31 UTC
It's a shame this got missed for the 3.22.4 release. Please could it be merged onto master and gtk-3-22 ASAP?
Comment 8 Carlos Garnacho 2016-11-19 16:11:00 UTC
Review of attachment 339762 [details] [review]:

Looks good to me, thus preserving a-c_n.

::: gdk/win32/gdkdevicemanager-win32.c
@@ +631,3 @@
            * orientation axis's azimuth resolution is zero.
            */
+          if ((device->pktdata & PK_ORIENTATION) && axis_or[0].axResolution != 0)

Indeed seems like a typo, according to the comment above.

A side question, could altitude be used to fill in the info around GDK_AXIS_DISTANCE?

@@ +824,3 @@
   az = TWOPI * packet->pkOrientation.orAzimuth /
     (axes[0].axResolution / 65536.);
+  az -= G_PI / 2;

Wow, so 0-angle is not perpendicular to the tablet? Odd :), I anyway trust your testing.
Comment 9 Andrew Chadwick 2016-11-19 16:23:50 UTC
Integrating this with the patch for bug 774699 revealed some oddities on the Huion: *HUGE* positive(?) tilts, down and to the right, outside the bounds of what gtk3-demo can show. Can you shed any light on that?
Comment 10 Andrew Chadwick 2016-11-19 16:27:39 UTC
According to the Wintab docs[1], orAzimuth:

> Specifies the clockwise rotation of the cursor about the z axis through a
> full circular range.

You may be thinking of orAltitude, which is indeed zero when the pen is perpendicular to the surface.


[1] http://www.wacomeng.com/windows/docs/Wintab_v140.htm#_Toc275759894
Comment 11 Andrew Chadwick 2016-11-19 17:41:23 UTC
Some sample weird values from tilt decoding, clearly the source of the bad tilt I'm seeing. I guess we should be looking at all the resulutions for both azimuth and altitude, since a reasonable GdkEvent tilt requires both.

Line numbers may be meaningless below, this is by now heavily patched code. Also I have no idea why the code isn't just bombing out when it does that zero division.

(gdb) r
Starting program: C:\msys64\mingw64\bin\gtk3-demo.exe "--gdk-debug=input" "--run=event_axes"
[New Thread 4532.0xdfc]
[New Thread 4532.0x1318]
[New Thread 4532.0xde8]
[New Thread 4532.0xb9c]
[New Thread 4532.0xfcc]
[New Thread 4532.0xe00]
warning:        WTContextManager() this:fa39e310
warning:        WTRoundArray() this:fa39eb90
Wintab interface version 1.1
NDEVICES: 1, NCURSORS: 1
Device 0: HUION Tablet
Note: Driver did not provide device specific default context info despite claiming to support version 1.1
Default context:
lcName = HUION Tablet Context
lcOptions = CXO_SYSTEM
lcStatus =
lcLocks = CXL_INSIZE CXL_INASPECT CXL_SENSITIVITY CXL_MARGIN
lcMsgBase = 0x7ff0, lcDevice = 0, lcPktRate = 450
lcPktData = PK_TIME PK_CURSOR PK_BUTTONS PK_X PK_Y PK_NORMAL_PRESSURE PK_TANGENT_PRESSURE PK_ORIENTATION
lcPktMode =
lcMoveMask = PK_TIME PK_CURSOR PK_BUTTONS PK_X PK_Y PK_NORMAL_PRESSURE PK_TANGENT_PRESSURE PK_ORIENTATION
lcBtnDnMask = 0xffffffff, lcBtnUpMask = 0xffffffff
lcInOrgX = 0, lcInOrgY = 0, lcInOrgZ = 0
lcInExtX = 40000, lcInExtY = 25000, lcInExtZ = 0
lcOutOrgX = 0, lcOutOrgY = 0, lcOutOrgZ = 0
lcOutExtX = 1920, lcOutExtY = 1080, lcOutExtZ = 0
lcSensX = 1, lcSensY = 1, lcSensZ = 1
lcSysMode = 0
lcSysOrgX = 0, lcSysOrgY = 0
lcSysExtX = 1920, lcSysExtY = 1080
lcSysSensX = 1, lcSysSensY = 1
context for device 0:
lcName = HUION Tablet Context
lcOptions = CXO_SYSTEM CXO_MESSAGES CXO_CSRMESSAGES
lcStatus =
lcLocks = CXL_INSIZE CXL_INASPECT CXL_SENSITIVITY CXL_MARGIN
lcMsgBase = 0x7ff0, lcDevice = 0, lcPktRate = 0
lcPktData = PK_CONTEXT PK_CURSOR PK_BUTTONS PK_X PK_Y PK_NORMAL_PRESSURE PK_ORIENTATION
lcPktMode =
lcMoveMask = PK_CONTEXT PK_CURSOR PK_BUTTONS PK_X PK_Y PK_NORMAL_PRESSURE PK_ORIENTATION
lcBtnDnMask = 0xffffffff, lcBtnUpMask = 0xffffffff
lcInOrgX = 0, lcInOrgY = 0, lcInOrgZ = 0
lcInExtX = 40000, lcInExtY = 25000, lcInExtZ = 0
lcOutOrgX = 0, lcOutOrgY = 0, lcOutOrgZ = 0
lcOutExtX = 40001, lcOutExtY = -25001, lcOutExtZ = 0
lcSensX = 1, lcSensY = 1, lcSensZ = 1
lcSysMode = 0
lcSysOrgX = 0, lcSysOrgY = 0
lcSysExtX = 1920, lcSysExtY = 1080
lcSysSensX = 1, lcSysSensY = 1
opened Wintab device 0 000000000000000f
context for device 0 after WTOpen:
lcName = HUION Tablet Context
lcOptions = CXO_SYSTEM CXO_MESSAGES CXO_CSRMESSAGES
lcStatus =
lcLocks = CXL_INSIZE CXL_INASPECT CXL_SENSITIVITY CXL_MARGIN
lcMsgBase = 0x7ff0, lcDevice = 0, lcPktRate = 0
lcPktData = PK_CONTEXT PK_CURSOR PK_BUTTONS PK_X PK_Y PK_NORMAL_PRESSURE PK_ORIENTATION
lcPktMode =
lcMoveMask = PK_CONTEXT PK_CURSOR PK_BUTTONS PK_X PK_Y PK_NORMAL_PRESSURE PK_ORIENTATION
lcBtnDnMask = 0xffffffff, lcBtnUpMask = 0xffffffff
lcInOrgX = 0, lcInOrgY = 0, lcInOrgZ = 0
lcInExtX = 40000, lcInExtY = 25000, lcInExtZ = 0
lcOutOrgX = 0, lcOutOrgY = 0, lcOutOrgZ = 0
lcOutExtX = 40001, lcOutExtY = -25001, lcOutExtZ = 0
lcSensX = 1, lcSensY = 1, lcSensZ = 1
lcSysMode = 0
lcSysOrgX = 0, lcSysOrgY = 0
lcSysExtX = 1920, lcSysExtY = 1080
lcSysSensX = 1, lcSysSensY = 1
Attempting to increase queue size
Queue size set to 128
Cursor 0:
NAME: Huion
ACTIVE: YES
PKTDATA: 0x1de4: TIME BUTTONS X Y NORMAL_PRESSURE TANGENT_PRESSURE ORIENTATION
BUTTONS: 1
BUTTONBITS: 0
BTNNAMES:
BUTTONMAP: 0
SYSBTNMAP: 1
NPBUTTON: 0
NPBTNMARKS: 0 1
NPRESPONSE:
TPBUTTON: 0
TPBTNMARKS: 0 0
TPRESPONSE:
PHYSID: 0x10
CAPABILITIES: 0x1: MULTIMODE
MODE: 0
device: (0) HUION Tablet Huion axes: 5
Cursor 1:
NAME: Huion
ACTIVE: YES
PKTDATA: 0x1de4: TIME BUTTONS X Y NORMAL_PRESSURE TANGENT_PRESSURE ORIENTATION
BUTTONS: 1
BUTTONBITS: 0
BTNNAMES:
BUTTONMAP: 0
SYSBTNMAP: 1
NPBUTTON: 0
NPBTNMARKS: 0 1
NPRESPONSE:
TPBUTTON: 0
TPBTNMARKS: 0 0
TPRESPONSE:
PHYSID: 0x10
CAPABILITIES: 0x1: MULTIMODE
MODE: 0
device: (1) HUION Tablet Huion axes: 10
[New Thread 4532.0x1250]
[New Thread 4532.0x1178]
_gdk_input_set_tablet_active: Bringing Wintab contexts to the top of the overlap order
_gdk_input_set_tablet_active: Bringing Wintab contexts to the top of the overlap order
gdk_input_other_event: window=00000000000b0334 +220+154
gdk_input_other_event: window=00000000000b0334 +102+145
gdk_input_other_event: window=00000000000b0334 +102+145
gdk_input_other_event: window=00000000000b0334 +102+145
gdk_input_other_event: window=00000000000b0334 +102+145

Thread 1 hit Breakpoint 1, decode_tilt (axis_data=0x8128ec,
    axes=axes@entry=0x7e41a0, packet=packet@entry=0x22f450)
    at gdkdevicemanager-win32.c:817
817     {
(gdb) n
824       az = TWOPI * packet->pkOrientation.orAzimuth /
(gdb) n
825         (axes[0].axResolution / 65536.);
(gdb) n
824       az = TWOPI * packet->pkOrientation.orAzimuth /
(gdb) n
826       az -= G_PI / 2;
(gdb) l
821        * Tested with a Wacom Intuos 5 touch M (PTH-650) + Wacom drivers 6.3.18-5.
822        * Wintab's reference angle leads gdk's by 90 degrees.
823        */
824       az = TWOPI * packet->pkOrientation.orAzimuth /
825         (axes[0].axResolution / 65536.);
826       az -= G_PI / 2;
827       el = TWOPI * packet->pkOrientation.orAltitude /
828         (axes[1].axResolution / 65536.);
829
830       /* X tilt */
(gdb) n
827       el = TWOPI * packet->pkOrientation.orAltitude /
(gdb) n
828         (axes[1].axResolution / 65536.);
(gdb) p el
$1 = <optimized out>
(gdb) p axes[0]
$2 = {axMin = 0, axMax = 3600, axUnits = 3, axResolution = 235929600}
(gdb) p axes[1]
$3 = {axMin = -900, axMax = 900, axUnits = 3, axResolution = 0}
(gdb) p packet->pkOrientation
$4 = {orAzimuth = 900, orAltitude = 1000, orTwist = 0}
(gdb) p axes[2]
$5 = {axMin = 85794000, axMax = 0, axUnits = 1, axResolution = 0}
(gdb) q
Comment 12 Andrew Chadwick 2016-11-19 19:12:46 UTC
Created attachment 340323 [details] [review]
0001-win32-Fix-tilt-from-Wintab-devices.patch

An approach I want to test when I can. This isn't even tested yet.

Carlos agrees that trusting the hardware+driver to *declare* tilt axes is OK on IRC.

> <garnacho__> achadwick: it's pretty much up to the hw to advertise its
> capabilities, even on x11/libinput worlds.

On the other hand, we very obviously need a trust-but-verify approach for decoding the data. Putting that stuff near the point of use seems wisest to me.

I think getting the packet-handling code back into sync with the init and axis detection code is wise. Otherwise something's going to end up reading+writing off the end of an array somewhere.
Comment 13 Andrew Chadwick 2016-11-20 17:09:26 UTC
[[This comment is duplicated into the 3 active Wintab bugs, sorry for the spam]]

Okay, big testing rollup. Incorporating the following patches was *VERY* positive, getting rid of all inti-time crashes and almost all missing pressure. Tilt and eraser are back online and looking good for more advanced tablets.

* bug 774699 comment 1
  "0001-wintab-fix-skipping-of-odd-numbered-devices.patch"
* bug 774379 comment 29
  "0001-wintab-init-only-after-the-display-is-assigned.patch"
* bug 774379 comment 20
  "0001-Fix-segfault-during-init-for-some-wintab32-DLLs.patch"
* Sorry LRN, your patch from bug 774379 comment 26 didn't apply cleanly
  onto the stable branch (gtk-3-22) alongside all my patches. I was not
  able to test it.
* bug 774265 comment 12
  "0001-win32-Fix-tilt-from-Wintab-devices.patch"

I made a debug build and installation on MSYS2 of all the software needed, for both i686 and x86_64, and tested them all from their appropriate MSYS2 shell.

Arch    Tablet model                      Drivers   gtk3-demo  mypaint-git
----    -------------                     -------   ---------  ------------
x86_64  Huion H610PRO                     12.2.16   OK         OK
i686    Huion H610PRO                     12.2.16   OK         OK
x86_64  Wacom Intuos 5 M (PTH-650)        6.3.18-5  OK +T +E   OK +T +E
i686    Wacom Intuos 5 M (PTH-650)        6.3.18-5  OK +T +E   OK +T +E
x86_64  Genius EasyPen i405X (GT-100007)  2.5.1.1*  OK         OK
i686    Genius EasyPen i405X (GT-100007)  2.5.1.1*  NOEVENTS   NOEVENTS

My system is AMD64, so it is possible that the Genius driver installers made a decision about what to install for me. It is possible we've seen this behaviour before in MyPaint-land, so the lone failure above isn't a worry for me right now.

Key
---
NOEVENTS  Device presents only as "System Aggregated Pointer", and does
          not send events with pressure. Evidence of wintab32.dll
          being loaded by wintab_init_check, though!
OK  Device presents as something other than "System Aggregated Pointer",
    and sends events with pressure.
+T  Device also correctly presents tilt.
+E  Device also presents the eraser end of the stylus as a named device.
*  Drivers are branded "ioTablet", manufacturer "KYE",
   but were downloaded from geniusnet.com as recommended.
Comment 14 Andrew Chadwick 2016-11-20 17:23:20 UTC
This one would be nice to have in the next release. There *may* be a hint of a crasher if the axis tests done at message processing time are looser than the axis tests done at wintab init time, so I'm thinking this one should be merged for safety as well as correctness.

Master and stable, please ☺
Comment 15 Carlos Garnacho 2016-11-21 18:49:59 UTC
Comment on attachment 340323 [details] [review]
0001-win32-Fix-tilt-from-Wintab-devices.patch

LGTM, the approach makes sense too.
Comment 16 Ignacio Casal Quinteiro (nacho) 2016-11-22 09:52:50 UTC
Review of attachment 340323 [details] [review]:

See the style issue

::: gdk/win32/gdkdevicemanager-win32.c
@@ +824,3 @@
+   */
+  if ((axes == NULL) || (axis_data == NULL)
+      || (axes[0].axResolution == 0)

wrong style here, || should be on the previous line

@@ +825,3 @@
+  if ((axes == NULL) || (axis_data == NULL)
+      || (axes[0].axResolution == 0)
+      || (axes[1].axResolution == 0))

same here, move || to the previous line, i.e:

if (pipo ||
    pepe)
  {
  }
Comment 17 Andrew Chadwick 2016-11-22 10:19:32 UTC
Created attachment 340504 [details] [review]
0001-win32-Fix-tilt-from-Wintab-devices.patch

Okay, let me know if this is OK.
Comment 18 Ignacio Casal Quinteiro (nacho) 2016-11-22 10:21:47 UTC
Thanks
Comment 19 Andrew Chadwick 2016-11-22 10:31:33 UTC
Created attachment 340505 [details] [review]
0001-wintab-tilt-Check-return-location-for-validity.patch

Whoops, this should clearly have gone in the previous patch. Thanks for nacho on IRC for spotting the error.
Comment 20 Andrew Chadwick 2016-11-22 10:40:36 UTC
Created attachment 340506 [details] [review]
0001-wintab-tilt-Check-return-location-for-validity.patch

Better plan is to use g_return_if_fail(). This is a precondition, after all.
Comment 21 Andrew Chadwick 2016-11-22 10:55:54 UTC
Reopening while the patch in comment 20 is considered. I don't think it ever *can* be null, but it would seem a good idea to cover the possibility as a standardized precondition check.

Will test shortly.
Comment 22 Andrew Chadwick 2016-11-22 11:14:57 UTC
Okay, I can confirm that tilt is working correctly for my Intuos 5 with the current stable and the patch in comment 20. My other test tablets would test the no-tilt case, but I'm currently blocked by bug 774379 for testing those :)
Comment 23 Ignacio Casal Quinteiro (nacho) 2016-11-22 11:17:08 UTC
Thanks