From bf58c4cb7738e1b8918b83f97649e83e441ff678 Mon Sep 17 00:00:00 2001 From: rolandlo Date: Sun, 4 Dec 2022 07:44:02 +0100 Subject: [PATCH] Apply more reviewer suggestions --- src/core/control/Control.cpp | 69 ++++++---- src/core/control/GeometryToolController.cpp | 10 +- src/core/control/GeometryToolController.h | 8 +- src/core/control/SetsquareController.cpp | 7 +- src/core/control/SetsquareController.h | 2 +- .../inputdevices/GeometryToolInputHandler.cpp | 127 +++++++++--------- .../inputdevices/GeometryToolInputHandler.h | 9 +- src/core/model/GeometryTool.cpp | 5 +- src/core/model/GeometryTool.h | 2 +- src/core/model/Setsquare.cpp | 3 +- src/core/model/Setsquare.h | 7 + src/core/view/SetsquareView.cpp | 3 +- src/core/view/SetsquareView.h | 2 +- 13 files changed, 139 insertions(+), 115 deletions(-) diff --git a/src/core/control/Control.cpp b/src/core/control/Control.cpp index f35502502617..31f704e6cc43 100644 --- a/src/core/control/Control.cpp +++ b/src/core/control/Control.cpp @@ -649,18 +649,15 @@ void Control::actionPerformed(ActionType type, ActionGroup group, GtkToolButton* selectTool(TOOL_HAND); } break; - case ACTION_SETSQUARE: - if (auto xournal = this->win->getXournal(); - !this->geometryToolController || - this->geometryToolController->getType() != GeometryToolType::SETSQUARE) { - resetGeometryTool(); + case ACTION_SETSQUARE: { + bool needsNewSetsquare = !this->geometryToolController || + this->geometryToolController->getType() != GeometryToolType::SETSQUARE; + resetGeometryTool(); + if (needsNewSetsquare) { makeGeometryTool(GeometryToolType::SETSQUARE); - xournal->getViewFor(getCurrentPageNo())->rerenderPage(); - } else { - resetGeometryTool(); - xournal->getViewFor(getCurrentPageNo())->rerenderPage(); } break; + } case ACTION_TOOL_FLOATING_TOOLBOX: if (enabled) { selectTool(TOOL_FLOATING_TOOLBOX); @@ -1066,25 +1063,40 @@ void Control::actionPerformed(ActionType type, ActionGroup group, GtkToolButton* void Control::makeGeometryTool(GeometryToolType tool) { auto view = this->win->getXournal()->getViewFor(getCurrentPageNo()); - if (tool == GeometryToolType::SETSQUARE) { - auto setsquare = new Setsquare(); - view->addOverlayView(std::make_unique(setsquare, view, zoom)); - this->geometryTool = std::unique_ptr(setsquare); - this->geometryToolController = std::make_unique(view, setsquare); - std::unique_ptr geometryToolInputHandler = - std::make_unique(this->win->getXournal(), geometryToolController.get()); - geometryToolInputHandler->registerToPool(setsquare->getHandlerPool()); - auto xournal = GTK_XOURNAL(this->win->getXournal()->getWidget()); - xournal->input->setGeometryToolInputHandler(std::move(geometryToolInputHandler)); - fireActionSelected(GROUP_GEOMETRY_TOOL, ACTION_SETSQUARE); + auto* xournal = GTK_XOURNAL(this->win->getXournal()->getWidget()); + switch (tool) { + case SETSQUARE: { + auto setsquare = new Setsquare(); + view->addOverlayView(std::make_unique(setsquare, view, zoom)); + this->geometryTool = std::unique_ptr(setsquare); + this->geometryToolController = std::make_unique(view, setsquare); + std::unique_ptr geometryToolInputHandler = + std::make_unique(this->win->getXournal(), geometryToolController.get()); + geometryToolInputHandler->registerToPool(setsquare->getHandlerPool()); + xournal->input->setGeometryToolInputHandler(std::move(geometryToolInputHandler)); + fireActionSelected(GROUP_GEOMETRY_TOOL, ACTION_SETSQUARE); + geometryTool->notify(); + break; + } + default: + g_warning("Unknown geometry tool type %d", tool); } } void Control::resetGeometryTool() { + Range rg; + bool hasGeometryTool = false; + if (this->geometryTool) { + hasGeometryTool = true; + rg = this->geometryTool->getToolRange(true); + } this->geometryToolController.reset(); this->geometryTool.reset(); - auto xournal = GTK_XOURNAL(this->win->getXournal()->getWidget()); + auto* xournal = GTK_XOURNAL(this->win->getXournal()->getWidget()); xournal->input->resetGeometryToolInputHandler(); + if (win && hasGeometryTool) { + (win->getXournal()->getViewFor(getCurrentPageNo()))->rerenderRange(rg); + } fireActionSelected(GROUP_GEOMETRY_TOOL, ACTION_NONE); } @@ -1388,11 +1400,14 @@ void Control::deletePage() { // if the current page contains the geometry tool, reset it size_t pNr = getCurrentPageNo(); - doc->lock(); - if (geometryToolController && doc->indexOf(geometryToolController->getPage()) == pNr) { - resetGeometryTool(); + if (geometryToolController) { + doc->lock(); + auto page = doc->indexOf(geometryToolController->getPage()); + doc->unlock(); + if (page == pNr) { + resetGeometryTool(); + } } - doc->unlock(); // don't allow delete pages if we have less than 2 pages, // so we can be (more or less) sure there is at least one page. if (this->doc->getPageCount() < 2) { @@ -2271,8 +2286,8 @@ auto Control::openFile(fs::path filepath, int scrollToPage, bool forceOpen) -> b parentFolderPath = missingFilePath.parent_path().string(); filename = missingFilePath.filename().string(); #else - // since POSIX systems detect the whole Windows path as a filename, this checks whether missingFilePath contains - // a Windows path + // since POSIX systems detect the whole Windows path as a filename, this checks whether missingFilePath + // contains a Windows path std::regex regex(R"([A-Z]:\\(?:.*\\)*(.*))"); std::cmatch matchInfo; diff --git a/src/core/control/GeometryToolController.cpp b/src/core/control/GeometryToolController.cpp index 375a18e7602c..f6cf5c5a9688 100644 --- a/src/core/control/GeometryToolController.cpp +++ b/src/core/control/GeometryToolController.cpp @@ -19,7 +19,7 @@ GeometryToolController::GeometryToolController(XojPageView* view, GeometryTool* GeometryToolController::~GeometryToolController() = default; -void GeometryToolController::move(double x, double y) { +void GeometryToolController::translate(double x, double y) { geometryTool->setTranslationX(geometryTool->getTranslationX() + x); geometryTool->setTranslationY(geometryTool->getTranslationY() + y); geometryTool->notify(); @@ -59,15 +59,17 @@ 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)); + const auto doc = control->getDocument(); const auto page = view->getPage(); + doc->lock(); control->getLayerController()->ensureLayerExists(page); const auto layer = page->getSelectedLayer(); + layer->addElement(cross); + doc->unlock(); const auto undo = control->getUndoRedoHandler(); undo->addUndoAction(std::make_unique(page, layer, cross)); - layer->addElement(cross); - const Rectangle rect{cross->getX(), cross->getY(), cross->getElementWidth(), cross->getElementHeight()}; view->rerenderRect(rect.x, rect.y, rect.width, rect.height); } @@ -116,6 +118,6 @@ void GeometryToolController::initializeStroke() { } } -auto GeometryToolController::getPage() const -> PageRef { return view->getPage(); } +auto GeometryToolController::getPage() const -> const PageRef { return view->getPage(); } auto GeometryToolController::getView() const -> XojPageView* { return view; } diff --git a/src/core/control/GeometryToolController.h b/src/core/control/GeometryToolController.h index 7e9590dc09cd..42fcc3b15263 100644 --- a/src/core/control/GeometryToolController.h +++ b/src/core/control/GeometryToolController.h @@ -34,11 +34,11 @@ class GeometryToolController { public: /** - * @brief moves the geometry tool in x- and y-direction + * @brief translates the geometry tool in x- and y-direction * @param x the translation in x-direction (in document coordinates) * @param y the translation in y-direction (in document coordinates) */ - void move(double x, double y); + void translate(double x, double y); /** * @brief rotates the geometry tool around its rotation center @@ -57,7 +57,7 @@ class GeometryToolController { void scale(double f, double cx, double cy); /** - * @brief marks a point with a "x" + * @brief marks a point with a "x" and puts the mark onto the current layer * @param x the x-coordinate of the point (in document coordinates) * @param y the y-coordinate of the point (in document coordinates) */ @@ -81,7 +81,7 @@ class GeometryToolController { /** * @brief the page with respect to which the setsquare is initialized */ - PageRef getPage() const; + const PageRef getPage() const; virtual bool isInsideGeometryTool(double x, double y, double border = 0.0) const = 0; diff --git a/src/core/control/SetsquareController.cpp b/src/core/control/SetsquareController.cpp index 16dd2bf8d155..7adac8d27df0 100644 --- a/src/core/control/SetsquareController.cpp +++ b/src/core/control/SetsquareController.cpp @@ -7,7 +7,6 @@ #include "model/Setsquare.h" #include "model/Stroke.h" #include "model/XojPage.h" -#include "undo/InsertUndoAction.h" // for InsertUndoAction using xoj::util::Rectangle; SetsquareController::SetsquareController(XojPageView* view, Setsquare* setsquare): @@ -89,7 +88,7 @@ void SetsquareController::updateRadialStroke(double x, double y) { const auto p = posRelToSide(HYPOTENUSE, x, y); const auto rad = std::hypot(p.x, p.y); - if (rad >= geometryTool->getHeight() / std::sqrt(2.) - 1.15 || p.y > 0) { // TODO + if (rad >= Setsquare::radiusFromHeight(geometryTool->getHeight()) || p.y > 0) { this->strokeAngle = std::atan2(p.y, p.x); stroke->addPoint(Point(x, y)); } else { @@ -104,7 +103,7 @@ void SetsquareController::updateRadialStroke(double x, double y) { } void SetsquareController::finalizeEdgeStroke() { - hypotenuseMax = std::numeric_limits::min(); + hypotenuseMax = std::numeric_limits::lowest(); hypotenuseMin = std::numeric_limits::max(); addStrokeToLayer(); } @@ -114,6 +113,6 @@ void SetsquareController::finalizeRadialStroke() { addStrokeToLayer(); } -auto SetsquareController::existsEdgeStroke() -> bool { return !(hypotenuseMax == std::numeric_limits::min()); } +auto SetsquareController::existsEdgeStroke() -> bool { return hypotenuseMax != std::numeric_limits::lowest(); } auto SetsquareController::existsRadialStroke() -> bool { return !std::isnan(strokeAngle); } diff --git a/src/core/control/SetsquareController.h b/src/core/control/SetsquareController.h index 318a739305a3..518bbf1a0246 100644 --- a/src/core/control/SetsquareController.h +++ b/src/core/control/SetsquareController.h @@ -120,7 +120,7 @@ class SetsquareController: public GeometryToolController { * x-coordinates of the point to be drawn (with respect to an unrotated, and untranslated coordinate system) are * saved in the variables hypotenuseMin and hypotenuseMax */ - double hypotenuseMax = std::numeric_limits::min(); + double hypotenuseMax = std::numeric_limits::lowest(); double hypotenuseMin = std::numeric_limits::max(); /** diff --git a/src/core/gui/inputdevices/GeometryToolInputHandler.cpp b/src/core/gui/inputdevices/GeometryToolInputHandler.cpp index 93b81bed7afe..891259781ede 100644 --- a/src/core/gui/inputdevices/GeometryToolInputHandler.cpp +++ b/src/core/gui/inputdevices/GeometryToolInputHandler.cpp @@ -1,3 +1,5 @@ +#include "GeometryToolInputHandler.h" + #include // for max #include // for atan2, abs, cos, remainder #include // for __shared_ptr_access, shar... @@ -13,6 +15,7 @@ #include "gui/XournalView.h" // for XournalView #include "gui/inputdevices/InputContext.h" // for InputContext::DeviceType #include "gui/inputdevices/InputEvents.h" // for InputEvent, BUTTON_PRESS_... +#include "model/Document.h" // for Document #include "model/Element.h" // for Element, ELEMENT_STROKE #include "model/GeometryTool.h" // for HALF_CM #include "model/Layer.h" // for Layer @@ -20,8 +23,6 @@ #include "model/Stroke.h" // for Stroke #include "model/XojPage.h" // for XojPage -#include "GeometryToolInputHandler.h" - constexpr double MOVE_AMOUNT = HALF_CM / 2.0; constexpr double MOVE_AMOUNT_SMALL = HALF_CM / 20.0; constexpr double ROTATE_AMOUNT = M_PI * 5.0 / 180.0; @@ -73,8 +74,7 @@ void GeometryToolInputHandler::on(UpdateValuesRequest, double h, double rot, dou } auto GeometryToolInputHandler::handleTouchscreen(InputEvent const& event) -> bool { - const auto zoomGesturesEnabled = xournal->getControl()->getSettings()->isZoomGesturesEnabled(); - // Don't handle more then 2 inputs + // Don't handle more than 2 inputs if (this->primarySequence && this->primarySequence != event.sequence && this->secondarySequence && this->secondarySequence != event.sequence) { return false; @@ -82,7 +82,7 @@ auto GeometryToolInputHandler::handleTouchscreen(InputEvent const& event) -> boo if (event.type == BUTTON_PRESS_EVENT) { // Start scrolling when a sequence starts and we currently have none other if (this->primarySequence == nullptr && this->secondarySequence == nullptr) { - auto coords = getCoords(event); + const utl::Point coords = getCoords(event); if (controller->isInsideGeometryTool(coords.x, coords.y, 0)) { // Set sequence data @@ -101,39 +101,32 @@ auto GeometryToolInputHandler::handleTouchscreen(InputEvent const& event) -> boo // Set sequence data sequenceStart(event); - - // Even if zoom gestures are disabled, - // this is still the start of a sequence. Just - // don't start zooming. - if (zoomGesturesEnabled) { - zoomStart(); - return true; - } + rotateAndZoomStart(); + return true; } } if (event.type == MOTION_EVENT && this->primarySequence) { - if (this->primarySequence && this->secondarySequence && zoomGesturesEnabled) { - zoomMotion(event); + if (this->secondarySequence) { + rotateAndZoomMotion(event); return true; } else if (event.sequence == this->primarySequence) { scrollMotion(event); return true; - } else if (this->primarySequence && this->secondarySequence) { - sequenceStart(event); - return true; } + return false; } if (event.type == BUTTON_RELEASE_EVENT) { if (event.sequence == this->primarySequence) { // If secondarySequence is nullptr, this sets primarySequence // to nullptr. If it isn't, then it is now the primary sequence! - this->primarySequence = this->secondarySequence; - this->secondarySequence = nullptr; + this->primarySequence = std::exchange(this->secondarySequence, nullptr); this->priLastPageRel = this->secLastPageRel; - } else { + return true; + } else if (event.sequence == this->secondarySequence) { this->secondarySequence = nullptr; + return true; } } return false; @@ -141,15 +134,16 @@ auto GeometryToolInputHandler::handleTouchscreen(InputEvent const& event) -> boo auto GeometryToolInputHandler::handleKeyboard(InputEvent const& event) -> bool { GdkEvent* gdkEvent = event.sourceEvent; - if (gdkEvent->type == GDK_KEY_PRESS) { - auto keyEvent = reinterpret_cast(gdkEvent); - + if (gdk_event_get_event_type(gdkEvent) == GDK_KEY_PRESS) { double xdir = 0; double ydir = 0; double angle = 0.0; double scale = 1.0; - - switch (keyEvent->keyval) { + guint key; + if (!gdk_event_get_keyval(gdkEvent, &key)) { + return false; + } + switch (key) { case GDK_KEY_Left: xdir = -1; break; @@ -172,7 +166,7 @@ auto GeometryToolInputHandler::handleKeyboard(InputEvent const& event) -> bool { case GDK_KEY_S: { scale = (event.state & GDK_MOD1_MASK) ? SCALE_AMOUNT_SMALL : SCALE_AMOUNT; scale = (event.state & GDK_SHIFT_MASK) ? 1.0 / scale : scale; - const auto h = height * scale; + const double h = height * scale; if (h > getMaxHeight() || h < getMinHeight()) { scale = 1.0; } @@ -185,11 +179,11 @@ auto GeometryToolInputHandler::handleKeyboard(InputEvent const& event) -> bool { } if (xdir != 0 || ydir != 0) { - const auto c = std::cos(rotation); - const auto s = std::sin(rotation); + const double c = std::cos(rotation); + const double s = std::sin(rotation); double xshift{0.0}; double yshift{0.0}; - const auto amount = (event.state & GDK_MOD1_MASK) ? MOVE_AMOUNT_SMALL : MOVE_AMOUNT; + const double amount = (event.state & GDK_MOD1_MASK) ? MOVE_AMOUNT_SMALL : MOVE_AMOUNT; if (event.state & GDK_SHIFT_MASK) { xshift = amount * (c * xdir - s * ydir); yshift = amount * (s * xdir + c * ydir); @@ -197,7 +191,7 @@ auto GeometryToolInputHandler::handleKeyboard(InputEvent const& event) -> bool { xshift = amount * xdir; yshift = amount * ydir; } - controller->move(xshift, yshift); + controller->translate(xshift, yshift); return true; } @@ -222,42 +216,46 @@ void GeometryToolInputHandler::sequenceStart(InputEvent const& event) { } const auto page = controller->getPage(); - const auto l = page->getSelectedLayer(); + const Layer* layer = page->getSelectedLayer(); this->lines.clear(); - for (const auto& e: l->getElements()) { + Document* doc = xournal->getDocument(); + doc->lock(); + // Performance improvement might be obtained by avoiding filtering all elements each + // time a finger has been put onto the screen + for (const auto& e: layer->getElements()) { if (e->getType() == ELEMENT_STROKE) { - auto s = dynamic_cast(e); + Stroke* s = dynamic_cast(e); if (s->getPointCount() == 2) { lines.push_back(s); } } } + doc->unlock(); } void GeometryToolInputHandler::scrollMotion(InputEvent const& event) { - // Will only be called if there is a single sequence (zooming handles two sequences) + // Will only be called if there is a single sequence (rotation/zooming handles two sequences) auto offset = [&]() { - auto coords = this->getCoords(event); + utl::Point coords = this->getCoords(event); if (event.sequence == this->primarySequence) { - const auto offset = coords - this->priLastPageRel; + const utl::Point offset = coords - this->priLastPageRel; this->priLastPageRel = coords; return offset; } else { - const auto offset = coords - this->secLastPageRel; + const utl::Point offset = coords - this->secLastPageRel; this->secLastPageRel = coords; return offset; } }(); const auto pos = Point(translationX + offset.x, translationY + offset.y); double minDist = SNAPPING_DISTANCE_TOLERANCE; - const auto angleSetsquare = rotation; double diffAngle{NAN}; for (const auto& l: lines) { - auto first = l->getPoint(0); - auto second = l->getPoint(1); - auto dist = Snapping::distanceLine(pos, first, second); - auto angleLine = std::atan2(second.y - first.y, second.x - first.x); - auto diff = std::remainder(angleLine - angleSetsquare, M_PI_2); + const Point first = l->getPoint(0); + const Point second = l->getPoint(1); + const double dist = Snapping::distanceLine(pos, first, second); + const double angleLine = std::atan2(second.y - first.y, second.x - first.x); + const double diff = std::remainder(angleLine - rotation, M_PI_2); if (dist < minDist && std::abs(diff) <= SNAPPING_ROTATION_TOLERANCE) { minDist = dist; diffAngle = diff; @@ -266,23 +264,23 @@ void GeometryToolInputHandler::scrollMotion(InputEvent const& event) { if (!std::isnan(diffAngle)) { controller->rotate(diffAngle, pos.x, pos.y); } - controller->move(offset.x, offset.y); + controller->translate(offset.x, offset.y); } -void GeometryToolInputHandler::zoomStart() { +void GeometryToolInputHandler::rotateAndZoomStart() { this->startZoomDistance = std::max(this->priLastPageRel.distance(this->secLastPageRel), ZOOM_DISTANCE_MIN); // Whether we can ignore the zoom portion of the gesture (e.g. distance between touch points - // hasn't changed enough). + // hasn't changed enough or zoom gestures are disabled). this->canBlockZoom = true; this->lastZoomScrollCenter = (this->priLastPageRel + this->secLastPageRel) / 2.0; - const auto shift = this->secLastPageRel - this->priLastPageRel; + const utl::Point shift = this->secLastPageRel - this->priLastPageRel; this->lastAngle = atan2(shift.y, shift.x); this->lastDist = this->startZoomDistance; } -void GeometryToolInputHandler::zoomMotion(InputEvent const& event) { +void GeometryToolInputHandler::rotateAndZoomMotion(InputEvent const& event) { if (event.sequence == this->primarySequence) { this->priLastPageRel = this->getCoords(event); } else { @@ -291,27 +289,28 @@ void GeometryToolInputHandler::zoomMotion(InputEvent const& event) { const double dist = std::max(this->priLastPageRel.distance(this->secLastPageRel), ZOOM_DISTANCE_MIN); - const auto zoomTriggerThreshold = xournal->getControl()->getSettings()->getTouchZoomStartThreshold(); + const double zoomTriggerThreshold = xournal->getControl()->getSettings()->getTouchZoomStartThreshold(); const double zoomChangePercentage = std::abs(dist - startZoomDistance) / startZoomDistance * 100; - // Has the touch points moved far enough to trigger a zoom? - if (zoomChangePercentage >= zoomTriggerThreshold) { + // Has the touch points moved far enough to trigger a zoom and are zoom gestures enabled? + const bool zoomGesturesEnabled = xournal->getControl()->getSettings()->isZoomGesturesEnabled(); + if (zoomChangePercentage >= zoomTriggerThreshold && zoomGesturesEnabled) { this->canBlockZoom = false; } - const auto center = (this->priLastPageRel + this->secLastPageRel) / 2; - const auto shift = this->secLastPageRel - this->priLastPageRel; - const auto angle = atan2(shift.y, shift.x); + const utl::Point center = (this->priLastPageRel + this->secLastPageRel) / 2; + const utl::Point shift = this->secLastPageRel - this->priLastPageRel; + const double angle = atan2(shift.y, shift.x); - const auto offset = center - lastZoomScrollCenter; - controller->move(offset.x, offset.y); - const auto angleIncrease = angle - lastAngle; - const auto centerRel = (this->priLastPageRel + this->secLastPageRel) / 2.0; + const utl::Point offset = center - lastZoomScrollCenter; + controller->translate(offset.x, offset.y); + const double angleIncrease = angle - lastAngle; + const utl::Point centerRel = (this->priLastPageRel + this->secLastPageRel) / 2.0; if (controller->isInsideGeometryTool(secLastPageRel.x, secLastPageRel.y, 0.0)) { controller->rotate(angleIncrease, centerRel.x, centerRel.y); } // allow moving without accidental rotation - const auto scaleFactor = dist / lastDist; - const auto h = height * scaleFactor; + const double scaleFactor = dist / lastDist; + const double h = height * scaleFactor; if (!canBlockZoom && h <= getMaxHeight() && h >= getMinHeight()) { controller->scale(scaleFactor, centerRel.x, centerRel.y); @@ -323,10 +322,10 @@ void GeometryToolInputHandler::zoomMotion(InputEvent const& event) { } auto GeometryToolInputHandler::getCoords(InputEvent const& event) -> utl::Point { - const auto zoom = xournal->getZoom(); + const double zoom = xournal->getZoom(); const auto view = controller->getView(); - const auto posX = event.relativeX - static_cast(view->getX()); - const auto posY = event.relativeY - static_cast(view->getY()); + const double posX = event.relativeX - static_cast(view->getX()); + const double posY = event.relativeY - static_cast(view->getY()); return utl::Point(posX / zoom, posY / zoom); } diff --git a/src/core/gui/inputdevices/GeometryToolInputHandler.h b/src/core/gui/inputdevices/GeometryToolInputHandler.h index eb1f327d1e1b..bdbba4e3b908 100644 --- a/src/core/gui/inputdevices/GeometryToolInputHandler.h +++ b/src/core/gui/inputdevices/GeometryToolInputHandler.h @@ -125,15 +125,16 @@ class GeometryToolInputHandler: public xoj::util::Listener // for initializer_list -GeometryTool::GeometryTool(double h, double r, double tx, double ty): +GeometryTool::GeometryTool(double h, double r, double tx, double ty, Range toolRange): height(h), rotation(r), translationX(tx), translationY(ty), viewPool(std::make_shared>()), - handlerPool(std::make_shared>()) {} + handlerPool(std::make_shared>()), + lastRepaintRange(toolRange) {} GeometryTool::~GeometryTool() {} diff --git a/src/core/model/GeometryTool.h b/src/core/model/GeometryTool.h index 5d9a99af2583..049a48a209c9 100644 --- a/src/core/model/GeometryTool.h +++ b/src/core/model/GeometryTool.h @@ -49,7 +49,7 @@ class GeometryTool: public OverlayBase { * @param x the x-coordinate (in pt) of the rotation center * @param y the y-coordinate (in pt) of the rotation center */ - GeometryTool(double height, double rotation, double x, double y); + GeometryTool(double height, double rotation, double x, double y, Range toolRange); virtual ~GeometryTool(); diff --git a/src/core/model/Setsquare.cpp b/src/core/model/Setsquare.cpp index f4ac43343419..9f278a1adbe8 100644 --- a/src/core/model/Setsquare.cpp +++ b/src/core/model/Setsquare.cpp @@ -8,7 +8,8 @@ Setsquare::Setsquare(): Setsquare(INITIAL_HEIGHT, .0, INITIAL_X, INITIAL_Y) {} -Setsquare::Setsquare(double height, double rotation, double x, double y): GeometryTool(height, rotation, x, y) {} +Setsquare::Setsquare(double height, double rotation, double x, double y): + GeometryTool(height, rotation, x, y, getToolRange(true)) {} Setsquare::~Setsquare() { viewPool->dispatchAndClear(xoj::view::SetsquareView::FINALIZATION_REQUEST, Range()); } diff --git a/src/core/model/Setsquare.h b/src/core/model/Setsquare.h index df857edeedc9..fabb387ed162 100644 --- a/src/core/model/Setsquare.h +++ b/src/core/model/Setsquare.h @@ -11,6 +11,8 @@ #pragma once +#include + #include "model/GeometryTool.h" /** @@ -49,4 +51,9 @@ class Setsquare: public GeometryTool { static constexpr double INITIAL_HEIGHT = 8.0; static constexpr double INITIAL_X = 21. * HALF_CM; static constexpr double INITIAL_Y = 15. * HALF_CM; + // relation between setsquare height and radius + static constexpr double DISTANCE_SEMICIRCLE_FROM_LEGS = 1.15; + static inline double radiusFromHeight(double height) { + return height / std::sqrt(2.) - DISTANCE_SEMICIRCLE_FROM_LEGS; + } }; diff --git a/src/core/view/SetsquareView.cpp b/src/core/view/SetsquareView.cpp index 3ca2cb5dd309..d2d15c6f9b77 100644 --- a/src/core/view/SetsquareView.cpp +++ b/src/core/view/SetsquareView.cpp @@ -23,7 +23,6 @@ constexpr double FONT_SIZE = .2; constexpr double CIRCLE_RAD = .3; constexpr double TICK_SMALL = .1; constexpr double TICK_LARGE = .2; -constexpr double DISTANCE_SEMICIRCLE_FROM_LEGS = 1.15; constexpr double MAX_HOR_POS_VMARKS = 2.5; constexpr int MIN_OFFSET_ANG_MARKS = 2; constexpr double RELATIVE_CIRCLE_POS = .75; @@ -49,7 +48,7 @@ void SetsquareView::on(UpdateValuesRequest, double h, double rot, cairo_matrix_t height = h; rotation = rot; matrix = m; - radius = height / std::sqrt(2.) - DISTANCE_SEMICIRCLE_FROM_LEGS; + radius = Setsquare::radiusFromHeight(height); circlePos = height * RELATIVE_CIRCLE_POS; horPosVmarks = std::min(MAX_HOR_POS_VMARKS, radius * RELATIVE_MAX_HOR_POS_VMARKS); minVmark = (std::abs(horPosVmarks - HMARK_POS - TICK_SMALL / 2.) < MIN_DIST_FROM_HMARK - TICK_SMALL / 2.) ? diff --git a/src/core/view/SetsquareView.h b/src/core/view/SetsquareView.h index daa38621881f..c4cef4f3aedb 100644 --- a/src/core/view/SetsquareView.h +++ b/src/core/view/SetsquareView.h @@ -46,7 +46,7 @@ class SetsquareView: public GeometryToolView { public: SetsquareView(const Setsquare* s, Repaintable* parent, ZoomControl* zoomControl); - ~SetsquareView() override; + ~SetsquareView() noexcept override; void on(FlagDirtyRegionRequest, const Range& rg) override; void on(UpdateValuesRequest, double h, double rot, cairo_matrix_t m) override;