From e609bc3f0db6eb158e464a061f0735f1e92e1c15 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@gmail.com>
Date: Tue, 21 Feb 2017 19:09:10 +0800
Subject: [PATCH 1/5] Use wxStaticBitmaps for layer/render swatches

On Linux with recent GTK+ toolkits, these swatches had very wide
borders, resulting in colour swatches of only a few pixels. On OSX, all
borders were disabled, leaving only the swatch.

This commit changes the wxBitmapButton to wxStaticBitmap, which fixes
the issue on Linux GTK+ and also removes the platform-specific element,
as it will now be the same on all systems.

These widgets aren't used as buttons anyway, a single click is handled
across the whole row, not just the swatch. So using a button was not the
most intuitive affordance.

Fixes: lp:1605411
* https://bugs.launchpad.net/kicad/+bug/1605411
---
 pcbnew/layer_widget.cpp | 106 ++++++++++++++++++++--------------------
 pcbnew/layer_widget.h   |   8 ---
 2 files changed, 53 insertions(+), 61 deletions(-)

diff --git a/pcbnew/layer_widget.cpp b/pcbnew/layer_widget.cpp
index 5eb11b047d..d63c3f9141 100644
--- a/pcbnew/layer_widget.cpp
+++ b/pcbnew/layer_widget.cpp
@@ -26,7 +26,7 @@
 
 
 /*  This source module implements the layer visibility and selection widget
-    @todo make the bitmapbutton a staticbitmap, and make dependent on the point size.
+    @todo make bitmap size dependent on the point size.
 */
 
 
@@ -43,9 +43,8 @@
 #include <algorithm>
 
 
-#define BUTT_SIZE_X             20
-#define BUTT_SIZE_Y             18
-#define BUTT_VOID               2
+const static int SWATCH_SIZE_X = 14;
+const static int SWATCH_SIZE_Y = 12;
 
 // See selcolor.cpp:
 extern COLOR4D DisplayColorFrame( wxWindow* aParent, COLOR4D aOldColor );
@@ -161,6 +160,45 @@ static wxString makeColorTxt( COLOR4D aColor )
 }
 
 
+static wxBitmap makeBitmap( COLOR4D aColor )
+{
+    wxBitmap    bitmap( SWATCH_SIZE_X, SWATCH_SIZE_Y );
+    wxBrush     brush;
+    wxMemoryDC  iconDC;
+
+    iconDC.SelectObject( bitmap );
+
+    brush.SetColour( aColor.ToColour() );
+    brush.SetStyle( wxBRUSHSTYLE_SOLID );
+
+    iconDC.SetBrush( brush );
+
+    iconDC.DrawRectangle( 0, 0, SWATCH_SIZE_X, SWATCH_SIZE_Y );
+
+    return bitmap;
+}
+
+
+/**
+ * Function makeColorButton
+ * creates a wxStaticBitmap and assigns it a solid color and a control ID
+ */
+static std::unique_ptr<wxStaticBitmap> makeColorSwatch( wxWindow* aParent,
+                                                        COLOR4D aColor, int aID )
+{
+    // dynamically make a wxBitMap and brush it with the appropriate color,
+    // then create a wxBitmapButton from it.
+    wxBitmap bitmap = makeBitmap( aColor );
+
+    // save the color value in the name, no where else to put it.
+    auto ret = std::make_unique<wxStaticBitmap>( aParent, aID, bitmap );
+
+    ret->SetName( makeColorTxt( aColor ) );
+
+    return ret;
+}
+
+
 /**
  * Function shrinkFont
  * reduces the size of the wxFont associated with \a aControl
@@ -187,46 +225,6 @@ LAYER_NUM LAYER_WIDGET::getDecodedId( int aControlId )
 }
 
 
-wxBitmap LAYER_WIDGET::makeBitmap( COLOR4D aColor )
-{
-    // the bitmap will be BUTT_VOID*2 pixels smaller than the button, leaving a
-    // border of BUTT_VOID pixels on each side.
-    wxBitmap    bitmap( BUTT_SIZE_X - 2 * BUTT_VOID, BUTT_SIZE_Y - 2 * BUTT_VOID );
-    wxBrush     brush;
-    wxMemoryDC  iconDC;
-
-    iconDC.SelectObject( bitmap );
-
-    brush.SetColour( aColor.ToColour() );
-    brush.SetStyle( wxBRUSHSTYLE_SOLID );
-
-    iconDC.SetBrush( brush );
-
-    iconDC.DrawRectangle( 0, 0, BUTT_SIZE_X - 2 * BUTT_VOID, BUTT_SIZE_Y - 2 * BUTT_VOID );
-
-    return bitmap;
-}
-
-
-wxBitmapButton* LAYER_WIDGET::makeColorButton( wxWindow* aParent, COLOR4D aColor, int aID )
-{
-    // dynamically make a wxBitMap and brush it with the appropriate color,
-    // then create a wxBitmapButton from it.
-    wxBitmap bitmap = makeBitmap( aColor );
-
-#ifdef __WXMAC__
-    wxBitmapButton* ret = new wxBitmapButton( aParent, aID, bitmap,
-        wxDefaultPosition, wxSize(BUTT_SIZE_X, BUTT_SIZE_Y), wxBORDER_NONE );
-#else
-    wxBitmapButton* ret = new wxBitmapButton( aParent, aID, bitmap,
-        wxDefaultPosition, wxSize(BUTT_SIZE_X, BUTT_SIZE_Y), wxBORDER_RAISED );
-#endif
-    // save the color value in the name, no where else to put it.
-    ret->SetName( makeColorTxt( aColor ) );
-    return ret;
-}
-
-
 void LAYER_WIDGET::OnLeftDownLayers( wxMouseEvent& event )
 {
     int row;
@@ -277,7 +275,7 @@ void LAYER_WIDGET::OnLeftDownLayers( wxMouseEvent& event )
 
 void LAYER_WIDGET::OnMiddleDownLayerColor( wxMouseEvent& aEvent )
 {
-    wxBitmapButton* eventSource = (wxBitmapButton*) aEvent.GetEventObject();
+    auto eventSource = static_cast<wxStaticBitmap*>( aEvent.GetEventObject() );
 
     wxString colorTxt = eventSource->GetName();
 
@@ -306,7 +304,7 @@ void LAYER_WIDGET::OnMiddleDownLayerColor( wxMouseEvent& aEvent )
         eventSource->SetName( makeColorTxt( newColor ) );
 
         wxBitmap bm = makeBitmap( newColor.ToColour() );
-        eventSource->SetBitmapLabel( bm );
+        eventSource->SetBitmap( bm );
 
         LAYER_NUM layer = getDecodedId( eventSource->GetId() );
 
@@ -331,9 +329,9 @@ void LAYER_WIDGET::OnLayerCheckBox( wxCommandEvent& event )
 }
 
 
-void LAYER_WIDGET::OnMiddleDownRenderColor( wxMouseEvent& event )
+void LAYER_WIDGET::OnMiddleDownRenderColor( wxMouseEvent& aEvent )
 {
-    wxBitmapButton* eventSource = (wxBitmapButton*) event.GetEventObject();
+    auto eventSource = static_cast<wxStaticBitmap*>( aEvent.GetEventObject() );
 
     wxString colorTxt = eventSource->GetName();
 
@@ -362,7 +360,7 @@ void LAYER_WIDGET::OnMiddleDownRenderColor( wxMouseEvent& event )
         eventSource->SetName( makeColorTxt( newColor ) );
 
         wxBitmap bm = makeBitmap( newColor );
-        eventSource->SetBitmapLabel( bm );
+        eventSource->SetBitmap( bm );
 
         LAYER_NUM id = getDecodedId( eventSource->GetId() );
 
@@ -460,12 +458,12 @@ void LAYER_WIDGET::insertLayerRow( int aRow, const ROW& aSpec )
     // column 1 (COLUMN_COLORBM)
     col = COLUMN_COLORBM;
 
-    wxBitmapButton* bmb = makeColorButton( m_LayerScrolledWindow, aSpec.color, encodeId( col, aSpec.id ) );
+    auto bmb = makeColorSwatch( m_LayerScrolledWindow, aSpec.color, encodeId( col, aSpec.id ) );
     bmb->Connect( wxEVT_LEFT_DOWN, wxMouseEventHandler( LAYER_WIDGET::OnLeftDownLayers ), NULL, this );
     bmb->Connect( wxEVT_LEFT_DCLICK, wxMouseEventHandler( LAYER_WIDGET::OnMiddleDownLayerColor ), NULL, this );
     bmb->Connect( wxEVT_MIDDLE_DOWN, wxMouseEventHandler( LAYER_WIDGET::OnMiddleDownLayerColor ), NULL, this );
     bmb->SetToolTip( _("Left double click or middle click for color change, right click for menu" ) );
-    m_LayersFlexGridSizer->wxSizer::Insert( index+col, bmb, 0, flags );
+    m_LayersFlexGridSizer->wxSizer::Insert( index+col, bmb.release(), 0, flags );
 
     // column 2 (COLUMN_COLOR_LYR_CB)
     col = COLUMN_COLOR_LYR_CB;
@@ -497,11 +495,13 @@ void LAYER_WIDGET::insertRenderRow( int aRow, const ROW& aSpec )
     col = 0;
     if( aSpec.color != COLOR4D::UNSPECIFIED )
     {
-        wxBitmapButton* bmb = makeColorButton( m_RenderScrolledWindow, aSpec.color, encodeId( col, aSpec.id ) );
+        auto bmb = makeColorSwatch(m_RenderScrolledWindow, aSpec.color, encodeId( col, aSpec.id ) );
+
+        //makeColorSwatch( m_RenderScrolledWindow, aSpec.color, encodeId( col, aSpec.id ) );
         bmb->Connect( wxEVT_LEFT_DCLICK, wxMouseEventHandler( LAYER_WIDGET::OnMiddleDownRenderColor ), NULL, this );
         bmb->Connect( wxEVT_MIDDLE_DOWN, wxMouseEventHandler( LAYER_WIDGET::OnMiddleDownRenderColor ), NULL, this );
         bmb->SetToolTip( _( "Left double click or middle click for color change" ) );
-        m_RenderFlexGridSizer->wxSizer::Insert( index+col, bmb, 0, flags );
+        m_RenderFlexGridSizer->wxSizer::Insert( index+col, bmb.release(), 0, flags );
 
         // could add a left click handler on the color button that toggles checkbox.
     }
diff --git a/pcbnew/layer_widget.h b/pcbnew/layer_widget.h
index 23624e340a..892de638d2 100644
--- a/pcbnew/layer_widget.h
+++ b/pcbnew/layer_widget.h
@@ -121,8 +121,6 @@ protected:
     int                 m_CurrentRow;           ///< selected row of layer list
     int                 m_PointSize;
 
-    static wxBitmap makeBitmap( COLOR4D aColor );
-
     /**
      * Virtual Function useAlternateBitmap
      * @return true if bitmaps shown in Render layer list
@@ -156,12 +154,6 @@ protected:
      */
     static LAYER_NUM getDecodedId( int aControlId );
 
-    /**
-     * Function makeColorButton
-     * creates a wxBitmapButton and assigns it a solid color and a control ID
-     */
-    wxBitmapButton* makeColorButton( wxWindow* aParent, COLOR4D aColor, int aID );
-
     void OnLeftDownLayers( wxMouseEvent& event );
 
     /**

From b8d0b0d7beb934b06b7bfdcf6147998307280d22 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@gmail.com>
Date: Tue, 21 Feb 2017 20:31:19 +0800
Subject: [PATCH 2/5] Move layer/render swatches to own class

This introduces COLOR_SWATCH, which is a reusable
widget that shows a color swatch and can invoke the colour picker
when duble/middle clicked.

It uses it's own wxCommandEvent to signal the change.

This makes the layer widget simpler internally, and also allows other
code to show identical swatches if needed.
---
 common/CMakeLists.txt           |   1 +
 common/widgets/color_swatch.cpp | 174 ++++++++++++++++++++++++++++++++
 include/widgets/color_swatch.h  |  92 +++++++++++++++++
 pcbnew/layer_widget.cpp         | 172 +++++--------------------------
 pcbnew/layer_widget.h           |  12 ++-
 5 files changed, 303 insertions(+), 148 deletions(-)
 create mode 100644 common/widgets/color_swatch.cpp
 create mode 100644 include/widgets/color_swatch.h

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index 2199f07497..df08770f15 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -165,6 +165,7 @@ set( COMMON_DLG_SRCS
     )
 
 set( COMMON_WIDGET_SRCS
+    widgets/color_swatch.cpp
     widgets/mathplot.cpp
     widgets/widget_hotkey_list.cpp
     widgets/two_column_tree_list.cpp
diff --git a/common/widgets/color_swatch.cpp b/common/widgets/color_swatch.cpp
new file mode 100644
index 0000000000..745537d4ba
--- /dev/null
+++ b/common/widgets/color_swatch.cpp
@@ -0,0 +1,174 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+
+ * Copyright (C) 2017 KiCad Developers, see AUTHORS.TXT for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#include <widgets/color_swatch.h>
+
+#include <wx/colour.h>
+#include <wx/colordlg.h>
+
+wxDEFINE_EVENT(COLOR_SWATCH_CHANGED, wxCommandEvent);
+
+using KIGFX::COLOR4D;
+
+
+const static int SWATCH_SIZE_X = 14;
+const static int SWATCH_SIZE_Y = 12;
+
+// See selcolor.cpp:
+extern COLOR4D DisplayColorFrame( wxWindow* aParent, COLOR4D aOldColor );
+
+
+/**
+ * Make a simple color swatch bitmap
+ */
+static wxBitmap makeBitmap( COLOR4D aColor )
+{
+    wxBitmap    bitmap( SWATCH_SIZE_X, SWATCH_SIZE_Y );
+    wxBrush     brush;
+    wxMemoryDC  iconDC;
+
+    iconDC.SelectObject( bitmap );
+
+    brush.SetColour( aColor.ToColour() );
+    brush.SetStyle( wxBRUSHSTYLE_SOLID );
+
+    iconDC.SetBrush( brush );
+
+    iconDC.DrawRectangle( 0, 0, SWATCH_SIZE_X, SWATCH_SIZE_Y );
+
+    return bitmap;
+}
+
+
+/**
+ * Function makeColorButton
+ * creates a wxStaticBitmap and assigns it a solid color and a control ID
+ */
+static std::unique_ptr<wxStaticBitmap> makeColorSwatch(
+        wxWindow* aParent, COLOR4D aColor, int aID )
+{
+    // construct a bitmap of the right color and make the swatch from it
+    wxBitmap bitmap = makeBitmap( aColor );
+    auto ret = std::make_unique<wxStaticBitmap>( aParent, aID, bitmap );
+
+    return ret;
+}
+
+
+COLOR_SWATCH::COLOR_SWATCH( wxWindow* aParent, COLOR4D aColor, int aID,
+                            bool aArbitraryColors ):
+        wxPanel( aParent, aID ),
+        m_arbitraryColors( aArbitraryColors ),
+        m_color( aColor )
+{
+    auto sizer = new wxBoxSizer( wxHORIZONTAL );
+    SetSizer( sizer );
+
+    auto swatch = makeColorSwatch( this, m_color, aID );
+    m_swatch = swatch.get(); // hold a handle
+
+    sizer->Add( swatch.release(), 0, 0 );
+
+    // forward click to any other listeners, since we don't want them
+    m_swatch->Bind( wxEVT_LEFT_DOWN, &COLOR_SWATCH::rePostEvent, this );
+    m_swatch->Bind( wxEVT_RIGHT_DOWN, &COLOR_SWATCH::rePostEvent, this );
+
+    // bind the events that trigger the dialog
+    m_swatch->Bind( wxEVT_LEFT_DCLICK, [this] ( wxMouseEvent& aEvt ) {
+        GetNewSwatchColor();
+    } );
+
+    m_swatch->Bind( wxEVT_MIDDLE_DOWN, [this] ( wxMouseEvent& aEvt ) {
+        GetNewSwatchColor();
+    } );
+}
+
+
+void COLOR_SWATCH::rePostEvent( wxEvent& aEvt )
+{
+    wxPostEvent( this, aEvt );
+}
+
+
+static void sendSwatchChangeEvent( COLOR_SWATCH& aSender )
+{
+    wxCommandEvent changeEvt( COLOR_SWATCH_CHANGED );
+
+    // use this class as the object (alternative might be to
+    // set a custom event class but that's more work)
+    changeEvt.SetEventObject( &aSender );
+
+    wxPostEvent( &aSender, changeEvt );
+}
+
+
+void COLOR_SWATCH::SetSwatchColor( COLOR4D aColor, bool sendEvent )
+{
+    m_color = aColor;
+
+    wxBitmap bm = makeBitmap( aColor );
+    m_swatch->SetBitmap( bm );
+
+    if( sendEvent )
+    {
+        sendSwatchChangeEvent( *this );
+    }
+}
+
+
+COLOR4D COLOR_SWATCH::GetSwatchColor() const
+{
+    return m_color;
+}
+
+
+void COLOR_SWATCH::GetNewSwatchColor()
+{
+    COLOR4D newColor = COLOR4D::UNSPECIFIED;
+
+    if( m_arbitraryColors )
+    {
+        wxColourData colourData;
+        colourData.SetColour( m_color.ToColour() );
+        wxColourDialog* dialog = new wxColourDialog( this, &colourData );
+
+        if( dialog->ShowModal() == wxID_OK )
+        {
+            newColor = COLOR4D( dialog->GetColourData().GetColour() );
+        }
+    }
+    else
+    {
+        newColor = DisplayColorFrame( this, m_color );
+    }
+
+    if( newColor != COLOR4D::UNSPECIFIED )
+    {
+        m_color = newColor;
+
+        wxBitmap bm = makeBitmap( newColor );
+        m_swatch->SetBitmap( bm );
+
+        sendSwatchChangeEvent( *this );
+    }
+}
diff --git a/include/widgets/color_swatch.h b/include/widgets/color_swatch.h
new file mode 100644
index 0000000000..910f84b09d
--- /dev/null
+++ b/include/widgets/color_swatch.h
@@ -0,0 +1,92 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2017 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#ifndef COLOR_SWATCH__H_
+#define COLOR_SWATCH__H_
+
+#include <wx/wx.h>
+
+#include <common.h>
+
+#include <gal/color4d.h>
+
+
+/**
+ * Class representing a simple color swatch, of the kind used to
+ * set layer colors
+ */
+class COLOR_SWATCH: public wxPanel
+{
+public:
+
+    /**
+     * Construct a COLOR_SWATCH
+     *
+     * @param aParent parent window
+     * @param aColor initial swatch color
+     * @param aID id to use when sending swatch events
+     */
+    COLOR_SWATCH( wxWindow* aParent, KIGFX::COLOR4D aColor, int aID,
+                  bool aArbitraryColors );
+
+    /**
+     * Set the current swatch color directly.
+     */
+    void SetSwatchColor( KIGFX::COLOR4D aColor, bool sendEvent );
+
+    /**
+     * @return the current swatch color
+     */
+    KIGFX::COLOR4D GetSwatchColor() const;
+
+    /**
+     * Prompt for a new colour, using the colour picker dialog.
+     *
+     * A colour change event will be sent if it's set.
+     */
+    void GetNewSwatchColor();
+
+private:
+
+    /**
+     * Pass unwanted events on to listeners of this object
+     */
+    void rePostEvent( wxEvent& aEvt );
+
+    ///> Can the swatch have any color, or only preset ones?
+    bool m_arbitraryColors;
+
+    ///> The current colour of the swatch
+    KIGFX::COLOR4D m_color;
+
+    ///> Handle of the actual swatch shown
+    wxStaticBitmap* m_swatch;
+};
+
+
+/**
+ * Event signalling a swatch has changed color
+ */
+wxDECLARE_EVENT(COLOR_SWATCH_CHANGED, wxCommandEvent);
+
+#endif // COLOR_SWATCH__H_
diff --git a/pcbnew/layer_widget.cpp b/pcbnew/layer_widget.cpp
index d63c3f9141..b29541dd3a 100644
--- a/pcbnew/layer_widget.cpp
+++ b/pcbnew/layer_widget.cpp
@@ -37,18 +37,12 @@
 
 #include <macros.h>
 #include <common.h>
-#include <wx/colour.h>
-#include <wx/colordlg.h>
+
+#include <widgets/color_swatch.h>
 
 #include <algorithm>
 
 
-const static int SWATCH_SIZE_X = 14;
-const static int SWATCH_SIZE_Y = 12;
-
-// See selcolor.cpp:
-extern COLOR4D DisplayColorFrame( wxWindow* aParent, COLOR4D aOldColor );
-
 const wxEventType LAYER_WIDGET::EVT_LAYER_COLOR_CHANGE = wxNewEventType();
 
 /* XPM
@@ -149,56 +143,6 @@ static const char * rightarrow_alternate_xpm[] = {
 "..oO      "};
 
 
-/**
- * Function makeColorTxt
- * returns a string representing the color in CSS format
- * For example: "rgba(255, 0, 0, 255)"
- */
-static wxString makeColorTxt( COLOR4D aColor )
-{
-    return aColor.ToWxString( wxC2S_CSS_SYNTAX );
-}
-
-
-static wxBitmap makeBitmap( COLOR4D aColor )
-{
-    wxBitmap    bitmap( SWATCH_SIZE_X, SWATCH_SIZE_Y );
-    wxBrush     brush;
-    wxMemoryDC  iconDC;
-
-    iconDC.SelectObject( bitmap );
-
-    brush.SetColour( aColor.ToColour() );
-    brush.SetStyle( wxBRUSHSTYLE_SOLID );
-
-    iconDC.SetBrush( brush );
-
-    iconDC.DrawRectangle( 0, 0, SWATCH_SIZE_X, SWATCH_SIZE_Y );
-
-    return bitmap;
-}
-
-
-/**
- * Function makeColorButton
- * creates a wxStaticBitmap and assigns it a solid color and a control ID
- */
-static std::unique_ptr<wxStaticBitmap> makeColorSwatch( wxWindow* aParent,
-                                                        COLOR4D aColor, int aID )
-{
-    // dynamically make a wxBitMap and brush it with the appropriate color,
-    // then create a wxBitmapButton from it.
-    wxBitmap bitmap = makeBitmap( aColor );
-
-    // save the color value in the name, no where else to put it.
-    auto ret = std::make_unique<wxStaticBitmap>( aParent, aID, bitmap );
-
-    ret->SetName( makeColorTxt( aColor ) );
-
-    return ret;
-}
-
-
 /**
  * Function shrinkFont
  * reduces the size of the wxFont associated with \a aControl
@@ -273,48 +217,20 @@ void LAYER_WIDGET::OnLeftDownLayers( wxMouseEvent& event )
 }
 
 
-void LAYER_WIDGET::OnMiddleDownLayerColor( wxMouseEvent& aEvent )
+void LAYER_WIDGET::OnLayerSwatchChanged( wxCommandEvent& aEvent )
 {
-    auto eventSource = static_cast<wxStaticBitmap*>( aEvent.GetEventObject() );
+    auto eventSource = static_cast<COLOR_SWATCH*>( aEvent.GetEventObject() );
 
-    wxString colorTxt = eventSource->GetName();
+    COLOR4D newColor = eventSource->GetSwatchColor();
 
-    COLOR4D oldColor;
-    wxASSERT( oldColor.SetFromWxString( colorTxt ) );
-    COLOR4D newColor = COLOR4D::UNSPECIFIED;
+    LAYER_NUM layer = getDecodedId( eventSource->GetId() );
 
-    if( AreArbitraryColorsAllowed() )
-    {
-        wxColourData colourData;
-        colourData.SetColour( oldColor.ToColour() );
-        wxColourDialog* dialog = new wxColourDialog( m_LayerScrolledWindow, &colourData );
+    // tell the client code.
+    OnLayerColorChange( layer, newColor );
 
-        if( dialog->ShowModal() == wxID_OK )
-        {
-            newColor = COLOR4D( dialog->GetColourData().GetColour() );
-        }
-    }
-    else
-    {
-        newColor = DisplayColorFrame( this, oldColor );
-    }
-
-    if( newColor != COLOR4D::UNSPECIFIED )
-    {
-        eventSource->SetName( makeColorTxt( newColor ) );
-
-        wxBitmap bm = makeBitmap( newColor.ToColour() );
-        eventSource->SetBitmap( bm );
-
-        LAYER_NUM layer = getDecodedId( eventSource->GetId() );
-
-        // tell the client code.
-        OnLayerColorChange( layer, newColor );
-
-        // notify others
-        wxCommandEvent event( EVT_LAYER_COLOR_CHANGE );
-        wxPostEvent( this, event );
-    }
+    // notify others
+    wxCommandEvent event( EVT_LAYER_COLOR_CHANGE );
+    wxPostEvent( this, event );
 
     passOnFocus();
 }
@@ -329,44 +245,17 @@ void LAYER_WIDGET::OnLayerCheckBox( wxCommandEvent& event )
 }
 
 
-void LAYER_WIDGET::OnMiddleDownRenderColor( wxMouseEvent& aEvent )
+void LAYER_WIDGET::OnRenderSwatchChanged( wxCommandEvent& aEvent )
 {
-    auto eventSource = static_cast<wxStaticBitmap*>( aEvent.GetEventObject() );
+    auto eventSource = static_cast<COLOR_SWATCH*>( aEvent.GetEventObject() );
 
-    wxString colorTxt = eventSource->GetName();
+    COLOR4D newColor = eventSource->GetSwatchColor();
 
-    COLOR4D oldColor;
-    wxASSERT( oldColor.SetFromWxString( colorTxt ) );
-    COLOR4D newColor = COLOR4D::UNSPECIFIED;
+    LAYER_NUM id = getDecodedId( eventSource->GetId() );
 
-    if( AreArbitraryColorsAllowed() )
-    {
-        wxColourData colourData;
-        colourData.SetColour( oldColor.ToColour() );
-        wxColourDialog *dialog = new wxColourDialog( m_LayerScrolledWindow, &colourData );
+    // tell the client code.
+    OnRenderColorChange( id, newColor );
 
-        if( dialog->ShowModal() == wxID_OK )
-        {
-            newColor = COLOR4D( dialog->GetColourData().GetColour() );
-        }
-    }
-    else
-    {
-        newColor = DisplayColorFrame( this, oldColor );
-    }
-
-    if( newColor != COLOR4D::UNSPECIFIED )
-    {
-        eventSource->SetName( makeColorTxt( newColor ) );
-
-        wxBitmap bm = makeBitmap( newColor );
-        eventSource->SetBitmap( bm );
-
-        LAYER_NUM id = getDecodedId( eventSource->GetId() );
-
-        // tell the client code.
-        OnRenderColorChange( id, newColor );
-    }
     passOnFocus();
 }
 
@@ -458,12 +347,12 @@ void LAYER_WIDGET::insertLayerRow( int aRow, const ROW& aSpec )
     // column 1 (COLUMN_COLORBM)
     col = COLUMN_COLORBM;
 
-    auto bmb = makeColorSwatch( m_LayerScrolledWindow, aSpec.color, encodeId( col, aSpec.id ) );
-    bmb->Connect( wxEVT_LEFT_DOWN, wxMouseEventHandler( LAYER_WIDGET::OnLeftDownLayers ), NULL, this );
-    bmb->Connect( wxEVT_LEFT_DCLICK, wxMouseEventHandler( LAYER_WIDGET::OnMiddleDownLayerColor ), NULL, this );
-    bmb->Connect( wxEVT_MIDDLE_DOWN, wxMouseEventHandler( LAYER_WIDGET::OnMiddleDownLayerColor ), NULL, this );
+    auto bmb = new COLOR_SWATCH( m_LayerScrolledWindow, aSpec.color, encodeId( col, aSpec.id ),
+                                 AreArbitraryColorsAllowed() );
+    bmb->Bind( wxEVT_LEFT_DOWN, &LAYER_WIDGET::OnLeftDownLayers, this );
+    bmb->Bind( COLOR_SWATCH_CHANGED, &LAYER_WIDGET::OnLayerSwatchChanged, this );
     bmb->SetToolTip( _("Left double click or middle click for color change, right click for menu" ) );
-    m_LayersFlexGridSizer->wxSizer::Insert( index+col, bmb.release(), 0, flags );
+    m_LayersFlexGridSizer->wxSizer::Insert( index+col, bmb, 0, flags );
 
     // column 2 (COLUMN_COLOR_LYR_CB)
     col = COLUMN_COLOR_LYR_CB;
@@ -495,13 +384,11 @@ void LAYER_WIDGET::insertRenderRow( int aRow, const ROW& aSpec )
     col = 0;
     if( aSpec.color != COLOR4D::UNSPECIFIED )
     {
-        auto bmb = makeColorSwatch(m_RenderScrolledWindow, aSpec.color, encodeId( col, aSpec.id ) );
-
-        //makeColorSwatch( m_RenderScrolledWindow, aSpec.color, encodeId( col, aSpec.id ) );
-        bmb->Connect( wxEVT_LEFT_DCLICK, wxMouseEventHandler( LAYER_WIDGET::OnMiddleDownRenderColor ), NULL, this );
-        bmb->Connect( wxEVT_MIDDLE_DOWN, wxMouseEventHandler( LAYER_WIDGET::OnMiddleDownRenderColor ), NULL, this );
+        auto bmb = new COLOR_SWATCH( m_RenderScrolledWindow, aSpec.color, encodeId( col, aSpec.id ),
+                                     AreArbitraryColorsAllowed() );
+        bmb->Bind( COLOR_SWATCH_CHANGED, &LAYER_WIDGET::OnRenderSwatchChanged, this );
         bmb->SetToolTip( _( "Left double click or middle click for color change" ) );
-        m_RenderFlexGridSizer->wxSizer::Insert( index+col, bmb.release(), 0, flags );
+        m_RenderFlexGridSizer->wxSizer::Insert( index+col, bmb, 0, flags );
 
         // could add a left click handler on the color button that toggles checkbox.
     }
@@ -803,13 +690,10 @@ void LAYER_WIDGET::SetLayerColor( LAYER_NUM aLayer, COLOR4D aColor )
     if( row >= 0 )
     {
         int col = 1;    // bitmap button is column 1
-        wxBitmapButton* bmb = (wxBitmapButton*) getLayerComp( row, col );
+        auto bmb = static_cast<COLOR_SWATCH*>( getLayerComp( row, col ) );
         wxASSERT( bmb );
 
-        wxBitmap bm = makeBitmap( aColor );
-
-        bmb->SetBitmapLabel( bm );
-        bmb->SetName( makeColorTxt( aColor ) ); // save color value in name as string
+        bmb->SetSwatchColor( aColor, false );
     }
 }
 
diff --git a/pcbnew/layer_widget.h b/pcbnew/layer_widget.h
index 892de638d2..635ee18a23 100644
--- a/pcbnew/layer_widget.h
+++ b/pcbnew/layer_widget.h
@@ -157,10 +157,10 @@ protected:
     void OnLeftDownLayers( wxMouseEvent& event );
 
     /**
-     * Function OnMiddleDownLayerColor
-     * is called only from a color button when user right clicks.
+     * Function OnSwatchChanged()
+     * is called when a user changes a swatch color
      */
-    void OnMiddleDownLayerColor( wxMouseEvent& event );
+    void OnLayerSwatchChanged( wxCommandEvent& aEvent );
 
     /**
      * Function OnLayerCheckBox
@@ -169,7 +169,11 @@ protected:
      */
     void OnLayerCheckBox( wxCommandEvent& event );
 
-    void OnMiddleDownRenderColor( wxMouseEvent& event );
+    /**
+     * Function OnRenderSwatchChanged
+     * Called when user has changed the swatch color of a render entry
+     */
+    void OnRenderSwatchChanged( wxCommandEvent& aEvent );
 
     void OnRenderCheckBox( wxCommandEvent& event );
 

From 72354c5a818e74288539d5ddb99ccd645cb6195d Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@gmail.com>
Date: Tue, 21 Feb 2017 20:43:02 +0800
Subject: [PATCH 3/5] Use Bind rather than Connect in LAYER_WIDGET

Bind is safer than Connect. Bind is more flexible and has a more concise
signature.
---
 pcbnew/layer_widget.cpp | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/pcbnew/layer_widget.cpp b/pcbnew/layer_widget.cpp
index b29541dd3a..617e0a14c4 100644
--- a/pcbnew/layer_widget.cpp
+++ b/pcbnew/layer_widget.cpp
@@ -341,7 +341,7 @@ void LAYER_WIDGET::insertLayerRow( int aRow, const ROW& aSpec )
     wxStaticBitmap* sbm = new wxStaticBitmap( m_LayerScrolledWindow, encodeId( col, aSpec.id ),
                             useAlternateBitmap(aRow) ? *m_BlankAlternateBitmap : *m_BlankBitmap,
                             wxDefaultPosition, m_BitmapSize );
-    sbm->Connect( wxEVT_LEFT_DOWN, wxMouseEventHandler( LAYER_WIDGET::OnLeftDownLayers ), NULL, this );
+    sbm->Bind( wxEVT_LEFT_DOWN, &LAYER_WIDGET::OnLeftDownLayers, this );
     m_LayersFlexGridSizer->wxSizer::Insert( index+col, sbm, 0, flags );
 
     // column 1 (COLUMN_COLORBM)
@@ -358,7 +358,7 @@ void LAYER_WIDGET::insertLayerRow( int aRow, const ROW& aSpec )
     col = COLUMN_COLOR_LYR_CB;
     wxCheckBox* cb = new wxCheckBox( m_LayerScrolledWindow, encodeId( col, aSpec.id ), wxEmptyString );
     cb->SetValue( aSpec.state );
-    cb->Connect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( LAYER_WIDGET::OnLayerCheckBox ), NULL, this );
+    cb->Bind( wxEVT_COMMAND_CHECKBOX_CLICKED, &LAYER_WIDGET::OnLayerCheckBox, this );
     cb->SetToolTip( _( "Enable this for visibility" ) );
     m_LayersFlexGridSizer->wxSizer::Insert( index+col, cb, 0, flags );
 
@@ -366,7 +366,7 @@ void LAYER_WIDGET::insertLayerRow( int aRow, const ROW& aSpec )
     col = COLUMN_COLOR_LYRNAME;
     wxStaticText* st = new wxStaticText( m_LayerScrolledWindow, encodeId( col, aSpec.id ), aSpec.rowName );
     shrinkFont( st, m_PointSize );
-    st->Connect( wxEVT_LEFT_DOWN, wxMouseEventHandler( LAYER_WIDGET::OnLeftDownLayers ), NULL, this );
+    st->Bind( wxEVT_LEFT_DOWN, &LAYER_WIDGET::OnLeftDownLayers, this );
     st->SetToolTip( aSpec.tooltip );
     m_LayersFlexGridSizer->wxSizer::Insert( index+col, st, 0, flags );
 }
@@ -405,8 +405,7 @@ void LAYER_WIDGET::insertRenderRow( int aRow, const ROW& aSpec )
                         aSpec.rowName, wxDefaultPosition, wxDefaultSize, wxALIGN_LEFT );
     shrinkFont( cb, m_PointSize );
     cb->SetValue( aSpec.state );
-    cb->Connect( wxEVT_COMMAND_CHECKBOX_CLICKED,
-        wxCommandEventHandler( LAYER_WIDGET::OnRenderCheckBox ), NULL, this );
+    cb->Bind( wxEVT_COMMAND_CHECKBOX_CLICKED, &LAYER_WIDGET::OnRenderCheckBox, this );
     cb->SetToolTip( aSpec.tooltip );
     m_RenderFlexGridSizer->wxSizer::Insert( index+col, cb, 0, flags );
 }
@@ -503,8 +502,7 @@ LAYER_WIDGET::LAYER_WIDGET( wxWindow* aParent, wxWindow* aFocusOwner, int aPoint
     m_BitmapSize = wxSize(m_BlankBitmap->GetWidth(), m_BlankBitmap->GetHeight());
 
     // trap the tab changes so that we can call passOnFocus().
-    m_notebook->Connect( -1, wxEVT_COMMAND_NOTEBOOK_PAGE_CHANGED,
-        wxNotebookEventHandler( LAYER_WIDGET::OnTabChange ), NULL, this );
+    m_notebook->Bind( wxEVT_COMMAND_NOTEBOOK_PAGE_CHANGED, &LAYER_WIDGET::OnTabChange, this );
 
     Layout();
 }

From 1d039cb4c1a1ff846c3b8d67a1c5186e4aa6279d Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@gmail.com>
Date: Wed, 22 Feb 2017 20:34:23 +0800
Subject: [PATCH 4/5] Break row indicators out to own class

The introduces INDICATOR_ICON, which is a very simple class holding a
bitmap that can toggle on or off.

The ICON_PROVIDER class then provides icons to INDICATOR_ICONS, which
means the class can be used for more than just row indicators.

A default row icon provider is also provided for use in the standard row
selector.
---
 common/CMakeLists.txt                    |   1 +
 common/widgets/indicator_icon.cpp        | 191 +++++++++++++++++++++++
 gerbview/class_gerbview_layer_widget.cpp |  21 ---
 gerbview/class_gerbview_layer_widget.h   |   7 -
 include/widgets/indicator_icon.h         | 140 +++++++++++++++++
 pcbnew/layer_widget.cpp                  | 152 +++++-------------
 pcbnew/layer_widget.h                    |  13 +-
 7 files changed, 375 insertions(+), 150 deletions(-)
 create mode 100644 common/widgets/indicator_icon.cpp
 create mode 100644 include/widgets/indicator_icon.h

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index df08770f15..a482108abf 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -170,6 +170,7 @@ set( COMMON_WIDGET_SRCS
     widgets/widget_hotkey_list.cpp
     widgets/two_column_tree_list.cpp
     widgets/footprint_preview_panel.cpp
+    widgets/indicator_icon.cpp
     )
 
 set( COMMON_PAGE_LAYOUT_SRCS
diff --git a/common/widgets/indicator_icon.cpp b/common/widgets/indicator_icon.cpp
new file mode 100644
index 0000000000..c365c390ca
--- /dev/null
+++ b/common/widgets/indicator_icon.cpp
@@ -0,0 +1,191 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+
+ * Copyright (C) 2017 KiCad Developers, see AUTHORS.TXT for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#include <widgets/indicator_icon.h>
+
+
+INDICATOR_ICON::INDICATOR_ICON( wxWindow* aParent,
+                              ICON_PROVIDER& aIconProvider,
+                              ICON_ID aInitialIcon, int aID ):
+        wxPanel( aParent, aID ),
+        m_iconProvider( aIconProvider ),
+        m_currentId( aInitialIcon )
+{
+    auto sizer = new wxBoxSizer( wxHORIZONTAL );
+    SetSizer( sizer );
+
+    const wxBitmap& initBitmap = m_iconProvider.GetIndicatorIcon( m_currentId );
+
+    m_bitmap = new wxStaticBitmap( this, aID,
+                                   initBitmap, wxDefaultPosition,
+                                   initBitmap.GetSize() );
+
+    sizer->Add( m_bitmap, 0, 0 );
+
+    auto evtSkipper = [this] ( wxEvent& aEvent ) {
+        wxPostEvent( this, aEvent );
+    };
+
+    m_bitmap->Bind( wxEVT_LEFT_DOWN, evtSkipper );
+}
+
+
+void INDICATOR_ICON::SetIndicatorState( ICON_ID aIconId )
+{
+    if( aIconId == m_currentId )
+        return;
+
+    m_currentId = aIconId;
+
+    m_bitmap->SetBitmap( m_iconProvider.GetIndicatorIcon( m_currentId ) );
+}
+
+
+INDICATOR_ICON::ICON_ID INDICATOR_ICON::GetIndicatorState() const
+{
+    return m_currentId;
+}
+
+// ====================================================================
+// Common icon providers
+
+/* XPM
+ * This bitmap is used for not selected layers
+ */
+static const char * clear_xpm[] = {
+"10 14 1 1",
+" 	c None",
+"          ",
+"          ",
+"          ",
+"          ",
+"          ",
+"          ",
+"          ",
+"          ",
+"          ",
+"          ",
+"          ",
+"          ",
+"          ",
+"          "};
+
+/* XPM
+ * This bitmap can be used to show a not selected layer
+ * with special property (mainly not selected layers not in use in GerbView)
+ */
+static const char * clear_alternate_xpm[] = {
+"10 14 4 1",
+"       c None",
+"X      c #008080",
+"o      c GREEN",
+"O      c #00B080",
+"          ",
+"          ",
+"          ",
+"          ",
+"    X     ",
+"   XXX    ",
+"  XXXXX   ",
+" OOOOOOO  ",
+"  ooooo   ",
+"   ooo    ",
+"    o     ",
+"          ",
+"          ",
+"          "};
+
+
+/* XPM
+ * This bitmap  is used for a normale selected layer
+ */
+static const char * rightarrow_xpm[] = {
+"10 14 4 1",
+"       c None",
+"X      c #8080ff",
+"o      c BLUE",
+"O      c gray56",
+"  X       ",
+"  XX      ",
+"  XXX     ",
+"  XXXX    ",
+"  XXXXX   ",
+"  XXXXXX  ",
+"  XXXXXXX ",
+"  oooooooO",
+"  ooooooO ",
+"  oooooO  ",
+"  ooooO   ",
+"  oooO    ",
+"  ooO     ",
+"  oO      "};
+
+/* XPM
+ * This bitmap can be used to show the selected layer
+ * with special property (mainly a layer in use in GerbView)
+ */
+static const char * rightarrow_alternate_xpm[] = {
+"10 14 5 1",
+"       c None",
+".      c #00B000",
+"X      c #8080ff",
+"o      c BLUE",
+"O      c gray56",
+"..X       ",
+"..XX      ",
+"..XXX     ",
+"..XXXX    ",
+"..XXXXX   ",
+"..XXXXXX  ",
+"..XXXXXXX ",
+"..oooooooO",
+"..ooooooO ",
+"..oooooO  ",
+"..ooooO   ",
+"..oooO    ",
+"..ooO     ",
+"..oO      "};
+
+
+
+static wxBitmap rightArrowBitmap( rightarrow_xpm );
+static wxBitmap rightArrowAlternateBitmap( rightarrow_alternate_xpm );
+static wxBitmap blankBitmap( clear_xpm );
+static wxBitmap blankAlternateBitmap( clear_alternate_xpm );
+
+
+ROW_ICON_PROVIDER::ROW_ICON_PROVIDER( bool aAlt ):
+        m_alt( aAlt )
+{}
+
+
+const wxBitmap& ROW_ICON_PROVIDER::GetIndicatorIcon(
+            INDICATOR_ICON::ICON_ID aIconId ) const
+{
+    const bool on = ( aIconId == STATE::ON );
+
+    if( m_alt )
+        return ( on ? rightArrowAlternateBitmap : blankAlternateBitmap );
+
+    return ( on ? rightArrowBitmap : blankBitmap );
+}
diff --git a/gerbview/class_gerbview_layer_widget.cpp b/gerbview/class_gerbview_layer_widget.cpp
index f6c6ca9bb4..1470417d44 100644
--- a/gerbview/class_gerbview_layer_widget.cpp
+++ b/gerbview/class_gerbview_layer_widget.cpp
@@ -314,24 +314,3 @@ bool GERBER_LAYER_WIDGET::useAlternateBitmap(int aRow)
 {
     return GetImagesList()->GetGbrImage( aRow ) != NULL;
 }
-
-/*
- * Update the layer manager icons (layers only)
- * Useful when loading a file or clearing a layer because they change
- */
-void GERBER_LAYER_WIDGET::UpdateLayerIcons()
-{
-    int row_count = GetLayerRowCount();
-    for( int row = 0; row < row_count ; row++ )
-    {
-        wxStaticBitmap* bm = (wxStaticBitmap*) getLayerComp( row, COLUMN_ICON_ACTIVE );
-        if( bm == NULL)
-            continue;
-
-        if( row == m_CurrentRow )
-            bm->SetBitmap( useAlternateBitmap(row) ? *m_RightArrowAlternateBitmap :
-                           *m_RightArrowBitmap );
-        else
-            bm->SetBitmap( useAlternateBitmap(row) ? *m_BlankAlternateBitmap : *m_BlankBitmap );
-    }
-}
diff --git a/gerbview/class_gerbview_layer_widget.h b/gerbview/class_gerbview_layer_widget.h
index 8e5c6ef18f..ad40533af6 100644
--- a/gerbview/class_gerbview_layer_widget.h
+++ b/gerbview/class_gerbview_layer_widget.h
@@ -125,13 +125,6 @@ public:
     bool OnLayerSelected();     // postprocess after an active layer selection
                                 // ensure active layer visible if
                                 // m_alwaysShowActiveCopperLayer is true;
-
-    /**
-     * Function UpdateLayerIcons
-     * Update the layer manager icons (layers only)
-     * Useful when loading a file or clearing a layer because they change
-     */
-    void UpdateLayerIcons();
 };
 
 #endif  // _CLASS_GERBER_LAYER_WIDGET_H_
diff --git a/include/widgets/indicator_icon.h b/include/widgets/indicator_icon.h
new file mode 100644
index 0000000000..fc3436ab9f
--- /dev/null
+++ b/include/widgets/indicator_icon.h
@@ -0,0 +1,140 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2017 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#ifndef ROW_INDICATOR__H_
+#define ROW_INDICATOR__H_
+
+#include <wx/wx.h>
+
+/**
+ * Class representing a row indicator icon for use in
+ * places like the layer widget
+ */
+class INDICATOR_ICON: public wxPanel
+{
+public:
+
+    /**
+     * An id that refers to a certain icon state.
+     *
+     * Exactly what that state might mean in terms of icons is up
+     * to the icon provider.
+     */
+    using ICON_ID = int;
+
+    /**
+     * A simple object that can provide fixed bitmaps for use as row
+     * indicators
+     */
+    class ICON_PROVIDER
+    {
+    public:
+
+        virtual ~ICON_PROVIDER() {};
+
+        /**
+         * Gets a reference to the row icon in the given mode
+         *
+         * @param aIconId the id of the icon to get (depends on the
+         * provider).
+         */
+        virtual const wxBitmap& GetIndicatorIcon( ICON_ID aIconId ) const = 0;
+    };
+
+    /**
+     * Accessor for the default icon providers, which take
+     * true and false for IDs, meaining on/off.
+     *
+     * @param aAlternative false for blue arrow/blank, true for the
+     * green diamond
+     */
+    static ICON_PROVIDER& GetDefaultRowIconProvider( bool aAlternative );
+
+    /**
+     * @param aParent the owning window
+     * @param aIconProvider the icon provider to get icons from
+     * @param aID the ID to use for the widgets - events will have
+     * this ID.
+     */
+    INDICATOR_ICON( wxWindow* aParent,
+                   ICON_PROVIDER& aIconProvider,
+                   ICON_ID aInitialIcon, int aID );
+
+    /**
+     * Sets the row indiciator to the given state
+     *
+     * @param aIconId the icon ID to pass to the provider.
+     */
+    void SetIndicatorState( ICON_ID aIconId );
+
+
+    /**
+     * @return the current state of the indicator
+     */
+    ICON_ID GetIndicatorState() const;
+
+private:
+
+    ///> An class that delivers icons for the indictor (currently just
+    ///> uses a default implementation).
+    ICON_PROVIDER& m_iconProvider;
+
+    ///> Handle on the bitmap widget
+    wxStaticBitmap* m_bitmap;
+
+    ///> Is the icon currently "on"
+    ICON_ID m_currentId;
+};
+
+
+/**
+ * Icon provider for the "standard" row indicators, for example in
+ * layer selection lists
+ */
+class ROW_ICON_PROVIDER: public INDICATOR_ICON::ICON_PROVIDER
+{
+public:
+
+    ///> State constants to select the right icons
+    enum STATE
+    {
+        OFF,    ///> Row "off" or "deselected"
+        ON,     ///> Row "on" or "selected"
+    };
+
+    /**
+     * @param aAlt false: normal icons (blue arrow/blank), true:
+     * alternative icons (blue arrow/green diamond)
+     */
+    ROW_ICON_PROVIDER( bool aAlt );
+
+    ///> @copydoc INDICATOR_ICON::ICON_PROVIDER::GetIndicatorIcon()
+    const wxBitmap& GetIndicatorIcon( INDICATOR_ICON::ICON_ID aIconId ) const override;
+
+private:
+    bool m_alt;
+};
+
+
+
+#endif // ROW_INDICATOR__H_
diff --git a/pcbnew/layer_widget.cpp b/pcbnew/layer_widget.cpp
index 617e0a14c4..a043930288 100644
--- a/pcbnew/layer_widget.cpp
+++ b/pcbnew/layer_widget.cpp
@@ -39,109 +39,18 @@
 #include <common.h>
 
 #include <widgets/color_swatch.h>
+#include <widgets/indicator_icon.h>
 
 #include <algorithm>
 
 
 const wxEventType LAYER_WIDGET::EVT_LAYER_COLOR_CHANGE = wxNewEventType();
 
-/* XPM
- * This bitmap is used for not selected layers
+/*
+ * Icon providers for the row icons
  */
-static const char * clear_xpm[] = {
-"10 14 1 1",
-" 	c None",
-"          ",
-"          ",
-"          ",
-"          ",
-"          ",
-"          ",
-"          ",
-"          ",
-"          ",
-"          ",
-"          ",
-"          ",
-"          ",
-"          "};
-
-/* XPM
- * This bitmap can be used to show a not selected layer
- * with special property (mainly not selected layers not in use in GerbView)
- */
-static const char * clear_alternate_xpm[] = {
-"10 14 4 1",
-"       c None",
-"X      c #008080",
-"o      c GREEN",
-"O      c #00B080",
-"          ",
-"          ",
-"          ",
-"          ",
-"    X     ",
-"   XXX    ",
-"  XXXXX   ",
-" OOOOOOO  ",
-"  ooooo   ",
-"   ooo    ",
-"    o     ",
-"          ",
-"          ",
-"          "};
-
-
-/* XPM
- * This bitmap  is used for a normale selected layer
- */
-static const char * rightarrow_xpm[] = {
-"10 14 4 1",
-"       c None",
-"X      c #8080ff",
-"o      c BLUE",
-"O      c gray56",
-"  X       ",
-"  XX      ",
-"  XXX     ",
-"  XXXX    ",
-"  XXXXX   ",
-"  XXXXXX  ",
-"  XXXXXXX ",
-"  oooooooO",
-"  ooooooO ",
-"  oooooO  ",
-"  ooooO   ",
-"  oooO    ",
-"  ooO     ",
-"  oO      "};
-
-/* XPM
- * This bitmap can be used to show the selected layer
- * with special property (mainly a layer in use in GerbView)
- */
-static const char * rightarrow_alternate_xpm[] = {
-"10 14 5 1",
-"       c None",
-".      c #00B000",
-"X      c #8080ff",
-"o      c BLUE",
-"O      c gray56",
-"..X       ",
-"..XX      ",
-"..XXX     ",
-"..XXXX    ",
-"..XXXXX   ",
-"..XXXXXX  ",
-"..XXXXXXX ",
-"..oooooooO",
-"..ooooooO ",
-"..oooooO  ",
-"..ooooO   ",
-"..oooO    ",
-"..ooO     ",
-"..oO      "};
-
+static ROW_ICON_PROVIDER defaultRowIcons( false );
+static ROW_ICON_PROVIDER alternativeRowIcons( true );
 
 /**
  * Function shrinkFont
@@ -336,11 +245,13 @@ void LAYER_WIDGET::insertLayerRow( int aRow, const ROW& aSpec )
     int         index = aRow * LYR_COLUMN_COUNT;
     const int   flags = wxALIGN_CENTER_VERTICAL | wxALIGN_LEFT;
 
+    auto& iconProvider = useAlternateBitmap(aRow) ? alternativeRowIcons : defaultRowIcons;
+
     // column 0
-    col = 0;
-    wxStaticBitmap* sbm = new wxStaticBitmap( m_LayerScrolledWindow, encodeId( col, aSpec.id ),
-                            useAlternateBitmap(aRow) ? *m_BlankAlternateBitmap : *m_BlankBitmap,
-                            wxDefaultPosition, m_BitmapSize );
+    col = COLUMN_ICON_ACTIVE;
+    auto sbm = new INDICATOR_ICON( m_LayerScrolledWindow, iconProvider,
+                                   ROW_ICON_PROVIDER::STATE::OFF,
+                                   encodeId( col, aSpec.id ) );
     sbm->Bind( wxEVT_LEFT_DOWN, &LAYER_WIDGET::OnLeftDownLayers, this );
     m_LayersFlexGridSizer->wxSizer::Insert( index+col, sbm, 0, flags );
 
@@ -494,13 +405,6 @@ LAYER_WIDGET::LAYER_WIDGET( wxWindow* aParent, wxWindow* aFocusOwner, int aPoint
 
     m_CurrentRow = -1;  // hide the arrow initially
 
-    m_RightArrowBitmap = new wxBitmap( rightarrow_xpm );
-    m_RightArrowAlternateBitmap = new wxBitmap( rightarrow_alternate_xpm );
-
-    m_BlankBitmap = new wxBitmap( clear_xpm );     // translucent
-    m_BlankAlternateBitmap = new wxBitmap( clear_alternate_xpm );
-    m_BitmapSize = wxSize(m_BlankBitmap->GetWidth(), m_BlankBitmap->GetHeight());
-
     // trap the tab changes so that we can call passOnFocus().
     m_notebook->Bind( wxEVT_COMMAND_NOTEBOOK_PAGE_CHANGED, &LAYER_WIDGET::OnTabChange, this );
 
@@ -510,10 +414,6 @@ LAYER_WIDGET::LAYER_WIDGET( wxWindow* aParent, wxWindow* aFocusOwner, int aPoint
 
 LAYER_WIDGET::~LAYER_WIDGET()
 {
-    delete m_BlankBitmap;
-    delete m_BlankAlternateBitmap;
-    delete m_RightArrowBitmap;
-    delete m_RightArrowAlternateBitmap;
 }
 
 
@@ -616,14 +516,14 @@ void LAYER_WIDGET::SelectLayerRow( int aRow )
     // enable the layer tab at index 0
     m_notebook->SetSelection( 0 );
 
-    wxStaticBitmap* oldbm = (wxStaticBitmap*) getLayerComp( m_CurrentRow, 0 );
-    if( oldbm )
-        oldbm->SetBitmap( useAlternateBitmap(m_CurrentRow) ? *m_BlankAlternateBitmap : *m_BlankBitmap );
+    INDICATOR_ICON* oldIndicator = (INDICATOR_ICON*) getLayerComp( m_CurrentRow, 0 );
+    if( oldIndicator )
+        oldIndicator->SetIndicatorState( ROW_ICON_PROVIDER::STATE::OFF );
 
-    wxStaticBitmap* newbm = (wxStaticBitmap*) getLayerComp( aRow, 0 );
-    if( newbm )
+    INDICATOR_ICON* newIndicator = (INDICATOR_ICON*) getLayerComp( aRow, 0 );
+    if( newIndicator )
     {
-        newbm->SetBitmap( useAlternateBitmap(aRow) ? *m_RightArrowAlternateBitmap : *m_RightArrowBitmap );
+        newIndicator->SetIndicatorState( ROW_ICON_PROVIDER::STATE::ON );
 
         // Make sure the desired layer row is visible.
         // It seems that as of 2.8.2, setting the focus does this.
@@ -749,6 +649,24 @@ void LAYER_WIDGET::UpdateLayouts()
     FitInside();
 }
 
+
+void LAYER_WIDGET::UpdateLayerIcons()
+{
+    int rowCount = GetLayerRowCount();
+    for( int row = 0; row < rowCount ; row++ )
+    {
+        INDICATOR_ICON* indicator = (INDICATOR_ICON*) getLayerComp( row, COLUMN_ICON_ACTIVE );
+
+        if( indicator )
+        {
+            auto state = ( row == m_CurrentRow ) ? ROW_ICON_PROVIDER::STATE::ON
+                                                 : ROW_ICON_PROVIDER::STATE::OFF;
+            indicator->SetIndicatorState( state );
+        }
+    }
+}
+
+
 #if defined(STAND_ALONE)
 
 #include <wx/aui/aui.h>
diff --git a/pcbnew/layer_widget.h b/pcbnew/layer_widget.h
index 635ee18a23..99949b59d9 100644
--- a/pcbnew/layer_widget.h
+++ b/pcbnew/layer_widget.h
@@ -113,11 +113,6 @@ protected:
     wxFlexGridSizer*    m_RenderFlexGridSizer;
 
     wxWindow*           m_FocusOwner;
-    wxBitmap*           m_BlankBitmap;
-    wxBitmap*           m_BlankAlternateBitmap;
-    wxBitmap*           m_RightArrowBitmap;
-    wxBitmap*           m_RightArrowAlternateBitmap;
-    wxSize              m_BitmapSize;
     int                 m_CurrentRow;           ///< selected row of layer list
     int                 m_PointSize;
 
@@ -363,6 +358,14 @@ public:
 
     void UpdateLayouts();
 
+    /**
+     * Function UpdateLayerIcons
+     * Update all layer manager icons (layers only)
+     * Useful when loading a file or clearing a layer because they change,
+     * and the indicator arrow icon needs to be updated
+     */
+    void UpdateLayerIcons();
+
 /*  did not help:
     void Freeze()
     {

From f4be80a3af135f1a449ee54e3764774ad19d67df Mon Sep 17 00:00:00 2001
From: Maciej Suminski <maciej.suminski@cern.ch>
Date: Wed, 1 Mar 2017 09:50:36 +0100
Subject: [PATCH 5/5] Code formatting

---
 pcbnew/layer_widget.cpp | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/pcbnew/layer_widget.cpp b/pcbnew/layer_widget.cpp
index a043930288..2fbf7be80d 100644
--- a/pcbnew/layer_widget.cpp
+++ b/pcbnew/layer_widget.cpp
@@ -190,8 +190,10 @@ void LAYER_WIDGET::OnTabChange( wxNotebookEvent& event )
 wxWindow* LAYER_WIDGET::getLayerComp( int aRow, int aColumn ) const
 {
     unsigned ndx = aRow * LYR_COLUMN_COUNT + aColumn;
+
     if( ndx < m_LayersFlexGridSizer->GetChildren().GetCount() )
         return m_LayersFlexGridSizer->GetChildren()[ndx]->GetWindow();
+
     return NULL;
 }
 
@@ -199,7 +201,8 @@ wxWindow* LAYER_WIDGET::getLayerComp( int aRow, int aColumn ) const
 int LAYER_WIDGET::findLayerRow( LAYER_NUM aLayer ) const
 {
     int count = GetLayerRowCount();
-    for( int row=0;  row<count;  ++row )
+
+    for( int row = 0; row < count; ++row )
     {
         // column 0 in the layer scroll window has a wxStaticBitmap, get its ID.
         wxWindow* w = getLayerComp( row, 0 );
@@ -208,6 +211,7 @@ int LAYER_WIDGET::findLayerRow( LAYER_NUM aLayer ) const
         if( aLayer == getDecodedId( w->GetId() ))
             return row;
     }
+
     return -1;
 }
 
@@ -215,8 +219,10 @@ int LAYER_WIDGET::findLayerRow( LAYER_NUM aLayer ) const
 wxWindow* LAYER_WIDGET::getRenderComp( int aRow, int aColumn ) const
 {
     int ndx = aRow * RND_COLUMN_COUNT + aColumn;
+
     if( (unsigned) ndx < m_RenderFlexGridSizer->GetChildren().GetCount() )
         return m_RenderFlexGridSizer->GetChildren()[ndx]->GetWindow();
+
     return NULL;
 }
 
@@ -224,7 +230,8 @@ wxWindow* LAYER_WIDGET::getRenderComp( int aRow, int aColumn ) const
 int LAYER_WIDGET::findRenderRow( int aId ) const
 {
     int count = GetRenderRowCount();
-    for( int row=0;  row<count;  ++row )
+
+    for( int row = 0; row < count; ++row )
     {
         // column 0 in the layer scroll window has a wxStaticBitmap, get its ID.
         wxWindow* w = getRenderComp( row, 0 );
@@ -233,6 +240,7 @@ int LAYER_WIDGET::findRenderRow( int aId ) const
         if( aId == getDecodedId( w->GetId() ))
             return row;
     }
+
     return -1;
 }
 
@@ -422,14 +430,16 @@ wxSize LAYER_WIDGET::GetBestSize() const
     // size of m_LayerScrolledWindow --------------
     wxArrayInt widths = m_LayersFlexGridSizer->GetColWidths();
     int totWidth = 0;
+
     if( widths.GetCount() )
     {
-        for( int i=0;  i<LYR_COLUMN_COUNT;  ++i )
+        for( int i = 0; i < LYR_COLUMN_COUNT; ++i )
         {
             totWidth += widths[i] + m_LayersFlexGridSizer->GetHGap();
             // printf("widths[%d]:%d\n", i, widths[i] );
         }
     }
+
     // Account for the parent's frame:
     totWidth += 10;
 
@@ -447,9 +457,10 @@ wxSize LAYER_WIDGET::GetBestSize() const
     // size of m_RenderScrolledWindow --------------
     widths = m_RenderFlexGridSizer->GetColWidths();
     totWidth = 0;
+
     if( widths.GetCount() )
     {
-        for( int i=0;  i<RND_COLUMN_COUNT;  ++i )
+        for( int i = 0; i < RND_COLUMN_COUNT; ++i )
         {
             totWidth += widths[i] + m_RenderFlexGridSizer->GetHGap();
             // printf("widths[%d]:%d\n", i, widths[i] );
@@ -616,6 +627,7 @@ COLOR4D LAYER_WIDGET::GetLayerColor( LAYER_NUM aLayer ) const
 void LAYER_WIDGET::SetRenderState( int aId, bool isSet )
 {
     int row = findRenderRow( aId );
+
     if( row >= 0 )
     {
         int col = 1;    // checkbox is column 1
@@ -629,6 +641,7 @@ void LAYER_WIDGET::SetRenderState( int aId, bool isSet )
 bool LAYER_WIDGET::GetRenderState( int aId )
 {
     int row = findRenderRow( aId );
+
     if( row >= 0 )
     {
         int col = 1;    // checkbox is column 1
@@ -636,6 +649,7 @@ bool LAYER_WIDGET::GetRenderState( int aId )
         wxASSERT( cb );
         return cb->GetValue();
     }
+
     return false;   // the value of a non-existent row
 }
 
@@ -653,6 +667,7 @@ void LAYER_WIDGET::UpdateLayouts()
 void LAYER_WIDGET::UpdateLayerIcons()
 {
     int rowCount = GetLayerRowCount();
+
     for( int row = 0; row < rowCount ; row++ )
     {
         INDICATOR_ICON* indicator = (INDICATOR_ICON*) getLayerComp( row, COLUMN_ICON_ACTIVE );