Re: [Kicad-developers] [PATCH] Rework footprint selection filtering to improve behavior

2018-03-07 Thread Jon Evans
Fixed that issue, and pushed to master.  Thanks for testing, Andrzej!

On Mon, Mar 5, 2018 at 12:57 PM, Andrzej Wolski 
wrote:

> Hi Jon,
>
> another small issue: if only enabled layer is In1.Cu, THT pads are
> visible, but footprint cannot be selected.
> I think that breaks your visible=selectable rule.
>
> Fragment of code from D_PAD::ViewGetLOD that may help here:
>
> if( ( GetAttribute() == PAD_ATTRIB_STANDARD || GetAttribute() ==
> PAD_ATTRIB_HOLE_NOT_PLATED )
>  && !aView->IsLayerVisible( LAYER_PADS_TH ) )
> return HIDE;
>
> Cheers,
> Andrzej
>
> W dniu 2018-03-03 o 23:39, Jon Evans pisze:
>
> Hey Andy, Andrzej,
>
> Updated patch attached, let me know if this behavior makes more sense
>
> Best,
> Jon
>
> On Fri, Mar 2, 2018 at 3:29 PM, Andy Peters  wrote:
>
>>
>>
>> On Mar 2, 2018, at 10:28 AM, Andrzej Wolski 
>> wrote:
>>
>> Jon,
>>
>> I probably didn't express myself clearly. What I mean is a situation when
>> *only* enabled layer if F.Paste and then you disable "Pads Front". Now
>> nothing is visible on the board, but footprints are still selectable.
>>
>> In other words, when no single item belonging to footprint is visible,
>> footprints should not be selectable.
>>
>> Do you still disagree with me?
>>
>>
>> I agree with Andrzej. This is the crux of my bug report.
>>
>> But I understand what Jon is saying, and it doesn’t contradict. If the
>> _pads_ (for example) are invisible, but say the silkscreen is visible, then
>> the footprint _should_ be selectable.
>>
>> That, I think, is the difference between layer visibility and item
>> visibility. All of the layers could be visible, but if I disable Footprints
>> Front, then everything associated with all top-layer footprints vanishes
>> and none of the footprints can be selected. That’s the proper operation.
>>
>> The converse of that is if I leave Footprints Front visibility enabled,
>> and then I go and disable all of the layers associated with front
>> footprints, like I did in my bug report case (disable all layers except
>> Cmts.User), I am still able to select all of the invisible footprints.
>> That’s not correct (IMHO).
>>
>> -a
>>
>>
>>
>> Andrzej
>>
>> W dniu 2018-03-02 o 15:42, Jon Evans pisze:
>>
>> Hi Andrzej,
>>
>> This was my intention, which is why I said I was prepared for other
>> people to have other opinions :-)
>>
>> I think that you should still be able to select footprints even if the
>> "front pads" is hidden from layers like the paste layer, *unless* you are
>> in high contrast mode.
>>
>> -Jon
>>
>> On Fri, Mar 2, 2018 at 3:53 AM, Andrzej Wolski 
>> wrote:
>>
>>> I've tried this patch, and there is a small issue: if you have only eg
>>> front paste layer enabled and front pads are hidden, footprint is still
>>> selectable.
>>>
>>> Andrzej
>>>
>>>
>>> W dniu 2018-02-27 o 04:11, Jon Evans pisze:
>>>
>>> This patch changes the selection logic for footprints to fix a reported
>>> issue[1] and to make the behavior more logical to me.
>>>
>>> I know that correct selection behavior is something of a personal
>>> preference, so I'm ready to be flamed :-)
>>>
>>> The new behavior:
>>>
>>> A footprint may be selected if:
>>> 1) The corresponding "Footprints" switch is on in the Items tab, AND
>>> 2) One or more of the layers that the footprint draws on is visible
>>>
>>> This means that if all of the layers are turned off, footprints are not
>>> selectable (fixes the bug), but it also means that now footprints can be
>>> selected if any draw layers are visible (for example, if you have only
>>> F.Mask enabled, you can select a footprint that has solder mask and is on
>>> the front layer).
>>>
>>> Before anyone complains, this is only if high-contrast mode is turned
>>> OFF.  When it is on, you can still only select items that *only* exist on
>>> that layer (to make moving silkscreen around easier, for example)
>>>
>>> Even though this adds some more for-loops to selection filtering, I have
>>> not noticed any performance hits on some selection of large boards that I
>>> tested.  More testing is welcome.
>>>
>>> [1] https://bugs.launchpad.net/kicad/+bug/1751960
>>>
>>> -Jon
>>>
>>>
>>
>> ___
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to : kicad-developers@lists.launchpad.net
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
>>
>
>
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
>
>
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : 

Re: [Kicad-developers] [PATCH] Rework footprint selection filtering to improve behavior

2018-03-05 Thread Andrzej Wolski

Hi Jon,

another small issue: if only enabled layer is In1.Cu, THT pads are 
visible, but footprint cannot be selected.

I think that breaks your visible=selectable rule.

Fragment of code from D_PAD::ViewGetLOD that may help here:

    if( ( GetAttribute() == PAD_ATTRIB_STANDARD || GetAttribute() == 
PAD_ATTRIB_HOLE_NOT_PLATED )

 && !aView->IsLayerVisible( LAYER_PADS_TH ) )
    return HIDE;

Cheers,
Andrzej

W dniu 2018-03-03 o 23:39, Jon Evans pisze:

Hey Andy, Andrzej,

Updated patch attached, let me know if this behavior makes more sense

Best,
Jon

On Fri, Mar 2, 2018 at 3:29 PM, Andy Peters > wrote:





On Mar 2, 2018, at 10:28 AM, Andrzej Wolski
> wrote:

Jon,

I probably didn't express myself clearly. What I mean is a
situation when *only* enabled layer if F.Paste and then you
disable "Pads Front". Now nothing is visible on the board, but
footprints are still selectable.

In other words, when no single item belonging to footprint is
visible, footprints should not be selectable.

Do you still disagree with me?


I agree with Andrzej. This is the crux of my bug report.

But I understand what Jon is saying, and it doesn’t contradict. If
the _pads_ (for example) are invisible, but say the silkscreen is
visible, then the footprint _should_ be selectable.

That, I think, is the difference between layer visibility and item
visibility. All of the layers could be visible, but if I disable
Footprints Front, then everything associated with all top-layer
footprints vanishes and none of the footprints can be selected.
That’s the proper operation.

The converse of that is if I leave Footprints Front visibility
enabled, and then I go and disable all of the layers associated
with front footprints, like I did in my bug report case (disable
all layers except Cmts.User), I am still able to select all of the
invisible footprints. That’s not correct (IMHO).

-a




Andrzej

W dniu 2018-03-02 o 15:42, Jon Evans pisze:

Hi Andrzej,

This was my intention, which is why I said I was prepared for
other people to have other opinions :-)

I think that you should still be able to select footprints even
if the "front pads" is hidden from layers like the paste layer,
*unless* you are in high contrast mode.

-Jon

On Fri, Mar 2, 2018 at 3:53 AM, Andrzej Wolski
> wrote:

I've tried this patch, and there is a small issue: if you
have only eg front paste layer enabled and front pads are
hidden, footprint is still selectable.

Andrzej


W dniu 2018-02-27 o 04:11, Jon Evans pisze:

This patch changes the selection logic for footprints to
fix a reported issue[1] and to make the behavior more
logical to me.

I know that correct selection behavior is something of a
personal preference, so I'm ready to be flamed :-)

The new behavior:

A footprint may be selected if:
1) The corresponding "Footprints" switch is on in the Items
tab, AND
2) One or more of the layers that the footprint draws on is
visible

This means that if all of the layers are turned off,
footprints are not selectable (fixes the bug), but it also
means that now footprints can be selected if any draw
layers are visible (for example, if you have only F.Mask
enabled, you can select a footprint that has solder mask
and is on the front layer).

Before anyone complains, this is only if high-contrast mode
is turned OFF.  When it is on, you can still only select
items that *only* exist on that layer (to make moving
silkscreen around easier, for example)

Even though this adds some more for-loops to selection
filtering, I have not noticed any performance hits on some
selection of large boards that I tested. More testing is
welcome.

[1] https://bugs.launchpad.net/kicad/+bug/1751960


-Jon




___
Mailing list: https://launchpad.net/~kicad-developers

Post to     : kicad-developers@lists.launchpad.net

Unsubscribe : https://launchpad.net/~kicad-developers

More help   : https://help.launchpad.net/ListHelp





___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   

Re: [Kicad-developers] [PATCH] Rework footprint selection filtering to improve behavior

2018-03-05 Thread Wayne Stambaugh
I like this change!  I've never liked the fact that objects on invisible
layers get selected.  Has anyone else had a chance to test this?  If
there are no objections, I recommend that this patch get merged for rc2.

On 2/26/2018 10:11 PM, Jon Evans wrote:
> This patch changes the selection logic for footprints to fix a reported
> issue[1] and to make the behavior more logical to me.
> 
> I know that correct selection behavior is something of a personal
> preference, so I'm ready to be flamed :-)
> 
> The new behavior:
> 
> A footprint may be selected if:
> 1) The corresponding "Footprints" switch is on in the Items tab, AND
> 2) One or more of the layers that the footprint draws on is visible
> 
> This means that if all of the layers are turned off, footprints are not
> selectable (fixes the bug), but it also means that now footprints can be
> selected if any draw layers are visible (for example, if you have only
> F.Mask enabled, you can select a footprint that has solder mask and is
> on the front layer).
> 
> Before anyone complains, this is only if high-contrast mode is turned
> OFF.  When it is on, you can still only select items that *only* exist
> on that layer (to make moving silkscreen around easier, for example)
> 
> Even though this adds some more for-loops to selection filtering, I have
> not noticed any performance hits on some selection of large boards that
> I tested.  More testing is welcome.
> 
> [1] https://bugs.launchpad.net/kicad/+bug/1751960
> 
> -Jon
> 
> 
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 

___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Kicad-developers] [PATCH] Rework footprint selection filtering to improve behavior

2018-03-03 Thread Jon Evans
Hey Andy, Andrzej,

Updated patch attached, let me know if this behavior makes more sense

Best,
Jon

On Fri, Mar 2, 2018 at 3:29 PM, Andy Peters  wrote:

>
>
> On Mar 2, 2018, at 10:28 AM, Andrzej Wolski 
> wrote:
>
> Jon,
>
> I probably didn't express myself clearly. What I mean is a situation when
> *only* enabled layer if F.Paste and then you disable "Pads Front". Now
> nothing is visible on the board, but footprints are still selectable.
>
> In other words, when no single item belonging to footprint is visible,
> footprints should not be selectable.
>
> Do you still disagree with me?
>
>
> I agree with Andrzej. This is the crux of my bug report.
>
> But I understand what Jon is saying, and it doesn’t contradict. If the
> _pads_ (for example) are invisible, but say the silkscreen is visible, then
> the footprint _should_ be selectable.
>
> That, I think, is the difference between layer visibility and item
> visibility. All of the layers could be visible, but if I disable Footprints
> Front, then everything associated with all top-layer footprints vanishes
> and none of the footprints can be selected. That’s the proper operation.
>
> The converse of that is if I leave Footprints Front visibility enabled,
> and then I go and disable all of the layers associated with front
> footprints, like I did in my bug report case (disable all layers except
> Cmts.User), I am still able to select all of the invisible footprints.
> That’s not correct (IMHO).
>
> -a
>
>
>
> Andrzej
>
> W dniu 2018-03-02 o 15:42, Jon Evans pisze:
>
> Hi Andrzej,
>
> This was my intention, which is why I said I was prepared for other people
> to have other opinions :-)
>
> I think that you should still be able to select footprints even if the
> "front pads" is hidden from layers like the paste layer, *unless* you are
> in high contrast mode.
>
> -Jon
>
> On Fri, Mar 2, 2018 at 3:53 AM, Andrzej Wolski 
> wrote:
>
>> I've tried this patch, and there is a small issue: if you have only eg
>> front paste layer enabled and front pads are hidden, footprint is still
>> selectable.
>>
>> Andrzej
>>
>>
>> W dniu 2018-02-27 o 04:11, Jon Evans pisze:
>>
>> This patch changes the selection logic for footprints to fix a reported
>> issue[1] and to make the behavior more logical to me.
>>
>> I know that correct selection behavior is something of a personal
>> preference, so I'm ready to be flamed :-)
>>
>> The new behavior:
>>
>> A footprint may be selected if:
>> 1) The corresponding "Footprints" switch is on in the Items tab, AND
>> 2) One or more of the layers that the footprint draws on is visible
>>
>> This means that if all of the layers are turned off, footprints are not
>> selectable (fixes the bug), but it also means that now footprints can be
>> selected if any draw layers are visible (for example, if you have only
>> F.Mask enabled, you can select a footprint that has solder mask and is on
>> the front layer).
>>
>> Before anyone complains, this is only if high-contrast mode is turned
>> OFF.  When it is on, you can still only select items that *only* exist on
>> that layer (to make moving silkscreen around easier, for example)
>>
>> Even though this adds some more for-loops to selection filtering, I have
>> not noticed any performance hits on some selection of large boards that I
>> tested.  More testing is welcome.
>>
>> [1] https://bugs.launchpad.net/kicad/+bug/1751960
>>
>> -Jon
>>
>>
>
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
>


0001-Rework-footprint-selection-filtering-to-improve-beha.patch
Description: Binary data
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Kicad-developers] [PATCH] Rework footprint selection filtering to improve behavior

2018-03-02 Thread Andy Peters


> On Mar 2, 2018, at 10:28 AM, Andrzej Wolski  wrote:
> 
> Jon,
> 
> I probably didn't express myself clearly. What I mean is a situation when 
> *only* enabled layer if F.Paste and then you disable "Pads Front". Now 
> nothing is visible on the board, but footprints are still selectable.
> 
> In other words, when no single item belonging to footprint is visible, 
> footprints should not be selectable.
> 
> Do you still disagree with me?

I agree with Andrzej. This is the crux of my bug report.

But I understand what Jon is saying, and it doesn’t contradict. If the _pads_ 
(for example) are invisible, but say the silkscreen is visible, then the 
footprint _should_ be selectable.

That, I think, is the difference between layer visibility and item visibility. 
All of the layers could be visible, but if I disable Footprints Front, then 
everything associated with all top-layer footprints vanishes and none of the 
footprints can be selected. That’s the proper operation.

The converse of that is if I leave Footprints Front visibility enabled, and 
then I go and disable all of the layers associated with front footprints, like 
I did in my bug report case (disable all layers except Cmts.User), I am still 
able to select all of the invisible footprints. That’s not correct (IMHO).

-a


> 
> Andrzej
> 
> W dniu 2018-03-02 o 15:42, Jon Evans pisze:
>> Hi Andrzej,
>> 
>> This was my intention, which is why I said I was prepared for other people 
>> to have other opinions :-)
>> 
>> I think that you should still be able to select footprints even if the 
>> "front pads" is hidden from layers like the paste layer, *unless* you are in 
>> high contrast mode.
>> 
>> -Jon
>> 
>> On Fri, Mar 2, 2018 at 3:53 AM, Andrzej Wolski > > wrote:
>> I've tried this patch, and there is a small issue: if you have only eg front 
>> paste layer enabled and front pads are hidden, footprint is still selectable.
>> 
>> Andrzej
>> 
>> 
>> W dniu 2018-02-27 o 04:11, Jon Evans pisze:
>>> This patch changes the selection logic for footprints to fix a reported 
>>> issue[1] and to make the behavior more logical to me.
>>> 
>>> I know that correct selection behavior is something of a personal 
>>> preference, so I'm ready to be flamed :-)
>>> 
>>> The new behavior:
>>> 
>>> A footprint may be selected if:
>>> 1) The corresponding "Footprints" switch is on in the Items tab, AND
>>> 2) One or more of the layers that the footprint draws on is visible
>>> 
>>> This means that if all of the layers are turned off, footprints are not 
>>> selectable (fixes the bug), but it also means that now footprints can be 
>>> selected if any draw layers are visible (for example, if you have only 
>>> F.Mask enabled, you can select a footprint that has solder mask and is on 
>>> the front layer).
>>> 
>>> Before anyone complains, this is only if high-contrast mode is turned OFF.  
>>> When it is on, you can still only select items that *only* exist on that 
>>> layer (to make moving silkscreen around easier, for example)
>>> 
>>> Even though this adds some more for-loops to selection filtering, I have 
>>> not noticed any performance hits on some selection of large boards that I 
>>> tested.  More testing is welcome.
>>> 
>>> [1] https://bugs.launchpad.net/kicad/+bug/1751960 
>>> 
>>> 
>>> -Jon
>>> 

___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Kicad-developers] [PATCH] Rework footprint selection filtering to improve behavior

2018-03-02 Thread Jon Evans
No, I agree with you in that case, that sounds like a bug in my patch and
I'll check it out this weekend when I can get back to coding.

Thanks,
Jon

On Fri, Mar 2, 2018 at 12:28 PM, Andrzej Wolski 
wrote:

> Jon,
>
> I probably didn't express myself clearly. What I mean is a situation when
> *only* enabled layer if F.Paste and then you disable "Pads Front". Now
> nothing is visible on the board, but footprints are still selectable.
>
> In other words, when no single item belonging to footprint is visible,
> footprints should not be selectable.
>
> Do you still disagree with me?
>
> Andrzej
>
> W dniu 2018-03-02 o 15:42, Jon Evans pisze:
>
> Hi Andrzej,
>
> This was my intention, which is why I said I was prepared for other people
> to have other opinions :-)
>
> I think that you should still be able to select footprints even if the
> "front pads" is hidden from layers like the paste layer, *unless* you are
> in high contrast mode.
>
> -Jon
>
> On Fri, Mar 2, 2018 at 3:53 AM, Andrzej Wolski 
> wrote:
>
>> I've tried this patch, and there is a small issue: if you have only eg
>> front paste layer enabled and front pads are hidden, footprint is still
>> selectable.
>>
>> Andrzej
>>
>>
>> W dniu 2018-02-27 o 04:11, Jon Evans pisze:
>>
>> This patch changes the selection logic for footprints to fix a reported
>> issue[1] and to make the behavior more logical to me.
>>
>> I know that correct selection behavior is something of a personal
>> preference, so I'm ready to be flamed :-)
>>
>> The new behavior:
>>
>> A footprint may be selected if:
>> 1) The corresponding "Footprints" switch is on in the Items tab, AND
>> 2) One or more of the layers that the footprint draws on is visible
>>
>> This means that if all of the layers are turned off, footprints are not
>> selectable (fixes the bug), but it also means that now footprints can be
>> selected if any draw layers are visible (for example, if you have only
>> F.Mask enabled, you can select a footprint that has solder mask and is on
>> the front layer).
>>
>> Before anyone complains, this is only if high-contrast mode is turned
>> OFF.  When it is on, you can still only select items that *only* exist on
>> that layer (to make moving silkscreen around easier, for example)
>>
>> Even though this adds some more for-loops to selection filtering, I have
>> not noticed any performance hits on some selection of large boards that I
>> tested.  More testing is welcome.
>>
>> [1] https://bugs.launchpad.net/kicad/+bug/1751960
>>
>> -Jon
>>
>>
>> ___
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to : kicad-developers@lists.launchpad.net
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
>>
>>
>> ___
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to : kicad-developers@lists.launchpad.net
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
>>
>
>
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Kicad-developers] [PATCH] Rework footprint selection filtering to improve behavior

2018-03-02 Thread Andrzej Wolski

Jon,

I probably didn't express myself clearly. What I mean is a situation 
when *only* enabled layer if F.Paste and then you disable "Pads Front". 
Now nothing is visible on the board, but footprints are still selectable.


In other words, when no single item belonging to footprint is visible, 
footprints should not be selectable.


Do you still disagree with me?

Andrzej

W dniu 2018-03-02 o 15:42, Jon Evans pisze:

Hi Andrzej,

This was my intention, which is why I said I was prepared for other 
people to have other opinions :-)


I think that you should still be able to select footprints even if the 
"front pads" is hidden from layers like the paste layer, *unless* you 
are in high contrast mode.


-Jon

On Fri, Mar 2, 2018 at 3:53 AM, Andrzej Wolski 
> wrote:


I've tried this patch, and there is a small issue: if you have
only eg front paste layer enabled and front pads are hidden,
footprint is still selectable.

Andrzej


W dniu 2018-02-27 o 04:11, Jon Evans pisze:

This patch changes the selection logic for footprints to fix a
reported issue[1] and to make the behavior more logical to me.

I know that correct selection behavior is something of a personal
preference, so I'm ready to be flamed :-)

The new behavior:

A footprint may be selected if:
1) The corresponding "Footprints" switch is on in the Items tab, AND
2) One or more of the layers that the footprint draws on is visible

This means that if all of the layers are turned off, footprints
are not selectable (fixes the bug), but it also means that now
footprints can be selected if any draw layers are visible (for
example, if you have only F.Mask enabled, you can select a
footprint that has solder mask and is on the front layer).

Before anyone complains, this is only if high-contrast mode is
turned OFF.  When it is on, you can still only select items that
*only* exist on that layer (to make moving silkscreen around
easier, for example)

Even though this adds some more for-loops to selection filtering,
I have not noticed any performance hits on some selection of
large boards that I tested.  More testing is welcome.

[1] https://bugs.launchpad.net/kicad/+bug/1751960


-Jon


___
Mailing list:https://launchpad.net/~kicad-developers

Post to :kicad-developers@lists.launchpad.net

Unsubscribe :https://launchpad.net/~kicad-developers

More help   :https://help.launchpad.net/ListHelp





___
Mailing list: https://launchpad.net/~kicad-developers

Post to     : kicad-developers@lists.launchpad.net

Unsubscribe : https://launchpad.net/~kicad-developers

More help   : https://help.launchpad.net/ListHelp





___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Kicad-developers] [PATCH] Rework footprint selection filtering to improve behavior

2018-03-02 Thread Jon Evans
Hi Andrzej,

This was my intention, which is why I said I was prepared for other people
to have other opinions :-)

I think that you should still be able to select footprints even if the
"front pads" is hidden from layers like the paste layer, *unless* you are
in high contrast mode.

-Jon

On Fri, Mar 2, 2018 at 3:53 AM, Andrzej Wolski 
wrote:

> I've tried this patch, and there is a small issue: if you have only eg
> front paste layer enabled and front pads are hidden, footprint is still
> selectable.
>
> Andrzej
>
>
> W dniu 2018-02-27 o 04:11, Jon Evans pisze:
>
> This patch changes the selection logic for footprints to fix a reported
> issue[1] and to make the behavior more logical to me.
>
> I know that correct selection behavior is something of a personal
> preference, so I'm ready to be flamed :-)
>
> The new behavior:
>
> A footprint may be selected if:
> 1) The corresponding "Footprints" switch is on in the Items tab, AND
> 2) One or more of the layers that the footprint draws on is visible
>
> This means that if all of the layers are turned off, footprints are not
> selectable (fixes the bug), but it also means that now footprints can be
> selected if any draw layers are visible (for example, if you have only
> F.Mask enabled, you can select a footprint that has solder mask and is on
> the front layer).
>
> Before anyone complains, this is only if high-contrast mode is turned
> OFF.  When it is on, you can still only select items that *only* exist on
> that layer (to make moving silkscreen around easier, for example)
>
> Even though this adds some more for-loops to selection filtering, I have
> not noticed any performance hits on some selection of large boards that I
> tested.  More testing is welcome.
>
> [1] https://bugs.launchpad.net/kicad/+bug/1751960
>
> -Jon
>
>
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
>
>
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
>
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Kicad-developers] [PATCH] Rework footprint selection filtering to improve behavior

2018-03-02 Thread Andrzej Wolski
I've tried this patch, and there is a small issue: if you have only eg 
front paste layer enabled and front pads are hidden, footprint is still 
selectable.


Andrzej


W dniu 2018-02-27 o 04:11, Jon Evans pisze:
This patch changes the selection logic for footprints to fix a 
reported issue[1] and to make the behavior more logical to me.


I know that correct selection behavior is something of a personal 
preference, so I'm ready to be flamed :-)


The new behavior:

A footprint may be selected if:
1) The corresponding "Footprints" switch is on in the Items tab, AND
2) One or more of the layers that the footprint draws on is visible

This means that if all of the layers are turned off, footprints are 
not selectable (fixes the bug), but it also means that now footprints 
can be selected if any draw layers are visible (for example, if you 
have only F.Mask enabled, you can select a footprint that has solder 
mask and is on the front layer).


Before anyone complains, this is only if high-contrast mode is turned 
OFF.  When it is on, you can still only select items that *only* exist 
on that layer (to make moving silkscreen around easier, for example)


Even though this adds some more for-loops to selection filtering, I 
have not noticed any performance hits on some selection of large 
boards that I tested.  More testing is welcome.


[1] https://bugs.launchpad.net/kicad/+bug/1751960

-Jon


___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp



___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp


[Kicad-developers] [PATCH] Rework footprint selection filtering to improve behavior

2018-02-26 Thread Jon Evans
This patch changes the selection logic for footprints to fix a reported
issue[1] and to make the behavior more logical to me.

I know that correct selection behavior is something of a personal
preference, so I'm ready to be flamed :-)

The new behavior:

A footprint may be selected if:
1) The corresponding "Footprints" switch is on in the Items tab, AND
2) One or more of the layers that the footprint draws on is visible

This means that if all of the layers are turned off, footprints are not
selectable (fixes the bug), but it also means that now footprints can be
selected if any draw layers are visible (for example, if you have only
F.Mask enabled, you can select a footprint that has solder mask and is on
the front layer).

Before anyone complains, this is only if high-contrast mode is turned OFF.
When it is on, you can still only select items that *only* exist on that
layer (to make moving silkscreen around easier, for example)

Even though this adds some more for-loops to selection filtering, I have
not noticed any performance hits on some selection of large boards that I
tested.  More testing is welcome.

[1] https://bugs.launchpad.net/kicad/+bug/1751960

-Jon
From c6346d1042965957ff34baf514e4bea79776eca2 Mon Sep 17 00:00:00 2001
From: Jon Evans 
Date: Mon, 26 Feb 2018 22:06:31 -0500
Subject: [PATCH] Rework footprint selection filtering to improve behavior

Fixes: lp:1751960
* https://bugs.launchpad.net/kicad/+bug/1751960
---
 pcbnew/class_module.cpp | 28 
 pcbnew/class_module.h   |  8 
 pcbnew/tools/selection_tool.cpp | 31 ++-
 3 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/pcbnew/class_module.cpp b/pcbnew/class_module.cpp
index 9968991b4..eed3da407 100644
--- a/pcbnew/class_module.cpp
+++ b/pcbnew/class_module.cpp
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -924,6 +925,33 @@ void MODULE::RunOnChildren( const std::function& aFunction )
 }
 }

+
+void MODULE::GetAllDrawingLayers( int aLayers[], int& aCount ) const
+{
+std::unordered_set layers;
+
+for( BOARD_ITEM* item = m_Drawings; item; item = item->Next() )
+{
+layers.insert( static_cast( item->GetLayer() ) );
+}
+
+for( D_PAD* pad = m_Pads; pad; pad = pad->Next() )
+{
+int pad_layers[KIGFX::VIEW::VIEW_MAX_LAYERS], pad_layers_count;
+pad->ViewGetLayers( pad_layers, pad_layers_count );
+
+for( int i = 0; i < pad_layers_count; i++ )
+layers.insert( pad_layers[i] );
+}
+
+aCount = layers.size();
+int i = 0;
+
+for( auto layer : layers )
+aLayers[i++] = layer;
+}
+
+
 void MODULE::ViewGetLayers( int aLayers[], int& aCount ) const
 {
 aCount = 2;
diff --git a/pcbnew/class_module.h b/pcbnew/class_module.h
index 0af325301..5e06a2c5d 100644
--- a/pcbnew/class_module.h
+++ b/pcbnew/class_module.h
@@ -610,6 +610,14 @@ public:
  */
 void RunOnChildren( const std::function& aFunction );

+/**
+ * Returns a set of all layers that this module has drawings on
+ * similar to ViewGetLayers()
+ *
+ * @param aLayers is an array to store layer ids
+ * @param aCount is the number of layers stored in the array
+ */
+void GetAllDrawingLayers( int aLayers[], int& aCount ) const;

 virtual void ViewGetLayers( int aLayers[], int& aCount ) const override;

diff --git a/pcbnew/tools/selection_tool.cpp b/pcbnew/tools/selection_tool.cpp
index b8eb01f10..7222f6c05 100644
--- a/pcbnew/tools/selection_tool.cpp
+++ b/pcbnew/tools/selection_tool.cpp
@@ -1601,13 +1601,34 @@ bool SELECTION_TOOL::selectable( const BOARD_ITEM* aItem ) const
 if( viewArea > 0.0 && modArea / viewArea > 0.9 )
 return false;

-if( aItem->IsOnLayer( F_Cu ) && board()->IsElementVisible( LAYER_MOD_FR ) )
-return !m_editModules;
+// Allow selection of footprints if at least one draw layer is on and
+// the appropriate LAYER_MOD is on

-if( aItem->IsOnLayer( B_Cu ) && board()->IsElementVisible( LAYER_MOD_BK ) )
-return !m_editModules;
+bool layer_mod = ( ( aItem->IsOnLayer( F_Cu ) && board()->IsElementVisible( LAYER_MOD_FR ) ) ||
+   ( aItem->IsOnLayer( B_Cu ) && board()->IsElementVisible( LAYER_MOD_BK ) ) );

-return false;
+bool draw_layer_visible = false;
+int draw_layers[KIGFX::VIEW::VIEW_MAX_LAYERS], draw_layers_count;
+
+static_cast( aItem )->GetAllDrawingLayers( draw_layers,
+  draw_layers_count );
+
+for( int i = 0; i < draw_layers_count; ++i )
+{
+if( board()->IsLayerVisible( static_cast( draw_layers[i] ) ) )
+draw_layer_visible = true;
+}
+
+// And finally, the pads layers count as draw layers too, if the copper layer is on
+if( (