From f8fb55710deb1a2c64c1e960e34bbe3e73f17ca3 Mon Sep 17 00:00:00 2001 From: smolBlackCat <66385489+smolBlackCat@users.noreply.github.com> Date: Mon, 23 Dec 2024 20:43:13 -0300 Subject: [PATCH] (core): Reduce memory leak caused by continuously opening card dialogs By adding class scope, we don't need to allocate memory every time the user opens the card dialog. The approach taken is to define a CardDetailsDialog instance as a member of ProgressWindow and have a CardWidget get the root (ProgressWindow) and open the dialog. The functionality is as equivalent as the later implementation. The memory leak is still in investigation since the approach taken by the app is quite new. Also GTK is known to quite lazy when dealing with memory pages --- src/dialog/card-dialog.cpp | 101 ++++++++++++++++++++----------------- src/dialog/card-dialog.h | 23 ++++++--- src/widgets/card.cpp | 7 +-- src/window.cpp | 17 ++++++- src/window.h | 5 ++ 5 files changed, 97 insertions(+), 56 deletions(-) diff --git a/src/dialog/card-dialog.cpp b/src/dialog/card-dialog.cpp index a6346c3..fe40774 100644 --- a/src/dialog/card-dialog.cpp +++ b/src/dialog/card-dialog.cpp @@ -10,9 +10,8 @@ namespace ui { -CardDetailsDialog::CardDetailsDialog(CardWidget& card_widget) - : card_widget{card_widget}, - checklist_add_button{_("Add Task")}, +CardDetailsDialog::CardDetailsDialog() + : checklist_add_button{_("Add Task")}, builder{Gtk::Builder::create_from_resource( "/io/github/smolblackcat/Progress/card-details-dialog.ui")}, card_title_entry{builder->get_widget("card-title-entry")}, @@ -28,25 +27,17 @@ CardDetailsDialog::CardDetailsDialog(CardWidget& card_widget) tasks_box{builder->get_widget("tasks-box")}, notes_textbuffer{ builder->get_object("notes-textbuffer")}, - dialog{builder->get_object("card-dialog")} { + adw_dialog{builder->get_object("card-dialog")} { checklist_add_button.signal_clicked().connect( sigc::mem_fun(*this, &CardDetailsDialog::on_add_task)); - auto card_ptr = this->card_widget.get_card(); - card_delete_button->signal_clicked().connect( sigc::mem_fun(*this, &CardDetailsDialog::on_delete_card)); - if (card_ptr->get_due_date().ok()) { - update_due_date_label(); - checkbutton_revealer->set_reveal_child(true); - checkbutton->set_active(card_ptr->get_complete()); - } - checkbutton->signal_toggled().connect([this]() { - this->card_widget.get_card()->set_complete( + this->cur_card_widget->get_card()->set_complete( this->checkbutton->get_active()); - this->card_widget.update_due_date_label_style(); + this->cur_card_widget->update_due_date_label_style(); }); calendar->signal_day_selected().connect( @@ -55,7 +46,7 @@ CardDetailsDialog::CardDetailsDialog(CardWidget& card_widget) unset_due_date_button->signal_clicked().connect( sigc::mem_fun(*this, &CardDetailsDialog::on_unset_due_date)); - g_signal_connect(dialog->gobj(), "close-attempt", + g_signal_connect(adw_dialog->gobj(), "close-attempt", G_CALLBACK(+[](AdwDialog* self, gpointer data) { reinterpret_cast(data)->on_save(); reinterpret_cast(data)->close(); @@ -63,47 +54,52 @@ CardDetailsDialog::CardDetailsDialog(CardWidget& card_widget) this); tasks_box->append(checklist_add_button); - - card_title_entry->set_text(card_ptr->get_name()); - for (auto& task : card_ptr->get_tasks()) { - _add_task(task); - } - notes_textbuffer->set_text(card_ptr->get_notes()); -} - -CardDetailsDialog* CardDetailsDialog::create(CardWidget& card_widget) { - auto card_dialog = new CardDetailsDialog{card_widget}; - return card_dialog; } CardDetailsDialog::~CardDetailsDialog() {} void CardDetailsDialog::remove_task(TaskWidget& task) { - auto card = card_widget.get_card(); + auto card = cur_card_widget->get_card(); card->remove(*task.get_task()); tasks_box->remove(task); - card_widget.update_complete_tasks(); + cur_card_widget->update_complete_tasks(); } void CardDetailsDialog::reorder_task_widget(TaskWidget& next, TaskWidget& sibling) { tasks_box->reorder_child_after(next, sibling); - card_widget.get_card()->reorder(*next.get_task(), *sibling.get_task()); + cur_card_widget->get_card()->reorder(*next.get_task(), *sibling.get_task()); } -void CardDetailsDialog::open(Gtk::Window& parent) { - adw_dialog_present(ADW_DIALOG(dialog->gobj()), +void CardDetailsDialog::open(Gtk::Window& parent, CardWidget* card_widget) { + // Load the card contents into the dialog + cur_card_widget = card_widget; + auto card_ptr = this->cur_card_widget->get_card(); + + if (card_ptr->get_due_date().ok()) { + update_due_date_label(); + checkbutton_revealer->set_reveal_child(true); + checkbutton->set_active(card_ptr->get_complete()); + } + + card_title_entry->set_text(card_ptr->get_name()); + for (auto& task : card_ptr->get_tasks()) { + _add_task(task); + } + notes_textbuffer->set_text(card_ptr->get_notes()); + + adw_dialog_present(ADW_DIALOG(adw_dialog->gobj()), static_cast(parent).gobj()); } void CardDetailsDialog::close() { - adw_dialog_force_close(ADW_DIALOG(dialog->gobj())); - delete this; // Suicide + adw_dialog_force_close(ADW_DIALOG(adw_dialog->gobj())); + clear(); } void CardDetailsDialog::update_due_date_label() { auto sys_days = - std::chrono::sys_days(card_widget.get_card()->get_due_date()); + std::chrono::sys_days(cur_card_widget->get_card()->get_due_date()); sys_days++; std::time_t time = std::chrono::system_clock::to_time_t(sys_days); char date_str[255]; @@ -111,20 +107,20 @@ void CardDetailsDialog::update_due_date_label() { date_menubutton->set_label(Glib::ustring{date_str}); } -CardWidget& CardDetailsDialog::get_card_widget() { return card_widget; } +CardWidget* CardDetailsDialog::get_card_widget() { return cur_card_widget; } void CardDetailsDialog::on_add_task() { - _add_task(card_widget.get_card()->add(Task{_("New Task")}), true); - card_widget.update_complete_tasks(); + _add_task(cur_card_widget->get_card()->add(Task{_("New Task")}), true); + cur_card_widget->update_complete_tasks(); } void CardDetailsDialog::on_save() { - auto card = card_widget.get_card(); + auto card = cur_card_widget->get_card(); std::string new_card_name = card_title_entry->get_text(); std::string new_notes = notes_textbuffer->get_text(); if (card->get_name() != new_card_name) { - card_widget.set_title(new_card_name); + cur_card_widget->set_title(new_card_name); card->set_name(new_card_name); } @@ -132,28 +128,28 @@ void CardDetailsDialog::on_save() { card->set_notes(new_notes); } - card_widget.set_tooltip_text(card_widget.create_details_text()); + cur_card_widget->set_tooltip_text(cur_card_widget->create_details_text()); } void CardDetailsDialog::on_delete_card() { - card_widget.remove_from_parent(); + cur_card_widget->remove_from_parent(); close(); } void CardDetailsDialog::on_unset_due_date() { - auto card_ptr = card_widget.get_card(); + auto card_ptr = cur_card_widget->get_card(); date_menubutton->set_label(_("Set Due Date")); card_ptr->set_due_date(Date{}); checkbutton_revealer->set_reveal_child(false); - card_widget.update_due_date(); + cur_card_widget->update_due_date(); } void CardDetailsDialog::on_set_due_date() { using namespace std::chrono; using namespace std::chrono_literals; - auto card_ptr = card_widget.get_card(); + auto card_ptr = cur_card_widget->get_card(); int y, m, d; calendar->get_date().get_ymd(y, m, d); @@ -161,16 +157,29 @@ void CardDetailsDialog::on_set_due_date() { day{static_cast(d)}}; card_ptr->set_due_date(new_date); checkbutton_revealer->set_reveal_child(true); - card_widget.update_due_date(); + cur_card_widget->update_due_date(); update_due_date_label(); } +void CardDetailsDialog::clear() { + card_title_entry->set_text(""); + for (TaskWidget* task_widget : tasks_tracker) { + tasks_box->remove(*task_widget); + } + tasks_tracker.clear(); + + notes_textbuffer->set_text(""); + + cur_card_widget = nullptr; +} + void CardDetailsDialog::_add_task(const std::shared_ptr task, bool is_new) { auto builder = Gtk::Builder::create_from_resource( "/io/github/smolblackcat/Progress/checklist-item-widget.ui"); auto task_widget = Gtk::Builder::get_widget_derived( - builder, "task-widget", *this, card_widget, task, is_new); + builder, "task-widget", *this, *cur_card_widget, task, is_new); tasks_box->append(*task_widget); + tasks_tracker.push_back(task_widget); } } // namespace ui diff --git a/src/dialog/card-dialog.h b/src/dialog/card-dialog.h index e03673d..ab40c16 100644 --- a/src/dialog/card-dialog.h +++ b/src/dialog/card-dialog.h @@ -20,6 +20,11 @@ class CardDetailsDialog { public: static CardDetailsDialog* create(CardWidget& card_widget); + /** + * @brief Default CardDetailsDialog constructor + */ + CardDetailsDialog(); + ~CardDetailsDialog(); /** @@ -31,7 +36,13 @@ class CardDetailsDialog { void reorder_task_widget(TaskWidget& next, TaskWidget& sibling); - void open(Gtk::Window& parent); + /** + * @brief Opens the details of a card widget + * + * @param parent transient window + * @param card_widget card widget pointer to modify settings from + */ + void open(Gtk::Window& parent, CardWidget* card_widget); void on_save(); @@ -42,19 +53,18 @@ class CardDetailsDialog { /** * @brief Returns the CardWidget object pointer */ - CardWidget& get_card_widget(); + CardWidget* get_card_widget(); protected: - CardDetailsDialog(CardWidget& card_widget); - void on_add_task(); void on_delete_card(); void on_unset_due_date(); void on_set_due_date(); + void clear(); Glib::RefPtr builder; - Glib::RefPtr dialog; + Glib::RefPtr adw_dialog; Gtk::Entry* card_title_entry; Gtk::Button checklist_add_button; Gtk::Button *unset_due_date_button, *card_delete_button; @@ -65,7 +75,8 @@ class CardDetailsDialog { Gtk::Box* tasks_box; Glib::RefPtr notes_textbuffer; - CardWidget& card_widget; + CardWidget* cur_card_widget; + std::vector tasks_tracker; private: void _add_task(const std::shared_ptr task, bool is_new = false); diff --git a/src/widgets/card.cpp b/src/widgets/card.cpp index 9843eff..ee17013 100644 --- a/src/widgets/card.cpp +++ b/src/widgets/card.cpp @@ -8,6 +8,7 @@ #include #include "cardlist-widget.h" +#include "window.h" ui::CardWidget::CardWidget(BaseObjectType* cobject, const Glib::RefPtr& builder, @@ -308,14 +309,14 @@ void ui::CardWidget::open_color_dialog() { } void ui::CardWidget::open_card_details_dialog() { - auto& parent_window = *(static_cast(get_root())); - auto card_details_dialog = CardDetailsDialog::create(*this); - card_details_dialog->open(parent_window); + auto& parent_window = *(static_cast(get_root())); + parent_window.show_card_dialog(this); } void ui::CardWidget::on_rename() { card_entry_revealer->set_reveal_child(true); card_label->set_visible(false); + card_entry->grab_focus(); } void ui::CardWidget::off_rename() { diff --git a/src/window.cpp b/src/window.cpp index b1c3d98..ba19322 100644 --- a/src/window.cpp +++ b/src/window.cpp @@ -58,7 +58,8 @@ ProgressWindow::ProgressWindow(BaseObjectType* cobject, adw_style_manager{ adw_style_manager_get_for_display(this->get_display()->gobj())}, css_provider{Gtk::CssProvider::create()}, - progress_settings{progress_settings} { + progress_settings{progress_settings}, + card_dialog{} { Gtk::StyleProvider::add_provider_for_display( get_display(), css_provider, GTK_STYLE_PROVIDER_PRIORITY_USER); @@ -123,6 +124,7 @@ void ProgressWindow::add_local_board(BoardBackend board_backend) { auto board_card_button = Gtk::make_managed(board_backend); auto fb_child_p = Gtk::make_managed(); fb_child_p->set_child(*board_card_button); + fb_child_p->set_focusable(false); boards_grid_p->append(*fb_child_p); board_card_button->signal_clicked().connect( [this, board_card_button, fb_child_p]() { @@ -156,12 +158,21 @@ void ProgressWindow::on_delete_board_mode() { on_delete_mode = true; boards_grid_p->set_selection_mode(Gtk::SelectionMode::MULTIPLE); delete_boards_bar.set_reveal_child(); + + // TAB clicks won't include them, making more manageable to delete boards + add_board_button_p->set_focusable(false); + app_menu_button_p->set_focusable(false); + app_menu_button_p->set_sensitive(false); } void ProgressWindow::off_delete_board_mode() { on_delete_mode = false; delete_boards_bar.set_reveal_child(false); boards_grid_p->set_selection_mode(Gtk::SelectionMode::NONE); + + add_board_button_p->set_focusable(); + app_menu_button_p->set_focusable(); + app_menu_button_p->set_sensitive(); } void ProgressWindow::on_main_menu() { @@ -207,6 +218,10 @@ void ProgressWindow::show_about_dialog() { "https://github.com/smolBlackCat/progress-tracker", NULL); } +void ProgressWindow::show_card_dialog(CardWidget* card_widget) { + card_dialog.open(*this, card_widget); +} + void ProgressWindow::setup_menu_button() { auto action_group = Gio::SimpleActionGroup::create(); action_group->add_action( diff --git a/src/window.h b/src/window.h index dd38e93..485d0ea 100644 --- a/src/window.h +++ b/src/window.h @@ -3,6 +3,7 @@ #include #include #include +#include "dialog/card-dialog.h" namespace ui { @@ -76,6 +77,8 @@ class ProgressWindow : public Gtk::ApplicationWindow { */ void show_about_dialog(); + void show_card_dialog(CardWidget* card_widget); + protected: AdwStyleManager* adw_style_manager; Glib::RefPtr css_provider; @@ -95,6 +98,8 @@ class ProgressWindow : public Gtk::ApplicationWindow { Glib::RefPtr board_grid_menu_p, board_menu_p; Gtk::MenuButton* app_menu_button_p; + CardDetailsDialog card_dialog; + void setup_menu_button(); void load_appropriate_style();