Possible #612 fix

[probablySophie]

Issue #612 is about scrolling not working on secondary monitors when running under Wayland.

@tuomovee says that removing the check for the mousewheel event in dispatchEvent_Window() appears to resolve the issue, but is unaware of any potential consequences.

This does fix the issue on Wayland, with seemingly no consequences, split views appear to behave fine.
I haven't tested this on X11, but I can.

So barring anyone coming in and saying this is the worst idea ever, it looks like a fix?

[skyjake]

My concern with removing that condition is about performance: wheel events occur frequently, at least once per frame, so it is disadvantegeous to offer them to widgets that we already know shouldn't handle them. My educated guess is that the actual problem is that the wheel event does not know the current cursor position, or the coordinates are unexpectedly affected by the secondary monitor.

I have no way to test a dual monitor Wayland setup myself, but if anyone can print out those wheel event coordinates and see if they make sense, it could enlighten us here.

[probablySophie]

Super hacky:

if (ev->type == SDL_MOUSEWHEEL)
{
    printf("Scroll on %ld %lu\n", i, (unsigned long)time(NULL));
}

Shows that when scrolling in a window with a split view, there appears to only be one event, but the first scroll in a different split widget has two events which is fun and weird.

My educated guess is that the actual problem is that the wheel event does not know the current cursor position

You're probably right that the fix shouldn't be in the dispatchEvent_Window function, but should instead be in contains_Rect.

I'll have a look and get back!

Edit:
For the coord_MouseWheelEvent function in src/ui/util.c
Adding printf("coord_MouseWheelEvent %d %d\n", mousePos.x, mousePos.y); appears to show that on my second monitor the X coord goes from -1920 to -3840.
So that's probably the culprit, but still looking

[probablySophie]

I'm not going to open a new PR just yet, but it looks like for a real fix swapping:

// original
if (ev->type == SDL_MOUSEWHEEL && !contains_Rect(rect_Root(root),
                                                 coord_MouseWheelEvent(&ev->wheel)))

for

// updated
if (ev->type == SDL_MOUSEWHEEL && !contains_Rect(rect_Root(root),
                                                 coord_Window(d, ev->wheel.mouseX, ev->wheel.mouseY)))

works

From printing the various variables in coord_MouseWheelEvent I also found that winPos.x & winPos.y both appear to be static? They're 0, 0 on my screen 1 and 1920, 0 on my screen 2

Just for fun, I tried messing with mousePos.x in coord_MouseWheelEvent and adding:

[...]
SDL_GetGlobalMouseState(&mousePos.x, &mousePos.y);
        
mousePos.x += 1920;

SDL_GetWindowPosition(win->win, &winPos.x, &winPos.y);
[...]

and screen 2 starts working under the original/current dispatchEvent_Window function, with screen 1 no longer working

[shurizzle]

I'm not going to open a new PR just yet, but it looks like for a real fix swapping:

// original
if (ev->type == SDL_MOUSEWHEEL && !contains_Rect(rect_Root(root),
                                                 coord_MouseWheelEvent(&ev->wheel)))

for

// updated
if (ev->type == SDL_MOUSEWHEEL && !contains_Rect(rect_Root(root),
                                                 coord_Window(d, ev->wheel.mouseX, ev->wheel.mouseY)))

I tried this last patch and it works. Thank you.

[skyjake]

If using ev->wheel.mouseX/Y solves the issue, that would be an ideal fix. These were added in SDL 2.26, which I suppose is good enough here. I'll incorporate this into v1.18 and you can test with the dev branch.

[shurizzle]

@skyjake I believe the commit message for 83d29c1 is incorrect. I only confirmed that it works on Linux; the solution was written by @probablySophie, who has nothing to do with me.

[skyjake]

@shurizzle Ah, right you are, sorry for the confusion.