← Back to team overview

kicad-developers team mailing list archive

[PATCH] Fix layer color swatches in Linux

 

Hi,

These patches fix the layer/render widget swatches in Linux under new
GTK+ toolkits. Fix for: https://bugs.launchpad.net/kicad/+bug/1605411

The patches remove the "button" nature of the swatches, since they
weren't actually actuated by a single click, so the button affordance
was misleading anyway. Also on OSX, the button was invisible, so it
just looked like a flat swatch anyway.

* Linux as it was:
https://drive.google.com/file/d/0BxVhl5qZbpYoZlZPeXV1Q0ttT2s/view
* OSX as it was:
https://launchpadlibrarian.net/274428737/Screen%20Shot%202016-07-22%20at%2019.40.14.png
* Linux after this patch: see attachment

This is followed by a refactor to pull the swatch logic out of the
layer widget into common, where it can be used by other clients, for
example the eeschema display color dialog, if wanted.

Patch #3 is a simple replacement of old WX Connect with Bind for
consistency in that file.

Cheers,

John
From 92185ffb58305a7975d96c8a80c641ccb6cf80a4 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Tue, 21 Feb 2017 20:43:02 +0800
Subject: [PATCH 3/3] 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 0027a99d3..372be5b9d 100644
--- a/pcbnew/layer_widget.cpp
+++ b/pcbnew/layer_widget.cpp
@@ -342,7 +342,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)
@@ -357,7 +357,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 );
 
@@ -365,7 +365,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 );
 }
@@ -403,8 +403,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 );
 }
@@ -501,8 +500,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();
 }
-- 
2.11.0

From 5230a50e22cddd52dceddd9fdc98c16b7f74f5cf Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Tue, 21 Feb 2017 20:31:19 +0800
Subject: [PATCH 2/3] 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/color_swatch.cpp | 151 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/color_swatch.h  |  88 ++++++++++++++++++++++++++++
 pcbnew/layer_widget.cpp | 134 ++++++++----------------------------------
 pcbnew/layer_widget.h   |  12 ++--
 5 files changed, 273 insertions(+), 113 deletions(-)
 create mode 100644 common/color_swatch.cpp
 create mode 100644 include/color_swatch.h

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index 09324e812..9e573edee 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -197,6 +197,7 @@ set( COMMON_SRCS
     class_marker_base.cpp
     class_plotter.cpp
     class_undoredo_container.cpp
+    color_swatch.cpp
     colors.cpp
     commit.cpp
     common.cpp
diff --git a/common/color_swatch.cpp b/common/color_swatch.cpp
new file mode 100644
index 000000000..b45b83a1b
--- /dev/null
+++ b/common/color_swatch.cpp
@@ -0,0 +1,151 @@
+/*
+ * 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 <color_swatch.h>
+
+wxDEFINE_EVENT(COLOR_SWATCH_CHANGED, wxCommandEvent);
+
+
+const static int SWATCH_SIZE_X = 14;
+const static int SWATCH_SIZE_Y = 12;
+
+// See selcolor.cpp:
+extern EDA_COLOR_T DisplayColorFrame( wxWindow* aParent, EDA_COLOR_T aOldColor );
+
+
+/**
+ * Make a simple color swatch bitmap
+ */
+static wxBitmap makeBitmap( EDA_COLOR_T aColor )
+{
+    wxBitmap    bitmap( SWATCH_SIZE_X, SWATCH_SIZE_Y );
+    wxBrush     brush;
+    wxMemoryDC  iconDC;
+
+    iconDC.SelectObject( bitmap );
+
+    brush.SetColour( MakeColour( aColor ) );
+    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, EDA_COLOR_T 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, EDA_COLOR_T aColor, int aID ):
+        wxPanel( aParent, aID ),
+        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( EDA_COLOR_T aColor, bool sendEvent )
+{
+    m_color = aColor;
+
+    wxBitmap bm = makeBitmap( aColor );
+    m_swatch->SetBitmap( bm );
+
+    if( sendEvent )
+    {
+        sendSwatchChangeEvent( *this );
+    }
+}
+
+
+EDA_COLOR_T COLOR_SWATCH::GetSwatchColor() const
+{
+    return m_color;
+}
+
+
+void COLOR_SWATCH::GetNewSwatchColor()
+{
+    EDA_COLOR_T newColor = DisplayColorFrame( this, m_color );
+
+    if( newColor >= 0 )
+    {
+        m_color = newColor;
+
+        wxBitmap bm = makeBitmap( newColor );
+        m_swatch->SetBitmap( bm );
+
+        sendSwatchChangeEvent( *this );
+    }
+}
diff --git a/include/color_swatch.h b/include/color_swatch.h
new file mode 100644
index 000000000..dcb8743ad
--- /dev/null
+++ b/include/color_swatch.h
@@ -0,0 +1,88 @@
+/*
+ * 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 <colors.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, EDA_COLOR_T aColor, int aID );
+
+    /**
+     * Set the current swatch color directly.
+     */
+    void SetSwatchColor( EDA_COLOR_T aColor, bool sendEvent );
+
+    /**
+     * @return the current swatch color
+     */
+    EDA_COLOR_T 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 );
+
+    ///> The current colour of the swatch
+    EDA_COLOR_T     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 d416d2f6b..0027a99d3 100644
--- a/pcbnew/layer_widget.cpp
+++ b/pcbnew/layer_widget.cpp
@@ -38,17 +38,12 @@
 #include <macros.h>
 #include <common.h>
 #include <colors.h>
+#include <color_swatch.h>
 #include <wx/colour.h>
 
 #include <algorithm>
 
 
-const static int SWATCH_SIZE_X = 14;
-const static int SWATCH_SIZE_Y = 12;
-
-// See selcolor.cpp:
-extern EDA_COLOR_T DisplayColorFrame( wxWindow* aParent, EDA_COLOR_T aOldColor );
-
 const wxEventType LAYER_WIDGET::EVT_LAYER_COLOR_CHANGE = wxNewEventType();
 
 /* XPM
@@ -150,57 +145,6 @@ static const char * rightarrow_alternate_xpm[] = {
 
 
 /**
- * Function makeColorTxt
- * returns a string containing the numeric value of the color.
- * in a form like 0x00000000.  (Color is currently an index, not RGB).
- */
-static wxString makeColorTxt( EDA_COLOR_T aColor )
-{
-    wxString txt;
-    txt.Printf( wxT("0x%08x"), aColor );
-    return txt;
-}
-
-
-wxBitmap makeBitmap( EDA_COLOR_T aColor )
-{
-    wxBitmap    bitmap( SWATCH_SIZE_X, SWATCH_SIZE_Y );
-    wxBrush     brush;
-    wxMemoryDC  iconDC;
-
-    iconDC.SelectObject( bitmap );
-
-    brush.SetColour( MakeColour( aColor ) );
-    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
- */
-std::unique_ptr<wxStaticBitmap> makeColorSwatch( wxWindow* aParent,
-                                                 EDA_COLOR_T 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
  */
@@ -274,31 +218,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();
+    EDA_COLOR_T newColor = eventSource->GetSwatchColor();
 
-    EDA_COLOR_T oldColor = ColorFromInt( strtoul( TO_UTF8(colorTxt), NULL, 0 ) );
-    EDA_COLOR_T newColor = DisplayColorFrame( this, oldColor );
-
-    if( newColor >= 0 )
-    {
-        eventSource->SetName( makeColorTxt( newColor ) );
-
-        wxBitmap bm = makeBitmap( newColor );
-        eventSource->SetBitmap( bm );
+    LAYER_NUM layer = getDecodedId( eventSource->GetId() );
 
-        LAYER_NUM layer = getDecodedId( eventSource->GetId() );
+    // tell the client code.
+    OnLayerColorChange( layer, newColor );
 
-        // 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();
 }
@@ -313,27 +246,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() );
-
-    wxString colorTxt = eventSource->GetName();
+    auto eventSource = static_cast<COLOR_SWATCH*>( aEvent.GetEventObject() );
 
-    EDA_COLOR_T oldColor = ColorFromInt( strtoul( TO_UTF8(colorTxt), NULL, 0 ) );
-    EDA_COLOR_T newColor = DisplayColorFrame( this, oldColor );
+    EDA_COLOR_T newColor = eventSource->GetSwatchColor();
 
-    if( newColor >= 0 )
-    {
-        eventSource->SetName( makeColorTxt( newColor ) );
-
-        wxBitmap bm = makeBitmap( newColor );
-        eventSource->SetBitmap( bm );
+    LAYER_NUM id = getDecodedId( eventSource->GetId() );
 
-        LAYER_NUM id = getDecodedId( eventSource->GetId() );
+    // tell the client code.
+    OnRenderColorChange( id, newColor );
 
-        // tell the client code.
-        OnRenderColorChange( id, newColor );
-    }
     passOnFocus();
 }
 
@@ -424,12 +347,11 @@ 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 ) );
+    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;
@@ -461,13 +383,10 @@ void LAYER_WIDGET::insertRenderRow( int aRow, const ROW& aSpec )
     col = 0;
     if( aSpec.color != -1 )
     {
-        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 ) );
+        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.
     }
@@ -769,13 +688,10 @@ void LAYER_WIDGET::SetLayerColor( LAYER_NUM aLayer, EDA_COLOR_T 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 45cf51a68..2fad22314 100644
--- a/pcbnew/layer_widget.h
+++ b/pcbnew/layer_widget.h
@@ -151,10 +151,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
@@ -163,7 +163,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 );
 
-- 
2.11.0

From 5aacddb948bf9c53076e1c366c553a843313ff98 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Tue, 21 Feb 2017 19:09:10 +0800
Subject: [PATCH 1/3] 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 | 103 ++++++++++++++++++++++++------------------------
 pcbnew/layer_widget.h   |   7 ----
 2 files changed, 51 insertions(+), 59 deletions(-)

diff --git a/pcbnew/layer_widget.cpp b/pcbnew/layer_widget.cpp
index da861fcd8..d416d2f6b 100644
--- a/pcbnew/layer_widget.cpp
+++ b/pcbnew/layer_widget.cpp
@@ -43,9 +43,8 @@
 #include <algorithm>
 
 
-#define BUTT_SIZE_X             20
-#define BUTT_SIZE_Y             18
-#define BUTT_VOID               4
+const static int SWATCH_SIZE_X = 14;
+const static int SWATCH_SIZE_Y = 12;
 
 // See selcolor.cpp:
 extern EDA_COLOR_T DisplayColorFrame( wxWindow* aParent, EDA_COLOR_T aOldColor );
@@ -163,37 +162,9 @@ static wxString makeColorTxt( EDA_COLOR_T aColor )
 }
 
 
-/**
- * Function shrinkFont
- * reduces the size of the wxFont associated with \a aControl
- */
-static void shrinkFont( wxWindow* aControl, int aPointSize )
+wxBitmap makeBitmap( EDA_COLOR_T aColor )
 {
-    wxFont font = aControl->GetFont();
-    font.SetPointSize( aPointSize );
-    aControl->SetFont( font );              // need this?
-}
-
-
-int LAYER_WIDGET::encodeId( int aColumn, int aId )
-{
-    int id = aId * LYR_COLUMN_COUNT + aColumn;
-    return id;
-}
-
-
-LAYER_NUM LAYER_WIDGET::getDecodedId( int aControlId )
-{
-    int id = aControlId / LYR_COLUMN_COUNT;    // rounding is OK.
-    return id;
-}
-
-
-wxBitmap LAYER_WIDGET::makeBitmap( EDA_COLOR_T 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 );
+    wxBitmap    bitmap( SWATCH_SIZE_X, SWATCH_SIZE_Y );
     wxBrush     brush;
     wxMemoryDC  iconDC;
 
@@ -204,31 +175,57 @@ wxBitmap LAYER_WIDGET::makeBitmap( EDA_COLOR_T aColor )
 
     iconDC.SetBrush( brush );
 
-    iconDC.DrawRectangle( 0, 0, BUTT_SIZE_X - 2 * BUTT_VOID, BUTT_SIZE_Y - 2 * BUTT_VOID );
+    iconDC.DrawRectangle( 0, 0, SWATCH_SIZE_X, SWATCH_SIZE_Y );
 
     return bitmap;
 }
 
-
-wxBitmapButton* LAYER_WIDGET::makeColorButton( wxWindow* aParent, EDA_COLOR_T aColor, int aID )
+/**
+ * Function makeColorButton
+ * creates a wxStaticBitmap and assigns it a solid color and a control ID
+ */
+std::unique_ptr<wxStaticBitmap> makeColorSwatch( wxWindow* aParent,
+                                                 EDA_COLOR_T 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.
+    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
+ */
+static void shrinkFont( wxWindow* aControl, int aPointSize )
+{
+    wxFont font = aControl->GetFont();
+    font.SetPointSize( aPointSize );
+    aControl->SetFont( font );              // need this?
+}
+
+
+int LAYER_WIDGET::encodeId( int aColumn, int aId )
+{
+    int id = aId * LYR_COLUMN_COUNT + aColumn;
+    return id;
+}
+
+
+LAYER_NUM LAYER_WIDGET::getDecodedId( int aControlId )
+{
+    int id = aControlId / LYR_COLUMN_COUNT;    // rounding is OK.
+    return id;
+}
+
+
 void LAYER_WIDGET::OnLeftDownLayers( wxMouseEvent& event )
 {
     int row;
@@ -279,7 +276,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();
 
@@ -291,7 +288,7 @@ void LAYER_WIDGET::OnMiddleDownLayerColor( wxMouseEvent& aEvent )
         eventSource->SetName( makeColorTxt( newColor ) );
 
         wxBitmap bm = makeBitmap( newColor );
-        eventSource->SetBitmapLabel( bm );
+        eventSource->SetBitmap( bm );
 
         LAYER_NUM layer = getDecodedId( eventSource->GetId() );
 
@@ -316,9 +313,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();
 
@@ -330,7 +327,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() );
 
@@ -427,12 +424,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;
@@ -464,11 +461,13 @@ void LAYER_WIDGET::insertRenderRow( int aRow, const ROW& aSpec )
     col = 0;
     if( aSpec.color != -1 )
     {
-        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 e35be8e27..45cf51a68 100644
--- a/pcbnew/layer_widget.h
+++ b/pcbnew/layer_widget.h
@@ -120,8 +120,6 @@ protected:
     int                 m_CurrentRow;           ///< selected row of layer list
     int                 m_PointSize;
 
-    static wxBitmap makeBitmap( EDA_COLOR_T aColor );
-
     /**
      * Virtual Function useAlternateBitmap
      * @return true if bitmaps shown in Render layer list
@@ -149,11 +147,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, EDA_COLOR_T aColor, int aID );
 
     void OnLeftDownLayers( wxMouseEvent& event );
 
-- 
2.11.0

Attachment: 2017-02-21-171945_126x460_scrot.png
Description: PNG image


Follow ups