← Back to team overview

kicad-developers team mailing list archive

Issue with polygon shape start/end points


Hello devs,

There was a bug report
my bom plugin about incorrect behavior related to polygons that were
moved/duplicated. Upon inspection it turns out that KiCad API returns
inconsistent values when move occurs. Move operation affects both start
point and the outline of the polygon but only until pcbnew is restarted.
Upon reload API returns 0,0 for start point (which makes sense since
start/end points are not saved in the file) and only outline is moved.

My question is whether this can be considered as a bug. I think it should
be since pcbnew should not change API call results just because it was
restarted. If we don't save start/end points then we should not modify them
internally either.
My plugin generally treats start point as an offset for canvas and while I
can add a workaround just for polygons it might be cleaner to fix this in
KiCad. Let me know if you disagree.

I've attached the patch to avoid modifying polygon start/end points on
move. Btw that would make the operation consistent with how rotate works,
it only affects outline but not DRAWSEGMENT's angle.
This affects both 5.1 and master branches.

From c23b08233653ae5f3466eb91db8c5bcb1cf88e31 Mon Sep 17 00:00:00 2001
From: qu1ck <anlutsenko@xxxxxxxxx>
Date: Mon, 13 Jan 2020 17:04:35 -0800
Subject: [PATCH] Fix polygon shape move

Move vector should not affect start/end points as they are assumed to
always be 0,0. Polygon is defined by outline only.
 pcbnew/class_drawsegment.cpp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/pcbnew/class_drawsegment.cpp b/pcbnew/class_drawsegment.cpp
index 5f708f33a..6d5e43413 100644
--- a/pcbnew/class_drawsegment.cpp
+++ b/pcbnew/class_drawsegment.cpp
@@ -95,8 +95,13 @@ double DRAWSEGMENT::GetLength() const
 void DRAWSEGMENT::Move( const wxPoint& aMoveVector )
-    m_Start += aMoveVector;
-    m_End   += aMoveVector;
+    // Move vector should not affect start/end for polygon since it will
+    // be applied directly to polygon outline.
+    if ( m_Shape != S_POLYGON )
+    {
+        m_Start += aMoveVector;
+        m_End   += aMoveVector;
+    }
     switch ( m_Shape )

Follow ups