Skip to content

Commit

Permalink
(core): Reduce memory leak caused by continuously opening card dialogs
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@smolBlackCat
smolBlackCat committed Dec 23, 2024
1 parent 00d3930 commit f8fb557
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 56 deletions.
101 changes: 55 additions & 46 deletions src/dialog/card-dialog.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Gtk::Entry>("card-title-entry")},
Expand All @@ -28,25 +27,17 @@ CardDetailsDialog::CardDetailsDialog(CardWidget& card_widget)
tasks_box{builder->get_widget<Gtk::Box>("tasks-box")},
notes_textbuffer{
builder->get_object<Gtk::TextBuffer>("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(
Expand All @@ -55,122 +46,140 @@ 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<CardDetailsDialog*>(data)->on_save();
reinterpret_cast<CardDetailsDialog*>(data)->close();
}),
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<Gtk::Widget&>(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];
strftime(date_str, 255, "%x", std::localtime(&time));
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);
}

if (card->get_notes() != new_notes) {
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);
auto new_date = Date{year{y}, month{static_cast<unsigned int>(m)},
day{static_cast<unsigned int>(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> 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<TaskWidget>(
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
23 changes: 17 additions & 6 deletions src/dialog/card-dialog.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ class CardDetailsDialog {
public:
static CardDetailsDialog* create(CardWidget& card_widget);

/**
* @brief Default CardDetailsDialog constructor
*/
CardDetailsDialog();

~CardDetailsDialog();

/**
Expand All @@ -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();

Expand All @@ -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<Gtk::Builder> builder;

Glib::RefPtr<Glib::Object> dialog;
Glib::RefPtr<Glib::Object> adw_dialog;
Gtk::Entry* card_title_entry;
Gtk::Button checklist_add_button;
Gtk::Button *unset_due_date_button, *card_delete_button;
Expand All @@ -65,7 +75,8 @@ class CardDetailsDialog {
Gtk::Box* tasks_box;
Glib::RefPtr<Gtk::TextBuffer> notes_textbuffer;

CardWidget& card_widget;
CardWidget* cur_card_widget;
std::vector<TaskWidget*> tasks_tracker;

private:
void _add_task(const std::shared_ptr<Task> task, bool is_new = false);
Expand Down
7 changes: 4 additions & 3 deletions src/widgets/card.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <numeric>

#include "cardlist-widget.h"
#include "window.h"

ui::CardWidget::CardWidget(BaseObjectType* cobject,
const Glib::RefPtr<Gtk::Builder>& builder,
Expand Down Expand Up @@ -308,14 +309,14 @@ void ui::CardWidget::open_color_dialog() {
}

void ui::CardWidget::open_card_details_dialog() {
auto& parent_window = *(static_cast<Gtk::Window*>(get_root()));
auto card_details_dialog = CardDetailsDialog::create(*this);
card_details_dialog->open(parent_window);
auto& parent_window = *(static_cast<ProgressWindow*>(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() {
Expand Down
17 changes: 16 additions & 1 deletion src/window.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -123,6 +124,7 @@ void ProgressWindow::add_local_board(BoardBackend board_backend) {
auto board_card_button = Gtk::make_managed<BoardCardButton>(board_backend);
auto fb_child_p = Gtk::make_managed<Gtk::FlowBoxChild>();
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]() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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(
Expand Down
5 changes: 5 additions & 0 deletions src/window.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <adwaita.h>
#include <gtkmm.h>
#include <widgets/board-widget.h>
#include "dialog/card-dialog.h"

namespace ui {

Expand Down Expand Up @@ -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<Gtk::CssProvider> css_provider;
Expand All @@ -95,6 +98,8 @@ class ProgressWindow : public Gtk::ApplicationWindow {
Glib::RefPtr<Gio::MenuModel> board_grid_menu_p, board_menu_p;
Gtk::MenuButton* app_menu_button_p;

CardDetailsDialog card_dialog;

void setup_menu_button();
void load_appropriate_style();

Expand Down

0 comments on commit f8fb557

Please sign in to comment.