kicad-developers team mailing list archive

[Maciej Sumiński <maciej.suminski@xxxxxxx>]

Hi Jean-Pierre,

Thank you, the new color picker looks very nice. I noticed is there is a
typo in the right sizer ('RGB Values' vs 'HS Values'), and I am getting
an assert when opening the dialog. Both problems are fixed in the
attached patch.

I do not want to be a complainer, but I would consider two things:
- Turn the vertical 'Value' slider into a spin box control, so the look
is consistent (RGB and HSV values selectors).
- Do you think it would be hard to select color by clicking on the HSV map?

No matter what is the answer to the above questions, I am in favor of
committing the patch as in the current state it is already an improvement.

Regards,
Orson

On 07/17/2017 08:20 PM, jp charras wrote:
> Hi alls,
> 
> Attached a patch which adds a new (and I hope better) color selector for GAL canvas, with support of
> opacity.
> 
> This patch also ensures the color opacity is now stored in pcbnew config.
> So it should be not lost between sessions,
> and not lost when switching between legacy and gal canvases.
> 
> Of course, because the legacy canvas has no support of opacity, there is no change in legacy canvas,
> unless bug.
> 
> Please test it.
> 
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 

From 42b75d3a20e4836795e7f81c9ce92f26d540cada Mon Sep 17 00:00:00 2001
From: Maciej Suminski <maciej.suminski@xxxxxxx>
Date: Wed, 19 Jul 2017 09:33:26 +0200
Subject: [PATCH] COLOR4D_PICKER assert and typo fix

---
 common/widgets/color4Dpickerdlg_base.cpp | 27 +++++++++++++--------------
 common/widgets/color4Dpickerdlg_base.fbp |  6 +++---
 common/widgets/color4Dpickerdlg_base.h   |  2 +-
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/common/widgets/color4Dpickerdlg_base.cpp b/common/widgets/color4Dpickerdlg_base.cpp
index e1f30e958..62d07b49a 100644
--- a/common/widgets/color4Dpickerdlg_base.cpp
+++ b/common/widgets/color4Dpickerdlg_base.cpp
@@ -1,5 +1,5 @@
 ///////////////////////////////////////////////////////////////////////////
-// C++ code generated with wxFormBuilder (version Jul  2 2017)
+// C++ code generated with wxFormBuilder (version Oct 17 2016)
 // http://www.wxformbuilder.org/
 //
 // PLEASE DO "NOT" EDIT THIS FILE!
@@ -125,36 +125,35 @@ COLOR4D_PICKER_DLG_BASE::COLOR4D_PICKER_DLG_BASE( wxWindow* parent, wxWindowID i
 	bSizerLowerFreeColors->Add( sbSizerSetRGB, 1, wxEXPAND, 5 );
 	
 	wxStaticBoxSizer* sbSizerSetHSV;
-	sbSizerSetHSV = new wxStaticBoxSizer( new wxStaticBox( m_panelFreeColors, wxID_ANY, wxT("RGB Values") ), wxHORIZONTAL );
+	sbSizerSetHSV = new wxStaticBoxSizer( new wxStaticBox( m_panelFreeColors, wxID_ANY, wxT("HS Values") ), wxHORIZONTAL );
 	
-	wxFlexGridSizer* fgSizerHSB;
-	fgSizerHSB = new wxFlexGridSizer( 0, 2, 0, 0 );
-	fgSizerHSB->AddGrowableCol( 0 );
-	fgSizerHSB->AddGrowableCol( 1 );
-	fgSizerHSB->AddGrowableCol( 2 );
-	fgSizerHSB->SetFlexibleDirection( wxBOTH );
-	fgSizerHSB->SetNonFlexibleGrowMode( wxFLEX_GROWMODE_SPECIFIED );
+	wxFlexGridSizer* fgSizerHSV;
+	fgSizerHSV = new wxFlexGridSizer( 0, 2, 0, 0 );
+	fgSizerHSV->AddGrowableCol( 0 );
+	fgSizerHSV->AddGrowableCol( 1 );
+	fgSizerHSV->SetFlexibleDirection( wxBOTH );
+	fgSizerHSV->SetNonFlexibleGrowMode( wxFLEX_GROWMODE_SPECIFIED );
 	
 	m_staticTextHue = new wxStaticText( sbSizerSetHSV->GetStaticBox(), wxID_ANY, wxT("Hue"), wxDefaultPosition, wxDefaultSize, 0 );
 	m_staticTextHue->Wrap( -1 );
-	fgSizerHSB->Add( m_staticTextHue, 0, wxTOP|wxRIGHT|wxLEFT, 5 );
+	fgSizerHSV->Add( m_staticTextHue, 0, wxTOP|wxRIGHT|wxLEFT, 5 );
 	
 	m_staticTextSat = new wxStaticText( sbSizerSetHSV->GetStaticBox(), wxID_ANY, wxT("Saturation"), wxDefaultPosition, wxDefaultSize, 0 );
 	m_staticTextSat->Wrap( -1 );
-	fgSizerHSB->Add( m_staticTextSat, 0, wxTOP|wxRIGHT|wxLEFT, 5 );
+	fgSizerHSV->Add( m_staticTextSat, 0, wxTOP|wxRIGHT|wxLEFT, 5 );
 	
 	m_spinCtrlHue = new wxSpinCtrl( sbSizerSetHSV->GetStaticBox(), wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, wxSP_ARROW_KEYS|wxSP_WRAP, 0, 359, 0 );
 	m_spinCtrlHue->SetMinSize( wxSize( 80,-1 ) );
 	
-	fgSizerHSB->Add( m_spinCtrlHue, 0, wxALL|wxEXPAND, 5 );
+	fgSizerHSV->Add( m_spinCtrlHue, 0, wxALL|wxEXPAND, 5 );
 	
 	m_spinCtrlSaturation = new wxSpinCtrl( sbSizerSetHSV->GetStaticBox(), wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, wxSP_ARROW_KEYS, 0, 255, 128 );
 	m_spinCtrlSaturation->SetMinSize( wxSize( 80,-1 ) );
 	
-	fgSizerHSB->Add( m_spinCtrlSaturation, 0, wxALL|wxEXPAND, 5 );
+	fgSizerHSV->Add( m_spinCtrlSaturation, 0, wxALL|wxEXPAND, 5 );
 	
 	
-	sbSizerSetHSV->Add( fgSizerHSB, 1, wxEXPAND, 5 );
+	sbSizerSetHSV->Add( fgSizerHSV, 1, wxEXPAND, 5 );
 	
 	
 	bSizerLowerFreeColors->Add( sbSizerSetHSV, 1, wxEXPAND, 5 );
diff --git a/common/widgets/color4Dpickerdlg_base.fbp b/common/widgets/color4Dpickerdlg_base.fbp
index e7fe5e219..fdb5afbdf 100644
--- a/common/widgets/color4Dpickerdlg_base.fbp
+++ b/common/widgets/color4Dpickerdlg_base.fbp
@@ -1273,7 +1273,7 @@
                                                         <property name="proportion">1</property>
                                                         <object class="wxStaticBoxSizer" expanded="1">
                                                             <property name="id">wxID_ANY</property>
-                                                            <property name="label">RGB Values</property>
+                                                            <property name="label">HS Values</property>
                                                             <property name="minimum_size"></property>
                                                             <property name="name">sbSizerSetHSV</property>
                                                             <property name="orient">wxHORIZONTAL</property>
@@ -1287,11 +1287,11 @@
                                                                 <object class="wxFlexGridSizer" expanded="1">
                                                                     <property name="cols">2</property>
                                                                     <property name="flexible_direction">wxBOTH</property>
-                                                                    <property name="growablecols">0,1,2</property>
+                                                                    <property name="growablecols">0,1</property>
                                                                     <property name="growablerows"></property>
                                                                     <property name="hgap">0</property>
                                                                     <property name="minimum_size"></property>
-                                                                    <property name="name">fgSizerHSB</property>
+                                                                    <property name="name">fgSizerHSV</property>
                                                                     <property name="non_flexible_grow_mode">wxFLEX_GROWMODE_SPECIFIED</property>
                                                                     <property name="permission">none</property>
                                                                     <property name="rows">0</property>
diff --git a/common/widgets/color4Dpickerdlg_base.h b/common/widgets/color4Dpickerdlg_base.h
index 867f1856d..d51b0ce54 100644
--- a/common/widgets/color4Dpickerdlg_base.h
+++ b/common/widgets/color4Dpickerdlg_base.h
@@ -1,5 +1,5 @@
 ///////////////////////////////////////////////////////////////////////////
-// C++ code generated with wxFormBuilder (version Jul  2 2017)
+// C++ code generated with wxFormBuilder (version Oct 17 2016)
 // http://www.wxformbuilder.org/
 //
 // PLEASE DO "NOT" EDIT THIS FILE!
-- 
2.13.3

Attachment: signature.asc
Description: OpenPGP digital signature