From 46ff069be36183c93014007a8147a21dbf26a9bc Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Mon, 2 Feb 2026 11:37:29 -0800 Subject: [PATCH] make textbox handles spacing from text The textbox resize handles could be positioned at the box geometry corners even when the box was smaller than the rendered text, causing visual misalignment between the handles and the visible text content. Add PCB_TEXTBOX::GetMinSize() which computes the minimum box dimensions needed to contain the text (accounting for margins, font metrics, line wrapping, and rotation). Thread this minimum through PinEditedCorner() during interactive resize so the textbox cannot be shrunk below the text content bounds. Fixes https://gitlab.com/kicad/code/kicad/-/issues/22887 --- eeschema/dialogs/dialog_text_properties.cpp | 23 +++ eeschema/sch_textbox.cpp | 20 +++ eeschema/sch_textbox.h | 7 + eeschema/tools/sch_point_editor.cpp | 13 +- pcbnew/dialogs/dialog_textbox_properties.cpp | 20 +++ pcbnew/pcb_textbox.cpp | 27 +++ pcbnew/pcb_textbox.h | 7 + pcbnew/tools/pcb_point_editor.cpp | 18 +- qa/tests/eeschema/CMakeLists.txt | 1 + qa/tests/pcbnew/CMakeLists.txt | 1 + qa/tests/pcbnew/test_pcb_textbox.cpp | 178 +++++++++++++++++++ 11 files changed, 306 insertions(+), 9 deletions(-) create mode 100644 qa/tests/pcbnew/test_pcb_textbox.cpp diff --git a/eeschema/dialogs/dialog_text_properties.cpp b/eeschema/dialogs/dialog_text_properties.cpp index 33a3bea883..a00bc78b91 100644 --- a/eeschema/dialogs/dialog_text_properties.cpp +++ b/eeschema/dialogs/dialog_text_properties.cpp @@ -565,6 +565,29 @@ bool DIALOG_TEXT_PROPERTIES::TransferDataFromWindow() textBox->SetFillMode( m_filledCtrl->GetValue() ? FILL_T::FILLED_WITH_COLOR : FILL_T::NO_FILL ); textBox->SetFillColor( m_fillColorSwatch->GetSwatchColor() ); + + textBox->ClearBoundingBoxCache(); + textBox->ClearRenderCache(); + + VECTOR2I minBoxSize = textBox->GetMinSize(); + VECTOR2I start = textBox->GetStart(); + VECTOR2I end = textBox->GetEnd(); + bool expanded = false; + + if( minBoxSize.x > 0 && std::abs( end.x - start.x ) < minBoxSize.x ) + { + end.x = ( end.x >= start.x ) ? start.x + minBoxSize.x : start.x - minBoxSize.x; + expanded = true; + } + + if( minBoxSize.y > 0 && std::abs( end.y - start.y ) < minBoxSize.y ) + { + end.y = ( end.y >= start.y ) ? start.y + minBoxSize.y : start.y - minBoxSize.y; + expanded = true; + } + + if( expanded ) + textBox->SetEnd( end ); } if( !commit.Empty() ) diff --git a/eeschema/sch_textbox.cpp b/eeschema/sch_textbox.cpp index 472fef3465..41e24ac89b 100644 --- a/eeschema/sch_textbox.cpp +++ b/eeschema/sch_textbox.cpp @@ -84,6 +84,26 @@ int SCH_TEXTBOX::GetLegacyTextMargin() const } +VECTOR2I SCH_TEXTBOX::GetMinSize() const +{ + if( GetText().IsEmpty() ) + return VECTOR2I( 0, 0 ); + + BOX2I textBox = GetTextBox( nullptr ); + + int textHeight = std::abs( textBox.GetHeight() ); + + if( GetTextAngle().IsVertical() ) + { + textHeight += GetMarginLeft() + GetMarginRight(); + return VECTOR2I( textHeight, 0 ); + } + + textHeight += GetMarginTop() + GetMarginBottom(); + return VECTOR2I( 0, textHeight ); +} + + void SCH_TEXTBOX::MirrorHorizontally( int aCenter ) { SCH_SHAPE::MirrorHorizontally( aCenter ); diff --git a/eeschema/sch_textbox.h b/eeschema/sch_textbox.h index f20abe9d9c..d6bea83ad6 100644 --- a/eeschema/sch_textbox.h +++ b/eeschema/sch_textbox.h @@ -55,6 +55,13 @@ public: int GetLegacyTextMargin() const; + /** + * Return the minimum height needed to contain the textbox's wrapped text content + * plus margins. Width is unconstrained (returns 0) so text freely rewraps. + * The constrained axis depends on orientation: y-axis for horizontal, x-axis for vertical. + */ + VECTOR2I GetMinSize() const; + void SetMarginLeft( int aLeft ) { m_marginLeft = aLeft; } void SetMarginTop( int aTop ) { m_marginTop = aTop; } void SetMarginRight( int aRight ) { m_marginRight = aRight; } diff --git a/eeschema/tools/sch_point_editor.cpp b/eeschema/tools/sch_point_editor.cpp index d9f2fa31bb..0b03d34d2e 100644 --- a/eeschema/tools/sch_point_editor.cpp +++ b/eeschema/tools/sch_point_editor.cpp @@ -498,14 +498,18 @@ public: } } - static void UpdateItem( SCH_SHAPE& aRect, const EDIT_POINT& aEditedPoint, EDIT_POINTS& aPoints ) + static void UpdateItem( SCH_SHAPE& aRect, const EDIT_POINT& aEditedPoint, + EDIT_POINTS& aPoints, const VECTOR2I& aMinSize = { 0, 0 } ) { VECTOR2I topLeft = aPoints.Point( RECT_TOPLEFT ).GetPosition(); VECTOR2I topRight = aPoints.Point( RECT_TOPRIGHT ).GetPosition(); VECTOR2I botLeft = aPoints.Point( RECT_BOTLEFT ).GetPosition(); VECTOR2I botRight = aPoints.Point( RECT_BOTRIGHT ).GetPosition(); - PinEditedCorner( aEditedPoint, aPoints, schIUScale.MilsToIU( 1 ), schIUScale.MilsToIU( 1 ), + int minWidth = std::max( schIUScale.MilsToIU( 1 ), aMinSize.x ); + int minHeight = std::max( schIUScale.MilsToIU( 1 ), aMinSize.y ); + + PinEditedCorner( aEditedPoint, aPoints, minWidth, minHeight, topLeft, topRight, botLeft, botRight ); if( isModified( aEditedPoint, aPoints.Point( RECT_TOPLEFT ) ) @@ -731,7 +735,10 @@ public: void UpdateItem( const EDIT_POINT& aEditedPoint, EDIT_POINTS& aPoints, COMMIT& aCommit, std::vector& aUpdatedItems ) override { - RECTANGLE_POINT_EDIT_BEHAVIOR::UpdateItem( m_textbox, aEditedPoint, aPoints ); + m_textbox.ClearBoundingBoxCache(); + VECTOR2I minSize = m_textbox.GetMinSize(); + + RECTANGLE_POINT_EDIT_BEHAVIOR::UpdateItem( m_textbox, aEditedPoint, aPoints, minSize ); m_textbox.ClearRenderCache(); } diff --git a/pcbnew/dialogs/dialog_textbox_properties.cpp b/pcbnew/dialogs/dialog_textbox_properties.cpp index 5497e26bdc..7f4bc06fd5 100644 --- a/pcbnew/dialogs/dialog_textbox_properties.cpp +++ b/pcbnew/dialogs/dialog_textbox_properties.cpp @@ -483,6 +483,26 @@ bool DIALOG_TEXTBOX_PROPERTIES::TransferDataFromWindow() m_textBox->ClearBoundingBoxCache(); m_textBox->ClearRenderCache(); + VECTOR2I minBoxSize = m_textBox->GetMinSize(); + VECTOR2I start = m_textBox->GetStart(); + VECTOR2I end = m_textBox->GetEnd(); + bool expanded = false; + + if( minBoxSize.x > 0 && std::abs( end.x - start.x ) < minBoxSize.x ) + { + end.x = ( end.x >= start.x ) ? start.x + minBoxSize.x : start.x - minBoxSize.x; + expanded = true; + } + + if( minBoxSize.y > 0 && std::abs( end.y - start.y ) < minBoxSize.y ) + { + end.y = ( end.y >= start.y ) ? start.y + minBoxSize.y : start.y - minBoxSize.y; + expanded = true; + } + + if( expanded ) + m_textBox->SetEnd( end ); + if( pushCommit ) commit.Push( _( "Edit Text Box Properties" ) ); diff --git a/pcbnew/pcb_textbox.cpp b/pcbnew/pcb_textbox.cpp index c59c7e0de4..71bf44788b 100644 --- a/pcbnew/pcb_textbox.cpp +++ b/pcbnew/pcb_textbox.cpp @@ -193,6 +193,33 @@ int PCB_TEXTBOX::GetLegacyTextMargin() const } +VECTOR2I PCB_TEXTBOX::GetMinSize() const +{ + if( GetText().IsEmpty() ) + return VECTOR2I( 0, 0 ); + + BOX2I textBox = GetTextBox( nullptr ); + + int textHeight = std::abs( textBox.GetHeight() ); + + if( GetTextAngle().IsHorizontal() ) + textHeight += GetMarginTop() + GetMarginBottom(); + else + textHeight += GetMarginLeft() + GetMarginRight(); + + // Only enforce minimum height. Width returns 0 so the user can freely shrink width + // (text rewraps) while height is constrained to fit the wrapped text content. + // GetTextBox returns dimensions in text-local coordinates. For 90/270 degree rotations, + // the text's natural height maps to screen x-axis. + EDA_ANGLE rotation = GetDrawRotation(); + + if( rotation == ANGLE_90 || rotation == ANGLE_270 ) + return VECTOR2I( textHeight, 0 ); + + return VECTOR2I( 0, textHeight ); +} + + VECTOR2I PCB_TEXTBOX::GetTopLeft() const { EDA_ANGLE rotation = GetDrawRotation(); diff --git a/pcbnew/pcb_textbox.h b/pcbnew/pcb_textbox.h index dbfad6e483..1ac0155c30 100644 --- a/pcbnew/pcb_textbox.h +++ b/pcbnew/pcb_textbox.h @@ -72,6 +72,13 @@ public: VECTOR2I GetTopLeft() const override; VECTOR2I GetBotRight() const override; + /** + * Return the minimum height needed to contain the textbox's wrapped text content + * plus margins. Width is unconstrained (returns 0) so text freely rewraps. + * The constrained axis depends on rotation: y-axis for 0/180, x-axis for 90/270. + */ + VECTOR2I GetMinSize() const; + void SetTop( int aVal ) override; void SetLeft( int aVal ) override; void SetRight( int aVal ) override; diff --git a/pcbnew/tools/pcb_point_editor.cpp b/pcbnew/tools/pcb_point_editor.cpp index d174ce1d70..238328ac95 100644 --- a/pcbnew/tools/pcb_point_editor.cpp +++ b/pcbnew/tools/pcb_point_editor.cpp @@ -269,7 +269,7 @@ public: } static void UpdateItem( PCB_SHAPE& aRectangle, const EDIT_POINT& aEditedPoint, - EDIT_POINTS& aPoints ) + EDIT_POINTS& aPoints, const VECTOR2I& aMinSize = { 0, 0 } ) { // You can have more points if your item wants to have more points // (this class assumes the rect points come first, but that can be changed) @@ -301,7 +301,8 @@ public: VECTOR2I botLeft = aPoints.Point( RECT_BOT_LEFT ).GetPosition(); VECTOR2I botRight = aPoints.Point( RECT_BOT_RIGHT ).GetPosition(); - PinEditedCorner( aEditedPoint, aPoints, topLeft, topRight, botLeft, botRight ); + PinEditedCorner( aEditedPoint, aPoints, topLeft, topRight, botLeft, botRight, + { 0, 0 }, { 0, 0 }, aMinSize ); if( isModified( aEditedPoint, aPoints.Point( RECT_TOP_LEFT ) ) || isModified( aEditedPoint, aPoints.Point( RECT_TOP_RIGHT ) ) @@ -417,10 +418,11 @@ public: */ static void PinEditedCorner( const EDIT_POINT& aEditedPoint, const EDIT_POINTS& aEditPoints, VECTOR2I& aTopLeft, VECTOR2I& aTopRight, VECTOR2I& aBotLeft, VECTOR2I& aBotRight, - const VECTOR2I& aHole = { 0, 0 }, const VECTOR2I& aHoleSize = { 0, 0 } ) + const VECTOR2I& aHole = { 0, 0 }, const VECTOR2I& aHoleSize = { 0, 0 }, + const VECTOR2I& aMinSize = { 0, 0 } ) { - int minWidth = EDA_UNIT_UTILS::Mils2IU( pcbIUScale, 1 ); - int minHeight = EDA_UNIT_UTILS::Mils2IU( pcbIUScale, 1 ); + int minWidth = std::max( EDA_UNIT_UTILS::Mils2IU( pcbIUScale, 1 ), aMinSize.x ); + int minHeight = std::max( EDA_UNIT_UTILS::Mils2IU( pcbIUScale, 1 ), aMinSize.y ); if( isModified( aEditedPoint, aEditPoints.Point( RECT_TOP_LEFT ) ) ) { @@ -1637,7 +1639,11 @@ public: std::vector& aUpdatedItems ) override { if( m_textbox.GetShape() == SHAPE_T::RECTANGLE ) - RECTANGLE_POINT_EDIT_BEHAVIOR::UpdateItem( m_textbox, aEditedPoint, aPoints ); + { + m_textbox.ClearBoundingBoxCache(); + VECTOR2I minSize = m_textbox.GetMinSize(); + RECTANGLE_POINT_EDIT_BEHAVIOR::UpdateItem( m_textbox, aEditedPoint, aPoints, minSize ); + } } private: diff --git a/qa/tests/eeschema/CMakeLists.txt b/qa/tests/eeschema/CMakeLists.txt index 1b45f74aac..dfdc8e1499 100644 --- a/qa/tests/eeschema/CMakeLists.txt +++ b/qa/tests/eeschema/CMakeLists.txt @@ -111,6 +111,7 @@ set( QA_EESCHEMA_SRCS test_sch_sheet_list.cpp test_sch_symbol.cpp test_sch_table.cpp + test_sch_textbox.cpp test_schematic.cpp test_flat_hierarchy.cpp test_multi_top_level_sheets.cpp diff --git a/qa/tests/pcbnew/CMakeLists.txt b/qa/tests/pcbnew/CMakeLists.txt index ec4e2a30f9..5d7979cd8e 100644 --- a/qa/tests/pcbnew/CMakeLists.txt +++ b/qa/tests/pcbnew/CMakeLists.txt @@ -51,6 +51,7 @@ set( QA_PCBNEW_SRCS test_pns_basics.cpp test_pad_flashing.cpp test_pad_numbering.cpp + test_pcb_textbox.cpp test_prettifier.cpp test_pcb_render_settings.cpp test_libeval_compiler.cpp diff --git a/qa/tests/pcbnew/test_pcb_textbox.cpp b/qa/tests/pcbnew/test_pcb_textbox.cpp new file mode 100644 index 0000000000..3c540c61be --- /dev/null +++ b/qa/tests/pcbnew/test_pcb_textbox.cpp @@ -0,0 +1,178 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright The KiCad Developers, see AUTHORS.TXT for contributors. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, you may find one here: + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html + * or you may search the http://www.gnu.org website for the version 2 license, + * or you may write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include +#include +#include + + +struct PCB_TEXTBOX_FIXTURE +{ + BOARD m_board; + PCB_TEXTBOX m_textbox; + + PCB_TEXTBOX_FIXTURE() : + m_board(), + m_textbox( &m_board ) + { + m_textbox.SetText( wxT( "Hello World" ) ); + m_textbox.SetTextSize( VECTOR2I( pcbIUScale.mmToIU( 1.0 ), pcbIUScale.mmToIU( 1.0 ) ) ); + m_textbox.SetStart( VECTOR2I( 0, 0 ) ); + m_textbox.SetEnd( VECTOR2I( pcbIUScale.mmToIU( 20.0 ), pcbIUScale.mmToIU( 5.0 ) ) ); + } +}; + + +BOOST_FIXTURE_TEST_SUITE( PcbTextbox, PCB_TEXTBOX_FIXTURE ) + + +/** + * Verify that GetMinSize() returns height-only constraint for non-empty text. + * Width should be 0 (unconstrained) so text can rewrap freely. + */ +BOOST_AUTO_TEST_CASE( GetMinSizeReturnsHeightOnly ) +{ + VECTOR2I minSize = m_textbox.GetMinSize(); + + BOOST_CHECK_EQUAL( minSize.x, 0 ); + BOOST_CHECK_GT( minSize.y, 0 ); +} + + +/** + * Verify that GetMinSize() height grows when more text is added. + */ +BOOST_AUTO_TEST_CASE( GetMinSizeGrowsWithMoreText ) +{ + VECTOR2I minSizeShort = m_textbox.GetMinSize(); + + m_textbox.SetText( wxT( "Hello World\nSecond Line\nThird Line" ) ); + m_textbox.ClearBoundingBoxCache(); + m_textbox.ClearRenderCache(); + VECTOR2I minSizeLong = m_textbox.GetMinSize(); + + BOOST_CHECK_GT( minSizeLong.y, minSizeShort.y ); +} + + +/** + * Verify that GetMinSize() height grows when vertical margins are increased. + */ +BOOST_AUTO_TEST_CASE( GetMinSizeIncludesMargins ) +{ + m_textbox.SetText( wxT( "A" ) ); + m_textbox.SetMarginLeft( 0 ); + m_textbox.SetMarginRight( 0 ); + m_textbox.SetMarginTop( 0 ); + m_textbox.SetMarginBottom( 0 ); + m_textbox.ClearBoundingBoxCache(); + m_textbox.ClearRenderCache(); + + VECTOR2I minSizeNoMargin = m_textbox.GetMinSize(); + + int margin = pcbIUScale.mmToIU( 1.0 ); + m_textbox.SetMarginTop( margin ); + m_textbox.SetMarginBottom( margin ); + m_textbox.ClearBoundingBoxCache(); + m_textbox.ClearRenderCache(); + + VECTOR2I minSizeWithMargin = m_textbox.GetMinSize(); + + BOOST_CHECK_EQUAL( minSizeNoMargin.x, 0 ); + BOOST_CHECK_EQUAL( minSizeWithMargin.x, 0 ); + BOOST_CHECK_GT( minSizeWithMargin.y, minSizeNoMargin.y ); +} + + +/** + * Verify that GetMinSize() height is at least the text height for a single line of text. + */ +BOOST_AUTO_TEST_CASE( GetMinSizeHeightAtLeastTextHeight ) +{ + m_textbox.SetText( wxT( "A" ) ); + m_textbox.ClearBoundingBoxCache(); + m_textbox.ClearRenderCache(); + VECTOR2I minSize = m_textbox.GetMinSize(); + + BOOST_CHECK_GE( minSize.y, m_textbox.GetTextSize().y ); +} + + +/** + * Verify that GetMinSize() returns zero for both axes with empty text. + */ +BOOST_AUTO_TEST_CASE( GetMinSizeEmptyText ) +{ + m_textbox.SetText( wxT( "" ) ); + m_textbox.ClearBoundingBoxCache(); + m_textbox.ClearRenderCache(); + + VECTOR2I minSize = m_textbox.GetMinSize(); + + BOOST_CHECK_EQUAL( minSize.x, 0 ); + BOOST_CHECK_EQUAL( minSize.y, 0 ); +} + + +/** + * Verify that GetMinSize() swaps the constrained axis for 90-degree rotation. + * At 0 degrees the height constraint is on y. At 90 degrees it moves to x. + */ +BOOST_AUTO_TEST_CASE( GetMinSizeRotated90 ) +{ + m_textbox.SetText( wxT( "Wide Text" ) ); + m_textbox.ClearBoundingBoxCache(); + m_textbox.ClearRenderCache(); + + VECTOR2I minSize0 = m_textbox.GetMinSize(); + + m_textbox.SetTextAngle( EDA_ANGLE( 90.0, DEGREES_T ) ); + m_textbox.ClearBoundingBoxCache(); + m_textbox.ClearRenderCache(); + + VECTOR2I minSize90 = m_textbox.GetMinSize(); + + BOOST_CHECK_EQUAL( minSize0.x, 0 ); + BOOST_CHECK_GT( minSize0.y, 0 ); + BOOST_CHECK_GT( minSize90.x, 0 ); + BOOST_CHECK_EQUAL( minSize90.y, 0 ); +} + + +/** + * Verify that width is always unconstrained regardless of text content. + */ +BOOST_AUTO_TEST_CASE( GetMinSizeAllowsWidthReduction ) +{ + m_textbox.SetText( wxT( "This is a very long line of text that should exceed the box width" ) ); + m_textbox.ClearBoundingBoxCache(); + m_textbox.ClearRenderCache(); + + VECTOR2I minSize = m_textbox.GetMinSize(); + + BOOST_CHECK_EQUAL( minSize.x, 0 ); + BOOST_CHECK_GT( minSize.y, 0 ); +} + + +BOOST_AUTO_TEST_SUITE_END()