From c33e95aab1137f2761744c9e40c0ce3c9d965696 Mon Sep 17 00:00:00 2001
From: Benjamin Hennion <benjamin.hennion@wanadoo.fr>
Date: Sun, 18 Sep 2022 14:33:10 +0200
Subject: [PATCH] Remove mutex from stroke tool Controller/View exchanges -
 same thread

---
 .../view/overlays/StrokeToolFilledView.cpp    | 11 ++-----
 src/core/view/overlays/StrokeToolView.cpp     | 31 ++++++-------------
 src/core/view/overlays/StrokeToolView.h       |  8 ++---
 3 files changed, 14 insertions(+), 36 deletions(-)

diff --git a/src/core/view/overlays/StrokeToolFilledView.cpp b/src/core/view/overlays/StrokeToolFilledView.cpp
index f28b2007cfb1..655e1038a09f 100644
--- a/src/core/view/overlays/StrokeToolFilledView.cpp
+++ b/src/core/view/overlays/StrokeToolFilledView.cpp
@@ -2,7 +2,6 @@
 
 #include <algorithm>
 #include <cassert>
-#include <mutex>
 
 #include "model/Stroke.h"
 #include "util/Color.h"
@@ -36,13 +35,9 @@ void StrokeToolFilledView::drawFilling(cairo_t* cr, const std::vector<Point>& pt
 }
 
 void StrokeToolFilledView::on(StrokeToolView::AddPointRequest, const Point& p) {
-    Point lastPoint;
-    {
-        std::lock_guard lock(this->bufferMutex);
-        assert(!this->pointBuffer.empty());
-        lastPoint = this->pointBuffer.back();
-        this->pointBuffer.emplace_back(p);
-    }
+    assert(!this->pointBuffer.empty());
+    Point lastPoint = this->pointBuffer.back();
+    this->pointBuffer.emplace_back(p);
     auto rg = this->getRepaintRange(lastPoint, p);
     // Add the first point, so that the range covers all the filling changes
     rg.addPoint(this->filling.firstPoint.x, this->filling.firstPoint.y);
diff --git a/src/core/view/overlays/StrokeToolView.cpp b/src/core/view/overlays/StrokeToolView.cpp
index 1407e4645dac..a0bcdd7f5f43 100644
--- a/src/core/view/overlays/StrokeToolView.cpp
+++ b/src/core/view/overlays/StrokeToolView.cpp
@@ -3,7 +3,6 @@
 #include <cassert>
 #include <functional>
 #include <memory>
-#include <mutex>
 #include <numeric>
 
 #include "control/tools/StrokeHandler.h"
@@ -74,36 +73,25 @@ void StrokeToolView::draw(cairo_t* cr) const {
 
 void StrokeToolView::on(StrokeToolView::AddPointRequest, const Point& p) {
     this->singleDot = false;
-    Point lastPoint;
-    {
-        std::lock_guard lock(this->bufferMutex);
-        assert(!this->pointBuffer.empty());  // front() is the last point we painted on the mask (see flushBuffer())
-        lastPoint = this->pointBuffer.back();
-        this->pointBuffer.emplace_back(p);
-    }
+    assert(!this->pointBuffer.empty());  // front() is the last point we painted on the mask (see flushBuffer())
+    Point lastPoint = this->pointBuffer.back();
+    this->pointBuffer.emplace_back(p);
     this->parent->flagDirtyRegion(this->getRepaintRange(lastPoint, p));
 }
 
 void StrokeToolView::on(StrokeToolView::ThickenFirstPointRequest, double newWidth) {
     assert(newWidth > 0.0);
-    Range rg;
-    {
-        std::lock_guard lock(this->bufferMutex);
-        assert(this->pointBuffer.size() == 1);
-        Point& p = this->pointBuffer.back();
-        assert(p.z <= newWidth);  // Thicken means thicken
-        p.z = newWidth;
-        rg = Range(p.x, p.y);
-    }
+    assert(this->pointBuffer.size() == 1);
+    Point& p = this->pointBuffer.back();
+    assert(p.z <= newWidth);  // Thicken means thicken
+    p.z = newWidth;
+    Range rg = Range(p.x, p.y);
     rg.addPadding(0.5 * newWidth);
     this->parent->flagDirtyRegion(rg);
 }
 
 void StrokeToolView::on(StrokeToolView::CancellationRequest, const Range& rg) {
-    {
-        std::lock_guard lock(this->bufferMutex);
-        this->pointBuffer.clear();
-    }
+    this->pointBuffer.clear();
     this->parent->flagDirtyRegion(rg);
 }
 
@@ -125,7 +113,6 @@ void StrokeToolView::drawDot(cairo_t* cr, const Point& p) const {
 
 std::vector<Point> StrokeToolView::flushBuffer() const {
     std::vector<Point> pts;
-    std::lock_guard lock(this->bufferMutex);
     std::swap(this->pointBuffer, pts);
     if (!pts.empty()) {
         // Keep the last point in the buffer - to be used in the next iteration
diff --git a/src/core/view/overlays/StrokeToolView.h b/src/core/view/overlays/StrokeToolView.h
index bb0ed41c0a83..10f8f3b0b58b 100644
--- a/src/core/view/overlays/StrokeToolView.h
+++ b/src/core/view/overlays/StrokeToolView.h
@@ -10,7 +10,6 @@
  */
 #pragma once
 
-#include <mutex>
 #include <vector>
 
 #include <cairo.h>
@@ -82,19 +81,16 @@ class StrokeToolView: public BaseStrokeToolView, public xoj::util::Listener<Stro
     mutable double dashOffset = 0;
 
     /**
-     * @brief Thread communication buffer and its mutex
+     * @brief Controller/View communication buffer
+     *      Those are in the same thread. Add mutex protection if this changes
      */
     mutable std::vector<Point> pointBuffer;  // Todo: implement a lock-free fifo?
-    mutable std::mutex bufferMutex;          //
 
     /**
      * @brief Drawing mask.
      *
      * The stroke is drawn to the mask and the mask is then blitted wherever needed.
      * Upon calls to draw(), the buffer is flushed and the corresponding part of stroke is added to the mask.
-     *
-     * WARNING: The mask is not mutex-protected. It can only be used in the drawing thread (i.e. in draw())
-     *      (or in constructors...)
      */
     mutable Mask mask;
 };