← Back to team overview

kicad-developers team mailing list archive

[PATCH] Fix aperture macro hit testing (Fixes lp:1730249)

 

Hi all,

This patch fixes a bug reported by Nick in GerbView GAL, where aperture
macro hit testing wasn't quite right.

https://bugs.launchpad.net/kicad/+bug/1730249

-Jon
From 2e20fe8d6130d9531ce99bb432e4f9cfce61e589 Mon Sep 17 00:00:00 2001
From: Jon Evans <jon@xxxxxxxxxxxxx>
Date: Sat, 11 Nov 2017 17:20:19 -0500
Subject: [PATCH] Fix aperture macro hit testing (Fixes lp:1730249)

Also update various things in class_aperture_macro.cpp to take a const
parent argument so that it is callable from within GERBER_DRAW_ITEM
---
 gerbview/class_aperture_macro.cpp   | 10 +++++-----
 gerbview/class_aperture_macro.h     | 12 +++++++-----
 gerbview/class_gerber_draw_item.cpp |  6 +++---
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/gerbview/class_aperture_macro.cpp b/gerbview/class_aperture_macro.cpp
index 07417f97f..ba15d62af 100644
--- a/gerbview/class_aperture_macro.cpp
+++ b/gerbview/class_aperture_macro.cpp
@@ -59,7 +59,7 @@ static wxPoint mapPt( double x, double y, bool isMetric )
 }
 
 
-bool AM_PRIMITIVE::IsAMPrimitiveExposureOn(GERBER_DRAW_ITEM* aParent) const
+bool AM_PRIMITIVE::IsAMPrimitiveExposureOn( const GERBER_DRAW_ITEM* aParent ) const
 {
     /*
      * Some but not all primitives use the first parameter as an exposure control.
@@ -94,7 +94,7 @@ bool AM_PRIMITIVE::IsAMPrimitiveExposureOn(GERBER_DRAW_ITEM* aParent) const
 
 const int seg_per_circle = 64;   // Number of segments to approximate a circle
 
-void AM_PRIMITIVE::DrawBasicShape( GERBER_DRAW_ITEM* aParent,
+void AM_PRIMITIVE::DrawBasicShape( const GERBER_DRAW_ITEM* aParent,
                                    SHAPE_POLY_SET& aShapeBuffer,
                                    wxPoint aShapePos )
 {
@@ -448,7 +448,7 @@ void AM_PRIMITIVE::DrawBasicShape( GERBER_DRAW_ITEM* aParent,
  * because circles are very easy to draw (no rotation problem) so convert them in polygons,
  * and draw them as polygons is not a good idea.
  */
-void AM_PRIMITIVE::ConvertShapeToPolygon( GERBER_DRAW_ITEM*     aParent,
+void AM_PRIMITIVE::ConvertShapeToPolygon( const GERBER_DRAW_ITEM* aParent,
                                           std::vector<wxPoint>& aBuffer )
 {
     D_CODE* tool = aParent->GetDcodeDescr();
@@ -668,7 +668,7 @@ void AM_PRIMITIVE::ConvertShapeToPolygon( GERBER_DRAW_ITEM*     aParent,
  * @param aParent = the parent GERBER_DRAW_ITEM which is actually drawn
  * @return a dimension, or -1 if no dim to calculate
   */
-int AM_PRIMITIVE::GetShapeDim( GERBER_DRAW_ITEM* aParent )
+int AM_PRIMITIVE::GetShapeDim( const GERBER_DRAW_ITEM* aParent )
 {
     int dim = -1;
     D_CODE* tool = aParent->GetDcodeDescr();
@@ -773,7 +773,7 @@ int AM_PRIMITIVE::GetShapeDim( GERBER_DRAW_ITEM* aParent )
 }
 
 
-SHAPE_POLY_SET* APERTURE_MACRO::GetApertureMacroShape( GERBER_DRAW_ITEM* aParent,
+SHAPE_POLY_SET* APERTURE_MACRO::GetApertureMacroShape( const GERBER_DRAW_ITEM* aParent,
                                                        wxPoint aShapePos )
 {
     SHAPE_POLY_SET holeBuffer;
diff --git a/gerbview/class_aperture_macro.h b/gerbview/class_aperture_macro.h
index c6579828e..0274762d0 100644
--- a/gerbview/class_aperture_macro.h
+++ b/gerbview/class_aperture_macro.h
@@ -114,7 +114,7 @@ public: AM_PRIMITIVE( bool aGerbMetric, AM_PRIMITIVE_ID aId = AMP_UNKNOWN )
      * In a aperture macro shape, a basic primitive with exposure off is a hole in the shape
      * it is NOT a negative shape
      */
-    bool  IsAMPrimitiveExposureOn( GERBER_DRAW_ITEM* aParent ) const;
+    bool  IsAMPrimitiveExposureOn( const GERBER_DRAW_ITEM* aParent ) const;
 
     /* Draw functions: */
 
@@ -128,7 +128,7 @@ public: AM_PRIMITIVE( bool aGerbMetric, AM_PRIMITIVE_ID aId = AMP_UNKNOWN )
      * @param aParent = the parent GERBER_DRAW_ITEM which is actually drawn
      * @return a dimension, or -1 if no dim to calculate
      */
-    int  GetShapeDim( GERBER_DRAW_ITEM* aParent );
+    int  GetShapeDim( const GERBER_DRAW_ITEM* aParent );
 
     /**
      * Function drawBasicShape
@@ -137,7 +137,8 @@ public: AM_PRIMITIVE( bool aGerbMetric, AM_PRIMITIVE_ID aId = AMP_UNKNOWN )
      * @param aShapeBuffer = a SHAPE_POLY_SET to put the shape converted to a polygon
      * @param aShapePos = the actual shape position
      */
-    void DrawBasicShape( GERBER_DRAW_ITEM* aParent, SHAPE_POLY_SET& aShapeBuffer,
+    void DrawBasicShape( const GERBER_DRAW_ITEM* aParent,
+                         SHAPE_POLY_SET& aShapeBuffer,
                          wxPoint aShapePos );
 private:
 
@@ -148,7 +149,8 @@ private:
      * Useful when a shape is not a graphic primitive (shape with hole,
      * rotated shape ... ) and cannot be easily drawn.
      */
-    void ConvertShapeToPolygon( GERBER_DRAW_ITEM* aParent, std::vector<wxPoint>& aBuffer );
+    void ConvertShapeToPolygon( const GERBER_DRAW_ITEM* aParent,
+                                std::vector<wxPoint>& aBuffer );
 };
 
 
@@ -195,7 +197,7 @@ struct APERTURE_MACRO
      * @param aParent = the parent GERBER_DRAW_ITEM which is actually drawn
      * @return The shape of the item
      */
-    SHAPE_POLY_SET* GetApertureMacroShape( GERBER_DRAW_ITEM* aParent, wxPoint aShapePos );
+    SHAPE_POLY_SET* GetApertureMacroShape( const GERBER_DRAW_ITEM* aParent, wxPoint aShapePos );
 
    /**
      * Function DrawApertureMacroShape
diff --git a/gerbview/class_gerber_draw_item.cpp b/gerbview/class_gerber_draw_item.cpp
index 4cc29f20b..7ffa108c0 100644
--- a/gerbview/class_gerber_draw_item.cpp
+++ b/gerbview/class_gerber_draw_item.cpp
@@ -703,10 +703,10 @@ bool GERBER_DRAW_ITEM::HitTest( const wxPoint& aRefPos ) const
 
     case GBR_SPOT_MACRO:
         // Aperture macro polygons are already in absolute coordinates
-        poly = GetDcodeDescr()->GetMacro()->m_shape;
-        for( int i = 0; i < poly.OutlineCount(); ++i )
+        auto p = GetDcodeDescr()->GetMacro()->GetApertureMacroShape( this, m_Start );
+        for( int i = 0; i < p->OutlineCount(); ++i )
         {
-            if( poly.Contains( VECTOR2I( aRefPos ), i ) )
+            if( p->Contains( VECTOR2I( aRefPos ), i ) )
                 return true;
         }
         return false;
-- 
2.14.1


Follow ups