Re: [Kicad-developers] [PATCH] Make BRIGHT_BOX line width dependent on zoom level

[Jon Evans]

I made a quick implementation of this in my GerbView branch and it seems to
work great.
I'm not sure about doing it this way for pcbnew because all the items
override ViewGetLayers -- do you think adding some superclass behavior to
ViewGetLayers would be a good approach, or maybe there is a better way to
temporarily have an item rendered on another layer?

-Jon

On Fri, Sep 15, 2017 at 3:21 PM, Maciej Suminski 
wrote:

> On 09/15/2017 09:09 PM, Jon Evans wrote:
> > Hi Orson,
> >
> > I understand your concern; I will back out this patch and look at other
> > ways of improving the behavior (the current behavior does not work very
> > well at all in GerbView when zoomed in, but I could maybe set another
> > factor for the variable line width for GerbView that would work better).
>
> There could also be a method to toggle enable/disable fixed line width,
> in case it looks better in GerbView.
>
> > Actually in the back of my mind I think it might be nicer to highlight
> the
> > shape of the item itself in the green color rather than its bounding box,
> > but that's another matter...
>
> I thought about the same thing, perhaps it is not a bad idea?
>
> Regards,
> Orson
>
> > -Jon
> >
> > On Fri, Sep 15, 2017 at 3:02 PM, Maciej Suminski <
> maciej.sumin...@cern.ch>
> > wrote:
> >
> >> Hi Jon,
> >>
> >> The implementation is correct, I tested the patch and it works as
> >> advertised, but I am not sure if we really want to switch to fixed line
> >> width for BRIGHT_BOX. Drawing a 10px wide outline looks fine at certain
> >> zoom levels, but the items are covered with the outline when the view is
> >> zoomed out too much. Perhaps a thinner line would work better here. The
> >> original line width (size dependent) has been chosen to work as a well
> >> visible outline for the most common track/pad/via sizes.
> >>
> >> Oliver, just to clarify: the method Jon used works on the same principle
> >> as the one you had proposed. The difference is Jon applied it to an item
> >> that is supposed to be displayed using a non-cached vertex container.
> >> You have proposed a generic function (GAL::SetFixedLineWidth()) which
> >> works correctly for non-cached items, but fails for cached items. This
> >> is the sole reason why I have not applied your patch, as it would add a
> >> method that works only under specific circumstances, whereas it needs to
> >> work in all cases. As you have guessed correctly, there are too many
> >> issues to fix now, so I could not really focus on this task. It is
> >> almost done, but I am stuck with a few visual artifacts I could not fix
> >> easily.
> >>
> >> Regards,
> >> Orson
> >>
> >> On 09/06/2017 02:24 AM, Jon Evans wrote:
> >>> Hi all,
> >>>
> >>> This patch is a quick one to make the line width of the BRIGHT_BOX
> >>> dependent on the zoom level so that it remains basically the same
> >> apparent
> >>> size on the screen.
> >>>
> >>> -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
> >>
> >
>
From d4badc6870a495fbbad3090ce70a137a5ceb2b7c Mon Sep 17 00:00:00 2001
From: Jon Evans 
Date: Sun, 17 Sep 2017 21:54:02 -0400
Subject: [PATCH] Highlight selection candidates instead of using BRIGHT_BOX

---
 gerbview/class_gerber_draw_item.cpp | 14 +++---
 gerbview/gerbview_painter.cpp   |  4 
 gerbview/tools/selection_tool.cpp   | 27 +--
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/gerbview/class_gerber_draw_item.cpp b/gerbview/class_gerber_draw_item.cpp
index 0a9ab84d3..4412fa34f 100644
--- a/gerbview/class_gerber_draw_item.cpp
+++ b/gerbview/class_gerber_draw_item.cpp
@@ -765,9 +765,17 @@ void GERBER_DRAW_ITEM::Show( int nestLevel, std::ostream& os ) const
 
 void GERBER_DRAW_ITEM::ViewGetLayers( int aLayers[], int& aCount ) const
 {
-aCount = 2;
-aLayers[0] = GERBER_DRAW_LAYER( GetLayer() );
-aLayers[1] = GERBER_DCODE_LAYER( aLayers[0] );
+aCount = IsBrightened() ? 1 : 2;
+
+if( IsBrightened() )
+{
+aLayers[0] = LAYER_GP_OVERLAY;
+}
+else
+{
+aLayers[0] = GERBER_DRAW_LAYER( GetLayer() );
+aLayers[1] = GERBER_DCODE_LAYER( aLayers[0] );
+}
 }
 
 
diff --git a/gerbview/gerbview_painter.cpp b/gerbview/gerbview_painter.cpp
index bc95af6b7..0e3dea3dd 100644
--- a/gerbview/gerbview_painter.cpp
+++ b/gerbview/gerbview_painter.cpp
@@ -232,6 +232,10 @@ void GERBVIEW_PAINTER::draw( /*const*/ 

Re: [Kicad-developers] [PATCH] Make BRIGHT_BOX line width dependent on zoom level

[Maciej Suminski]

On 09/15/2017 09:09 PM, Jon Evans wrote:
> Hi Orson,
> 
> I understand your concern; I will back out this patch and look at other
> ways of improving the behavior (the current behavior does not work very
> well at all in GerbView when zoomed in, but I could maybe set another
> factor for the variable line width for GerbView that would work better).

There could also be a method to toggle enable/disable fixed line width,
in case it looks better in GerbView.

> Actually in the back of my mind I think it might be nicer to highlight the
> shape of the item itself in the green color rather than its bounding box,
> but that's another matter...

I thought about the same thing, perhaps it is not a bad idea?

Regards,
Orson

> -Jon
> 
> On Fri, Sep 15, 2017 at 3:02 PM, Maciej Suminski 
> wrote:
> 
>> Hi Jon,
>>
>> The implementation is correct, I tested the patch and it works as
>> advertised, but I am not sure if we really want to switch to fixed line
>> width for BRIGHT_BOX. Drawing a 10px wide outline looks fine at certain
>> zoom levels, but the items are covered with the outline when the view is
>> zoomed out too much. Perhaps a thinner line would work better here. The
>> original line width (size dependent) has been chosen to work as a well
>> visible outline for the most common track/pad/via sizes.
>>
>> Oliver, just to clarify: the method Jon used works on the same principle
>> as the one you had proposed. The difference is Jon applied it to an item
>> that is supposed to be displayed using a non-cached vertex container.
>> You have proposed a generic function (GAL::SetFixedLineWidth()) which
>> works correctly for non-cached items, but fails for cached items. This
>> is the sole reason why I have not applied your patch, as it would add a
>> method that works only under specific circumstances, whereas it needs to
>> work in all cases. As you have guessed correctly, there are too many
>> issues to fix now, so I could not really focus on this task. It is
>> almost done, but I am stuck with a few visual artifacts I could not fix
>> easily.
>>
>> Regards,
>> Orson
>>
>> On 09/06/2017 02:24 AM, Jon Evans wrote:
>>> Hi all,
>>>
>>> This patch is a quick one to make the line width of the BRIGHT_BOX
>>> dependent on the zoom level so that it remains basically the same
>> apparent
>>> size on the screen.
>>>
>>> -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] Make BRIGHT_BOX line width dependent on zoom level

[Jon Evans]

Hi Orson,

I understand your concern; I will back out this patch and look at other
ways of improving the behavior (the current behavior does not work very
well at all in GerbView when zoomed in, but I could maybe set another
factor for the variable line width for GerbView that would work better).

Actually in the back of my mind I think it might be nicer to highlight the
shape of the item itself in the green color rather than its bounding box,
but that's another matter...

-Jon

On Fri, Sep 15, 2017 at 3:02 PM, Maciej Suminski 
wrote:

> Hi Jon,
>
> The implementation is correct, I tested the patch and it works as
> advertised, but I am not sure if we really want to switch to fixed line
> width for BRIGHT_BOX. Drawing a 10px wide outline looks fine at certain
> zoom levels, but the items are covered with the outline when the view is
> zoomed out too much. Perhaps a thinner line would work better here. The
> original line width (size dependent) has been chosen to work as a well
> visible outline for the most common track/pad/via sizes.
>
> Oliver, just to clarify: the method Jon used works on the same principle
> as the one you had proposed. The difference is Jon applied it to an item
> that is supposed to be displayed using a non-cached vertex container.
> You have proposed a generic function (GAL::SetFixedLineWidth()) which
> works correctly for non-cached items, but fails for cached items. This
> is the sole reason why I have not applied your patch, as it would add a
> method that works only under specific circumstances, whereas it needs to
> work in all cases. As you have guessed correctly, there are too many
> issues to fix now, so I could not really focus on this task. It is
> almost done, but I am stuck with a few visual artifacts I could not fix
> easily.
>
> Regards,
> Orson
>
> On 09/06/2017 02:24 AM, Jon Evans wrote:
> > Hi all,
> >
> > This patch is a quick one to make the line width of the BRIGHT_BOX
> > dependent on the zoom level so that it remains basically the same
> apparent
> > size on the screen.
> >
> > -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] Make BRIGHT_BOX line width dependent on zoom level

[Maciej Suminski]

Hi Jon,

The implementation is correct, I tested the patch and it works as
advertised, but I am not sure if we really want to switch to fixed line
width for BRIGHT_BOX. Drawing a 10px wide outline looks fine at certain
zoom levels, but the items are covered with the outline when the view is
zoomed out too much. Perhaps a thinner line would work better here. The
original line width (size dependent) has been chosen to work as a well
visible outline for the most common track/pad/via sizes.

Oliver, just to clarify: the method Jon used works on the same principle
as the one you had proposed. The difference is Jon applied it to an item
that is supposed to be displayed using a non-cached vertex container.
You have proposed a generic function (GAL::SetFixedLineWidth()) which
works correctly for non-cached items, but fails for cached items. This
is the sole reason why I have not applied your patch, as it would add a
method that works only under specific circumstances, whereas it needs to
work in all cases. As you have guessed correctly, there are too many
issues to fix now, so I could not really focus on this task. It is
almost done, but I am stuck with a few visual artifacts I could not fix
easily.

Regards,
Orson

On 09/06/2017 02:24 AM, Jon Evans wrote:
> Hi all,
> 
> This patch is a quick one to make the line width of the BRIGHT_BOX
> dependent on the zoom level so that it remains basically the same apparent
> size on the screen.
> 
> -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] Make BRIGHT_BOX line width dependent on zoom level

[Jon Evans]

Hi Orson,

Could you please review this and advise when you have a chance?
If there is some problem with it I can back this out of my branch.
This is the only open patch before I start sending Gerbview patches.

Thanks,
Jon

On Tue, Sep 5, 2017 at 10:03 PM, Oliver Walters <
oliver.henry.walt...@gmail.com> wrote:

> Jon,
>
> I submitted a very similar patch earlier this year, and Orson raised an
> issue, saying that it would cause issues with cached targets on OpenGl?
>
> I don't know much about this particular issue but it would be good to get
> his sign off on this patch.
>
> IIRC he said he would look into it when he had time. Perhaps he has not
> had time ;)
>
> Something to consider.
>
> Cheers,
> Oliver
>
> On 6 Sep 2017 10:25, "Jon Evans"  wrote:
>
>> Hi all,
>>
>> This patch is a quick one to make the line width of the BRIGHT_BOX
>> dependent on the zoom level so that it remains basically the same apparent
>> size on the screen.
>>
>> -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] Make BRIGHT_BOX line width dependent on zoom level

[Jon Evans]

Bump

On Tue, Sep 5, 2017 at 10:21 PM, Jon Evans  wrote:

> OK, will wait to hear from Orson then.  I did test and it seems to do what
> I want on OpenGL + Cairo
>
> -Jon
>
> On Tue, Sep 5, 2017 at 10:03 PM, Oliver Walters <
> oliver.henry.walt...@gmail.com> wrote:
>
>> Jon,
>>
>> I submitted a very similar patch earlier this year, and Orson raised an
>> issue, saying that it would cause issues with cached targets on OpenGl?
>>
>> I don't know much about this particular issue but it would be good to get
>> his sign off on this patch.
>>
>> IIRC he said he would look into it when he had time. Perhaps he has not
>> had time ;)
>>
>> Something to consider.
>>
>> Cheers,
>> Oliver
>>
>> On 6 Sep 2017 10:25, "Jon Evans"  wrote:
>>
>>> Hi all,
>>>
>>> This patch is a quick one to make the line width of the BRIGHT_BOX
>>> dependent on the zoom level so that it remains basically the same apparent
>>> size on the screen.
>>>
>>> -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] Make BRIGHT_BOX line width dependent on zoom level

[Jon Evans]

OK, will wait to hear from Orson then.  I did test and it seems to do what
I want on OpenGL + Cairo

-Jon

On Tue, Sep 5, 2017 at 10:03 PM, Oliver Walters <
oliver.henry.walt...@gmail.com> wrote:

> Jon,
>
> I submitted a very similar patch earlier this year, and Orson raised an
> issue, saying that it would cause issues with cached targets on OpenGl?
>
> I don't know much about this particular issue but it would be good to get
> his sign off on this patch.
>
> IIRC he said he would look into it when he had time. Perhaps he has not
> had time ;)
>
> Something to consider.
>
> Cheers,
> Oliver
>
> On 6 Sep 2017 10:25, "Jon Evans"  wrote:
>
>> Hi all,
>>
>> This patch is a quick one to make the line width of the BRIGHT_BOX
>> dependent on the zoom level so that it remains basically the same apparent
>> size on the screen.
>>
>> -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] Make BRIGHT_BOX line width dependent on zoom level

[Oliver Walters]

Jon,

I submitted a very similar patch earlier this year, and Orson raised an
issue, saying that it would cause issues with cached targets on OpenGl?

I don't know much about this particular issue but it would be good to get
his sign off on this patch.

IIRC he said he would look into it when he had time. Perhaps he has not had
time ;)

Something to consider.

Cheers,
Oliver

On 6 Sep 2017 10:25, "Jon Evans"  wrote:

> Hi all,
>
> This patch is a quick one to make the line width of the BRIGHT_BOX
> dependent on the zoom level so that it remains basically the same apparent
> size on the screen.
>
> -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] Make BRIGHT_BOX line width dependent on zoom level

[Jon Evans]

Hi all,

This patch is a quick one to make the line width of the BRIGHT_BOX
dependent on the zoom level so that it remains basically the same apparent
size on the screen.

-Jon
From fe5c8f7879c8c74ad67ea2c45a1945a9692150e7 Mon Sep 17 00:00:00 2001
From: Jon Evans 
Date: Tue, 5 Sep 2017 20:22:42 -0400
Subject: [PATCH] Make BRIGHT_BOX line width dependent on zoom level

---
 common/preview_items/bright_box.cpp | 5 +++--
 pcbnew/tools/pcb_bright_box.cpp | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/common/preview_items/bright_box.cpp b/common/preview_items/bright_box.cpp
index cc55dfa37..18e87b79c 100644
--- a/common/preview_items/bright_box.cpp
+++ b/common/preview_items/bright_box.cpp
@@ -28,7 +28,7 @@
 
 using namespace KIGFX;
 
-const double BRIGHT_BOX::LINE_WIDTH = 1.0;
+const double BRIGHT_BOX::LINE_WIDTH = 10.0;
 const COLOR4D BRIGHT_BOX::BOX_COLOR = KIGFX::COLOR4D( 0.0, 1.0, 0.0, 1.0 );
 
 BRIGHT_BOX::BRIGHT_BOX() :
@@ -49,7 +49,8 @@ void BRIGHT_BOX::ViewDraw( int aLayer, KIGFX::VIEW* aView ) const
 
 gal->SetIsStroke( true );
 gal->SetIsFill( false );
-gal->SetLineWidth( m_lineWidth );
+double scale = gal->GetWorldScale();
+gal->SetLineWidth( m_lineWidth / scale );
 gal->SetStrokeColor( m_color );
 
 BOX2I box = m_item->ViewBBox();
diff --git a/pcbnew/tools/pcb_bright_box.cpp b/pcbnew/tools/pcb_bright_box.cpp
index aee6462cb..b9e1d254b 100644
--- a/pcbnew/tools/pcb_bright_box.cpp
+++ b/pcbnew/tools/pcb_bright_box.cpp
@@ -27,7 +27,7 @@
 
 using namespace KIGFX;
 
-const double PCB_BRIGHT_BOX::PCB_LINE_WIDTH = 10.0;
+const double PCB_BRIGHT_BOX::PCB_LINE_WIDTH = 10.0;
 
 
 PCB_BRIGHT_BOX::PCB_BRIGHT_BOX() :
@@ -50,7 +50,8 @@ void PCB_BRIGHT_BOX::ViewDraw( int aLayer, KIGFX::VIEW* aView ) const
 
 gal->SetIsStroke( true );
 gal->SetIsFill( false );
-gal->SetLineWidth( m_lineWidth );
+double scale = gal->GetWorldScale();
+gal->SetLineWidth( m_lineWidth / scale );
 gal->SetStrokeColor( m_color );
 
 gal->DrawSegment( track->GetStart(), track->GetEnd(), track->GetWidth() );
-- 
2.11.0

___
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