Handle DXF files with out of bounds values

First, ensure that we calculate the bbox of the imported elements.

If the total BBOX is outside our allowed value, refused to import.  This
needs to be scaled or addressed in an external editor

If the bbox fits, then make sure that we clamp it such that the largest
element is still within our bounds

Fixes https://gitlab.com/kicad/code/kicad/-/issues/18523

common/commit.cpp

@@ -163,7 +163,8 @@ void COMMIT::makeEntry( EDA_ITEM* aItem, CHANGE_TYPE aType, EDA_ITEM* aCopy, BAS
     ent.m_copy = aCopy;
     ent.m_screen = aScreen;
 
-    wxASSERT( m_changedItems.find( aItem ) == m_changedItems.end() );
+    // N.B. Do not throw an assertion for multiple changed items.  An item can be changed multiple times
+    // in a single commit such as when importing graphics and grouping them.
 
     m_changedItems.insert( aItem );
     m_changes.push_back( ent );

common/import_gfx/dxf_import_plugin.cpp

@@ -131,7 +131,7 @@ bool DXF_IMPORT_PLUGIN::Load( const wxString& aFileName )
 
         m_internalImporter.ClearShapes();
 
-        reportMsg( _( "Memory was exhausted trying to load the DXF, it may be too large." ) );
+        ReportMsg( _( "Memory was exhausted trying to load the DXF, it may be too large." ) );
         return false;
     }
 }
@@ -151,7 +151,7 @@ bool DXF_IMPORT_PLUGIN::LoadFromMemory( const wxMemoryBuffer& aMemBuffer )
 
         m_internalImporter.ClearShapes();
 
-        reportMsg( _( "Memory was exhausted trying to load the DXF, it may be too large." ) );
+        ReportMsg( _( "Memory was exhausted trying to load the DXF, it may be too large." ) );
         return false;
     }
 }
@@ -247,7 +247,7 @@ bool DXF_IMPORT_PLUGIN::ImportDxfFile( const wxMemoryBuffer& aMemBuffer )
 }
 
 
-void DXF_IMPORT_PLUGIN::reportMsg( const wxString& aMessage )
+void DXF_IMPORT_PLUGIN::ReportMsg( const wxString& aMessage )
 {
     // Add message to keep trace of not handled dxf entities
     m_messages += aMessage;
@@ -1535,7 +1535,7 @@ void DXF_IMPORT_PLUGIN::insertSpline( double aWidth )
                                         // as runtime errors
     {
         // invalid spline definition, drop this block
-        reportMsg( _( "Invalid spline definition encountered" ) );
+        ReportMsg( _( "Invalid spline definition encountered" ) );
         return;
     }
 

common/import_gfx/dxf_import_plugin.h

@@ -320,10 +320,10 @@ public:
     {
         return m_messages;
     }
+    // report message to keep trace of not supported dxf entities:
+    void ReportMsg( const wxString& aMessage ) override;
 
 private:
-    // report message to keep trace of not supported dxf entities:
-    void reportMsg( const wxString& aMessage );
 
     // coordinate conversions from dxf file to mm
     double mapX( double aDxfCoordX );
@@ -457,77 +457,77 @@ private:
     // Not yet handled DXF entities:
     virtual void addXLine( const DL_XLineData& ) override
     {
-        reportMsg( _( "DXF construction lines not currently supported." ) );
+        ReportMsg( _( "DXF construction lines not currently supported." ) );
     }
     virtual void addRay( const DL_RayData& ) override
     {
-        reportMsg( _( "DXF construction lines not currently supported." ) );
+        ReportMsg( _( "DXF construction lines not currently supported." ) );
     }
 
     virtual void addArcAlignedText( const DL_ArcAlignedTextData& ) override
     {
-        reportMsg( _( "DXF arc-aligned text not currently supported." ) );
+        ReportMsg( _( "DXF arc-aligned text not currently supported." ) );
     }
 
     virtual void addDimAlign( const DL_DimensionData&, const DL_DimAlignedData& ) override
     {
-        reportMsg( _( "DXF dimensions not currently supported." ) );
+        ReportMsg( _( "DXF dimensions not currently supported." ) );
     }
     virtual void addDimLinear( const DL_DimensionData&, const DL_DimLinearData& ) override
     {
-        reportMsg( _( "DXF dimensions not currently supported." ) );
+        ReportMsg( _( "DXF dimensions not currently supported." ) );
     }
     virtual void addDimRadial( const DL_DimensionData&, const DL_DimRadialData& ) override
     {
-        reportMsg( _( "DXF dimensions not currently supported." ) );
+        ReportMsg( _( "DXF dimensions not currently supported." ) );
     }
     virtual void addDimDiametric( const DL_DimensionData&, const DL_DimDiametricData& ) override
     {
-        reportMsg( _( "DXF dimensions not currently supported." ) );
+        ReportMsg( _( "DXF dimensions not currently supported." ) );
     }
     virtual void addDimAngular( const DL_DimensionData&, const DL_DimAngular2LData& ) override
     {
-        reportMsg( _( "DXF dimensions not currently supported." ) );
+        ReportMsg( _( "DXF dimensions not currently supported." ) );
     }
     virtual void addDimAngular3P( const DL_DimensionData&, const DL_DimAngular3PData& ) override
     {
-        reportMsg( _( "DXF dimensions not currently supported." ) );
+        ReportMsg( _( "DXF dimensions not currently supported." ) );
     }
     virtual void addDimOrdinate( const DL_DimensionData&, const DL_DimOrdinateData& ) override
     {
-        reportMsg( _( "DXF dimensions not currently supported." ) );
+        ReportMsg( _( "DXF dimensions not currently supported." ) );
     }
     virtual void addLeader( const DL_LeaderData& ) override
     {
-        reportMsg( _( "DXF dimensions not currently supported." ) );
+        ReportMsg( _( "DXF dimensions not currently supported." ) );
     }
     virtual void addLeaderVertex( const DL_LeaderVertexData& ) override { }
 
     virtual void addHatch( const DL_HatchData& ) override
     {
-        reportMsg( _( "DXF hatches not currently supported." ) );
+        ReportMsg( _( "DXF hatches not currently supported." ) );
     }
     virtual void addHatchLoop( const DL_HatchLoopData& ) override { }
     virtual void addHatchEdge( const DL_HatchEdgeData& ) override { }
 
     virtual void addTrace( const DL_TraceData& ) override
     {
-        reportMsg( _( "DXF traces not currently supported." ) );
+        ReportMsg( _( "DXF traces not currently supported." ) );
     }
 
     virtual void add3dFace( const DL_3dFaceData& ) override
     {
-        reportMsg( _( "DXF 3dfaces not currently supported." ) );
+        ReportMsg( _( "DXF 3dfaces not currently supported." ) );
     }
 
     virtual void addSolid( const DL_SolidData& ) override
     {
-        reportMsg( _( "DXF solids not currently supported." ) );
+        ReportMsg( _( "DXF solids not currently supported." ) );
     }
 
     virtual void addImage( const DL_ImageData& ) override
     {
-        reportMsg( _( "DXF images not currently supported." ) );
+        ReportMsg( _( "DXF images not currently supported." ) );
     }
     virtual void linkImage( const DL_ImageDefData& ) override {}
 

common/import_gfx/graphics_import_plugin.h

@@ -131,6 +131,8 @@ public:
      */
     const virtual wxString& GetMessages() const = 0;
 
+    virtual void ReportMsg( const wxString& aMessage ) = 0;
+
 protected:
     ///< Importer used to create objects representing the imported shapes.
     GRAPHICS_IMPORTER* m_importer;

common/import_gfx/graphics_importer.h

@@ -125,6 +125,11 @@ public:
         return m_plugin->GetMessages();
     }
 
+    void ReportMsg( const wxString& aMessage )
+    {
+        m_plugin->ReportMsg( aMessage );
+    }
+
     /**
      * Get original image Width.
      *

common/import_gfx/graphics_importer_buffer.cpp

@@ -98,6 +98,89 @@ void GRAPHICS_IMPORTER_BUFFER::AddShape( std::unique_ptr<IMPORTED_SHAPE>& aShape
 
 void GRAPHICS_IMPORTER_BUFFER::ImportTo( GRAPHICS_IMPORTER& aImporter )
 {
+    BOX2D boundingBox;
+
+    for( std::unique_ptr<IMPORTED_SHAPE>& shape : m_shapes )
+    {
+        BOX2D box = shape->GetBoundingBox();
+
+        if( box.IsValid() )
+            boundingBox.Merge( box );
+    }
+
+    boundingBox.SetOrigin( boundingBox.GetPosition().x * aImporter.GetScale().x,
+                           boundingBox.GetPosition().y * aImporter.GetScale().y );
+    boundingBox.SetSize( boundingBox.GetSize().x * aImporter.GetScale().x,
+                         boundingBox.GetSize().y * aImporter.GetScale().y );
+
+    // Check that the scaled graphics fit in the KiCad numeric limits
+    if( boundingBox.GetSize().x * aImporter.GetMillimeterToIuFactor() > std::numeric_limits<int>::max() ||
+        boundingBox.GetSize().y * aImporter.GetMillimeterToIuFactor() > std::numeric_limits<int>::max() )
+    {
+        double scale_factor = std::numeric_limits<int>::max() / ( aImporter.GetMillimeterToIuFactor() + 100 );
+        double max_scale = std::max( scale_factor / boundingBox.GetSize().x,
+                                     scale_factor / boundingBox.GetSize().y );
+        aImporter.ReportMsg( wxString::Format( _( "Imported graphic is too large. Maximum scale is %f" ),
+                                             max_scale ) );
+        return;
+    }
+    // They haven't set the import offset, so we set it to the bounding box origin to keep the graphics
+    // in the KiCad drawing area
+    else if( aImporter.GetImportOffsetMM() == VECTOR2D( 0, 0 ) )
+    {
+        VECTOR2D offset = boundingBox.GetOrigin();
+        aImporter.SetImportOffsetMM( -offset );
+    }
+    else
+    {
+        VECTOR2D bbox_origin = boundingBox.GetOrigin();
+        aImporter.SetImportOffsetMM( -bbox_origin + aImporter.GetImportOffsetMM() );
+
+        double total_scale_x = aImporter.GetScale().x * aImporter.GetMillimeterToIuFactor();
+        double total_scale_y = aImporter.GetScale().y * aImporter.GetMillimeterToIuFactor();
+
+        double max_offset_x =
+                ( aImporter.GetImportOffsetMM().x + boundingBox.GetRight() ) * total_scale_x;
+        double max_offset_y =
+                ( aImporter.GetImportOffsetMM().y + boundingBox.GetBottom() ) * total_scale_y;
+        double min_offset_x =
+                ( aImporter.GetImportOffsetMM().x + boundingBox.GetLeft() ) * total_scale_x;
+        double min_offset_y =
+                ( aImporter.GetImportOffsetMM().y + boundingBox.GetTop() ) * total_scale_y;
+
+        VECTOR2D newOffset = aImporter.GetImportOffsetMM();
+        bool needsAdjustment = false;
+
+        if( max_offset_x >= std::numeric_limits<int>::max() )
+        {
+            newOffset.x -= ( max_offset_x - std::numeric_limits<int>::max() + 100.0 ) / total_scale_x;
+            needsAdjustment = true;
+        }
+        else if( min_offset_x <= std::numeric_limits<int>::min() )
+        {
+            newOffset.x -= ( min_offset_x - std::numeric_limits<int>::min() - 100 ) / total_scale_x;
+            needsAdjustment = true;
+        }
+
+        if( max_offset_y >= std::numeric_limits<int>::max() )
+        {
+            newOffset.y -= ( max_offset_y - std::numeric_limits<int>::max() + 100 ) / total_scale_y;
+            needsAdjustment = true;
+        }
+        else if( min_offset_y <= std::numeric_limits<int>::min() )
+        {
+            newOffset.y -= ( min_offset_y - std::numeric_limits<int>::min() - 100 ) / total_scale_y;
+            needsAdjustment = true;
+        }
+
+        if( needsAdjustment )
+        {
+            aImporter.ReportMsg( wxString::Format( _( "Import offset adjusted to (%f, %f) to fit within numeric limits" ),
+                                                   newOffset.x, newOffset.y ) );
+            aImporter.SetImportOffsetMM( newOffset );
+        }
+    }
+
     for( std::unique_ptr<IMPORTED_SHAPE>& shape : m_shapes )
         shape->ImportTo( aImporter );
 }

common/import_gfx/graphics_importer_buffer.h

@@ -30,6 +30,7 @@
 #include "graphics_importer.h"
 
 #include <math/matrix3x3.h>
+#include <math/box2.h>
 #include <list>
 
 
@@ -46,6 +47,8 @@ public:
     void SetParentShapeIndex( int aIndex ) { m_parentShapeIndex = aIndex; }
     int  GetParentShapeIndex() const { return m_parentShapeIndex; }
 
+    virtual BOX2D GetBoundingBox() const = 0;
+
 protected:
     int m_parentShapeIndex = -1;
 };
@@ -75,6 +78,14 @@ public:
         m_end = aTransform * m_end + aTranslation;
     }
 
+    BOX2D GetBoundingBox() const override
+    {
+        BOX2D box;
+        box.Merge( m_start );
+        box.Merge( m_end );
+        return box;
+    }
+
 private:
     VECTOR2D        m_start;
     VECTOR2D        m_end;
@@ -113,6 +124,14 @@ public:
         m_radius = newRadius.EuclideanNorm();
     }
 
+    BOX2D GetBoundingBox() const override
+    {
+        BOX2D box;
+        box.Merge( m_center - VECTOR2D( m_radius, m_radius ) );
+        box.Merge( m_center + VECTOR2D( m_radius, m_radius ) );
+        return box;
+    }
+
 private:
     VECTOR2D        m_center;
     double          m_radius;
@@ -148,6 +167,26 @@ public:
         m_center = aTransform * m_center + aTranslation;
     }
 
+    BOX2D GetBoundingBox() const override
+    {
+        BOX2D box;
+
+        box.Merge( m_start + m_stroke.GetWidth() );
+        box.Merge( m_start - m_stroke.GetWidth() );
+
+        for( double angle = 0; angle < m_angle.AsDegrees(); angle += 5 )
+        {
+            EDA_ANGLE ang = EDA_ANGLE( angle );
+            VECTOR2D start = m_center + m_start;
+            VECTOR2D pt = {start.x * ang.Cos() + start.y * ang.Sin(),
+                           start.x * ang.Sin() - start.y * ang.Cos()};
+
+            box.Merge( pt );
+        }
+
+        return box;
+    }
+
 private:
     VECTOR2D        m_center;
     VECTOR2D        m_start;
@@ -192,6 +231,16 @@ public:
 
     const IMPORTED_STROKE& GetStroke() const { return m_stroke; }
 
+    BOX2D GetBoundingBox() const override
+    {
+        BOX2D box;
+
+        for( const VECTOR2D& vert : m_vertices )
+            box.Merge( vert );
+
+        return box;
+    }
+
 private:
     std::vector<VECTOR2D> m_vertices;
     IMPORTED_STROKE       m_stroke;
@@ -233,6 +282,15 @@ public:
         m_height = textSize.y;
     }
 
+    BOX2D GetBoundingBox() const override
+    {
+        BOX2D box;
+        box.Merge( m_origin );
+        box.Merge( m_origin + VECTOR2D( m_width * m_text.length(), m_height ) );
+
+        return box;
+    }
+
 private:
     VECTOR2D          m_origin;
     const wxString    m_text;
@@ -276,6 +334,14 @@ public:
         m_end = aTransform * m_end + aTranslation;
     }
 
+    BOX2D GetBoundingBox() const override
+    {
+        BOX2D box;
+        box.Merge( m_start );
+        box.Merge( m_end );
+        return box;
+    }
+
 private:
     VECTOR2D        m_start;
     VECTOR2D        m_bezierControl1;

common/import_gfx/svg_import_plugin.cpp

@@ -426,3 +426,11 @@ static float distanceFromPointToLine( const VECTOR2D& aPoint, const VECTOR2D& aL
 
     return fabs( distance );
 }
+
+
+void SVG_IMPORT_PLUGIN::ReportMsg( const wxString& aMessage )
+{
+    // Add message to keep trace of not handled svg entities
+    m_messages += aMessage;
+    m_messages += '\n';
+}

common/import_gfx/svg_import_plugin.h

@@ -62,6 +62,8 @@ public:
         return m_messages;
     }
 
+    void ReportMsg( const wxString& aMessage ) override;
+
     bool Import() override;
     bool Load( const wxString& aFileName ) override;
     bool LoadFromMemory( const wxMemoryBuffer& aMemBuffer ) override;

include/view/view_rtree.h

@@ -48,8 +48,8 @@ public:
      */
     void Insert( VIEW_ITEM* aItem, const BOX2I& bbox )
     {
-        const int       mmin[2] = { bbox.GetX(), bbox.GetY() };
+        const int       mmin[2] = { std::min( bbox.GetX(), bbox.GetRight() ), std::min( bbox.GetY(), bbox.GetBottom() ) };
-        const int       mmax[2] = { bbox.GetRight(), bbox.GetBottom() };
+        const int       mmax[2] = { std::max( bbox.GetX(), bbox.GetRight() ), std::max( bbox.GetY(), bbox.GetBottom() ) };
 
         VIEW_RTREE_BASE::Insert( mmin, mmax, aItem );
     }
@@ -65,8 +65,8 @@ public:
 
         if( aBbox )
         {
-            const int mmin[2] = { aBbox->GetX(), aBbox->GetY() };
+            const int mmin[2] = { std::min( aBbox->GetX(), aBbox->GetRight() ), std::min( aBbox->GetY(), aBbox->GetBottom() ) };
-            const int mmax[2] = { aBbox->GetRight(), aBbox->GetBottom() };
+            const int mmax[2] = { std::max( aBbox->GetX(), aBbox->GetRight() ), std::max( aBbox->GetY(), aBbox->GetBottom() ) };
             VIEW_RTREE_BASE::Remove( mmin, mmax, aItem );
             return;
         }
@@ -85,8 +85,8 @@ public:
     template <class Visitor>
     void Query( const BOX2I& aBounds, Visitor& aVisitor ) const
     {
-        int   mmin[2] = { aBounds.GetX(), aBounds.GetY() };
+        int   mmin[2] = { std::min( aBounds.GetX(), aBounds.GetRight() ), std::min( aBounds.GetY(), aBounds.GetBottom() ) };
-        int   mmax[2] = { aBounds.GetRight(), aBounds.GetBottom() };
+        int   mmax[2] = { std::max( aBounds.GetX(), aBounds.GetRight() ), std::max( aBounds.GetY(), aBounds.GetBottom() ) };
 
         // We frequently use the maximum bounding box to recache all items
         // or for any item that overflows the integer width limits of BBOX2I

pcbnew/import_gfx/dialog_import_graphics.cpp

@@ -249,7 +249,7 @@ bool DIALOG_IMPORT_GRAPHICS::TransferDataFromWindow()
     if( cfg->m_Display.m_DisplayInvertYAxis )
         yscale *= -1.0;
 
-    VECTOR2D origin( m_xOrigin.GetIntValue() / xscale, m_yOrigin.GetIntValue() / yscale );
+    VECTOR2D origin( m_xOrigin.GetDoubleValue() / xscale, m_yOrigin.GetDoubleValue() / yscale );
 
     if( std::unique_ptr<GRAPHICS_IMPORT_PLUGIN> plugin = m_gfxImportMgr->GetPluginByExt( ext ) )
     {
@@ -277,8 +277,7 @@ bool DIALOG_IMPORT_GRAPHICS::TransferDataFromWindow()
         else
             m_importer->SetLayer( m_parent->GetActiveLayer() );
 
-        m_importer->SetImportOffsetMM( { pcbIUScale.IUTomm( origin.x ),
+        m_importer->SetImportOffsetMM( origin * pcbIUScale.IUTomm( 1 ) );
-                                         pcbIUScale.IUTomm( origin.y ) } );
 
         LOCALE_IO dummy;    // Ensure floats can be read.