From d1f283d962995673f11c5d8b247f9e5a4e9f4801 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 3 Aug 2022 12:32:24 +0200 Subject: [PATCH] Math: fix batch min()/max()/minmax() to work with const input views. Apart from returning const T instead of T it kinda worked, but in case of floating-point vectors it tried to operate with `std::pair` internally and failed miserably. --- src/Magnum/Math/FunctionsBatch.h | 6 +++--- src/Magnum/Math/Test/FunctionsBatchTest.cpp | 21 ++++++++++++++++++++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/Magnum/Math/FunctionsBatch.h b/src/Magnum/Math/FunctionsBatch.h index c52f03811..736a21d30 100644 --- a/src/Magnum/Math/FunctionsBatch.h +++ b/src/Magnum/Math/FunctionsBatch.h @@ -40,9 +40,9 @@ namespace Implementation { /** @todo Utility/Algorithms.h has a similar (but different) variant of this, maybe turn that into some public utility once we have one more use case? */ -template::type>::from(std::declval()))> static auto stridedArrayViewTypeFor(T&&) -> typename View::Type; -template static T stridedArrayViewTypeFor(const Corrade::Containers::ArrayView&); -template static T stridedArrayViewTypeFor(const Corrade::Containers::StridedArrayView1D&); +template::type>::from(std::declval()))> static auto stridedArrayViewTypeFor(T&&) -> typename std::remove_const::type; +template static typename std::remove_const::type stridedArrayViewTypeFor(const Corrade::Containers::ArrayView&); +template static typename std::remove_const::type stridedArrayViewTypeFor(const Corrade::Containers::StridedArrayView1D&); } diff --git a/src/Magnum/Math/Test/FunctionsBatchTest.cpp b/src/Magnum/Math/Test/FunctionsBatchTest.cpp index 5043edda8..253eb9b43 100644 --- a/src/Magnum/Math/Test/FunctionsBatchTest.cpp +++ b/src/Magnum/Math/Test/FunctionsBatchTest.cpp @@ -24,6 +24,7 @@ */ #include +#include #include #include @@ -44,6 +45,8 @@ struct FunctionsBatchTest: Corrade::TestSuite::Tester { void nanIgnoring(); void nanIgnoringVector(); + + void constIterable(); }; using namespace Literals; @@ -62,7 +65,9 @@ FunctionsBatchTest::FunctionsBatchTest() { &FunctionsBatchTest::minmax, &FunctionsBatchTest::nanIgnoring, - &FunctionsBatchTest::nanIgnoringVector}); + &FunctionsBatchTest::nanIgnoringVector, + + &FunctionsBatchTest::constIterable}); } void FunctionsBatchTest::isInf() { @@ -284,6 +289,20 @@ void FunctionsBatchTest::nanIgnoringVector() { CORRADE_COMPARE(Math::minmax(allNan).second[1], Constants::nan()); } +void FunctionsBatchTest::constIterable() { + const Vector2 data[]{{5, -3}, {-2, 14}, {9, -5}}; + + /* It shouldn't try to operate with a const type (such as trying to to + assign to `std::pair`) internally, instead + it should remove the const */ + CORRADE_COMPARE(Math::min(Corrade::Containers::arrayView(data)), + (Vector2{-2, -5})); + CORRADE_COMPARE(Math::max(Corrade::Containers::stridedArrayView(data)), + (Vector2{9, 14})); + CORRADE_COMPARE(Math::minmax(Corrade::Containers::Array{data, 3, [](const Vector2*, std::size_t){}}), + std::make_pair(Vector2{-2, -5}, Vector2{9, 14})); +} + }}}} CORRADE_TEST_MAIN(Magnum::Math::Test::FunctionsBatchTest)