Rework how widget ownership works in Container

The class now supports both owned and non-owned children.  Owned children
are passed in as unique_ptr or can be created by the container with the
add_new() function template.  This is likely to introduce memory leaks in
existing code.

diff --git a/examples/widgetdemo/buttondemo.cpp b/examples/widgetdemo/buttondemo.cpp
index 170ce85636fa7b229d9f185f51a07587e95024e9..871244ab8d6fd0f99e1e932b18bf218bdef2d080 100644 (file)
--- a/examples/widgetdemo/buttondemo.cpp
+++ b/examples/widgetdemo/buttondemo.cpp
@@ -10,17 +10,15 @@ ButtonDemo::ButtonDemo()
 {
        get_or_create_layout();
 
-       GLtk::Label *lbl_message = new GLtk::Label;
-       add(*lbl_message);
+       GLtk::Label *lbl_message = &add_new<GLtk::Label>();
 
        vector<GLtk::Button *> buttons;
        for(unsigned i=0; i<5; ++i)
        {
                string text = format("Button %d", i+1);
-               GLtk::Button *btn = new GLtk::Button(text);
+               GLtk::Button *btn = &add_new<GLtk::Button>(text);
                btn->signal_clicked.connect(sigc::bind(sigc::mem_fun(lbl_message, &GLtk::Label::set_text), text+" was clicked"));
                buttons.push_back(btn);
-               add(*btn);
                if(i>0)
                {
                        layout->add_constraint(*btn, GLtk::Layout::COPY_WIDTH, *buttons[0]);
@@ -41,10 +39,9 @@ ButtonDemo::ButtonDemo()
 
        layout->add_constraint(*buttons[0], GLtk::Layout::BELOW, *lbl_message);
 
-       GLtk::Button *btn = new GLtk::Button("A big one");
+       GLtk::Button *btn = &add_new<GLtk::Button>("A big one");
        btn->signal_clicked.connect(sigc::bind(sigc::mem_fun(lbl_message, &GLtk::Label::set_text), "The big button was clicked"));
        buttons.push_back(btn);
-       add(*btn);
        layout->add_constraint(*btn, GLtk::Layout::ALIGN_TOP, *buttons[0]);
        layout->add_constraint(*btn, GLtk::Layout::ALIGN_BOTTOM, *buttons[1]);
        layout->add_constraint(*btn, GLtk::Layout::ALIGN_LEFT, *buttons[3]);

diff --git a/examples/widgetdemo/demoselector.cpp b/examples/widgetdemo/demoselector.cpp
index 52ba458eed0b6f9e227ad8c91be64d5c26653cde..507815c1f663720ab9433c36bcb220362cb8b92a 100644 (file)
--- a/examples/widgetdemo/demoselector.cpp
+++ b/examples/widgetdemo/demoselector.cpp
@@ -14,8 +14,7 @@ DemoSelector::DemoSelector():
 
        GLtk::Column col(*layout);
 
-       GLtk::Label *lbl = new GLtk::Label("Select a widget:");
-       add(*lbl);
+       add_new<GLtk::Label>("Select a widget:");
 
        list.set_data(demos);
        list.signal_item_selected.connect(sigc::mem_fun(this, &DemoSelector::item_selected));

diff --git a/examples/widgetdemo/dialogdemo.cpp b/examples/widgetdemo/dialogdemo.cpp
index 63fb68e5b085bcf7c074e4303a6bea1515648e31..db8ba8d5c9370cd6736d7e0d1f4ad70754560a08 100644 (file)
--- a/examples/widgetdemo/dialogdemo.cpp
+++ b/examples/widgetdemo/dialogdemo.cpp
@@ -27,8 +27,7 @@ DialogDemo::DialogDemo()
 {
        get_or_create_layout();
 
-       GLtk::Button *btn_prompt = new GLtk::Button("Enter some text");
-       add(*btn_prompt);
+       GLtk::Button *btn_prompt = &add_new<GLtk::Button>("Enter some text");
        btn_prompt->signal_clicked.connect(sigc::mem_fun(this, &DialogDemo::button_clicked));
 
        add(lbl_text);
@@ -59,8 +58,7 @@ PromptDialog::PromptDialog()
        add(ent_text);
        layout->set_expand(ent_text, true, false);
 
-       GLtk::Button *btn_ok = new GLtk::Button("OK");
-       add_button(*btn_ok, 1);
+       GLtk::Button *btn_ok = &add_button("OK", 1);
        layout->add_constraint(*btn_ok, GLtk::Layout::FAR_BELOW, ent_text);
        layout->set_gravity(*btn_ok, 1, -1);
 }

diff --git a/examples/widgetdemo/dropdowndemo.cpp b/examples/widgetdemo/dropdowndemo.cpp
index 5ab8782f4211a236dac5751081b2c37f448187fd..8d6d276b85f429b039fb2764cdd6a745f1ef2cea 100644 (file)
--- a/examples/widgetdemo/dropdowndemo.cpp
+++ b/examples/widgetdemo/dropdowndemo.cpp
@@ -9,9 +9,8 @@ DropdownDemo::DropdownDemo()
 {
        get_or_create_layout();
 
-       GLtk::Dropdown *drp = new GLtk::Dropdown(categories);
+       GLtk::Dropdown *drp = &add_new<GLtk::Dropdown>(categories);
        drp->signal_item_selected.connect(sigc::mem_fun(this, &DropdownDemo::category_selected));
-       add(*drp);
 
        categories.append("Arabic numerals");
        categories.append("Roman numerals");
@@ -19,8 +18,7 @@ DropdownDemo::DropdownDemo()
        categories.append("Uppercase letters");
 
        GLtk::Widget *prev = drp;
-       drp = new GLtk::Dropdown(values);
-       add(*drp);
+       drp = &add_new<GLtk::Dropdown>(values);
 
        layout->add_constraint(*drp, GLtk::Layout::BELOW, *prev);
        layout->add_constraint(*drp, GLtk::Layout::ALIGN_LEFT, *prev);

diff --git a/examples/widgetdemo/entrydemo.cpp b/examples/widgetdemo/entrydemo.cpp
index 062ce8bf8e581c4c93df4c6eb4736241c148c137..ad94010d6a919f0429f28d1550e9897d0d903160 100644 (file)
--- a/examples/widgetdemo/entrydemo.cpp
+++ b/examples/widgetdemo/entrydemo.cpp
@@ -8,27 +8,23 @@ EntryDemo::EntryDemo()
 {
        get_or_create_layout();
 
-       GLtk::Label *lbl = new GLtk::Label("Single-line entries:");
-       add(*lbl);
+       GLtk::Label *lbl = &add_new<GLtk::Label>("Single-line entries:");
 
        GLtk::Widget *prev = lbl;
        for(unsigned i=0; i<2; ++i)
        {
-               GLtk::Entry *ent = new GLtk::Entry;
+               GLtk::Entry *ent = &add_new<GLtk::Entry>();
                ent->set_edit_size(20+i*40, 1);
-               add(*ent);
                layout->add_constraint(*ent, GLtk::Layout::BELOW, *prev);
                prev = ent;
        }
 
-       lbl = new GLtk::Label("Multi-line entry:");
-       add(*lbl);
+       lbl = &add_new<GLtk::Label>("Multi-line entry:");
        layout->add_constraint(*lbl, GLtk::Layout::BELOW, *prev);
        prev = lbl;
 
-       GLtk::Entry *ent = new GLtk::Entry;
+       GLtk::Entry *ent = &add_new<GLtk::Entry>();
        ent->set_multiline(true);
        ent->set_edit_size(60, 5);
-       add(*ent);
        layout->add_constraint(*ent, GLtk::Layout::BELOW, *prev);
 }

diff --git a/examples/widgetdemo/toggledemo.cpp b/examples/widgetdemo/toggledemo.cpp
index 91849e688ef0a1f81bc0450de9e28441791fc92d..2cb6df53735b1028b6a3689667cbfe62f653df4c 100644 (file)
--- a/examples/widgetdemo/toggledemo.cpp
+++ b/examples/widgetdemo/toggledemo.cpp
@@ -9,14 +9,12 @@ ToggleDemo::ToggleDemo()
 {
        get_or_create_layout();
 
-       GLtk::Label *lbl = new GLtk::Label("Standalone toggles:");
-       add(*lbl);
+       GLtk::Label *lbl = &add_new<GLtk::Label>("Standalone toggles:");
 
        GLtk::Widget *prev = lbl;
        for(unsigned i=0; i<4; ++i)
        {
-               GLtk::Toggle *tgl = new GLtk::Toggle(format("Toggle %d", i+1));
-               add(*tgl);
+               GLtk::Toggle *tgl = &add_new<GLtk::Toggle>(format("Toggle %d", i+1));
 
                layout->add_constraint(*tgl, GLtk::Layout::BELOW, *prev);
                layout->add_constraint(*tgl, GLtk::Layout::ALIGN_LEFT, *prev);
@@ -27,23 +25,20 @@ ToggleDemo::ToggleDemo()
 
        for(unsigned i=0; i<2; ++i)
        {
-               GLtk::Panel *group = new GLtk::Panel;
-               add(*group);
+               GLtk::Panel *group = &add_new<GLtk::Panel>();
                layout->add_constraint(*group, (i==0 ? GLtk::Layout::BELOW : GLtk::Layout::RIGHT_OF), *prev);
                if(i>0)
                        layout->add_constraint(*group, GLtk::Layout::ALIGN_TOP, *prev);
                GLtk::Layout &group_layout = group->get_or_create_layout();
 
-               lbl = new GLtk::Label(format("Group %d:", i+1));
-               group->add(*lbl);
+               lbl = &group->add_new<GLtk::Label>(format("Group %d:", i+1));
 
                prev = lbl;
                for(unsigned j=0; j<4; ++j)
                {
-                       GLtk::Toggle *tgl = new GLtk::Toggle(format("Option %d", j+1));
+                       GLtk::Toggle *tgl = &group->add_new<GLtk::Toggle>(format("Option %d", j+1));
                        tgl->set_style("option");
                        tgl->set_exclusive(true);
-                       group->add(*tgl);
 
                        group_layout.add_constraint(*tgl, GLtk::Layout::BELOW, *prev);
                        group_layout.add_constraint(*tgl, GLtk::Layout::ALIGN_LEFT, *prev);

diff --git a/examples/widgetdemo/widgetdemo.cpp b/examples/widgetdemo/widgetdemo.cpp
index b1482e72ae93f10c9652f5cadacdc954e6afbaae..1332ee1ffee363edc1e2597180728666a690f48c 100644 (file)
--- a/examples/widgetdemo/widgetdemo.cpp
+++ b/examples/widgetdemo/widgetdemo.cpp
@@ -25,11 +25,11 @@ WidgetDemo::WidgetDemo(int, char **):
        root.add(selector);
        root_layout.set_expand(selector, false, true);
 
-       add_demo("Button", new ButtonDemo);
-       add_demo("Dialog", new DialogDemo);
-       add_demo("Dropdown", new DropdownDemo);
-       add_demo("Entry", new EntryDemo);
-       add_demo("Toggle", new ToggleDemo);
+       add_demo<ButtonDemo>("Button");
+       add_demo<DialogDemo>("Dialog");
+       add_demo<DropdownDemo>("Dropdown");
+       add_demo<EntryDemo>("Entry");
+       add_demo<ToggleDemo>("Toggle");
 
        view.set_content(&root);
 }
@@ -46,14 +46,15 @@ void WidgetDemo::tick()
        view.render();
 }
 
-void WidgetDemo::add_demo(const string &title, GLtk::Panel *demo)
+template<typename T>
+void WidgetDemo::add_demo(const string &title)
 {
        GLtk::Layout &root_layout = root.get_or_create_layout();
 
-       demo->set_visible(false);
-       root.add(*demo);
-       root_layout.add_constraint(*demo, GLtk::Layout::RIGHT_OF, selector);
-       root_layout.set_expand(*demo, true, true);
+       T &demo = root.add_new<T>();
+       demo.set_visible(false);
+       root_layout.add_constraint(demo, GLtk::Layout::RIGHT_OF, selector);
+       root_layout.set_expand(demo, true, true);
 
-       selector.add_demo(title, demo);
+       selector.add_demo(title, &demo);
 }

diff --git a/examples/widgetdemo/widgetdemo.h b/examples/widgetdemo/widgetdemo.h
index cab80b7ed394965fe4539d0bed6767f85c6513cc..f6ef7f6996d070884f6b917101610da8ca6559e8 100644 (file)
--- a/examples/widgetdemo/widgetdemo.h
+++ b/examples/widgetdemo/widgetdemo.h
@@ -26,7 +26,8 @@ public:
 private:
        virtual void tick();
 
-       void add_demo(const std::string &, Msp::GLtk::Panel *);
+       template<typename T>
+       void add_demo(const std::string &);
 };
 
 #endif

diff --git a/source/container.cpp b/source/container.cpp
index 88e59524e8ddac754a69b7506d230d6d019957be..f1f1934c071d724caaf17086076ccde92c111369 100644 (file)
--- a/source/container.cpp
+++ b/source/container.cpp
@@ -14,19 +14,21 @@ hierarchy_error::hierarchy_error(const string &w):
 
 Container::~Container()
 {
+       // Clear children here while members are still valid
        while(!children.empty())
-               delete children.front()->widget;
+       {
+               if(children.back()->own_widget)
+                       /* Avoid destroying the unique_ptr for the widget from within its own
+                       reset() function */
+                       unique_ptr<Widget> w = move(children.back()->own_widget);
+               else
+                       children.pop_back();
+       }
 }
 
-void Container::add(Widget &wdg)
+void Container::add(unique_ptr<Widget> wdg)
 {
-       wdg.set_parent(this);
-       children.push_back(make_unique<Child>(*this, &wdg));
-       if(wdg.get_animation_interval())
-               check_animation_interval();
-       children_rebuild_needed = true;
-       signal_rebuild_needed.emit();
-       on_child_added(wdg);
+       add_child(*wdg).own_widget = move(wdg);
 }
 
 void Container::remove(Widget &wdg)
@@ -35,6 +37,7 @@ void Container::remove(Widget &wdg)
        if(i==children.end())
                throw hierarchy_error("widget not in container");
 
+       unique_ptr<Widget> owned = move((*i)->own_widget);
        if(&wdg==saved_input_focus)
                saved_input_focus = nullptr;
        wdg.set_parent(nullptr);
@@ -44,6 +47,18 @@ void Container::remove(Widget &wdg)
        on_child_removed(wdg);
 }
 
+Container::Child &Container::add_child(Widget &wdg)
+{
+       wdg.set_parent(this);
+       children.push_back(make_unique<Child>(*this, &wdg));
+       if(wdg.get_animation_interval())
+               check_animation_interval();
+       children_rebuild_needed = true;
+       signal_rebuild_needed.emit();
+       on_child_added(wdg);
+       return *children.back();
+}
+
 Geometry Container::determine_child_geometry(const Widget &child, const Part &part) const
 {
        Geometry pgeom = part.get_geometry();
@@ -381,6 +396,12 @@ Container::Child::Child(Container &c, Widget *w):
        widget->signal_rebuild_needed.connect(sigc::mem_fun(this, &Child::rebuild_needed));
 }
 
+Container::Child::Child(Container &c, unique_ptr<Widget> w):
+       Child(c, w.get())
+{
+       own_widget = move(w);
+}
+
 Container::Child::~Child()
 {
        visibility_changed(false);

diff --git a/source/container.h b/source/container.h
index 6311a411a57fb1b16e03dcdd24564242a2ac5902..d100dacf94aaa80538194fc77e2d95a486c869b3 100644 (file)
--- a/source/container.h
+++ b/source/container.h
@@ -24,10 +24,12 @@ protected:
        struct Child: public sigc::trackable
        {
                Container &container;
+               std::unique_ptr<Widget> own_widget;
                Widget *widget = nullptr;
                Time::TimeDelta time_since_animate;
 
                Child(Container &, Widget *);
+               Child(Container &, std::unique_ptr<Widget>);
                virtual ~Child();
 
                void visibility_changed(bool);
@@ -52,9 +54,15 @@ protected:
 public:
        virtual ~Container();
 
-       void add(Widget &);
+       void add(Widget &w) { add_child(w); }
+       void add(std::unique_ptr<Widget>);
+
+       template<typename T, typename... Args>
+       T &add_new(Args &&...);
+
        void remove(Widget &);
 protected:
+       Child &add_child(Widget &);
        Geometry determine_child_geometry(const Widget &, const Part &) const;
        void autosize_child(const Widget &, const Part &, Geometry &) const;
        void reposition_child(Widget &, const Part &) const;
@@ -102,6 +110,16 @@ protected:
        virtual void on_input_focus_changed(Widget *);
 };
 
+
+template<typename T, typename... Args>
+T &Container::add_new(Args &&... args)
+{
+       std::unique_ptr<T> wdg = std::make_unique<T>(std::forward<Args>(args)...);
+       T *ptr = wdg.get();
+       add(move(wdg));
+       return *ptr;
+}
+
 } // namespace GLtk
 } // namespace Msp
 

diff --git a/source/dialog.cpp b/source/dialog.cpp
index 5e5e390e7cadadf91d3e5af4e978d4e3f333dc33..fed9c859f0003ae73e63542e5a0a011b6d0a3485 100644 (file)
--- a/source/dialog.cpp
+++ b/source/dialog.cpp
@@ -9,7 +9,21 @@ namespace GLtk {
 void Dialog::add_button(Button &button, int code)
 {
        add(button);
-       button.signal_clicked.connect(sigc::bind(sigc::mem_fun(this, &Dialog::response), code));
+       connect_button(button, code);
+}
+
+void Dialog::add_button(unique_ptr<Button> button, int code)
+{
+       Button &b = *button;
+       add(move(button));
+       connect_button(b, code);
+}
+
+Button &Dialog::add_button(const string &text, int code)
+{
+       Button &b = add_new<Button>(text);
+       connect_button(b, code);
+       return b;
 }
 
 void Dialog::set_modal(bool m)
@@ -24,6 +38,11 @@ void Dialog::set_modal(bool m)
                signal_ungrab_pointer.emit();
 }
 
+void Dialog::connect_button(Button &button, int code)
+{
+       button.signal_clicked.connect(sigc::bind(sigc::mem_fun(this, &Dialog::response), code));
+}
+
 void Dialog::response(int code)
 {
        on_response(code);
@@ -42,8 +61,9 @@ void Dialog::Loader::action_button(const string &n, int c)
 {
        unique_ptr<Button> btn = make_unique<Button>();
        load_sub(*btn);
-       obj.add_button(*btn.get(), c);
-       last_widget = wdg_map[n] = btn.release();
+       Widget *wdg = btn.get();
+       obj.add_button(move(btn), c);
+       last_widget = wdg_map[n] = wdg;
 }
 
 } // namespace GLtk

diff --git a/source/dialog.h b/source/dialog.h
index cc5ed41d1cfa6a4d967b376a71114ec6f8636b59..6209ab29c4fa42ebe00678ffb170a8b4b0206da4 100644 (file)
--- a/source/dialog.h
+++ b/source/dialog.h
@@ -33,11 +33,16 @@ public:
        response handlers and delete the dialog. */
        void add_button(Button &, int);
 
+       void add_button(std::unique_ptr<Button>, int);
+
+       Button &add_button(const std::string &, int);
+
        /** Sets the modality of the dialog.  When modal, the user can't navigate
        away from the dialog. */
        void set_modal(bool);
 
 protected:
+       void connect_button(Button &, int);
        void response(int);
 
        /** Called when an action button is pressed. */

diff --git a/source/entry.cpp b/source/entry.cpp
index b212f5f6ecdf66a557ee614b37ca9cf0c1765089..b2c78004b233b6ad9243e781639a6756a417387e 100644 (file)
--- a/source/entry.cpp
+++ b/source/entry.cpp
@@ -118,8 +118,9 @@ void Entry::set_multiline(bool m)
        {
                if(!slider)
                {
-                       slider = new VSlider;
-                       add(*slider);
+                       unique_ptr<VSlider> s = make_unique<VSlider>();
+                       slider = s.get();
+                       add(move(s));
                        slider->set_step(1);
                        slider->signal_value_changed.connect(sigc::mem_fun(this, &Entry::slider_value_changed));
                        mark_rebuild();

diff --git a/source/panel.cpp b/source/panel.cpp
index d4e737bee7de6e9b48ef459b02f35f111a8e453f..ba66c682e85decb9608c084caeaaa6b5763c0318 100644 (file)
--- a/source/panel.cpp
+++ b/source/panel.cpp
@@ -295,8 +295,9 @@ void Panel::Loader::unnamed_child<Panel>()
 {
        unique_ptr<Panel> pnl = make_unique<Panel>();
        load_sub(*pnl, wdg_map);
-       obj.add(*pnl.get());
-       last_widget = pnl.release();
+       Widget *wdg = pnl.get();
+       obj.add(move(pnl));
+       last_widget = wdg;
 }
 
 

diff --git a/source/panel.h b/source/panel.h
index 3abfee1cadb072c74f9f0b4229fda6acfb98b6cd..3061032789574f168c2bb2933f787d4f439c1284 100644 (file)
--- a/source/panel.h
+++ b/source/panel.h
@@ -117,8 +117,9 @@ void Panel::Loader::unnamed_child()
 {
        std::unique_ptr<T> chl = std::make_unique<T>();
        load_sub(*chl);
-       obj.add(*chl.get());
-       last_widget = chl.release();
+       Widget *wdg = chl.get();
+       obj.add(move(chl));
+       last_widget = wdg;
 }
 
 

diff --git a/source/root.cpp b/source/root.cpp
index 8d32c467b26eb5924e1825d6d51ff37d01de65a7..16ed5bd58c1d9f94a22236b8900c1db8a1be0831 100644 (file)
--- a/source/root.cpp
+++ b/source/root.cpp
@@ -90,8 +90,7 @@ void Root::tick()
                {
                        if(!lbl_tooltip)
                        {
-                               lbl_tooltip = new Label;
-                               add(*lbl_tooltip);
+                               lbl_tooltip = &add_new<Label>();
                                lbl_tooltip->set_style("tooltip");
                        }