From 3ac64a18ea1825174b84adb8570368df51e66231 Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Tue, 22 Aug 2023 23:07:49 +0300 Subject: [PATCH] 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. --- examples/widgetdemo/buttondemo.cpp | 9 +++---- examples/widgetdemo/demoselector.cpp | 3 +-- examples/widgetdemo/dialogdemo.cpp | 6 ++--- examples/widgetdemo/dropdowndemo.cpp | 6 ++--- examples/widgetdemo/entrydemo.cpp | 12 +++------ examples/widgetdemo/toggledemo.cpp | 15 ++++------- examples/widgetdemo/widgetdemo.cpp | 23 ++++++++-------- examples/widgetdemo/widgetdemo.h | 3 ++- source/container.cpp | 39 +++++++++++++++++++++------- source/container.h | 20 +++++++++++++- source/dialog.cpp | 26 ++++++++++++++++--- source/dialog.h | 5 ++++ source/entry.cpp | 5 ++-- source/panel.cpp | 5 ++-- source/panel.h | 5 ++-- source/root.cpp | 3 +-- 16 files changed, 118 insertions(+), 67 deletions(-) diff --git a/examples/widgetdemo/buttondemo.cpp b/examples/widgetdemo/buttondemo.cpp index 170ce85..871244a 100644 --- 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(); vector 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(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("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 52ba458..507815c 100644 --- 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("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 63fb68e..db8ba8d 100644 --- 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("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 5ab8782..8d6d276 100644 --- 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(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(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 062ce8b..ad94010 100644 --- 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("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(); 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("Multi-line entry:"); layout->add_constraint(*lbl, GLtk::Layout::BELOW, *prev); prev = lbl; - GLtk::Entry *ent = new GLtk::Entry; + GLtk::Entry *ent = &add_new(); 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 91849e6..2cb6df5 100644 --- 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("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(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(); 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(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(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 b1482e7..1332ee1 100644 --- 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("Button"); + add_demo("Dialog"); + add_demo("Dropdown"); + add_demo("Entry"); + add_demo("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 +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(); + 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 cab80b7..f6ef7f6 100644 --- 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 + void add_demo(const std::string &); }; #endif diff --git a/source/container.cpp b/source/container.cpp index 88e5952..f1f1934 100644 --- 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 w = move(children.back()->own_widget); + else + children.pop_back(); + } } -void Container::add(Widget &wdg) +void Container::add(unique_ptr wdg) { - wdg.set_parent(this); - children.push_back(make_unique(*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 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(*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 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 6311a41..d100dac 100644 --- a/source/container.h +++ b/source/container.h @@ -24,10 +24,12 @@ protected: struct Child: public sigc::trackable { Container &container; + std::unique_ptr own_widget; Widget *widget = nullptr; Time::TimeDelta time_since_animate; Child(Container &, Widget *); + Child(Container &, std::unique_ptr); 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); + + template + 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 +T &Container::add_new(Args &&... args) +{ + std::unique_ptr wdg = std::make_unique(std::forward(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 5e5e390..fed9c85 100644 --- 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