Conversation
Under the Interface setting, a user can now select which monitor a game will launch to. This setting works with launching in full screen and launching in a separate window.
In some cases, the game might render on a different monitor then the game. When the game ends, the main window will now return to the correct monitor, if needed.
|
Fill out our PR template. |
e40df2d to
771078a
Compare
|
Please rebase instead of merging |
| m_display_surface = new DisplaySurface(); | ||
| const int monitor_index = Host::GetBaseIntSettingValue("UI", "DisplayMonitor", 0); | ||
| const QList<QScreen*> screens = QGuiApplication::screens(); | ||
| QScreen* target_screen = (monitor_index > 0 && monitor_index <= screens.size()) ? screens[monitor_index - 1] : QGuiApplication::primaryScreen(); |
There was a problem hiding this comment.
why -1 on the index? isn't the combobox index (which gets saved to the settings) zero-based?
There was a problem hiding this comment.
Because the default monitor takes up two slots in the combo box (default and monitor 1, in this case monitor 1 is my default). For example, monitor 2 is index 2 in the combo box but monitor 2 is in index 1 in what Qt returns in screens. That's why I subtract 1.
In my setup, my combobox looks like: Default, Monitor 1, Monitor 2. Default is always at index 0.
There was a problem hiding this comment.
ah, I missed the first addItem() on line 126. That explains it, thanks.
There was a problem hiding this comment.
I have not tested, and have only skim read the pr code, so bear with if I've misunderstood something.
Strictly speaking, PCSX2 should fullscreen on the same monitor that the window currently exists on. Nevertheless, having explicit control could be useful.
This PR, however, seems to always override that logic, as it forces the primary screen when no screen is specified.
Your handling of render to separate (where render_to_main is false) appears incorrect.
You aren’t adjusting the separate window position in the fullscreen case, instead seemingly adjusting it only when fullscreen is false.
You do, however, have added additional code to restore the window position. this is probably plastering over the above issue.
What platform have you tested your changes on?
pcsx2-qt/MainWindow.cpp
Outdated
| if (monitor_index > 0) | ||
| { | ||
| const QSize windowSize = m_display_surface->size(); | ||
| const QRect screenGeo = target_screen->availableGeometry(); | ||
| const QPoint center(screenGeo.x() + (screenGeo.width() - windowSize.width()) / 2, | ||
| screenGeo.y() + (screenGeo.height() - windowSize.height()) / 2); | ||
| m_display_surface->setGeometry(QRect(center, windowSize)); | ||
| } |
There was a problem hiding this comment.
Why are these changes made?
This section of code looks like it would only run when fullscreen is false
There was a problem hiding this comment.
When I render to a separate window, I want to make sure the window is at good position on the screen. I've had cases where the window would open on a different monitor and the title bar is off screen and I couldn't move it around. Originally, I had it open on the top left, that's when I discovered the issue.
Putting the separate window on the center of the screen makes sure this does not happen. I don't do this with Fullscreen because accidentally making the title bar off screen is not really an issue.
What I can add, once the window is open at a safe position, the user can move the window around and it can save that position for the session so every time a separate windowed window is launched it will be in that position.
I can also fix it so if it is windowed, using the main window, and already at the desired monitor, I can skip re-centering. I know it's at a safe location already.
There was a problem hiding this comment.
So restoreDisplayWindowGeometryFromConfig() ends up restoring the window offscreen?
Ideally restoreDisplayWindowGeometryFromConfig() or the associated Qt functions it calls would handle this.
What situations did you have the window appear offscreen?
If correcting the position does prove to be necessary, we should verify that the window is offscreen before repositioning it.
Code for saving and loading the window position already exists and is in use.
You, however, are currently overriding the restored position when you are centring the window.
There was a problem hiding this comment.
restoreDisplayWindowGeometryFromConfig() returns the saved screen coordinates on whatever monitor it was last on, right? What happens if my display set up changes? My ini files are on the cloud so I'm not always launching pcsx2 on the same device.
We can check if a window is off screen and recenter it when it is not. I think we can use Qt's contains function.
There was a problem hiding this comment.
restoreDisplayWindowGeometryFromConfig() will use Qt's restoreGeomatry() function.
Qt documents restoreGeometry() to adjust the window position if the saved geometry was offscreen.
See Qt's documentation.
As such, I would assume the window position would be corrected, but I must admit I've never tested this.
There was a problem hiding this comment.
I updated restoreDisplayWindowGeometryFromConfig() and tested restoreGeometry(). When I load from the config and the geometry is not within the target screen, I have to get the geometry from the target screen to pass it to restoreGeometry(), but if I do that, I have to saveGeometry() to get the target screen's geometry which requires me choosing a position anyways, and the center of the screen is still the safest position to save.
Anyways, I still do use restoreDisplayWindowGeometryFromConfig() and cleaned it up a bit. This way it also saves the position of the separate window when it matches the target screen within and across different sessions.
pcsx2-qt/MainWindow.cpp
Outdated
| if (monitor_index > 0) | ||
| { | ||
| const QSize windowSize = m_display_container->size(); | ||
| const QRect screenGeo = target_screen->availableGeometry(); | ||
| const QPoint center(screenGeo.x() + (screenGeo.width() - windowSize.width()) / 2, | ||
| screenGeo.y() + (screenGeo.height() - windowSize.height()) / 2); | ||
| m_display_container->setGeometry(QRect(center, windowSize)); | ||
| } |
| if (monitor_index > 0) | ||
| { | ||
| m_pre_game_main_window_geometry = saveGeometry(); | ||
| const QRect screenGeo = target_screen->availableGeometry(); | ||
| const QPoint center(screenGeo.x() + (screenGeo.width() - width()) / 2, | ||
| screenGeo.y() + (screenGeo.height() - height()) / 2); | ||
| move(center); | ||
| } |
pcsx2-qt/MainWindow.cpp
Outdated
| const int monitor_index = Host::GetBaseIntSettingValue("UI", "DisplayMonitor", 0); | ||
| const QList<QScreen*> screens = QGuiApplication::screens(); | ||
| QScreen* target_screen = (monitor_index > 0 && monitor_index <= screens.size()) ? screens[monitor_index - 1] : QGuiApplication::primaryScreen(); | ||
| m_display_surface->setScreen(target_screen); | ||
|
|
There was a problem hiding this comment.
Probably should only perform this when fullscreen is true.
I also found that setScreen() may also be ineffective.
positioning the window (for which you've adjusted/added code for) proved more reliable in my testing.
There was a problem hiding this comment.
You are right. I can move setScreen() in the fullscreen path.
I've only tested on windows.
If windowed, rendering to main window, and already at the target screen, the window will not be recentered.
Now within and across different sessions, the position of the seperate windowed screen is saved is used if it matches the target screen. When using the main window to render and the target screen is on another display, that position is now saved within the session.
|
Adding that, this doesn't work on Wayland. |
I don't have wayland to test it out right now, but what doesn't work specifically? |
The entire PR, in which to be fair is caused by limiations of Wayland itself (It doesn't allow the application itself to set their own window position). I haven't checked X11 but it theoretically can work there. |
Wayland seems to allow apps to pick the screen they show up on. (at least that's how it seems to behave under weston) |
|
So I should make sure to setScreen whenever possible for Wayland before any repositioning? Wayland will just ignore all the other repositioning logic but setScreen should, at least, put the window on the right screen. |
|
I created a Wayland VM to test this. When starting in fullscreen, Wayland respects the selected monitor. However, all windowed mode options are controlled by the compositor, which ignores the selected monitor. Even setScreen appears to be ignored. I tried a few workarounds, such as starting in fullscreen and then switching to windowed mode, but that did not work. It seems like this would require rendering at least one frame first before switching, which adds a fair amount of complexity just to support monitor selection in windowed mode on Wayland. |
Testing this myself, this seems to work for me under both WSL, and a manjaro VM |
Description of Changes
Adds a display monitor selector to the Qt frontend, allowing users to choose which monitor PCSX2 opens on. Also restores the main window position when a game shuts down.
Rationale Behind Changes
Users with multi-monitor setups previously had no control over which display PCSX2 opened on. This change provides explicit control and ensures the window position is remembered per monitor.
Suggested Testing Steps
Did you use AI to help find, test, or implement this issue or feature?
Yes. I am unfamiliar with Qt, so I used AI to help learn it during implementation.