Ensured memory safety for Elements

 - Every instance which theoretically owned a Element, models this now via an unique_ptr
 - Renamed *insertOrder* -> *insertionOrder*

refactor ElementContainer
 - the only purpose for ElementContainer is to use it in ElementContainerView, to draw all elements in the container
 - the draw function could be moved into the element container itself, or we can use a forEach implementation which can then be used elsewhere for other purposes.

VerticalToolHandler: rename getElements to refElements
 - made refElements private
 - only one use internally
 - still copies Element*
 - "ref" means, that instead of returning a reference to the datastructure holding the owned Elements, a copy of each Ptr is returned in a vector

src/core/control/Control.cpp

@@ -479,16 +479,18 @@ void Control::selectAllOnPage() {
 
     win->getXournal()->clearSelection();
 
-    std::vector<Element*> elements = layer->clearNoFree();
+    auto elements = layer->clearNoFree();
     this->doc->unlock();
 
     if (!elements.empty()) {
-        InsertionOrder insertOrder;
-        insertOrder.reserve(elements.size());
+        InsertionOrder insertionOrder;
+        insertionOrder.reserve(elements.size());
         Element::Index n = 0;
-        std::transform(elements.begin(), elements.end(), std::back_inserter(insertOrder),
-                       [&n](Element* e) { return InsertionPosition(e, n++); });
-        auto [sel, rg] = SelectionFactory::createFromFloatingElements(this, page, layer, view, std::move(insertOrder));
+        for (auto&& e: elements) {
+            insertionOrder.emplace_back(std::move(e), n++);
+        }
+        auto [sel, rg] =
+                SelectionFactory::createFromFloatingElements(this, page, layer, view, std::move(insertionOrder));
 
         page->fireRangeChanged(rg);
 
@@ -502,7 +504,7 @@ void Control::reorderSelection(EditSelection::OrderChange change) {
         return;
     }
 
-    auto undoAction = sel->rearrangeInsertOrder(change);
+    auto undoAction = sel->rearrangeInsertionOrder(change);
     this->undoRedo->addUndoAction(std::move(undoAction));
 }
 
@@ -1998,16 +2000,16 @@ void Control::clipboardCutCopyEnabled(bool enabled) {
 void Control::clipboardPasteEnabled(bool enabled) { this->actionDB->enableAction(Action::PASTE, enabled); }
 
 void Control::clipboardPasteText(string text) {
-    Text* t = new Text();
+    auto t = std::make_unique<Text>();
     t->setText(text);
     t->setFont(settings->getFont());
     t->setColor(toolHandler->getTool(TOOL_TEXT).getColor());
 
-    clipboardPaste(t);
+    clipboardPaste(std::move(t));
 }
 
 void Control::clipboardPasteImage(GdkPixbuf* img) {
-    auto image = new Image();
+    auto image = std::make_unique<Image>();
     image->setImage(img);
 
     auto width =
@@ -2046,10 +2048,10 @@ void Control::clipboardPasteImage(GdkPixbuf* img) {
     image->setWidth(scaledWidth);
     image->setHeight(scaledHeight);
 
-    clipboardPaste(image);
+    clipboardPaste(std::move(image));
 }
 
-void Control::clipboardPaste(Element* e) {
+void Control::clipboardPaste(ElementPtr e) {
     double x = 0;
     double y = 0;
     auto pageNr = getCurrentPageNo();
@@ -2078,8 +2080,8 @@ void Control::clipboardPaste(Element* e) {
     e->setX(x);
     e->setY(y);
 
-    undoRedo->addUndoAction(std::make_unique<InsertUndoAction>(page, layer, e));
-    auto sel = SelectionFactory::createFromFloatingElement(this, page, layer, view, e);
+    undoRedo->addUndoAction(std::make_unique<InsertUndoAction>(page, layer, e.get()));
+    auto sel = SelectionFactory::createFromFloatingElement(this, page, layer, view, std::move(e));
 
     win->getXournal()->setSelection(sel.release());
 }
@@ -2109,7 +2111,7 @@ void Control::clipboardPasteXournal(ObjectInputStream& in) {
     this->doc->unlock();
 
     try {
-        std::unique_ptr<Element> element;
+        ElementPtr element;
         std::string version = in.readString();
         if (version != PROJECT_STRING) {
             g_warning("Paste from Xournal Version %s to Xournal Version %s", version.c_str(), PROJECT_STRING);
@@ -2143,7 +2145,7 @@ void Control::clipboardPasteXournal(ObjectInputStream& in) {
 
             pasteAddUndoAction->addElement(layer, element.get(), layer->indexOf(element.get()));
             // Todo: unique_ptr
-            selection->addElement(element.release(), std::numeric_limits<Element::Index>::max());
+            selection->addElement(std::move(element), std::numeric_limits<Element::Index>::max());
         }
         undoRedo->addUndoAction(std::move(pasteAddUndoAction));
 

src/core/control/Control.h

@@ -324,7 +324,7 @@ class Control:
     void clipboardPasteXournal(ObjectInputStream& in) override;
     void deleteSelection() override;
 
-    void clipboardPaste(Element* e);
+    void clipboardPaste(ElementPtr e);
 
 public:
     void registerPluginToolButtons(ToolMenuHandler* toolMenuHandler);

src/core/control/GeometryToolController.cpp

@@ -1,5 +1,7 @@
 #include "GeometryToolController.h"
 
+#include <memory>
+
 #include "control/Control.h"
 #include "control/layer/LayerController.h"
 #include "gui/XournalView.h"
@@ -50,7 +52,7 @@ void GeometryToolController::scale(double f, double cx, double cy) {
 void GeometryToolController::markPoint(double x, double y) {
     const auto control = view->getXournal()->getControl();
     const auto h = control->getToolHandler();
-    Stroke* cross = new Stroke();
+    auto cross = std::make_unique<Stroke>();
     cross->setWidth(h->getToolThickness(TOOL_PEN)[TOOL_SIZE_FINE]);
     cross->setColor(h->getTool(TOOL_PEN).getColor());
     cross->addPoint(Point(x + MARK_SIZE, y + MARK_SIZE));
@@ -59,17 +61,19 @@ void GeometryToolController::markPoint(double x, double y) {
     cross->addPoint(Point(x + MARK_SIZE, y - MARK_SIZE));
     cross->addPoint(Point(x - MARK_SIZE, y + MARK_SIZE));
 
+    auto* ptr = cross.get();
     const auto doc = control->getDocument();
     const auto page = view->getPage();
     doc->lock();
     const auto layer = page->getSelectedLayer();
-    layer->addElement(cross);
+    layer->addElement(std::move(cross));
     doc->unlock();
 
     const auto undo = control->getUndoRedoHandler();
-    undo->addUndoAction(std::make_unique<InsertUndoAction>(page, layer, cross));
+    undo->addUndoAction(std::make_unique<InsertUndoAction>(page, layer, ptr));
+
 
-    const Rectangle<double> rect{cross->getX(), cross->getY(), cross->getElementWidth(), cross->getElementHeight()};
+    const Rectangle<double> rect{ptr->getX(), ptr->getY(), ptr->getElementWidth(), ptr->getElementHeight()};
     view->rerenderRect(rect.x, rect.y, rect.width, rect.height);
 }
 
@@ -79,15 +83,16 @@ void GeometryToolController::addStrokeToLayer() {
     const auto doc = control->getDocument();
     const auto page = view->getPage();
 
+    auto ptr = stroke.get();
     doc->lock();
     const auto layer = page->getSelectedLayer();
-    layer->addElement(stroke.get());
+    layer->addElement(std::move(this->stroke));
     doc->unlock();
+
     const auto undo = control->getUndoRedoHandler();
-    undo->addUndoAction(std::make_unique<InsertUndoAction>(page, layer, stroke.get()));
-    const Rectangle<double> rect{stroke->getX(), stroke->getY(), stroke->getElementWidth(), stroke->getElementHeight()};
+    undo->addUndoAction(std::make_unique<InsertUndoAction>(page, layer, ptr));
+    const Rectangle<double> rect{ptr->getX(), ptr->getY(), ptr->getElementWidth(), ptr->getElementHeight()};
     view->rerenderRect(rect.x, rect.y, rect.width, rect.height);
-    stroke.release();
     geometryTool->setStroke(nullptr);
     xournal->getCursor()->updateCursor();
 }

src/core/control/LatexController.cpp

@@ -123,7 +123,7 @@ void LatexController::findSelectedTexElement() {
 
         if (auto* img = dynamic_cast<TexImage*>(this->selectedElem)) {
             this->initialTex = img->getText();
-            this->temporaryRender = std::unique_ptr<TexImage>(img->clone());
+            this->temporaryRender = img->cloneTexImage();
             this->isValidTex = true;
         } else if (auto* txt = dynamic_cast<Text*>(this->selectedElem)) {
             this->initialTex = "\\text{" + txt->getText() + "}";
@@ -327,15 +327,16 @@ auto LatexController::loadRendered(string renderedTex) -> std::unique_ptr<TexIma
 void LatexController::insertTexImage() {
     xoj_assert(this->isValidTex);
     xoj_assert(this->temporaryRender != nullptr);
-    TexImage* img = this->temporaryRender.release();
 
     this->control->clearSelectionEndText();
     this->deleteOldImage();
 
-    control->getUndoRedoHandler()->addUndoAction(std::make_unique<InsertUndoAction>(page, layer, img));
+    control->getUndoRedoHandler()->addUndoAction(
+            std::make_unique<InsertUndoAction>(page, layer, this->temporaryRender.get()));
 
     // Select element
-    auto selection = SelectionFactory::createFromFloatingElement(control, page, layer, view, img);
+    auto selection =
+            SelectionFactory::createFromFloatingElement(control, page, layer, view, std::move(this->temporaryRender));
     view->getXournal()->setSelection(selection.release());
 }
 

src/core/control/SearchControl.cpp

@@ -41,9 +41,9 @@ auto SearchControl::search(const std::string& text, size_t index, size_t* occurr
                 continue;
             }
 
-            for (Element* e: l->getElements()) {
+            for (auto&& e: l->getElements()) {
                 if (e->getType() == ELEMENT_TEXT) {
-                    Text* t = dynamic_cast<Text*>(e);
+                    Text* t = dynamic_cast<Text*>(e.get());
 
                     std::vector<XojPdfRectangle> textResult = t->findText(text);
                     this->results.insert(this->results.end(), textResult.begin(), textResult.end());

src/core/control/UndoRedoController.cpp

@@ -8,6 +8,7 @@
 #include "gui/MainWindow.h"               // for MainWindow
 #include "gui/XournalView.h"              // for XournalView
 #include "model/Document.h"               // for Document
+#include "model/ElementInsertionPosition.h"
 #include "model/Layer.h"                  // for Layer
 #include "model/PageRef.h"                // for PageRef
 #include "model/XojPage.h"                // for XojPage
@@ -51,18 +52,18 @@ void UndoRedoController::after() {
         return;
     }
 
-    InsertionOrder remainingElements;
+    InsertionOrderRef remainingElements;
     for (Element* e: elements) {
-        // Test is the element has been removed since
+        // Test, if the element has been removed since
         if (auto pos = layer->indexOf(e); pos != -1) {
             remainingElements.emplace_back(e, pos);
         }
     }
     if (!remainingElements.empty()) {
-        layer->removeElementsAt(remainingElements, false);
+        auto removedElements = layer->removeElementsAt(remainingElements);
         lock.unlock();  // Not needed anymore. For all other paths, the lock is released via ~unique_lock()
         auto [sel, bounds] =
-                SelectionFactory::createFromFloatingElements(control, page, layer, view, std::move(remainingElements));
+                SelectionFactory::createFromFloatingElements(control, page, layer, view, std::move(removedElements));
         control->getWindow()->getXournal()->setSelection(sel.release());
         page->fireRangeChanged(bounds);
     }

src/core/control/shaperecognizer/CircleRecognizer.cpp

@@ -2,6 +2,7 @@
 
 #include <cmath>     // for hypot, cos, fabs, sin, M_PI
 #include <iterator>  // for begin, end, next
+#include <memory>
 #include <vector>    // for vector
 
 #include "model/Point.h"   // for Point
@@ -13,13 +14,13 @@
 /**
  * Create circle stroke for inertia
  */
-auto CircleRecognizer::makeCircleShape(Stroke* originalStroke, Inertia& inertia) -> Stroke* {
+auto CircleRecognizer::makeCircleShape(Stroke* originalStroke, Inertia& inertia) -> std::unique_ptr<Stroke> {
     int npts = static_cast<int>(2 * inertia.rad());
     if (npts < 24) {
         npts = 24;  // min. number of points
     }
 
-    auto* s = new Stroke();
+    auto s = std::make_unique<Stroke>();
     s->applyStyleFrom(originalStroke);
 
     for (int i = 0; i <= npts; i++) {
@@ -58,7 +59,7 @@ auto CircleRecognizer::scoreCircle(Stroke* s, Inertia& inertia) -> double {
     return sum / (divisor);
 }
 
-auto CircleRecognizer::recognize(Stroke* stroke) -> Stroke* {
+auto CircleRecognizer::recognize(Stroke* stroke) -> std::unique_ptr<Stroke> {
     Inertia s;
     s.calc(stroke->getPoints(), 0, static_cast<int>(stroke->getPointCount()));
     RDEBUG("Mass=%.0f, Center=(%.1f,%.1f), I=(%.0f,%.0f, %.0f), Rad=%.2f, Det=%.4f", s.getMass(), s.centerX(),

src/core/control/shaperecognizer/CircleRecognizer.h

@@ -11,6 +11,7 @@
 
 #pragma once
 
+#include <memory>
 class Stroke;
 class Inertia;
 
@@ -20,9 +21,9 @@ class CircleRecognizer {
     virtual ~CircleRecognizer();
 
 public:
-    static Stroke* recognize(Stroke* s);
+    static auto recognize(Stroke* s) -> std::unique_ptr<Stroke>;
 
 private:
-    static Stroke* makeCircleShape(Stroke* originalStroke, Inertia& inertia);
-    static double scoreCircle(Stroke* s, Inertia& inertia);
+    static auto makeCircleShape(Stroke* originalStroke, Inertia& inertia) -> std::unique_ptr<Stroke>;
+    static auto scoreCircle(Stroke* s, Inertia& inertia) -> double;
 };

src/core/control/shaperecognizer/ShapeRecognizer.cpp

@@ -35,7 +35,7 @@ void ShapeRecognizer::resetRecognizer() {
 /**
  *  Test if segments form standard shapes
  */
-auto ShapeRecognizer::tryRectangle() -> Stroke* {
+auto ShapeRecognizer::tryRectangle() -> std::unique_ptr<Stroke> {
     // first, we need whole strokes to combine to 4 segments...
     if (this->queueLength < 4) {
         return nullptr;
@@ -85,7 +85,7 @@ auto ShapeRecognizer::tryRectangle() -> Stroke* {
         avgAngle = M_PI / 2;
     }
 
-    auto* s = new Stroke();
+    auto s = std::make_unique<Stroke>();
     s->applyStyleFrom(this->stroke);
 
     for (int i = 0; i <= 3; i++) {
@@ -260,7 +260,7 @@ auto ShapeRecognizer::isStrokeLargeEnough(Stroke* stroke, double strokeMinSize)
 /**
  * The main pattern recognition function
  */
-auto ShapeRecognizer::recognizePatterns(Stroke* stroke, double strokeMinSize) -> Stroke* {
+auto ShapeRecognizer::recognizePatterns(Stroke* stroke, double strokeMinSize) -> std::unique_ptr<Stroke> {
     this->stroke = stroke;
 
     if (!isStrokeLargeEnough(stroke, strokeMinSize)) {
@@ -305,7 +305,7 @@ auto ShapeRecognizer::recognizePatterns(Stroke* stroke, double strokeMinSize) ->
             rs[i].calcSegmentGeometry(stroke->getPoints(), brk[i], brk[i + 1], ss + i);
         }
 
-        if (Stroke* result = tryRectangle(); result != nullptr) {
+        if (auto result = tryRectangle(); result != nullptr) {
             RDEBUG("return rectangle");
             return result;
         }
@@ -326,7 +326,7 @@ auto ShapeRecognizer::recognizePatterns(Stroke* stroke, double strokeMinSize) ->
                 aligned = false;
             }
 
-            auto* s = new Stroke();
+            auto s = std::make_unique<Stroke>();
             s->applyStyleFrom(this->stroke);
 
             if (aligned) {
@@ -344,7 +344,7 @@ auto ShapeRecognizer::recognizePatterns(Stroke* stroke, double strokeMinSize) ->
     }
 
     // not a polygon: maybe a circle ?
-    Stroke* s = CircleRecognizer::recognize(stroke);
+    auto s = CircleRecognizer::recognize(stroke);
     if (s) {
         RDEBUG("return circle");
         return s;

src/core/control/shaperecognizer/ShapeRecognizer.h

@@ -12,6 +12,7 @@
 #pragma once
 
 #include <array>  // for array
+#include <memory>
 
 #include "RecoSegment.h"
 #include "ShapeRecognizerConfig.h"  // for MAX_POLYGON_SIDES
@@ -26,11 +27,11 @@ class ShapeRecognizer {
     ShapeRecognizer();
     virtual ~ShapeRecognizer();
 
-    Stroke* recognizePatterns(Stroke* stroke, double strokeMinSize);
+    auto recognizePatterns(Stroke* stroke, double strokeMinSize) -> std::unique_ptr<Stroke>;
     void resetRecognizer();
 
 private:
-    Stroke* tryRectangle();
+    auto tryRectangle() -> std::unique_ptr<Stroke>;
     // function Stroke* tryArrow(); removed after commit a3f7a251282dcfea8b4de695f28ce52bf2035da2
 
     static void optimizePolygonal(const Point* pt, int nsides, int* breaks, Inertia* ss);

src/core/control/tools/BaseShapeHandler.cpp

@@ -110,12 +110,12 @@ void BaseShapeHandler::onButtonReleaseEvent(const PositionInputData& pos, double
 
     undo->addUndoAction(std::make_unique<InsertUndoAction>(page, layer, stroke.get()));
 
+    auto ptr = stroke.get();
     Document* doc = control->getDocument();
     doc->lock();
-    layer->addElement(stroke.get());
+    layer->addElement(std::move(stroke));
     doc->unlock();
-    page->fireElementChanged(stroke.get());
-    stroke.release();
+    page->fireElementChanged(ptr);
 
     control->getCursor()->updateCursor();
 }