From 6411d4bf21d9b6b408deb44778e1f22e2f52ae7a Mon Sep 17 00:00:00 2001 From: Thomas Spranger Date: Fri, 30 May 2025 21:32:27 +0200 Subject: [PATCH 1/8] Handle units in margin spinedits * Convert margin spinedits to UI::Widget::SpinButton and sync their unit with the document's display unit. * Merge together marginEdited() functions. Fixes https://gitlab.com/inkscape/inkscape/-/issues/4948 --- src/ui/toolbar/page-toolbar.cpp | 54 ++++++++++++++++----------------- src/ui/toolbar/page-toolbar.h | 16 +++++----- 2 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/ui/toolbar/page-toolbar.cpp b/src/ui/toolbar/page-toolbar.cpp index 47dd19694b..7a318cad28 100644 --- a/src/ui/toolbar/page-toolbar.cpp +++ b/src/ui/toolbar/page-toolbar.cpp @@ -31,6 +31,7 @@ #include "ui/icon-names.h" #include "ui/popup-menu.h" #include "ui/widget/spinbutton.h" +#include "ui/widget/unit-menu.h" using Inkscape::IO::Resource::UIS; @@ -70,10 +71,10 @@ PageToolbar::PageToolbar(Glib::RefPtr const &builder) , _sep1(get_widget(builder, "_sep1")) , _sizes_list(get_object(builder, "_sizes_list")) , _sizes_search(get_object(builder, "_sizes_search")) - , _margin_top(get_derived_widget(builder, "_margin_top")) - , _margin_right(get_derived_widget(builder, "_margin_right")) - , _margin_bottom(get_derived_widget(builder, "_margin_bottom")) - , _margin_left(get_derived_widget(builder, "_margin_left")) + , _margin_top(UI::get_derived_widget(builder, "_margin_top")) + , _margin_right(UI::get_derived_widget(builder, "_margin_right")) + , _margin_bottom(UI::get_derived_widget(builder, "_margin_bottom")) + , _margin_left(UI::get_derived_widget(builder, "_margin_left")) { set_name("PageToolbar"); @@ -102,18 +103,33 @@ PageToolbar::PageToolbar(Glib::RefPtr const &builder) auto const &margin = page->getMarginBox(); auto unit = _document->getDisplayUnit()->abbr; auto scale = _document->getDocumentScale(); + + // Set the unit of the unit-selector, so the units of the margin spin-edits + // are updated to the selected display unit. + unitSelector->setUnit(unit); + _margin_top.set_value(margin.top().toValue(unit) * scale[Geom::Y]); _margin_right.set_value(margin.right().toValue(unit) * scale[Geom::X]); _margin_bottom.set_value(margin.bottom().toValue(unit) * scale[Geom::Y]); _margin_left.set_value(margin.left().toValue(unit) * scale[Geom::X]); + _text_page_bleeds.set_text(page->getBleedLabel()); } UI::popup_at(_margin_popover, _text_page_margins); }); - _margin_top.signal_value_changed().connect(sigc::mem_fun(*this, &PageToolbar::marginTopEdited)); - _margin_right.signal_value_changed().connect(sigc::mem_fun(*this, &PageToolbar::marginRightEdited)); - _margin_bottom.signal_value_changed().connect(sigc::mem_fun(*this, &PageToolbar::marginBottomEdited)); - _margin_left.signal_value_changed().connect(sigc::mem_fun(*this, &PageToolbar::marginLeftEdited)); + + unitSelector = Gtk::make_managed(); + unitSelector->setUnitType(Inkscape::Util::UNIT_TYPE_LINEAR); + + const auto pairs = { + std::pair { _margin_top, BoxSide::BOX_TOP }, + std::pair { _margin_right, BoxSide::BOX_RIGHT }, + std::pair { _margin_bottom, BoxSide::BOX_BOTTOM }, + std::pair { _margin_left, BoxSide::BOX_LEFT } }; + for (auto &pair : pairs) { + pair.first.setUnitMenu(unitSelector); + pair.first.signal_value_changed().connect(std::bind(&PageToolbar::marginSideEdited, this, pair.second, std::ref(pair.first))); + } dynamic_cast(*_combo_page_sizes.get_child()).set_completion(get_object(builder, "_sizes_searcher")); @@ -259,30 +275,14 @@ void PageToolbar::marginsEdited() } } -void PageToolbar::marginTopEdited() -{ - marginSideEdited(0, _margin_top.get_text()); -} -void PageToolbar::marginRightEdited() +void PageToolbar::marginSideEdited(const BoxSide side, const UI::Widget::SpinButton &entry) { - marginSideEdited(1, _margin_right.get_text()); -} -void PageToolbar::marginBottomEdited() -{ - marginSideEdited(2, _margin_bottom.get_text()); -} -void PageToolbar::marginLeftEdited() -{ - marginSideEdited(3, _margin_left.get_text()); -} -void PageToolbar::marginSideEdited(int side, const Glib::ustring &value) -{ - // And modifiction to the margin causes pages to be enabled + // And modification to the margin causes pages to be enabled auto &pm = _document->getPageManager(); pm.enablePages(); if (auto page = pm.getSelected()) { - page->setMarginSide(side, value, false); + page->setMarginSide(side, entry.get_value(), false); DocumentUndo::maybeDone(_document, "page-margin", _("Edit page margin"), INKSCAPE_ICON("tool-pages")); setMarginText(page); } diff --git a/src/ui/toolbar/page-toolbar.h b/src/ui/toolbar/page-toolbar.h index a911863d51..d5ffa10833 100644 --- a/src/ui/toolbar/page-toolbar.h +++ b/src/ui/toolbar/page-toolbar.h @@ -61,11 +61,7 @@ private: void labelEdited(); void bleedsEdited(); void marginsEdited(); - void marginTopEdited(); - void marginRightEdited(); - void marginBottomEdited(); - void marginLeftEdited(); - void marginSideEdited(int side, const Glib::ustring &value); + void marginSideEdited(BoxSide side, const UI::Widget::SpinButton &entry); void sizeChoose(const std::string &preset_key); void sizeChanged(); void setLabelText(SPPage *page = nullptr); @@ -101,10 +97,12 @@ private: Glib::RefPtr _sizes_list; Glib::RefPtr _sizes_search; - Inkscape::UI::Widget::MathSpinButton &_margin_top; - Inkscape::UI::Widget::MathSpinButton &_margin_right; - Inkscape::UI::Widget::MathSpinButton &_margin_bottom; - Inkscape::UI::Widget::MathSpinButton &_margin_left; + UI::Widget::SpinButton &_margin_top; + UI::Widget::SpinButton &_margin_right; + UI::Widget::SpinButton &_margin_bottom; + UI::Widget::SpinButton &_margin_left; + + UI::Widget::UnitMenu *unitSelector; double _unit_to_size(std::string number, std::string unit_str, std::string const &backup); }; -- GitLab From e028d502887b740fc05b760db8906270540260b2 Mon Sep 17 00:00:00 2001 From: PBS Date: Sun, 29 Jun 2025 11:58:51 +0200 Subject: [PATCH 2/8] Coding style fixes --- src/ui/toolbar/page-toolbar.cpp | 20 +++++++++++--------- src/ui/toolbar/page-toolbar.h | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/ui/toolbar/page-toolbar.cpp b/src/ui/toolbar/page-toolbar.cpp index 7a318cad28..f1ae8e52cf 100644 --- a/src/ui/toolbar/page-toolbar.cpp +++ b/src/ui/toolbar/page-toolbar.cpp @@ -32,6 +32,7 @@ #include "ui/popup-menu.h" #include "ui/widget/spinbutton.h" #include "ui/widget/unit-menu.h" +#include "util/units.h" using Inkscape::IO::Resource::UIS; @@ -121,14 +122,15 @@ PageToolbar::PageToolbar(Glib::RefPtr const &builder) unitSelector = Gtk::make_managed(); unitSelector->setUnitType(Inkscape::Util::UNIT_TYPE_LINEAR); - const auto pairs = { - std::pair { _margin_top, BoxSide::BOX_TOP }, - std::pair { _margin_right, BoxSide::BOX_RIGHT }, - std::pair { _margin_bottom, BoxSide::BOX_BOTTOM }, - std::pair { _margin_left, BoxSide::BOX_LEFT } }; - for (auto &pair : pairs) { - pair.first.setUnitMenu(unitSelector); - pair.first.signal_value_changed().connect(std::bind(&PageToolbar::marginSideEdited, this, pair.second, std::ref(pair.first))); + auto const pairs = std::to_array({std::pair + {_margin_top, BoxSide::BOX_TOP}, + {_margin_right, BoxSide::BOX_RIGHT}, + {_margin_bottom, BoxSide::BOX_BOTTOM}, + {_margin_left, BoxSide::BOX_LEFT} + }); + for (auto &[btn, side] : pairs) { + btn.setUnitMenu(unitSelector); + btn.signal_value_changed().connect(std::bind(&PageToolbar::marginSideEdited, this, side, std::ref(btn))); } dynamic_cast(*_combo_page_sizes.get_child()).set_completion(get_object(builder, "_sizes_searcher")); @@ -275,7 +277,7 @@ void PageToolbar::marginsEdited() } } -void PageToolbar::marginSideEdited(const BoxSide side, const UI::Widget::SpinButton &entry) +void PageToolbar::marginSideEdited(BoxSide side, UI::Widget::SpinButton const &entry) { // And modification to the margin causes pages to be enabled auto &pm = _document->getPageManager(); diff --git a/src/ui/toolbar/page-toolbar.h b/src/ui/toolbar/page-toolbar.h index d5ffa10833..1cba0634ef 100644 --- a/src/ui/toolbar/page-toolbar.h +++ b/src/ui/toolbar/page-toolbar.h @@ -61,7 +61,7 @@ private: void labelEdited(); void bleedsEdited(); void marginsEdited(); - void marginSideEdited(BoxSide side, const UI::Widget::SpinButton &entry); + void marginSideEdited(BoxSide side, UI::Widget::SpinButton const &entry); void sizeChoose(const std::string &preset_key); void sizeChanged(); void setLabelText(SPPage *page = nullptr); -- GitLab From 3933169fc8f3264e94e0faf4819a92700d513c6a Mon Sep 17 00:00:00 2001 From: PBS Date: Sun, 29 Jun 2025 12:13:26 +0200 Subject: [PATCH 3/8] Convert unit menu -> unit tracker to fix a leak --- src/ui/toolbar/page-toolbar.cpp | 10 ++++------ src/ui/toolbar/page-toolbar.h | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/ui/toolbar/page-toolbar.cpp b/src/ui/toolbar/page-toolbar.cpp index f1ae8e52cf..e2d5ec1cbc 100644 --- a/src/ui/toolbar/page-toolbar.cpp +++ b/src/ui/toolbar/page-toolbar.cpp @@ -31,7 +31,7 @@ #include "ui/icon-names.h" #include "ui/popup-menu.h" #include "ui/widget/spinbutton.h" -#include "ui/widget/unit-menu.h" +#include "ui/widget/unit-tracker.h" #include "util/units.h" using Inkscape::IO::Resource::UIS; @@ -76,6 +76,7 @@ PageToolbar::PageToolbar(Glib::RefPtr const &builder) , _margin_right(UI::get_derived_widget(builder, "_margin_right")) , _margin_bottom(UI::get_derived_widget(builder, "_margin_bottom")) , _margin_left(UI::get_derived_widget(builder, "_margin_left")) + , _unit_tracker{std::make_unique(Util::UNIT_TYPE_LINEAR)} { set_name("PageToolbar"); @@ -107,7 +108,7 @@ PageToolbar::PageToolbar(Glib::RefPtr const &builder) // Set the unit of the unit-selector, so the units of the margin spin-edits // are updated to the selected display unit. - unitSelector->setUnit(unit); + _unit_tracker->setActiveUnit(_document->getDisplayUnit()); _margin_top.set_value(margin.top().toValue(unit) * scale[Geom::Y]); _margin_right.set_value(margin.right().toValue(unit) * scale[Geom::X]); @@ -119,9 +120,6 @@ PageToolbar::PageToolbar(Glib::RefPtr const &builder) UI::popup_at(_margin_popover, _text_page_margins); }); - unitSelector = Gtk::make_managed(); - unitSelector->setUnitType(Inkscape::Util::UNIT_TYPE_LINEAR); - auto const pairs = std::to_array({std::pair {_margin_top, BoxSide::BOX_TOP}, {_margin_right, BoxSide::BOX_RIGHT}, @@ -129,7 +127,7 @@ PageToolbar::PageToolbar(Glib::RefPtr const &builder) {_margin_left, BoxSide::BOX_LEFT} }); for (auto &[btn, side] : pairs) { - btn.setUnitMenu(unitSelector); + btn.addUnitTracker(_unit_tracker.get()); btn.signal_value_changed().connect(std::bind(&PageToolbar::marginSideEdited, this, side, std::ref(btn))); } diff --git a/src/ui/toolbar/page-toolbar.h b/src/ui/toolbar/page-toolbar.h index 1cba0634ef..5e6974870f 100644 --- a/src/ui/toolbar/page-toolbar.h +++ b/src/ui/toolbar/page-toolbar.h @@ -102,7 +102,7 @@ private: UI::Widget::SpinButton &_margin_bottom; UI::Widget::SpinButton &_margin_left; - UI::Widget::UnitMenu *unitSelector; + std::unique_ptr _unit_tracker; double _unit_to_size(std::string number, std::string unit_str, std::string const &backup); }; -- GitLab From c1bfc85ce926fee4a19f221059409482319a176b Mon Sep 17 00:00:00 2001 From: Thomas Spranger Date: Sun, 20 Jul 2025 21:47:51 +0200 Subject: [PATCH 4/8] Set active unit during setDesktop method --- src/ui/toolbar/page-toolbar.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ui/toolbar/page-toolbar.cpp b/src/ui/toolbar/page-toolbar.cpp index e2d5ec1cbc..b8962391a8 100644 --- a/src/ui/toolbar/page-toolbar.cpp +++ b/src/ui/toolbar/page-toolbar.cpp @@ -198,6 +198,8 @@ void PageToolbar::setDesktop(SPDesktop *desktop) _page_selected = page_manager.connectPageSelected(sigc::mem_fun(*this, &PageToolbar::selectionChanged)); // Update everything now. pagesChanged(); + + _unit_tracker->setActiveUnit(_document->getDisplayUnit()); } } @@ -264,7 +266,7 @@ void PageToolbar::marginsEdited() { auto text = _text_page_margins.get_text(); - // And modifiction to the margin causes pages to be enabled + // And modification to the margin causes pages to be enabled auto &pm = _document->getPageManager(); pm.enablePages(); -- GitLab From 40e8a0417cc599b6dd9a79edad219f6b4660f572 Mon Sep 17 00:00:00 2001 From: PBS Date: Thu, 24 Jul 2025 23:09:22 +0200 Subject: [PATCH 5/8] Fix #include breakage after rebase --- src/ui/toolbar/page-toolbar.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ui/toolbar/page-toolbar.h b/src/ui/toolbar/page-toolbar.h index 5e6974870f..043cc3d922 100644 --- a/src/ui/toolbar/page-toolbar.h +++ b/src/ui/toolbar/page-toolbar.h @@ -15,7 +15,9 @@ #ifndef INKSCAPE_UI_TOOLBAR_PAGE_TOOLBAR_H #define INKSCAPE_UI_TOOLBAR_PAGE_TOOLBAR_H +#include "svg/svg-box.h" #include "toolbar.h" +#include "ui/widget/spinbutton.h" namespace Gtk { class ComboBoxText; -- GitLab From aa06dd8803601f6fee702862d369cf84447c4535 Mon Sep 17 00:00:00 2001 From: PBS Date: Fri, 25 Jul 2025 00:43:21 +0200 Subject: [PATCH 6/8] Move unit assignment out of check --- src/ui/toolbar/page-toolbar.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/ui/toolbar/page-toolbar.cpp b/src/ui/toolbar/page-toolbar.cpp index b8962391a8..9298a048df 100644 --- a/src/ui/toolbar/page-toolbar.cpp +++ b/src/ui/toolbar/page-toolbar.cpp @@ -101,15 +101,15 @@ PageToolbar::PageToolbar(Glib::RefPtr const &builder) _margin_popover.set_parent(*this); _text_page_margins.signal_icon_press().connect([&](Gtk::Entry::IconPosition) { + // Set the unit of the unit-selector, so the units of the margin spin-edits + // are updated to the selected display unit. + _unit_tracker->setActiveUnit(_document->getDisplayUnit()); + if (auto page = _document->getPageManager().getSelected()) { auto const &margin = page->getMarginBox(); auto unit = _document->getDisplayUnit()->abbr; auto scale = _document->getDocumentScale(); - // Set the unit of the unit-selector, so the units of the margin spin-edits - // are updated to the selected display unit. - _unit_tracker->setActiveUnit(_document->getDisplayUnit()); - _margin_top.set_value(margin.top().toValue(unit) * scale[Geom::Y]); _margin_right.set_value(margin.right().toValue(unit) * scale[Geom::X]); _margin_bottom.set_value(margin.bottom().toValue(unit) * scale[Geom::Y]); @@ -198,8 +198,6 @@ void PageToolbar::setDesktop(SPDesktop *desktop) _page_selected = page_manager.connectPageSelected(sigc::mem_fun(*this, &PageToolbar::selectionChanged)); // Update everything now. pagesChanged(); - - _unit_tracker->setActiveUnit(_document->getDisplayUnit()); } } -- GitLab From 1475e9521fdf8ab3a852a52f7f3a81db76751c89 Mon Sep 17 00:00:00 2001 From: PBS Date: Fri, 25 Jul 2025 00:43:47 +0200 Subject: [PATCH 7/8] Put in some operation blockers To stop feedback loops of reading values we've just set. --- src/ui/toolbar/page-toolbar.cpp | 10 ++++++++++ src/ui/toolbar/page-toolbar.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/src/ui/toolbar/page-toolbar.cpp b/src/ui/toolbar/page-toolbar.cpp index 9298a048df..ef09b04b67 100644 --- a/src/ui/toolbar/page-toolbar.cpp +++ b/src/ui/toolbar/page-toolbar.cpp @@ -101,6 +101,11 @@ PageToolbar::PageToolbar(Glib::RefPtr const &builder) _margin_popover.set_parent(*this); _text_page_margins.signal_icon_press().connect([&](Gtk::Entry::IconPosition) { + if (_blocker.pending()) { + return; + } + auto guard = _blocker.block(); + // Set the unit of the unit-selector, so the units of the margin spin-edits // are updated to the selected display unit. _unit_tracker->setActiveUnit(_document->getDisplayUnit()); @@ -277,6 +282,11 @@ void PageToolbar::marginsEdited() void PageToolbar::marginSideEdited(BoxSide side, UI::Widget::SpinButton const &entry) { + if (_blocker.pending()) { + return; + } + auto guard = _blocker.block(); + // And modification to the margin causes pages to be enabled auto &pm = _document->getPageManager(); pm.enablePages(); diff --git a/src/ui/toolbar/page-toolbar.h b/src/ui/toolbar/page-toolbar.h index 043cc3d922..3ea07d254a 100644 --- a/src/ui/toolbar/page-toolbar.h +++ b/src/ui/toolbar/page-toolbar.h @@ -17,6 +17,7 @@ #include "svg/svg-box.h" #include "toolbar.h" +#include "ui/operation-blocker.h" #include "ui/widget/spinbutton.h" namespace Gtk { @@ -105,6 +106,7 @@ private: UI::Widget::SpinButton &_margin_left; std::unique_ptr _unit_tracker; + OperationBlocker _blocker; double _unit_to_size(std::string number, std::string unit_str, std::string const &backup); }; -- GitLab From 363102364ffa8a2c4e2bfbbe4380616144937eb8 Mon Sep 17 00:00:00 2001 From: PBS Date: Fri, 25 Jul 2025 00:44:42 +0200 Subject: [PATCH 8/8] Fix margin getting rescaled each time popover opened --- src/ui/toolbar/page-toolbar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/toolbar/page-toolbar.cpp b/src/ui/toolbar/page-toolbar.cpp index ef09b04b67..1f68b03a48 100644 --- a/src/ui/toolbar/page-toolbar.cpp +++ b/src/ui/toolbar/page-toolbar.cpp @@ -292,7 +292,7 @@ void PageToolbar::marginSideEdited(BoxSide side, UI::Widget::SpinButton const &e pm.enablePages(); if (auto page = pm.getSelected()) { - page->setMarginSide(side, entry.get_value(), false); + page->setMarginSide(side, entry.get_text(), false); DocumentUndo::maybeDone(_document, "page-margin", _("Edit page margin"), INKSCAPE_ICON("tool-pages")); setMarginText(page); } -- GitLab