← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Collision-based select in library editor

 

Thank you for testing, JP.  I've corrected the issue.  I had missed the
inverted Y-axis in lib editor.  The attached patch functions correctly on
all circles, in all quadrants.

Best-
Seth

On Tue, Oct 10, 2017 at 12:41 AM, jp charras <jp.charras@xxxxxxxxxx> wrote:

> Le 06/10/2017 à 23:56, Seth Hillbrand a écrit :
> > ​Crosses off a couple FIXMEs.  Previous code checked if text and circle
> centers were inside
> > selection rectangle.  This patch adds collision detection to full
> objects.  Makes selecting items in
> > library editor a bit easier.
> >
> > -Seth
> >
>
> Hi Seth,
>
> I tested your patch. It works fine for texts, but not for the circle.
>
> The issue is related to the vertical axis: any rectangle on the same
> vertical alignment as the
> circle selects the circle.
>
> Thanks.
>
> --
> Jean-Pierre CHARRAS
>
From 63c0cbc5aefe5070307452bf7cf419eff66c2110 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Fri, 6 Oct 2017 13:58:27 -0700
Subject: [PATCH] Eeschema: Add collision-based selection code to circles and
 text

---
 eeschema/lib_circle.cpp | 16 +++++++++++-----
 eeschema/lib_field.cpp  | 15 ++++++++++-----
 eeschema/lib_text.cpp   | 15 ++++++++++-----
 3 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/eeschema/lib_circle.cpp b/eeschema/lib_circle.cpp
index 78a915325..29be04d4a 100644
--- a/eeschema/lib_circle.cpp
+++ b/eeschema/lib_circle.cpp
@@ -146,11 +146,17 @@ void LIB_CIRCLE::SetOffset( const wxPoint& aOffset )
 
 bool LIB_CIRCLE::Inside( EDA_RECT& aRect ) const
 {
-    /*
-     * FIXME: This fails to take into account the radius around the center
-     *        point.
-     */
-    return aRect.Contains( m_Pos.x, -m_Pos.y );
+    wxPoint delta;
+
+    // Y coordinates for LIB_ITEMS are bottom to top, so we must invert the Y position
+    aRect.RevertYAxis();
+    delta = m_Pos - aRect.ClosestPointTo( m_Pos );
+    aRect.RevertYAxis();
+
+    delta.x = abs( delta.x );
+    delta.y = abs( delta.y );
+
+    return ( delta.x * delta.x + delta.y * delta.y <= m_Radius * m_Radius );
 }
 
 
diff --git a/eeschema/lib_field.cpp b/eeschema/lib_field.cpp
index cd7e524b6..0cea51fbb 100644
--- a/eeschema/lib_field.cpp
+++ b/eeschema/lib_field.cpp
@@ -447,11 +447,16 @@ void LIB_FIELD::SetOffset( const wxPoint& aOffset )
 
 bool LIB_FIELD::Inside( EDA_RECT& rect ) const
 {
-    /*
-     * FIXME: This fails to take into account the size and/or orientation of
-     *        the text.
-     */
-    return rect.Contains( GetTextPos().x, -GetTextPos().y );
+    EDA_RECT bb = GetBoundingBox();
+    bb.Normalize();
+    rect.Normalize();
+    wxPoint p1 = bb.GetPosition();
+    wxPoint p2 = p1 + bb.GetSize();
+    wxPoint p3 = rect.GetPosition();
+    wxPoint p4 = p3 + rect.GetSize();
+
+    //This works because the rotation can only be a multiple of 90 deg
+    return p2.y >= p3.y && p1.y <= p4.y && p2.x >= p3.x && p1.x <= p4.x;
 }
 
 
diff --git a/eeschema/lib_text.cpp b/eeschema/lib_text.cpp
index 428a28d0f..be95249ad 100644
--- a/eeschema/lib_text.cpp
+++ b/eeschema/lib_text.cpp
@@ -268,11 +268,16 @@ void LIB_TEXT::SetOffset( const wxPoint& aOffset )
 
 bool LIB_TEXT::Inside( EDA_RECT& rect ) const
 {
-    /*
-     * FIXME: This should calculate the text size and justification and
-     *        use rectangle intersect.
-     */
-    return rect.Contains( GetTextPos().x, -GetTextPos().y );
+    EDA_RECT bb = GetBoundingBox();
+    bb.Normalize();
+    rect.Normalize();
+    wxPoint p1 = bb.GetPosition();
+    wxPoint p2 = p1 + bb.GetSize();
+    wxPoint p3 = rect.GetPosition();
+    wxPoint p4 = p3 + rect.GetSize();
+
+    //This works because the rotation can only be a multiple of 90 deg
+    return p2.y >= p3.y && p1.y <= p4.y && p2.x >= p3.x && p1.x <= p4.x;
 }
 
 
-- 
2.11.0


Follow ups

References