From 3f3c8823ac22e34a37d74bc824e00a807535b91b Mon Sep 17 00:00:00 2001
From: Alexander Alekhin <alexander.a.alekhin@gmail.com>
Date: Tue, 18 Dec 2018 05:33:21 +0000
Subject: [PATCH] features2d: fix retainBest() implementation

---
 modules/features2d/src/keypoint.cpp    |  2 +-
 modules/features2d/test/test_utils.cpp | 38 ++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 modules/features2d/test/test_utils.cpp

diff --git a/modules/features2d/src/keypoint.cpp b/modules/features2d/src/keypoint.cpp
index 8b116cbbab..219634e5b4 100644
--- a/modules/features2d/src/keypoint.cpp
+++ b/modules/features2d/src/keypoint.cpp
@@ -77,7 +77,7 @@ void KeyPointsFilter::retainBest(std::vector<KeyPoint>& keypoints, int n_points)
             return;
         }
         //first use nth element to partition the keypoints into the best and worst.
-        std::nth_element(keypoints.begin(), keypoints.begin() + n_points, keypoints.end(), KeypointResponseGreater());
+        std::nth_element(keypoints.begin(), keypoints.begin() + n_points - 1, keypoints.end(), KeypointResponseGreater());
         //this is the boundary response, and in the case of FAST may be ambiguous
         float ambiguous_response = keypoints[n_points - 1].response;
         //use std::partition to grab all of the keypoints with the boundary response.
diff --git a/modules/features2d/test/test_utils.cpp b/modules/features2d/test/test_utils.cpp
new file mode 100644
index 0000000000..78febd353a
--- /dev/null
+++ b/modules/features2d/test/test_utils.cpp
@@ -0,0 +1,38 @@
+// This file is part of OpenCV project.
+// It is subject to the license terms in the LICENSE file found in the top-level directory
+// of this distribution and at http://opencv.org/license.html.
+#include "test_precomp.hpp"
+
+namespace opencv_test { namespace {
+
+TEST(Features2D_KeypointUtils, retainBest_issue_12594)
+{
+    const size_t N = 9;
+
+    // Construct 4-way tie for 3rd highest - correct answer for "3 best" is 6
+    const float no_problem[] = { 5.0f, 4.0f, 1.0f, 2.0f, 0.0f, 3.0f, 3.0f, 3.0f, 3.0f };
+
+    // Same set, different order that exposes partial sort property of std::nth_element
+    // Note: the problem case may depend on your particular implementation of STL
+    const float problem[] = { 3.0f, 3.0f, 3.0f, 3.0f, 4.0f, 5.0f, 0.0f, 1.0f, 2.0f };
+
+    const size_t NBEST  = 3u;
+    const size_t ANSWER = 6u;
+
+    std::vector<cv::KeyPoint> sorted_cv(N);
+    std::vector<cv::KeyPoint> unsorted_cv(N);
+
+    for (size_t i = 0; i < N; ++i)
+    {
+        sorted_cv[i].response   = no_problem[i];
+        unsorted_cv[i].response = problem[i];
+    }
+
+    cv::KeyPointsFilter::retainBest(sorted_cv, NBEST);
+    cv::KeyPointsFilter::retainBest(unsorted_cv, NBEST);
+
+    EXPECT_EQ(ANSWER, sorted_cv.size());
+    EXPECT_EQ(ANSWER, unsorted_cv.size());
+}
+
+}} // namespace