CompoundEditor : Fix maximizing detached windows#6201
CompoundEditor : Fix maximizing detached windows#6201ericmehl wants to merge 1 commit intoGafferHQ:1.5_maintenancefrom
Conversation
Qt can only `setScreen()` on the top-level Window, which meant that layouts with detached windows that were set to maximized would not be moved to a non-primary screen. By setting the window geometry based on the desired screen, before maximizing, the detached windows get maximized on the right screen.
johnhaddon
left a comment
johnhaddon
left a comment
There was a problem hiding this comment.
Thanks for taking this on Eric - it will be good to have it fixed. I can't test as I only have the one monitor, but I've made some comments based on what I see in the code and the feedback on the PR build in Discord.
Qt can only setScreen() on the top-level Window
I don't see how that is relevant here - detached panels are top-level windows aren't they?
| # \todo In Qt6, `setScreen()` is added to `QWidget` which we should | ||
| # be able to use to set the screen including for detached windows. | ||
|
|
||
| window.setWindowState( QtCore.Qt.WindowNoState ) | ||
|
|
||
| screenGeom = targetScreen.availableGeometry() | ||
| geometry = window.geometry() | ||
| window.setGeometry( screenGeom.x(), screenGeom.y(), geometry.width(), geometry.height() ) |
There was a problem hiding this comment.
If this is a fix related to maximisation, then perhaps it belongs down below in the elif boundData["maximized"] block? It seems that could also fix the problem reported on Discord, where this code isn't being run for the primary screen.
There's a bit of an assumption here when doing window.setGeometry() that the target screen will have at least enough resolution to hold the originally created window.
Alternatively, perhaps we should always restore the geometry (our final else : clause below) and then maximise? From the looks of it, the code for restoring the geometry is smart enough not to overshoot the screeen.
There was a problem hiding this comment.
If this is a fix related to maximisation, then perhaps it belongs down below in the elif boundData["maximized"] block?
If we take this approach, it might be worth double-checking that the same issue doesn't apply for the boundData["fullscreen"] case, it may need the same special handling as maximisation to ensure we end up fullscreen on the correct screen? If so, that's likely a vote for always restoring the geometry...
3 participants
Qt can only
setScreen()on the top-level Window (https://doc.qt.io/qt-5/qwindow.html#setScreen), which means that layouts with detached windows that set to maximized would not be moved to a non-primary screen. By setting the window geometry based on the desired screen, before maximizing, the detached windows get maximized on the right screen.There's a bit of an assumption here when doing
window.setGeometry()that the target screen will have at least enough resolution to hold the originally created window. From what I've seen so far, if the screen can't hold the dimensions requested, at worst you get a warning on the terminal that the window is being resized.Checklist