Clean up page type control code

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<PageTypeMenu>(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<PageTemplateDialog>(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;

src/control/Control.h

@@ -401,7 +401,7 @@ private:
     MetadataManager* metadata;
 
     PageTypeHandler* pageTypes;
-    PageTypeMenu* newPageType;
+    std::unique_ptr<PageTypeMenu> newPageType;
 
     PageBackgroundChangeController* pageBackgroundChangeController;
 

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<GroupUndoAction>();
 
     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<UndoAction> {
+    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<PageBackgroundChangedUndoAction>(
-            page, origType, origPdfPage, origBackgroundImage, origW, origH));
+    return std::make_unique<PageBackgroundChangedUndoAction>(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;
 }
 

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<UndoAction>;
+
 private:
     Control* control = nullptr;
-    PageTypeMenu* currentPageType = nullptr;
+    PageTypeMenu currentPageType;
     bool ignoreEvent = false;
 };

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<MainBackgroundPainter>();
+    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; }

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;
 

src/gui/dialog/PageTemplateDialog.cpp

@@ -1,6 +1,7 @@
 #include "PageTemplateDialog.h"
 
 #include <fstream>
+#include <memory>
 #include <sstream>
 
 #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<FormatDialog>(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;
 }
 
 /**

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<PageTypeMenu> pageMenu;
 
-    PopupMenuButton* popupMenuButton;
+    std::unique_ptr<PopupMenuButton> popupMenuButton;
 
     /**
      * The dialog was confirmed / saved