From 9939af3e270089347727abe19bd04dad5d92bb33 Mon Sep 17 00:00:00 2001 From: Ian McInerney Date: Fri, 9 Apr 2021 00:51:30 +0100 Subject: [PATCH] Fix color handling in the stackup manager panel Fixes https://gitlab.com/kicad/code/kicad/issues/5671 --- .../panel_board_stackup.cpp | 89 +++++++++++-------- .../panel_board_stackup.h | 23 +++-- 2 files changed, 65 insertions(+), 47 deletions(-) diff --git a/pcbnew/board_stackup_manager/panel_board_stackup.cpp b/pcbnew/board_stackup_manager/panel_board_stackup.cpp index d2ec6b15c1..9e94382a26 100644 --- a/pcbnew/board_stackup_manager/panel_board_stackup.cpp +++ b/pcbnew/board_stackup_manager/panel_board_stackup.cpp @@ -87,8 +87,6 @@ PANEL_SETUP_BOARD_STACKUP::PANEL_SETUP_BOARD_STACKUP( PAGED_DIALOG* aParent, PCB // Calculates a good size for color swatches (icons) in this dialog wxClientDC dc( this ); m_colorSwatchesSize = dc.GetTextExtent( "XX" ); - m_colorComboSize = dc.GetTextExtent( wxString::Format( "XXX %s XXX", - wxGetTranslation( NotSpecifiedPrm() ) ) ); m_colorIconsSize = dc.GetTextExtent( "XXXX" ); // Calculates a good size for wxTextCtrl to enter Epsilon R and Loss tan @@ -313,10 +311,8 @@ wxColor PANEL_SETUP_BOARD_STACKUP::GetSelectedColor( int aRow ) const if( idx != GetColorUserDefinedListIdx() ) // a standard color is selected return GetColorStandardList()[idx].m_Color; - else if( m_UserColors.count( aRow ) ) - return m_UserColors.at( aRow ); - else - return wxNullColour; + + return m_rowUiItemsList[aRow].m_UserColor; } @@ -376,8 +372,6 @@ void PANEL_SETUP_BOARD_STACKUP::synchronizeWithBoard( bool aFullSync ) m_impedanceControlled->SetValue( brd_stackup.m_HasDielectricConstrains ); } - int row = 0; - for( BOARD_STACKUP_ROW_UI_ITEM& ui_row_item : m_rowUiItemsList ) { BOARD_STACKUP_ITEM* item = ui_row_item.m_Item; @@ -429,12 +423,15 @@ void PANEL_SETUP_BOARD_STACKUP::synchronizeWithBoard( bool aFullSync ) if( item->GetColor().StartsWith( "#" ) ) // User defined color { wxColour color( item->GetColor() ); - m_UserColors[row] = color; + ui_row_item.m_UserColor = color; color_idx = GetColorUserDefinedListIdx(); if( bm_combo ) // Update user color shown in the wxBitmapComboBox { - bm_combo->SetString( color_idx, color.GetAsString( wxC2S_HTML_SYNTAX ) ); + wxString label = wxString::Format( _( "Custom (%s)" ), + color.GetAsString( wxC2S_HTML_SYNTAX ) ); + + bm_combo->SetString( color_idx, label ); wxBitmap layerbmp( m_colorSwatchesSize.x, m_colorSwatchesSize.y ); LAYER_SELECTOR::DrawColorSwatch( layerbmp, COLOR4D(), COLOR4D( color ) ); bm_combo->SetItemBitmap( color_idx, layerbmp ); @@ -683,13 +680,17 @@ BOARD_STACKUP_ROW_UI_ITEM PANEL_SETUP_BOARD_STACKUP::createRowData( int aRow, if( item->IsColorEditable() ) { - int color_idx = 0; + int color_idx = 0; + int user_color_idx = GetColorUserDefinedListIdx(); + + // Always init the user-defined color for a row + ui_row_item.m_UserColor = GetColorStandardList()[user_color_idx].m_Color; if( item->GetColor().StartsWith( "#" ) ) // User defined color { wxColour color( item->GetColor() ); - m_UserColors[row] = color; - color_idx = GetColorUserDefinedListIdx(); + ui_row_item.m_UserColor = color; + color_idx = user_color_idx; } else { @@ -704,8 +705,6 @@ BOARD_STACKUP_ROW_UI_ITEM PANEL_SETUP_BOARD_STACKUP::createRowData( int aRow, } wxBitmapComboBox* bm_combo = createBmComboBox( item, row ); - m_colorComboSize.y = bm_combo->GetSize().y; - bm_combo->SetMinSize( m_colorComboSize ); m_fgGridSizer->Add( bm_combo, 0, wxLEFT|wxRIGHT|wxALIGN_CENTER_VERTICAL, 2 ); bm_combo->SetSelection( color_idx ); ui_row_item.m_ColorCtrl = bm_combo; @@ -778,7 +777,6 @@ void PANEL_SETUP_BOARD_STACKUP::rebuildLayerStackPanel() } m_rowUiItemsList.clear(); - m_UserColors.clear(); // In order to recreate a clean grid layer list, we have to delete and // recreate the sizer m_fgGridSizer (just deleting items in this size is not enough) @@ -1019,8 +1017,7 @@ bool PANEL_SETUP_BOARD_STACKUP::transferDataFromUIToStackup() if( idx == GetColorUserDefinedListIdx() ) { - wxASSERT( m_UserColors.count( row ) ); - wxColour color = m_UserColors[row]; + wxColour color = ui_item.m_UserColor; item->SetColor( color.GetAsString( wxC2S_HTML_SYNTAX ) ); } else @@ -1147,20 +1144,28 @@ void PANEL_SETUP_BOARD_STACKUP::onColorSelected( wxCommandEvent& event ) if( GetColorStandardListCount()-1 == idx ) // Set user color is the last option in list { - COLOR4D defaultColor( GetColorStandardList()[GetColorUserDefinedListIdx()].m_Color ); - COLOR4D currentColor( - m_UserColors.count( row ) ? m_UserColors[row] : COLOR4D( 0.5, 0.5, 0.5, 1.0 ) ); + wxColour userColour = m_rowUiItemsList[row].m_UserColor; + COLOR4D currentColor( userColour.IsOk() ? userColour : COLOR4D( 0.5, 0.5, 0.5, 1.0 ) ); + COLOR4D defaultColor( GetColorStandardList()[GetColorUserDefinedListIdx()].m_Color ); DIALOG_COLOR_PICKER dlg( this, currentColor, false, nullptr, defaultColor ); + // Give a time-slice to close the menu before opening the dialog. + // (Only matters on some versions of GTK.) + wxSafeYield(); + if( dlg.ShowModal() == wxID_OK ) { wxBitmapComboBox* combo = static_cast( FindWindowById( item_id ) ); - wxColour color = dlg.GetColor().ToColour(); - m_UserColors[row] = color; + wxColour color = dlg.GetColor().ToColour(); - combo->SetString( idx, color.GetAsString( wxC2S_HTML_SYNTAX ) ); + m_rowUiItemsList[row].m_UserColor = color; + + wxString label = wxString::Format( _( "Custom (%s)" ), + color.GetAsString( wxC2S_HTML_SYNTAX ) ); + + combo->SetString( idx, label ); wxBitmap layerbmp( m_colorSwatchesSize.x, m_colorSwatchesSize.y ); LAYER_SELECTOR::DrawColorSwatch( layerbmp, COLOR4D( 0, 0, 0, 0 ), @@ -1316,6 +1321,8 @@ wxColor PANEL_SETUP_BOARD_STACKUP::getColorIconItem( int aRow ) break; } + wxASSERT_MSG( color.IsOk(), "Invalid color in PCB stackup" ); + return color; } @@ -1325,6 +1332,7 @@ void PANEL_SETUP_BOARD_STACKUP::updateIconColor( int aRow ) if( aRow >= 0 ) { wxStaticBitmap* st_bitmap = m_rowUiItemsList[aRow].m_Icon; + // explicit depth important under MSW wxBitmap bmp( m_colorIconsSize.x, m_colorIconsSize.y / 2, 28 ); drawBitmap( bmp, getColorIconItem( aRow ) ); @@ -1348,6 +1356,7 @@ wxBitmapComboBox* PANEL_SETUP_BOARD_STACKUP::createBmComboBox( BOARD_STACKUP_ITE wxBitmapComboBox* combo = new wxBitmapComboBox( m_scGridWin, ID_ITEM_COLOR + aRow, wxEmptyString, wxDefaultPosition, wxDefaultSize, 0, nullptr, wxCB_READONLY ); + // Fills the combo box with choice list + bitmaps const FAB_LAYER_COLOR* color_list = GetColorStandardList(); @@ -1366,14 +1375,10 @@ wxBitmapComboBox* PANEL_SETUP_BOARD_STACKUP::createBmComboBox( BOARD_STACKUP_ITE else // Append the user color, if specified, else add a default user color { if( aStackupItem && aStackupItem->GetColor().StartsWith( "#" ) ) - { curr_color = wxColour( aStackupItem->GetColor() ); - label = aStackupItem->GetColor(); - } - else - { - label = curr_color.GetAsString( wxC2S_HTML_SYNTAX ); - } + + label = wxString::Format( _( "Custom (%s)" ), + curr_color.GetAsString( wxC2S_HTML_SYNTAX ) ); } wxBitmap layerbmp( m_colorSwatchesSize.x, m_colorSwatchesSize.y ); @@ -1382,14 +1387,20 @@ wxBitmapComboBox* PANEL_SETUP_BOARD_STACKUP::createBmComboBox( BOARD_STACKUP_ITE combo->Append( label, layerbmp ); } -#ifdef __WXGTK__ - // Adjust the minimal width. On GTK, the size calculated by wxWidgets is not good - // bitmaps are ignored - combo->Fit(); - int min_width = combo->GetSize().x; - min_width += m_colorSwatchesSize.x; - combo->SetMinSize( wxSize( min_width, -1 ) ); -#endif + // Ensure the size of the widget is enough to show the text and the icon + // We have to have a selected item when doing this, because otherwise GTK + // will just choose a random size that might not fit the actual data + // (such as in cases where the font size is very large). So we select + // the longest item (which should be the last item), and size it that way. + int sel = combo->GetSelection(); + combo->SetSelection( combo->GetCount() - 1 ); + + combo->SetMinSize( wxSize( -1, -1 ) ); + wxSize bestSize = combo->GetBestSize(); + + bestSize.x = bestSize.x + m_colorSwatchesSize.x; + combo->SetMinSize( bestSize ); + combo->SetSelection( sel ); // add the wxBitmapComboBox to wxControl list, to be able to disconnect the event // on exit diff --git a/pcbnew/board_stackup_manager/panel_board_stackup.h b/pcbnew/board_stackup_manager/panel_board_stackup.h index 5b701f7887..374c95e0d3 100644 --- a/pcbnew/board_stackup_manager/panel_board_stackup.h +++ b/pcbnew/board_stackup_manager/panel_board_stackup.h @@ -28,6 +28,7 @@ #include #include +#include #include "panel_board_stackup_base.h" #include "class_board_stackup.h" @@ -63,14 +64,23 @@ struct BOARD_STACKUP_ROW_UI_ITEM wxControl* m_EpsilonCtrl; // control shown in column 8 wxControl* m_LossTgCtrl; // control shown in column 9 + wxColour m_UserColor; // User-specified color (if any) + BOARD_STACKUP_ROW_UI_ITEM( BOARD_STACKUP_ITEM* aItem, int aSubItem = 1 ) : - m_Item( aItem ), m_SubItem( aSubItem ), - m_isEnabled( true ), m_Icon( nullptr ), m_LayerName( nullptr ), + m_Item( aItem ), + m_SubItem( aSubItem ), + m_isEnabled( true ), + m_Icon( nullptr ), + m_LayerName( nullptr ), m_LayerTypeCtrl( nullptr ), - m_MaterialCtrl( nullptr ),m_MaterialButt( nullptr ), - m_ThicknessCtrl( nullptr ), m_ThicknessLockCtrl( nullptr ), + m_MaterialCtrl( nullptr ), + m_MaterialButt( nullptr ), + m_ThicknessCtrl( nullptr ), + m_ThicknessLockCtrl( nullptr ), m_ColorCtrl( nullptr ), - m_EpsilonCtrl( nullptr ), m_LossTgCtrl( nullptr ) + m_EpsilonCtrl( nullptr ), + m_LossTgCtrl( nullptr ), + m_UserColor( wxNullColour ) {} }; @@ -110,8 +120,6 @@ public: // Called by wxWidgets: transfer current settings stored in m_stackup to the board bool TransferDataFromWindow() override; - std::map m_UserColors; // the list of user colors for each grid row - // other colors are defined colors, and are not stored private: /** Creates a BOARD_STACKUP_ROW_UI_ITEM relative to the aStackupItem. * @return a BOARD_STACKUP_ROW_UI_ITEM filled with corresponding widgets @@ -245,7 +253,6 @@ private: wxSize m_numericFieldsSize; // Best size to enter double values in wxTextCtrl wxArrayString m_core_prepreg_choice; // Used to display the option list in dialog wxSize m_colorSwatchesSize; // the size of color swatches in the wxBitmapComboBox. - wxSize m_colorComboSize; // the size of the wxBitmapComboBox. wxSize m_colorIconsSize; // the size of color swatches in grid, left column. // The list of controls (wxChoice, wxBitmapComboBox, wxTextCtrl) added to the panel