From 72c985bc07bc2a9fbbb5d557aa2bda9b02f28eb1 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Wed, 8 Nov 2023 12:29:14 +0000 Subject: [PATCH] Rewrite scoring algo again. (Third time's the charm?) Fixes https://gitlab.com/kicad/code/kicad/-/issues/16042 --- common/lib_tree_model.cpp | 70 ++++++++++----------- common/lib_tree_model_adapter.cpp | 50 ++++++++------- eeschema/widgets/panel_symbol_chooser.cpp | 6 +- include/lib_tree_model.h | 22 ++++--- include/lib_tree_model_adapter.h | 7 ++- pcbnew/dialogs/dialog_footprint_chooser.cpp | 5 +- pcbnew/footprint_chooser_frame.cpp | 51 ++++++++------- pcbnew/footprint_chooser_frame.h | 4 +- pcbnew/widgets/panel_footprint_chooser.cpp | 3 +- pcbnew/widgets/panel_footprint_chooser.h | 5 +- 10 files changed, 116 insertions(+), 107 deletions(-) diff --git a/common/lib_tree_model.cpp b/common/lib_tree_model.cpp index 65dd61638a..10ee9889a9 100644 --- a/common/lib_tree_model.cpp +++ b/common/lib_tree_model.cpp @@ -1,8 +1,9 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. - *lib_tree_model + * * Copyright (C) 2017 Chris Pavlina * Copyright (C) 2014 Henner Zeller + * Copyright (C) 2023 CERN * Copyright (C) 2014-2023 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software: you can redistribute it and/or modify it @@ -29,10 +30,10 @@ -void LIB_TREE_NODE::ResetScore( std::function* aFilter ) +void LIB_TREE_NODE::ResetScore() { for( std::unique_ptr& child: m_Children ) - child->ResetScore( aFilter ); + child->ResetScore(); m_Score = 0; } @@ -214,27 +215,24 @@ void LIB_TREE_NODE_LIB_ID::Update( LIB_TREE_ITEM* aItem ) } -void LIB_TREE_NODE_LIB_ID::ResetScore( std::function* aFilter ) +void LIB_TREE_NODE_LIB_ID::UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib, + std::function* aFilter ) { - for( std::unique_ptr& child: m_Children ) - child->ResetScore( aFilter ); + // aMatcher test is additive + if( aMatcher ) + m_Score += aMatcher->ScoreTerms( m_SearchTerms ); - if( aFilter ) - m_Score = (*aFilter)(*this); - else + // aLib test is additive + if( !aLib.IsEmpty() && m_Parent->m_Name.Lower() == aLib ) + m_Score += 1; + + // aFilter test is subtractive + if( aFilter && !(*aFilter)(*this) ) m_Score = 0; -} - -void LIB_TREE_NODE_LIB_ID::UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib ) -{ - if( aLib.IsEmpty() || m_Parent->m_Name.Lower() == aLib ) - { - if( aMatcher ) - m_Score += aMatcher->ScoreTerms( m_SearchTerms ); - else - m_Score = std::max( m_Score, 1 ); - } + // show all nodes if no search/filter/etc. criteria are given + if( !aMatcher && aLib.IsEmpty() && ( !aFilter || (*aFilter)(*this) ) ) + m_Score = 1; } @@ -259,30 +257,29 @@ LIB_TREE_NODE_LIB_ID& LIB_TREE_NODE_LIB::AddItem( LIB_TREE_ITEM* aItem ) } -void LIB_TREE_NODE_LIB::UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib ) +void LIB_TREE_NODE_LIB::UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib, + std::function* aFilter ) { if( m_Children.size() ) { for( std::unique_ptr& child: m_Children ) { - child->UpdateScore( aMatcher, aLib ); + child->UpdateScore( aMatcher, aLib, aFilter ); m_Score = std::max( m_Score, child->m_Score ); } } - if( !aLib.IsEmpty() ) - { - if( m_Name.Lower() == aLib ) - m_Score += 1; - } - else if( aMatcher ) - { + // aLib test is additive + if( !aLib.IsEmpty() && m_Name.Lower() == aLib ) + m_Score += 1; + + // aMatcher test is additive + if( aMatcher ) m_Score += aMatcher->ScoreTerms( m_SearchTerms ); - } - else // No search string or library filters; show, but don't expand (mScore == 1) - { - m_Score = std::max( m_Score, 1 ); - } + + // show all nodes if no search/filter/etc. criteria are given + if( m_Children.empty() && !aMatcher && aLib.IsEmpty() && ( !aFilter || (*aFilter)(*this) ) ) + m_Score = 1; } @@ -300,9 +297,10 @@ LIB_TREE_NODE_LIB& LIB_TREE_NODE_ROOT::AddLib( wxString const& aName, wxString c } -void LIB_TREE_NODE_ROOT::UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib ) +void LIB_TREE_NODE_ROOT::UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib, + std::function* aFilter ) { for( std::unique_ptr& child: m_Children ) - child->UpdateScore( aMatcher, aLib ); + child->UpdateScore( aMatcher, aLib, aFilter ); } diff --git a/common/lib_tree_model_adapter.cpp b/common/lib_tree_model_adapter.cpp index 518e263786..16731bfcdf 100644 --- a/common/lib_tree_model_adapter.cpp +++ b/common/lib_tree_model_adapter.cpp @@ -3,6 +3,7 @@ * * Copyright (C) 2017 Chris Pavlina * Copyright (C) 2014 Henner Zeller + * Copyright (C) 2023 CERN * Copyright (C) 2014-2023 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software: you can redistribute it and/or modify it @@ -165,39 +166,40 @@ void LIB_TREE_MODEL_ADAPTER::UpdateSearchString( const wxString& aSearch, bool a Freeze(); BeforeReset(); - m_tree.ResetScore( m_filter ); + m_tree.ResetScore(); wxStringTokenizer tokenizer( aSearch ); + bool firstTerm = true; - if( tokenizer.HasMoreTokens() ) + while( tokenizer.HasMoreTokens() ) { - while( tokenizer.HasMoreTokens() ) + // First search for the full token, in case it appears in a search string + wxString term = tokenizer.GetNextToken().Lower(); + EDA_COMBINED_MATCHER termMatcher( term, CTX_LIBITEM ); + + m_tree.UpdateScore( &termMatcher, wxEmptyString, firstTerm ? m_filter : nullptr ); + firstTerm = false; + + if( term.Contains( ":" ) ) { - // First search for the full token, in case it appears in a search string - wxString term = tokenizer.GetNextToken().Lower(); - EDA_COMBINED_MATCHER termMatcher( term, CTX_LIBITEM ); + // Next search for the library:item_name + wxString lib = term.BeforeFirst( ':' ); + wxString itemName = term.AfterFirst( ':' ); + EDA_COMBINED_MATCHER itemNameMatcher( itemName, CTX_LIBITEM ); - m_tree.UpdateScore( &termMatcher, wxEmptyString ); - - if( term.Contains( ":" ) ) - { - // Next search for the library:item_name - wxString lib = term.BeforeFirst( ':' ); - wxString itemName = term.AfterFirst( ':' ); - EDA_COMBINED_MATCHER itemNameMatcher( itemName, CTX_LIBITEM ); - - m_tree.UpdateScore( &itemNameMatcher, lib ); - } - else - { - // In case the full token happens to be a library name - m_tree.UpdateScore( nullptr, term ); - } + m_tree.UpdateScore( &itemNameMatcher, lib, nullptr ); + } + else + { + // In case the full token happens to be a library name + m_tree.UpdateScore( nullptr, term, nullptr ); } } - else + + if( firstTerm ) { - m_tree.UpdateScore( nullptr, wxEmptyString ); + // No terms processed; just run the filter + m_tree.UpdateScore( nullptr, wxEmptyString, m_filter ); } m_tree.SortNodes( m_sort_mode == BEST_MATCH ); diff --git a/eeschema/widgets/panel_symbol_chooser.cpp b/eeschema/widgets/panel_symbol_chooser.cpp index 352298cd75..d2474dc84f 100644 --- a/eeschema/widgets/panel_symbol_chooser.cpp +++ b/eeschema/widgets/panel_symbol_chooser.cpp @@ -106,10 +106,10 @@ PANEL_SYMBOL_CHOOSER::PANEL_SYMBOL_CHOOSER( SCH_BASE_FRAME* aFrame, wxWindow* aP // HACK ALERT: the only filter ever used for symbols is the power filter, so we // just look for the function pointer being non-null. (What the function does is // therefore immaterial as it's never called.) - static std::function powerFilter = - []( LIB_TREE_NODE& ) -> int + static std::function powerFilter = + []( LIB_TREE_NODE& ) -> bool { - return 0; + return false; }; adapter->SetFilter( &powerFilter ); diff --git a/include/lib_tree_model.h b/include/lib_tree_model.h index d50218e689..af408250fc 100644 --- a/include/lib_tree_model.h +++ b/include/lib_tree_model.h @@ -3,6 +3,7 @@ * * Copyright (C) 2017 Chris Pavlina * Copyright (C) 2014 Henner Zeller + * Copyright (C) 2023 CERN * Copyright (C) 2014-2023 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software: you can redistribute it and/or modify it @@ -80,12 +81,13 @@ public: * * @param aMatcher an EDA_COMBINED_MATCHER initialized with the search term */ - virtual void UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib ) = 0; + virtual void UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib, + std::function* aFilter ) = 0; /** - * Initialize score to kLowestDefaultScore, recursively. + * Initialize scores recursively. */ - virtual void ResetScore( std::function* aFilter ); + virtual void ResetScore(); /** * Store intrinsic ranks on all children of this node. See m_IntrinsicRank @@ -175,7 +177,8 @@ public: /** * Do nothing, units just take the parent's score */ - virtual void UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib ) override {} + virtual void UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib, + std::function* aFilter ) override {} }; @@ -211,12 +214,11 @@ public: */ void Update( LIB_TREE_ITEM* aItem ); - void ResetScore( std::function* aFilter ) override; - /** * Perform the actual search. */ - virtual void UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib ) override; + virtual void UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib, + std::function* aFilter ) override; protected: /** @@ -257,7 +259,8 @@ public: */ LIB_TREE_NODE_LIB_ID& AddItem( LIB_TREE_ITEM* aItem ); - virtual void UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib ) override; + virtual void UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib, + std::function* aFilter ) override; }; @@ -284,7 +287,8 @@ public: */ LIB_TREE_NODE_LIB& AddLib( wxString const& aName, wxString const& aDesc ); - virtual void UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib ) override; + virtual void UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib, + std::function* aFilter ) override; }; diff --git a/include/lib_tree_model_adapter.h b/include/lib_tree_model_adapter.h index 572ea9af5f..2eeb749d21 100644 --- a/include/lib_tree_model_adapter.h +++ b/include/lib_tree_model_adapter.h @@ -3,6 +3,7 @@ * * Copyright (C) 2017 Chris Pavlina * Copyright (C) 2014 Henner Zeller + * Copyright (C) 2023 CERN * Copyright (C) 2014-2023 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software: you can redistribute it and/or modify it @@ -141,12 +142,12 @@ public: * * @param aFilter if SYM_FILTER_POWER, only power parts are loaded */ - void SetFilter( std::function* aFilter ) { m_filter = aFilter; } + void SetFilter( std::function* aFilter ) { m_filter = aFilter; } /** * Return the active filter. */ - std::function* GetFilter() const { return m_filter; } + std::function* GetFilter() const { return m_filter; } void SetSortMode( SORT_MODE aMode ) { m_sort_mode = aMode; } SORT_MODE GetSortMode() const { return m_sort_mode; } @@ -415,7 +416,7 @@ private: wxDataViewCtrl* m_widget; - std::function* m_filter; + std::function* m_filter; std::vector m_columns; std::map m_colNameMap; diff --git a/pcbnew/dialogs/dialog_footprint_chooser.cpp b/pcbnew/dialogs/dialog_footprint_chooser.cpp index bb76145c27..f8c8646244 100644 --- a/pcbnew/dialogs/dialog_footprint_chooser.cpp +++ b/pcbnew/dialogs/dialog_footprint_chooser.cpp @@ -1,6 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * + * Copyright (C) 2023 CERN * Copyright (C) 2023 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or @@ -37,9 +38,9 @@ DIALOG_FOOTPRINT_CHOOSER::DIALOG_FOOTPRINT_CHOOSER( PCB_BASE_FRAME* aParent, wxBoxSizer* sizer = new wxBoxSizer( wxVERTICAL ); m_chooserPanel = new PANEL_FOOTPRINT_CHOOSER( aParent, this, aFootprintHistoryList, // Filter - []( LIB_TREE_NODE& aNode ) -> int + []( LIB_TREE_NODE& aNode ) -> bool { - return 0; + return true; }, // Close handler [this]() diff --git a/pcbnew/footprint_chooser_frame.cpp b/pcbnew/footprint_chooser_frame.cpp index 9599490dc2..f70b726483 100644 --- a/pcbnew/footprint_chooser_frame.cpp +++ b/pcbnew/footprint_chooser_frame.cpp @@ -1,6 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * + * Copyright (C) 2023 CERN * Copyright (C) 2023 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or @@ -79,10 +80,22 @@ FOOTPRINT_CHOOSER_FRAME::FOOTPRINT_CHOOSER_FRAME( KIWAY* aKiway, wxWindow* aPare { SetModal( true ); + m_filterByFPFilters = new wxCheckBox( this, wxID_ANY, _( "Apply footprint filters" ) ); + m_filterByPinCount = new wxCheckBox( this, wxID_ANY, _( "Filter by pin count" ) ); + + m_filterByFPFilters->Show( false ); + m_filterByPinCount->Show( false ); + + if( PCBNEW_SETTINGS* cfg = dynamic_cast( Kiface().KifaceSettings() ) ) + { + m_filterByFPFilters->SetValue( cfg->m_FootprintChooser.use_fp_filters ); + m_filterByPinCount->SetValue( cfg->m_FootprintChooser.filter_on_pin_count ); + } + wxBoxSizer* sizer = new wxBoxSizer( wxVERTICAL ); m_chooserPanel = new PANEL_FOOTPRINT_CHOOSER( this, this, s_FootprintHistoryList, // Filter - [this]( LIB_TREE_NODE& aNode ) -> int + [this]( LIB_TREE_NODE& aNode ) -> bool { return filterFootprint( aNode ); }, @@ -96,24 +109,11 @@ FOOTPRINT_CHOOSER_FRAME::FOOTPRINT_CHOOSER_FRAME( KIWAY* aKiway, wxWindow* aPare sizer->Add( m_chooserPanel, 1, wxEXPAND | wxBOTTOM, 2 ); wxBoxSizer* fpFilterSizer = new wxBoxSizer( wxVERTICAL ); - - m_filterByFPFilters = new wxCheckBox( this, wxID_ANY, _( "Apply footprint filters" ) ); fpFilterSizer->Add( m_filterByFPFilters, 0, wxTOP | wxEXPAND, 5 ); - m_filterByFPFilters->Show( false ); - sizer->Add( fpFilterSizer, 0, wxEXPAND | wxLEFT, 10 ); wxBoxSizer* buttonsSizer = new wxBoxSizer( wxHORIZONTAL ); - - m_filterByPinCount = new wxCheckBox( this, wxID_ANY, _( "Filter by pin count" ) ); buttonsSizer->Add( m_filterByPinCount, 0, wxLEFT | wxTOP | wxALIGN_TOP, 5 ); - m_filterByPinCount->Show( false ); - - if( PCBNEW_SETTINGS* cfg = dynamic_cast( Kiface().KifaceSettings() ) ) - { - m_filterByFPFilters->SetValue( cfg->m_FootprintChooser.use_fp_filters ); - m_filterByPinCount->SetValue( cfg->m_FootprintChooser.filter_on_pin_count ); - } wxStdDialogButtonSizer* sdbSizer = new wxStdDialogButtonSizer(); wxButton* okButton = new wxButton( this, wxID_OK ); @@ -157,9 +157,15 @@ FOOTPRINT_CHOOSER_FRAME::~FOOTPRINT_CHOOSER_FRAME() } } -int FOOTPRINT_CHOOSER_FRAME::filterFootprint( LIB_TREE_NODE& aNode ) +bool FOOTPRINT_CHOOSER_FRAME::filterFootprint( LIB_TREE_NODE& aNode ) { - bool filtering = false; + if( aNode.m_Type == LIB_TREE_NODE::TYPE::LIB ) + { + // Normally lib nodes get scored by the max of their children's scores. However, if a + // lib node *has* no children then the scorer will call the filter on the lib node itself, + // and we just want to return true if we're not filtering at all. + return !m_filterByPinCount->GetValue() && !m_filterByFPFilters->GetValue(); + } auto patternMatch = []( LIB_ID& id, std::vector>& filters ) -> bool @@ -186,24 +192,17 @@ int FOOTPRINT_CHOOSER_FRAME::filterFootprint( LIB_TREE_NODE& aNode ) if( m_pinCount > 0 && m_filterByPinCount->GetValue() ) { - filtering = true; - if( aNode.m_PinCount != m_pinCount ) - return -1; + return false; } if( !m_fpFilters.empty() && m_filterByFPFilters->GetValue() ) { - filtering = true; - if( !patternMatch( aNode.m_LibId, m_fpFilters ) ) - return -1; + return false; } - if( filtering ) - return 1; - else - return 0; + return true; } diff --git a/pcbnew/footprint_chooser_frame.h b/pcbnew/footprint_chooser_frame.h index 302ebba89d..cd0bf4d1a7 100644 --- a/pcbnew/footprint_chooser_frame.h +++ b/pcbnew/footprint_chooser_frame.h @@ -1,6 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * + * Copyright (C) 2023 CERN * Copyright (C) 2023 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or @@ -29,6 +30,7 @@ #include #include #include +#include class PANEL_FOOTPRINT_CHOOSER; class wxCheckBox; @@ -65,7 +67,7 @@ protected: FOOTPRINT_CHOOSER_FRAME( KIWAY* aKiway, wxWindow* aParent ); private: - int filterFootprint( LIB_TREE_NODE& aNode ); + bool filterFootprint( LIB_TREE_NODE& aNode ); void OnPaint( wxPaintEvent& aEvent ); void OnOK( wxCommandEvent& aEvent ); diff --git a/pcbnew/widgets/panel_footprint_chooser.cpp b/pcbnew/widgets/panel_footprint_chooser.cpp index cbfd2c2b3a..fa1b53fd24 100644 --- a/pcbnew/widgets/panel_footprint_chooser.cpp +++ b/pcbnew/widgets/panel_footprint_chooser.cpp @@ -2,6 +2,7 @@ * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2014 Henner Zeller + * Copyright (C) 2023 CERN * Copyright (C) 2016-2023 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or @@ -43,7 +44,7 @@ PANEL_FOOTPRINT_CHOOSER::PANEL_FOOTPRINT_CHOOSER( PCB_BASE_FRAME* aFrame, wxTopLevelWindow* aParent, const wxArrayString& aFootprintHistoryList, - std::function aFilter, + std::function aFilter, std::function aCloseHandler ) : wxPanel( aParent, wxID_ANY, wxDefaultPosition, wxDefaultSize ), m_hsplitter( nullptr ), diff --git a/pcbnew/widgets/panel_footprint_chooser.h b/pcbnew/widgets/panel_footprint_chooser.h index c665cfe7db..b19d47c68b 100644 --- a/pcbnew/widgets/panel_footprint_chooser.h +++ b/pcbnew/widgets/panel_footprint_chooser.h @@ -1,6 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * + * Copyright (C) 2023 CERN * Copyright (C) 2014-2023 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or @@ -46,7 +47,7 @@ public: */ PANEL_FOOTPRINT_CHOOSER( PCB_BASE_FRAME* aFrame, wxTopLevelWindow* aParent, const wxArrayString& aFootprintHistoryList, - std::function aFilter, + std::function aFilter, std::function aCloseHandler ); ~PANEL_FOOTPRINT_CHOOSER(); @@ -94,7 +95,7 @@ protected: LIB_TREE* m_tree; PCB_BASE_FRAME* m_frame; - std::function m_filter; + std::function m_filter; std::function m_closeHandler; };