From 92664dabab7c8828f152c6fff4024e1539f0eda0 Mon Sep 17 00:00:00 2001 From: Rafael Siejakowski Date: Sun, 18 May 2025 17:46:28 +0200 Subject: [PATCH 1/2] Split off SPDesktop's dependent types into headers Split off the class DesktopAffine and the enum CanvasFlip into separate header files. This reduces the length of desktop.h from 579 to 485 lines - more than 16%! These entities are moved to the Inkscape namespace. The enum CanvasFlip is converted to enum class. A bool member is reordered to reduce padding. The includes in the header desktop.h are sorted and cleaned up. The only other place using the enum Inkscape::CanvasFlip: the file actions-canvas -transform.cpp, is updated and also has its includes cleaned up. This refactoring will make it possible to one day write unit tests for the class DesktopAffine. --- src/actions/actions-canvas-transform.cpp | 21 ++-- src/desktop.h | 136 ++++------------------- src/ui/CMakeLists.txt | 2 + src/ui/desktop/canvas-flip.h | 30 +++++ src/ui/desktop/desktop-affine.h | 120 ++++++++++++++++++++ 5 files changed, 181 insertions(+), 128 deletions(-) create mode 100644 src/ui/desktop/canvas-flip.h create mode 100644 src/ui/desktop/desktop-affine.h diff --git a/src/actions/actions-canvas-transform.cpp b/src/actions/actions-canvas-transform.cpp index 00cdefcf02..34dc7d304d 100644 --- a/src/actions/actions-canvas-transform.cpp +++ b/src/actions/actions-canvas-transform.cpp @@ -8,26 +8,21 @@ * */ -#include +#include "actions-canvas-transform.h" -#include // Not ! To eventually allow a headless version! +#include <2geom/angle.h> // rad_from_deg +#include // Not ! To eventually allow a headless version! #include -#include <2geom/angle.h> // rad_from_deg - -#include "actions-canvas-transform.h" #include "actions-helper.h" +#include "desktop.h" #include "inkscape-application.h" #include "inkscape-window.h" -#include "desktop.h" - -#include "object/sp-namedview.h" #include "page-manager.h" - +#include "ui/desktop/canvas-flip.h" #include "ui/tools/freehand-base.h" // SP_DRAW_CONTEXT #include "ui/tools/pen-tool.h" #include "ui/tools/pencil-tool.h" - #include "ui/widget/canvas.h" // Canvas area enum { @@ -152,15 +147,15 @@ canvas_transform(InkscapeWindow *win, const int& option) break; case INK_CANVAS_FLIP_HORIZONTAL: - dt->flip_relative_center_point (midpoint, SPDesktop::FLIP_HORIZONTAL); + dt->flip_relative_center_point(midpoint, Inkscape::CanvasFlip::FLIP_HORIZONTAL); break; case INK_CANVAS_FLIP_VERTICAL: - dt->flip_relative_center_point (midpoint, SPDesktop::FLIP_VERTICAL); + dt->flip_relative_center_point(midpoint, Inkscape::CanvasFlip::FLIP_VERTICAL); break; case INK_CANVAS_FLIP_RESET: - dt->flip_absolute_center_point (midpoint, SPDesktop::FLIP_NONE); + dt->flip_absolute_center_point(midpoint, Inkscape::CanvasFlip::FLIP_NONE); break; default: diff --git a/src/desktop.h b/src/desktop.h index 63c4c01064..8a90167bda 100644 --- a/src/desktop.h +++ b/src/desktop.h @@ -27,25 +27,25 @@ #ifndef INKSCAPE_DESKTOP_H #define INKSCAPE_DESKTOP_H +#include <2geom/affine.h> +#include <2geom/parallelogram.h> +#include <2geom/transforms.h> #include -#include -#include -#include -#include -#include +#include #include #include #include // EventController et al. -#include +#include +#include +#include #include -#include <2geom/affine.h> -#include <2geom/transforms.h> -#include <2geom/parallelogram.h> +#include +#include #include "display/rendermode.h" -#include #include "message-stack.h" #include "object/sp-gradient.h" // TODO refactor enums out to their own .h file +#include "ui/desktop/desktop-affine.h" namespace Gtk { class Box; @@ -109,6 +109,7 @@ class CanvasItemCatchall; class CanvasItemDrawing; class CanvasItemGroup; class CanvasItemRotate; +enum class CanvasFlip : int; namespace UI { @@ -352,16 +353,11 @@ public: void rotate_absolute_center_point(Geom::Point const &c, double rotate); void rotate_relative_center_point(Geom::Point const &c, double rotate); - enum CanvasFlip { - FLIP_NONE = 0, - FLIP_HORIZONTAL = 1, - FLIP_VERTICAL = 2 - }; - void flip_absolute_keep_point (Geom::Point const &c, CanvasFlip flip); - void flip_relative_keep_point (Geom::Point const &c, CanvasFlip flip); - void flip_absolute_center_point(Geom::Point const &c, CanvasFlip flip); - void flip_relative_center_point(Geom::Point const &c, CanvasFlip flip); - bool is_flipped(CanvasFlip flip); + void flip_absolute_keep_point(Geom::Point const &c, Inkscape::CanvasFlip flip); + void flip_relative_keep_point(Geom::Point const &c, Inkscape::CanvasFlip flip); + void flip_absolute_center_point(Geom::Point const &c, Inkscape::CanvasFlip flip); + void flip_relative_center_point(Geom::Point const &c, Inkscape::CanvasFlip flip); + bool is_flipped(Inkscape::CanvasFlip flip); Geom::Rotate const ¤t_rotation() const { return _current_affine.getRotation(); } @@ -438,102 +434,12 @@ private: void _attachDocument(); void _detachDocument(); - // This simple class ensures that _w2d is always in sync with _rotation and _scale - // We keep rotation and scale separate to avoid having to extract them from the affine. - // With offset, this describes fully how to map the drawing to the window. - // Future: merge offset as a translation in w2d. - class DesktopAffine - { - public: - Geom::Affine const &w2d() const { return _w2d; }; - Geom::Affine const &d2w() const { return _d2w; }; - - void setScale(Geom::Scale scale) { - _scale = scale; - _update(); - } - void addScale(Geom::Scale scale) { - _scale *= scale; - _update(); - } - - void setRotate(Geom::Rotate rotate) { - _rotate = rotate; - _update(); - } - void setRotate(double rotate) { - setRotate(Geom::Rotate{rotate}); - } - void addRotate(Geom::Rotate rotate) { - _rotate *= rotate; - _update(); - } - void addRotate(double rotate) { - addRotate(Geom::Rotate{rotate}); - } - - void setFlip(CanvasFlip flip) { - _flip = Geom::Scale(); - addFlip( flip ); - } - - bool isFlipped(CanvasFlip flip) { - if ((flip & FLIP_HORIZONTAL) && Geom::are_near(_flip[0], -1)) { - return true; - } - if ((flip & FLIP_VERTICAL) && Geom::are_near(_flip[1], -1)) { - return true; - } - return false; - } - - void addFlip(CanvasFlip flip) { - if (flip & FLIP_HORIZONTAL) { - _flip *= Geom::Scale(-1.0, 1.0); - } - if (flip & FLIP_VERTICAL) { - _flip *= Geom::Scale(1.0, -1.0); - } - _update(); - } - - double getZoom() const { - return _d2w.descrim(); - } - - Geom::Rotate const &getRotation() const { - return _rotate; - } - - void setOffset(Geom::Point offset) { - _offset = offset; - } - void addOffset(Geom::Point offset) { - _offset += offset; - } - Geom::Point const &getOffset() { - return _offset; - } - - private: - void _update() { - _d2w = _scale * _rotate * _flip; - _w2d = _d2w.inverse(); - } - Geom::Affine _w2d; // Window to desktop - Geom::Affine _d2w; // Desktop to window - Geom::Rotate _rotate; // Rotate part of _w2d - Geom::Scale _scale; // Scale part of _w2d, holds y-axis direction - Geom::Scale _flip; // Flip part of _w2d - Geom::Point _offset; // Point on canvas to align to (0,0) of window - }; - - DesktopAffine _current_affine; - std::list transforms_past; - std::list transforms_future; - bool _quick_zoom_enabled = false; ///< Signifies that currently we're in quick zoom mode - DesktopAffine _quick_zoom_affine; ///< The transform of the screen before quick zoom + Inkscape::DesktopAffine _current_affine; + std::list transforms_past; + std::list transforms_future; + Inkscape::DesktopAffine _quick_zoom_affine; ///< The transform of the screen before quick zoom + bool _quick_zoom_enabled = false; ///< Signifies that currently we're in quick zoom mode bool _overlays_visible = true; ///< Whether the overlays are temporarily hidden bool _saved_guides_visible = false; ///< Remembers guides' visibility when hiding overlays diff --git a/src/ui/CMakeLists.txt b/src/ui/CMakeLists.txt index df176e0343..fcea472c69 100644 --- a/src/ui/CMakeLists.txt +++ b/src/ui/CMakeLists.txt @@ -323,6 +323,8 @@ set(ui_SRC cache/svg_preview_cache.h + desktop/canvas-flip.h + desktop/desktop-affine.h desktop/document-check.h desktop/menubar.h desktop/menu-set-tooltips-shift-icons.h diff --git a/src/ui/desktop/canvas-flip.h b/src/ui/desktop/canvas-flip.h new file mode 100644 index 0000000000..d2289b5d46 --- /dev/null +++ b/src/ui/desktop/canvas-flip.h @@ -0,0 +1,30 @@ +/** @file + * Enum describing canvas flips + */ +/* + * Authors: see git history + * + * Copyright (C) 2025 Authors + * + * Released under GNU GPL v2+, read the file 'COPYING' for more information. + */ + +#ifndef INKSCAPE_UI_CANVAS_FLIP_H +#define INKSCAPE_UI_CANVAS_FLIP_H + +namespace Inkscape { + +enum class CanvasFlip : int +{ + FLIP_NONE = 0, + FLIP_HORIZONTAL = 1, + FLIP_VERTICAL = 2 +}; + +constexpr int operator&(CanvasFlip lhs, CanvasFlip rhs) +{ + return static_cast(lhs) & static_cast(rhs); +} +} // namespace Inkscape + +#endif // INKSCAPE_UI_CANVAS_FLIP_H \ No newline at end of file diff --git a/src/ui/desktop/desktop-affine.h b/src/ui/desktop/desktop-affine.h new file mode 100644 index 0000000000..77848b7508 --- /dev/null +++ b/src/ui/desktop/desktop-affine.h @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/** @file + * Affine transformation of the desktop. + */ +/* + * Authors: see git history + * + * Copyright (C) 2025 Authors + * + * Released under GNU GPL v2+, read the file 'COPYING' for more information. + */ + +#ifndef INKSCAPE_UI_DESKTOP_AFFINE_H +#define INKSCAPE_UI_DESKTOP_AFFINE_H + +#include <2geom/affine.h> +#include <2geom/point.h> +#include <2geom/transforms.h> + +#include "ui/desktop/canvas-flip.h" + +namespace Inkscape { + +/** @class DesktopAffine + * @brief This simple class ensures that _w2d is always in sync with _rotation and _scale. + * + * We keep rotation and scale separate to avoid having to extract them from the affine. + * With offset, this describes fully how to map the drawing to the window. + * Future: merge offset as a translation in w2d. + */ +class DesktopAffine +{ +public: + Geom::Affine const &w2d() const { return _w2d; } + + Geom::Affine const &d2w() const { return _d2w; } + + void setScale(Geom::Scale scale) + { + _scale = scale; + _update(); + } + + void addScale(Geom::Scale scale) + { + _scale *= scale; + _update(); + } + + void setRotate(Geom::Rotate rotate) + { + _rotate = rotate; + _update(); + } + + void setRotate(double rotate) { setRotate(Geom::Rotate{rotate}); } + + void addRotate(Geom::Rotate rotate) + { + _rotate *= rotate; + _update(); + } + + void addRotate(double rotate) { addRotate(Geom::Rotate{rotate}); } + + void setFlip(CanvasFlip flip) + { + _flip = Geom::Scale(); + addFlip(flip); + } + + bool isFlipped(CanvasFlip flip) + { + if ((flip & CanvasFlip::FLIP_HORIZONTAL) && Geom::are_near(_flip[0], -1)) { + return true; + } + if ((flip & CanvasFlip::FLIP_VERTICAL) && Geom::are_near(_flip[1], -1)) { + return true; + } + return false; + } + + void addFlip(CanvasFlip flip) + { + if (flip & CanvasFlip::FLIP_HORIZONTAL) { + _flip *= Geom::Scale(-1.0, 1.0); + } + if (flip & CanvasFlip::FLIP_VERTICAL) { + _flip *= Geom::Scale(1.0, -1.0); + } + _update(); + } + + double getZoom() const { return _d2w.descrim(); } + + Geom::Rotate const &getRotation() const { return _rotate; } + + void setOffset(Geom::Point offset) { _offset = offset; } + + void addOffset(Geom::Point offset) { _offset += offset; } + + Geom::Point const &getOffset() { return _offset; } + +private: + void _update() + { + _d2w = _scale * _rotate * _flip; + _w2d = _d2w.inverse(); + } + + Geom::Affine _w2d; // Window to desktop + Geom::Affine _d2w; // Desktop to window + Geom::Rotate _rotate; // Rotate part of _w2d + Geom::Scale _scale; // Scale part of _w2d, holds y-axis direction + Geom::Scale _flip; // Flip part of _w2d + Geom::Point _offset; // Point on canvas to align to (0,0) of window +}; +} // namespace Inkscape + +#endif // INKSCAPE_UI_DESKTOP_AFFINE_H -- GitLab From e0afdd3ff0575b104d12fe1bb59f4e89e657c099 Mon Sep 17 00:00:00 2001 From: Rafael Siejakowski Date: Sun, 18 May 2025 18:47:14 +0200 Subject: [PATCH 2/2] Add tests for class Inkscape::DesktopAffine This class maintains a decomposition of an affine transformation into its consituent factors: a scale, a rotation, and possibly a flip. Due to its fundamental role in the Inkscape desktop functionality, it is a good idea for the class to be tested. --- testfiles/CMakeLists.txt | 1 + testfiles/src/desktop-affine-test.cpp | 190 ++++++++++++++++++++++++++ 2 files changed, 191 insertions(+) create mode 100644 testfiles/src/desktop-affine-test.cpp diff --git a/testfiles/CMakeLists.txt b/testfiles/CMakeLists.txt index 5e50cd251a..4dbfa9da57 100644 --- a/testfiles/CMakeLists.txt +++ b/testfiles/CMakeLists.txt @@ -157,6 +157,7 @@ endforeach() include(${CMAKE_SOURCE_DIR}/CMakeScripts/UnitTest.cmake) ### Unit tests +add_unit_test(desktop-affine-test EXTRA_LIBS 2Geom::2geom) add_unit_test(version-test SOURCES version.cpp) add_unit_test(css-syntactic-decomposition-test SOURCES "css/syntactic-decomposition.cpp" EXTRA_LIBS croco_LIB) add_dependencies(tests unit_tests) diff --git a/testfiles/src/desktop-affine-test.cpp b/testfiles/src/desktop-affine-test.cpp new file mode 100644 index 0000000000..dd9d6e3350 --- /dev/null +++ b/testfiles/src/desktop-affine-test.cpp @@ -0,0 +1,190 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/** + * Authors: + * Rafał Siejakowski + * + * @copyright + * Copyright (C) 2025 Authors + * + * Released under GNU GPL v2+, read the file 'COPYING' for more information. + */ + +#include "ui/desktop/desktop-affine.h" + +#include <2geom/affine.h> +#include <2geom/transforms.h> +#include + +namespace Inkscape { + +using namespace ::testing; + +TEST(DesktopAffineTest, SetScale) +{ + auto const testScale = Geom::Scale{2.0, -3.0}; + auto const testScale2 = Geom::Scale{-1.0, 12.0}; + + DesktopAffine testAffine; + testAffine.setScale(testScale); + EXPECT_EQ(testAffine.d2w(), testScale); + + testAffine.setScale(testScale2); + EXPECT_EQ(testAffine.d2w(), testScale2); +} + +TEST(DesktopAffineTest, AddScale) +{ + auto const testScale = Geom::Scale{2.0, -3.0}; + auto const testScale2 = Geom::Scale{-1.0, 12.0}; + + DesktopAffine testAffine; + testAffine.addScale(testScale); + EXPECT_EQ(testAffine.d2w(), testScale); + + testAffine.addScale(testScale2); + auto const expected = testScale * testScale2; + EXPECT_EQ(testAffine.d2w(), expected); +} + +TEST(DesktopAffineTest, SetRotate) +{ + auto const testRotate = Geom::Rotate{15.0}; + auto const testRotate2 = Geom::Rotate{-60.0}; + + DesktopAffine testAffine; + testAffine.setRotate(testRotate); + EXPECT_EQ(testAffine.d2w(), testRotate); + + testAffine.setRotate(testRotate2); + EXPECT_EQ(testAffine.d2w(), testRotate2); +} + +TEST(DesktopAffineTest, AddRotate) +{ + auto const testRotate = Geom::Rotate{15.0}; + auto const testRotate2 = Geom::Rotate{-60.0}; + + DesktopAffine testAffine; + testAffine.addRotate(testRotate); + EXPECT_EQ(testAffine.d2w(), testRotate); + + testAffine.addRotate(testRotate2); + auto const expected = testRotate * testRotate2; + EXPECT_EQ(testAffine.d2w(), expected); +} + +TEST(DesktopAffineTest, AddRotateAndScale) +{ + auto const testRotate = Geom::Rotate{15.0}; + auto const testScale = Geom::Scale{2.0, -3.0}; + + DesktopAffine testAffine; + testAffine.addRotate(testRotate); + testAffine.addScale(testScale); + + auto const expected = testScale * testRotate; + EXPECT_EQ(testAffine.d2w(), expected); +} + +TEST(DesktopAffineTest, AddScaleAndRotate) +{ + auto const testScale = Geom::Scale{2.0, -3.0}; + auto const testRotate = Geom::Rotate{15.0}; + + DesktopAffine testAffine; + testAffine.addScale(testScale); + testAffine.addRotate(testRotate); + + auto const expected = testScale * testRotate; + EXPECT_EQ(testAffine.d2w(), expected); +} + +TEST(DesktopAffineTest, AddScaleAndFlip) +{ + auto const testScale = Geom::Scale{2.0, -3.0}; + auto const testFlip = CanvasFlip::FLIP_HORIZONTAL; + + DesktopAffine testAffine; + testAffine.addScale(testScale); + testAffine.addFlip(testFlip); + + auto const expected = testScale * Geom::Scale(-1.0, 1.0); + EXPECT_EQ(testAffine.d2w(), expected); + + // unflip now + testAffine.addFlip(testFlip); + EXPECT_EQ(testAffine.d2w(), testScale); +} + +TEST(DesktopAffineTest, AddMultipleFlips) +{ + DesktopAffine testAffine; + testAffine.addFlip(CanvasFlip::FLIP_HORIZONTAL); + EXPECT_EQ(testAffine.d2w(), Geom::Scale(-1.0, 1.0)); + + testAffine.addFlip(CanvasFlip::FLIP_VERTICAL); + EXPECT_EQ(testAffine.d2w(), Geom::Scale(-1.0, -1.0)); + + testAffine.addFlip(CanvasFlip::FLIP_HORIZONTAL); + EXPECT_EQ(testAffine.d2w(), Geom::Scale(1.0, -1.0)); + + testAffine.addFlip(CanvasFlip::FLIP_VERTICAL); + EXPECT_EQ(testAffine.d2w(), Geom::identity()); +} + +TEST(DesktopAffineTest, GetZoom) +{ + DesktopAffine testAffine; + EXPECT_DOUBLE_EQ(testAffine.getZoom(), 1.0); + + testAffine.addScale(Geom::Scale(2.0)); + EXPECT_DOUBLE_EQ(testAffine.getZoom(), 2.0); + + testAffine.addFlip(CanvasFlip::FLIP_VERTICAL); + EXPECT_DOUBLE_EQ(testAffine.getZoom(), 2.0); + + testAffine.addScale(Geom::Scale(3.0)); + EXPECT_DOUBLE_EQ(testAffine.getZoom(), 6.0); +} + +TEST(DesktopAffineTest, GetRotation) +{ + DesktopAffine testAffine; + EXPECT_EQ(testAffine.getRotation(), Geom::Rotate()); + + auto const testRotation = Geom::Rotate(32.0); + testAffine.addRotate(testRotation); + EXPECT_EQ(testAffine.getRotation(), testRotation); + + // A flip shouldn't change the rotation + testAffine.addFlip(CanvasFlip::FLIP_HORIZONTAL); + EXPECT_EQ(testAffine.getRotation(), testRotation); +} + +TEST(DesktopAffineTest, IsFlipped) +{ + DesktopAffine testAffine; + EXPECT_FALSE(testAffine.isFlipped(CanvasFlip::FLIP_HORIZONTAL)); + EXPECT_FALSE(testAffine.isFlipped(CanvasFlip::FLIP_VERTICAL)); + + // Add a horizontal flip + testAffine.addFlip(CanvasFlip::FLIP_HORIZONTAL); + EXPECT_TRUE(testAffine.isFlipped(CanvasFlip::FLIP_HORIZONTAL)); + EXPECT_FALSE(testAffine.isFlipped(CanvasFlip::FLIP_VERTICAL)); + + // Add a vertical flip + testAffine.addFlip(CanvasFlip::FLIP_VERTICAL); + EXPECT_TRUE(testAffine.isFlipped(CanvasFlip::FLIP_HORIZONTAL)); + EXPECT_TRUE(testAffine.isFlipped(CanvasFlip::FLIP_VERTICAL)); + + // Remove the horizontal flip + testAffine.addFlip(CanvasFlip::FLIP_HORIZONTAL); + EXPECT_FALSE(testAffine.isFlipped(CanvasFlip::FLIP_HORIZONTAL)); + EXPECT_TRUE(testAffine.isFlipped(CanvasFlip::FLIP_VERTICAL)); + + // Check that the result doesn't change when we rotate + testAffine.addRotate(90.0); + EXPECT_FALSE(testAffine.isFlipped(CanvasFlip::FLIP_HORIZONTAL)); + EXPECT_TRUE(testAffine.isFlipped(CanvasFlip::FLIP_VERTICAL)); +} +} // namespace Inkscape -- GitLab