kicad-developers team mailing list archive

[Seth Hillbrand <seth.hillbrand@xxxxxxxxx>]

Jeff-

I get your reasoning.  However, it seems like you could accomplish the same
goal (preventing disambiguation) by using the existing
GENERAL_COLLECTOR::Modules[] for your footprint disambiguation and maybe
modifying the COLLECTORS_GUIDE or Inspect() to get the track
disambiguation.  This would reduce the impact of your patch and not create
duplicate functionality.

In other aspects, the changes to selectPoint do not seem required (correct
me if I'm wrong) and they introduce a subtle bug by triggering a
Wait(TOOL_EVENT) even if the heuristics have removed all items from the
collector.

Overall, I like the idea.  It makes sense to have a post-filter.  But I
would worry that this is more invasive than is needed to resolve the issue
during feature-freeze.

-S

2018-01-09 11:36 GMT-08:00 Jeff Young <jeff@xxxxxxxxx>:

> Hi Seth,
>
> The test_window creates a  SELECTION_TOOL without the rest of kicad behind
> it.  Since I added methods which SELECTION_TOOL calls, I had to mock them
> in the test files.
>
> GENERAL_COLLECTOR::Modules[] can’t be used because the creator of the
> collector (SELECTION_TOOL) shouldn’t know anything about the semantics of
> the filter (only the specific action implementation knows that).  In the
> FootprintFilter case it’s simply enough that you’d nearly be willing to
> overlook that, but the other filters are more complicated and required
> things like ROUTER_TOOL knowledge, which definitely shouldn’t be leaking
> into SELECTION_TOOL.
>
> Cheers,
> Jeff.
>
>
> On 9 Jan 2018, at 19:26, Seth Hillbrand <seth.hillbrand@xxxxxxxxx> wrote:
>
> Same here.  Same errors.  Yes, cmaked.
>
> I don't understand why you are changing the test_window link libraries in
> this patch.
>
> Also, why did you decide to write a "FootprintFilter" routine instead of
> using the GENERAL_COLLECTOR::Modules[]?
>
> -S
>
> 2018-01-09 11:05 GMT-08:00 Jeff Young <jeff@xxxxxxxxx>:
>
>> Did you do a CMake?  There’s a change in the CMake files….
>>
>> > On 9 Jan 2018, at 18:59, kristoffer Ödmark <
>> kristofferodmark90@xxxxxxxxx> wrote:
>> >
>> > Hmm, cannot compile, get a lot of undefiner references. Even after
>> nuking the build dir.
>> >
>> >
>> > ../../common/libpcbcommon.a(class_pad.cpp.o): In function
>> `D_PAD::HitTest(EDA_RECT const&, bool, int) const':
>> > kicad/pcbnew/class_pad.cpp:982: undefined reference to
>> `TestPointInsidePolygon(wxPoint const*, int, wxPoint const&)'
>> > ../../common/libpcbcommon.a(class_zone.cpp.o): In function
>> `ZONE_CONTAINER::Hatch()':
>> > kicad/pcbnew/class_zone.cpp:1219: undefined reference to
>> `FindLineSegmentIntersection(double, double, int, int, int, int,
>> double*, double*, double*, double*, double*)'
>> > ../../common/libcommon.a(shape_poly_set.cpp.o): In function
>> `SHAPE_POLY_SET::convertToClipper(SHAPE_LINE_CHAIN const&, bool)':
>> > kicad/common/geometry/shape_poly_set.cpp:466: undefined reference to
>> `ClipperLib::Orientation(std::vector<ClipperLib::IntPoint,
>> std::allocator<ClipperLib::IntPoint> > const&)'
>> > kicad/common/geometry/shape_poly_set.cpp:467: undefined reference to
>> `ClipperLib::ReversePath(std::vector<ClipperLib::IntPoint,
>> std::allocator<ClipperLib::IntPoint> >&)'
>> > ../../common/libcommon.a(shape_poly_set.cpp.o): In function
>> `SHAPE_POLY_SET::booleanOp(ClipperLib::ClipType, SHAPE_POLY_SET const&,
>> SHAPE_POLY_SET::POLYGON_MODE)':
>> > kicad/common/geometry/shape_poly_set.cpp:489: undefined reference to
>> `ClipperLib::Clipper::Clipper(int)'
>> >
>> >
>> > On 2018-01-09 19:22, Wayne Stambaugh wrote:
>> >> Does the patch resolve your issue?
>> >>
>> >> On 1/9/2018 1:21 PM, kristoffer Ödmark wrote:
>> >>> Yes, I can reproduce this with very thick tracks!
>> >>>
>> >>>
>> >>> On 2018-01-09 16:55, Jeff Young wrote:
>> >>>> Hi Kristoffer,
>> >>>>
>> >>>> That’s odd.  Did you try it with your mouse pointer directly over the
>> >>>> corner?  (You may need a reasonably thick track for this to happen.
>> >>>> Try something on the order of 2mm.)
>> >>>>
>> >>>> Without my change the selection disambiguation is run *before* we
>> know
>> >>>> it’s a drag action on a simple corner, so the selection_tool thinks
>> it
>> >>>> needs to know which of the two segments is to be selected.
>> >>>>
>> >>>> Cheers,
>> >>>> Jeff.
>> >>>>
>> >>>>
>> >>>>> On 9 Jan 2018, at 15:37, Kristoffer Ödmark
>> >>>>> <kristofferodmark90@xxxxxxxxx> wrote:
>> >>>>>
>> >>>>> If i understand him correctly that would only be when on a corner, I
>> >>>>> think it would be the desired behaviour.
>> >>>>>
>> >>>>> Although, when I try it on my build on my laptop, there is no
>> clarify
>> >>>>> selection menu popping up when trying do drag ( 'd' ) or such on
>> >>>>> corners. Not in opengl anywas. So I dont know.
>> >>>>>
>> >>>>> Application: kicad
>> >>>>> Version: (2017-12-30 revision b55eb0b5f)-master, release build
>> >>>>> Libraries:
>> >>>>>     wxWidgets 3.0.3
>> >>>>>     libcurl/7.57.0 OpenSSL/1.1.0g zlib/1.2.11 libidn2/2.0.4
>> >>>>> libpsl/0.19.1 (+libidn2/2.0.4) libssh2/1.8.0 nghttp2/1.29.0
>> >>>>> Platform: Linux 4.14.12-1-MANJARO x86_64, 64 bit, Little endian,
>> wxGTK
>> >>>>> Build Info:
>> >>>>>     wxWidgets: 3.0.3 (wchar_t,wx containers,compatible with 2.8)
>> GTK+
>> >>>>> 2.24
>> >>>>>     Boost: 1.65.1
>> >>>>>     Curl: 7.57.0
>> >>>>>     Compiler: GCC 7.2.1 with C++ ABI 1011
>> >>>>>
>> >>>>> Build settings:
>> >>>>>     USE_WX_GRAPHICS_CONTEXT=OFF
>> >>>>>     USE_WX_OVERLAY=OFF
>> >>>>>     KICAD_SCRIPTING=ON
>> >>>>>     KICAD_SCRIPTING_MODULES=ON
>> >>>>>     KICAD_SCRIPTING_WXPYTHON=ON
>> >>>>>     KICAD_SCRIPTING_ACTION_MENU=ON
>> >>>>>     BUILD_GITHUB_PLUGIN=ON
>> >>>>>     KICAD_USE_OCE=ON
>> >>>>>     KICAD_SPICE=ON
>> >>>>>
>> >>>>>
>> >>>>> -Kristoffer
>> >>>>>
>> >>>>> On 01/09/2018 04:27 PM, Wayne Stambaugh wrote:
>> >>>>>> Jeff,
>> >>>>>> Have actually confirmed that this is the desired behavior for this
>> >>>>>> outside of you own objectives?  I'm not saying that this is or
>> isn't a
>> >>>>>> good idea but I personally don't drag trace corners around so I'm
>> not
>> >>>>>> sure what the appropriate behavior should be.  You should get
>> comments
>> >>>>>> from the dev list and users before you make a change like this.
>> As far
>> >>>>>> as pushing this to the dev repo, if it's not too invasive I will
>> >>>>>> consider it.  If it is a large change set, I would prefer that we
>> hold
>> >>>>>> off until after the stable release.
>> >>>>>> Thanks,
>> >>>>>> Wayne
>> >>>>>> On 1/8/2018 5:49 AM, Jeff Young wrote:
>> >>>>>>> Wayne, if I could get you to don that old project manager’s hat
>> one
>> >>>>>>> more time:
>> >>>>>>>
>> >>>>>>> If we’re still weeks out from declaring an RC, I wanted to make
>> one
>> >>>>>>> more plug for getting rid of the Clarify Selection dialog when
>> >>>>>>> dragging corners or using ‘U’ or ‘I’ over a corner[1].
>> >>>>>>>
>> >>>>>>> While it’s marked Wishlist, it seriously impacts productivity when
>> >>>>>>> editing tracks, and I think most users would consider it a bug
>> >>>>>>> (particularly in the corner case when dragging the corner is
>> >>>>>>> clearly moving both the tracks listed in the Clarify Selection
>> menu).
>> >>>>>>>
>> >>>>>>> I’ve been running the patch for about a week now with no issues.
>> >>>>>>>
>> >>>>>>> Cheers,
>> >>>>>>> Jeff.
>> >>>>>>>
>> >>>>>>> [1] https://bugs.launchpad.net/kicad/+bug/1708869
>> >>>>>>> _______________________________________________
>> >>>>>>> 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
>> >>>
>> >>> _______________________________________________
>> >>> 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
>>
>>
>> _______________________________________________
>> 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
>>
>
>
>