Description
When using the pen under X11, SDL synthesizes a mouse move when the pen is moved over a window. That is fine, but using the test code below one can see that two mouse events are sent, not one.
I think the issue is that xinput2 may be also emulating xi mouse events itself when using the pen. I'm not familiar with "master/slave" devices (just read a bit) but it seems like 2 xi events are sent: the first with same deviceid and sourceid (that device id is then used to check if the device is a pen); the second has the same sourceid as before but a lower deviceid (here it is 2). The 2 xi events are sent even if I use the mouse.
I see there is some code to check if the device is a master device in the button press/release and motion event handlers, something like:
if (xev->deviceid != xev->sourceid) {
// Discard events from "Master" devices to avoid duplicates.
break;
}In the case of button press/release it seems to work: only one sdl event is sent at the end, but in the case of the motion, two are sent.
This is the code that handles the motion event:
case XI_Motion:
{
const XIDeviceEvent *xev = (const XIDeviceEvent *)cookie->data;
#ifdef SDL_VIDEO_DRIVER_X11_XINPUT2_SUPPORTS_MULTITOUCH
bool pointer_emulated = ((xev->flags & XIPointerEmulated) != 0);
#else
bool pointer_emulated = false;
#endif
videodata->global_mouse_changed = true;
X11_PenHandle *pen = X11_FindPenByDeviceID(xev->deviceid);
if (pen) {
if (xev->deviceid != xev->sourceid) {
// Discard events from "Master" devices to avoid duplicates.
break;
}
SDL_Window *window = xinput2_get_sdlwindow(videodata, xev->event);
SDL_SendPenMotion(0, pen->pen, window, (float) xev->event_x, (float) xev->event_y);
float axes[SDL_PEN_AXIS_COUNT];
X11_PenAxesFromValuators(pen, xev->valuators.values, xev->valuators.mask, xev->valuators.mask_len, axes);
for (int i = 0; i < SDL_arraysize(axes); i++) {
if (pen->valuator_for_axis[i] != SDL_X11_PEN_AXIS_VALUATOR_MISSING) {
SDL_SendPenAxis(0, pen->pen, window, (SDL_PenAxis) i, axes[i]);
}
}
} else if (!pointer_emulated && xev->deviceid == videodata->xinput_master_pointer_device) {
// Use the master device for non-relative motion, as the slave devices can seemingly lag behind.
SDL_Mouse *mouse = SDL_GetMouse();
if (!mouse->relative_mode) {
SDL_Window *window = xinput2_get_sdlwindow(videodata, xev->event);
if (window) {
X11_ProcessHitTest(_this, window->internal, (float)xev->event_x, (float)xev->event_y, false);
SDL_SendMouseMotion(0, window, SDL_GLOBAL_MOUSE_ID, false, (float)xev->event_x, (float)xev->event_y);
}
}
}
} break;The code checks if the device is a pen using xev->deviceid, which is only true when the first xi event is sent (when deviceid and sourceid are the same). In that case a sdl mouse event is emulated in SDL_SendPenMotion.
But then the second xi event is sent, which has a different deviceid and so it is not considered a pen and the "else" path is executed. There a sdl mouse motion event is sent with SDL_SendMouseMotion. So at the end nothing prevents that 2 sdl events are sent.
In the else path, xev->deviceid == videodata->xinput_master_pointer_device is checked, but that is also true for the second xi pen event emited.
I did some tests and it seems that using the sourceid (instead of the deviceid) to check if the device is a pen is enough. Although if that is used in the press/release then other things need to be changed. This is what I tried and seemed to work, although I could not check stuff like the eraser tip (I don't know if that is important since the eraser is listed as a different device I think). As I said, I'm not really familiar with x11 or xinput code, so I don't know if this is the correct way of handling it:
diff --git a/src/video/x11/SDL_x11xinput2.c b/src/video/x11/SDL_x11xinput2.c
index afe4a7c85..98080ccfd 100644
--- a/src/video/x11/SDL_x11xinput2.c
+++ b/src/video/x11/SDL_x11xinput2.c
@@ -406,11 +406,15 @@ void X11_HandleXinput2Event(SDL_VideoDevice *_this, XGenericEventCookie *cookie)
case XI_ButtonRelease:
{
const XIDeviceEvent *xev = (const XIDeviceEvent *)cookie->data;
- X11_PenHandle *pen = X11_FindPenByDeviceID(xev->deviceid);
+ X11_PenHandle *pen = X11_FindPenByDeviceID(xev->sourceid);
const int button = xev->detail;
const bool down = (cookie->evtype == XI_ButtonPress);
if (pen) {
+ if (xev->deviceid != xev->sourceid) {
+ // Discard events from "Master" devices to avoid duplicates.
+ break;
+ }
// Only report button event; if there was also pen movement / pressure changes, we expect an XI_Motion event first anyway.
SDL_Window *window = xinput2_get_sdlwindow(videodata, xev->event);
if (button == 1) { // button 1 is the pen tip
@@ -422,11 +426,6 @@ void X11_HandleXinput2Event(SDL_VideoDevice *_this, XGenericEventCookie *cookie)
// Otherwise assume a regular mouse
SDL_WindowData *windowdata = xinput2_get_sdlwindowdata(videodata, xev->event);
- if (xev->deviceid != xev->sourceid) {
- // Discard events from "Master" devices to avoid duplicates.
- break;
- }
-
if (down) {
X11_HandleButtonPress(_this, windowdata, (SDL_MouseID)xev->sourceid, button,
(float)xev->event_x, (float)xev->event_y, xev->time);
@@ -449,7 +448,8 @@ void X11_HandleXinput2Event(SDL_VideoDevice *_this, XGenericEventCookie *cookie)
videodata->global_mouse_changed = true;
- X11_PenHandle *pen = X11_FindPenByDeviceID(xev->deviceid);
+ X11_PenHandle *pen = X11_FindPenByDeviceID(xev->sourceid);
+
if (pen) {
if (xev->deviceid != xev->sourceid) {
// Discard events from "Master" devices to avoid duplicates.
Test code:
#include <SDL3/SDL.h>
int main()
{
if (!SDL_Init(SDL_INIT_VIDEO)) {
SDL_Log("Couldn't initialize SDL: %s", SDL_GetError());
return SDL_APP_FAILURE;
}
SDL_Window *window = SDL_CreateWindow("hello", 800, 600, 0);
if (!window) {
SDL_Log("Couldn't create window: %s", SDL_GetError());
return SDL_APP_FAILURE;
}
SDL_ShowWindow(window);
bool exit_requested = false;
while(!exit_requested)
{
SDL_Event sdl_event;
while (SDL_PollEvent(&sdl_event))
{
switch (sdl_event.type)
{
case SDL_EVENT_QUIT:
case SDL_EVENT_WINDOW_CLOSE_REQUESTED:
{
exit_requested = true;
break;
}
case SDL_EVENT_MOUSE_BUTTON_DOWN:
{
SDL_Log("SDL_EVENT_MOUSE_BUTTON_DOWN");
break;
}
case SDL_EVENT_MOUSE_BUTTON_UP:
{
SDL_Log("SDL_EVENT_MOUSE_BUTTON_UP");
break;
}
case SDL_EVENT_MOUSE_MOTION:
{
SDL_Log("SDL_EVENT_MOUSE_MOTION");
break;
}
case SDL_EVENT_PEN_DOWN:
{
SDL_Log("SDL_EVENT_PEN_DOWN");
break;
}
case SDL_EVENT_PEN_UP:
{
SDL_Log("SDL_EVENT_PEN_UP");
break;
}
case SDL_EVENT_PEN_MOTION:
{
SDL_Log("SDL_EVENT_PEN_MOTION");
break;
}
default:
break;
}
}
}
return 0;
}