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 <maciej.sumin...@cern.ch>
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 <j...@craftyjon.com>
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*/ GERBER_DRAW_ITEM* aItem, int aLayer )
 
     color = m_gerbviewSettings.GetColor( aItem, aLayer );
 
+    // TODO: Should brightened color be a preference?
+    if( aItem->IsBrightened() )
+        color = COLOR4D( 0.0, 1.0, 0.0, 0.75 );
+
     if( isNegative )
     {
         if( m_gerbviewSettings.m_showNegativeItems )
diff --git a/gerbview/tools/selection_tool.cpp b/gerbview/tools/selection_tool.cpp
index 9e952f460..efd48a2b8 100644
--- a/gerbview/tools/selection_tool.cpp
+++ b/gerbview/tools/selection_tool.cpp
@@ -636,8 +636,6 @@ EDA_ITEM* GERBVIEW_SELECTION_TOOL::disambiguationMenu( GERBER_COLLECTOR* aCollec
     BRIGHT_BOX brightBox;
     CONTEXT_MENU menu;
 
-    getView()->Add( &brightBox );
-
     int limit = std::min( 10, aCollector->GetCount() );
 
     for( int i = 0; i < limit; ++i )
@@ -657,7 +655,11 @@ EDA_ITEM* GERBVIEW_SELECTION_TOOL::disambiguationMenu( GERBER_COLLECTOR* aCollec
         if( evt->Action() == TA_CONTEXT_MENU_UPDATE )
         {
             if( current )
+            {
                 current->ClearBrightened();
+                getView()->Update( current, KIGFX::LAYERS );
+                getView()->MarkTargetDirty( KIGFX::TARGET_OVERLAY );
+            }
 
             int id = *evt->GetCommandId();
 
@@ -666,6 +668,8 @@ EDA_ITEM* GERBVIEW_SELECTION_TOOL::disambiguationMenu( GERBER_COLLECTOR* aCollec
             {
                 current = ( *aCollector )[id - 1];
                 current->SetBrightened();
+                getView()->Update( current, KIGFX::LAYERS );
+                getView()->MarkTargetDirty( KIGFX::TARGET_OVERLAY );
             }
             else
             {
@@ -684,19 +688,14 @@ EDA_ITEM* GERBVIEW_SELECTION_TOOL::disambiguationMenu( GERBER_COLLECTOR* aCollec
 
             break;
         }
-
-        // Draw a mark to show which item is available to be selected
-        if( current && current->IsBrightened() )
-        {
-            brightBox.SetItem( current );
-            getView()->SetVisible( &brightBox, true );
-//          getView()->Hide( &brightBox, false );
-            getView()->Update( &brightBox, KIGFX::GEOMETRY );
-            getView()->MarkTargetDirty( KIGFX::TARGET_OVERLAY );
-        }
     }
 
-    getView()->Remove( &brightBox );
+    if( current && current->IsBrightened() )
+    {
+        current->ClearBrightened();
+        getView()->Update( current, KIGFX::LAYERS );
+        getView()->MarkTargetDirty( KIGFX::TARGET_OVERLAY );
+    }
 
     return current;
 }
@@ -762,7 +761,7 @@ void GERBVIEW_SELECTION_TOOL::selectVisually( EDA_ITEM* aItem ) const
 {
     // Move the item's layer to the front
     int layer = static_cast<GERBER_DRAW_ITEM*>( aItem )->GetLayer();
-    m_frame->GetGalCanvas()->SetTopLayer( GERBER_DRAW_LAYER( layer ) );
+    m_frame->SetActiveLayer( layer, true );
 
     // Hide the original item, so it is shown only on overlay
     aItem->SetSelected();
-- 
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

Reply via email to