From 3487124a4acefb84227feb29249bafd4cd880a29 Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Wed, 28 Jun 2023 23:54:27 -0400 Subject: [PATCH] Properties: Improve color picker UX --- common/properties/pg_cell_renderer.cpp | 8 ++-- common/properties/pg_editors.cpp | 36 +++++++++++++++++- common/properties/pg_properties.cpp | 39 ++------------------ common/widgets/color_swatch.cpp | 45 ++++++++++++++--------- eeschema/widgets/sch_properties_panel.cpp | 12 ++++++ eeschema/widgets/sch_properties_panel.h | 2 + include/properties/pg_editors.h | 24 ++++++++++++ include/properties/pg_properties.h | 4 +- include/widgets/color_swatch.h | 5 +++ 9 files changed, 116 insertions(+), 59 deletions(-) diff --git a/common/properties/pg_cell_renderer.cpp b/common/properties/pg_cell_renderer.cpp index 83bb928579..bbc31de69d 100644 --- a/common/properties/pg_cell_renderer.cpp +++ b/common/properties/pg_cell_renderer.cpp @@ -21,6 +21,7 @@ #include #include #include +#include PG_CELL_RENDERER::PG_CELL_RENDERER() : @@ -40,6 +41,7 @@ bool PG_CELL_RENDERER::Render( wxDC &aDC, const wxRect &aRect, const wxPropertyG wxAny av = colorProp->GetValue().GetAny(); KIGFX::COLOR4D color = av.IsNull() ? KIGFX::COLOR4D::UNSPECIFIED : av.As(); + KIGFX::COLOR4D background; PreDrawCell( aDC, aRect, aGrid, cell, aFlags ); @@ -47,9 +49,9 @@ bool PG_CELL_RENDERER::Render( wxDC &aDC, const wxRect &aRect, const wxPropertyG int offset = ( aRect.GetHeight() - swatchSize.GetHeight() ) / 2; wxRect swatch( aRect.GetPosition() + wxPoint( offset, offset ), swatchSize ); - aDC.SetPen( *wxTRANSPARENT_PEN ); - aDC.SetBrush( wxBrush( color.ToColour() ) ); - aDC.DrawRectangle( swatch ); + COLOR_SWATCH::RenderToDC( &aDC, color, background, swatch, + aGrid->ConvertDialogToPixels( CHECKERBOARD_SIZE_DU ), + aGrid->GetParent()->GetBackgroundColour() ); PostDrawCell( aDC, aGrid, cell, aFlags ); diff --git a/common/properties/pg_editors.cpp b/common/properties/pg_editors.cpp index a2a5ce949f..a955d05ecc 100644 --- a/common/properties/pg_editors.cpp +++ b/common/properties/pg_editors.cpp @@ -18,6 +18,7 @@ */ #include +#include #include #include #include @@ -27,6 +28,7 @@ const wxString PG_UNIT_EDITOR::EDITOR_NAME = wxS( "KiCadUnitEditor" ); const wxString PG_CHECKBOX_EDITOR::EDITOR_NAME = wxS( "KiCadCheckboxEditor" ); +const wxString PG_COLOR_EDITOR::EDITOR_NAME = wxS( "KiCadColorEditor" ); PG_UNIT_EDITOR::PG_UNIT_EDITOR( EDA_DRAW_FRAME* aFrame ) : @@ -73,7 +75,8 @@ wxPGWindowList PG_UNIT_EDITOR::CreateControls( wxPropertyGrid* aPropGrid, wxPGPr wxASSERT( m_unitBinder ); wxString text = aProperty->GetValueAsString( wxPG_EDITABLE_VALUE ); - wxWindow* win = aPropGrid->GenerateEditorTextCtrl(aPos, aSize, text, nullptr, 0, aProperty->GetMaxLength() ); + wxWindow* win = aPropGrid->GenerateEditorTextCtrl( aPos, aSize, text, nullptr, 0, + aProperty->GetMaxLength() ); wxPGWindowList ret( win, nullptr ); m_unitBinder->SetControl( win ); @@ -220,3 +223,34 @@ wxPGWindowList PG_CHECKBOX_EDITOR::CreateControls( wxPropertyGrid* aGrid, wxPGPr return wxPGCheckBoxEditor::CreateControls( aGrid, aProperty, aPos, aSize ); } + + +wxPGWindowList PG_COLOR_EDITOR::CreateControls( wxPropertyGrid* aGrid, wxPGProperty* aProperty, + const wxPoint& aPos, const wxSize& aSize ) const +{ + wxVariant val = aProperty->GetValue(); + + KIGFX::COLOR4D color = KIGFX::COLOR4D::UNSPECIFIED; + COLOR4D_VARIANT_DATA* data = nullptr; + + if( val.IsType( wxS( "COLOR4D" ) ) ) + { + data = static_cast( val.GetData() ); + color = data->Color(); + } + + DIALOG_COLOR_PICKER dialog( ::wxGetTopLevelParent( aGrid ), color, true, + nullptr, KIGFX::COLOR4D::UNSPECIFIED ); + + int res = dialog.ShowModal(); + + if( res == wxID_OK ) + { + data = new COLOR4D_VARIANT_DATA(); + data->SetColor( dialog.GetColor() ); + val.SetData( data ); + aGrid->ChangePropertyValue( aProperty, val ); + } + + return nullptr; +} diff --git a/common/properties/pg_properties.cpp b/common/properties/pg_properties.cpp index 1f314faaed..3c3ae47f57 100644 --- a/common/properties/pg_properties.cpp +++ b/common/properties/pg_properties.cpp @@ -24,7 +24,6 @@ #include #include -#include #include #include #include @@ -408,8 +407,10 @@ const wxPGEditor* PGPROPERTY_BOOL::DoGetEditorClass() const PGPROPERTY_COLOR4D::PGPROPERTY_COLOR4D( const wxString& aLabel, const wxString& aName, COLOR4D aValue ) : - wxLongStringProperty( aLabel, aName, aValue.ToCSSString() ) + wxStringProperty( aLabel, aName, aValue.ToCSSString() ) { + SetEditor( PG_COLOR_EDITOR::EDITOR_NAME ); + SetFlag( wxPG_PROP_NOEDITOR ); } @@ -428,39 +429,7 @@ wxString PGPROPERTY_COLOR4D::ValueToString( wxVariant& aValue, int aFlags ) cons if( aValue.IsType( wxS( "COLOR4D" ) ) ) static_cast( aValue.GetData() )->Write( ret ); else - return wxLongStringProperty::ValueToString( aValue, aFlags ); + return wxStringProperty::ValueToString( aValue, aFlags ); return ret; } - - -bool PGPROPERTY_COLOR4D::DisplayEditorDialog( wxPropertyGrid* aGrid, wxVariant& aValue ) -{ - KIGFX::COLOR4D color = KIGFX::COLOR4D::UNSPECIFIED; - - COLOR4D_VARIANT_DATA* data = nullptr; - - if( aValue.IsType( wxS( "COLOR4D" ) ) ) - { - data = static_cast( aValue.GetData() ); - color = data->Color(); - } - - // TODO: Disable opacity where required - DIALOG_COLOR_PICKER dialog( ::wxGetTopLevelParent( aGrid ), color, true, - nullptr, KIGFX::COLOR4D::UNSPECIFIED ); - - int res = dialog.ShowModal(); - - if( res == wxID_OK ) - { - if( !data ) - data = new COLOR4D_VARIANT_DATA(); - - data->SetColor( dialog.GetColor() ); - aValue.SetData( data ); - return true; - } - - return false; -} diff --git a/common/widgets/color_swatch.cpp b/common/widgets/color_swatch.cpp index 78c183262b..87ddae63b1 100644 --- a/common/widgets/color_swatch.cpp +++ b/common/widgets/color_swatch.cpp @@ -41,12 +41,25 @@ wxBitmap COLOR_SWATCH::MakeBitmap( const COLOR4D& aColor, const COLOR4D& aBackgr const wxSize& aSize, const wxSize& aCheckerboardSize, const COLOR4D& aCheckerboardBackground ) { - wxBitmap bitmap( aSize ); - wxBrush brush; - wxPen pen; - wxMemoryDC iconDC; + wxBitmap bitmap( aSize ); + wxMemoryDC iconDC; iconDC.SelectObject( bitmap ); + + RenderToDC( &iconDC, aColor, aBackground, aSize, aCheckerboardSize, aCheckerboardBackground ); + + return bitmap; +} + + +void COLOR_SWATCH::RenderToDC( wxDC* aDC, const KIGFX::COLOR4D& aColor, + const KIGFX::COLOR4D& aBackground, const wxRect& aRect, + const wxSize& aCheckerboardSize, + const KIGFX::COLOR4D& aCheckerboardBackground ) +{ + wxBrush brush; + wxPen pen; + brush.SetStyle( wxBRUSHSTYLE_SOLID ); if( aColor == COLOR4D::UNSPECIFIED ) @@ -69,19 +82,19 @@ wxBitmap COLOR_SWATCH::MakeBitmap( const COLOR4D& aColor, const COLOR4D& aBackgr rowCycle = false; } - for( int x = 0; x < aSize.x; x += aCheckerboardSize.x ) + for( int x = aRect.GetTop(); x < aRect.GetBottom(); x += aCheckerboardSize.x ) { bool colCycle = rowCycle; - for( int y = 0; y < aSize.y; y += aCheckerboardSize.y ) + for( int y = aRect.GetLeft(); y < aRect.GetRight(); y += aCheckerboardSize.y ) { COLOR4D color = colCycle ? black : white; brush.SetColour( color.ToColour() ); pen.SetColour( color.ToColour() ); - iconDC.SetBrush( brush ); - iconDC.SetPen( pen ); - iconDC.DrawRectangle( x, y, x + aCheckerboardSize.x, y + aCheckerboardSize.y ); + aDC->SetBrush( brush ); + aDC->SetPen( pen ); + aDC->DrawRectangle( x, y, x + aCheckerboardSize.x, y + aCheckerboardSize.y ); colCycle = !colCycle; } @@ -94,19 +107,17 @@ wxBitmap COLOR_SWATCH::MakeBitmap( const COLOR4D& aColor, const COLOR4D& aBackgr brush.SetColour( aBackground.WithAlpha(1.0).ToColour() ); pen.SetColour( aBackground.WithAlpha(1.0).ToColour() ); - iconDC.SetBrush( brush ); - iconDC.SetPen( pen ); - iconDC.DrawRectangle( 0, 0, aSize.x, aSize.y ); + aDC->SetBrush( brush ); + aDC->SetPen( pen ); + aDC->DrawRectangle( aRect ); brush.SetColour( aColor.ToColour() ); pen.SetColour( aColor.ToColour() ); - iconDC.SetBrush( brush ); - iconDC.SetPen( pen ); - iconDC.DrawRectangle( 0, 0, aSize.x, aSize.y ); + aDC->SetBrush( brush ); + aDC->SetPen( pen ); + aDC->DrawRectangle( aRect ); } - - return bitmap; } diff --git a/eeschema/widgets/sch_properties_panel.cpp b/eeschema/widgets/sch_properties_panel.cpp index dba021c3cb..3b96d3dad2 100644 --- a/eeschema/widgets/sch_properties_panel.cpp +++ b/eeschema/widgets/sch_properties_panel.cpp @@ -72,6 +72,18 @@ SCH_PROPERTIES_PANEL::SCH_PROPERTIES_PANEL( wxWindow* aParent, SCH_BASE_FRAME* a { m_checkboxEditorInstance = static_cast( it->second ); } + + it = wxPGGlobalVars->m_mapEditorClasses.find( PG_COLOR_EDITOR::EDITOR_NAME ); + + if( it == wxPGGlobalVars->m_mapEditorClasses.end() ) + { + PG_COLOR_EDITOR* colorEditor = new PG_COLOR_EDITOR(); + m_colorEditorInstance = static_cast( wxPropertyGrid::RegisterEditorClass( colorEditor ) ); + } + else + { + m_colorEditorInstance = static_cast( it->second ); + } } diff --git a/eeschema/widgets/sch_properties_panel.h b/eeschema/widgets/sch_properties_panel.h index 451adae10a..3f6d8f9846 100644 --- a/eeschema/widgets/sch_properties_panel.h +++ b/eeschema/widgets/sch_properties_panel.h @@ -30,6 +30,7 @@ class SCH_BASE_FRAME; class PROPERTY_MANAGER; class PG_UNIT_EDITOR; class PG_CHECKBOX_EDITOR; +class PG_COLOR_EDITOR; class SCH_PROPERTIES_PANEL : public PROPERTIES_PANEL { @@ -57,6 +58,7 @@ protected: PROPERTY_MANAGER& m_propMgr; PG_UNIT_EDITOR* m_unitEditorInstance; PG_CHECKBOX_EDITOR* m_checkboxEditorInstance; + PG_COLOR_EDITOR* m_colorEditorInstance; wxPGChoices m_nets; }; diff --git a/include/properties/pg_editors.h b/include/properties/pg_editors.h index 9770186937..0d21297441 100644 --- a/include/properties/pg_editors.h +++ b/include/properties/pg_editors.h @@ -84,4 +84,28 @@ public: const wxPoint& aPos, const wxSize& aSize ) const override; }; + +class PG_COLOR_EDITOR : public wxPGEditor +{ +public: + static const wxString EDITOR_NAME; + + PG_COLOR_EDITOR() {} + + virtual ~PG_COLOR_EDITOR() {} + + wxString GetName() const override { return EDITOR_NAME; } + + wxPGWindowList CreateControls( wxPropertyGrid* aGrid, wxPGProperty* aProperty, + const wxPoint& aPos, const wxSize& aSize ) const override; + + void UpdateControl( wxPGProperty* aProperty, wxWindow* aCtrl ) const override {} + + bool OnEvent( wxPropertyGrid* aGrid, wxPGProperty* aProperty, wxWindow* aWindow, + wxEvent& aEvent ) const override + { + return false; + } +}; + #endif //KICAD_PG_EDITORS_H diff --git a/include/properties/pg_properties.h b/include/properties/pg_properties.h index 9164e4337b..c9b3270649 100644 --- a/include/properties/pg_properties.h +++ b/include/properties/pg_properties.h @@ -190,7 +190,7 @@ public: }; -class PGPROPERTY_COLOR4D : public wxLongStringProperty +class PGPROPERTY_COLOR4D : public wxStringProperty { public: PGPROPERTY_COLOR4D( const wxString& aLabel = wxPG_LABEL, const wxString& aName = wxPG_LABEL, @@ -202,8 +202,6 @@ public: bool StringToValue( wxVariant &aVariant, const wxString &aText, int aFlags = 0 ) const override; - - bool DisplayEditorDialog( wxPropertyGrid* aGrid, wxVariant& aValue ) override; }; #endif /* PG_PROPERTIES_H */ diff --git a/include/widgets/color_swatch.h b/include/widgets/color_swatch.h index ae7b1030bf..dcf8d852b5 100644 --- a/include/widgets/color_swatch.h +++ b/include/widgets/color_swatch.h @@ -131,6 +131,11 @@ public: const wxSize& aSize, const wxSize& aCheckerboardSize, const KIGFX::COLOR4D& aCheckerboardBackground ); + static void RenderToDC( wxDC* aDC, const KIGFX::COLOR4D& aColor, + const KIGFX::COLOR4D& aBackground, const wxRect& aRect, + const wxSize& aCheckerboardSize, + const KIGFX::COLOR4D& aCheckerboardBackground ); + private: void setupEvents();