[PATCH v4 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. -- v4: winex11: Remove stub tablet_get_packet wow64 thunk wintab32: Align WTPACKET for 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, and it is used as an integer rather than a pointer, so this should be safe to do. This eliminates the need to have a wow64 thunk for tablet_get_packet. --- dlls/winex11.drv/wintab.c | 2 +- dlls/wintab32/context.c | 9 ++++++--- dlls/wintab32/wintab_internal.h | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/dlls/winex11.drv/wintab.c b/dlls/winex11.drv/wintab.c index 6f1437f14c6..eff872fa873 100644 --- a/dlls/winex11.drv/wintab.c +++ b/dlls/winex11.drv/wintab.c @@ -236,7 +236,7 @@ typedef struct tagWTI_DEVICES_INFO #define CSR_TYPE_OTHER 0x000 typedef struct tagWTPACKET { - HCTX pkContext; + UINT pkContext; UINT pkStatus; LONG pkTime; WTPKT pkChanged; diff --git a/dlls/wintab32/context.c b/dlls/wintab32/context.c index b432e902a09..488e2b7d075 100644 --- a/dlls/wintab32/context.c +++ b/dlls/wintab32/context.c @@ -102,7 +102,7 @@ static const char* DUMPBITS(int x) static inline void DUMPPACKET(WTPACKET packet) { - TRACE("pkContext: %p pkStatus: 0x%x pkTime : 0x%lx pkChanged: 0x%lx pkSerialNumber: 0x%x pkCursor : %i pkButtons: %lx pkX: %li pkY: %li pkZ: %li pkNormalPressure: %i pkTangentPressure: %i pkOrientation: (%i,%i,%i) pkRotation: (%i,%i,%i)\n", + TRACE("pkContext: 0x%x pkStatus: 0x%x pkTime : 0x%lx pkChanged: 0x%lx pkSerialNumber: 0x%x pkCursor : %i pkButtons: %lx pkX: %li pkY: %li pkZ: %li pkNormalPressure: %i pkTangentPressure: %i pkOrientation: (%i,%i,%i) pkRotation: (%i,%i,%i)\n", packet.pkContext, packet.pkStatus, packet.pkTime, packet.pkChanged, packet.pkSerialNumber, packet.pkCursor, packet.pkButtons, packet.pkX, packet.pkY, packet.pkZ, packet.pkNormalPressure, packet.pkTangentPressure, @@ -205,7 +205,7 @@ LPOPENCONTEXT AddPacketToContextQueue(LPWTPACKET packet, HWND hwnd) tgt = ptr->PacketsQueued; - packet->pkContext = ptr->handle; + packet->pkContext = (UINT)(DWORD_PTR)ptr->handle; /* translate packet data to the context */ packet->pkChanged = packet->pkChanged & ptr->context.lcPktData; @@ -296,12 +296,15 @@ static LPVOID TABLET_CopyPacketData(LPOPENCONTEXT context, LPVOID lpPkt, LPWTPACKET wtp) { LPBYTE ptr; + HCTX ctx; ptr = lpPkt; TRACE("Packet Bits %s\n",DUMPBITS(context->context.lcPktData)); + ctx = (HCTX)(DWORD_PTR)wtp->pkContext; + if (context->context.lcPktData & PK_CONTEXT) - ptr+=CopyTabletData(ptr,&wtp->pkContext,sizeof(HCTX)); + ptr+=CopyTabletData(ptr,&ctx,sizeof(HCTX)); if (context->context.lcPktData & PK_STATUS) ptr+=CopyTabletData(ptr,&wtp->pkStatus,sizeof(UINT)); if (context->context.lcPktData & PK_TIME) diff --git a/dlls/wintab32/wintab_internal.h b/dlls/wintab32/wintab_internal.h index b0a2e8fd58f..f1529a59bb1 100644 --- a/dlls/wintab32/wintab_internal.h +++ b/dlls/wintab32/wintab_internal.h @@ -117,7 +117,7 @@ typedef struct tagWTI_EXTENSIONS_INFO } WTI_EXTENSIONS_INFO, *LPWTI_EXTENSIONS_INFO; typedef struct tagWTPACKET { - HCTX pkContext; + UINT pkContext; 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 866866dfba4..e441298c9aa 100644 --- a/dlls/winex11.drv/x11drv_main.c +++ b/dlls/winex11.drv/x11drv_main.c @@ -811,12 +811,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 @@ -837,7 +831,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
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=149644 Your paranoid android. === debian11b (64 bit WoW report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 0000000000F400E6, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
On Wed Nov 13 16:14:10 2024 +0000, John Chadwick wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/6584/diffs?diff_id=143044&start_sha=0e5e1a3ff7849d23c900ddce13fb071cab403fba#e4ab419f33bcdcbdfae98307a4c5409f81a2e6c3_242_239) Ah now I understand, of course you're correct. I've made `pkContext` a UINT and adjusted the commit messages. Hopefully I didn't miss anything here. Tested with PaintTool SAI2 32-bit/64-bit.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6584#note_87344
participants (3)
-
John Chadwick -
John Chadwick (＠jchw)
-
Marvin