From 68655540eb01d74899a21b64fb701e445435ee49 Mon Sep 17 00:00:00 2001 From: dsa-t Date: Fri, 11 Feb 2022 16:42:52 +0000 Subject: [PATCH] Clamp cursor to limits of coordinates representation Also improves large distance handling. Fixes https://gitlab.com/kicad/code/kicad/-/issues/8846 --- common/view/wx_view_controls.cpp | 38 ++++++++++------- include/view/view_controls.h | 14 +++++++ libs/kimath/include/geometry/geometry_utils.h | 41 +++++++++++++++++++ pcbnew/tools/pcb_grid_helper.cpp | 8 ++-- 4 files changed, 83 insertions(+), 18 deletions(-) diff --git a/common/view/wx_view_controls.cpp b/common/view/wx_view_controls.cpp index a6e1446b58..f8f8990821 100644 --- a/common/view/wx_view_controls.cpp +++ b/common/view/wx_view_controls.cpp @@ -36,6 +36,7 @@ #include #include #include // for KiROUND +#include #include #include #include @@ -302,7 +303,7 @@ void WX_VIEW_CONTROLS::onMotion( wxMouseEvent& aEvent ) } if( m_updateCursor ) // do not update the cursor position if it was explicitly set - m_cursorPos = m_view->ToWorld( mousePos ); + m_cursorPos = GetClampedCoords( m_view->ToWorld( mousePos ) ); else m_updateCursor = true; @@ -648,7 +649,7 @@ VECTOR2D WX_VIEW_CONTROLS::GetMousePosition( bool aWorldCoordinates ) const wxPoint msp = getMouseScreenPosition(); VECTOR2D screenPos( msp.x, msp.y ); - return aWorldCoordinates ? m_view->ToWorld( screenPos ) : screenPos; + return aWorldCoordinates ? GetClampedCoords( m_view->ToWorld( screenPos ) ) : screenPos; } @@ -675,7 +676,7 @@ VECTOR2D WX_VIEW_CONTROLS::GetCursorPosition( bool aEnableSnapping ) const } else { - return GetRawCursorPosition( aEnableSnapping ); + return GetClampedCoords( GetRawCursorPosition( aEnableSnapping ) ); } } @@ -685,10 +686,12 @@ void WX_VIEW_CONTROLS::SetCursorPosition( const VECTOR2D& aPosition, bool aWarpV { m_updateCursor = false; + VECTOR2D clampedPosition = GetClampedCoords( aPosition ); + if( aTriggeredByArrows ) { m_settings.m_lastKeyboardCursorPositionValid = true; - m_settings.m_lastKeyboardCursorPosition = aPosition; + m_settings.m_lastKeyboardCursorPosition = clampedPosition; m_settings.m_lastKeyboardCursorCommand = aArrowCommand; m_cursorWarped = false; } @@ -700,8 +703,8 @@ void WX_VIEW_CONTROLS::SetCursorPosition( const VECTOR2D& aPosition, bool aWarpV m_cursorWarped = true; } - WarpCursor( aPosition, true, aWarpView ); - m_cursorPos = aPosition; + WarpCursor( clampedPosition, true, aWarpView ); + m_cursorPos = clampedPosition; } @@ -710,14 +713,16 @@ void WX_VIEW_CONTROLS::SetCrossHairCursorPosition( const VECTOR2D& aPosition, { m_updateCursor = false; + VECTOR2D clampedPosition = GetClampedCoords( aPosition ); + const VECTOR2I& screenSize = m_view->GetGAL()->GetScreenPixelSize(); - BOX2I screen( VECTOR2I( 0, 0 ), screenSize ); - VECTOR2D screenPos = m_view->ToScreen( aPosition ); + BOX2I screen( VECTOR2I( 0, 0 ), screenSize ); + VECTOR2D screenPos = m_view->ToScreen( clampedPosition ); if( aWarpView && !screen.Contains( screenPos ) ) - m_view->SetCenter( aPosition ); + m_view->SetCenter( clampedPosition ); - m_cursorPos = aPosition; + m_cursorPos = clampedPosition; } @@ -727,14 +732,15 @@ void WX_VIEW_CONTROLS::WarpCursor( const VECTOR2D& aPosition, bool aWorldCoordin if( aWorldCoordinates ) { const VECTOR2I& screenSize = m_view->GetGAL()->GetScreenPixelSize(); - BOX2I screen( VECTOR2I( 0, 0 ), screenSize ); - VECTOR2D screenPos = m_view->ToScreen( aPosition ); + BOX2I screen( VECTOR2I( 0, 0 ), screenSize ); + VECTOR2D clampedPosition = GetClampedCoords( aPosition ); + VECTOR2D screenPos = m_view->ToScreen( clampedPosition ); if( !screen.Contains( screenPos ) ) { if( aWarpView ) { - m_view->SetCenter( aPosition ); + m_view->SetCenter( clampedPosition ); m_parentPanel->WarpPointer( screenSize.x / 2, screenSize.y / 2 ); } } @@ -889,7 +895,7 @@ void WX_VIEW_CONTROLS::refreshMouse() moveEvent.SetShiftDown( wxGetKeyState( WXK_SHIFT ) ); moveEvent.SetAltDown( wxGetKeyState( WXK_ALT ) ); - m_cursorPos = m_view->ToWorld( VECTOR2D( msp.x, msp.y ) ); + m_cursorPos = GetClampedCoords( m_view->ToWorld( VECTOR2D( msp.x, msp.y ) ) ); wxPostEvent( m_parentPanel, moveEvent ); } @@ -943,6 +949,8 @@ void WX_VIEW_CONTROLS::UpdateScrollbars() void WX_VIEW_CONTROLS::ForceCursorPosition( bool aEnabled, const VECTOR2D& aPosition ) { + VECTOR2D clampedPosition = GetClampedCoords( aPosition ); + m_settings.m_forceCursorPosition = aEnabled; - m_settings.m_forcedPosition = aPosition; + m_settings.m_forcedPosition = clampedPosition; } diff --git a/include/view/view_controls.h b/include/view/view_controls.h index 2cab29d5fa..119d14feeb 100644 --- a/include/view/view_controls.h +++ b/include/view/view_controls.h @@ -204,6 +204,8 @@ public: * @note The position may be different from the cursor position if snapping is * enabled (@see GetCursorPosition()). * + * @note The position is clamped if outside of coordinates representation limits. + * * @param aWorldCoordinates if true, the result is given in world coordinates, otherwise * it is given in screen coordinates. * @return The current mouse pointer position in either world or screen coordinates. @@ -216,6 +218,8 @@ public: * @note The position may be different from the mouse pointer position if snapping is * enabled or cursor position is forced to a specific point. * + * @note The position is clamped if outside of coordinates representation limits. + * * @return The current cursor position in world coordinates. */ VECTOR2D GetCursorPosition() const @@ -237,6 +241,8 @@ public: * @note The position may be different from the mouse pointer position if snapping is * enabled or cursor position is forced to a specific point. * + * @note The position is clamped if outside of coordinates representation limits. + * * @param aEnableSnapping selects whether cursor position should be snapped to the grid. * @return The current cursor position in world coordinates. */ @@ -245,6 +251,8 @@ public: /** * Place the cursor immediately at a given point. Mouse movement is ignored. * + * @note The position is clamped if outside of coordinates representation limits. + * * @param aEnabled enable forced cursor position * @param aPosition the position (world coordinates). */ @@ -260,6 +268,8 @@ public: * The position is not forced and will be overridden with the next mouse motion event. * Mouse cursor follows the world cursor. * + * @note The position is clamped if outside of coordinates representation limits. + * * @param aPosition is the requested cursor position in the world coordinates. * @param aWarpView enables/disables view warp if the cursor is outside the current viewport. */ @@ -270,6 +280,8 @@ public: /** * Move the graphic crosshair cursor to the requested position expressed in world coordinates. * + * @note The position is clamped if outside of coordinates representation limits. + * * @param aPosition is the requested cursor position in the world coordinates. * @param aWarpView enables/disables view warp if the cursor is outside the current viewport. */ @@ -303,6 +315,8 @@ public: * If enabled (@see SetEnableCursorWarping(), warps the cursor to the specified position, * expressed either in the screen coordinates or the world coordinates. * + * @note The position is clamped if outside of coordinates representation limits. + * * @param aPosition is the position where the cursor should be warped. * @param aWorldCoordinates if true treats aPosition as the world coordinates, otherwise it * uses it as the screen coordinates. diff --git a/libs/kimath/include/geometry/geometry_utils.h b/libs/kimath/include/geometry/geometry_utils.h index 12c6095bef..607353e022 100644 --- a/libs/kimath/include/geometry/geometry_utils.h +++ b/libs/kimath/include/geometry/geometry_utils.h @@ -133,6 +133,47 @@ VECTOR2 GetVectorSnapped45( const VECTOR2& aVec, bool only45 = false ) } +/** + * Clamps a vector to values that can be negated, respecting numeric limits + * of coordinates data type with specified padding. + * + * Numeric limits are (-2^31 + 1) to (2^31 - 1). + * + * Takes care of rounding in case of floating point to integer conversion. + * + * @param aCoord - vector to clamp. + * @param aPadding - padding from the limits. Must not be negative. + * @return clamped vector. + */ +template ::value>::type> +VECTOR2 GetClampedCoords( const VECTOR2& aCoords, pad_type aPadding = 0u ) +{ + typedef std::numeric_limits coord_limits; + + long max = coord_limits::max() - aPadding; + long min = -max; + + in_type x = aCoords.x; + in_type y = aCoords.y; + + if( x < min ) + x = min; + else if( x > max ) + x = max; + + if( y < min ) + y = min; + else if( y > max ) + y = max; + + if( !std::is_integral() && std::is_integral() ) + return VECTOR2( KiROUND( x ), KiROUND( y ) ); + + return VECTOR2( x, y ); +} + + /** * Test if any part of a line falls within the bounds of a rectangle. * diff --git a/pcbnew/tools/pcb_grid_helper.cpp b/pcbnew/tools/pcb_grid_helper.cpp index c83994a8a9..8973293ce5 100644 --- a/pcbnew/tools/pcb_grid_helper.cpp +++ b/pcbnew/tools/pcb_grid_helper.cpp @@ -260,10 +260,12 @@ VECTOR2I PCB_GRID_HELPER::BestSnapAnchor( const VECTOR2I& aOrigin, const LSET& a // see https://gitlab.com/kicad/code/kicad/-/issues/7125 double snapScale = snapSize / m_toolMgr->GetView()->GetGAL()->GetWorldScale(); int snapRange = std::min( KiROUND( snapScale ), GetGrid().x ); - int snapDist = snapRange; + int snapDist = snapRange; - BOX2I bb( VECTOR2I( aOrigin.x - snapRange / 2, aOrigin.y - snapRange / 2 ), - VECTOR2I( snapRange, snapRange ) ); + //Respect limits of coordinates representation + BOX2I bb; + bb.SetOrigin( GetClampedCoords( VECTOR2D( aOrigin ) - snapRange / 2 ) ); + bb.SetEnd( GetClampedCoords( VECTOR2D( aOrigin ) + snapRange / 2 ) ); clearAnchors();