Document the comparator contract of find, find_all and find_next_in_subtree

README.md

@@ -267,6 +267,7 @@ Finds the first interval in the interval tree that has an exact match.
 ### (const)iterator find(interval_type const& ival, CompareFunctionT const& compare)
 Finds the first interval in the interval tree that has the following statement evaluate to true: compare(interval_in_tree, ival);
 Allows for propper float comparisons.
+compare must not return true for an interval whose high is below ival.high(): the search skips subtrees whose highest high is below ival.high(). For a float tolerance, lower the high of ival by the tolerance.
 #### Parameters
 * `ival` The interval to find.
 * `compare` The compare function to compare intervals with. Function is called like so: compare(interval_in_tree, ival).
@@ -296,6 +297,7 @@ tree.find_all({3, 7}, [](auto iter) /* iter will be const_iterator if tree is co
 ---
 ### (const)iterator find_all(interval_type const& ival, OnFindFunctionT const& on_find, CompareFunctionT const& compare)
 Find all intervals in the tree that the compare function returns true for.
+compare must not return true for an interval whose high is below ival.high(): the search skips subtrees whose highest high is below ival.high().
 #### Parameters
 * `ival` The interval to find.
 * `compare` The compare function to compare intervals with. Function is called like so: compare(interval_in_tree, ival).
@@ -316,8 +318,9 @@ You cannot find all matches this way, use find_all for that.
 
 ---
 ### (const)iterator find_next_in_subtree(iterator from, interval_type const& ival, CompareFunctionT const& compare)
-Finds the next exact match EXCLUDING from in the subtree originating from "from".
+Finds the next interval EXCLUDING from in the subtree originating from "from" that the compare function returns true for.
 You cannot find all matches this way, use find_all for that.
+compare must not return true for an interval whose high is below ival.high(): the search skips subtrees whose highest high is below ival.high().
 #### Parameters
 * `from` The iterator to start from (including this iterator!)
 * `ival` The interval to find.

include/interval-tree/interval_tree.hpp

@@ -1263,10 +1263,11 @@ namespace lib_interval_tree
         }
 
         /**
-         *  Finds the first exact match.
+         *  Finds the first interval for which compare(interval, ival) is true.
          *
-         *  @param ival The interval to find an exact match for within the tree.
-         *  @param compare A comparison function to use.
+         *  @param ival The interval to compare the intervals in the tree with.
+         *  @param compare A comparison function to use. Must not return true for an interval whose high is below
+         *  ival.high(), because subtrees whose max is below ival.high() are skipped.
          */
         template <typename CompareFunctionT>
         iterator find(interval_type const& ival, CompareFunctionT const& compare)
@@ -1277,10 +1278,11 @@ namespace lib_interval_tree
         }
 
         /**
-         *  Finds the first exact match.
+         *  Finds the first interval for which compare(interval, ival) is true.
          *
-         *  @param ival The interval to find an exact match for within the tree.
-         *  @param compare A comparison function to use.
+         *  @param ival The interval to compare the intervals in the tree with.
+         *  @param compare A comparison function to use. Must not return true for an interval whose high is below
+         *  ival.high(), because subtrees whose max is below ival.high() are skipped.
          */
         template <typename CompareFunctionT>
         const_iterator find(interval_type const& ival, CompareFunctionT const& compare) const
@@ -1314,7 +1316,12 @@ namespace lib_interval_tree
         }
 
         /**
-         *  Finds all exact matches and returns the amount of intervals found.
+         *  Finds all intervals for which compare(interval, ival) is true.
+         *
+         *  @param ival The interval to compare the intervals in the tree with.
+         *  @param on_find Called with each found interval. Return false to stop the search.
+         *  @param compare A comparison function to use. Must not return true for an interval whose high is below
+         *  ival.high(), because subtrees whose max is below ival.high() are skipped.
          */
         template <typename FunctionT, typename CompareFunctionT>
         void find_all(interval_type const& ival, FunctionT const& on_find, CompareFunctionT const& compare)
@@ -1347,11 +1354,12 @@ namespace lib_interval_tree
         }
 
         /**
-         *  Finds the next exact match EXCLUDING from.
+         *  Finds the next interval EXCLUDING from for which compare(interval, ival) is true.
          *
          *  @param from The iterator to search from EXCLUSIVE!
-         *  @param ival The interval to find an exact match for within the tree.
-         *  @param compare A comparison function to use.
+         *  @param ival The interval to compare the intervals in the tree with.
+         *  @param compare A comparison function to use. Must not return true for an interval whose high is below
+         *  ival.high(), because subtrees whose max is below ival.high() are skipped.
          */
         template <typename CompareFunctionT>
         iterator find_next_in_subtree(iterator from, interval_type const& ival, CompareFunctionT const& compare)

tests/find_tests.hpp

@@ -3,6 +3,7 @@
 #include <ctime>
 #include <random>
 #include <cmath>
+#include <algorithm>
 
 class FindTests : public ::testing::Test
 {
@@ -264,4 +265,170 @@ TEST_F(FindTests, FuzzyFindAllInTree)
 
     EXPECT_EQ(findCount, 3);
     ASSERT_TRUE(findIsConsistent);
-}
+}
+
+TEST_F(FindTests, FindAndFindAllWithCompareAgreeWithLinearScan)
+{
+    std::mt19937 gen{0};
+    std::uniform_int_distribution<int> distLow{0, 20};
+    std::uniform_int_distribution<int> distLength{0, 30};
+    for (int i = 0; i < 500; ++i)
+    {
+        const auto low = distLow(gen);
+        tree.insert({low, low + distLength(gen)});
+    }
+
+    // Never accepts a high below the searched high, as the comparator overloads require.
+    const auto compare = [](auto const& lhs, auto const& rhs) {
+        return lhs.low() == rhs.low() && lhs.high() >= rhs.high();
+    };
+    auto const& constTree = tree;
+    for (int low = 0; low <= 20; ++low)
+    {
+        const auto searchInterval = decltype(tree)::interval_type{low, low + 15};
+
+        int expected = 0;
+        for (auto const& ival : tree)
+        {
+            if (compare(ival, searchInterval))
+                ++expected;
+        }
+        ASSERT_GT(expected, 1);
+
+        int found = 0;
+        tree.find_all(
+            searchInterval,
+            [&found, &searchInterval, &compare](decltype(tree)::iterator iter) {
+                EXPECT_TRUE(compare(*iter, searchInterval));
+                ++found;
+                return true;
+            },
+            compare
+        );
+        EXPECT_EQ(found, expected) << "low = " << low;
+
+        int foundConst = 0;
+        constTree.find_all(
+            searchInterval,
+            [&foundConst](decltype(tree)::const_iterator) {
+                ++foundConst;
+                return true;
+            },
+            compare
+        );
+        EXPECT_EQ(foundConst, expected) << "low = " << low;
+
+        const auto first = tree.find(searchInterval, compare);
+        ASSERT_NE(first, std::end(tree)) << "low = " << low;
+        EXPECT_TRUE(compare(*first, searchInterval));
+
+        // There are at least two matches, so one lies below the root.
+        const auto next = tree.find_next_in_subtree(tree.root(), searchInterval, compare);
+        ASSERT_NE(next, std::end(tree)) << "low = " << low;
+        EXPECT_TRUE(compare(*next, searchInterval));
+    }
+}
+
+TEST_F(FindTests, WillFindWithFloatToleranceWhenSearchedHighIsLoweredByIt)
+{
+    constexpr double tolerance = 1e-6;
+    IntervalTypes<double>::tree_type floatTree;
+    floatTree.insert({5.0, 5.0});
+    floatTree.insert({0.0, 1.0});
+
+    // The stored high is slightly below the high searched for, so the search starts the tolerance lower.
+    const double searchedHigh = 1.0 + 1e-9;
+    const auto found = floatTree.find({0.0, searchedHigh - tolerance}, [](auto const& lhs, auto const& rhs) {
+        return std::abs(lhs.low() - rhs.low()) < tolerance && lhs.high() >= rhs.high() &&
+            lhs.high() - rhs.high() < 2 * tolerance;
+    });
+    ASSERT_NE(found, std::end(floatTree));
+    EXPECT_EQ(found->low(), 0.0);
+    EXPECT_EQ(found->high(), 1.0);
+}
+
+TEST_F(FindTests, ExactFindAndFindAllAgreeWithLinearScan)
+{
+    std::mt19937 gen{0};
+    std::uniform_int_distribution<int> distLow{0, 20};
+    std::uniform_int_distribution<int> distLength{0, 5};
+    std::vector<decltype(tree)::interval_type> intervals;
+    for (int i = 0; i < 500; ++i)
+    {
+        const auto low = distLow(gen);
+        intervals.emplace_back(low, low + distLength(gen));
+        tree.insert(intervals.back());
+    }
+
+    for (auto const& searchInterval : intervals)
+    {
+        const auto expected = std::count(std::begin(intervals), std::end(intervals), searchInterval);
+
+        int found = 0;
+        tree.find_all(searchInterval, [&found, &searchInterval](decltype(tree)::iterator iter) {
+            EXPECT_EQ(*iter, searchInterval);
+            ++found;
+            return true;
+        });
+        EXPECT_EQ(found, expected);
+
+        const auto first = tree.find(searchInterval);
+        ASSERT_NE(first, std::end(tree));
+        EXPECT_EQ(*first, searchInterval);
+    }
+    EXPECT_EQ(tree.find({21, 21}), std::end(tree));
+}
+
+struct FindVisitCountingHook : public lib_interval_tree::hooks::regular
+{
+    struct hook_state
+    {
+        mutable int find_visits = 0;
+        mutable int find_all_visits = 0;
+    };
+
+    template <typename tree_type, typename compare_function_type>
+    static inline void on_find(
+        tree_type const& tree,
+        typename tree_type::node_type*,
+        typename tree_type::interval_type const&,
+        compare_function_type
+    ) noexcept
+    {
+        ++tree.find_visits;
+    }
+
+    template <typename tree_type, typename compare_function_type>
+    static inline void on_find_all(
+        tree_type const& tree,
+        typename tree_type::node_type*,
+        typename tree_type::interval_type const&,
+        compare_function_type
+    ) noexcept
+    {
+        ++tree.find_all_visits;
+    }
+};
+
+TEST_F(FindTests, ExactFindAndFindAllPruneByMax)
+{
+    constexpr int amount = 1'000;
+
+    lib_interval_tree::interval_tree<decltype(tree)::interval_type, FindVisitCountingHook> countingTree;
+    for (int i = 0; i != amount; ++i)
+        countingTree.insert({i * 2, i * 2 + 1});
+
+    // Only the path to the last interval has a max at or above its high.
+    const auto lastInterval = decltype(tree)::interval_type{amount * 2 - 2, amount * 2 - 1};
+
+    ASSERT_NE(countingTree.find(lastInterval), std::end(countingTree));
+    EXPECT_LT(countingTree.find_visits, amount / 10);
+
+    int found = 0;
+    countingTree.find_all(lastInterval, [&found](auto) {
+        ++found;
+        return true;
+    });
+    EXPECT_EQ(found, 1);
+    EXPECT_LT(countingTree.find_all_visits, amount / 10);
+}