-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Qt: Add display monitor selector #14196
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
base: master
Are you sure you want to change the base?
Changes from 3 commits
078b2c2
5310c9a
771078a
93364e2
ddacf90
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2549,6 +2549,11 @@ void MainWindow::createDisplayWidget(bool fullscreen, bool render_to_main) | |
| } | ||
|
|
||
| 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(); | ||
| m_display_surface->setScreen(target_screen); | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably should only perform this when
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right. I can move setScreen() in the fullscreen path. I've only tested on windows. |
||
| if (fullscreen || !render_to_main) | ||
| { | ||
| #ifdef DISPLAY_SURFACE_WINDOW | ||
|
|
@@ -2571,8 +2576,9 @@ void MainWindow::createDisplayWidget(bool fullscreen, bool render_to_main) | |
| // Other platforms can position windows fine, but the only thing that matters here is the screen. | ||
|
|
||
| #ifdef DISPLAY_SURFACE_WINDOW | ||
| if (isVisible() && g_emu_thread->shouldRenderToMain()) | ||
| m_display_surface->setPosition(screen()->availableGeometry().topLeft()); | ||
| m_display_surface->create(); | ||
| if (monitor_index > 0 || (isVisible() && g_emu_thread->shouldRenderToMain())) | ||
| m_display_surface->setPosition(target_screen->availableGeometry().topLeft()); | ||
| else | ||
| restoreDisplayWindowGeometryFromConfig(); | ||
|
|
||
|
|
@@ -2581,8 +2587,8 @@ void MainWindow::createDisplayWidget(bool fullscreen, bool render_to_main) | |
| else | ||
| m_display_surface->showNormal(); | ||
| #else | ||
| if (isVisible() && g_emu_thread->shouldRenderToMain()) | ||
| m_display_container->move(screen()->availableGeometry().topLeft()); | ||
| if (monitor_index > 0 || (isVisible() && g_emu_thread->shouldRenderToMain())) | ||
| m_display_container->move(target_screen->availableGeometry().topLeft()); | ||
| else | ||
| restoreDisplayWindowGeometryFromConfig(); | ||
|
|
||
|
|
@@ -2600,16 +2606,40 @@ void MainWindow::createDisplayWidget(bool fullscreen, bool render_to_main) | |
| else | ||
| restoreDisplayWindowGeometryFromConfig(); | ||
| m_display_surface->showNormal(); | ||
| 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)); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these changes made?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So 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.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
As such, I would assume the window position would be corrected, but I must admit I've never tested this.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| #else | ||
| if (m_is_temporarily_windowed && g_emu_thread->shouldRenderToMain()) | ||
| m_display_container->setGeometry(geometry()); | ||
| else | ||
| restoreDisplayWindowGeometryFromConfig(); | ||
| m_display_container->showNormal(); | ||
| 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)); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above |
||
| #endif | ||
| } | ||
| else | ||
| { | ||
| 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); | ||
| } | ||
|
Comment on lines
+2618
to
+2632
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above |
||
| pxAssertRel(m_ui.mainContainer->count() == 1, "Has no display widget"); | ||
| m_ui.mainContainer->addWidget(m_display_container); | ||
| m_ui.mainContainer->setCurrentIndex(1); | ||
|
|
@@ -2716,6 +2746,11 @@ void MainWindow::destroyDisplayWidget(bool show_game_list) | |
| { | ||
| pxAssertRel(m_ui.mainContainer->indexOf(m_display_container) == 1, "Display widget in stack"); | ||
| m_ui.mainContainer->removeWidget(m_display_container); | ||
| if (!m_pre_game_main_window_geometry.isEmpty()) | ||
| { | ||
| restoreGeometry(m_pre_game_main_window_geometry); | ||
| m_pre_game_main_window_geometry.clear(); | ||
| } | ||
| if (show_game_list) | ||
| { | ||
| m_ui.mainContainer->setCurrentIndex(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.
why
-1on the index? isn't the combobox index (which gets saved to the settings) zero-based?Uh oh!
There was an error while loading. Please reload this page.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, I missed the first
addItem()on line 126. That explains it, thanks.