From a1f19b61f84f709ba20c350da34956c5455a6956 Mon Sep 17 00:00:00 2001 From: lat9nq <22451773+lat9nq@users.noreply.github.com> Date: Fri, 23 Jul 2021 12:41:07 -0400 Subject: [PATCH 1/4] settings: Implement setting ranges Clamps the setting's values against the specified minimum and maximum values. --- src/common/settings.h | 170 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 152 insertions(+), 18 deletions(-) diff --git a/src/common/settings.h b/src/common/settings.h index cfc1ab46f7..51f9a179b3 100644 --- a/src/common/settings.h +++ b/src/common/settings.h @@ -141,6 +141,67 @@ protected: const std::string label{}; ///< The setting's label }; +/** + * BasicRangedSetting class is intended for use with quantifiable settings that need a more + * restrictive range than implicitly defined by its type. Implements a minimum and maximum that is + * simply used to sanitize SetValue and the assignment overload. + */ +template +class BasicRangedSetting : virtual public BasicSetting { +public: + /** + * Sets a default value, minimum value, maximum value, and label. + * + * @param default_val Intial value of the setting, and default value of the setting + * @param min_val Sets the minimum allowed value of the setting + * @param max_val Sets the maximum allowed value of the setting + * @param name Label for the setting + */ + explicit BasicRangedSetting(const Type& default_val, const Type& min_val, const Type& max_val, + const std::string& name) + : BasicSetting{default_val, name}, minimum{min_val}, maximum{max_val} {} + ~BasicRangedSetting() = default; + + /** + * Like BasicSetting's SetValue, except value is clamped to the range of the setting. + * + * @param value The desired value + */ + void SetValue(const Type& value) { + Type temp; + if (value < minimum) { + temp = std::move(minimum); + } else if (value > maximum) { + temp = std::move(maximum); + } else { + temp = std::move(value); + } + std::swap(this->global, temp); + } + + /** + * Like BasicSetting's assignment overload, except value is clamped to the range of the setting. + * + * @param value The desired value + * @returns A reference to the setting's value + */ + const Type& operator=(const Type& value) { + Type temp; + if (value < minimum) { + temp = std::move(minimum); + } else if (value > maximum) { + temp = std::move(maximum); + } else { + temp = std::move(value); + } + std::swap(this->global, temp); + return this->global; + } + + const Type minimum; ///< Minimum allowed value of the setting + const Type maximum; ///< Maximum allowed value of the setting +}; + /** * The Setting class is a slightly more complex version of the BasicSetting class. This adds a * custom setting to switch to when a guest application specifically requires it. The effect is that @@ -152,7 +213,7 @@ protected: * Like the BasicSetting, this requires setting a default value and label to use. */ template -class Setting final : public BasicSetting { +class Setting : virtual public BasicSetting { public: /** * Sets a default value, label, and setting value. @@ -241,11 +302,80 @@ public: return custom; } -private: +protected: bool use_global{true}; ///< The setting's global state Type custom{}; ///< The custom value of the setting }; +/** + * RangedSetting is a Setting that implements a maximum and minimum value for its setting. Intended + * for use with quantifiable settings. + */ +template +class RangedSetting final : public BasicRangedSetting, public Setting { +public: + /** + * Sets a default value, minimum value, maximum value, and label. + * + * @param default_val Intial value of the setting, and default value of the setting + * @param min_val Sets the minimum allowed value of the setting + * @param max_val Sets the maximum allowed value of the setting + * @param name Label for the setting + */ + explicit RangedSetting(const Type& default_val, const Type& min_val, const Type& max_val, + const std::string& name) + : BasicSetting{default_val, name}, + BasicRangedSetting{default_val, min_val, max_val, name}, Setting{default_val, + name} {} + ~RangedSetting() = default; + + /** + * Like BasicSetting's SetValue, except value is clamped to the range of the setting. Sets the + * appropriate value depending on the global state. + * + * @param value The desired value + */ + void SetValue(const Type& value) { + Type temp; + if (value < this->minimum) { + temp = std::move(this->minimum); + } else if (value > this->maximum) { + temp = std::move(this->maximum); + } else { + temp = std::move(value); + } + if (this->use_global) { + std::swap(this->global, temp); + } else { + std::swap(this->custom, temp); + } + } + + /** + * Like BasicSetting's assignment overload, except value is clamped to the range of the setting. + * Uses the appropriate value depending on the global state. + * + * @param value The desired value + * @returns A reference to the setting's value + */ + const Type& operator=(const Type& value) { + Type temp; + if (value < this->minimum) { + temp = std::move(this->minimum); + } else if (value > this->maximum) { + temp = std::move(this->maximum); + } else { + temp = std::move(value); + } + if (this->use_global) { + std::swap(this->global, temp); + return this->global; + } + std::swap(this->custom, temp); + return this->custom; + } +}; + /** * The InputSetting class allows for getting a reference to either the global or custom members. * This is required as we cannot easily modify the values of user-defined types within containers @@ -289,13 +419,14 @@ struct Values { BasicSetting sink_id{"auto", "output_engine"}; BasicSetting audio_muted{false, "audio_muted"}; Setting enable_audio_stretching{true, "enable_audio_stretching"}; - Setting volume{100, "volume"}; + RangedSetting volume{100, 0, 100, "volume"}; // Core Setting use_multi_core{true, "use_multi_core"}; // Cpu - Setting cpu_accuracy{CPUAccuracy::Auto, "cpu_accuracy"}; + RangedSetting cpu_accuracy{CPUAccuracy::Auto, CPUAccuracy::Auto, + CPUAccuracy::Unsafe, "cpu_accuracy"}; // TODO: remove cpu_accuracy_first_time, migration setting added 8 July 2021 BasicSetting cpu_accuracy_first_time{true, "cpu_accuracy_first_time"}; BasicSetting cpu_debug_mode{false, "cpu_debug_mode"}; @@ -317,7 +448,8 @@ struct Values { Setting cpuopt_unsafe_fastmem_check{true, "cpuopt_unsafe_fastmem_check"}; // Renderer - Setting renderer_backend{RendererBackend::OpenGL, "backend"}; + RangedSetting renderer_backend{ + RendererBackend::OpenGL, RendererBackend::OpenGL, RendererBackend::Vulkan, "backend"}; BasicSetting renderer_debug{false, "debug"}; BasicSetting enable_nsight_aftermath{false, "nsight_aftermath"}; BasicSetting disable_shader_loop_safety_checks{false, @@ -327,26 +459,28 @@ struct Values { Setting resolution_factor{1, "resolution_factor"}; // *nix platforms may have issues with the borderless windowed fullscreen mode. // Default to exclusive fullscreen on these platforms for now. - Setting fullscreen_mode{ + RangedSetting fullscreen_mode{ #ifdef _WIN32 FullscreenMode::Borderless, #else FullscreenMode::Exclusive, #endif - "fullscreen_mode"}; - Setting aspect_ratio{0, "aspect_ratio"}; - Setting max_anisotropy{0, "max_anisotropy"}; + FullscreenMode::Borderless, FullscreenMode::Exclusive, "fullscreen_mode"}; + RangedSetting aspect_ratio{0, 0, 3, "aspect_ratio"}; + RangedSetting max_anisotropy{0, 0, 4, "max_anisotropy"}; Setting use_speed_limit{true, "use_speed_limit"}; - Setting speed_limit{100, "speed_limit"}; + RangedSetting speed_limit{100, 0, 9999, "speed_limit"}; Setting use_disk_shader_cache{true, "use_disk_shader_cache"}; - Setting gpu_accuracy{GPUAccuracy::High, "gpu_accuracy"}; + RangedSetting gpu_accuracy{GPUAccuracy::High, GPUAccuracy::Normal, + GPUAccuracy::Extreme, "gpu_accuracy"}; Setting use_asynchronous_gpu_emulation{true, "use_asynchronous_gpu_emulation"}; Setting use_nvdec_emulation{true, "use_nvdec_emulation"}; Setting accelerate_astc{true, "accelerate_astc"}; Setting use_vsync{true, "use_vsync"}; - BasicSetting fps_cap{1000, "fps_cap"}; + BasicRangedSetting fps_cap{1000, 1, 1000, "fps_cap"}; BasicSetting disable_fps_limit{false, "disable_fps_limit"}; - Setting shader_backend{ShaderBackend::GLASM, "shader_backend"}; + RangedSetting shader_backend{ShaderBackend::GLASM, ShaderBackend::GLSL, + ShaderBackend::SPIRV, "shader_backend"}; Setting use_asynchronous_shaders{false, "use_asynchronous_shaders"}; Setting use_fast_gpu_time{true, "use_fast_gpu_time"}; Setting use_caches_gc{false, "use_caches_gc"}; @@ -363,10 +497,10 @@ struct Values { std::chrono::seconds custom_rtc_differential; BasicSetting current_user{0, "current_user"}; - Setting language_index{1, "language_index"}; - Setting region_index{1, "region_index"}; - Setting time_zone_index{0, "time_zone_index"}; - Setting sound_index{1, "sound_index"}; + RangedSetting language_index{1, 0, 16, "language_index"}; + RangedSetting region_index{1, 0, 6, "region_index"}; + RangedSetting time_zone_index{0, 0, 45, "time_zone_index"}; + RangedSetting sound_index{1, 0, 2, "sound_index"}; // Controls InputSetting> players; @@ -383,7 +517,7 @@ struct Values { "udp_input_servers"}; BasicSetting mouse_panning{false, "mouse_panning"}; - BasicSetting mouse_panning_sensitivity{10, "mouse_panning_sensitivity"}; + BasicRangedSetting mouse_panning_sensitivity{10, 1, 100, "mouse_panning_sensitivity"}; BasicSetting mouse_enabled{false, "mouse_enabled"}; std::string mouse_device; MouseButtonsRaw mouse_buttons; From 7737bdfd1ac2868397b94ba26a9ccf06ea1dfcba Mon Sep 17 00:00:00 2001 From: lat9nq <22451773+lat9nq@users.noreply.github.com> Date: Fri, 30 Jul 2021 13:33:35 -0400 Subject: [PATCH 2/4] settings: Fix function virtualization Fixes a theoretical scenario where a Setting is using the BasicSetting's GetValue function. In practice this probably only happens on yuzu-cmd, where there is no need for a Setting's additional features. Need to fix regardless. --- src/common/settings.h | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/common/settings.h b/src/common/settings.h index 51f9a179b3..f54705a96e 100644 --- a/src/common/settings.h +++ b/src/common/settings.h @@ -81,7 +81,7 @@ public: * * @returns A reference to the setting */ - [[nodiscard]] const Type& GetValue() const { + [[nodiscard]] virtual const Type& GetValue() const { return global; } @@ -90,7 +90,7 @@ public: * * @param value The desired value */ - void SetValue(const Type& value) { + virtual void SetValue(const Type& value) { Type temp{value}; std::swap(global, temp); } @@ -120,7 +120,7 @@ public: * * @returns A reference to the setting */ - const Type& operator=(const Type& value) { + virtual const Type& operator=(const Type& value) { Type temp{value}; std::swap(global, temp); return global; @@ -131,7 +131,7 @@ public: * * @returns A reference to the setting */ - explicit operator const Type&() const { + explicit virtual operator const Type&() const { return global; } @@ -167,7 +167,7 @@ public: * * @param value The desired value */ - void SetValue(const Type& value) { + void SetValue(const Type& value) override { Type temp; if (value < minimum) { temp = std::move(minimum); @@ -185,7 +185,7 @@ public: * @param value The desired value * @returns A reference to the setting's value */ - const Type& operator=(const Type& value) { + const Type& operator=(const Type& value) override { Type temp; if (value < minimum) { temp = std::move(minimum); @@ -252,7 +252,13 @@ public: * * @returns The required value of the setting */ - [[nodiscard]] const Type& GetValue(bool need_global = false) const { + [[nodiscard]] const Type& GetValue() const override { + if (use_global) { + return this->global; + } + return custom; + } + [[nodiscard]] const Type& GetValue(bool need_global) const { if (use_global || need_global) { return this->global; } @@ -264,7 +270,7 @@ public: * * @param value The new value */ - void SetValue(const Type& value) { + void SetValue(const Type& value) override { Type temp{value}; if (use_global) { std::swap(this->global, temp); @@ -280,7 +286,7 @@ public: * * @returns A reference to the current setting value */ - const Type& operator=(const Type& value) { + const Type& operator=(const Type& value) override { Type temp{value}; if (use_global) { std::swap(this->global, temp); @@ -295,7 +301,7 @@ public: * * @returns A reference to the current setting value */ - explicit operator const Type&() const { + explicit operator const Type&() const override { if (use_global) { return this->global; } @@ -335,7 +341,7 @@ public: * * @param value The desired value */ - void SetValue(const Type& value) { + void SetValue(const Type& value) override { Type temp; if (value < this->minimum) { temp = std::move(this->minimum); @@ -358,7 +364,7 @@ public: * @param value The desired value * @returns A reference to the setting's value */ - const Type& operator=(const Type& value) { + const Type& operator=(const Type& value) override { Type temp; if (value < this->minimum) { temp = std::move(this->minimum); From e9cf08c2411197860330f77650a9aaac586b9725 Mon Sep 17 00:00:00 2001 From: lat9nq <22451773+lat9nq@users.noreply.github.com> Date: Fri, 30 Jul 2021 16:12:15 -0400 Subject: [PATCH 3/4] settings: Remove unnecessary std::move usages Addresses review feedback. Co-authored-by: Mai M. --- src/common/settings.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/common/settings.h b/src/common/settings.h index f54705a96e..4432b5ddde 100644 --- a/src/common/settings.h +++ b/src/common/settings.h @@ -170,11 +170,11 @@ public: void SetValue(const Type& value) override { Type temp; if (value < minimum) { - temp = std::move(minimum); + temp = minimum; } else if (value > maximum) { - temp = std::move(maximum); + temp = maximum; } else { - temp = std::move(value); + temp = value; } std::swap(this->global, temp); } @@ -188,11 +188,11 @@ public: const Type& operator=(const Type& value) override { Type temp; if (value < minimum) { - temp = std::move(minimum); + temp = minimum; } else if (value > maximum) { - temp = std::move(maximum); + temp = maximum; } else { - temp = std::move(value); + temp = value; } std::swap(this->global, temp); return this->global; @@ -344,11 +344,11 @@ public: void SetValue(const Type& value) override { Type temp; if (value < this->minimum) { - temp = std::move(this->minimum); + temp = this->minimum; } else if (value > this->maximum) { - temp = std::move(this->maximum); + temp = this->maximum; } else { - temp = std::move(value); + temp = value; } if (this->use_global) { std::swap(this->global, temp); @@ -367,11 +367,11 @@ public: const Type& operator=(const Type& value) override { Type temp; if (value < this->minimum) { - temp = std::move(this->minimum); + temp = this->minimum; } else if (value > this->maximum) { - temp = std::move(this->maximum); + temp = this->maximum; } else { - temp = std::move(value); + temp = value; } if (this->use_global) { std::swap(this->global, temp); From 3862511a9ad294918682dccf7765f43df166e0d7 Mon Sep 17 00:00:00 2001 From: lat9nq <22451773+lat9nq@users.noreply.github.com> Date: Sat, 31 Jul 2021 17:20:12 -0400 Subject: [PATCH 4/4] settings: Use std::clamp where possible Addresses PR review Co-authored-by: PixelyIon --- src/common/settings.h | 48 ++++++++----------------------------------- 1 file changed, 9 insertions(+), 39 deletions(-) diff --git a/src/common/settings.h b/src/common/settings.h index 4432b5ddde..69f4adaebc 100644 --- a/src/common/settings.h +++ b/src/common/settings.h @@ -4,6 +4,7 @@ #pragma once +#include #include #include #include @@ -168,15 +169,7 @@ public: * @param value The desired value */ void SetValue(const Type& value) override { - Type temp; - if (value < minimum) { - temp = minimum; - } else if (value > maximum) { - temp = maximum; - } else { - temp = value; - } - std::swap(this->global, temp); + this->global = std::clamp(value, minimum, maximum); } /** @@ -186,15 +179,7 @@ public: * @returns A reference to the setting's value */ const Type& operator=(const Type& value) override { - Type temp; - if (value < minimum) { - temp = minimum; - } else if (value > maximum) { - temp = maximum; - } else { - temp = value; - } - std::swap(this->global, temp); + this->global = std::clamp(value, minimum, maximum); return this->global; } @@ -342,19 +327,11 @@ public: * @param value The desired value */ void SetValue(const Type& value) override { - Type temp; - if (value < this->minimum) { - temp = this->minimum; - } else if (value > this->maximum) { - temp = this->maximum; - } else { - temp = value; - } + const Type temp = std::clamp(value, this->minimum, this->maximum); if (this->use_global) { - std::swap(this->global, temp); - } else { - std::swap(this->custom, temp); + this->global = temp; } + this->custom = temp; } /** @@ -365,19 +342,12 @@ public: * @returns A reference to the setting's value */ const Type& operator=(const Type& value) override { - Type temp; - if (value < this->minimum) { - temp = this->minimum; - } else if (value > this->maximum) { - temp = this->maximum; - } else { - temp = value; - } + const Type temp = std::clamp(value, this->minimum, this->maximum); if (this->use_global) { - std::swap(this->global, temp); + this->global = temp; return this->global; } - std::swap(this->custom, temp); + this->custom = temp; return this->custom; } };