From a3c74051c27cdbebbeaf7f692452e1f62690a8bf Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sun, 14 Jul 2019 13:46:59 +0100 Subject: [PATCH] Allow edits to self-intersecting polygons. 1) Intermediate states might be self-intersecting, and we shouldn't be policing our users on what order to do things in 2) The polygon might already be self-intersecting, at which point we're preventing the user from fixing it. Also includes better const management for SHAPE_POLY_SET API. Fixes: lp:1833831 * https://bugs.launchpad.net/kicad/+bug/1833831 --- common/geometry/shape_poly_set.cpp | 10 +++--- include/geometry/shape_poly_set.h | 33 +++++++++++++++++-- pcbnew/plot_brditems_plotter.cpp | 2 +- pcbnew/tools/point_editor.cpp | 49 +++++++++++------------------ pcbnew/tools/point_editor.h | 5 ++- pcbnew/tools/zone_create_helper.cpp | 2 +- 6 files changed, 59 insertions(+), 42 deletions(-) diff --git a/common/geometry/shape_poly_set.cpp b/common/geometry/shape_poly_set.cpp index e5c951629c..1832ac8992 100644 --- a/common/geometry/shape_poly_set.cpp +++ b/common/geometry/shape_poly_set.cpp @@ -396,12 +396,12 @@ bool SHAPE_POLY_SET::GetNeighbourIndexes( int aGlobalIndex, int* aPrevious, int* } -bool SHAPE_POLY_SET::IsPolygonSelfIntersecting( int aPolygonIndex ) +bool SHAPE_POLY_SET::IsPolygonSelfIntersecting( int aPolygonIndex ) const { - SEGMENT_ITERATOR iterator = IterateSegmentsWithHoles( aPolygonIndex ); - SEGMENT_ITERATOR innerIterator; + CONST_SEGMENT_ITERATOR iterator = CIterateSegmentsWithHoles( aPolygonIndex ); + CONST_SEGMENT_ITERATOR innerIterator; - for( iterator = IterateSegmentsWithHoles( aPolygonIndex ); iterator; iterator++ ) + for( iterator = CIterateSegmentsWithHoles( aPolygonIndex ); iterator; iterator++ ) { SEG firstSegment = *iterator; @@ -423,7 +423,7 @@ bool SHAPE_POLY_SET::IsPolygonSelfIntersecting( int aPolygonIndex ) } -bool SHAPE_POLY_SET::IsSelfIntersecting() +bool SHAPE_POLY_SET::IsSelfIntersecting() const { for( unsigned int polygon = 0; polygon < m_polys.size(); polygon++ ) { diff --git a/include/geometry/shape_poly_set.h b/include/geometry/shape_poly_set.h index 9b60997f43..58e4de1bfb 100644 --- a/include/geometry/shape_poly_set.h +++ b/include/geometry/shape_poly_set.h @@ -542,14 +542,14 @@ class SHAPE_POLY_SET : public SHAPE * @return bool - true if the aPolygonIndex-th polygon is self intersecting, false * otherwise. */ - bool IsPolygonSelfIntersecting( int aPolygonIndex ); + bool IsPolygonSelfIntersecting( int aPolygonIndex ) const; /** * Function IsSelfIntersecting * Checks whether any of the polygons in the set is self intersecting. * @return bool - true if any of the polygons is self intersecting, false otherwise. */ - bool IsSelfIntersecting(); + bool IsSelfIntersecting() const; ///> Returns the number of triangulated polygons unsigned int TriangulatedPolyCount() const { return m_triangulatedPolys.size(); } @@ -766,12 +766,35 @@ class SHAPE_POLY_SET : public SHAPE return iter; } + ///> Returns an iterator object, for iterating between aFirst and aLast outline, with or + /// without holes (default: without) + CONST_SEGMENT_ITERATOR CIterateSegments( int aFirst, int aLast, + bool aIterateHoles = false ) const + { + CONST_SEGMENT_ITERATOR iter; + + iter.m_poly = const_cast( this ); + iter.m_currentPolygon = aFirst; + iter.m_lastPolygon = aLast < 0 ? OutlineCount() - 1 : aLast; + iter.m_currentContour = 0; + iter.m_currentSegment = 0; + iter.m_iterateHoles = aIterateHoles; + + return iter; + } + ///> Returns an iterator object, for iterating aPolygonIdx-th polygon edges SEGMENT_ITERATOR IterateSegments( int aPolygonIdx ) { return IterateSegments( aPolygonIdx, aPolygonIdx ); } + ///> Returns an iterator object, for iterating aPolygonIdx-th polygon edges + CONST_SEGMENT_ITERATOR CIterateSegments( int aPolygonIdx ) const + { + return CIterateSegments( aPolygonIdx, aPolygonIdx ); + } + ///> Returns an iterator object, for all outlines in the set (no holes) SEGMENT_ITERATOR IterateSegments() { @@ -790,6 +813,12 @@ class SHAPE_POLY_SET : public SHAPE return IterateSegments( aOutline, aOutline, true ); } + ///> Returns an iterator object, for the aOutline-th outline in the set (with holes) + CONST_SEGMENT_ITERATOR CIterateSegmentsWithHoles( int aOutline ) const + { + return CIterateSegments( aOutline, aOutline, true ); + } + /** operations on polygons use a aFastMode param * if aFastMode is PM_FAST (true) the result can be a weak polygon * if aFastMode is PM_STRICTLY_SIMPLE (false) (default) the result is (theorically) a strictly diff --git a/pcbnew/plot_brditems_plotter.cpp b/pcbnew/plot_brditems_plotter.cpp index 70d0b7ec42..6c3c84df13 100644 --- a/pcbnew/plot_brditems_plotter.cpp +++ b/pcbnew/plot_brditems_plotter.cpp @@ -749,7 +749,7 @@ void BRDITEMS_PLOTTER::PlotDrawSegment( DRAWSEGMENT* aSeg ) { if( !aSeg->IsPolygonFilled() ) { - for( auto it = aSeg->GetPolyShape().IterateSegments( 0 ); it; it++ ) + for( auto it = aSeg->GetPolyShape().CIterateSegments( 0 ); it; it++ ) { auto seg = it.Get(); m_plotter->ThickSegment( wxPoint( seg.A ), wxPoint( seg.B ), diff --git a/pcbnew/tools/point_editor.cpp b/pcbnew/tools/point_editor.cpp index 237930843d..c7a836b510 100644 --- a/pcbnew/tools/point_editor.cpp +++ b/pcbnew/tools/point_editor.cpp @@ -637,25 +637,25 @@ void POINT_EDITOR::finishItem() } -bool POINT_EDITOR::validatePolygon( SHAPE_POLY_SET& aModified, const SHAPE_POLY_SET* aOriginal ) const +bool POINT_EDITOR::validatePolygon( SHAPE_POLY_SET& aPoly ) const { - if( !aModified.IsSelfIntersecting() ) - { - m_statusPopup->Hide(); - return true; - } + bool valid = !aPoly.IsSelfIntersecting(); if( m_statusPopup ) { - wxPoint p = wxGetMousePosition() + wxPoint( 20, 20 ); - m_statusPopup->Move( p ); - m_statusPopup->PopupFor( 1500 ); + if( valid ) + { + m_statusPopup->Hide(); + } + else + { + wxPoint p = wxGetMousePosition() + wxPoint( 20, 20 ); + m_statusPopup->Move( p ); + m_statusPopup->PopupFor( 1500 ); + } } - if( aOriginal ) - aModified = *aOriginal; - - return false; + return valid; } @@ -1108,14 +1108,13 @@ int POINT_EDITOR::removeCorner( const TOOL_EVENT& aEvent ) { const auto& vertexIdx = vertex.second; auto& outline = polygon->Polygon( vertexIdx.m_polygon )[vertexIdx.m_contour]; - bool valid = true; if( outline.PointCount() > 3 ) { // the usual case: remove just the corner when there are >3 vertices commit.Modify( item ); polygon->RemoveVertex( vertexIdx ); - valid = validatePolygon( *polygon ); + validatePolygon( *polygon ); } else { @@ -1135,23 +1134,13 @@ int POINT_EDITOR::removeCorner( const TOOL_EVENT& aEvent ) setEditedPoint( nullptr ); - if( valid ) - { - commit.Push( _( "Remove a zone/polygon corner" ) ); + commit.Push( _( "Remove a zone/polygon corner" ) ); - // Refresh zone hatching - if( item->Type() == PCB_ZONE_AREA_T) - static_cast( item )->Hatch(); + // Refresh zone hatching + if( item->Type() == PCB_ZONE_AREA_T) + static_cast( item )->Hatch(); - updatePoints(); - } - else - { - m_toolMgr->RunAction( PCB_ACTIONS::selectionClear, true ); - getView()->Remove( m_editPoints.get() ); - commit.Revert(); - m_editPoints.reset(); - } + updatePoints(); } return 0; diff --git a/pcbnew/tools/point_editor.h b/pcbnew/tools/point_editor.h index 245f84087a..153f4697e4 100644 --- a/pcbnew/tools/point_editor.h +++ b/pcbnew/tools/point_editor.h @@ -98,12 +98,11 @@ private: void finishItem(); /** - * Validates a polygon and restores it to its original version if available. + * Validates a polygon and displays a popup warning if invalid. * @param aModified is the polygon to be checked. - * @param aOriginal is the original copy that will be used to restore its state. * @return True if polygon is valid. */ - bool validatePolygon( SHAPE_POLY_SET& aModified, const SHAPE_POLY_SET* aOriginal = nullptr ) const; + bool validatePolygon( SHAPE_POLY_SET& aModified ) const; ///> Updates edit points with item's points. void updatePoints(); diff --git a/pcbnew/tools/zone_create_helper.cpp b/pcbnew/tools/zone_create_helper.cpp index 550f0d7c3d..99a6e32608 100644 --- a/pcbnew/tools/zone_create_helper.cpp +++ b/pcbnew/tools/zone_create_helper.cpp @@ -198,7 +198,7 @@ void ZONE_CREATE_HELPER::commitZone( std::unique_ptr aZone ) { auto outline = aZone->Outline(); - for( auto seg = outline->IterateSegments( 0 ); seg; seg++ ) + for( auto seg = outline->CIterateSegments( 0 ); seg; seg++ ) { auto new_seg = m_tool.m_editModules ? new EDGE_MODULE( (MODULE *) parent ) : new DRAWSEGMENT();