Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mouse position on scroll event is set to -1 #88

Closed
mannyamorim opened this issue Mar 19, 2020 · 7 comments
Closed

Mouse position on scroll event is set to -1 #88

mannyamorim opened this issue Mar 19, 2020 · 7 comments

Comments

@mannyamorim
Copy link
Contributor

For the WINCON port, when we use get a KEY_MOUSE event and use request_mouse_pos to get the event data, if it is a scroll event the x,y coordinates are both set to -1. This is happening here:

PDCurses/wincon/pdckbd.c

Lines 444 to 455 in 618e0aa

if (MEV.dwEventFlags == 4)
{
SP->mouse_status.changes = (MEV.dwButtonState & 0xFF000000) ?
PDC_MOUSE_WHEEL_DOWN : PDC_MOUSE_WHEEL_UP;
SP->mouse_status.x = -1;
SP->mouse_status.y = -1;
memset(&old_mouse_status, 0, sizeof(old_mouse_status));
return KEY_MOUSE;
}

It seems intentional, but I am confused as to why we are doing this. It is easy to change these lines to report the actual coordinates.

Is there a different way we should be getting the mouse coordinates in the case of a scroll event or something else that I am missing here?

@Bill-Gray
Copy link
Contributor

Bill-Gray commented Mar 19, 2020

This is not just a WinCon issue. All the ports do it. I think in all ports, it would be easy to report actual coordinates, as it would be here.

When starting my fork and adding new ports, I noticed this behavior, scratched my head, and said, "Well, I don't want to do anything that breaks expected behavior." But I can't really see how anybody would be relying on this (odd) behavior, and am considering changing it in my fork so that the actual mouse position is reported for all events.

@mannyamorim mannyamorim changed the title WINCON: Mouse position on scroll event is set to -1 Mouse position on scroll event is set to -1 Mar 20, 2020
@wmcbrine
Copy link
Owner

Perhaps @rexx-org can comment on the purpose of this feature, as it dates to his original addition of scroll wheel support in 2004. 5fff3d6

@rexx-org
Copy link

rexx-org commented May 9, 2020

Sorry, I can't provide any insight into the rationale for these settings :-(

@mannyamorim
Copy link
Contributor Author

Thanks for taking a look at this guys. I know, I'm digging up some old code here. Since the original reason for this behavior seems to be forgotten/lost, I am recommending that we bring it inline with the behavior in ncurses. NCurses treats all of the mouse events as button presses and therefore it sends the actual coordinates.

I have a hard time imagining why an application would be relying on receiving a (-1,-1) when looking for mouse scroll events so I feel like this should be safe to merge without worrying about breaking existing applications.

Since I'm primarily concerned with the WINCON port I have submitted a PR for that port #91. However, as @Bill-Gray mentioned above it seems to be the same for all of the ports and it probably makes sense to change it everywhere.

The patch is only a few lines, so when you have chance, please let me know what you are thinking about it. Thanks.

@Bill-Gray
Copy link
Contributor

I never liked the (-1,-1) coords, but assumed there was some backward compatibility issue. Since Mark doesn't recall one, I've decided to return the coordinates in my fork. If somebody reports things flying apart, we can go back to the (-1,-1) bit.

Note that I've only got WinGUI, X11, VT, and SDLn thus far, with the other platforms to follow. @mannyamorim, I'd appreciate it if you submitted your WinCon patch to my fork as well.
That'll leave me with DOS, DOSVGA, and OS/2. My mouse wheel is unrecognized on the first two, and I don't have OS/2, so I'll let those go for the nonce... the DOS fix looks obvious (and identical on both platforms), but I hate to make changes and say "of course it's going to work; I don't need to test it."

@GiorgosXou
Copy link
Contributor

Any update on this issue? It happens to be something that I use on my TUIFIManager, as a range in which the mouse-events should take any action (not something really critical, but it would be nice to work just like with ncurses)

@wmcbrine
Copy link
Owner

Patched for SDL, X11, wincon, DOS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants