Refactor and simplify sidebar code

xournalpp · May 7, 2024 · 7273bf9 · 7273bf9
1 parent 336543b
commit 7273bf9
Show file tree

Hide file tree

Showing 41 changed files with 698 additions and 1,456 deletions.
diff --git a/src/core/control/Control.cpp b/src/core/control/Control.cpp
@@ -86,6 +86,7 @@
 #include "undo/InsertDeletePageUndoAction.h"                     // for Inse...
 #include "undo/InsertUndoAction.h"                               // for Inse...
 #include "undo/MoveSelectionToLayerUndoAction.h"                 // for Move...
+#include "undo/SwapUndoAction.h"                                 // for SwapUndoAction
 #include "undo/UndoAction.h"                                     // for Undo...
 #include "util/Assert.h"                                         // for xoj_assert
 #include "util/Color.h"                                          // for oper...
@@ -523,7 +524,7 @@ auto Control::firePageSelected(const PageRef& page) -> size_t {
         return npos;
     }
 
-    DocumentHandler::firePageSelected(pageId);
+    firePageSelected(pageId);
     return pageId;
 }
 
@@ -649,12 +650,14 @@ void Control::addDefaultPage(string pageTemplate) {
     this->doc->lock();
     this->doc->addPage(std::move(page));
     this->doc->unlock();
-
-    updateDeletePageButton();
 }
 
-void Control::updateDeletePageButton() {
-    this->actionDB->enableAction(Action::DELETE_PAGE, this->doc->getPageCount() > 1);
+void Control::updatePageActions() {
+    auto currentPage = getCurrentPageNo();
+    auto nbPages = this->doc->getPageCount();
+    this->actionDB->enableAction(Action::DELETE_PAGE, nbPages > 1);
+    this->actionDB->enableAction(Action::MOVE_PAGE_TOWARDS_BEGINNING, currentPage != 0);
+    this->actionDB->enableAction(Action::MOVE_PAGE_TOWARDS_END, currentPage < nbPages - 1);
 }
 
 void Control::deletePage() {
@@ -692,7 +695,6 @@ void Control::deletePage() {
     doc->deletePage(pNr);
     this->doc->unlock();
 
-    updateDeletePageButton();
     this->undoRedo->addUndoAction(std::make_unique<InsertDeletePageUndoAction>(page, pNr, false));
 
     if (pNr >= this->doc->getPageCount()) {
@@ -713,6 +715,106 @@ void Control::duplicatePage() {
     insertPage(pageCopy, getCurrentPageNo() + 1);
 }
 
+void Control::movePageTowardsBeginning() {
+    auto currentPageNo = this->getCurrentPageNo();
+    if (currentPageNo < 1) {
+        g_warning("Control::movePageTowardsBeginning() called on the first page");
+        return;
+    }
+    if (currentPageNo == npos) {
+        g_warning("Control::movePageTowardsBeginning() called with current page selected");
+        return;
+    }
+
+    auto lock = std::unique_lock(*this->doc);
+    PageRef page = this->doc->getPage(currentPageNo);
+    PageRef otherPage = doc->getPage(currentPageNo - 1);
+    if (!page || !otherPage) {
+        g_warning("Control::movePageTowardsBeginning() called but no page %zu or %zu", currentPageNo,
+                  currentPageNo - 1);
+        return;
+    }
+
+    doc->deletePage(currentPageNo);
+    doc->insertPage(page, currentPageNo - 1);
+    lock.unlock();
+
+    UndoRedoHandler* undo = this->getUndoRedoHandler();
+    undo->addUndoAction(std::make_unique<SwapUndoAction>(currentPageNo - 1, true, page, otherPage));
+
+    this->firePageDeleted(currentPageNo);
+    this->firePageInserted(currentPageNo - 1);
+    this->firePageSelected(currentPageNo - 1);
+
+    this->getScrollHandler()->scrollToPage(currentPageNo - 1);
+}
+
+
+void Control::movePageTowardsEnd() {
+    auto currentPageNo = this->getCurrentPageNo();
+    if (currentPageNo == npos) {
+        g_warning("Control::movePageTowardsEnd() called with current page selected");
+        return;
+    }
+
+    auto lock = std::unique_lock(*this->doc);
+    auto nbPage = this->doc->getPageCount();
+    if (currentPageNo >= nbPage - 1) {
+        g_warning("Control::movePageTowardsEnd() called on last page");
+        return;
+    }
+
+    PageRef page = this->doc->getPage(currentPageNo);
+    PageRef otherPage = doc->getPage(currentPageNo + 1);
+    if (!page || !otherPage) {
+        g_warning("Control::movePageTowardsEnd() called but no page %zu or %zu", currentPageNo, currentPageNo + 1);
+        return;
+    }
+
+    doc->deletePage(currentPageNo);
+    doc->insertPage(page, currentPageNo + 1);
+    lock.unlock();
+
+    this->undoRedo->addUndoAction(std::make_unique<SwapUndoAction>(currentPageNo, false, page, otherPage));
+
+    this->firePageDeleted(currentPageNo);
+    this->firePageInserted(currentPageNo + 1);
+    this->firePageSelected(currentPageNo + 1);
+
+    this->getScrollHandler()->scrollToPage(currentPageNo + 1);
+}
+
+void Control::askInsertPdfPage(size_t pdfPage) {
+    using Responses = enum { CANCEL = 1, AFTER = 2, END = 3 };
+    std::vector<XojMsgBox::Button> buttons = {{_("Cancel"), Responses::CANCEL},
+                                              {_("Insert after current page"), Responses::AFTER},
+                                              {_("Insert at end"), Responses::END}};
+
+    XojMsgBox::askQuestion(this->getGtkWindow(),
+                           FC(_F("Your current document does not contain PDF Page no {1}\n"
+                                 "Would you like to insert this page?\n\n"
+                                 "Tip: You can select Journal → Paper Background → PDF Background "
+                                 "to insert a PDF page.") %
+                              static_cast<int64_t>(pdfPage + 1)),
+                           "", buttons, [ctrl = this, pdfPage](int response) {
+                               if (response == Responses::AFTER || response == Responses::END) {
+                                   Document* doc = ctrl->getDocument();
+
+                                   doc->lock();
+                                   size_t position = response == Responses::AFTER ? ctrl->getCurrentPageNo() + 1 :
+                                                                                    doc->getPageCount();
+                                   XojPdfPageSPtr pdf = doc->getPdfPage(pdfPage);
+                                   doc->unlock();
+
+                                   if (pdf) {
+                                       auto page = std::make_shared<XojPage>(pdf->getWidth(), pdf->getHeight());
+                                       page->setBackgroundPdfPageNr(pdfPage);
+                                       ctrl->insertPage(page, position);
+                                   }
+                               }
+                           });
+}
+
 void Control::insertNewPage(size_t position, bool shouldScrollToPage) {
     pageBackgroundChangeController->insertNewPage(position, shouldScrollToPage);
 }
@@ -771,8 +873,7 @@ void Control::insertPage(const PageRef& page, size_t position, bool shouldScroll
         firePageSelected(position);
     }
 
-
-    updateDeletePageButton();
+    updatePageActions();
     undoRedo->addUndoAction(std::make_unique<InsertDeletePageUndoAction>(page, position, true));
 }
 
@@ -1462,7 +1563,7 @@ void Control::fileLoaded(int scrollToPage) {
     updateWindowTitle();
     win->getXournal()->forceUpdatePagenumbers();
     getCursor()->updateCursor();
-    updateDeletePageButton();
+    updatePageActions();
 }
 
 enum class MissingPdfDialogOptions : gint { USE_PROPOSED, SELECT_OTHER, REMOVE, CANCEL };

diff --git a/src/core/control/Control.h b/src/core/control/Control.h
@@ -224,11 +224,19 @@ class Control:
     void appendNewPdfPages();
     void insertPage(const PageRef& page, size_t position, bool shouldScrollToPage = true);
     void deletePage();
+    void movePageTowardsBeginning();
+    void movePageTowardsEnd();
 
     /**
-     * Disable / enable delete page button
+     * Ask the user whether a page with the given id
+     * should be added to the document.
      */
-    void updateDeletePageButton();
+    void askInsertPdfPage(size_t pdfPage);
+
+    /**
+     * Disable / enable page action buttons
+     */
+    void updatePageActions();
 
     // selection handling
     void clearSelection();

diff --git a/src/core/control/ScrollHandler.cpp b/src/core/control/ScrollHandler.cpp
@@ -68,7 +68,6 @@ void ScrollHandler::scrollToPage(size_t page, XojPdfRectangle rect) {
 
 void ScrollHandler::scrollToLinkDest(const LinkDestination& dest) {
     size_t pdfPage = dest.getPdfPage();
-    Sidebar* sidebar = control->getSidebar();
 
     if (pdfPage != npos) {
         Document* doc = control->getDocument();
@@ -77,7 +76,7 @@ void ScrollHandler::scrollToLinkDest(const LinkDestination& dest) {
         doc->unlock();
 
         if (page == npos) {
-            sidebar->askInsertPdfPage(pdfPage);
+            control->askInsertPdfPage(pdfPage);
         } else {
             if (dest.shouldChangeTop()) {
                 control->getScrollHandler()->scrollToPage(page, {dest.getLeft(), dest.getTop(), -1, -1});

diff --git a/src/core/control/actions/ActionProperties.h b/src/core/control/actions/ActionProperties.h
@@ -500,6 +500,15 @@ template <>
 struct ActionProperties<Action::DUPLICATE_PAGE> {
     static void callback(GSimpleAction*, GVariant*, Control* ctrl) { ctrl->duplicatePage(); }
 };
+template <>
+struct ActionProperties<Action::MOVE_PAGE_TOWARDS_BEGINNING> {
+    static void callback(GSimpleAction*, GVariant*, Control* ctrl) { ctrl->movePageTowardsBeginning(); }
+};
+template <>
+struct ActionProperties<Action::MOVE_PAGE_TOWARDS_END> {
+    static void callback(GSimpleAction*, GVariant*, Control* ctrl) { ctrl->movePageTowardsEnd(); }
+};
+
 template <>
 struct ActionProperties<Action::APPEND_NEW_PDF_PAGES> {
     static void callback(GSimpleAction*, GVariant*, Control* ctrl) { ctrl->appendNewPdfPages(); }
@@ -930,6 +939,26 @@ template <>
 struct ActionProperties<Action::LAYER_NEW> {
     static void callback(GSimpleAction*, GVariant*, Control* ctrl) { ctrl->getLayerController()->addNewLayer(); }
 };
+
+template <>
+struct ActionProperties<Action::LAYER_COPY> {
+    static void callback(GSimpleAction*, GVariant*, Control* ctrl) { ctrl->getLayerController()->copyCurrentLayer(); }
+};
+
+template <>
+struct ActionProperties<Action::LAYER_MOVE_UP> {
+    static void callback(GSimpleAction*, GVariant*, Control* ctrl) {
+        ctrl->getLayerController()->moveCurrentLayer(true);
+    }
+};
+
+template <>
+struct ActionProperties<Action::LAYER_MOVE_DOWN> {
+    static void callback(GSimpleAction*, GVariant*, Control* ctrl) {
+        ctrl->getLayerController()->moveCurrentLayer(false);
+    }
+};
+
 template <>
 struct ActionProperties<Action::LAYER_DELETE> {
     static void callback(GSimpleAction*, GVariant*, Control* ctrl) { ctrl->getLayerController()->deleteCurrentLayer(); }

diff --git a/src/core/control/jobs/PreviewJob.cpp b/src/core/control/jobs/PreviewJob.cpp
@@ -34,37 +34,21 @@ auto PreviewJob::getSource() -> void* { return this->sidebarPreview; }
 auto PreviewJob::getType() -> JobType { return JOB_TYPE_PREVIEW; }
 
 void PreviewJob::initGraphics() {
-    GtkAllocation alloc;
-    gtk_widget_get_allocation(this->sidebarPreview->widget, &alloc);
-    crBuffer = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, alloc.width, alloc.height);
-    zoom = this->sidebarPreview->sidebar->getZoom();
-    cr2 = cairo_create(crBuffer);
-}
-
-void PreviewJob::drawBorder() {
-    cairo_translate(cr2, Shadow::getShadowTopLeftSize() + 2, Shadow::getShadowTopLeftSize() + 2);
-    cairo_scale(cr2, zoom, zoom);
+    auto w = this->sidebarPreview->imageWidth;
+    auto h = this->sidebarPreview->imageHeight;
+    auto DPIscaling = this->sidebarPreview->DPIscaling;
+    buffer.reset(cairo_image_surface_create(CAIRO_FORMAT_ARGB32, w, h), xoj::util::adopt);
+    cairo_surface_set_device_scale(buffer.get(), DPIscaling, DPIscaling);
+    cr.reset(cairo_create(buffer.get()), xoj::util::adopt);
+    double zoom = this->sidebarPreview->sidebar->getZoom();
+    cairo_translate(cr.get(), Shadow::getShadowTopLeftSize() + 2, Shadow::getShadowTopLeftSize() + 2);
+    cairo_scale(cr.get(), zoom, zoom);
 }
 
 void PreviewJob::finishPaint() {
-    this->sidebarPreview->drawingMutex.lock();
-
-    if (this->sidebarPreview->crBuffer) {
-        cairo_surface_destroy(this->sidebarPreview->crBuffer);
-    }
-    this->sidebarPreview->crBuffer = crBuffer;
-
-    // The preview widget can be referenced after this is deleted.
-    // Only it should be referenced in the callback.
-    GtkWidget* previewWidget = this->sidebarPreview->widget;
-    g_object_ref(previewWidget);
-
-    Util::execInUiThread([previewWidget]() {
-        gtk_widget_queue_draw(previewWidget);
-        g_object_unref(previewWidget);
-    });
-
-    this->sidebarPreview->drawingMutex.unlock();
+    auto lock = std::lock_guard(this->sidebarPreview->drawingMutex);
+    this->sidebarPreview->buffer = std::move(this->buffer);
+    Util::execInUiThread([btn = this->sidebarPreview->button]() { gtk_widget_queue_draw(btn.get()); });
 }
 
 void PreviewJob::drawPage() {
@@ -82,17 +66,17 @@ void PreviewJob::drawPage() {
         layer = (dynamic_cast<SidebarPreviewLayerEntry*>(this->sidebarPreview))->getLayer();
     }
 
-    auto context = xoj::view::Context::createDefault(cr2);
+    auto context = xoj::view::Context::createDefault(cr.get());
 
     switch (type) {
         case RENDER_TYPE_PAGE_PREVIEW:
             // render all layers
-            view.drawPage(page, cr2, true);
+            view.drawPage(page, cr.get(), true);
             break;
 
         case RENDER_TYPE_PAGE_LAYER:
             // render single layer
-            view.initDrawing(page, cr2, true);
+            view.initDrawing(page, cr.get(), true);
             if (layer == 0) {
                 view.drawBackground(xoj::view::BACKGROUND_SHOW_ALL);
             } else {
@@ -105,7 +89,7 @@ void PreviewJob::drawPage() {
 
         case RENDER_TYPE_PAGE_LAYERSTACK:
             // render all layers up to layer
-            view.initDrawing(page, cr2, true);
+            view.initDrawing(page, cr.get(), true);
             view.drawBackground(xoj::view::BACKGROUND_SHOW_ALL);
             for (Layer::Index i = 0; i < layer; i++) {
                 Layer* drawLayer = (*page->getLayers())[i];
@@ -120,15 +104,14 @@ void PreviewJob::drawPage() {
             break;
     }
 
-    cairo_destroy(cr2);
     doc->unlock();
 }
 
 void PreviewJob::clipToPage() {
     // Only render within the preview page. Without this, the when preview jobs attempt
     // to clear the display, we fill a region larger than the inside of the preview page!
-    cairo_rectangle(cr2, 0, 0, this->sidebarPreview->page->getWidth(), this->sidebarPreview->page->getHeight());
-    cairo_clip(cr2);
+    cairo_rectangle(cr.get(), 0, 0, this->sidebarPreview->page->getWidth(), this->sidebarPreview->page->getHeight());
+    cairo_clip(cr.get());
 }
 
 void PreviewJob::run() {
@@ -137,7 +120,6 @@ void PreviewJob::run() {
     }
 
     initGraphics();
-    drawBorder();
     clipToPage();
     drawPage();
     finishPaint();

diff --git a/src/core/control/jobs/PreviewJob.h b/src/core/control/jobs/PreviewJob.h
@@ -13,6 +13,8 @@
 
 #include <cairo.h>  // for cairo_surface_t, cairo_t
 
+#include "util/raii/CairoWrappers.h"
+
 #include "Job.h"  // for Job, JobType
 
 class SidebarPreviewBaseEntry;
@@ -38,20 +40,19 @@ class PreviewJob: public Job {
 private:
     void initGraphics();
     void clipToPage();
-    void drawBorder();
     void finishPaint();
     void drawPage();
 
 private:
     /**
      * Graphics buffer
      */
-    cairo_surface_t* crBuffer = nullptr;
+    xoj::util::CairoSurfaceSPtr buffer;
 
     /**
      * Graphics drawing
      */
-    cairo_t* cr2 = nullptr;
+    xoj::util::CairoSPtr cr;
 
     /**
      * Zoom factor