-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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 support for querying work area #989
Conversation
The function ```glfwGetMonitorWorkarea``` retrieves the rectangle work area of the specified monitor.
The function ```XGetWindowProperty``` was replaced with the GLFW function ```_glfwGetWindowPropertyX11``` in the operation to get the work area.
This is an initial implementation and needs review.
I won't be able to review this properly until Thursday at the earliest but it looks like a solid start. |
|
||
dc = CreateDCW(L"DISPLAY", monitor->win32.adapterName, NULL, NULL); | ||
|
||
if (!EnumDisplayMonitors(dc, NULL, MonitorEnumProc, (LPARAM)&workarea)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enumerating based on adapter DC doesn't work. Tested on Windows 7.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried every which way I could think of. Adapter device DCs yield no monitors and display device DCs yield all available monitors. Tested on dual-monitor system running Windows 7.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for testing. For some reason, using the DC worked here. We can put this patch on halt until we have the high DPI
patch with HMONITOR
on each _GLFWmonitorWin32
, then. And we won't have two implementations of callbacks for EnumDisplayMonitors
, making things cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wine is amazing but it's not a very reliable oracle for Windows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm working through these issues and have a resolution for this by using the monitor virtual position to derive a handle then use that to get the work area: dougbinks@921d983
What is the purpose of this API? Most likely, it wont be possible to implement completely on Wayland as there is usually no valid reason for querying this type of information (since the client have no possible use for it). What could be useful though is being aware of work area size, as this can be used to determine a good default size for a new window. |
@jadahl The purpose is exactly to retrieve the size of the work area. I started implementing due this issue #920. I mentioned there that Wayland in particular doesn't have the concept of work area. In my opinion, we should at least return the resolution of the screen, even if it is redundant with |
The concept of work area doesn't exist yet, true, but its introduction has been requested before, by for example GTK+ that uses the work area to figure out a default window size. The position, however, will probably not be exposed, as it would serve no purpose. What has to be considered is that it might not be useful to know any work area before knowing what monitor the window will be placed at. I expect this to be more of a compositor driven thing, as it has proven to be practically impossible to let applications them self place them in any predictable manner in a multi monitor configuration. Fullscreen is of course a different matter here, and in that case work area is irrelevant, as is with maximizing, as that is also done in a completely different way. So, I'm still wondering, what is the purpose of this API? What is its intended use case? Is it possible make any use of the API if the client doesn't have total control of exact positioning details? |
Ping @adambrown. |
If the window is created with |
@jadahl Sorry, I've been on vacation for 3 weeks and haven't been online. The purpose of this feature is detailed here in the ticket I created: #920 In the first usecase described there, trying to maximize an undecorated window, I suppose it wouldn't be strictly necessary to know which display it would end up on the first time. Although right now in my app I am determining the display for the window based on the current location of the mouse at init on first run unless there is a saved positioning configuration matching the current display setup. Or in other words, I check the count, size and relative positioning of the displays to see if I have a saved window position and size for that layout. If I don't have a stored position and size for the matching display layout then I put the window at a default small size in the center of the display the mouse is on. Most of this work I have to do with Java AWT right now because GLFW doesn't allow me to have all the data I need, such as the mouse location, without creating a window. The reason for doing it this way is that if you have a laptop that you take from home to work, or you install the app as portable on a USB stick, and there are different monitor setups in different locations, you can have positioning information stored for all of them. For the other usecase, trying to position notifications or other widgets, you would need to know what display you are putting the things on. I hope that clarifies the exact purpose and intended usecase of this API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m quite doubtful of this change, the only possible usecase would be to know how to maximise, but the maximise action will already give you this information in due time.
There is also no Wayland implementation, making this branch fail to build on this platform. It’s highly unlikely a protocol will be created for it, since it seems like a pure legacy feature.
I won’t block its inclusion in GLFW, but it will probably only clutter for the API without any benefit whatsoever.
* | ||
* @param[in] monitor The monitor to query. | ||
* @param[out] xpos Where to store the monitor x-coordinate, or `NULL`. | ||
* @param[out] ypos Where to store the monitor y-coordinate, or `NULL`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You’re missing width
and height
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed this here: dougbinks@147b0f5
* This function returns the position, in screen coordinates, of the upper-left | ||
* corner of the specified monitor. | ||
* | ||
* Any or all of the position arguments may be `NULL`. If an error occurs, all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The length arguments can also be NULL, according to the implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed this here: dougbinks@875cbdf
if (width) | ||
*width = 0; | ||
if (width) | ||
*width = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Itym height, same on the line above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed height here: dougbinks@196a7cf
* | ||
* @sa @ref monitor_properties | ||
* | ||
* @since Added in version 3.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, version will be at best 3.3. ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to 3.3: dougbinks@992e5da
/*! @brief Returns the work area of the monitor. | ||
* | ||
* This function returns the position, in screen coordinates, of the upper-left | ||
* corner of the specified monitor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added size and work area definition: dougbinks@1a1ad50
@@ -131,6 +132,8 @@ int main(void) | |||
glVertexAttribPointer(vcol_location, 3, GL_FLOAT, GL_FALSE, | |||
sizeof(vertices[0]), (void*) (sizeof(float) * 2)); | |||
|
|||
glfwGetMonitorWorkarea(glfwGetPrimaryMonitor(), &workarea_x, &workarea_y, &workarea_width, &workarea_height); | |||
printf("Monitor work area: %d, %d, %d, %d\n", workarea_x, workarea_y, workarea_width, workarea_height); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would probably be better in another example than the simple one, which is meant to be simple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved the example/test to monitors.c: dougbinks@a484f0d
Not only is no Wayland implementation possible at the moment, the usefulness of it would there be one is limited, as if you try to manually "maximize" (just set a size equal to the work area), the window would still not be maximized, nor moved to the top-left corner. To me, to make maximize work for undecorated windows, the only sane way would be to make glfwMaximizeWindow() do the right thing. Anything that tries to work around that function not being able to work as expected by bypassing the windowing system abstraction GLFW is trying to provide undoes exactly what GLFW is intended to do. |
Hello, Since the usefulness of this is being questioned above, may I humbly add that I also would very much like to have this API. My framework (same project as shown in #1236) creates pop-up windows that I position precisely relative to each others, with windows protrude out of the parent window. I currently use monitor information from GLFW to avoid creating popups that straddle multiple monitors but also want to avoid "task bars", this is one of the reason OS export the "work area" information. I can do this with Win32 and SDL2 ( For my use case this PR would perfectly solve the problem, and it is fine to have a fallback that return the entire monitor region on systems like Wayland that don't know of this concept yet. Regards, |
I concur with Omar that this is very useful outside of the now fixed For any platform without this concept the function can return coordinates from If the @ferreiradaselva does not have the time I can look into fixing any issues with the PR if the basic functionality requirements are approved. |
Hi, @dougbinks, feel free to work on this PR, or anyone interested! |
The fact that it cannot be made portable still remains however. What GLFW probably needs is a versatile popup placement API like the one GTK+ added. |
For various reasons I would probably be unable to rely on such specific "popup placement" feature. I want the most low-level feature that has a comparable implementation on all major frameworks. The concept of a work area rectangle per monitor is pretty well supported across frameworks and OS and has a sane fall back any other implementation can use (return the whole screen). |
The fallback is portable, and makes sense where the platform does not have such a concept. I don't think popups work for the multi-process window distribution problem, whereas the |
I have started work on resolving issues raised with this PR on my branch https://github.com/dougbinks/glfw/tree/pr-989. Either this can be merged into this PR branch by @ferreiradaselva when I'm done or I can submit a new PR. |
This has been merged via #1322 and additional fixes. |
This patch has the implementation for querying the work area on X11 and Windows.
The X11 implementation. I believe everything is done with this implementation. I tested with single monitor, but since the monitor is passed directly to the function, I don't think there will be any problem with it.
The Windows implementation. This implementation surely needs test on multiple monitors. I tested with a single monitor, and the work area retrieved was correct. However, we need to make sure that the
HMONITOR
that we grab the information from matches the_GLFWmonitor
passed to the function.The Cocoa implementation. The implementation for Cocoa is a draft and needs to be test and reviewed.
This patch is related to the issue #920.