From b7b6132395941081c6a57f68cb4cd255fb3f32d8 Mon Sep 17 00:00:00 2001 From: Andreas Butti Date: Fri, 1 Feb 2019 22:10:59 +0100 Subject: [PATCH 1/3] fixed #819 Toolbar ID working, but there is a double delete which needs to be fixed... --- src/control/Control.cpp | 33 +-------------- src/gui/dialog/ToolbarManageDialog.cpp | 3 ++ src/gui/toolbarMenubar/model/ToolbarModel.cpp | 42 ++++++++++++++++++- src/gui/toolbarMenubar/model/ToolbarModel.h | 1 + 4 files changed, 46 insertions(+), 33 deletions(-) diff --git a/src/control/Control.cpp b/src/control/Control.cpp index 6c76526e..554d8cd5 100644 --- a/src/control/Control.cpp +++ b/src/control/Control.cpp @@ -1029,38 +1029,7 @@ void Control::customizeToolbars() ToolbarData* data = new ToolbarData(*this->win->getSelectedToolbar()); ToolbarModel* model = this->win->getToolbarModel(); - - for (int i = 0; i < 100; i++) - { - string id = data->getId() + " Copy"; - - if (i != 0) - { - id += " "; - id += std::to_string(i); - } - - if (!model->existsId(id)) - { - if (i != 0) - { - string filename = data->getName(); - filename += " "; - filename += _("Copy"); - filename += " "; - filename += std::to_string(i); - - data->setName(filename); - } - else - { - data->setName(data->getName() + " " + _("Copy")); - } - data->setId(id); - break; - } - } - + model->initCopyNameId(data); model->add(data); this->win->toolbarSelected(data); this->win->updateToolbarMenu(); diff --git a/src/gui/dialog/ToolbarManageDialog.cpp b/src/gui/dialog/ToolbarManageDialog.cpp index c2e9eded..1d32d5fd 100644 --- a/src/gui/dialog/ToolbarManageDialog.cpp +++ b/src/gui/dialog/ToolbarManageDialog.cpp @@ -91,6 +91,8 @@ void ToolbarManageDialog::buttonNewCallback(GtkButton* button, ToolbarManageDial ToolbarData* data = new ToolbarData(false); data->setName(_("New")); + data->setId("custom"); + dlg->tbModel->initCopyNameId(data); dlg->addToolbarData(data); } @@ -129,6 +131,7 @@ void ToolbarManageDialog::buttonCopyCallback(GtkButton* button, ToolbarManageDia if (dlg->selected) { ToolbarData* data = new ToolbarData(*dlg->selected); + dlg->tbModel->initCopyNameId(data); dlg->addToolbarData(data); } } diff --git a/src/gui/toolbarMenubar/model/ToolbarModel.cpp b/src/gui/toolbarMenubar/model/ToolbarModel.cpp index e7c7f8c2..458c394e 100644 --- a/src/gui/toolbarMenubar/model/ToolbarModel.cpp +++ b/src/gui/toolbarMenubar/model/ToolbarModel.cpp @@ -2,6 +2,7 @@ #include "ToolbarData.h" #include +#include #include @@ -90,13 +91,52 @@ bool ToolbarModel::parse(string filename, bool predefined) return true; } +void ToolbarModel::initCopyNameId(ToolbarData* data) +{ + XOJ_CHECK_TYPE(ToolbarModel); + + for (int i = 0; i < 100; i++) + { + string id = data->getId() + " Copy"; + + if (i != 0) + { + id += " "; + id += std::to_string(i); + } + + if (!existsId(id)) + { + if (i != 0) + { + string filename = data->getName(); + filename += " "; + filename += _("Copy"); + filename += " "; + filename += std::to_string(i); + + data->setName(filename); + } + else + { + data->setName(data->getName() + " " + _("Copy")); + } + data->setId(id); + break; + } + } +} + bool ToolbarModel::existsId(string id) { XOJ_CHECK_TYPE(ToolbarModel); for (ToolbarData* data : this->toolbars) { - if (data->getId() == id) return true; + if (data->getId() == id) + { + return true; + } } return false; } diff --git a/src/gui/toolbarMenubar/model/ToolbarModel.h b/src/gui/toolbarMenubar/model/ToolbarModel.h index 11918d76..c76ed443 100644 --- a/src/gui/toolbarMenubar/model/ToolbarModel.h +++ b/src/gui/toolbarMenubar/model/ToolbarModel.h @@ -29,6 +29,7 @@ public: void remove(ToolbarData* data); void save(Path filename); bool existsId(string id); + void initCopyNameId(ToolbarData* data); private: void parseGroup(GKeyFile* config, const char* group, bool predefined); From 4ba308d196aca762465882656a894175a2ecb5ec Mon Sep 17 00:00:00 2001 From: Andreas Butti Date: Fri, 1 Feb 2019 22:24:27 +0100 Subject: [PATCH 2/3] Toolbar data, memory fixes, not all fixed yet --- src/gui/dialog/ToolbarManageDialog.cpp | 29 +++++++++++++++++--- src/gui/dialog/ToolbarManageDialog.h | 2 ++ src/gui/toolbarMenubar/model/ToolbarData.cpp | 13 +++++++++ src/gui/toolbarMenubar/model/ToolbarData.h | 2 ++ 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/gui/dialog/ToolbarManageDialog.cpp b/src/gui/dialog/ToolbarManageDialog.cpp index 1d32d5fd..d4d1bc7d 100644 --- a/src/gui/dialog/ToolbarManageDialog.cpp +++ b/src/gui/dialog/ToolbarManageDialog.cpp @@ -120,7 +120,8 @@ void ToolbarManageDialog::buttonDeleteCallback(GtkButton* button, ToolbarManageD while (gtk_tree_model_iter_next(GTK_TREE_MODEL(dlg->model), &iter)); } - dlg->entrySelected(NULL); + dlg->updateSelectionData(); + delete dlg->selected; } } @@ -192,19 +193,39 @@ void ToolbarManageDialog::entrySelected(ToolbarData* data) this->selected = data; } -void ToolbarManageDialog::treeSelectionChangedCallback(GtkTreeSelection* selection, ToolbarManageDialog* dlg) +void ToolbarManageDialog::updateSelectionData() { - XOJ_CHECK_TYPE_OBJ(dlg, ToolbarManageDialog); + XOJ_CHECK_TYPE(ToolbarManageDialog); GtkTreeIter iter; GtkTreeModel* model = NULL; ToolbarData* data = NULL; + GtkWidget* tree = get("toolbarList"); + + GtkTreeSelection* selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(tree)); + if (selection == NULL) + { + entrySelected(NULL); + return; + } + if (gtk_tree_selection_get_selected(selection, &model, &iter)) { gtk_tree_model_get(model, &iter, COLUMN_POINTER, &data, -1); - dlg->entrySelected(data); + entrySelected(data); } + else + { + entrySelected(NULL); + } +} + +void ToolbarManageDialog::treeSelectionChangedCallback(GtkTreeSelection* selection, ToolbarManageDialog* dlg) +{ + XOJ_CHECK_TYPE_OBJ(dlg, ToolbarManageDialog); + + dlg->updateSelectionData(); } void ToolbarManageDialog::show(GtkWindow* parent) diff --git a/src/gui/dialog/ToolbarManageDialog.h b/src/gui/dialog/ToolbarManageDialog.h index f7a40c8f..71dbd6af 100644 --- a/src/gui/dialog/ToolbarManageDialog.h +++ b/src/gui/dialog/ToolbarManageDialog.h @@ -37,6 +37,8 @@ private: void addToolbarData(ToolbarData* data); void entrySelected(ToolbarData* data); + void updateSelectionData(); + private: XOJ_TYPE_ATTRIB; diff --git a/src/gui/toolbarMenubar/model/ToolbarData.cpp b/src/gui/toolbarMenubar/model/ToolbarData.cpp index 8fc84770..bdd58ffd 100644 --- a/src/gui/toolbarMenubar/model/ToolbarData.cpp +++ b/src/gui/toolbarMenubar/model/ToolbarData.cpp @@ -21,6 +21,19 @@ ToolbarData::ToolbarData(const ToolbarData& data) this->predefined = false; } +void ToolbarData::operator=(const ToolbarData& other) +{ + this->id = other.id; + this->name = other.name; + this->predefined = other.predefined; + + contents.clear(); + for (const ToolbarEntry* e : other.contents) + { + contents.push_back(new ToolbarEntry(*e)); + } +} + ToolbarData::~ToolbarData() { XOJ_CHECK_TYPE(ToolbarData); diff --git a/src/gui/toolbarMenubar/model/ToolbarData.h b/src/gui/toolbarMenubar/model/ToolbarData.h index 6fec3dfd..3ac77d2d 100644 --- a/src/gui/toolbarMenubar/model/ToolbarData.h +++ b/src/gui/toolbarMenubar/model/ToolbarData.h @@ -22,6 +22,8 @@ public: ToolbarData(const ToolbarData& data); virtual ~ToolbarData(); + void operator=(const ToolbarData& other); + public: string getName(); void setName(string name); From ec6b2d7fc35de29e3ab7350060227e4b05a3aa41 Mon Sep 17 00:00:00 2001 From: Andreas Butti Date: Fri, 1 Feb 2019 22:36:23 +0100 Subject: [PATCH 3/3] Hopefully fixed all crashes and memory leaks in toolbar handling --- src/gui/dialog/ToolbarManageDialog.cpp | 73 +++++++++++--------- src/gui/dialog/ToolbarManageDialog.h | 4 +- src/gui/toolbarMenubar/model/ToolbarData.cpp | 2 + src/gui/toolbarMenubar/model/ToolbarEntry.h | 2 +- src/gui/toolbarMenubar/model/ToolbarItem.cpp | 1 - src/gui/toolbarMenubar/model/ToolbarItem.h | 4 ++ src/gui/toolbarMenubar/model/ToolbarModel.h | 4 ++ 7 files changed, 53 insertions(+), 37 deletions(-) diff --git a/src/gui/dialog/ToolbarManageDialog.cpp b/src/gui/dialog/ToolbarManageDialog.cpp index d4d1bc7d..bdbe74d3 100644 --- a/src/gui/dialog/ToolbarManageDialog.cpp +++ b/src/gui/dialog/ToolbarManageDialog.cpp @@ -12,13 +12,11 @@ enum }; ToolbarManageDialog::ToolbarManageDialog(GladeSearchpath* gladeSearchPath, ToolbarModel* model) - : GladeGui(gladeSearchPath, "toolbarManageDialog.glade", "DialogManageToolbar") + : GladeGui(gladeSearchPath, "toolbarManageDialog.glade", "DialogManageToolbar"), + tbModel(model) { XOJ_INIT_TYPE(ToolbarManageDialog); - this->tbModel = model; - this->selected = NULL; - GtkTreeIter iter; this->model = gtk_list_store_new(N_COLUMNS, G_TYPE_STRING, G_TYPE_INT, G_TYPE_POINTER, G_TYPE_BOOLEAN); gtk_list_store_append(this->model, &iter); @@ -60,7 +58,6 @@ ToolbarManageDialog::ToolbarManageDialog(GladeSearchpath* gladeSearchPath, Toolb COLUMN_EDITABLE, NULL); gtk_tree_view_append_column(GTK_TREE_VIEW(tree), column); - //gtk_tree_view_set_column_drag_function GtkTreeSelection* select = gtk_tree_view_get_selection(GTK_TREE_VIEW(tree)); gtk_tree_selection_set_mode(select, GTK_SELECTION_SINGLE); @@ -100,41 +97,47 @@ void ToolbarManageDialog::buttonDeleteCallback(GtkButton* button, ToolbarManageD { XOJ_CHECK_TYPE_OBJ(dlg, ToolbarManageDialog); - if (dlg->selected) + ToolbarData* selected = dlg->getSelectedEntry(); + if (selected == NULL) + { + return; + } + + dlg->tbModel->remove(selected); + GtkTreeIter iter; + if (gtk_tree_model_get_iter_first(GTK_TREE_MODEL(dlg->model), &iter)) { - dlg->tbModel->remove(dlg->selected); - GtkTreeIter iter; - if (gtk_tree_model_get_iter_first(GTK_TREE_MODEL(dlg->model), &iter)) + do { - do + ToolbarData* data = NULL; + gtk_tree_model_get(GTK_TREE_MODEL(dlg->model), &iter, COLUMN_POINTER, &data, -1); + + if (data == selected) { - ToolbarData* data = NULL; - gtk_tree_model_get(GTK_TREE_MODEL(dlg->model), &iter, COLUMN_POINTER, &data, -1); - - if (data == dlg->selected) - { - gtk_list_store_remove(dlg->model, &iter); - break; - } + gtk_list_store_remove(dlg->model, &iter); + break; } - while (gtk_tree_model_iter_next(GTK_TREE_MODEL(dlg->model), &iter)); } - - dlg->updateSelectionData(); - delete dlg->selected; + while (gtk_tree_model_iter_next(GTK_TREE_MODEL(dlg->model), &iter)); } + + dlg->updateSelectionData(); + delete selected; } void ToolbarManageDialog::buttonCopyCallback(GtkButton* button, ToolbarManageDialog* dlg) { XOJ_CHECK_TYPE_OBJ(dlg, ToolbarManageDialog); - if (dlg->selected) + ToolbarData* selected = dlg->getSelectedEntry(); + if (selected == NULL) { - ToolbarData* data = new ToolbarData(*dlg->selected); - dlg->tbModel->initCopyNameId(data); - dlg->addToolbarData(data); + return; } + + ToolbarData* data = new ToolbarData(*selected); + dlg->tbModel->initCopyNameId(data); + dlg->addToolbarData(data); } void ToolbarManageDialog::addToolbarData(ToolbarData* data) @@ -189,11 +192,9 @@ void ToolbarManageDialog::entrySelected(ToolbarData* data) gtk_widget_set_sensitive(btCopy, true); gtk_widget_set_sensitive(btDelete, !data->isPredefined()); } - - this->selected = data; } -void ToolbarManageDialog::updateSelectionData() +ToolbarData* ToolbarManageDialog::getSelectedEntry() { XOJ_CHECK_TYPE(ToolbarManageDialog); @@ -206,21 +207,27 @@ void ToolbarManageDialog::updateSelectionData() GtkTreeSelection* selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(tree)); if (selection == NULL) { - entrySelected(NULL); - return; + return NULL; } if (gtk_tree_selection_get_selected(selection, &model, &iter)) { gtk_tree_model_get(model, &iter, COLUMN_POINTER, &data, -1); - entrySelected(data); + return data; } else { - entrySelected(NULL); + return NULL; } } +void ToolbarManageDialog::updateSelectionData() +{ + XOJ_CHECK_TYPE(ToolbarManageDialog); + + entrySelected(getSelectedEntry()); +} + void ToolbarManageDialog::treeSelectionChangedCallback(GtkTreeSelection* selection, ToolbarManageDialog* dlg) { XOJ_CHECK_TYPE_OBJ(dlg, ToolbarManageDialog); diff --git a/src/gui/dialog/ToolbarManageDialog.h b/src/gui/dialog/ToolbarManageDialog.h index 71dbd6af..250a2ec2 100644 --- a/src/gui/dialog/ToolbarManageDialog.h +++ b/src/gui/dialog/ToolbarManageDialog.h @@ -39,11 +39,11 @@ private: void updateSelectionData(); + ToolbarData* getSelectedEntry(); + private: XOJ_TYPE_ATTRIB; ToolbarModel* tbModel; GtkListStore* model; - - ToolbarData* selected; }; diff --git a/src/gui/toolbarMenubar/model/ToolbarData.cpp b/src/gui/toolbarMenubar/model/ToolbarData.cpp index bdd58ffd..36b03957 100644 --- a/src/gui/toolbarMenubar/model/ToolbarData.cpp +++ b/src/gui/toolbarMenubar/model/ToolbarData.cpp @@ -23,6 +23,8 @@ ToolbarData::ToolbarData(const ToolbarData& data) void ToolbarData::operator=(const ToolbarData& other) { + XOJ_CHECK_TYPE(ToolbarData); + this->id = other.id; this->name = other.name; this->predefined = other.predefined; diff --git a/src/gui/toolbarMenubar/model/ToolbarEntry.h b/src/gui/toolbarMenubar/model/ToolbarEntry.h index 41e9a6f6..1b052c3a 100644 --- a/src/gui/toolbarMenubar/model/ToolbarEntry.h +++ b/src/gui/toolbarMenubar/model/ToolbarEntry.h @@ -24,9 +24,9 @@ public: void operator=(const ToolbarEntry& e); +public: void clearList(); -public: string getName(); void setName(string name); diff --git a/src/gui/toolbarMenubar/model/ToolbarItem.cpp b/src/gui/toolbarMenubar/model/ToolbarItem.cpp index cf9f72bd..916f52c8 100644 --- a/src/gui/toolbarMenubar/model/ToolbarItem.cpp +++ b/src/gui/toolbarMenubar/model/ToolbarItem.cpp @@ -21,7 +21,6 @@ ToolbarItem::ToolbarItem(const ToolbarItem& item) this->id = item.id; this->name = item.name; - this->sid = item.sid; } ToolbarItem::ToolbarItem() diff --git a/src/gui/toolbarMenubar/model/ToolbarItem.h b/src/gui/toolbarMenubar/model/ToolbarItem.h index 9380390c..2a071e99 100644 --- a/src/gui/toolbarMenubar/model/ToolbarItem.h +++ b/src/gui/toolbarMenubar/model/ToolbarItem.h @@ -21,6 +21,10 @@ public: ToolbarItem(); virtual ~ToolbarItem(); +private: + void operator=(const ToolbarItem& other); + +public: string getName(); bool operator==(ToolbarItem& other); diff --git a/src/gui/toolbarMenubar/model/ToolbarModel.h b/src/gui/toolbarMenubar/model/ToolbarModel.h index c76ed443..74542fb6 100644 --- a/src/gui/toolbarMenubar/model/ToolbarModel.h +++ b/src/gui/toolbarMenubar/model/ToolbarModel.h @@ -22,6 +22,10 @@ public: ToolbarModel(); virtual ~ToolbarModel(); +private: + ToolbarModel(const ToolbarModel& other); + void operator=(const ToolbarModel& other); + public: vector* getToolbars(); bool parse(string filename, bool predefined);