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 a function to get the monitor on which a window is currently being displayed #2404

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Scr3amer
Copy link
Contributor

@Scr3amer Scr3amer commented Sep 26, 2023

Hello there,

DISCLAIMER: It should be a draft. Not all OSes are implemented and edge cases are probably not properly handled (for instance I need to double check what happens if window is hidden and we call the new function).

Me again with another PR.

This time it is to add a function to retrieve the monitor on which a window is currently being displayed.

It is a different implementation compared to #2220 as I use win32 API MonitorFromWindow here instead of custom logic.

I only did null and win32

PRs are welcome for Linux and Mac OS.

Also I need some feedback on the naming documentation and such.

Cheers,
Scr3am

@dougbinks dougbinks self-requested a review November 17, 2023 11:47
@dougbinks dougbinks self-assigned this Nov 17, 2023
@dougbinks dougbinks added the enhancement Feature suggestions and PRs label Nov 17, 2023
@dougbinks
Copy link
Contributor

One consideration is that it is possible to get the monitor on which a window is currently being displayed by calculating the area a window covers and comparing this to the glfwGetMonitorWorkarea, returning the largest value if there is an overlap or either the nearest or primary if there is non.

I'll look into whether the cross platform approach would be best as a fall back implementation or if more 'platform correct' features exist.

One thing we may want to consider is the following:

  • Should the monitor returned correspond to the monitor which the window will fill if maximised? This is potentially not the same as the monitor the window covers the most.
  • What should be returned for a iconified window?

@Scr3amer
Copy link
Contributor Author

Hey good to see you :)) !

I think your proposal is actually the PR I linked in my description.
I thought using native API would always be better than custom logic by default. But sure it is always cool to have a fallback for OSes that don't provide equivalent feature.

I'll double check the iconified behaviour tomorrow if God wills. I would love the API to return the monitor it will be displayed on if maximised OR nullptr.

Also IIRC win32 api provides an ENUM for the way to handle edge cases :

MONITOR_DEFAULTTONEAREST | Returns a handle to the display monitor that is nearest to the window.
MONITOR_DEFAULTTONULL | Returns NULL.
MONITOR_DEFAULTTOPRIMARY | Returns a handle to the primary display monitor.

https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-monitorfromwindow

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

Successfully merging this pull request may close these issues.

None yet

3 participants