From ff0c2b6fe0b3eef225a0fd7ce2da9672993bf8bc Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Tue, 6 Jan 2026 12:41:45 -0800 Subject: [PATCH] Clean up UI for resetting unit groups By default, we want to maintain unit groups for symbols between annotations. But it may be desired to reset them in case the annotation is mixed up and you want a graphical reannotation. This provides a checkbox to make it clear that we do not reset groups by default but you can if you want to. Fixes https://gitlab.com/kicad/code/kicad/-/issues/14259 --- eeschema/annotate.cpp | 42 +++- eeschema/dialogs/dialog_annotate.cpp | 22 +- eeschema/dialogs/dialog_annotate_base.cpp | 27 ++- eeschema/dialogs/dialog_annotate_base.fbp | 261 +++++++++++++++++----- eeschema/dialogs/dialog_annotate_base.h | 7 +- eeschema/eeschema_settings.cpp | 3 + eeschema/eeschema_settings.h | 1 + eeschema/sch_edit_frame.h | 2 +- eeschema/tools/sch_drawing_tools.cpp | 3 +- eeschema/tools/sch_edit_tool.cpp | 3 +- 10 files changed, 288 insertions(+), 83 deletions(-) diff --git a/eeschema/annotate.cpp b/eeschema/annotate.cpp index 3ca185824a..0754f86a76 100644 --- a/eeschema/annotate.cpp +++ b/eeschema/annotate.cpp @@ -22,6 +22,7 @@ */ #include +#include #include #include @@ -228,7 +229,8 @@ std::unordered_set getInferredSymbols( const SCH_SELECTION& aSelect void SCH_EDIT_FRAME::AnnotateSymbols( SCH_COMMIT* aCommit, ANNOTATE_SCOPE_T aAnnotateScope, ANNOTATE_ORDER_T aSortOption, ANNOTATE_ALGO_T aAlgoOption, bool aRecursive, int aStartNumber, bool aResetAnnotation, - bool aRepairTimestamps, REPORTER& aReporter ) + bool aRegroupUnits, bool aRepairTimestamps, + REPORTER& aReporter ) { SCH_SELECTION_TOOL* selTool = m_toolManager->GetTool(); SCH_SELECTION& selection = selTool->GetSelection(); @@ -296,10 +298,10 @@ void SCH_EDIT_FRAME::AnnotateSymbols( SCH_COMMIT* aCommit, ANNOTATE_SCOPE_T aAn } } - // Collect all the sets that must be annotated together. When resetting annotations, we skip - // this step because we want fresh groupings based on symbol placement, not the potentially - // incorrect groupings from existing reference designators. - if( !aResetAnnotation ) + // Collect all the sets that must be annotated together. When regrouping, we skip this step + // to allow fresh groupings based on symbol placement. When resetting without regrouping, we + // collect locked symbols but then check for unit conflicts (duplicate units within a ref). + if( !aRegroupUnits ) { switch( aAnnotateScope ) { @@ -324,6 +326,36 @@ void SCH_EDIT_FRAME::AnnotateSymbols( SCH_COMMIT* aCommit, ANNOTATE_SCOPE_T aAn break; } + + // When resetting annotations, check for and remove groups with unit conflicts (duplicate + // units within the same reference designator). These will get fresh assignments. + if( aResetAnnotation ) + { + std::vector conflictingRefs; + + for( auto& [refBase, refList] : lockedSymbols ) + { + std::set seenUnits; + bool hasConflict = false; + + for( const SCH_REFERENCE& ref : refList ) + { + if( seenUnits.count( ref.GetUnit() ) ) + { + hasConflict = true; + break; + } + + seenUnits.insert( ref.GetUnit() ); + } + + if( hasConflict ) + conflictingRefs.push_back( refBase ); + } + + for( const wxString& ref : conflictingRefs ) + lockedSymbols.erase( ref ); + } } // Store previous annotations for building info messages diff --git a/eeschema/dialogs/dialog_annotate.cpp b/eeschema/dialogs/dialog_annotate.cpp index ebde35dbde..39e8f4296d 100644 --- a/eeschema/dialogs/dialog_annotate.cpp +++ b/eeschema/dialogs/dialog_annotate.cpp @@ -131,10 +131,9 @@ DIALOG_ANNOTATE::~DIALOG_ANNOTATE() // We still save/restore to config (instead of letting DIALOG_SHIM do it) because we also // allow editing of these settings in preferences. - SCHEMATIC_SETTINGS& schSettings = m_Parent->Schematic().Settings(); EESCHEMA_SETTINGS* cfg = static_cast( Kiface().KifaceSettings() ); - schSettings.m_AnnotateMethod = m_rbOptions->GetSelection(); + cfg->m_AnnotatePanel.options = m_rbReset_Annotations->GetValue() ? 1 : 0; if( m_rbScope_Schematic->IsEnabled() ) { @@ -142,6 +141,8 @@ DIALOG_ANNOTATE::~DIALOG_ANNOTATE() cfg->m_AnnotatePanel.recursive = m_checkRecursive->GetValue(); } + cfg->m_AnnotatePanel.regroup_units = m_checkRegroupUnits->GetValue(); + int sort = GetSortOrder(); int method = GetAnnotateAlgo(); int startNum = GetStartNumber(); @@ -194,7 +195,12 @@ bool DIALOG_ANNOTATE::TransferDataToWindow() m_checkRecursive->SetValue( cfg->m_AnnotatePanel.recursive ); } - m_rbOptions->SetSelection( cfg->m_AnnotatePanel.options ); + bool resetAnnotation = cfg->m_AnnotatePanel.options >= 1; + m_rbReset_Annotations->SetValue( resetAnnotation ); + m_rbKeep_Annotations->SetValue( !resetAnnotation ); + + m_checkRegroupUnits->SetValue( cfg->m_AnnotatePanel.regroup_units ); + m_checkRegroupUnits->Enable( cfg->m_AnnotatePanel.options >= 1 ); if( SCH_EDIT_FRAME* schFrame = dynamic_cast( m_parentFrame ) ) { @@ -243,8 +249,12 @@ void DIALOG_ANNOTATE::OnAnnotateClick( wxCommandEvent& event ) REPORTER& reporter = m_MessageWindow->Reporter(); m_MessageWindow->SetLazyUpdate( true ); // Don't update after each message - m_Parent->AnnotateSymbols( &commit, GetScope(), GetSortOrder(), GetAnnotateAlgo(), m_checkRecursive->GetValue(), - GetStartNumber(), m_rbOptions->GetSelection() >= 1, true, reporter ); + bool resetAnnotation = m_rbReset_Annotations->GetValue(); + bool regroupUnits = resetAnnotation && m_checkRegroupUnits->GetValue(); + + m_Parent->AnnotateSymbols( &commit, GetScope(), GetSortOrder(), GetAnnotateAlgo(), + m_checkRecursive->GetValue(), GetStartNumber(), resetAnnotation, + regroupUnits, true, reporter ); commit.Push( _( "Annotate" ) ); @@ -263,6 +273,8 @@ void DIALOG_ANNOTATE::OnClearAnnotationClick( wxCommandEvent& event ) void DIALOG_ANNOTATE::OnOptionChanged( wxCommandEvent& event ) { + m_checkRegroupUnits->Enable( m_rbReset_Annotations->GetValue() ); + m_sdbSizer1OK->Enable( true ); m_sdbSizer1OK->SetDefault(); } diff --git a/eeschema/dialogs/dialog_annotate_base.cpp b/eeschema/dialogs/dialog_annotate_base.cpp index 55c2544aaf..c04b06c449 100644 --- a/eeschema/dialogs/dialog_annotate_base.cpp +++ b/eeschema/dialogs/dialog_annotate_base.cpp @@ -1,5 +1,5 @@ /////////////////////////////////////////////////////////////////////////// -// C++ code generated with wxFormBuilder (version 4.2.1-0-g80c4cb6) +// C++ code generated with wxFormBuilder (version 4.2.1-0-g80c4cb6a-dirty) // http://www.wxformbuilder.org/ // // PLEASE DO *NOT* EDIT THIS FILE! @@ -87,11 +87,20 @@ DIALOG_ANNOTATE_BASE::DIALOG_ANNOTATE_BASE( wxWindow* parent, wxWindowID id, con fgSizer1->Add( sbSizer1, 0, wxALL|wxEXPAND, 5 ); - wxString m_rbOptionsChoices[] = { _("Keep existing annotations"), _("Reset existing annotations") }; - int m_rbOptionsNChoices = sizeof( m_rbOptionsChoices ) / sizeof( wxString ); - m_rbOptions = new wxRadioBox( this, wxID_ANY, _("Options"), wxDefaultPosition, wxDefaultSize, m_rbOptionsNChoices, m_rbOptionsChoices, 1, wxRA_SPECIFY_COLS ); - m_rbOptions->SetSelection( 0 ); - fgSizer1->Add( m_rbOptions, 0, wxALL|wxEXPAND, 5 ); + wxStaticBoxSizer* sbSizer4; + sbSizer4 = new wxStaticBoxSizer( new wxStaticBox( this, wxID_ANY, _("Options") ), wxVERTICAL ); + + m_rbKeep_Annotations = new wxRadioButton( sbSizer4->GetStaticBox(), wxID_ANY, _("Keep existing annotations"), wxDefaultPosition, wxDefaultSize, wxRB_GROUP ); + sbSizer4->Add( m_rbKeep_Annotations, 0, wxLEFT|wxRIGHT, 5 ); + + m_rbReset_Annotations = new wxRadioButton( sbSizer4->GetStaticBox(), wxID_ANY, _("Reset existing annotations"), wxDefaultPosition, wxDefaultSize, 0 ); + sbSizer4->Add( m_rbReset_Annotations, 0, wxLEFT|wxRIGHT, 5 ); + + m_checkRegroupUnits = new wxCheckBox( sbSizer4->GetStaticBox(), wxID_ANY, _("Regroup symbol units"), wxDefaultPosition, wxDefaultSize, 0 ); + sbSizer4->Add( m_checkRegroupUnits, 0, wxALL|wxEXPAND, 5 ); + + + fgSizer1->Add( sbSizer4, 0, wxALL|wxEXPAND, 5 ); wxStaticBoxSizer* sbSizer2; sbSizer2 = new wxStaticBoxSizer( new wxStaticBox( this, wxID_ANY, _("Numbering") ), wxVERTICAL ); @@ -161,7 +170,8 @@ DIALOG_ANNOTATE_BASE::DIALOG_ANNOTATE_BASE( wxWindow* parent, wxWindowID id, con this->Connect( wxEVT_CLOSE_WINDOW, wxCloseEventHandler( DIALOG_ANNOTATE_BASE::OnClose ) ); m_rbSortBy_X_Position->Connect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); m_rbSortBy_Y_Position->Connect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); - m_rbOptions->Connect( wxEVT_COMMAND_RADIOBOX_SELECTED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); + m_rbReset_Annotations->Connect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); + m_checkRegroupUnits->Connect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); m_rbFirstFree->Connect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); m_textNumberAfter->Connect( wxEVT_COMMAND_TEXT_UPDATED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); m_rbSheetX100->Connect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); @@ -177,7 +187,8 @@ DIALOG_ANNOTATE_BASE::~DIALOG_ANNOTATE_BASE() this->Disconnect( wxEVT_CLOSE_WINDOW, wxCloseEventHandler( DIALOG_ANNOTATE_BASE::OnClose ) ); m_rbSortBy_X_Position->Disconnect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); m_rbSortBy_Y_Position->Disconnect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); - m_rbOptions->Disconnect( wxEVT_COMMAND_RADIOBOX_SELECTED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); + m_rbReset_Annotations->Disconnect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); + m_checkRegroupUnits->Disconnect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); m_rbFirstFree->Disconnect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); m_textNumberAfter->Disconnect( wxEVT_COMMAND_TEXT_UPDATED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); m_rbSheetX100->Disconnect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_ANNOTATE_BASE::OnOptionChanged ), NULL, this ); diff --git a/eeschema/dialogs/dialog_annotate_base.fbp b/eeschema/dialogs/dialog_annotate_base.fbp index 27d60ae120..1b240d1178 100644 --- a/eeschema/dialogs/dialog_annotate_base.fbp +++ b/eeschema/dialogs/dialog_annotate_base.fbp @@ -715,68 +715,211 @@ 5 wxALL|wxEXPAND 0 - - 1 - 1 - 1 - 1 - 0 - - 0 - 0 - - - - 1 - 0 - "Keep existing annotations" "Reset existing annotations" - 1 - - 1 - 0 - Dock - 0 - Left - 0 - 1 - - 1 - - 0 - 0 + wxID_ANY Options - 1 - - 0 - - - 0 - 1 - m_rbOptions - 1 - - - protected - 1 - - Resizable - 0 - 1 - - wxRA_SPECIFY_COLS - ; forward_declare - 0 - - - wxFILTER_NONE - wxDefaultValidator - - - - - OnOptionChanged + sbSizer4 + wxVERTICAL + 1 + none + + 5 + wxLEFT|wxRIGHT + 0 + + 1 + 1 + 1 + 1 + 0 + + 0 + 0 + + + + 1 + 0 + 1 + + 1 + 0 + Dock + 0 + Left + 0 + 1 + + 1 + + 0 + 0 + wxID_ANY + Keep existing annotations + + 0 + + + 0 + + 1 + m_rbKeep_Annotations + 1 + + + protected + 1 + + Resizable + 1 + + wxRB_GROUP + ; ; forward_declare + 0 + + + wxFILTER_NONE + wxDefaultValidator + + 0 + + + + + + + 5 + wxLEFT|wxRIGHT + 0 + + 1 + 1 + 1 + 1 + 0 + + 0 + 0 + + + + 1 + 0 + 1 + + 1 + 0 + Dock + 0 + Left + 0 + 1 + + 1 + + 0 + 0 + wxID_ANY + Reset existing annotations + + 0 + + + 0 + + 1 + m_rbReset_Annotations + 1 + + + protected + 1 + + Resizable + 1 + + + ; ; forward_declare + 0 + + + wxFILTER_NONE + wxDefaultValidator + + 0 + + + + OnOptionChanged + + + + 5 + wxALL|wxEXPAND + 0 + + 1 + 1 + 1 + 1 + 0 + + 0 + 0 + + + + 1 + 0 + 0 + 1 + + 1 + 0 + Dock + 0 + Left + 0 + 1 + + 1 + + 0 + 0 + wxID_ANY + Regroup symbol units + + 0 + + + 0 + + 1 + m_checkRegroupUnits + 1 + + + protected + 1 + + Resizable + 1 + + + ; ; forward_declare + 0 + + + wxFILTER_NONE + wxDefaultValidator + + + + + OnOptionChanged + + diff --git a/eeschema/dialogs/dialog_annotate_base.h b/eeschema/dialogs/dialog_annotate_base.h index 17dee6eda8..1abdd24b9a 100644 --- a/eeschema/dialogs/dialog_annotate_base.h +++ b/eeschema/dialogs/dialog_annotate_base.h @@ -1,5 +1,5 @@ /////////////////////////////////////////////////////////////////////////// -// C++ code generated with wxFormBuilder (version 4.2.1-0-g80c4cb6) +// C++ code generated with wxFormBuilder (version 4.2.1-0-g80c4cb6a-dirty) // http://www.wxformbuilder.org/ // // PLEASE DO *NOT* EDIT THIS FILE! @@ -28,7 +28,6 @@ class WX_INFOBAR; #include #include #include -#include #include #include #include @@ -61,7 +60,9 @@ class DIALOG_ANNOTATE_BASE : public DIALOG_SHIM wxStaticBitmap* annotate_down_right_bitmap; wxRadioButton* m_rbSortBy_Y_Position; wxStaticBitmap* annotate_right_down_bitmap; - wxRadioBox* m_rbOptions; + wxRadioButton* m_rbKeep_Annotations; + wxRadioButton* m_rbReset_Annotations; + wxCheckBox* m_checkRegroupUnits; wxRadioButton* m_rbFirstFree; wxTextCtrl* m_textNumberAfter; wxRadioButton* m_rbSheetX100; diff --git a/eeschema/eeschema_settings.cpp b/eeschema/eeschema_settings.cpp index 22c27e3548..03cd2faadb 100644 --- a/eeschema/eeschema_settings.cpp +++ b/eeschema/eeschema_settings.cpp @@ -478,6 +478,9 @@ EESCHEMA_SETTINGS::EESCHEMA_SETTINGS() : m_params.emplace_back( new PARAM( "annotation.recursive", &m_AnnotatePanel.recursive, true ) ); + m_params.emplace_back( new PARAM( "annotation.regroup_units", + &m_AnnotatePanel.regroup_units, false ) ); + m_params.emplace_back( new PARAM( "annotation.scope", &m_AnnotatePanel.scope, 0, 0, 2 ) ); diff --git a/eeschema/eeschema_settings.h b/eeschema/eeschema_settings.h index 9be5ad760f..1c72113af2 100644 --- a/eeschema/eeschema_settings.h +++ b/eeschema/eeschema_settings.h @@ -237,6 +237,7 @@ public: { bool automatic; bool recursive; + bool regroup_units; int scope; int options; int messages_filter; diff --git a/eeschema/sch_edit_frame.h b/eeschema/sch_edit_frame.h index 7b92805a05..5825f0cd1b 100644 --- a/eeschema/sch_edit_frame.h +++ b/eeschema/sch_edit_frame.h @@ -387,7 +387,7 @@ public: void AnnotateSymbols( SCH_COMMIT* aCommit, ANNOTATE_SCOPE_T aAnnotateScope, ANNOTATE_ORDER_T aSortOption, ANNOTATE_ALGO_T aAlgoOption, bool aRecursive, int aStartNumber, bool aResetAnnotation, - bool aRepairTimestamps, REPORTER& aReporter ); + bool aRegroupUnits, bool aRepairTimestamps, REPORTER& aReporter ); /** * Check for annotation errors. diff --git a/eeschema/tools/sch_drawing_tools.cpp b/eeschema/tools/sch_drawing_tools.cpp index 94ab72a99c..f69a05a247 100644 --- a/eeschema/tools/sch_drawing_tools.cpp +++ b/eeschema/tools/sch_drawing_tools.cpp @@ -852,7 +852,7 @@ int SCH_DRAWING_TOOLS::ImportSheet( const TOOL_EVENT& aEvent ) m_frame->AnnotateSymbols( &commit, ANNOTATE_SELECTION, (ANNOTATE_ORDER_T) schSettings.m_AnnotateSortOrder, (ANNOTATE_ALGO_T) schSettings.m_AnnotateMethod, true /* recursive */, - schSettings.m_AnnotateStartNum, false, false, reporter ); + schSettings.m_AnnotateStartNum, false, false, false, reporter ); } // Annotation will clear selection, so we need to restore it @@ -3411,6 +3411,7 @@ int SCH_DRAWING_TOOLS::DrawSheet( const TOOL_EVENT& aEvent ) true, /* recursive */ schSettings.m_AnnotateStartNum, true, /* reset */ + false, /* regroup */ false, /* repair */ reporter ); } diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index e98326bfea..e3707f16a5 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -1920,7 +1920,8 @@ int SCH_EDIT_TOOL::RepeatDrawItem( const TOOL_EVENT& aEvent ) static_cast( newItem )->ClearAnnotation( nullptr, false ); NULL_REPORTER reporter; m_frame->AnnotateSymbols( &commit, ANNOTATE_SELECTION, annotateOrder, annotateAlgo, - true /* recursive */, annotateStartNum, false, false, reporter ); + true /* recursive */, annotateStartNum, false, false, false, + reporter ); } // Annotation clears the selection so re-add the item