kicad-developers team mailing list archive

[Element Green <jgreen@xxxxxxxxxxxxxxxxxxxxx>]

On Sun, Mar 11, 2012 at 1:55 PM, Dick Hollenbeck <dick@xxxxxxxxxxx> wrote:

> On 03/11/2012 03:51 PM, Dick Hollenbeck wrote:
> > The inheritance tree from GERBVIEW_LAYER_WIDGET goes up to LAYER_WIDGET
> and over to
> > PCB_LAYER_WIDGET.  It may be necessary to break off
> GERBVIEW_LAYER_WIDGET so as to not
> > negatively impact PCB_LAYER_WIDGET.  There were several objectives I had
> in writing
> > LAYER_WIDGET and PCB_LAYER_WIDGET, that I would not want to lose at this
> point.  Just to
> > name one:
> >
> > *) at most two clicks away from controlling anything relative to
> visibles.  This means
> > scroll bars would bother me, since I don't want to have to "look for" or
> reposition  anything.
> >
> > (My concerns are less with GERBVIEW_LAYER_WIDGET than PCB_LAYER_WIDGET,
> so I would not
> > easily embrace an attempted move to a list control within
> PCB_LAYER_WIDGET.)
> >
> > Exploring a path where you keep the existing base class, these
> considerations come into play:
> >
> > I think you have toggling of layer visibility now, range selection no,
> but single items yes.
> >
> > Other tips regarding only GERBVIEW_LAYER_WIDGET which do not entail
> changing base class:
> >
> > *) Much can be done with the right mouse popup menu that gets added in
> the derived
> > classes, below LAYER_WIDGET.
> >   a) a "move up" and "move down" choice for re-ordering.
> >   b) Unload this gerber file.
> >   c) group visibility toggling based on file extension, which is a cheap
> way to get your 4)
> >
> > You would have to add mouse hit testing for specific row choices.  But
> much is doable with
> > the right mouse popup menu idea.
> >
> >
> > *) Could keep the read only LAYER_WIDGET::ROW items at the LAYER_WIDGET
> level in a private
> > internal member list to ease re-ordering.
> >
> >
> > Regarding GERBVIEW itself:
> >
> > *) I had started down the path of keeping a separate display list for
> each gerber file.
> > This is fundamentally different than PCBNEW where all graphical items
> are on one display
> > list accessed via GetBoard(), and earmarked with a specific layer number.
> >
> > What a separate display list for each gerber file would give you is the
> ability to easily
> > unload any particular gerber file.
>
> And it allows you to easily alter the drawing order of the various layers,
> providing you
> keep this order in sync with the GERBIEW_LAYER_WIDGET.
>
>
> >  Before I could implement this, I got distracted some
> > 2-3 years ago.  The only disadvantage I can see is if you are going to
> support rich mouse
> > hit testing, then you have to search all the lists separately.
> >
> >
> > But I think that sounds like a for loop to me, and we have a very nice
> COLLECTOR support
> > which can and should be used to gather up items found during list
> walking.
> >
> >
> > However, mouse hit testing is mostly useful if you want to do something
> with the selected
> > item, and we are not talking about a full blown editor here.  But I
> guess you are
> > concerned about displaying something about the selected item.  Full
> blown hit testing
> > would involve the dis-ambiguating menus that we have in PCBNEW and
> EESCHEMA.
>
>
Thanks for the information on the layers browser.  As you described, I see
how a lot of the layer functionality in my wish list could indeed be
implemented with the current widget.  I'll shoot for adding desired
functionality to the existing objects.

I made an honest effort to add location and size information for basic
shapes.  In the process of testing it out, I experienced some of what
Jean-Pierre was hinting at.  That some shapes can be more complex and
determining what size the user wants to see can be difficult.  For example,
I clicked on a circular spot pad on a ground pour and got the size of the
pad plus a ring around it where the thermal traces connect it.  This could
give the user unexpected results, since the area around the pad, wouldn't
normally be thought of as being part of the size of the pad shape.  For
other layers which don't contain a pour, it is very useful though for
determining size and location of circles, spots, lines, ovals, etc.  Seems
like resolving issues like that would require a good overview of Gerber
geometric primitives and what would be considered as visually part of a
shape and what is not, something which I currently do not have.

One other thing I noticed, was that shapes on non visible layers still show
up when clicking on the Gerber view display.  It seems like non visible
layers should not be included in the target hit search.  Would you guys
agree?

I've attached a patch to this email for comment.  It would be nice to see
something like this merged into the official source code (so I don't keep
having to patch my local copy ;-)  I admit though, it is not perfect.  Some
additional notes about this patch:  I changed the "Graphic Layer"
information label to just "Layer" to free up some space on the status area.
 I also wasn't sure the best way to add the two support functions
DisplayPosition and DisplayValue, which are really local to
GERBER_DRAW_ITEM and probably not so useful outside of the DisplayInfo
method.  Currently they are part of the public interface.

Please let me know if I should be utilizing some other subsystem (like the
bug tracker) for getting feedback on this patch.

Best regards,
Element Green

=== modified file 'gerbview/class_gerber_draw_item.cpp'
--- gerbview/class_gerber_draw_item.cpp	2012-01-23 04:33:36 +0000
+++ gerbview/class_gerber_draw_item.cpp	2012-03-12 16:53:14 +0000
@@ -562,7 +562,39 @@
 
     // Display graphic layer number
     msg.Printf( wxT( "%d" ), GetLayer() + 1 );
-    frame->AppendMsgPanel( _( "Graphic layer" ), msg, BROWN );
+    frame->AppendMsgPanel( _( "Layer" ), msg, BROWN );
+
+    switch( m_Shape )
+    {
+    case GBR_SEGMENT:
+      DisplayPosition( frame, _( "Start" ), m_Start );
+      DisplayPosition( frame, _( "End" ), m_End );
+      DisplayValue( frame, _("Width" ), m_Size.x );
+      break;
+    case GBR_ARC:
+      DisplayPosition( frame, _( "Start" ), m_Start );
+      DisplayPosition( frame, _( "End" ), m_End );
+      DisplayPosition( frame, _( "Center" ), m_ArcCentre );
+      DisplayValue( frame, _("Width" ), m_Size.x );
+      break;
+    case GBR_CIRCLE:
+    case GBR_SPOT_CIRCLE:
+      DisplayPosition( frame, _( "Center" ), m_Start );
+      DisplayValue( frame, _("Size" ), m_Size.x );
+      break;
+    case GBR_SPOT_OVAL:
+    case GBR_SPOT_RECT:
+      DisplayPosition( frame, _( "Center" ), m_Start );
+      DisplayValue( frame, _("Width" ), m_Size.x );
+      DisplayValue( frame, _("Height" ), m_Size.y );
+      break;
+    case GBR_SPOT_POLY:
+      break;
+    case GBR_POLYGON:
+      break;
+    case GBR_SPOT_MACRO:
+      break;
+    }
 
     // Display item rotation
     // The full rotation is Image rotation + m_lyrRotation
@@ -587,6 +619,33 @@
 }
 
 
+void GERBER_DRAW_ITEM::DisplayPosition( EDA_DRAW_FRAME* frame,
+                                        const wxString& label,
+                                        const wxPoint& pos )
+{
+    wxString msg;
+    wxPoint absPos;
+
+    absPos = GetABPosition (pos);
+    msg = wxT( "X=" );
+    msg += ReturnStringFromValue( g_UserUnit, absPos.x, PCB_INTERNAL_UNIT, false );
+    msg += wxT( " Y=" );
+    msg += ReturnStringFromValue( g_UserUnit, absPos.y, PCB_INTERNAL_UNIT, false );
+    frame->AppendMsgPanel( label, msg, BLUE );
+}
+
+
+void GERBER_DRAW_ITEM::DisplayValue( EDA_DRAW_FRAME* frame,
+                                     const wxString& label,
+                                     int value )
+{
+    wxString msg;
+
+    msg = ReturnStringFromValue( g_UserUnit, value, PCB_INTERNAL_UNIT, false );
+    frame->AppendMsgPanel( label, msg, BLUE );
+}
+
+
 bool GERBER_DRAW_ITEM::HitTest( const wxPoint& aRefPos )
 {
     // calculate aRefPos in XY gerber axis:

=== modified file 'gerbview/class_gerber_draw_item.h'
--- gerbview/class_gerber_draw_item.h	2012-02-20 04:33:54 +0000
+++ gerbview/class_gerber_draw_item.h	2012-03-12 08:41:02 +0000
@@ -234,6 +234,14 @@
      */
     void DisplayInfo( EDA_DRAW_FRAME* frame );
 
+    void DisplayPosition( EDA_DRAW_FRAME* frame,
+                          const wxString& label,
+                          const wxPoint& pos );
+
+    void DisplayValue( EDA_DRAW_FRAME* frame,
+                       const wxString& label,
+                       int value );
+
     wxString ShowGBRShape();
 
     /**