From 034af9d217331a4898ed555d13285d1e507a6284 Mon Sep 17 00:00:00 2001 From: Damjan Prerad Date: Sun, 15 Mar 2026 10:41:47 +0100 Subject: [PATCH] Fix overlapping Edge.Cuts cutouts in DRC and 3D viewer Overlapping closed cutouts on Edge.Cuts were not reported in DRC and 3D view of these overlapped coutouts was not renderd correctly. This change adds explicit DRC detection for overlapping closed Edge.Cuts contours and footprint Edge.Cuts contours. Fixes https://gitlab.com/kicad/code/kicad/-/work_items/21829 --- pcbnew/convert_shape_list_to_polygon.cpp | 143 ++++++++++++++++-- .../drc/drc_test_provider_edge_clearance.cpp | 31 +++- pcbnew/drc/drc_test_provider_misc.cpp | 109 +++++++------ 3 files changed, 211 insertions(+), 72 deletions(-) diff --git a/pcbnew/convert_shape_list_to_polygon.cpp b/pcbnew/convert_shape_list_to_polygon.cpp index c0237c59a8..de05b070bc 100644 --- a/pcbnew/convert_shape_list_to_polygon.cpp +++ b/pcbnew/convert_shape_list_to_polygon.cpp @@ -433,28 +433,60 @@ static bool addOutlinesToPolygon( const std::vector& aContours return true; } -static void addHolesToPolygon( const std::vector& aContours, +static void addHolesToPolygon( const std::vector& aContours, const std::map>& aContourHierarchy, - const std::map& aContourToOutlineIdxMap, - SHAPE_POLY_SET& aPolygons ) + const std::map& aContourToOutlineIdxMap, SHAPE_POLY_SET& aPolygons, + bool aAllowUseArcsInPolygons, bool aHasMalformedOverlap ) { - for( const auto& [ contourIndex, parentIndexes ] : aContourHierarchy ) + if( aAllowUseArcsInPolygons || !aHasMalformedOverlap ) { - if( parentIndexes.size() % 2 == 1 ) + for( const auto& [contourIndex, parentIndexes] : aContourHierarchy ) { - // Odd number of parents; we're a hole in the parent which has one fewer parents - const SHAPE_LINE_CHAIN& hole = aContours[ contourIndex ]; - - for( int parentContourIdx : parentIndexes ) + if( parentIndexes.size() % 2 == 1 ) { - if( aContourHierarchy.at( parentContourIdx ).size() == parentIndexes.size() - 1 ) + // Odd number of parents; we're a hole in the parent which has one fewer parents + const SHAPE_LINE_CHAIN& hole = aContours[contourIndex]; + + for( int parentContourIdx : parentIndexes ) { - int outlineIdx = aContourToOutlineIdxMap.at( parentContourIdx ); - aPolygons.AddHole( hole, outlineIdx ); - break; + if( aContourHierarchy.at( parentContourIdx ).size() == parentIndexes.size() - 1 ) + { + int outlineIdx = aContourToOutlineIdxMap.at( parentContourIdx ); + aPolygons.AddHole( hole, outlineIdx ); + break; + } } } } + + return; + } + + // Malformed overlapping contours in the polygonized path. + SHAPE_POLY_SET cutoutCandidates; + SHAPE_POLY_SET islandCandidates; + + for( const auto& [contourIndex, parentIndexes] : aContourHierarchy ) + { + if( parentIndexes.empty() ) + continue; + + if( parentIndexes.size() % 2 == 1 ) + cutoutCandidates.AddOutline( aContours[contourIndex] ); + else + islandCandidates.AddOutline( aContours[contourIndex] ); + } + + if( cutoutCandidates.OutlineCount() ) + { + cutoutCandidates.Simplify(); + aPolygons.BooleanSubtract( cutoutCandidates ); + } + + if( islandCandidates.OutlineCount() ) + { + islandCandidates.Simplify(); + aPolygons.BooleanAdd( islandCandidates ); } } @@ -572,6 +604,24 @@ static PCB_SHAPE* findNext( PCB_SHAPE* aShape, const VECTOR2I& aPoint, const KDT return closest_graphic; } + +static bool hasOverlappingClosedContours( const std::vector& aContours ) +{ + for( size_t ii = 0; ii < aContours.size(); ++ii ) + { + for( size_t jj = ii + 1; jj < aContours.size(); ++jj ) + { + SHAPE_LINE_CHAIN::INTERSECTIONS intersections; + + if( aContours[ii].Intersect( aContours[jj], intersections, true ) != 0 ) + return true; + } + } + + return false; +} + + bool doConvertOutlineToPolygon( std::vector& aShapeList, SHAPE_POLY_SET& aPolygons, int aErrorMax, int aChainingEpsilon, bool aAllowDisjoint, OUTLINE_ERROR_HANDLER* aErrorHandler, bool aAllowUseArcsInPolygons, @@ -822,16 +872,19 @@ bool doConvertOutlineToPolygon( std::vector& aShapeList, SHAPE_POLY_ // Build contour hierarchy auto contourHierarchy = buildContourHierarchy( contours ); + bool hasMalformedOverlap = !aAllowUseArcsInPolygons && hasOverlappingClosedContours( contours ); + // Add outlines to polygon set std::map contourToOutlineIdxMap; - if( !addOutlinesToPolygon( contours, contourHierarchy, aPolygons, aAllowDisjoint, - aErrorHandler, fetchOwner, contourToOutlineIdxMap ) ) + if( !addOutlinesToPolygon( contours, contourHierarchy, aPolygons, aAllowDisjoint, aErrorHandler, fetchOwner, + contourToOutlineIdxMap ) ) { return false; } // Add holes to polygon set - addHolesToPolygon( contours, contourHierarchy, contourToOutlineIdxMap, aPolygons ); + addHolesToPolygon( contours, contourHierarchy, contourToOutlineIdxMap, aPolygons, aAllowUseArcsInPolygons, + hasMalformedOverlap ); // Check for self-intersections return checkSelfIntersections( aPolygons, aErrorHandler, fetchOwner ); @@ -966,6 +1019,63 @@ bool TestBoardOutlinesGraphicItems( BOARD* aBoard, int aMinDist, } } + std::vector> closedContours; + closedContours.reserve( shapeList.size() ); + + for( PCB_SHAPE* shape : shapeList ) + { + if( shape->GetShape() != SHAPE_T::POLY && shape->GetShape() != SHAPE_T::CIRCLE + && shape->GetShape() != SHAPE_T::RECTANGLE ) + { + continue; + } + + SHAPE_LINE_CHAIN contour; + std::map, PCB_SHAPE*> shapeOwners; + + processClosedShape( shape, contour, shapeOwners, shape->GetMaxError(), true ); + closedContours.emplace_back( shape, std::move( contour ) ); + } + + for( size_t ii = 0; ii < closedContours.size(); ++ii ) + { + const SHAPE_LINE_CHAIN& contourA = closedContours[ii].second; + + for( size_t jj = ii + 1; jj < closedContours.size(); ++jj ) + { + const SHAPE_LINE_CHAIN& contourB = closedContours[jj].second; + SHAPE_LINE_CHAIN::INTERSECTIONS intersections; + + // Ignore touching-only cases; report only real overlap/crossing. + if( contourA.Intersect( contourB, intersections, true ) == 0 ) + continue; + + success = false; + + if( aErrorHandler ) + { + PCB_SHAPE* shapeA = closedContours[ii].first; + PCB_SHAPE* shapeB = closedContours[jj].first; + + VECTOR2I midpoint = intersections.front().p; + std::shared_ptr effectiveShapeA = shapeA->GetEffectiveShape(); + std::shared_ptr effectiveShapeB = shapeB->GetEffectiveShape(); + + if( effectiveShapeA && effectiveShapeB ) + { + BOX2I bboxA = effectiveShapeA->BBox(); + BOX2I bboxB = effectiveShapeB->BBox(); + BOX2I overlapBox = bboxA.Intersect( bboxB ); + + if( overlapBox.GetWidth() > 0 && overlapBox.GetHeight() > 0 ) + midpoint = overlapBox.Centre(); + } + + ( *aErrorHandler )( _( "(self-intersecting)" ), shapeA, shapeB, midpoint ); + } + } + } + return success; } @@ -1101,6 +1211,7 @@ bool BuildBoardPolygonOutlines( BOARD* aBoard, SHAPE_POLY_SET& aOutlines, int aE } else { + fpHoles.Simplify(); aOutlines.BooleanSubtract( fpHoles ); } diff --git a/pcbnew/drc/drc_test_provider_edge_clearance.cpp b/pcbnew/drc/drc_test_provider_edge_clearance.cpp index ef05aae5a8..f5888a62da 100644 --- a/pcbnew/drc/drc_test_provider_edge_clearance.cpp +++ b/pcbnew/drc/drc_test_provider_edge_clearance.cpp @@ -282,15 +282,30 @@ bool DRC_TEST_PROVIDER_EDGE_CLEARANCE::Run() // A single rectangle for the board would defeat the RTree, so convert to edges if( shape->GetCornerRadius() > 0 ) { - for( SHAPE* seg : shape->MakeEffectiveShapes( true ) ) + for( SHAPE* subshape : shape->MakeEffectiveShapes( true ) ) { - wxCHECK2( dynamic_cast( seg ), continue ); - - edges.emplace_back( static_cast( shape->Clone() ) ); - edges.back()->SetShape( SHAPE_T::SEGMENT ); - edges.back()->SetStart( seg->GetStart() ); - edges.back()->SetEnd( seg->GetEnd() ); - edges.back()->SetStroke( stroke ); + if( SHAPE_SEGMENT* segment = dynamic_cast( subshape ) ) + { + edges.emplace_back( static_cast( shape->Clone() ) ); + edges.back()->SetShape( SHAPE_T::SEGMENT ); + edges.back()->SetStart( segment->GetStart() ); + edges.back()->SetEnd( segment->GetEnd() ); + edges.back()->SetStroke( stroke ); + } + else if( SHAPE_ARC* arc = dynamic_cast( subshape ) ) + { + edges.emplace_back( static_cast( shape->Clone() ) ); + edges.back()->SetShape( SHAPE_T::ARC ); + edges.back()->SetArcGeometry( arc->GetP0(), arc->GetArcMid(), arc->GetP1() ); + edges.back()->SetStroke( stroke ); + } + else + { + wxFAIL_MSG( + wxString::Format( "Unexpected effective shape type %d for rounded rectangle", + (int) subshape->Type() ) ); + continue; + } } } else diff --git a/pcbnew/drc/drc_test_provider_misc.cpp b/pcbnew/drc/drc_test_provider_misc.cpp index 2149ef37e1..20b4ff7f04 100644 --- a/pcbnew/drc/drc_test_provider_misc.cpp +++ b/pcbnew/drc/drc_test_provider_misc.cpp @@ -143,66 +143,79 @@ void DRC_TEST_PROVIDER_MISC::testOutline() OUTLINE_ERROR_HANDLER errorHandler = [&]( const wxString& msg, BOARD_ITEM* itemA, BOARD_ITEM* itemB, const VECTOR2I& pt ) + { + errorHandled = true; + + if( m_drcEngine->IsErrorLimitExceeded( DRCE_INVALID_OUTLINE ) ) + return; + + if( !itemA ) + std::swap( itemA, itemB ); + + VECTOR2I markerPos = pt; + int gap = 0; + PCB_SHAPE* shapeA = nullptr; + PCB_SHAPE* shapeB = nullptr; + bool usedGap = false; + + if( itemA && itemB && itemA->Type() == PCB_SHAPE_T && itemB->Type() == PCB_SHAPE_T ) + { + shapeA = static_cast( itemA ); + shapeB = static_cast( itemB ); + } + else + { + findClosestOutlineGap( m_board, shapeA, shapeB, markerPos, gap ); + itemA = shapeA; + itemB = shapeB; + usedGap = shapeA && shapeB; + } + + if( shapeA && shapeB ) + { + std::shared_ptr effectiveShapeA = shapeA->GetEffectiveShape(); + std::shared_ptr effectiveShapeB = shapeB->GetEffectiveShape(); + + if( effectiveShapeA && effectiveShapeB ) { - errorHandled = true; + BOX2I bboxA = effectiveShapeA->BBox(); + BOX2I bboxB = effectiveShapeB->BBox(); + BOX2I overlap = bboxA.Intersect( bboxB ); - if( m_drcEngine->IsErrorLimitExceeded( DRCE_INVALID_OUTLINE ) ) - return; - - if( !itemA ) // If we only have a single item, make sure it's A - std::swap( itemA, itemB ); - - VECTOR2I markerPos = pt; - int gap = 0; - PCB_SHAPE* shapeA = nullptr; - PCB_SHAPE* shapeB = nullptr; - - if( itemA && itemB && itemA->Type() == PCB_SHAPE_T && itemB->Type() == PCB_SHAPE_T ) + if( overlap.GetWidth() > 0 && overlap.GetHeight() > 0 ) { - shapeA = static_cast( itemA ); - shapeB = static_cast( itemB ); + markerPos = overlap.Centre(); + usedGap = false; } else { - findClosestOutlineGap( m_board, shapeA, shapeB, markerPos, gap ); + VECTOR2I ptA, ptB; - itemA = shapeA; - itemB = shapeB; + if( effectiveShapeA->NearestPoints( effectiveShapeB.get(), ptA, ptB ) ) + { + gap = ( ptA - ptB ).EuclideanNorm(); + markerPos = ( ptA + ptB ) / 2; + usedGap = true; + } } + } + } - if( shapeA && shapeB ) - { - VECTOR2I pts0[2] = { shapeA->GetStart(), shapeA->GetEnd() }; - VECTOR2I pts1[2] = { shapeB->GetStart(), shapeB->GetEnd() }; + std::shared_ptr drcItem = DRC_ITEM::Create( DRCE_INVALID_OUTLINE ); - SEG::ecoord d[4]; - d[0] = ( pts0[0] - pts1[0] ).SquaredEuclideanNorm(); - d[1] = ( pts0[0] - pts1[1] ).SquaredEuclideanNorm(); - d[2] = ( pts0[1] - pts1[0] ).SquaredEuclideanNorm(); - d[3] = ( pts0[1] - pts1[1] ).SquaredEuclideanNorm(); + if( itemA && itemB && usedGap ) + { + drcItem->SetErrorDetail( wxString::Format( _( "%s (gap %s)" ), msg, MessageTextFromValue( gap ) ) ); + } + else + { + drcItem->SetErrorDetail( msg ); + } - int idx = std::min_element( d, d + 4 ) - d; - gap = std::sqrt( d[idx] ); - markerPos = ( pts0[idx / 2] + pts1[idx % 2] ) / 2; - } + drcItem->SetItems( itemA, itemB ); - std::shared_ptr drcItem = DRC_ITEM::Create( DRCE_INVALID_OUTLINE ); - - if( itemA && itemB ) - { - drcItem->SetErrorDetail( wxString::Format( _( "%s (gap %s)" ), - msg, - MessageTextFromValue( gap ) ) ); - } - else - { - drcItem->SetErrorDetail( msg ); - } - - drcItem->SetItems( itemA, itemB ); - - reportViolation( drcItem, markerPos, Edge_Cuts ); - }; + reportViolation( drcItem, markerPos, Edge_Cuts ); + }; // Test for very small graphic items (a few nm size) that can create issues // when trying to build the board outlines, and they are not easy to locate on screen.