Remove BrushPopupDelegate

At this moment we prefer to use AppBrushes and ContextBar directly
from BrushPopup.

src/app/app_brushes.cpp

@@ -26,8 +26,7 @@ AppBrushes::slot_id AppBrushes::addBrushSlot(const BrushSlot& brush)
 {
   // Use an empty slot
   for (size_t i=0; i<m_slots.size(); ++i) {
-    if (!m_slots[i].locked() ||
+    if (!m_slots[i].locked() || m_slots[i].isEmpty()) {
-        !m_slots[i].brush()) {
       m_slots[i] = brush;
       return i+1;
     }
@@ -46,7 +45,7 @@ void AppBrushes::removeBrushSlot(slot_id slot)
 
     // Erase empty trailing slots
     while (!m_slots.empty() &&
-           !m_slots[m_slots.size()-1].brush())
+           m_slots[m_slots.size()-1].isEmpty())
       m_slots.erase(--m_slots.end());
 
     ItemsChange();
@@ -90,7 +89,7 @@ void AppBrushes::lockBrushSlot(slot_id slot)
 {
   --slot;
   if (slot >= 0 && slot < (int)m_slots.size() &&
-      m_slots[slot].brush()) {
+      !m_slots[slot].isEmpty()) {
     m_slots[slot].setLocked(true);
   }
 }
@@ -99,7 +98,7 @@ void AppBrushes::unlockBrushSlot(slot_id slot)
 {
   --slot;
   if (slot >= 0 && slot < (int)m_slots.size() &&
-      m_slots[slot].brush()) {
+      !m_slots[slot].isEmpty()) {
     m_slots[slot].setLocked(false);
   }
 }

src/app/ui/brush_popup.cpp

@@ -12,7 +12,6 @@
 #include "app/ui/brush_popup.h"
 
 #include "app/app.h"
-#include "app/app_brushes.h"
 #include "app/brush_slot.h"
 #include "app/commands/command.h"
 #include "app/commands/commands.h"
@@ -21,7 +20,9 @@
 #include "app/pref/preferences.h"
 #include "app/ui/app_menuitem.h"
 #include "app/ui/button_set.h"
+#include "app/ui/context_bar.h"
 #include "app/ui/keyboard_shortcuts.h"
+#include "app/ui/main_window.h"
 #include "app/ui/skin/skin_theme.h"
 #include "app/ui_context.h"
 #include "base/bind.h"
@@ -73,8 +74,8 @@ void show_popup_menu(PopupWindow* popupWindow, Menu* popupMenu,
 
 class SelectBrushItem : public ButtonSet::Item {
 public:
-  SelectBrushItem(BrushPopupDelegate* delegate, const BrushSlot& brush, int slot = -1)
+  SelectBrushItem(const BrushSlot& brush, int slot = -1)
-    : m_delegate(delegate)
+    : m_brushes(App::instance()->brushes())
     , m_brush(brush)
     , m_slot(slot) {
     if (m_brush.hasBrush()) {
@@ -89,17 +90,17 @@ public:
   }
 
 private:
-
   void onClick() override {
+    ContextBar* contextBar =
+      App::instance()->getMainWindow()->getContextBar();
+
     if (m_slot >= 0)
-      m_delegate->onSelectBrushSlot(m_slot);
+      contextBar->setActiveBrushBySlot(m_slot);
     else if (m_brush.hasBrush())
-      m_delegate->onSelectBrush(m_brush.brush());
+      contextBar->setActiveBrush(m_brush.brush());
   }
 
-private:
+  AppBrushes& m_brushes;
-
-  BrushPopupDelegate* m_delegate;
   BrushSlot m_brush;
   int m_slot;
 };
@@ -134,9 +135,9 @@ private:
 
 class BrushOptionsItem : public ButtonSet::Item {
 public:
-  BrushOptionsItem(BrushPopup* popup, BrushPopupDelegate* delegate, int slot)
+  BrushOptionsItem(BrushPopup* popup, int slot)
     : m_popup(popup)
-    , m_delegate(delegate)
+    , m_brushes(App::instance()->brushes())
     , m_slot(slot) {
     setIcon(SkinTheme::instance()->parts.iconArrowDown(), true);
   }
@@ -150,7 +151,7 @@ private:
     AppMenuItem deleteItem("Delete");
     AppMenuItem deleteAllItem("Delete All");
 
-    lockItem.setSelected(m_delegate->onIsBrushSlotLocked(m_slot));
+    lockItem.setSelected(m_brushes.isBrushSlotLocked(m_slot));
 
     save.Click.connect(&BrushOptionsItem::onSaveBrush, this);
     lockItem.Click.connect(&BrushOptionsItem::onLockBrush, this);
@@ -170,8 +171,7 @@ private:
     menu.addChild(&params);
 
     // Load preferences
-    AppBrushes& brushes = App::instance()->brushes();
+    BrushSlot brush = m_brushes.getBrushSlot(m_slot);
-    BrushSlot brush = brushes.getBrushSlot(m_slot);
 
     params.brushType()->setSelected(brush.hasFlag(BrushSlot::Flags::BrushType));
     params.brushSize()->setSelected(brush.hasFlag(BrushSlot::Flags::BrushSize));
@@ -199,56 +199,58 @@ private:
 
     if (brush.flags() != BrushSlot::Flags(flags)) {
       brush.setFlags(BrushSlot::Flags(flags));
-      brushes.setBrushSlot(m_slot, brush);
+      m_brushes.setBrushSlot(m_slot, brush);
     }
   }
 
 private:
 
   void onSaveBrush() {
-    AppBrushes& brushes = App::instance()->brushes();
+    ContextBar* contextBar =
-    brushes.setBrushSlot(
+      App::instance()->getMainWindow()->getContextBar();
-      m_slot, m_delegate->onCreateBrushSlotFromActivePreferences());
+
-    brushes.lockBrushSlot(m_slot);
+    m_brushes.setBrushSlot(
+      m_slot, contextBar->createBrushSlotFromPreferences());
+    m_brushes.lockBrushSlot(m_slot);
   }
 
   void onLockBrush() {
-    if (m_delegate->onIsBrushSlotLocked(m_slot))
+    if (m_brushes.isBrushSlotLocked(m_slot))
-      m_delegate->onUnlockBrushSlot(m_slot);
+      m_brushes.unlockBrushSlot(m_slot);
     else
-      m_delegate->onLockBrushSlot(m_slot);
+      m_brushes.lockBrushSlot(m_slot);
   }
 
   void onDeleteBrush() {
-    m_delegate->onDeleteBrushSlot(m_slot);
+    m_brushes.removeBrushSlot(m_slot);
   }
 
   void onDeleteAllBrushes() {
-    m_delegate->onDeleteAllBrushes();
+    m_brushes.removeAllBrushSlots();
   }
 
   BrushPopup* m_popup;
-  BrushPopupDelegate* m_delegate;
+  AppBrushes& m_brushes;
   BrushRef m_brush;
   int m_slot;
 };
 
 class NewCustomBrushItem : public ButtonSet::Item {
 public:
-  NewCustomBrushItem(BrushPopupDelegate* delegate)
+  NewCustomBrushItem() {
-    : m_delegate(delegate) {
     setText("Save Brush");
   }
 
 private:
   void onClick() override {
-    AppBrushes& brushes = App::instance()->brushes();
+    ContextBar* contextBar =
-    auto slot = brushes.addBrushSlot(
+      App::instance()->getMainWindow()->getContextBar();
-      m_delegate->onCreateBrushSlotFromActivePreferences());
+
+    auto& brushes = App::instance()->brushes();
+    int slot = brushes.addBrushSlot(
+      contextBar->createBrushSlotFromPreferences());
     brushes.lockBrushSlot(slot);
   }
-
-  BrushPopupDelegate* m_delegate;
 };
 
 class NewBrushOptionsItem : public ButtonSet::Item {
@@ -305,13 +307,14 @@ private:
 
 } // anonymous namespace
 
-BrushPopup::BrushPopup(BrushPopupDelegate* delegate)
+BrushPopup::BrushPopup()
   : PopupWindow("", ClickBehavior::CloseOnClickInOtherWindow)
   , m_tooltipManager(nullptr)
   , m_standardBrushes(3)
   , m_customBrushes(nullptr)
-  , m_delegate(delegate)
 {
+  auto& brushes = App::instance()->brushes();
+
   setAutoRemap(false);
   setBorder(gfx::Border(2)*guiscale());
   setChildSpacing(0);
@@ -327,17 +330,15 @@ BrushPopup::BrushPopup(BrushPopupDelegate* delegate)
   m_box.addChild(top);
   m_box.addChild(new Separator("", HORIZONTAL));
 
-  const doc::Brushes& brushes = App::instance()->brushes().getStandardBrushes();
+  for (const auto& brush : brushes.getStandardBrushes())
-  for (const auto& brush : brushes)
     m_standardBrushes.addItem(
       new SelectBrushItem(
-        m_delegate, BrushSlot(BrushSlot::Flags::BrushType, brush)));
+        BrushSlot(BrushSlot::Flags::BrushType, brush)));
 
   m_standardBrushes.setTransparent(true);
   m_standardBrushes.setBgColor(gfx::ColorNone);
 
-  App::instance()->brushes()
+  brushes.ItemsChange.connect(&BrushPopup::onBrushChanges, this);
-    .ItemsChange.connect(&BrushPopup::onBrushChanges, this);
 }
 
 void BrushPopup::setBrush(Brush* brush)
@@ -387,12 +388,12 @@ void BrushPopup::regenerate(const gfx::Rect& box)
       if (key && !key->accels().empty())
         shortcut = key->accels().front().toString();
     }
-    m_customBrushes->addItem(new SelectBrushItem(m_delegate, brush, slot));
+    m_customBrushes->addItem(new SelectBrushItem(brush, slot));
     m_customBrushes->addItem(new BrushShortcutItem(shortcut, slot));
-    m_customBrushes->addItem(new BrushOptionsItem(this, m_delegate, slot));
+    m_customBrushes->addItem(new BrushOptionsItem(this, slot));
   }
 
-  m_customBrushes->addItem(new NewCustomBrushItem(m_delegate), 2, 1);
+  m_customBrushes->addItem(new NewCustomBrushItem, 2, 1);
   m_customBrushes->addItem(new NewBrushOptionsItem);
   m_customBrushes->setExpansive(true);
   m_box.addChild(m_customBrushes);

src/app/ui/brush_popup.h

@@ -18,24 +18,9 @@
 
 namespace app {
 
-  class BrushSlot;
-
-  class BrushPopupDelegate {
-  public:
-    virtual ~BrushPopupDelegate() { }
-    virtual BrushSlot onCreateBrushSlotFromActivePreferences() = 0;
-    virtual void onSelectBrush(const doc::BrushRef& brush) = 0;
-    virtual void onSelectBrushSlot(int slot) = 0;
-    virtual void onDeleteBrushSlot(int slot) = 0;
-    virtual void onDeleteAllBrushes() = 0;
-    virtual bool onIsBrushSlotLocked(int slot) const = 0;
-    virtual void onLockBrushSlot(int slot) = 0;
-    virtual void onUnlockBrushSlot(int slot) = 0;
-  };
-
   class BrushPopup : public ui::PopupWindow {
   public:
-    BrushPopup(BrushPopupDelegate* delegate);
+    BrushPopup();
 
     void setBrush(doc::Brush* brush);
     void regenerate(const gfx::Rect& box);
@@ -54,7 +39,6 @@ namespace app {
     ui::VBox m_box;
     ButtonSet m_standardBrushes;
     ButtonSet* m_customBrushes;
-    BrushPopupDelegate* m_delegate;
   };
 
 } // namespace app

src/app/ui/context_bar.cpp

@@ -72,14 +72,12 @@ using namespace tools;
 
 static bool g_updatingFromCode = false;
 
-class ContextBar::BrushTypeField : public ButtonSet
+class ContextBar::BrushTypeField : public ButtonSet {
-                                 , public BrushPopupDelegate {
 public:
   BrushTypeField(ContextBar* owner)
     : ButtonSet(1)
     , m_owner(owner)
-    , m_brushes(App::instance()->brushes())
+    , m_brushes(App::instance()->brushes()) {
-    , m_popupWindow(this) {
     SkinPartPtr part(new SkinPart);
     part->setBitmap(
       0, BrushPopup::createSurfaceForBrush(BrushRef(nullptr)));
@@ -122,72 +120,6 @@ protected:
     ev.setSizeHint(Size(16, 18)*guiscale());
   }
 
-  // BrushPopupDelegate impl
-  BrushSlot onCreateBrushSlotFromActivePreferences() override {
-    auto& pref = Preferences::instance();
-    auto& saveBrush = pref.saveBrush;
-    auto& toolPref = pref.tool(App::instance()->activeTool());
-
-    int flags = 0;
-    if (saveBrush.brushType()) flags |= int(BrushSlot::Flags::BrushType);
-    if (saveBrush.brushSize()) flags |= int(BrushSlot::Flags::BrushSize);
-    if (saveBrush.brushAngle()) flags |= int(BrushSlot::Flags::BrushAngle);
-    if (saveBrush.fgColor()) flags |= int(BrushSlot::Flags::FgColor);
-    if (saveBrush.bgColor()) flags |= int(BrushSlot::Flags::BgColor);
-    if (saveBrush.inkType()) flags |= int(BrushSlot::Flags::InkType);
-    if (saveBrush.inkOpacity()) flags |= int(BrushSlot::Flags::InkOpacity);
-    if (saveBrush.shade()) flags |= int(BrushSlot::Flags::Shade);
-    if (saveBrush.pixelPerfect()) flags |= int(BrushSlot::Flags::PixelPerfect);
-
-    return BrushSlot(
-      BrushSlot::Flags(flags),
-      ContextBar::createBrushFromPreferences(),
-      pref.colorBar.fgColor(),
-      pref.colorBar.bgColor(),
-      toolPref.ink(),
-      toolPref.opacity(),
-      m_owner->getShade(),
-      toolPref.freehandAlgorithm() == tools::FreehandAlgorithm::PIXEL_PERFECT);
-  }
-
-  void onSelectBrush(const BrushRef& brush) override {
-    if (brush->type() == kImageBrushType)
-      m_owner->setActiveBrush(brush);
-    else {
-      Tool* tool = App::instance()->activeTool();
-      ToolPreferences::Brush& brushPref = Preferences::instance().tool(tool).brush;
-
-      brushPref.type(static_cast<app::gen::BrushType>(brush->type()));
-
-      m_owner->setActiveBrush(
-        ContextBar::createBrushFromPreferences(&brushPref));
-    }
-  }
-
-  void onSelectBrushSlot(int slot) override {
-    m_owner->setActiveBrushBySlot(slot);
-  }
-
-  void onDeleteBrushSlot(int slot) override {
-    m_brushes.removeBrushSlot(slot);
-  }
-
-  void onDeleteAllBrushes() override {
-    m_brushes.removeAllBrushSlots();
-  }
-
-  bool onIsBrushSlotLocked(int slot) const override {
-    return m_brushes.isBrushSlotLocked(slot);
-  }
-
-  void onLockBrushSlot(int slot) override {
-    m_brushes.lockBrushSlot(slot);
-  }
-
-  void onUnlockBrushSlot(int slot) override {
-    m_brushes.unlockBrushSlot(slot);
-  }
-
 private:
   // Returns a little rectangle that can be used by the popup as the
   // first brush position.
@@ -1737,7 +1669,18 @@ void ContextBar::setActiveBrushBySlot(int slot)
 
 void ContextBar::setActiveBrush(const doc::BrushRef& brush)
 {
-  m_activeBrush = brush;
+  if (brush->type() == kImageBrushType)
+    m_activeBrush = brush;
+  else {
+    Tool* tool = App::instance()->activeTool();
+    auto& brushPref = Preferences::instance().tool(tool).brush;
+    auto newBrushType = static_cast<app::gen::BrushType>(brush->type());
+    if (brushPref.type() != newBrushType)
+      brushPref.type(newBrushType);
+
+    m_activeBrush = brush;
+  }
+
   BrushChange();
 
   updateForCurrentTool();
@@ -1778,6 +1721,34 @@ doc::BrushRef ContextBar::createBrushFromPreferences(ToolPreferences::Brush* bru
   return brush;
 }
 
+BrushSlot ContextBar::createBrushSlotFromPreferences()
+{
+  auto& pref = Preferences::instance();
+  auto& saveBrush = pref.saveBrush;
+  auto& toolPref = pref.tool(App::instance()->activeTool());
+
+  int flags = 0;
+  if (saveBrush.brushType()) flags |= int(BrushSlot::Flags::BrushType);
+  if (saveBrush.brushSize()) flags |= int(BrushSlot::Flags::BrushSize);
+  if (saveBrush.brushAngle()) flags |= int(BrushSlot::Flags::BrushAngle);
+  if (saveBrush.fgColor()) flags |= int(BrushSlot::Flags::FgColor);
+  if (saveBrush.bgColor()) flags |= int(BrushSlot::Flags::BgColor);
+  if (saveBrush.inkType()) flags |= int(BrushSlot::Flags::InkType);
+  if (saveBrush.inkOpacity()) flags |= int(BrushSlot::Flags::InkOpacity);
+  if (saveBrush.shade()) flags |= int(BrushSlot::Flags::Shade);
+  if (saveBrush.pixelPerfect()) flags |= int(BrushSlot::Flags::PixelPerfect);
+
+  return BrushSlot(
+    BrushSlot::Flags(flags),
+    createBrushFromPreferences(),
+    pref.colorBar.fgColor(),
+    pref.colorBar.bgColor(),
+    toolPref.ink(),
+    toolPref.opacity(),
+    getShade(),
+    toolPref.freehandAlgorithm() == tools::FreehandAlgorithm::PIXEL_PERFECT);
+}
+
 Shade ContextBar::getShade() const
 {
   return m_inkShades->getShade();

src/app/ui/context_bar.h

@@ -37,8 +37,7 @@ namespace tools {
 
 namespace app {
 
-  class IBrushSettings;
+  class BrushSlot;
-  class IToolSettings;
 
   class ContextBar : public ui::Box,
                      public base::Observable<ContextBarObserver> {
@@ -57,6 +56,7 @@ namespace app {
     doc::BrushRef activeBrush(tools::Tool* tool = nullptr) const;
     void discardActiveBrush();
 
+    BrushSlot createBrushSlotFromPreferences();
     static doc::BrushRef createBrushFromPreferences(
       ToolPreferences::Brush* brushPref = nullptr);