From 70cb3ab6d21f0cbea1be0311577f7e1247872e09 Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Thu, 26 Sep 2024 04:41:56 -0500 Subject: [PATCH] Update settings category signals to be in line with variables and only fire once Properly connect line labels to the category signals --- .../source/scwx/qt/settings/line_settings.cpp | 23 +++- .../source/scwx/qt/settings/line_settings.hpp | 9 ++ .../scwx/qt/settings/settings_category.cpp | 120 ++++++++++++++++-- .../scwx/qt/settings/settings_category.hpp | 25 ++-- .../scwx/qt/settings/settings_variable.cpp | 18 +++ .../qt/settings/settings_variable_base.cpp | 13 ++ .../qt/settings/settings_variable_base.hpp | 15 +++ scwx-qt/source/scwx/qt/ui/line_label.cpp | 80 ++++-------- .../alert_palette_settings_widget.cpp | 66 ++++------ 9 files changed, 252 insertions(+), 117 deletions(-) diff --git a/scwx-qt/source/scwx/qt/settings/line_settings.cpp b/scwx-qt/source/scwx/qt/settings/line_settings.cpp index bc467186..105be812 100644 --- a/scwx-qt/source/scwx/qt/settings/line_settings.cpp +++ b/scwx-qt/source/scwx/qt/settings/line_settings.cpp @@ -1,8 +1,6 @@ #include #include -#include - namespace scwx { namespace qt @@ -106,6 +104,27 @@ SettingsVariable& LineSettings::line_width() const return p->lineWidth_; } +void LineSettings::StageValues(boost::gil::rgba8_pixel_t borderColor, + boost::gil::rgba8_pixel_t highlightColor, + boost::gil::rgba8_pixel_t lineColor, + std::int64_t borderWidth, + std::int64_t highlightWidth, + std::int64_t lineWidth) +{ + set_block_signals(true); + + p->borderColor_.StageValue(util::color::ToArgbString(borderColor)); + p->highlightColor_.StageValue(util::color::ToArgbString(highlightColor)); + p->lineColor_.StageValue(util::color::ToArgbString(lineColor)); + p->borderWidth_.StageValue(borderWidth); + p->highlightWidth_.StageValue(highlightWidth); + p->lineWidth_.StageValue(lineWidth); + + set_block_signals(false); + + staged_signal()(); +} + bool operator==(const LineSettings& lhs, const LineSettings& rhs) { return (lhs.p->borderColor_ == rhs.p->borderColor_ && diff --git a/scwx-qt/source/scwx/qt/settings/line_settings.hpp b/scwx-qt/source/scwx/qt/settings/line_settings.hpp index e99d9b3e..bc9a9cb8 100644 --- a/scwx-qt/source/scwx/qt/settings/line_settings.hpp +++ b/scwx-qt/source/scwx/qt/settings/line_settings.hpp @@ -6,6 +6,8 @@ #include #include +#include + namespace scwx { namespace qt @@ -33,6 +35,13 @@ public: SettingsVariable& highlight_width() const; SettingsVariable& line_width() const; + void StageValues(boost::gil::rgba8_pixel_t borderColor, + boost::gil::rgba8_pixel_t highlightColor, + boost::gil::rgba8_pixel_t lineColor, + std::int64_t borderWidth, + std::int64_t highlightWidth, + std::int64_t lineWidth); + friend bool operator==(const LineSettings& lhs, const LineSettings& rhs); private: diff --git a/scwx-qt/source/scwx/qt/settings/settings_category.cpp b/scwx-qt/source/scwx/qt/settings/settings_category.cpp index 29b3a022..3714c4ca 100644 --- a/scwx-qt/source/scwx/qt/settings/settings_category.cpp +++ b/scwx-qt/source/scwx/qt/settings/settings_category.cpp @@ -4,8 +4,6 @@ #include -#include - namespace scwx { namespace qt @@ -23,6 +21,9 @@ public: ~Impl() {} + void ConnectSubcategory(SettingsCategory& category); + void ConnectVariable(SettingsVariableBase* variable); + const std::string name_; std::vector>> @@ -30,7 +31,11 @@ public: std::vector subcategories_; std::vector variables_; - boost::signals2::signal resetSignal_; + boost::signals2::signal changedSignal_ {}; + boost::signals2::signal stagedSignal_ {}; + bool blockSignals_ {false}; + + std::vector connections_ {}; }; SettingsCategory::SettingsCategory(const std::string& name) : @@ -48,8 +53,27 @@ std::string SettingsCategory::name() const return p->name_; } +boost::signals2::signal& SettingsCategory::changed_signal() +{ + return p->changedSignal_; +} + +boost::signals2::signal& SettingsCategory::staged_signal() +{ + return p->stagedSignal_; +} + +void SettingsCategory::set_block_signals(bool blockSignals) +{ + p->blockSignals_ = blockSignals; +} + void SettingsCategory::SetDefaults() { + // Don't allow individual variables to invoke the signal when operating over + // the entire category + p->blockSignals_ = true; + // Set subcategory array defaults for (auto& subcategoryArray : p->subcategoryArrays_) { @@ -70,12 +94,22 @@ void SettingsCategory::SetDefaults() { variable->SetValueToDefault(); } + + // Unblock signals + p->blockSignals_ = false; + + p->changedSignal_(); + p->stagedSignal_(); } bool SettingsCategory::Commit() { bool committed = false; + // Don't allow individual variables to invoke the signal when operating over + // the entire category + p->blockSignals_ = true; + // Commit subcategory arrays for (auto& subcategoryArray : p->subcategoryArrays_) { @@ -97,11 +131,23 @@ bool SettingsCategory::Commit() committed |= variable->Commit(); } + // Unblock signals + p->blockSignals_ = false; + + if (committed) + { + p->changedSignal_(); + } + return committed; } void SettingsCategory::Reset() { + // Don't allow individual variables to invoke the signal when operating over + // the entire category + p->blockSignals_ = true; + // Reset subcategory arrays for (auto& subcategoryArray : p->subcategoryArrays_) { @@ -123,7 +169,10 @@ void SettingsCategory::Reset() variable->Reset(); } - p->resetSignal_(); + // Unblock signals + p->blockSignals_ = false; + + p->stagedSignal_(); } bool SettingsCategory::ReadJson(const boost::json::object& json) @@ -242,6 +291,7 @@ void SettingsCategory::WriteJson(boost::json::object& json) const void SettingsCategory::RegisterSubcategory(SettingsCategory& subcategory) { + p->ConnectSubcategory(subcategory); p->subcategories_.push_back(&subcategory); } @@ -254,7 +304,11 @@ void SettingsCategory::RegisterSubcategoryArray( std::transform(subcategories.begin(), subcategories.end(), std::back_inserter(newSubcategories.second), - [](SettingsCategory& subcategory) { return &subcategory; }); + [this](SettingsCategory& subcategory) + { + p->ConnectSubcategory(subcategory); + return &subcategory; + }); } void SettingsCategory::RegisterSubcategoryArray( @@ -266,26 +320,74 @@ void SettingsCategory::RegisterSubcategoryArray( std::transform(subcategories.begin(), subcategories.end(), std::back_inserter(newSubcategories.second), - [](SettingsCategory* subcategory) { return subcategory; }); + [this](SettingsCategory* subcategory) + { + p->ConnectSubcategory(*subcategory); + return subcategory; + }); } void SettingsCategory::RegisterVariables( std::initializer_list variables) { + for (auto& variable : variables) + { + p->ConnectVariable(variable); + } p->variables_.insert(p->variables_.end(), variables); } void SettingsCategory::RegisterVariables( std::vector variables) { + for (auto& variable : variables) + { + p->ConnectVariable(variable); + } p->variables_.insert( p->variables_.end(), variables.cbegin(), variables.cend()); } -boost::signals2::connection -SettingsCategory::RegisterResetCallback(std::function callback) +void SettingsCategory::Impl::ConnectSubcategory(SettingsCategory& category) { - return p->resetSignal_.connect(callback); + connections_.emplace_back(category.changed_signal().connect( + [this]() + { + if (!blockSignals_) + { + changedSignal_(); + } + })); + + connections_.emplace_back(category.staged_signal().connect( + [this]() + { + if (!blockSignals_) + { + stagedSignal_(); + } + })); +} + +void SettingsCategory::Impl::ConnectVariable(SettingsVariableBase* variable) +{ + connections_.emplace_back(variable->changed_signal().connect( + [this]() + { + if (!blockSignals_) + { + changedSignal_(); + } + })); + + connections_.emplace_back(variable->staged_signal().connect( + [this]() + { + if (!blockSignals_) + { + stagedSignal_(); + } + })); } } // namespace settings diff --git a/scwx-qt/source/scwx/qt/settings/settings_category.hpp b/scwx-qt/source/scwx/qt/settings/settings_category.hpp index 9b5613be..ee80ba46 100644 --- a/scwx-qt/source/scwx/qt/settings/settings_category.hpp +++ b/scwx-qt/source/scwx/qt/settings/settings_category.hpp @@ -6,7 +6,7 @@ #include #include -#include +#include namespace scwx { @@ -29,6 +29,20 @@ public: std::string name() const; + /** + * Gets the signal invoked when a variable within the category is changed. + * + * @return Changed signal + */ + boost::signals2::signal& changed_signal(); + + /** + * Gets the signal invoked when a variable within the category is staged. + * + * @return Staged signal + */ + boost::signals2::signal& staged_signal(); + /** * Set all variables to their defaults. */ @@ -73,13 +87,8 @@ public: RegisterVariables(std::initializer_list variables); void RegisterVariables(std::vector variables); - /** - * Registers a function to be called when the category is reset. - * - * @param callback Function to be called - */ - boost::signals2::connection - RegisterResetCallback(std::function callback); +protected: + void set_block_signals(bool blockSignals); private: class Impl; diff --git a/scwx-qt/source/scwx/qt/settings/settings_variable.cpp b/scwx-qt/source/scwx/qt/settings/settings_variable.cpp index 71db421e..c4a5f6c9 100644 --- a/scwx-qt/source/scwx/qt/settings/settings_variable.cpp +++ b/scwx-qt/source/scwx/qt/settings/settings_variable.cpp @@ -81,10 +81,13 @@ bool SettingsVariable::SetValue(const T& value) p->value_ = (p->transform_ != nullptr) ? p->transform_(value) : value; validated = true; + changed_signal()(); for (auto& callback : p->valueChangedCallbackFunctions_) { callback.second(p->value_); } + + staged_signal()(); for (auto& callback : p->valueStagedCallbackFunctions_) { callback.second(p->value_); @@ -129,10 +132,13 @@ bool SettingsVariable::SetValueOrDefault(const T& value) p->value_ = p->default_; } + changed_signal()(); for (auto& callback : p->valueChangedCallbackFunctions_) { callback.second(p->value_); } + + staged_signal()(); for (auto& callback : p->valueStagedCallbackFunctions_) { callback.second(p->value_); @@ -146,10 +152,13 @@ void SettingsVariable::SetValueToDefault() { p->value_ = p->default_; + changed_signal()(); for (auto& callback : p->valueChangedCallbackFunctions_) { callback.second(p->value_); } + + staged_signal()(); for (auto& callback : p->valueStagedCallbackFunctions_) { callback.second(p->value_); @@ -168,6 +177,7 @@ void SettingsVariable::StageDefault() p->staged_.reset(); } + staged_signal()(); for (auto& callback : p->valueStagedCallbackFunctions_) { callback.second(p->default_); @@ -194,6 +204,7 @@ bool SettingsVariable::StageValue(const T& value) validated = true; + staged_signal()(); for (auto& callback : p->valueStagedCallbackFunctions_) { callback.second(transformed); @@ -214,10 +225,13 @@ bool SettingsVariable::Commit() p->staged_.reset(); committed = true; + changed_signal()(); for (auto& callback : p->valueChangedCallbackFunctions_) { callback.second(p->value_); } + + staged_signal()(); for (auto& callback : p->valueStagedCallbackFunctions_) { callback.second(p->value_); @@ -232,6 +246,7 @@ void SettingsVariable::Reset() { p->staged_.reset(); + staged_signal()(); for (auto& callback : p->valueStagedCallbackFunctions_) { callback.second(p->value_); @@ -336,10 +351,13 @@ bool SettingsVariable::ReadValue(const boost::json::object& json) p->value_ = p->default_; } + changed_signal()(); for (auto& callback : p->valueChangedCallbackFunctions_) { callback.second(p->value_); } + + staged_signal()(); for (auto& callback : p->valueStagedCallbackFunctions_) { callback.second(p->value_); diff --git a/scwx-qt/source/scwx/qt/settings/settings_variable_base.cpp b/scwx-qt/source/scwx/qt/settings/settings_variable_base.cpp index 55ce72ea..7e31fb5f 100644 --- a/scwx-qt/source/scwx/qt/settings/settings_variable_base.cpp +++ b/scwx-qt/source/scwx/qt/settings/settings_variable_base.cpp @@ -18,6 +18,9 @@ public: ~Impl() {} const std::string name_; + + boost::signals2::signal changedSignal_ {}; + boost::signals2::signal stagedSignal_ {}; }; SettingsVariableBase::SettingsVariableBase(const std::string& name) : @@ -38,6 +41,16 @@ std::string SettingsVariableBase::name() const return p->name_; } +boost::signals2::signal& SettingsVariableBase::changed_signal() +{ + return p->changedSignal_; +} + +boost::signals2::signal& SettingsVariableBase::staged_signal() +{ + return p->stagedSignal_; +} + bool SettingsVariableBase::Equals(const SettingsVariableBase& o) const { return p->name_ == o.p->name_; diff --git a/scwx-qt/source/scwx/qt/settings/settings_variable_base.hpp b/scwx-qt/source/scwx/qt/settings/settings_variable_base.hpp index d7211197..fba1eff9 100644 --- a/scwx-qt/source/scwx/qt/settings/settings_variable_base.hpp +++ b/scwx-qt/source/scwx/qt/settings/settings_variable_base.hpp @@ -4,6 +4,7 @@ #include #include +#include namespace scwx { @@ -30,6 +31,20 @@ public: std::string name() const; + /** + * Gets the signal invoked when the settings variable is changed. + * + * @return Changed signal + */ + boost::signals2::signal& changed_signal(); + + /** + * Gets the signal invoked when the settings variable is staged. + * + * @return Staged signal + */ + boost::signals2::signal& staged_signal(); + /** * Sets the current value of the settings variable to default. */ diff --git a/scwx-qt/source/scwx/qt/ui/line_label.cpp b/scwx-qt/source/scwx/qt/ui/line_label.cpp index a8be0507..0b2f8f75 100644 --- a/scwx-qt/source/scwx/qt/ui/line_label.cpp +++ b/scwx-qt/source/scwx/qt/ui/line_label.cpp @@ -18,12 +18,13 @@ static const auto logger_ = scwx::util::Logger::Create(logPrefix_); class LineLabel::Impl { public: - explicit Impl() {}; + explicit Impl(LineLabel* self) : self_ {self} {}; ~Impl() = default; - void ResetLineSettings(); - QImage GenerateImage() const; + void UpdateLineLabel(const settings::LineSettings& lineSettings); + + LineLabel* self_; std::size_t borderWidth_ {1}; std::size_t highlightWidth_ {1}; @@ -33,26 +34,18 @@ public: boost::gil::rgba8_pixel_t highlightColor_ {255, 255, 0, 255}; boost::gil::rgba8_pixel_t lineColor_ {0, 0, 255, 255}; - settings::LineSettings* lineSettings_ {nullptr}; - QPixmap pixmap_ {}; bool pixmapDirty_ {true}; + + boost::signals2::scoped_connection settingsStaged_ {}; }; LineLabel::LineLabel(QWidget* parent) : - QFrame(parent), p {std::make_unique()} + QFrame(parent), p {std::make_unique(this)} { } -LineLabel::~LineLabel() -{ - p->ResetLineSettings(); -} - -void LineLabel::Impl::ResetLineSettings() -{ - lineSettings_ = nullptr; -} +LineLabel::~LineLabel() {} boost::gil::rgba8_pixel_t LineLabel::border_color() const { @@ -90,11 +83,6 @@ void LineLabel::set_border_width(std::size_t width) p->pixmapDirty_ = true; updateGeometry(); update(); - - if (p->lineSettings_ != nullptr) - { - p->lineSettings_->border_width().StageValue(width); - } } void LineLabel::set_highlight_width(std::size_t width) @@ -103,11 +91,6 @@ void LineLabel::set_highlight_width(std::size_t width) p->pixmapDirty_ = true; updateGeometry(); update(); - - if (p->lineSettings_ != nullptr) - { - p->lineSettings_->highlight_width().StageValue(width); - } } void LineLabel::set_line_width(std::size_t width) @@ -116,11 +99,6 @@ void LineLabel::set_line_width(std::size_t width) p->pixmapDirty_ = true; updateGeometry(); update(); - - if (p->lineSettings_ != nullptr) - { - p->lineSettings_->line_width().StageValue(width); - } } void LineLabel::set_border_color(boost::gil::rgba8_pixel_t color) @@ -128,12 +106,6 @@ void LineLabel::set_border_color(boost::gil::rgba8_pixel_t color) p->borderColor_ = color; p->pixmapDirty_ = true; update(); - - if (p->lineSettings_ != nullptr) - { - p->lineSettings_->border_color().StageValue( - util::color::ToArgbString(color)); - } } void LineLabel::set_highlight_color(boost::gil::rgba8_pixel_t color) @@ -141,12 +113,6 @@ void LineLabel::set_highlight_color(boost::gil::rgba8_pixel_t color) p->highlightColor_ = color; p->pixmapDirty_ = true; update(); - - if (p->lineSettings_ != nullptr) - { - p->lineSettings_->highlight_color().StageValue( - util::color::ToArgbString(color)); - } } void LineLabel::set_line_color(boost::gil::rgba8_pixel_t color) @@ -154,30 +120,30 @@ void LineLabel::set_line_color(boost::gil::rgba8_pixel_t color) p->lineColor_ = color; p->pixmapDirty_ = true; update(); - - if (p->lineSettings_ != nullptr) - { - p->lineSettings_->line_color().StageValue( - util::color::ToArgbString(color)); - } } void LineLabel::set_line_settings(settings::LineSettings& lineSettings) { - p->ResetLineSettings(); + p->settingsStaged_ = lineSettings.staged_signal().connect( + [this, &lineSettings]() { p->UpdateLineLabel(lineSettings); }); - set_border_color(util::color::ToRgba8PixelT( + p->UpdateLineLabel(lineSettings); +} + +void LineLabel::Impl::UpdateLineLabel( + const settings::LineSettings& lineSettings) +{ + self_->set_border_color(util::color::ToRgba8PixelT( lineSettings.border_color().GetStagedOrValue())); - set_highlight_color(util::color::ToRgba8PixelT( + self_->set_highlight_color(util::color::ToRgba8PixelT( lineSettings.highlight_color().GetStagedOrValue())); - set_line_color( + self_->set_line_color( util::color::ToRgba8PixelT(lineSettings.line_color().GetStagedOrValue())); - set_border_width(lineSettings.border_width().GetStagedOrValue()); - set_highlight_width(lineSettings.highlight_width().GetStagedOrValue()); - set_line_width(lineSettings.line_width().GetStagedOrValue()); - - p->lineSettings_ = &lineSettings; + self_->set_border_width(lineSettings.border_width().GetStagedOrValue()); + self_->set_highlight_width( + lineSettings.highlight_width().GetStagedOrValue()); + self_->set_line_width(lineSettings.line_width().GetStagedOrValue()); } QSize LineLabel::minimumSizeHint() const diff --git a/scwx-qt/source/scwx/qt/ui/settings/alert_palette_settings_widget.cpp b/scwx-qt/source/scwx/qt/ui/settings/alert_palette_settings_widget.cpp index 8e77bdb6..9775b9c8 100644 --- a/scwx-qt/source/scwx/qt/ui/settings/alert_palette_settings_widget.cpp +++ b/scwx-qt/source/scwx/qt/ui/settings/alert_palette_settings_widget.cpp @@ -37,13 +37,7 @@ public: SetupUi(); ConnectSignals(); } - ~Impl() - { - for (auto& c : bs2Connections_) - { - c.disconnect(); - } - }; + ~Impl() {}; void AddPhenomenonLine(const std::string& name, settings::LineSettings& lineSettings, @@ -59,10 +53,8 @@ public: QStackedWidget* phenomenonPagesWidget_; QListWidget* phenomenonListView_; - EditLineDialog* editLineDialog_; - LineLabel* activeLineLabel_ {nullptr}; - - std::vector bs2Connections_ {}; + EditLineDialog* editLineDialog_; + settings::LineSettings* activeLineSettings_ {nullptr}; boost::unordered_flat_map phenomenonPages_ {}; }; @@ -147,30 +139,27 @@ void AlertPaletteSettingsWidget::Impl::ConnectSignals() } }); - connect( - editLineDialog_, - &EditLineDialog::accepted, - self_, - [this]() - { - // If the active line label was set - if (activeLineLabel_ != nullptr) - { - // Update the active line label with selected line settings - activeLineLabel_->set_border_color(editLineDialog_->border_color()); - activeLineLabel_->set_highlight_color( - editLineDialog_->highlight_color()); - activeLineLabel_->set_line_color(editLineDialog_->line_color()); + connect(editLineDialog_, + &EditLineDialog::accepted, + self_, + [this]() + { + // If the active line label was set + if (activeLineSettings_ != nullptr) + { + // Update the active line settings with selected line settings + activeLineSettings_->StageValues( + editLineDialog_->border_color(), + editLineDialog_->highlight_color(), + editLineDialog_->line_color(), + editLineDialog_->border_width(), + editLineDialog_->highlight_width(), + editLineDialog_->line_width()); - activeLineLabel_->set_border_width(editLineDialog_->border_width()); - activeLineLabel_->set_highlight_width( - editLineDialog_->highlight_width()); - activeLineLabel_->set_line_width(editLineDialog_->line_width()); - - // Reset the active line label - activeLineLabel_ = nullptr; - } - }); + // Reset the active line settings + activeLineSettings_ = nullptr; + } + }); } void AlertPaletteSettingsWidget::Impl::SelectPhenomenon( @@ -260,19 +249,14 @@ void AlertPaletteSettingsWidget::Impl::AddPhenomenonLine( self_->AddSettingsCategory(&lineSettings); - boost::signals2::connection c = lineSettings.RegisterResetCallback( - [lineLabel, &lineSettings]() - { lineLabel->set_line_settings(lineSettings); }); - bs2Connections_.push_back(c); - connect( toolButton, &QAbstractButton::clicked, self_, - [this, lineLabel]() + [this, lineLabel, &lineSettings]() { // Set the active line label for when the dialog is finished - activeLineLabel_ = lineLabel; + activeLineSettings_ = &lineSettings; // Initialize dialog with current line settings editLineDialog_->set_border_color(lineLabel->border_color());