From e6037e9af902de3fea5585b1dc8d45cf5cf3d4fb Mon Sep 17 00:00:00 2001 From: Bryan Tan Date: Sat, 30 Jan 2021 11:57:42 -0800 Subject: [PATCH] Clean up page type control code --- src/control/Control.cpp | 10 +-- src/control/Control.h | 2 +- .../PageBackgroundChangeController.cpp | 73 ++++++++----------- src/control/PageBackgroundChangeController.h | 10 ++- src/control/pagetype/PageTypeMenu.cpp | 18 ++--- src/control/pagetype/PageTypeMenu.h | 10 +-- src/gui/dialog/PageTemplateDialog.cpp | 22 ++---- src/gui/dialog/PageTemplateDialog.h | 8 +- 8 files changed, 69 insertions(+), 84 deletions(-) diff --git a/src/control/Control.cpp b/src/control/Control.cpp index c46a9e5e..d5b501e3 100644 --- a/src/control/Control.cpp +++ b/src/control/Control.cpp @@ -81,7 +81,7 @@ Control::Control(GApplication* gtkApp, GladeSearchpath* gladeSearchPath): gtkApp TextView::setDpi(settings->getDisplayDpi()); this->pageTypes = new PageTypeHandler(gladeSearchPath); - this->newPageType = new PageTypeMenu(this->pageTypes, settings, true, true); + this->newPageType = std::make_unique(this->pageTypes, settings, true, true); this->audioController = new AudioController(this->settings, this); @@ -147,8 +147,6 @@ Control::~Control() { this->searchBar = nullptr; delete this->scrollHandler; this->scrollHandler = nullptr; - delete this->newPageType; - this->newPageType = nullptr; delete this->pageTypes; this->pageTypes = nullptr; delete this->metadata; @@ -1303,14 +1301,12 @@ void Control::updateBackgroundSizeButton() { } void Control::paperTemplate() { - auto* dlg = new PageTemplateDialog(this->gladeSearchPath, settings, pageTypes); + auto dlg = std::make_unique(this->gladeSearchPath, settings, pageTypes); dlg->show(GTK_WINDOW(this->win->getWindow())); if (dlg->isSaved()) { newPageType->loadDefaultPage(); } - - delete dlg; } void Control::paperFormat() { @@ -2847,7 +2843,7 @@ auto Control::getAudioController() -> AudioController* { return this->audioContr auto Control::getPageTypes() -> PageTypeHandler* { return this->pageTypes; } -auto Control::getNewPageType() -> PageTypeMenu* { return this->newPageType; } +auto Control::getNewPageType() -> PageTypeMenu* { return this->newPageType.get(); } auto Control::getPageBackgroundChangeController() -> PageBackgroundChangeController* { return this->pageBackgroundChangeController; diff --git a/src/control/Control.h b/src/control/Control.h index 59b5472c..3123c005 100644 --- a/src/control/Control.h +++ b/src/control/Control.h @@ -401,7 +401,7 @@ private: MetadataManager* metadata; PageTypeHandler* pageTypes; - PageTypeMenu* newPageType; + std::unique_ptr newPageType; PageBackgroundChangeController* pageBackgroundChangeController; diff --git a/src/control/PageBackgroundChangeController.cpp b/src/control/PageBackgroundChangeController.cpp index 94450d1e..09838cf2 100644 --- a/src/control/PageBackgroundChangeController.cpp +++ b/src/control/PageBackgroundChangeController.cpp @@ -15,23 +15,17 @@ PageBackgroundChangeController::PageBackgroundChangeController(Control* control): - control(control), - currentPageType(new PageTypeMenu(control->getPageTypes(), control->getSettings(), false, true)) { - currentPageType->setListener(this); + control(control), currentPageType(control->getPageTypes(), control->getSettings(), false, true) { + currentPageType.setListener(this); - currentPageType->hideCopyPage(); + currentPageType.hideCopyPage(); - currentPageType->addApplyBackgroundButton(this, true); + currentPageType.addApplyBackgroundButton(this, true); registerListener(control); } -PageBackgroundChangeController::~PageBackgroundChangeController() { - delete this->currentPageType; - this->currentPageType = nullptr; -} - -auto PageBackgroundChangeController::getMenu() -> GtkWidget* { return currentPageType->getMenu(); } +auto PageBackgroundChangeController::getMenu() -> GtkWidget* { return currentPageType.getMenu(); } void PageBackgroundChangeController::changeAllPagesBackground(const PageType& pt) { control->clearSelectionEndText(); @@ -41,28 +35,10 @@ void PageBackgroundChangeController::changeAllPagesBackground(const PageType& pt auto groupUndoAction = std::make_unique(); for (size_t p = 0; p < doc->getPageCount(); p++) { - PageRef page = doc->getPage(p); - if (!page) { - // Should not happen - continue; + auto undoAction = commitPageTypeChange(p, pt); + if (undoAction) { + groupUndoAction->addAction(undoAction.release()); } - - // Get values for Undo / Redo - double origW = page->getWidth(); - double origH = page->getHeight(); - BackgroundImage origBackgroundImage = page->getBackgroundImage(); - int origPdfPage = page->getPdfPageNr(); - PageType origType = page->getBackgroundType(); - - // Apply the new background - applyPageBackground(page, pt); - - control->firePageChanged(p); - control->updateBackgroundSizeButton(); - - UndoAction* undo = - new PageBackgroundChangedUndoAction(page, origType, origPdfPage, origBackgroundImage, origW, origH); - groupUndoAction->addAction(undo); } control->getUndoRedoHandler()->addUndoAction(std::move(groupUndoAction)); @@ -85,16 +61,31 @@ void PageBackgroundChangeController::changeCurrentPageBackground(PageType& pageT } Document* doc = control->getDocument(); - size_t pageNr = doc->indexOf(page); - if (pageNr == npos) { - return; // should not happen... + const size_t pageNr = doc->indexOf(page); + g_assert(pageNr != npos); + + auto undoAction = commitPageTypeChange(pageNr, pageType); + if (undoAction) { + control->getUndoRedoHandler()->addUndoAction(std::move(undoAction)); } +} + +auto PageBackgroundChangeController::commitPageTypeChange(const size_t pageNum, const PageType& pageType) + -> std::unique_ptr { + PageRef page = control->getDocument()->getPage(pageNum); + if (!page) { + return {}; + } + + Document* doc = control->getDocument(); + const size_t pageNr = doc->indexOf(page); + g_assert(pageNr != npos); // Get values for Undo / Redo - double origW = page->getWidth(); - double origH = page->getHeight(); + const double origW = page->getWidth(); + const double origH = page->getHeight(); BackgroundImage origBackgroundImage = page->getBackgroundImage(); - int origPdfPage = page->getPdfPageNr(); + const size_t origPdfPage = page->getPdfPageNr(); PageType origType = page->getBackgroundType(); // Apply the new background @@ -102,8 +93,8 @@ void PageBackgroundChangeController::changeCurrentPageBackground(PageType& pageT control->firePageChanged(pageNr); control->updateBackgroundSizeButton(); - control->getUndoRedoHandler()->addUndoAction(std::make_unique( - page, origType, origPdfPage, origBackgroundImage, origW, origH)); + return std::make_unique(page, origType, origPdfPage, origBackgroundImage, origW, + origH); } /** @@ -296,7 +287,7 @@ void PageBackgroundChangeController::pageSelected(size_t page) { } ignoreEvent = true; - currentPageType->setSelected(current->getBackgroundType()); + currentPageType.setSelected(current->getBackgroundType()); ignoreEvent = false; } diff --git a/src/control/PageBackgroundChangeController.h b/src/control/PageBackgroundChangeController.h index 98eb4663..d38605f0 100644 --- a/src/control/PageBackgroundChangeController.h +++ b/src/control/PageBackgroundChangeController.h @@ -24,6 +24,7 @@ class PageTypeMenu; class Control; class XojPage; +class UndoAction; class PageBackgroundChangeController: public PageTypeMenuChangeListener, @@ -31,7 +32,7 @@ class PageBackgroundChangeController: public PageTypeApplyListener { public: PageBackgroundChangeController(Control* control); - virtual ~PageBackgroundChangeController(); + virtual ~PageBackgroundChangeController() = default; public: virtual void changeCurrentPageBackground(PageType& pageType); @@ -80,8 +81,13 @@ private: */ bool applyImageBackground(PageRef page); + /** + * Perform the page type change. + */ + auto commitPageTypeChange(size_t pageNum, const PageType& pageType) -> std::unique_ptr; + private: Control* control = nullptr; - PageTypeMenu* currentPageType = nullptr; + PageTypeMenu currentPageType; bool ignoreEvent = false; }; diff --git a/src/control/pagetype/PageTypeMenu.cpp b/src/control/pagetype/PageTypeMenu.cpp index 9983b696..3f58d2c0 100644 --- a/src/control/pagetype/PageTypeMenu.cpp +++ b/src/control/pagetype/PageTypeMenu.cpp @@ -22,7 +22,6 @@ PageTypeMenu::PageTypeMenu(PageTypeHandler* types, Settings* settings, bool show listener(nullptr), menuX(0), menuY(0), - backgroundPainter(nullptr), showPreview(showPreview), pageTypeApplyListener(nullptr) { initDefaultMenu(); @@ -43,7 +42,7 @@ void PageTypeMenu::loadDefaultPage() { setSelected(model.getPageInsertType()); } -auto PageTypeMenu::createPreviewImage(const PageType& pt) -> cairo_surface_t* { +auto PageTypeMenu::createPreviewImage(MainBackgroundPainter* bgPainter, const PageType& pt) -> cairo_surface_t* { int previewWidth = 100; int previewHeight = 141; double zoom = 0.5; @@ -54,7 +53,7 @@ auto PageTypeMenu::createPreviewImage(const PageType& pt) -> cairo_surface_t* { cairo_t* cr = cairo_create(surface); cairo_scale(cr, zoom, zoom); - backgroundPainter->paint(pt, cr, std::move(page)); + bgPainter->paint(pt, cr, std::move(page)); cairo_identity_matrix(cr); @@ -71,13 +70,13 @@ auto PageTypeMenu::createPreviewImage(const PageType& pt) -> cairo_surface_t* { return surface; } -void PageTypeMenu::addMenuEntry(PageTypeInfo* t) { +void PageTypeMenu::addMenuEntry(MainBackgroundPainter* bgPainter, PageTypeInfo* t) { bool special = t->page.isSpecial(); bool showImg = !special && showPreview; GtkWidget* entry = nullptr; if (showImg) { - cairo_surface_t* img = createPreviewImage(t->page); + cairo_surface_t* img = createPreviewImage(bgPainter, t->page); GtkWidget* preview = gtk_image_new_from_surface(img); entry = gtk_check_menu_item_new(); @@ -220,8 +219,8 @@ auto PageTypeMenu::createApplyMenuItem(const char* text) -> GtkWidget* { } void PageTypeMenu::initDefaultMenu() { - this->backgroundPainter = new MainBackgroundPainter(); - this->backgroundPainter->setLineWidthFactor(2); + auto bgPainter = std::make_unique(); + bgPainter->setLineWidthFactor(2); bool special = false; for (PageTypeInfo* t: this->types->getPageTypes()) { @@ -246,11 +245,8 @@ void PageTypeMenu::initDefaultMenu() { gtk_container_add(GTK_CONTAINER(menu), separator); } } - addMenuEntry(t); + addMenuEntry(bgPainter.get(), t); } - - delete this->backgroundPainter; - this->backgroundPainter = nullptr; } auto PageTypeMenu::getMenu() -> GtkWidget* { return menu; } diff --git a/src/control/pagetype/PageTypeMenu.h b/src/control/pagetype/PageTypeMenu.h index 84f57f3d..d7825dc2 100644 --- a/src/control/pagetype/PageTypeMenu.h +++ b/src/control/pagetype/PageTypeMenu.h @@ -64,9 +64,9 @@ public: private: static GtkWidget* createApplyMenuItem(const char* text); void initDefaultMenu(); - void addMenuEntry(PageTypeInfo* t); + void addMenuEntry(MainBackgroundPainter* bgPainter, PageTypeInfo* t); void entrySelected(PageTypeInfo* t); - cairo_surface_t* createPreviewImage(const PageType& pt); + cairo_surface_t* createPreviewImage(MainBackgroundPainter* bgPainter, const PageType& pt); private: bool showSpecial; @@ -83,10 +83,8 @@ private: PageTypeMenuChangeListener* listener; - int menuX; - int menuY; - - MainBackgroundPainter* backgroundPainter; + guint menuX; + guint menuY; bool showPreview; diff --git a/src/gui/dialog/PageTemplateDialog.cpp b/src/gui/dialog/PageTemplateDialog.cpp index d842023e..72031b17 100644 --- a/src/gui/dialog/PageTemplateDialog.cpp +++ b/src/gui/dialog/PageTemplateDialog.cpp @@ -1,6 +1,7 @@ #include "PageTemplateDialog.h" #include +#include #include #include "control/pagetype/PageTypeHandler.h" @@ -18,7 +19,8 @@ using std::ofstream; PageTemplateDialog::PageTemplateDialog(GladeSearchpath* gladeSearchPath, Settings* settings, PageTypeHandler* types): GladeGui(gladeSearchPath, "pageTemplate.glade", "templateDialog"), settings(settings), - pageMenu(new PageTypeMenu(types, settings, true, false)) { + pageMenu(new PageTypeMenu(types, settings, true, false)), + popupMenuButton(new PopupMenuButton(get("btBackgroundDropdown"), pageMenu->getMenu())) { model.parse(settings->getPageTemplate()); pageMenu->setListener(this); @@ -36,17 +38,10 @@ PageTemplateDialog::PageTemplateDialog(GladeSearchpath* gladeSearchPath, Setting G_CALLBACK(+[](GtkToggleButton* togglebutton, PageTemplateDialog* self) { self->saveToFile(); }), this); - popupMenuButton = new PopupMenuButton(get("btBackgroundDropdown"), pageMenu->getMenu()); - updateDataFromModel(); } -PageTemplateDialog::~PageTemplateDialog() { - delete pageMenu; - pageMenu = nullptr; - delete popupMenuButton; - popupMenuButton = nullptr; -} +PageTemplateDialog::~PageTemplateDialog() = default; void PageTemplateDialog::updateDataFromModel() { GdkRGBA color = Util::rgb_to_GdkRGBA(model.getBackgroundColor()); @@ -146,11 +141,12 @@ void PageTemplateDialog::updatePageSize() { } void PageTemplateDialog::showPageSizeDialog() { - auto* dlg = new FormatDialog(getGladeSearchPath(), settings, model.getPageWidth(), model.getPageHeight()); + auto dlg = + std::make_unique(getGladeSearchPath(), settings, model.getPageWidth(), model.getPageHeight()); dlg->show(GTK_WINDOW(this->window)); - double width = dlg->getWidth(); - double height = dlg->getHeight(); + const double width = dlg->getWidth(); + const double height = dlg->getHeight(); if (width > 0) { model.setPageWidth(width); @@ -158,8 +154,6 @@ void PageTemplateDialog::showPageSizeDialog() { updatePageSize(); } - - delete dlg; } /** diff --git a/src/gui/dialog/PageTemplateDialog.h b/src/gui/dialog/PageTemplateDialog.h index 19ad67c3..0d8aeaf9 100644 --- a/src/gui/dialog/PageTemplateDialog.h +++ b/src/gui/dialog/PageTemplateDialog.h @@ -24,6 +24,10 @@ class PopupMenuButton; class PageTemplateDialog: public GladeGui, public PageTypeMenuChangeListener { public: PageTemplateDialog(GladeSearchpath* gladeSearchPath, Settings* settings, PageTypeHandler* types); + PageTemplateDialog(PageTemplateDialog&) = delete; + PageTemplateDialog(PageTemplateDialog&&) = delete; + PageTemplateDialog& operator=(PageTemplateDialog&) = delete; + PageTemplateDialog&& operator=(PageTemplateDialog&&) = delete; virtual ~PageTemplateDialog(); public: @@ -49,9 +53,9 @@ private: PageTemplateSettings model; - PageTypeMenu* pageMenu; + std::unique_ptr pageMenu; - PopupMenuButton* popupMenuButton; + std::unique_ptr popupMenuButton; /** * The dialog was confirmed / saved