← Back to team overview

kicad-developers team mailing list archive

Re: [FEATURE] Partial selection in pcbnew

 

Two more patches for this set (attached)

0017 - Slight fix for arc segment hit test (line width was not accounted
for)
0018 - SELECTION_AREA color now indicates selection mode as discussed above:

a) Normal selection = BLUE
b) Addition selection = GREEN (Shift modifier)
c) Subtraction selection = RED (Control modifier)

Additionally the line color indicates whether it is window selection (left
to right) or crossing selection (right to left). This is in lieu of making
the line dashed which does not seem to be possible unless that is added to
GAL_CAIRO and GAL_OPENGL.

Regards,
Oliver

On Tue, May 9, 2017 at 3:22 PM, Andrey Kuznetsov <kandrey89@xxxxxxxxx>
wrote:

> What's CTRL taken by?
> I thought CTRL would be used to toggle grid snapping?
>
> On Mon, May 8, 2017 at 3:39 PM, José Ignacio <jose.cyborg@xxxxxxxxx>
> wrote:
>
>> Or switching between object and grid snap :)
>>
>> On Mon, May 8, 2017 at 5:34 PM, Wayne Stambaugh <stambaughw@xxxxxxxxx>
>> wrote:
>>
>>> I tend to lean toward Oliver's approach.  Most CAD tools I've used have
>>> this type of includes vs intersects selection paradigm.  I don't see the
>>> need to tie up the modifier key if we don't have to.  I would prefer
>>> that we keep a modifier key open for something like orthogonal move.
>>>
>>> On 5/8/2017 5:51 PM, Oliver Walters wrote:
>>> > I was approaching this from having used mechanical CAD tools where the
>>> > direction of selection is the standard approach. Whatever function is
>>> > chosen, it will still be required that the users adjust to the new
>>> > style, manuals updated, etc.
>>> >
>>> > Is assigning what is essentially the last remaining modifier key worth
>>> > it for this?
>>> >
>>> > On 8 May 2017 23:55, "Nick Østergaard" <oe.nick@xxxxxxxxx
>>> > <mailto:oe.nick@xxxxxxxxx>> wrote:
>>> >
>>> >     2017-05-08 14:59 GMT+02:00 Maciej Sumiński <
>>> maciej.suminski@xxxxxxx
>>> >     <mailto:maciej.suminski@xxxxxxx>>:
>>> >     > Hi Oliver,
>>> >     >
>>> >     > I took your set of patches for a test drive. I am glad that you
>>> >     thought
>>> >     > about the subtractive mode in the selection tool, it really fits
>>> >     there.
>>> >     > Regarding different selection modes - I like the idea, but I
>>> think the
>>> >     > two modes should be more distinct, changing the selection
>>> direction
>>> >     > might be not enough.
>>> >     >
>>> >
>>> >     I personally prefer modifier keys as we are used to in Gimp and
>>> >     Inkscape.
>>> >
>>> >     > I observed another user trying out the tool and he could not
>>> tell how
>>> >     > does it work, but noticed it is a bit different than the old
>>> tool.
>>> >     >
>>> >     > Perhaps one of the following would work:
>>> >     >
>>> >     > - change the selection box colors so they are easier to tell
>>> apart (my
>>> >     > mate was surprised to find out there were two colors for the
>>> >     selection box)
>>> >     >
>>> >     > - change the mode using a key modifier (I think only Alt is left
>>> free)
>>> >     > or mouse button
>>> >     >
>>> >     > - add an option to select the default mode (though I do not
>>> really
>>> >     like
>>> >     > having too many options to set)
>>> >     >
>>> >     > I agree with Tom about the geometry library. IIRC currently it
>>> is only
>>> >     > used by the PNS router, but ultimately we would like to use it
>>> in the
>>> >     > primary model. The library already provides methods to check for
>>> >     > collisions between basic shapes, yet we still need a few more.
>>> >     > It would be a pity to drop your code now, so perhaps we could
>>> >     merge the
>>> >     > code as is and fix the methods during the model refactor.
>>> >     >
>>> >     > Just to let you know, there are a few code formatting violations
>>> >     (mostly
>>> >     > not keeping two empty lines between method definitions in .cpp
>>> files),
>>> >     > but as they are infrequent - I can handle them myself.
>>> >     >
>>> >     > Regards,
>>> >     > Orson
>>> >     >
>>> >     > On 05/07/2017 02:11 AM, Oliver Walters wrote:
>>> >     >> Maciej,
>>> >     >>
>>> >     >> That was it! Thanks for the hint.
>>> >     >>
>>> >     >> #0016 attached, which fixes both issues:
>>> >     >>
>>> >     >> a) No more double-selection of module and module-items (pads /
>>> >     lines / etc)
>>> >     >> in PCBNEW
>>> >     >> b) Disable selection of entire module in MODEDIT
>>> >     >>
>>> >     >> As far as I can tell this patchset is now working very well.
>>> >     >>
>>> >     >> Regards,
>>> >     >> Oliver
>>> >     >>
>>> >     >> On Sat, May 6, 2017 at 10:21 PM, Oliver Walters <
>>> >     >> oliver.henry.walters@xxxxxxxxx
>>> >     <mailto:oliver.henry.walters@xxxxxxxxx>> wrote:
>>> >     >>
>>> >     >>> Maciej,
>>> >     >>>
>>> >     >>> Thanks, I'll look into that. If you have a chance to look over
>>> >     what I've
>>> >     >>> done, I'd appreciate that :)
>>> >     >>>
>>> >     >>> On Sat, May 6, 2017 at 10:17 PM, Maciej Suminski
>>> >     <maciej.suminski@xxxxxxx <mailto:maciej.suminski@xxxxxxx>>
>>> >     >>> wrote:
>>> >     >>>
>>> >     >>>> Hi Oliver,
>>> >     >>>>
>>> >     >>>> I have not tested the patches yet, but my gut feeling says
>>> that
>>> >     you miss
>>> >     >>>> calling SELECTION_TOOL::selectable() to filter out redundant
>>> items.
>>> >     >>>>
>>> >     >>>> Regards,
>>> >     >>>> Orson
>>> >     >>>>
>>> >     >>>> On 05/06/2017 09:21 AM, Oliver Walters wrote:
>>> >     >>>>> Three further patch files attached:
>>> >     >>>>>
>>> >     >>>>> - Different color select box based on direction
>>> >     >>>>> - Fixed HitTest for EDA_TEXT
>>> >     >>>>> - Control modifier unselects anything in rectangle.
>>> >     >>>>>
>>> >     >>>>> The major piece of feedback I need right now is how to
>>> perfect the
>>> >     >>>>> behaviour of the tool in PCBNEW and MODEDIT:
>>> >     >>>>>
>>> >     >>>>> a) PCBNEW
>>> >     >>>>>
>>> >     >>>>> Selecting part of a MODULE (right to left) will select both
>>> >     the entire
>>> >     >>>>> module and also any parts of the module that you touched
>>> >     (lines, pads,
>>> >     >>>>> etc). Then, when you move the module, the doubly-selected
>>> >     items are
>>> >     >>>> moved
>>> >     >>>>> twice! It is hard to describe properly but if you try this
>>> you
>>> >     will see
>>> >     >>>>> what I mean.
>>> >     >>>>>
>>> >     >>>>> b) MODEDIT
>>> >     >>>>>
>>> >     >>>>> Selecting any item in the footprint selects the entire
>>> >     footprint, which
>>> >     >>>> is
>>> >     >>>>> highly undesirable. In this case I think the best approach is
>>> >     to filter
>>> >     >>>> the
>>> >     >>>>> MODULE from the selection entirely. But I am not sure how to
>>> >     do this.
>>> >     >>>>>
>>> >     >>>>> Feedback welcome :)
>>> >     >>>>>
>>> >     >>>>> Regards,
>>> >     >>>>> Oliver
>>> >     >>>>>
>>> >     >>>>> On Tue, May 2, 2017 at 5:25 PM, Oliver Walters <
>>> >     >>>>> oliver.henry.walters@xxxxxxxxx
>>> >     <mailto:oliver.henry.walters@xxxxxxxxx>> wrote:
>>> >     >>>>>
>>> >     >>>>>> I have attached a patch-set that implements "partial
>>> >     selection" of
>>> >     >>>> objects
>>> >     >>>>>> when the selection box is dragged right-to-left.
>>> >     >>>>>>
>>> >     >>>>>> L -> R = Objects must be completely enclosed to be selected
>>> >     >>>>>> R -> L = Objects that intersect the selection rectangle
>>> will be
>>> >     >>>> selected.
>>> >     >>>>>>
>>> >     >>>>>> To achieve this I had to fix a lot of the HitTest
>>> >     implementations as
>>> >     >>>> this
>>> >     >>>>>> was broken for most shapes, under a variety of edge cases
>>> >     (some HitTest
>>> >     >>>>>> code did not work at all).
>>> >     >>>>>>
>>> >     >>>>>> There are two issues I see as outstanding, and am unsure
>>> how to
>>> >     >>>> proceed:
>>> >     >>>>>>
>>> >     >>>>>> 1. When editing a PCB, selecting part of a footprint (e.g. a
>>> >     line of
>>> >     >>>> the
>>> >     >>>>>> courtyard) selects both that line and the entire footprint.
>>> >     This causes
>>> >     >>>>>> some issues when the footprint is dragged around the PCB. I
>>> >     believe
>>> >     >>>> that
>>> >     >>>>>> the line should not be selected separately, but the entire
>>> >     footprint
>>> >     >>>> should.
>>> >     >>>>>>
>>> >     >>>>>> 2. The inverse of 1. In the footprint editor, selecting a
>>> single
>>> >     >>>> graphical
>>> >     >>>>>> item selects the entire footprint. Somehow I would like to
>>> >     filter the
>>> >     >>>>>> selection such that individual items are selected but NOT
>>> the
>>> >     entire
>>> >     >>>>>> footprint.
>>> >     >>>>>>
>>> >     >>>>>> Feedback please! :)
>>> >     >>>>>>
>>> >     >>>>>> I have fixed hit testing (both for wxPoint and EDA_RECT
>>> >     comparison)
>>> >     >>>> for:
>>> >     >>>>>>
>>> >     >>>>>> - Pads (all shapes)
>>> >     >>>>>> - Lines
>>> >     >>>>>> - Circles
>>> >     >>>>>> - Arcs
>>> >     >>>>>> - Text items
>>> >     >>>>>> - Zones
>>> >     >>>>>> - Footprints
>>> >     >>>>>>
>>> >     >>>>>> Cheers,
>>> >     >>>>>> Oliver
>>> >     >>>>>>
>>> >     >>>>>>
>>> >     >>>>>>
>>> >     >>>>>
>>> >     >>>>>
>>> >     >>>>>
>>> >     >>>>> _______________________________________________
>>> >     >>>>> Mailing list: https://launchpad.net/~kicad-developers
>>> >     <https://launchpad.net/~kicad-developers>
>>> >     >>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>> >     >>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>> >     <https://launchpad.net/~kicad-developers>
>>> >     >>>>> More help   : https://help.launchpad.net/ListHelp
>>> >     <https://help.launchpad.net/ListHelp>
>>> >     >>>>>
>>> >     >>>>
>>> >     >>>> _______________________________________________
>>> >     >>>> Mailing list: https://launchpad.net/~kicad-developers
>>> >     <https://launchpad.net/~kicad-developers>
>>> >     >>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>> >     >>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>> >     <https://launchpad.net/~kicad-developers>
>>> >     >>>> More help   : https://help.launchpad.net/ListHelp
>>> >     <https://help.launchpad.net/ListHelp>
>>> >     >>>>
>>> >     >>>
>>> >     >>>
>>> >     >>
>>> >     >
>>> >     >
>>> >     >
>>> >     > _______________________________________________
>>> >     > Mailing list: https://launchpad.net/~kicad-developers
>>> >     <https://launchpad.net/~kicad-developers>
>>> >     > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>> >     > Unsubscribe : https://launchpad.net/~kicad-developers
>>> >     <https://launchpad.net/~kicad-developers>
>>> >     > More help   : https://help.launchpad.net/ListHelp
>>> >     <https://help.launchpad.net/ListHelp>
>>> >     >
>>> >
>>> >
>>> >
>>> >
>>> > _______________________________________________
>>> > Mailing list: https://launchpad.net/~kicad-developers
>>> > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> > Unsubscribe : https://launchpad.net/~kicad-developers
>>> > More help   : https://help.launchpad.net/ListHelp
>>> >
>>>
>>>
>>> _______________________________________________
>>> Mailing list: https://launchpad.net/~kicad-developers
>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>> More help   : https://help.launchpad.net/ListHelp
>>>
>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
>>
>
>
> --
> Remember The Past, Live The Present, Change The Future
> Those who look only to the past or the present are certain to miss the
> future [JFK]
>
> kandrey89@xxxxxxxxx
> Live Long and Prosper,
> Andrey
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
>
From dc398f4c9dc0a3db808a223500e451b69c0fa7d2 Mon Sep 17 00:00:00 2001
From: Oliver Walters <oliver.henry.walters@xxxxxxxxx>
Date: Tue, 9 May 2017 17:28:01 +1000
Subject: [PATCH 1/2] Fix for Arc HitTest

compureArcBBox does not account for line width
---
 pcbnew/class_drawsegment.cpp | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/pcbnew/class_drawsegment.cpp b/pcbnew/class_drawsegment.cpp
index 30f234a..b454c3a 100644
--- a/pcbnew/class_drawsegment.cpp
+++ b/pcbnew/class_drawsegment.cpp
@@ -431,7 +431,7 @@ const EDA_RECT DRAWSEGMENT::GetBoundingBox() const
         break;
 
     default:
-        ;
+        break;
     }
 
     bbox.Inflate( ((m_Width+1) / 2) + 1 );
@@ -521,17 +521,18 @@ bool DRAWSEGMENT::HitTest( const EDA_RECT& aRect, bool aContained, int aAccuracy
     arect.Inflate( aAccuracy );
 
     EDA_RECT arcRect;
+    EDA_RECT bb = GetBoundingBox();
 
     switch( m_Shape )
     {
     case S_CIRCLE:
         // Test if area intersects or contains the circle:
         if( aContained )
-            return arect.Contains( GetBoundingBox() );
+            return arect.Contains( bb );
         else
         {
             // If the rectangle does not intersect the bounding box, this is a much quicker test
-            if( !aRect.Intersects( GetBoundingBox() ) )
+            if( !aRect.Intersects( bb ) )
             {
                 return false;
             }
@@ -545,17 +546,15 @@ bool DRAWSEGMENT::HitTest( const EDA_RECT& aRect, bool aContained, int aAccuracy
 
     case S_ARC:
 
-        computeArcBBox( arcRect );
-
         // Test for full containment of this arc in the rect
         if( aContained )
         {
-            return arect.Contains( arcRect );
+            return arect.Contains( bb );
         }
         // Test if the rect crosses the arc
         else
         {
-            arcRect = arcRect.Common( arect );
+            arcRect = bb.Common( arect );
 
             /* All following tests must pass:
              * 1. Rectangle must intersect arc BoundingBox
-- 
2.7.4

From a6d013f0d0eb9d7996ce734444ca5f9de41f8396 Mon Sep 17 00:00:00 2001
From: Oliver Walters <oliver.henry.walters@xxxxxxxxx>
Date: Tue, 9 May 2017 17:28:54 +1000
Subject: [PATCH 2/2] SELECTION_AREA color now indicates selection mode

BLUE = Normal
GREEN = Addition
RED = Subtraction

Line color indicates "window" or "crossing" selection mode
---
 common/preview_items/selection_area.cpp | 49 ++++++++++++++++++++++++++++-----
 include/preview_items/selection_area.h  | 10 +++++++
 pcbnew/tools/selection_tool.cpp         |  7 ++---
 3 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/common/preview_items/selection_area.cpp b/common/preview_items/selection_area.cpp
index 001e6df..ecee3c3 100644
--- a/common/preview_items/selection_area.cpp
+++ b/common/preview_items/selection_area.cpp
@@ -30,16 +30,36 @@
 using namespace KIGFX::PREVIEW;
 
 // Selection area colours
-const COLOR4D SELECT_COLOR_L2R( 0.3, 0.3, 0.6, 0.3 );  // Slight blue
-const COLOR4D SELECT_COLOR_R2L( 0.3, 0.6, 0.3, 0.3 );  // Slight green
+const COLOR4D SELECT_MODE_NORMAL( 0.3, 0.3, 0.7, 0.3 ); // Slight blue
+const COLOR4D SELECT_MODE_ADDITIVE( 0.3, 0.7, 0.3, 0.3 ); // Slight green
+const COLOR4D SELECT_MODE_SUBTRACT( 0.7, 0.3, 0.3, 0.3 ); // Slight red
 
+const COLOR4D SELECT_OUTLINE_L2R( 1.0, 1.0, 0.4, 1.0 );
+const COLOR4D SELECT_OUTLINE_R2L( 0.4, 0.4, 1.0, 1.0 );
 
-SELECTION_AREA::SELECTION_AREA()
+SELECTION_AREA::SELECTION_AREA() :
+        m_additive( false ),
+        m_subtractive( false )
 {
-    SetStrokeColor( COLOR4D( 1.0, 1.0, 0.4, 1.0 ) );
-    SetFillColor( COLOR4D( 0.3, 0.3, 0.5, 0.3 ) );
+    SetStrokeColor( SELECT_OUTLINE_L2R );
+    SetFillColor( SELECT_MODE_NORMAL );
 }
 
+void SELECTION_AREA::SetAdditive( bool aAdditive )
+{
+    m_additive = aAdditive;
+
+    if( m_additive )
+        m_subtractive = false;
+}
+
+void SELECTION_AREA::SetSubtractive( bool aSubtractive )
+{
+    m_subtractive = aSubtractive;
+
+    if ( m_subtractive )
+        m_additive = false;
+}
 
 const BOX2I SELECTION_AREA::ViewBBox() const
 {
@@ -54,8 +74,23 @@ const BOX2I SELECTION_AREA::ViewBBox() const
 
 void SELECTION_AREA::drawPreviewShape( KIGFX::GAL& aGal ) const
 {
-    // Set the fill color based on the direction of selection
-    aGal.SetFillColor( ( m_origin.x <= m_end.x ) ? SELECT_COLOR_L2R : SELECT_COLOR_R2L );
+    // Set the fill of the selection rectangle
+    // based on the selection mode
+    if( m_additive )
+    {
+        aGal.SetFillColor( SELECT_MODE_ADDITIVE );
+    }
+    else if( m_subtractive )
+    {
+        aGal.SetFillColor( SELECT_MODE_SUBTRACT );
+    }
+    else
+    {
+        aGal.SetFillColor( SELECT_MODE_NORMAL );
+    }
+
+    // Set the stroke color to indicate window or crossing selection
+    aGal.SetStrokeColor( ( m_origin.x <= m_end.x ) ? SELECT_OUTLINE_L2R : SELECT_OUTLINE_R2L );
 
     aGal.DrawRectangle( m_origin, m_end );
 }
diff --git a/include/preview_items/selection_area.h b/include/preview_items/selection_area.h
index d45924b..a2b1db2 100644
--- a/include/preview_items/selection_area.h
+++ b/include/preview_items/selection_area.h
@@ -80,8 +80,18 @@ public:
 
     VECTOR2I GetEnd() const { return m_end; }
 
+    bool IsAdditive() const { return m_additive; }
+    bool IsSubtractive() const { return m_subtractive; }
+
+    void SetAdditive( bool aAdditive );
+    void SetSubtractive( bool aSubtractive );
+
+
 private:
 
+    bool m_additive;
+    bool m_subtractive;
+
     /**
      * Draw the selection rectangle onto the GAL
      */
diff --git a/pcbnew/tools/selection_tool.cpp b/pcbnew/tools/selection_tool.cpp
index ec4e980..dd339f3 100644
--- a/pcbnew/tools/selection_tool.cpp
+++ b/pcbnew/tools/selection_tool.cpp
@@ -488,14 +488,13 @@ bool SELECTION_TOOL::selectMultiple()
 
         if( evt->IsDrag( BUT_LEFT ) )
         {
-            if( !m_additive && !m_subtractive )
-            {
-                clearSelection();
-            }
 
             // Start drawing a selection box
             area.SetOrigin( evt->DragOrigin() );
             area.SetEnd( evt->Position() );
+            area.SetAdditive( m_additive );
+            area.SetSubtractive( m_subtractive );
+
             view->SetVisible( &area, true );
             view->Update( &area );
         }
-- 
2.7.4


Follow ups

References