Skip to content

Commit

Permalink
Remove mutex from stroke tool Controller/View exchanges - same thread
Browse files Browse the repository at this point in the history
  • Loading branch information
bhennion committed Sep 19, 2022
1 parent 52fdd28 commit c33e95a
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 36 deletions.
11 changes: 3 additions & 8 deletions src/core/view/overlays/StrokeToolFilledView.cpp
Expand Up @@ -2,7 +2,6 @@

#include <algorithm>
#include <cassert>
#include <mutex>

#include "model/Stroke.h"
#include "util/Color.h"
Expand Down Expand Up @@ -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);
Expand Down
31 changes: 9 additions & 22 deletions src/core/view/overlays/StrokeToolView.cpp
Expand Up @@ -3,7 +3,6 @@
#include <cassert>
#include <functional>
#include <memory>
#include <mutex>
#include <numeric>

#include "control/tools/StrokeHandler.h"
Expand Down Expand Up @@ -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);
}

Expand All @@ -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
Expand Down
8 changes: 2 additions & 6 deletions src/core/view/overlays/StrokeToolView.h
Expand Up @@ -10,7 +10,6 @@
*/
#pragma once

#include <mutex>
#include <vector>

#include <cairo.h>
Expand Down Expand Up @@ -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;
};
Expand Down

0 comments on commit c33e95a

Please sign in to comment.