Update settings category signals to be in line with variables and only fire once

Properly connect line labels to the category signals

scwx-qt/source/scwx/qt/settings/line_settings.cpp

@@ -1,8 +1,6 @@
 #include <scwx/qt/settings/line_settings.hpp>
 #include <scwx/qt/util/color.hpp>
 
-#include <boost/gil.hpp>
-
 namespace scwx
 {
 namespace qt
@@ -106,6 +104,27 @@ SettingsVariable<std::int64_t>& 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_ &&

scwx-qt/source/scwx/qt/settings/line_settings.hpp

@@ -6,6 +6,8 @@
 #include <memory>
 #include <string>
 
+#include <boost/gil/typedefs.hpp>
+
 namespace scwx
 {
 namespace qt
@@ -33,6 +35,13 @@ public:
    SettingsVariable<std::int64_t>& highlight_width() const;
    SettingsVariable<std::int64_t>& 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:

scwx-qt/source/scwx/qt/settings/settings_category.cpp

@@ -4,8 +4,6 @@
 
 #include <algorithm>
 
-#include <boost/signals2/signal.hpp>
-
 namespace scwx
 {
 namespace qt
@@ -23,6 +21,9 @@ public:
 
    ~Impl() {}
 
+   void ConnectSubcategory(SettingsCategory& category);
+   void ConnectVariable(SettingsVariableBase* variable);
+
    const std::string name_;
 
    std::vector<std::pair<std::string, std::vector<SettingsCategory*>>>
@@ -30,7 +31,11 @@ public:
    std::vector<SettingsCategory*>     subcategories_;
    std::vector<SettingsVariableBase*> variables_;
 
-   boost::signals2::signal<void()> resetSignal_;
+   boost::signals2::signal<void()> changedSignal_ {};
+   boost::signals2::signal<void()> stagedSignal_ {};
+   bool                            blockSignals_ {false};
+
+   std::vector<boost::signals2::scoped_connection> connections_ {};
 };
 
 SettingsCategory::SettingsCategory(const std::string& name) :
@@ -48,8 +53,27 @@ std::string SettingsCategory::name() const
    return p->name_;
 }
 
+boost::signals2::signal<void()>& SettingsCategory::changed_signal()
+{
+   return p->changedSignal_;
+}
+
+boost::signals2::signal<void()>& 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<SettingsVariableBase*> variables)
 {
+   for (auto& variable : variables)
+   {
+      p->ConnectVariable(variable);
+   }
    p->variables_.insert(p->variables_.end(), variables);
 }
 
 void SettingsCategory::RegisterVariables(
    std::vector<SettingsVariableBase*> variables)
 {
+   for (auto& variable : variables)
+   {
+      p->ConnectVariable(variable);
+   }
    p->variables_.insert(
       p->variables_.end(), variables.cbegin(), variables.cend());
 }
 
-boost::signals2::connection
+void SettingsCategory::Impl::ConnectSubcategory(SettingsCategory& category)
-SettingsCategory::RegisterResetCallback(std::function<void()> callback)
 {
-   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

scwx-qt/source/scwx/qt/settings/settings_category.hpp

@@ -6,7 +6,7 @@
 #include <string>
 
 #include <boost/json/object.hpp>
-#include <boost/signals2/connection.hpp>
+#include <boost/signals2/signal.hpp>
 
 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<void()>& changed_signal();
+
+   /**
+    * Gets the signal invoked when a variable within the category is staged.
+    *
+    * @return Staged signal
+    */
+   boost::signals2::signal<void()>& staged_signal();
+
    /**
     * Set all variables to their defaults.
     */
@@ -73,13 +87,8 @@ public:
    RegisterVariables(std::initializer_list<SettingsVariableBase*> variables);
    void RegisterVariables(std::vector<SettingsVariableBase*> variables);
 
-   /**
+protected:
-    * Registers a function to be called when the category is reset.
+   void set_block_signals(bool blockSignals);
-    *
-    * @param callback Function to be called
-    */
-   boost::signals2::connection
-   RegisterResetCallback(std::function<void()> callback);
 
 private:
    class Impl;

scwx-qt/source/scwx/qt/settings/settings_variable.cpp

@@ -81,10 +81,13 @@ bool SettingsVariable<T>::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<T>::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<T>::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<T>::StageDefault()
       p->staged_.reset();
    }
 
+   staged_signal()();
    for (auto& callback : p->valueStagedCallbackFunctions_)
    {
       callback.second(p->default_);
@@ -194,6 +204,7 @@ bool SettingsVariable<T>::StageValue(const T& value)
 
       validated = true;
 
+      staged_signal()();
       for (auto& callback : p->valueStagedCallbackFunctions_)
       {
          callback.second(transformed);
@@ -214,10 +225,13 @@ bool SettingsVariable<T>::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<T>::Reset()
 {
    p->staged_.reset();
 
+   staged_signal()();
    for (auto& callback : p->valueStagedCallbackFunctions_)
    {
       callback.second(p->value_);
@@ -336,10 +351,13 @@ bool SettingsVariable<T>::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_);

scwx-qt/source/scwx/qt/settings/settings_variable_base.cpp

@@ -18,6 +18,9 @@ public:
    ~Impl() {}
 
    const std::string name_;
+
+   boost::signals2::signal<void()> changedSignal_ {};
+   boost::signals2::signal<void()> stagedSignal_ {};
 };
 
 SettingsVariableBase::SettingsVariableBase(const std::string& name) :
@@ -38,6 +41,16 @@ std::string SettingsVariableBase::name() const
    return p->name_;
 }
 
+boost::signals2::signal<void()>& SettingsVariableBase::changed_signal()
+{
+   return p->changedSignal_;
+}
+
+boost::signals2::signal<void()>& SettingsVariableBase::staged_signal()
+{
+   return p->stagedSignal_;
+}
+
 bool SettingsVariableBase::Equals(const SettingsVariableBase& o) const
 {
    return p->name_ == o.p->name_;

scwx-qt/source/scwx/qt/settings/settings_variable_base.hpp

@@ -4,6 +4,7 @@
 #include <string>
 
 #include <boost/json/object.hpp>
+#include <boost/signals2/signal.hpp>
 
 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<void()>& changed_signal();
+
+   /**
+    * Gets the signal invoked when the settings variable is staged.
+    *
+    * @return Staged signal
+    */
+   boost::signals2::signal<void()>& staged_signal();
+
    /**
     * Sets the current value of the settings variable to default.
     */

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<Impl>()}
+    QFrame(parent), p {std::make_unique<Impl>(this)}
 {
 }
 
-LineLabel::~LineLabel()
+LineLabel::~LineLabel() {}
-{
-   p->ResetLineSettings();
-}
-
-void LineLabel::Impl::ResetLineSettings()
-{
-   lineSettings_ = nullptr;
-}
 
 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());
+   self_->set_border_width(lineSettings.border_width().GetStagedOrValue());
-   set_highlight_width(lineSettings.highlight_width().GetStagedOrValue());
+   self_->set_highlight_width(
-   set_line_width(lineSettings.line_width().GetStagedOrValue());
+      lineSettings.highlight_width().GetStagedOrValue());
-
+   self_->set_line_width(lineSettings.line_width().GetStagedOrValue());
-   p->lineSettings_ = &lineSettings;
 }
 
 QSize LineLabel::minimumSizeHint() const

scwx-qt/source/scwx/qt/ui/settings/alert_palette_settings_widget.cpp

@@ -37,13 +37,7 @@ public:
       SetupUi();
       ConnectSignals();
    }
-   ~Impl()
+   ~Impl() {};
-   {
-      for (auto& c : bs2Connections_)
-      {
-         c.disconnect();
-      }
-   };
 
    void     AddPhenomenonLine(const std::string&      name,
                               settings::LineSettings& lineSettings,
@@ -59,10 +53,8 @@ public:
    QStackedWidget* phenomenonPagesWidget_;
    QListWidget*    phenomenonListView_;
 
-   EditLineDialog* editLineDialog_;
+   EditLineDialog*         editLineDialog_;
-   LineLabel*      activeLineLabel_ {nullptr};
+   settings::LineSettings* activeLineSettings_ {nullptr};
-
-   std::vector<boost::signals2::connection> bs2Connections_ {};
 
    boost::unordered_flat_map<awips::Phenomenon, QWidget*> phenomenonPages_ {};
 };
@@ -147,30 +139,27 @@ void AlertPaletteSettingsWidget::Impl::ConnectSignals()
          }
       });
 
-   connect(
+   connect(editLineDialog_,
-      editLineDialog_,
+           &EditLineDialog::accepted,
-      &EditLineDialog::accepted,
+           self_,
-      self_,
+           [this]()
-      [this]()
+           {
-      {
+              // If the active line label was set
-         // If the active line label was set
+              if (activeLineSettings_ != nullptr)
-         if (activeLineLabel_ != nullptr)
+              {
-         {
+                 // Update the active line settings with selected line settings
-            // Update the active line label with selected line settings
+                 activeLineSettings_->StageValues(
-            activeLineLabel_->set_border_color(editLineDialog_->border_color());
+                    editLineDialog_->border_color(),
-            activeLineLabel_->set_highlight_color(
+                    editLineDialog_->highlight_color(),
-               editLineDialog_->highlight_color());
+                    editLineDialog_->line_color(),
-            activeLineLabel_->set_line_color(editLineDialog_->line_color());
+                    editLineDialog_->border_width(),
+                    editLineDialog_->highlight_width(),
+                    editLineDialog_->line_width());
 
-            activeLineLabel_->set_border_width(editLineDialog_->border_width());
+                 // Reset the active line settings
-            activeLineLabel_->set_highlight_width(
+                 activeLineSettings_ = nullptr;
-               editLineDialog_->highlight_width());
+              }
-            activeLineLabel_->set_line_width(editLineDialog_->line_width());
+           });
-
-            // Reset the active line label
-            activeLineLabel_ = 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());