Fix issues with the DisplayConfig dialog (#1066)

The DisplayConfig dialog had a few issues:
* The OK/Cancel buttons were not working as expected. The configuration
  options would be immediately written whenever some control values were
  changed. The configuration options are now properly being set on a
  click to OK, and ignored on a click to Cancel.
* The X button works again.
* There is now proper validation for the Display Scale option,
  validation ensures that the value is in the right range and incorrect
  data can no longer be input in that field.
* The Direct3D option is now properly enabled on Direct3D builds.

src/wx/config/option.cpp

@@ -483,6 +483,36 @@ bool Option::SetGbPalette(const wxString& value) {
     return true;
 }
 
+double Option::GetDoubleMin() const {
+    assert(is_double());
+    return nonstd::get<double>(min_);
+}
+
+double Option::GetDoubleMax() const {
+    assert(is_double());
+    return nonstd::get<double>(max_);
+}
+
+int32_t Option::GetIntMin() const {
+    assert(is_int());
+    return nonstd::get<int32_t>(min_);
+}
+
+int32_t Option::GetIntMax() const {
+    assert(is_int());
+    return nonstd::get<int32_t>(max_);
+}
+
+uint32_t Option::GetUnsignedMin() const {
+    assert(is_unsigned());
+    return nonstd::get<uint32_t>(min_);
+}
+
+uint32_t Option::GetUnsignedMax() const {
+    assert(is_unsigned());
+    return nonstd::get<uint32_t>(max_);
+}
+
 void Option::NextFilter() {
     assert(is_filter());
     const int old_value = static_cast<int>(GetFilter());

src/wx/config/option.h

@@ -192,6 +192,14 @@ public:
     bool SetEnumString(const wxString& value);
     bool SetGbPalette(const wxString& value);
 
+    // Min/Max accessors.
+    double GetDoubleMin() const;
+    double GetDoubleMax() const;
+    int32_t GetIntMin() const;
+    int32_t GetIntMax() const;
+    uint32_t GetUnsignedMin() const;
+    uint32_t GetUnsignedMax() const;
+
     // Special convenience modifiers.
     void NextFilter();
     void NextInterframe();

src/wx/dialogs/display-config.cpp

@@ -11,6 +11,8 @@
 #include <wx/spinctrl.h>
 #include <wx/stdpaths.h>
 #include <wx/textctrl.h>
+#include <wx/valnum.h>
+
 #include <wx/xrc/xmlres.h>
 
 #include "config/option.h"
@@ -25,57 +27,60 @@ namespace dialogs {
 
 namespace {
 
-// Validator for a wxTextCtrl with a double value.
+// Custom validator for the kDispScale option. We rely on the existing
-class ScaleValidator : public widgets::OptionValidator {
+// wxFloatingPointValidator validator for this.
+class ScaleValidator : public wxFloatingPointValidator<double>,
+                       public config::Option::Observer {
 public:
-    ScaleValidator() : widgets::OptionValidator(config::OptionID::kDispScale) {}
+    ScaleValidator()
+        : wxFloatingPointValidator<double>(&value_),
+          config::Option::Observer(config::OptionID::kDispScale),
+          value_(option()->GetDouble()) {
+        // Let wxFloatingPointValidator do the hard work.
+        SetMin(option()->GetDoubleMin());
+        SetMax(option()->GetDoubleMax());
+        SetPrecision(1);
+    }
     ~ScaleValidator() override = default;
 
-private:
+    // Returns a copy of the object.
-    // OptionValidator implementation.
     wxObject* Clone() const override { return new ScaleValidator(); }
 
-    bool IsWindowValueValid() override {
+private:
-        double value;
+    // wxValidator implementation.
-        if (!wxDynamicCast(GetWindow(), wxTextCtrl)
+    bool TransferFromWindow() final {
-                 ->GetValue()
+        if (!wxFloatingPointValidator<double>::TransferFromWindow()) {
-                 .ToDouble(&value)) {
+            wxLogWarning(_("Invalid value for Default magnification."));
             return false;
         }
-
+        return option()->SetDouble(value_);
-        return true;
     }
 
-    bool WriteToWindow() override {
+    bool TransferToWindow() final {
-        wxDynamicCast(GetWindow(), wxTextCtrl)
+        value_ = option()->GetDouble();
-            ->SetValue(wxString::Format("%.1f", option()->GetDouble()));
+        return wxFloatingPointValidator<double>::TransferToWindow();
-        return true;
+    };
-    }
 
-    bool WriteToOption() override {
+#if WX_HAS_VALIDATOR_SET_WINDOW_OVERRIDE
-        double value;
+    void SetWindow(wxWindow* window) final {
-        if (!wxDynamicCast(GetWindow(), wxTextCtrl)
+        wxFloatingPointValidator<double>::SetWindow(window);
-                 ->GetValue()
-                 .ToDouble(&value)) {
-            return false;
-        }
-
-        return option()->SetDouble(value);
     }
+#endif
+
+    // config::Option::Observer implementation.
+    void OnValueChanged() final { TransferToWindow(); }
+
+    // A local value used for configuration.
+    double value_;
 };
 
 // Validator for a wxChoice with a Filter value.
 class FilterValidator : public widgets::OptionValidator {
 public:
-    FilterValidator() : OptionValidator(config::OptionID::kDispFilter) {
+    FilterValidator() : OptionValidator(config::OptionID::kDispFilter) {}
-        Bind(wxEVT_CHOICE, &FilterValidator::OnChoice, this);
-    }
     ~FilterValidator() override = default;
 
 private:
-    // wxChoice event handler.
-    void OnChoice(wxCommandEvent&) { WriteToOption(); }
-
     // OptionValidator implementation.
     wxObject* Clone() const override { return new FilterValidator(); }
 
@@ -105,15 +110,10 @@ private:
 // Validator for a wxChoice with an Interframe value.
 class InterframeValidator : public widgets::OptionValidator {
 public:
-    InterframeValidator() : OptionValidator(config::OptionID::kDispIFB) {
+    InterframeValidator() : OptionValidator(config::OptionID::kDispIFB) {}
-        Bind(wxEVT_CHOICE, &InterframeValidator::OnChoice, this);
-    }
     ~InterframeValidator() override = default;
 
 private:
-    // wxChoice event handler.
-    void OnChoice(wxCommandEvent&) { WriteToOption(); }
-
     // OptionValidator implementation.
     wxObject* Clone() const override { return new InterframeValidator(); }
 
@@ -148,14 +148,10 @@ public:
         : OptionValidator(config::OptionID::kDispRenderMethod),
           render_method_(render_method) {
         assert(render_method != config::RenderMethod::kLast);
-        Bind(wxEVT_RADIOBUTTON, &RenderValidator::OnRadioButton, this);
     }
     ~RenderValidator() override = default;
 
 private:
-    // wxRadioButton event handler.
-    void OnRadioButton(wxCommandEvent&) { WriteToOption(); }
-
     // OptionValidator implementation.
     wxObject* Clone() const override {
         return new RenderValidator(render_method_);
@@ -307,11 +303,18 @@ DisplayConfig::DisplayConfig(wxWindow* parent)
         ->SetValidator(RenderValidator(config::RenderMethod::kOpenGL));
 #endif  // NO_OGL
 
-    // Direct3D is not implemented so hide the option on every platform.
+#if defined(__WXMSW__) && !defined(NO_D3D)
+    // Enable the Direct3D option on Windows.
+    GetValidatedChild(this, "OutputDirect3D")
+        ->SetValidator(RenderValidator(config::RenderMethod::kDirect3d));
+#else
     GetValidatedChild(this, "OutputDirect3D")->Hide();
+#endif
 
     filter_selector_ = GetValidatedChild<wxChoice>(this, "Filter");
     filter_selector_->SetValidator(FilterValidator());
+    filter_selector_->Bind(wxEVT_CHOICE, &DisplayConfig::UpdatePlugin, this,
+                           GetId());
 
     // These are filled and/or hidden at dialog load time.
     plugin_label_ = GetValidatedChild<wxControl>(this, "PluginLab");
@@ -320,14 +323,21 @@ DisplayConfig::DisplayConfig(wxWindow* parent)
     interframe_selector_ = GetValidatedChild<wxChoice>(this, "IFB");
     interframe_selector_->SetValidator(InterframeValidator());
 
-    Bind(wxEVT_SHOW, &DisplayConfig::OnDialogShown, this, GetId());
+    Bind(wxEVT_SHOW, &DisplayConfig::OnDialogShowEvent, this, GetId());
-    Bind(wxEVT_CLOSE_WINDOW, &DisplayConfig::OnDialogClosed, this, GetId());
 
     // Finally, fit everything nicely.
     Fit();
 }
 
-void DisplayConfig::OnDialogShown(wxShowEvent&) {
+void DisplayConfig::OnDialogShowEvent(wxShowEvent& event) {
+    if (event.IsShown()) {
+        PopulatePluginOptions();
+    } else {
+        StopPluginHandler();
+    }
+}
+
+void DisplayConfig::PopulatePluginOptions() {
     // Populate the plugin values, if any.
     wxArrayString plugins;
     const wxString plugin_path = wxGetApp().GetPluginsDir();
@@ -387,17 +397,21 @@ void DisplayConfig::OnDialogShown(wxShowEvent&) {
     Fit();
 }
 
-void DisplayConfig::OnDialogClosed(wxCloseEvent&) {
+void DisplayConfig::StopPluginHandler() {
-    // Reset the validator to stop handling events while this dialog is not
+    // Reset the validator to stop handling events while this dialog is hidden.
-    // shown.
     plugin_selector_->SetValidator(wxValidator());
 }
 
+void DisplayConfig::UpdatePlugin(wxCommandEvent& event) {
+    const bool is_plugin = (static_cast<config::Filter>(event.GetSelection()) ==
+                            config::Filter::kPlugin);
+    plugin_label_->Enable(is_plugin);
+    plugin_selector_->Enable(is_plugin);
+}
+
 void DisplayConfig::OnFilterChanged(config::Option* option) {
     const config::Filter option_filter = option->GetFilter();
     const bool is_plugin = (option_filter == config::Filter::kPlugin);
-    plugin_label_->Enable(is_plugin);
-    plugin_selector_->Enable(is_plugin);
 
     // Plugin needs to be handled separately, in case it was removed. The panel
     // will eventually reset if it can't actually use the plugin.

src/wx/dialogs/display-config.h

@@ -29,11 +29,17 @@ private:
     // owner, `parent` is destroyed. This prevents accidental deletion.
     DisplayConfig(wxWindow* parent);
 
+    // Handler for the wxEVT_SHOW event.
+    void OnDialogShowEvent(wxShowEvent& event);
+
     // Populates the plugin options.
-    void OnDialogShown(wxShowEvent&);
+    void PopulatePluginOptions();
 
     // Stops handling the plugin options.
-    void OnDialogClosed(wxCloseEvent&);
+    void StopPluginHandler();
+
+    // Update the plugin display.
+    void UpdatePlugin(wxCommandEvent& event);
 
     // Displays the new filter name on the screen.
     void OnFilterChanged(config::Option* option);