Qt: Implement graphics window #5615

spycrab · 2017-06-15T23:12:26Z

Screenshots:

TODO:

Fix the windows build
Implement blocking options when a game is running
Fix that certain options aren't disabled when they should be
Implement Software Rendering dialogue
~~Implement Shader Configuration Window (Low priority; Separate PR?)~~ (Currently not testable on my end).

Testing:

~~Test if every element has a description~~
~~Check if there are any mistakes in those descriptions~~
~~Check if all options behave as they should~~ (All do except two; Though seemingly not the fault of this PR, has to be investigated)
~~Check if you can crash Dolphin by changing certain settings~~

spycrab · 2017-06-15T23:13:17Z

@ligfx @Starsam80 @MayImilae

ligfx · 2017-06-15T23:21:18Z

Could you break this into separate commits, please? 1,700 LOC is a lot to review at once :(

spycrab · 2017-06-15T23:22:26Z

@ligfx I would. Except for the fact that reverting past commits that are buried under commits you don't want to touch, is not something I have figured out yet.

Helios747 · 2017-06-15T23:31:29Z

Kinda bugs me that we went with a sidebar tab system for Config, but a topbar tab system for Graphics.

Ultimately I don't really care enough for this to block this PR, but grr.

spycrab · 2017-06-15T23:33:00Z

@Helios747 I discussed this on #dolphin-dev: Although there is a plan to integrate it into the Settings Dialog later down the line, that's not happening right now as we want Qt to get up to speed. (Atleast that was the consensus).

MayImilae · 2017-06-16T00:15:02Z

Some small nitpicks:

The backend/fullscreen res/aspect ratio etc settings are divided a bit differently than WX. And eeeh, it matches the other windows more so I don't really care. Just noting it.

The hacks tab Texture Cache setting is missing the "Accuracy:" line. It's kind of important to say what safe and fast are related to!

In the XFB section, please don't have XFB in the individual setting names, it's already stated at the top, and it's only there so users know what devs are talking about when they talk about XFB. Also, please place Virtual on the left, for the whole left is less right is more thing.

And if you are about to say that Texture Cache Accuracy has more accurate to the left... you're right! And it bugs me. It should be the other way around. But if you just copy what WX does or address it, it's fine. Just don't make it worse please!

@spycrab @Helios747 I don't think graphics should be integrated with the main settings, honestly, because divisions like that really help keep it from being intimidating. But that's an argument for another time!

Source/Core/DolphinQt2/Settings.cpp

+  return SConfig::GetInstance().bRenderToMain;
+}
+
+void Settings::SetRenderToMain(bool rendertomain)


Source/Core/DolphinQt2/Settings.cpp

+  return SConfig::GetInstance().bRenderWindowAutoSize;
+}
+
+void Settings::SetAutoadjustWindowSize(bool autoadjust)


Source/Core/DolphinQt2/Settings.h

@@ -70,10 +70,22 @@ class Settings final : public QSettings, NonCopyable

  // Graphics
  bool GetRenderToMain() const;
+  void SetRenderToMain(bool rendertomain);


Source/Core/DolphinQt2/Config/Graphics/GraphicsWidget.h

+public:
+  explicit GraphicsWidget(QWidget* parent);
+
+  bool eventFilter(QObject* obj, QEvent* event);


Source/Core/DolphinQt2/MainWindow.cpp

 #include <QDir>
 #include <QFileDialog>
 #include <QIcon>
 #include <QMessageBox>
-
+#include <qpa/qplatformnativeinterface.h>


Source/Core/DolphinQt2/Config/Graphics/GeneralWidget.cpp

+// We need QObject::tr
+#if defined(_WIN32)
+  static const QString TR_BACKEND_DESCRIPTION =
+      tr("Selects what graphics API to use internally.\nThe software renderer is extremely "


Source/Core/DolphinQt2/Config/Graphics/GeneralWidget.h

+  QCheckBox* m_show_fps;
+  QCheckBox* m_show_ping;
+  QCheckBox* m_log_rendertime;
+  QCheckBox* m_autoadjust_windowsize;


Source/Core/DolphinQt2/Config/Graphics/GeneralWidget.h

+  // Options
+  QCheckBox* m_show_fps;
+  QCheckBox* m_show_ping;
+  QCheckBox* m_log_rendertime;


Source/Core/DolphinQt2/Config/Graphics/GeneralWidget.h

+  QCheckBox* m_show_messages;
+  QCheckBox* m_keep_window_top;
+  QCheckBox* m_hide_cursor;
+  QCheckBox* m_render_mainwindow;


Source/Core/DolphinQt2/Config/Graphics/HacksWidget.cpp

+void HacksWidget::AddDescriptions()
+{
+  static const QString TR_SKIP_EFB_CPU_ACCESS_DESCRIPTION =
+      tr("Ignore any requests from the CPU to read from or write to the EFB.\nImproves "


Source/Core/VideoCommon/VideoUtils.cpp

@@ -0,0 +1,73 @@
+#include "VideoCommon/VideoUtils.h"


Source/Core/VideoCommon/VideoUtils.h

@@ -0,0 +1,15 @@
+#pragma once


Source/Core/VideoCommon/VideoUtils.h

+
+namespace VideoUtils
+{
+std::vector<std::string> GetAvailableResolutions(X11Utils::XRRConfiguration* xrr_config);


Source/Core/VideoCommon/VideoUtils.h

+namespace VideoUtils
+{
+std::vector<std::string> GetAvailableResolutions(X11Utils::XRRConfiguration* xrr_config);
+std::vector<std::string> GetAvailableAntialiasingModes(int& m_msaa_modes);


Source/Core/DolphinQt2/Settings.cpp

+  return SConfig::GetInstance().bRenderToMain;
+}
+
+void Settings::SetRenderToMain(bool rendertomain)


ghost · 2017-06-16T03:54:50Z

About textures, one small thing, keep in mind to make some space for the "Preload custom textures to VRAM" that I talked about with Stenzek, no I didn't mention to be an option at the time, but I can see it now that with bigger custom texture packs, it could be hitting over VRAM for some mid-to-lower end cards, certainly 3GB or lower of VRAM won't be enough for Hypatia's Zelda WW HD pack, which is at 3 GB (there's at least 150-300 MB VRAM overhead from browsers/desktop/etc)

This option would would be more suited to be where the "Prefetch custom Textures" (to RAM) is, but now I seem to not agree where it is currently in 5.0 - Advanced Tab in the Utilities group, for some reason it just doesn't agree with my taste, since "dumping textures" and wireframe mode are more developer oriented things, but using the custom textures is meant for end users so it just doesn't fit imo.

There should also be a debate wether to unify this "Prefetch custom textures to RAM/VRAM" from being two checkboxes to double radiobuttons in one line as in "Prefetch custom textures to: o RAM o VRAM"

Also the current description should be updated, since Native custom texture support is here, there is no more exponential memory requirement. The exponential part should be removed and the sentence rewritten.

Sidenote: "prefetching" more fits for usually system/OS cache stuff, the term "preload" may be more fitting as the textures become a solid part of the program and will never get flushed out in a session.

TODO: Need to resolve potential conflict with "Texture Cache" ...

Source/Core/DolphinQt2/MainWindow.cpp

+  m_graphics_window = new GraphicsWindow(
+      this,
+      new X11Utils::XRRConfiguration(
+          static_cast<Display*>(QGuiApplication::platformNativeInterface()->nativeResourceForWindow(


Source/Core/DolphinQt2/MainWindow.cpp

 #include <QDir>
 #include <QFileDialog>
 #include <QIcon>
 #include <QMessageBox>
-
+#include <qpa/qplatformnativeinterface.h>


Source/Core/DolphinQt2/Config/Graphics/GraphicsWindow.h

+{
+  Q_OBJECT
+public:
+  explicit GraphicsWindow(QWidget* parent, X11Utils::XRRConfiguration* xrr_config);


Source/Core/DolphinQt2/Config/Graphics/GraphicsWindow.cpp

+  main_layout->addWidget(m_tab_widget);
+  main_layout->addWidget(m_button_box);
+
+  auto* general_widget = new GeneralWidget(this, m_xrr_config);


Source/Core/DolphinQt2/Config/Graphics/GraphicsWindow.cpp

+  ConnectWidgets();
+
+  setWindowTitle(tr("Graphics"));
+  setWindowFlags(windowFlags() & ~Qt::WindowContextHelpButtonHint);


Source/Core/DolphinQt2/Config/Graphics/GraphicsSlider.h

@@ -0,0 +1,23 @@
+// Copyright 2017 Dolphin Emulator Project


Source/Core/DolphinQt2/Config/Graphics/GraphicsWidget.h

+class QGroupBox;
+class QLabel;
+
+class GraphicsWidget : public QWidget


MayImilae · 2017-06-16T05:17:53Z

@ZexaronS Please keep the bikeshedding away from Qt work, we're trying to get what we have in WX copied over to Qt, and bikeshedding subjective UI elements is a deathkneel for UI PRs!

spycrab · 2017-06-16T13:48:12Z

@MayImilae Implemented the changes you requested.

Source/Core/DolphinQt2/Config/Graphics/EnhancementsWidget.cpp

+void EnhancementsWidget::SaveSettings()
+{
+  bool is_ssaa = m_aa_combo->currentText().startsWith(QStringLiteral("SSAA"));
+  g_Config.iMultisamples = m_aa_combo->currentIndex() - (is_ssaa ? m_msaa_modes : 0);


Source/Core/DolphinQt2/Config/Graphics/EnhancementsWidget.cpp

+  bool is_ssaa = m_aa_combo->currentText().startsWith(QStringLiteral("SSAA"));
+  g_Config.iMultisamples = m_aa_combo->currentIndex() - (is_ssaa ? m_msaa_modes : 0);
+
+  g_Config.sPostProcessingShader = m_pp_effect->currentText().toStdString();


shuffle2 · 2017-06-27T18:45:12Z

Sorry looks like there's another conflict?

shuffle2 · 2017-06-27T19:40:38Z

per irc there is some disagreement...

mimimi085181 · 2017-06-27T20:35:35Z

Did you fix saving the wireframe option? It seems to be working now, and the info about it being broken could be removed. The only thing i don't like is the inconsistency on the spacing of the checkboxes, especially on the last tab. But that's not a blocker for me, it could be addressed later in another pr.

ligfx

Overall comments:

A more appropriate place for the BackendChanged signals is in a model like Settings, rather than connecting all the views correctly (which is prone to mistakes).
The XRRConfiguration stuff still looks weird to me. I don't think non-X11 platforms should have to pass around XRRConfiguration pointers, or even know they exist.
Signals sent from parent objects to children, like the EmulationStarted/Stopped signals, should be hooked up in the parents, not the children (happens in constructors).
Rows and indices for QGridLayout should start at 0.
Labels before comboboxes and sliders need to be suffixed with colons.
Lambdas that are used to drop arguments from signals can be replaced with direct pointer functions, since Qt knows how to drop arguments already.
The common pattern combobox->setCurrentIndex(combobox->findItem(text)) (or the variant, finding the item manually as part of a loop) can be replaced with combobox->setCurrentText.

Source/Core/DolphinQt2/Config/Graphics/AdvancedWidget.cpp

+
+  m_enable_cropping = new GraphicsBool(tr("Crop"), Config::GFX_CROP);
+  m_enable_prog_scan = new QCheckBox(tr("Enable Progressive Scan (Currently broken)"));
+  m_borderless_fullscreen =


Source/Core/DolphinQt2/Config/Graphics/AdvancedWidget.cpp

+{
+  connect(m_enable_prog_scan, &QCheckBox::toggled, this, &AdvancedWidget::SaveSettings);
+  connect(m_load_custom_textures, &QCheckBox::toggled, this, &AdvancedWidget::SaveSettings);
+  connect(m_enable_prog_scan, &QCheckBox::toggled, this, &AdvancedWidget::SaveSettings);


Source/Core/DolphinQt2/Config/Graphics/AdvancedWidget.cpp

+  misc_box->setLayout(misc_layout);
+
+  m_enable_cropping = new GraphicsBool(tr("Crop"), Config::GFX_CROP);
+  m_enable_prog_scan = new QCheckBox(tr("Enable Progressive Scan (Currently broken)"));


Source/Core/DolphinQt2/Config/Graphics/AdvancedWidget.cpp

+  SConfig::GetInstance().bProgressive = m_enable_prog_scan->isChecked();
+  const auto hires_enabled = Config::Get(Config::GFX_HIRES_TEXTURES);
+  m_prefetch_custom_textures->setEnabled(hires_enabled);
+  if (!hires_enabled)


Source/Core/DolphinQt2/Config/Graphics/AdvancedWidget.cpp

+
+void AdvancedWidget::OnBackendChanged()
+{
+  const auto supports_fr_framedumps = g_Config.backend_info.bSupportsInternalResolutionFrameDumps;


Source/Core/DolphinQt2/Config/Graphics/EnhancementsWidget.cpp

+
+void EnhancementsWidget::LoadSettings()
+{
+  m_block_save = true;


Source/Core/DolphinQt2/Config/Graphics/GraphicsWidget.cpp

+
+GraphicsWidget::GraphicsWidget(GraphicsWindow* parent)
+{
+  parent->RegisterWidget(this);


Source/Core/DolphinQt2/Config/Graphics/GraphicsWidget.cpp

+  parent->RegisterWidget(this);
+}
+
+void GraphicsWidget::AddDescription(QWidget* widget, const char* description)


Source/Core/DolphinQt2/Config/Graphics/GeneralWidget.cpp

+
+  // Video Section
+  auto* m_video_box = new QGroupBox(tr("Video"));
+  m_video_layout = new QFormLayout();


Source/Core/DolphinQt2/Config/Graphics/GraphicsWindow.cpp

+void GraphicsWindow::OnBackendChanged(const QString& backend)
+{
+  setWindowTitle(tr("Dolphin %1 Graphics Configuration").arg(backend));
+  if (backend == QStringLiteral("Software Renderer") && m_tab_widget->count() > 1)


Source/Core/DolphinQt2/Config/Graphics/AdvancedWidget.cpp

@@ -0,0 +1,208 @@
+// Copyright 2017 Dolphin5~ Emulator Project


sepalani

Feel free to ignore these comments if these modifications were done by Visual Studio itself. If so, I'll have to check my config. If not, revert them please otherwise they'll be reverted by anyone else using VS even if they aren't planning to modify these files.

Source/Core/DolphinQt2/DolphinQt2.vcxproj

@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>


Source/Core/VideoCommon/VideoCommon.vcxproj

@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>


Source/Core/VideoCommon/VideoCommon.vcxproj

shuffle2 · 2017-06-28T16:48:15Z

Lets just get this merged, and iterations on it can be done in successive prs? This is looking to become the victim of bike shedding.

Source/Core/DolphinQt2/Config/Graphics/EnhancementsWidget.cpp

@@ -0,0 +1,301 @@
+// Copyright 2015~7 Dolphin Emulator Project


spycrab force-pushed the qt_graphics branch from 4afbf05 to d4edca7 Compare June 15, 2017 23:20

spycrab force-pushed the qt_graphics branch from d4edca7 to 6e64236 Compare June 15, 2017 23:44

lioncash reviewed Jun 16, 2017

View reviewed changes

spycrab force-pushed the qt_graphics branch 2 times, most recently from 8d2eed6 to ec40213 Compare June 16, 2017 00:59

ligfx suggested changes Jun 16, 2017

View reviewed changes

spycrab force-pushed the qt_graphics branch 7 times, most recently from 4edf65a to d5fd043 Compare June 16, 2017 13:46

spycrab force-pushed the qt_graphics branch 3 times, most recently from 478a230 to 86fcf42 Compare June 16, 2017 17:16

merryhime reviewed Jun 16, 2017

View reviewed changes

spycrab force-pushed the qt_graphics branch from 86fcf42 to 8e3ab0e Compare June 16, 2017 21:29

spycrab force-pushed the qt_graphics branch from 7b01a21 to 76f154d Compare June 27, 2017 19:15

spycrab force-pushed the qt_graphics branch from 76f154d to 84796e1 Compare June 27, 2017 20:06

spycrab force-pushed the qt_graphics branch from 84796e1 to 776bb22 Compare June 27, 2017 20:40

ligfx suggested changes Jun 27, 2017

View reviewed changes

spycrab force-pushed the qt_graphics branch 2 times, most recently from e5d4b5c to eb30e29 Compare June 27, 2017 23:08

sepalani suggested changes Jun 27, 2017

View reviewed changes

Source/Core/DolphinQt2/Config/Graphics/AdvancedWidget.cpp Outdated

@@ -0,0 +1,208 @@

// Copyright 2017 Dolphin5~ Emulator Project

This comment was marked as off-topic.

Sign in to view

spycrab force-pushed the qt_graphics branch 2 times, most recently from 16a5865 to 26f4845 Compare June 28, 2017 12:16

sepalani suggested changes Jun 28, 2017

View reviewed changes

spycrab added 4 commits June 28, 2017 20:13

UICommon: Move Wx Graphics Settings functions

7c97c14

Qt: Add Graphics option to tool bar

1b1ab31

Qt: Implement graphics window and controls

908e1e9

Qt/GraphicsSettings: Implement advanced tab

b6690b2

spycrab force-pushed the qt_graphics branch from 26f4845 to 1ed8474 Compare June 28, 2017 18:27

sepalani reviewed Jun 28, 2017

View reviewed changes

Source/Core/DolphinQt2/Config/Graphics/EnhancementsWidget.cpp Outdated

@@ -0,0 +1,301 @@

// Copyright 2015~7 Dolphin Emulator Project

This comment was marked as off-topic.

Sign in to view

spycrab added 5 commits June 28, 2017 20:41

Qt/GraphicsSettings: Implement enhancements tab

e64a2d4

Qt/GraphicsSettings: Implement general tab

6598c23

Qt/GraphicsSettings: Implement software rendering tab

0b9f0d4

Qt/GraphicsSettings: Implement hacks tab

04bb6b3

Make Visual Studio happy

4d37190

spycrab force-pushed the qt_graphics branch from 1ed8474 to 4d37190 Compare June 28, 2017 18:42

shuffle2 merged commit c29c1f9 into dolphin-emu:master Jun 28, 2017

spycrab deleted the qt_graphics branch June 28, 2017 19:16

leoetlino modified the milestone: Qt Sep 13, 2017

		@@ -0,0 +1,208 @@
		// Copyright 2017 Dolphin5~ Emulator Project

		@@ -1,4 +1,4 @@
		<?xml version="1.0" encoding="utf-8"?>
		<?xml version="1.0" encoding="utf-8"?>

		@@ -0,0 +1,301 @@
		// Copyright 2015~7 Dolphin Emulator Project

Qt: Implement graphics window #5615

Qt: Implement graphics window #5615

Conversation

spycrab commented Jun 15, 2017 • edited Loading

spycrab commented Jun 15, 2017

ligfx commented Jun 15, 2017

spycrab commented Jun 15, 2017

Helios747 commented Jun 15, 2017

spycrab commented Jun 15, 2017

MayImilae commented Jun 16, 2017 • edited Loading

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

ghost commented Jun 16, 2017 • edited by ghost Loading

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

MayImilae commented Jun 16, 2017

spycrab commented Jun 16, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

shuffle2 commented Jun 27, 2017

shuffle2 commented Jun 27, 2017

mimimi085181 commented Jun 27, 2017

ligfx left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

spycrab commented Jun 15, 2017 •

edited

Loading

MayImilae commented Jun 16, 2017 •

edited

Loading

ghost commented Jun 16, 2017 •

edited by ghost

Loading

sepalani left a comment •

edited

Loading