Reference image: avoid overflow on large scales

This has always been possible (especially through the properties
panel with large scales), but it's even easier if the transform
origin is near a manipulated corner.

Check and reject scales that result in an overflowed image box.

libs/kimath/include/geometry/geometry_utils.h

@@ -174,6 +174,19 @@ VECTOR2<ret_type> GetClampedCoords( const VECTOR2<in_type>& aCoords, pad_type aP
 }
 
 
+/**
+ * Check if both coordinates of a vector are within the limits of the integer type.
+ */
+template <typename T>
+inline bool IsVec2SafeXY( const VECTOR2<T>& aVec )
+{
+    constexpr T min = std::numeric_limits<int>::min();
+    constexpr T max = std::numeric_limits<int>::max();
+
+    return aVec.x > min && aVec.x < max && aVec.y > min && aVec.y < max;
+}
+
+
 /**
  * Test if any part of a line falls within the bounds of a rectangle.
  *

libs/kimath/include/math/box2.h

@@ -72,6 +72,11 @@ public:
         return BOX2( aCorner1, aCorner2 - aCorner1 );
     }
 
+    static constexpr BOX2<Vec> ByCenter( const Vec& aCenter, const SizeVec& aSize )
+    {
+        return BOX2( aCenter - aSize / 2, aSize );
+    }
+
     constexpr void SetMaximum()
     {
         if constexpr( std::is_floating_point<coord_type>() )
@@ -916,6 +921,7 @@ private:
 /* Default specializations */
 typedef BOX2<VECTOR2I>    BOX2I;
 typedef BOX2<VECTOR2D>    BOX2D;
+typedef BOX2<VECTOR2L>    BOX2L;
 
 typedef std::optional<BOX2I> OPT_BOX2I;
 
@@ -935,6 +941,21 @@ inline constexpr BOX2I BOX2ISafe( const BOX2D& aInput )
 }
 
 
+/**
+ * Check if a BOX2 is safe for use with BOX2D
+ * (probably BOX2D or BOX2L)
+ */
+template <typename Vec>
+inline constexpr bool IsBOX2Safe( const BOX2<Vec>& aInput )
+{
+    constexpr double high = std::numeric_limits<int>::max();
+    constexpr double low = -std::numeric_limits<int>::max();
+
+    return ( aInput.GetLeft() >= low && aInput.GetTop() >= low &&
+             aInput.GetRight() <= high && aInput.GetBottom() <= high );
+}
+
+
 inline constexpr BOX2I BOX2ISafe( const VECTOR2D& aPos, const VECTOR2D& aSize )
 {
     constexpr double high = std::numeric_limits<int>::max();

pcbnew/pcb_reference_image.cpp

@@ -36,6 +36,7 @@
 #include <core/mirror.h>
 #include <board.h>
 #include <trigo.h>
+#include <geometry/geometry_utils.h>
 #include <geometry/shape_rect.h>
 
 #include <wx/mstream.h>
@@ -165,32 +166,65 @@ double PCB_REFERENCE_IMAGE::ViewGetLOD( int aLayer, KIGFX::VIEW* aView ) const
 const BOX2I PCB_REFERENCE_IMAGE::GetBoundingBox() const
 {
     // Bitmaps are center origin, BOX2Is need top-left origin
-    VECTOR2I size = m_bitmapBase->GetSize();
-    VECTOR2I topLeft = { m_pos.x - size.x / 2, m_pos.y - size.y / 2 };
+    const VECTOR2I size = m_bitmapBase->GetSize();
+    const VECTOR2I topLeft{ m_pos.x - size.x / 2, m_pos.y - size.y / 2 };
 
-    return BOX2I( topLeft, size );
+    return BOX2I{ topLeft, size };
 }
 
 
 std::shared_ptr<SHAPE> PCB_REFERENCE_IMAGE::GetEffectiveShape( PCB_LAYER_ID aLayer,
                                                                FLASHING aFlash ) const
 {
-    BOX2I box = GetBoundingBox();
+    const BOX2I box = GetBoundingBox();
     return std::make_shared<SHAPE_RECT>( box.GetPosition(), box.GetWidth(), box.GetHeight() );
 }
 
 
+void PCB_REFERENCE_IMAGE::SetPosition( const VECTOR2I& aPos )
+{
+    const BOX2D newBox = BOX2D::ByCenter( aPos, m_bitmapBase->GetSize() );
+
+    if( !IsBOX2Safe( newBox ) )
+        return;
+
+    m_pos = aPos;
+}
+
+
+void PCB_REFERENCE_IMAGE::Move( const VECTOR2I& aMoveVector )
+{
+    // Defer to SetPosition to check the new position overflow
+    SetPosition( m_pos + aMoveVector );
+}
+
+
 void PCB_REFERENCE_IMAGE::SetImageScale( double aScale )
 {
+    if( aScale < 0 )
+        return;
+
     const double ratio = aScale / m_bitmapBase->GetScale();
 
+    const VECTOR2D currentOrigin = m_pos + m_transformOriginOffset;
+    const VECTOR2D newOffset = m_transformOriginOffset * ratio;
+    const VECTOR2D newCenter = currentOrigin - newOffset;
+    const VECTOR2D newSize = m_bitmapBase->GetSize() * ratio;
+
+    // The span of the image is limited to the size of the coordinate system
+    if( !IsVec2SafeXY( newSize ) )
+        return;
+
+    const BOX2D newBox = BOX2D::ByCenter( newCenter, newSize );
+
+    // Any overflow, just reject the call
+    if( !IsBOX2Safe( newBox ) )
+        return;
+
     m_bitmapBase->SetScale( aScale );
-
-    const VECTOR2I currentOrigin = m_pos + m_transformOriginOffset;
-    const VECTOR2I newOffset = m_transformOriginOffset * ratio;
-
-    SetTransformOriginOffset( newOffset );
-    SetPosition( currentOrigin - newOffset );
+    SetTransformOriginOffset( KiROUND( newOffset ) );
+    // Don't need to recheck the box, we just did that
+    m_pos = KiROUND( newCenter );
 }
 
 
@@ -202,7 +236,15 @@ const VECTOR2I PCB_REFERENCE_IMAGE::GetSize() const
 
 void PCB_REFERENCE_IMAGE::Flip( const VECTOR2I& aCentre, FLIP_DIRECTION aFlipDirection )
 {
-    MIRROR( m_pos, aCentre, aFlipDirection );
+    VECTOR2I newPos = m_pos;
+    MIRROR( newPos, aCentre, aFlipDirection );
+
+    const BOX2D newBox = BOX2D::ByCenter( newPos, m_bitmapBase->GetSize() );
+
+    if( !IsBOX2Safe( newBox ) )
+        return;
+
+    m_pos = newPos;
     m_bitmapBase->Mirror( aFlipDirection );
 }
 

pcbnew/pcb_reference_image.h

@@ -72,6 +72,9 @@ public:
      *
      * The image is scaled such that the position of the image's
      * transform origin is unchanged.
+     *
+     * If the scale is negaive or the image would overflow the
+     * the coordinate system, nothing is updated.
      */
     void SetImageScale( double aScale );
 
@@ -119,7 +122,7 @@ public:
      */
     bool ReadImageFile( wxMemoryBuffer& aBuf );
 
-    void Move( const VECTOR2I& aMoveVector ) override { m_pos += aMoveVector; }
+    void Move( const VECTOR2I& aMoveVector ) override;
 
     void Flip( const VECTOR2I& aCentre, FLIP_DIRECTION aFlipDirection ) override;
     void Rotate( const VECTOR2I& aCenter, const EDA_ANGLE& aAngle ) override;
@@ -133,8 +136,17 @@ public:
 
     void GetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vector<MSG_PANEL_ITEM>& aList ) override;
 
+    /**
+     * Get the position of the image (this is the center of the image).
+     */
     VECTOR2I GetPosition() const override { return m_pos; }
-    void     SetPosition( const VECTOR2I& aPosition ) override { m_pos = aPosition; }
+
+    /**
+     * Set the position of the image.
+     *
+     * If this results in the image overflowing the coordinate system, nothing is updated.
+     */
+    void SetPosition( const VECTOR2I& aPosition ) override;
 
     /**
      * Get the center of scaling, etc, relative to the image center (GetPosition()).

qa/tests/libs/kimath/math/test_box2.cpp

@@ -104,6 +104,14 @@ BOOST_AUTO_TEST_CASE( ByCorners )
     BOOST_CHECK( boxByCorners == boxByPosSize );
 }
 
+BOOST_AUTO_TEST_CASE( ByCentre )
+{
+    const BOX2I boxByCenter = BOX2I::ByCenter( VECTOR2I( 100, 100 ), VECTOR2I( 20, 20 ) );
+    const BOX2I boxByPosSize = BOX2I( VECTOR2I( 90, 90 ), VECTOR2I( 20, 20 ) );
+
+    BOOST_CHECK( boxByCenter == boxByPosSize );
+}
+
 BOOST_AUTO_TEST_CASE( test_closest_point_to, *boost::unit_test::tolerance( 0.000001 ) )
 {
     BOX2D box( VECTOR2D( 1, 2 ), VECTOR2D( 3, 4 ) );