From 16f9e697ab8079224dbf0249782747bc160cca1c 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 (cherry picked from commit a3c74051c27cdbebbeaf7f692452e1f62690a8bf) --- 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 | 36 +++++++++++++---------------- pcbnew/tools/point_editor.h | 5 ++-- pcbnew/tools/zone_create_helper.cpp | 2 +- 6 files changed, 56 insertions(+), 32 deletions(-) diff --git a/common/geometry/shape_poly_set.cpp b/common/geometry/shape_poly_set.cpp index 63c6f2341a..4a3d31cb3c 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 27e29ca2c8..bb25cb1d47 100644 --- a/include/geometry/shape_poly_set.h +++ b/include/geometry/shape_poly_set.h @@ -538,14 +538,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(); } @@ -762,12 +762,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() { @@ -786,6 +809,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 0b7b30dce9..4816b88f8e 100644 --- a/pcbnew/plot_brditems_plotter.cpp +++ b/pcbnew/plot_brditems_plotter.cpp @@ -780,7 +780,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 458df2aaf7..0ad3e0148b 100644 --- a/pcbnew/tools/point_editor.cpp +++ b/pcbnew/tools/point_editor.cpp @@ -642,26 +642,26 @@ 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->Popup( getEditFrame() ); - m_statusPopup->Expire( 1500 ); + if( valid ) + { + m_statusPopup->Hide(); + } + else + { + wxPoint p = wxGetMousePosition() + wxPoint( 20, 20 ); + m_statusPopup->Move( p ); + m_statusPopup->Popup( getEditFrame() ); + m_statusPopup->Expire( 1500 ); + } } - if( aOriginal ) - aModified = *aOriginal; - - return false; + return valid; } @@ -1115,14 +1115,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 { @@ -1143,10 +1142,7 @@ int POINT_EDITOR::removeCorner( const TOOL_EVENT& aEvent ) setEditedPoint( nullptr ); updatePoints(); - if( valid ) - commit.Push( _( "Remove a zone/polygon corner" ) ); - else - commit.Revert(); + commit.Push( _( "Remove a zone/polygon corner" ) ); // Refresh zone hatching if( item->Type() == PCB_ZONE_AREA_T) diff --git a/pcbnew/tools/point_editor.h b/pcbnew/tools/point_editor.h index 6ebcb17872..1e21e6d4a5 100644 --- a/pcbnew/tools/point_editor.h +++ b/pcbnew/tools/point_editor.h @@ -92,12 +92,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 189f7bd953..942502cd73 100644 --- a/pcbnew/tools/zone_create_helper.cpp +++ b/pcbnew/tools/zone_create_helper.cpp @@ -207,7 +207,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 ) :