[PATCH v2 0/2] MR6584: wintab32: Improve wow64 support
wintab32 is currently broken in the new wow64 mode because the `tablet_get_packet` thunk in winex11.drv is not implemented. Since the structure it is copying is entirely internal to Wine, and the HCTX handle value is not actually read or written by the driver side, I found that simply padding the structure to keep the rest of it aligned was enough to make wow64 happy and allow wintab32 to function. This is somewhat a stopgap. What would be much better is to refactor the wintab code such that more of the logic is in the wintab32 implementation and not the driver. My goal is to accomplish this as part of trying to get my winewayland graphics tablet patch into an acceptable state for upstreaming. However, I think this is still a worthwhile improvement to have in the interim, if it is acceptable. -- v2: winex11: Remove stub tablet_get_packet wow64 thunk wintab32: Pad WTPACKET to align 32/64-bit archs https://gitlab.winehq.org/wine/wine/-/merge_requests/6584
From: John Chadwick <john(a)jchw.io> WTPACKET's structure is never directly exposed via the API; it's internal to Wine. The HCTX value is only used on the wintab32 side, not the driver side, so this is safe to do. This eliminates the need to have a wow64 thunk for tablet_get_packet. --- dlls/winex11.drv/wintab.c | 3 +++ dlls/wintab32/wintab_internal.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/dlls/winex11.drv/wintab.c b/dlls/winex11.drv/wintab.c index 6f1437f14c6..951395d9ee5 100644 --- a/dlls/winex11.drv/wintab.c +++ b/dlls/winex11.drv/wintab.c @@ -237,6 +237,9 @@ typedef struct tagWTI_DEVICES_INFO typedef struct tagWTPACKET { HCTX pkContext; +#ifndef _WIN64 + DWORD pkPadding; +#endif UINT pkStatus; LONG pkTime; WTPKT pkChanged; diff --git a/dlls/wintab32/wintab_internal.h b/dlls/wintab32/wintab_internal.h index b0a2e8fd58f..c901f3cf64b 100644 --- a/dlls/wintab32/wintab_internal.h +++ b/dlls/wintab32/wintab_internal.h @@ -118,6 +118,9 @@ typedef struct tagWTI_EXTENSIONS_INFO typedef struct tagWTPACKET { HCTX pkContext; +#ifndef _WIN64 + DWORD pkPadding; +#endif UINT pkStatus; LONG pkTime; WTPKT pkChanged; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6584
From: John Chadwick <john(a)jchw.io> The previous commit ensures that the WTPACKET fields align between 32-bit and 64-bit architectures, so now we can use the same tablet_get_packet without needing another thunk. --- dlls/winex11.drv/x11drv_main.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c index 3f8e48a7a8d..c4ec133cb0b 100644 --- a/dlls/winex11.drv/x11drv_main.c +++ b/dlls/winex11.drv/x11drv_main.c @@ -826,12 +826,6 @@ C_ASSERT( ARRAYSIZE(__wine_unix_call_funcs) == unix_funcs_count ); #ifdef _WIN64 -static NTSTATUS x11drv_wow64_tablet_get_packet( void *arg ) -{ - FIXME( "%p\n", arg ); - return 0; -} - static NTSTATUS x11drv_wow64_tablet_info( void *arg ) { struct @@ -852,7 +846,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = { x11drv_init, x11drv_tablet_attach_queue, - x11drv_wow64_tablet_get_packet, + x11drv_tablet_get_packet, x11drv_wow64_tablet_info, x11drv_tablet_load_info, }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6584
v2: renamed `dwPadding` to `pkPadding` to match the rest of the struct members. (P.S.: I don't know why my MRs always seem to tag a reviewer automatically when nobody elses seem to. If I am doing something wrong please let me know, I am not trying to be a nuisance.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6584#note_83664
Do you have any application that uses WinTab? I thought it was some old and unused part of the code, but I've seen people asking for it for the wayland driver too, and I'm wondering if we could implement it in a generic way on top of the early pointer / HID device support we now have. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6584#note_83668
On Mon Sep 30 17:14:00 2024 +0000, Rémi Bernon wrote: > Do you have any application that uses WinTab? I thought it was some old > and unused part of the code, but I've seen people asking for it for the > wayland driver too, and I'm wondering if we could implement it in a > generic way on top of the early pointer / HID device support we now have. Hmm. I think most applications that support WinTab32 now also support using the newer pointer events API directly, e.g. Clip Studio, Painttool SAI 2, etc. I think we definitely want to keep WinTab32 support overall: - Older software will still use WinTab32 only (e.g. Painttool SAI 1, which people still use) - Even newer software may still need WinTab32 for certain things. (As an example, SAI 2 supports using SAI 1's brush engine, but then you're stuck with WinTab32. Some artists rely on this feature.) - Some features of the WinTab32 API (e.g. absolute input mode) can't be implemented, as far as I know, directly with the pointer API. (I'm not sure if anyone actually cares about this feature anymore, though.) - The Pointer API is limited to 1024 pressure levels. On Wayland we're only limited to 65536 pressure levels. (In practice I think this is not a huge concern, but it is worth calling out.) Even with that said, it would be immensely cleaner, so I would love to implement WinTab32 in terms of the pointer API instead if we could, but I have struggled over the years to make patches to implement the Windows pointer events API in the first place. It's hard to test it since we can't really implement the synthetic pointer API, there's no way to e.g. inject events at the display server level with XI2. That said, I did a lot of ground work trying to [figure out how the pointer API works](https://github.com/jchv/WmPointerDemo/) (completely clean-room of course.) I'm not sure how much we should try to mirror what Windows does. I'm guessing first-party software will use the undocumented per-window setting for mouse-in-pointer, for example. I think I posted to the mailing list about this a while ago but didn't manage to catch any attention so I assumed there wasn't much interest. I'm happy to work on this if there is interest. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6584#note_83673
I see, thanks. It would be nice if we could implement it on top of the WM_POINTER messages. Fwiw we can, and have, some pointer/touchpad tests now (see https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/dinput/tests/device8....), which can be run without any actual hardware as they are implemented using a virtual HID device. Then, although they are not exposed as an actual HID device, the X11 XI2 touch events are also routed through the same path (see https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/winex11.drv/mouse.c#L...), and translated into WM_POINTER* messages, we could do the same for Wayland events. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6584#note_83675
On Mon Sep 30 17:37:11 2024 +0000, Rémi Bernon wrote:
I see, thanks. It would be nice if we could implement it on top of the WM_POINTER messages. Fwiw we can, and have, some pointer/touchpad tests now (see https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/dinput/tests/device8....), which can be run without any actual hardware as they are implemented using a virtual HID device. Then, although they are not exposed as an actual HID device, the X11 XI2 touch events are also routed through the same path (see https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/winex11.drv/mouse.c#L...), and translated into WM_POINTER* messages, we could do the same for Wayland events. OK, I think I now understand. That sounds like a promising avenue.
(I will note that if my messages seem confusing it's partly because I never noticed WM_POINTER* support was added to Wine. That's good news.) For Wayland it would be trivial in theory to add WM_POINTER* events for the tablet code, and in fact it might make life easier. The tablet-v2 interface maps nicely to the WM_POINTER* API. Once we bind to the tablet-v2 manager, the compositor will stop sending wl_pointer events, but if we generate WM_POINTER* events, this should be fine, since DefWindowProc will translate them to WM_MOUSE* events, just like on Windows. For the XI2 tablet code I think it's a little more hairy because I think enabling XI2/WinTab32 has other side-effects and it's only done as-needed. Maybe it would be acceptable to implement graphics tablet WM_POINTER* events in winewayland first? It's a bit awkward because in that case, winex11 will implement only WinTab32 graphics tablet support, and winewayland will implement only WM_POINTER* graphics tablet support. (edit: for now, I mean.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6584#note_83677
participants (3)
-
John Chadwick -
John Chadwick (＠jchw)
-
Rémi Bernon