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

Add pygame.mouse.get_desktop_pos #3070

Closed
wants to merge 7 commits into from

Conversation

damusss
Copy link
Contributor

@damusss damusss commented Aug 20, 2024

Original discord message:

pygame.mouse.get_pos() returns the last recorded window position, meaning (it seems to me) there is no way to check if the window is hovered (regardless of the window focus status) without something like pyautogui (pretty heavy library). having it would also facilitate window dragging (possible without this, but not straight forward)

This is simply a wrapper around SDL_GetGlobalMouseState which was not wrapped before.
Function name changes are welcome.

Test code:

import pygame

win = pygame.Window("desktop pos test", (500, 500))

# if a member of the SC is reading this, Window.surface >>>> Window.get_surface!
screen = win.get_surface()

clock = pygame.Clock()

while True:
    for event in pygame.event.get():
        if event.type == pygame.WINDOWCLOSE and event.window == win:
            pygame.quit()
            raise SystemExit

    screen.fill(0)

    dpos = pygame.mouse.get_desktop_pos()
    win_rect = pygame.Rect(win.position, win.size)
    if win_rect.collidepoint(dpos):
        print(f"MOUSE HOVERING! desktop pos: {dpos}")
    else:
        print(f"NO HOVER. desktop pos: {dpos}")

    win.flip()
    clock.tick(60)

@damusss damusss added New API This pull request may need extra debate as it adds a new class or function to pygame mouse pygame.mouse labels Aug 20, 2024
@damusss damusss requested a review from a team as a code owner August 20, 2024 12:45
@Starbuck5
Copy link
Member

How does this work with multiple desktops / monitors?

In SDL3 this function will return floats, should this function cast to floats before returning to be ready for that change?

@damusss
Copy link
Contributor Author

damusss commented Aug 21, 2024

How does this work with multiple desktops / monitors?

In SDL3 this function will return floats, should this function cast to floats before returning to be ready for that change?

Unfortunately I can't really test as I only have one monitor. In the past andrew has been the monitor tester so that could be the case here too.

About the floats, yeah it's good to return them so for pygame 3 we will have one api breaking change less to worry about. People (when I say people I mean me) usually cast to Vector2 anyways. I can't see a scenario where floats would be annoying.

@damusss
Copy link
Contributor Author

damusss commented Aug 21, 2024

I also added the test code I forgot previously in the PR description

@rethanon
Copy link
Contributor

I tried this on my dual monitor setup - I have 1440p monitors and with the monitor layout set to side by side, the mouse positions reported as follows:
Left monitor: topleft = (-2560, 0), bottomleft = (-2560, 1439), topright = (-1, 0), bottomright = (-1, 1439)
Right monitor: topleft = (0, 0), bottomleft = (0, 1439), topright = 2559, 0), bottomright = (2559, 1439)

As you can probably guess, when I changed the monitor layout to vertical i.e. one above the other, the mouse positions reported as follows:
Top monitor: topleft = (0, -1440), bottomleft = (0, -1), topright = (2559, -1440), bottomright = (2559, -1)
Bottom monitor: topleft = (0, 0), bottomleft = (0, 1439), topright = (2559, 0), bottomright = (2559, 1439)

This also updated realtime, when changing the monitor layout.

@rethanon
Copy link
Contributor

Also - detecting mouse hover was based on window location and not focus - it was reporting hover even when other apps were in the foreground and my mouse moved over the position where the window was located.

@damusss
Copy link
Contributor Author

damusss commented Aug 21, 2024

Also - detecting mouse hover was based on window location and not focus - it was reporting hover even when other apps were in the foreground and my mouse moved over the position where the window was located.

I am aware of that. That hover detection only works for always-on-top windows. But hovering has nothing to do with this PR core concept therefore I don't think it's a problem. It was only an example

@damusss
Copy link
Contributor Author

damusss commented Aug 21, 2024

I tried this on my dual monitor setup - I have 1440p monitors and with the monitor layout set to side by side, the mouse positions reported as follows: Left monitor: topleft = (-2560, 0), bottomleft = (-2560, 1439), topright = (-1, 0), bottomright = (-1, 1439) Right monitor: topleft = (0, 0), bottomleft = (0, 1439), topright = 2559, 0), bottomright = (2559, 1439)

As you can probably guess, when I changed the monitor layout to vertical i.e. one above the other, the mouse positions reported as follows: Top monitor: topleft = (0, -1440), bottomleft = (0, -1), topright = (2559, -1440), bottomright = (2559, -1) Bottom monitor: topleft = (0, 0), bottomleft = (0, 1439), topright = (2559, 0), bottomright = (2559, 1439)

This also updated realtime, when changing the monitor layout.

so I guess the position is relative to the main monitor, not the current monitor, am I wrong?

@rethanon
Copy link
Contributor

rethanon commented Aug 21, 2024

so I guess the position is relative to the main monitor, not the current monitor, am I wrong?

Yes, looks like it treats it as one big display area, but the co-ordinates depend on which display is set as primary monitor. The results I gave above are when the right monitor is set to be the primary display. When I change it so the left monitor is the primary display the coordinates range from (0, 0) to (5119, 1439).

The left (primary) monitor ranged from (0, 0) to (2559, 1439).
The right (primary) monitor ranged from (2560, 0) to (5119, 1439).

@damusss
Copy link
Contributor Author

damusss commented Aug 21, 2024

so I guess the position is relative to the main monitor, not the current monitor, am I wrong?

Looks like it treats it as one big display area, but the co-ordinates depend on which display is set as primary monitor. The results I gave above are when the right monitor is set to be the primary display. When I change it so the left monitor is the primary display the coordinates range from (0, 0) to (5119, 1439).

The left (primary) monitor ranged from (0, 0) to (2559, 1439). The right (primary) monitor ranged from (2560, 0) to (5119, 1439).

Alright, that's what I'm gonna say in the documentation. Thank you very much for testing it!! ❤️

@rethanon
Copy link
Contributor

Alright, that's what I'm gonna say in the documentation. Thank you very much for testing it!! ❤️

No problem! In fact, in testing this I discovered that my monitors had not been properly aligned in Windows display settings, with one being 5 pixels higher than the other, so really I should be thanking you 😁

@ankith26
Copy link
Member

This seems broken on both X11 and wayland, and in different ways. I know it's not your fault, but I'd prefer to not expose SDL API that isn't portable, atleast until SDL can fix the issues

docs/reST/ref/mouse.rst Outdated Show resolved Hide resolved
@Starbuck5
Copy link
Member

Starbuck5 commented Aug 29, 2024

This seems broken on both X11 and wayland, and in different ways. I know it's not your fault, but I'd prefer to not expose SDL API that isn't portable, atleast until SDL can fix the issues.

I see where you're coming from, but if we only exposed stuff that worked on Wayland we'd have to delete most of the Window module.

What about a ..warning:: in the docs?

Also what does broken mean? I don't see any exception raising in here.

@damusss
Copy link
Contributor Author

damusss commented Aug 29, 2024

@Starbuck5

  • broken on x11 means it's somehow "not fixed", as if there was a bug (but I can't see that in the SDL source code, and can't test, I'd be happy if someone tested this)
  • broken on wayland means it's not actually the global mouse position, because it is impossible to get that from the os - the window manager doesn't tell you, so the return is pretty much the same as get_pos. I personally think this is not something to worry about (except for a docs warning, I guess) as you can't use Window.position on wayland either.

docs/reST/ref/mouse.rst Outdated Show resolved Hide resolved
@ankith26
Copy link
Member

I see where you're coming from, but if we only exposed stuff that worked on Wayland we'd have to delete most of the Window module.

I'm not against exporting stuff that don't work on wayland, for example I think exporting opacity and always-on-top is a good thing despite it not working on wayland because I can see the usecases. In this instance, I don't really see a strong usecase for why an application would need this and not get_pos that can justify us exposing non-portable API.

Though feel free to let me know if you have a usecase in mind that is useful enough to ignore the x11/wayland concerns

@damusss
Copy link
Contributor Author

damusss commented Aug 30, 2024

@ankith26, so I can think of at least 2 usecases in my most recent application:

  • know if my always-on-top window is hovered, regardless of wether the window is focused or not. It's my miniplayer, I only want to show the pause/next/close buttons if you are hovering it, but when you click another app the window (which stays on top) loses focus so mouse.get_pos returns the wrong position I can't check the hover status. I had to add PySDL2 as a dependency just for that.
  • Move/Resize the window without using the provided functionality from the window manager, without glitches. In my app I have an option to have a custom titlebar, which means the window needs to be borderless, which means I have to reimplement drag & resize myself, but when I resize the topleft, the window must be resized and the position updated, but since get_pos lags behind the window position, the window glitches like crazy.having the global position would remove any mouse position glitches.

Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I am approving this PR. I think the warning note in the doc on wayland is a good addition. Thanks for contributing! 🎉

@ankith26 ankith26 marked this pull request as draft September 23, 2024 08:20
@bilhox bilhox closed this in #3105 Sep 26, 2024
@damusss damusss deleted the mouse-get_desktop_pos branch September 26, 2024 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dont merge mouse pygame.mouse New API This pull request may need extra debate as it adds a new class or function to pygame
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants