Rewrite scoring algo again. (Third time's the charm?)

Fixes https://gitlab.com/kicad/code/kicad/-/issues/16042

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 <pavlina.chris@gmail.com>
  * Copyright (C) 2014 Henner Zeller <h.zeller@acm.org>
+ * 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<int( LIB_TREE_NODE& aNode )>* aFilter )
+void LIB_TREE_NODE::ResetScore()
 {
     for( std::unique_ptr<LIB_TREE_NODE>& 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<int( LIB_TREE_NODE& aNode )>* aFilter )
+void LIB_TREE_NODE_LIB_ID::UpdateScore( EDA_COMBINED_MATCHER* aMatcher, const wxString& aLib,
+                                        std::function<bool( LIB_TREE_NODE& aNode )>* aFilter )
 {
-    for( std::unique_ptr<LIB_TREE_NODE>& child: m_Children )
+    // aMatcher test is additive
-        child->ResetScore( aFilter );
-
-    if( aFilter )
-        m_Score = (*aFilter)(*this);
-    else
-        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 );
+    // 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;
+
+    // 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<bool( LIB_TREE_NODE& aNode )>* aFilter )
 {
     if( m_Children.size() )
     {
         for( std::unique_ptr<LIB_TREE_NODE>& child: m_Children )
         {
-            child->UpdateScore( aMatcher, aLib );
+            child->UpdateScore( aMatcher, aLib, aFilter );
             m_Score = std::max( m_Score, child->m_Score );
         }
     }
 
-    if( !aLib.IsEmpty() )
+    // aLib test is additive
-    {
+    if( !aLib.IsEmpty() && m_Name.Lower() == aLib )
-        if( m_Name.Lower() == aLib )
         m_Score += 1;
-    }
+
-    else if( aMatcher )
+    // 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)
+    // show all nodes if no search/filter/etc. criteria are given
-    {
+    if( m_Children.empty() && !aMatcher && aLib.IsEmpty() && ( !aFilter || (*aFilter)(*this) ) )
-        m_Score = std::max( m_Score, 1 );
+        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<bool( LIB_TREE_NODE& aNode )>* aFilter )
 {
     for( std::unique_ptr<LIB_TREE_NODE>& child: m_Children )
-        child->UpdateScore( aMatcher, aLib );
+        child->UpdateScore( aMatcher, aLib, aFilter );
 }
 

common/lib_tree_model_adapter.cpp

@@ -3,6 +3,7 @@
  *
  * Copyright (C) 2017 Chris Pavlina <pavlina.chris@gmail.com>
  * Copyright (C) 2014 Henner Zeller <h.zeller@acm.org>
+ * 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,19 +166,19 @@ 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() )
         {
             // 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 );
+            m_tree.UpdateScore( &termMatcher, wxEmptyString, firstTerm ? m_filter : nullptr );
+            firstTerm = false;
 
             if( term.Contains( ":" ) )
             {
@@ -186,18 +187,19 @@ void LIB_TREE_MODEL_ADAPTER::UpdateSearchString( const wxString& aSearch, bool a
                 wxString             itemName = term.AfterFirst( ':' );
                 EDA_COMBINED_MATCHER itemNameMatcher( itemName, CTX_LIBITEM );
 
-                    m_tree.UpdateScore( &itemNameMatcher, lib );
+                m_tree.UpdateScore( &itemNameMatcher, lib, nullptr );
             }
             else
             {
                 // In case the full token happens to be a library name
-                    m_tree.UpdateScore( nullptr, term );
+                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 );

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<int( LIB_TREE_NODE& )> powerFilter =
+            static std::function<bool( LIB_TREE_NODE& )> powerFilter =
-                    []( LIB_TREE_NODE& ) -> int
+                    []( LIB_TREE_NODE& ) -> bool
                     {
-                        return 0;
+                        return false;
                     };
 
             adapter->SetFilter( &powerFilter );

include/lib_tree_model.h

@@ -3,6 +3,7 @@
  *
  * Copyright (C) 2017 Chris Pavlina <pavlina.chris@gmail.com>
  * Copyright (C) 2014 Henner Zeller <h.zeller@acm.org>
+ * 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<bool( LIB_TREE_NODE& aNode )>* aFilter ) = 0;
 
     /**
-     * Initialize score to kLowestDefaultScore, recursively.
+     * Initialize scores recursively.
      */
-    virtual void ResetScore( std::function<int( LIB_TREE_NODE& aNode )>* 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<bool( LIB_TREE_NODE& aNode )>* aFilter ) override {}
 };
 
 
@@ -211,12 +214,11 @@ public:
      */
     void Update( LIB_TREE_ITEM* aItem );
 
-    void ResetScore( std::function<int( LIB_TREE_NODE& aNode )>* 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<bool( LIB_TREE_NODE& aNode )>* 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<bool( LIB_TREE_NODE& aNode )>* 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<bool( LIB_TREE_NODE& aNode )>* aFilter ) override;
 };
 
 

include/lib_tree_model_adapter.h

@@ -3,6 +3,7 @@
  *
  * Copyright (C) 2017 Chris Pavlina <pavlina.chris@gmail.com>
  * Copyright (C) 2014 Henner Zeller <h.zeller@acm.org>
+ * 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<int( LIB_TREE_NODE& aNode )>* aFilter ) { m_filter = aFilter; }
+    void SetFilter( std::function<bool( LIB_TREE_NODE& aNode )>* aFilter ) { m_filter = aFilter; }
 
     /**
      * Return the active filter.
      */
-    std::function<int( LIB_TREE_NODE& aNode )>* GetFilter() const { return m_filter; }
+    std::function<bool( LIB_TREE_NODE& aNode )>* 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<int( LIB_TREE_NODE& aNode )>*  m_filter;
+    std::function<bool( LIB_TREE_NODE& aNode )>* m_filter;
 
     std::vector<wxDataViewColumn*>               m_columns;
     std::map<wxString, wxDataViewColumn*>        m_colNameMap;

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]()

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<PCBNEW_SETTINGS*>( 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<PCBNEW_SETTINGS*>( 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<std::unique_ptr<EDA_PATTERN_MATCH>>& 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 true;
-        return 1;
-    else
-        return 0;
 }
 
 

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 <pcb_base_frame.h>
 #include <pcbnew_settings.h>
 #include <netlist_reader/pcb_netlist.h>
+#include <lib_tree_model.h>
 
 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 );

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 <h.zeller@acm.org>
+ * 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<int( LIB_TREE_NODE& )> aFilter,
+                                                  std::function<bool( LIB_TREE_NODE& )> aFilter,
                                                   std::function<void()> aCloseHandler ) :
         wxPanel( aParent, wxID_ANY, wxDefaultPosition, wxDefaultSize ),
         m_hsplitter( nullptr ),

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<int( LIB_TREE_NODE& )> aFilter,
+                             std::function<bool( LIB_TREE_NODE& )> aFilter,
                              std::function<void()> aCloseHandler );
 
     ~PANEL_FOOTPRINT_CHOOSER();
@@ -94,7 +95,7 @@ protected:
     LIB_TREE*                               m_tree;
 
     PCB_BASE_FRAME*                         m_frame;
-    std::function<int( LIB_TREE_NODE& )>    m_filter;
+    std::function<bool( LIB_TREE_NODE& )>   m_filter;
     std::function<void()>                   m_closeHandler;
 };