diff --git a/doc/changelog.dox b/doc/changelog.dox index 0731aed80..0b45bbee3 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -1480,6 +1480,11 @@ See also: for implicit conversions from/to a @ref std::string and/or @ref std::pair, but in some cases you may be forced to change the code that uses those APIs. +- @ref Math::Algorithms::svd() that used to return a @ref std::tuple and + printed a message to @relativeref{Magnum,Error} if it didn't converge now + returns a @ref Containers::Tuple wrapped in an @ref Containers::Optional. + As the optional makes this a breaking change, no backwards compatibility + header for tuple conversion is included. - @ref Image, @ref ImageView and @ref Trade::ImageData now look for a @cpp pixelFormatSize() @ce API via ADL instead of @cpp pixelSize() @ce. In case you were passing a custom pixel format enum to the image classes, you diff --git a/doc/snippets/MagnumMathAlgorithms.cpp b/doc/snippets/MagnumMathAlgorithms.cpp index cb1caa31a..d47e1c2df 100644 --- a/doc/snippets/MagnumMathAlgorithms.cpp +++ b/doc/snippets/MagnumMathAlgorithms.cpp @@ -62,11 +62,9 @@ enum: std::size_t { cols = 3, rows = 4 }; /* [svd] */ Math::RectangularMatrix m; -Math::RectangularMatrix uPart; -Math::Vector wDiagonal; -Math::Matrix v; - -std::tie(uPart, wDiagonal, v) = Math::Algorithms::svd(m); +auto svd = Math::Algorithms::svd(m); +Math::RectangularMatrix uPart = svd->first(); +Math::Vector wDiagonal = svd->second(); // Extend U Math::Matrix u{Math::ZeroInit}; diff --git a/src/Magnum/Math/Algorithms/Svd.h b/src/Magnum/Math/Algorithms/Svd.h index 003ce9041..c618e3178 100644 --- a/src/Magnum/Math/Algorithms/Svd.h +++ b/src/Magnum/Math/Algorithms/Svd.h @@ -29,7 +29,8 @@ * @brief Function @ref Magnum::Math::Algorithms::svd() */ -#include +#include +#include #include "Magnum/Math/Functions.h" #include "Magnum/Math/Matrix.h" @@ -65,7 +66,7 @@ on given matrix where @p rows >= `cols`: @f[ Returns first @p cols column vectors of @f$ \boldsymbol{U} @f$, diagonal of @f$ \boldsymbol{\Sigma} @f$ and non-transposed @f$ \boldsymbol{V} @f$. If the -solution doesn't converge, returns zero matrices. +solution doesn't converge, returns @ref Containers::NullOpt. Full @f$ \boldsymbol{U} @f$, @f$ \boldsymbol{\Sigma} @f$ matrices and original @f$ \boldsymbol{M} @f$ matrix can be reconstructed from the values as @@ -81,7 +82,7 @@ for an example. Implementation based on *Golub, G. H.; Reinsch, C. (1970). @see @ref qr(), @ref Matrix3::rotationShear(), @ref Matrix4::rotationShear() */ /* The matrix is passed by value because it is changed inside */ -template std::tuple, Vector, Matrix> svd(RectangularMatrix m) { +template Containers::Optional, Vector, Matrix>> svd(RectangularMatrix m) { static_assert(rows >= cols, "Unsupported matrix aspect ratio"); static_assert(T(1)+TypeTraits::epsilon() > T(1), "Epsilon too small"); constexpr T tol = Implementation::smallestDelta()/TypeTraits::epsilon(); @@ -255,8 +256,7 @@ template std::tuple= maxIterations-1) { - Error{} << "Magnum::Math::Algorithms::svd(): no convergence"; - return std::make_tuple(RectangularMatrix{}, Vector{}, Matrix{ZeroInit}); + return {}; } /* Shift from bottom 2x2 minor */ @@ -316,7 +316,7 @@ template std::tuple void SvdTest::test() { Vector8{T{ 7}, T{ 8}, T{ 3}, T{ 4}, T{ 4}, T{-1}, T{ 1}, T{ 2}}}; const Vector5 expected(std::sqrt(T{1248}), T{0}, T{20}, std::sqrt(T{384}), T{0}); - Matrix5x8 u{Magnum::NoInit}; - Vector5 w{Magnum::NoInit}; - Matrix5 v{Magnum::NoInit}; - std::tie(u, w, v) = Algorithms::svd(a); + Containers::Optional, Vector5, Matrix5>> uwv = Algorithms::svd(a); + CORRADE_VERIFY(uwv); /* Test composition */ - Matrix8 u2{u[0], u[1], u[2], u[3], u[4], Vector8{}, Vector8{}, Vector8{}}; - Matrix5x8 w2 = Matrix5x8::fromDiagonal(w); + Matrix8 u2{uwv->first()[0], uwv->first()[1], uwv->first()[2], uwv->first()[3], uwv->first()[4], Vector8{}, Vector8{}, Vector8{}}; + Matrix5x8 w2 = Matrix5x8::fromDiagonal(uwv->second()); { #ifdef CORRADE_TARGET_EMSCRIPTEN - CORRADE_EXPECT_FAIL_IF((std::is_same::value) && u2*w2*v.transposed() != a, + CORRADE_EXPECT_FAIL_IF((std::is_same::value) && u2*w2*uwv->third().transposed() != a, "Some strange problems with Double on recent Emscripten versions " "(1.36.5 worked fine, 1.37.1 works fine on larger optimization " "levels, not on -O1)."); #endif - CORRADE_COMPARE(u2*w2*v.transposed(), a); + CORRADE_COMPARE(u2*w2*uwv->third().transposed(), a); } /* Test that V is unitary */ - CORRADE_COMPARE(v*v.transposed(), Matrix5{IdentityInit}); - CORRADE_COMPARE(v.transposed()*v, Matrix5{IdentityInit}); + CORRADE_COMPARE(uwv->third()*uwv->third().transposed(), Matrix5{IdentityInit}); + CORRADE_COMPARE(uwv->third().transposed()*uwv->third(), Matrix5{IdentityInit}); /* Test W */ { #ifdef CORRADE_TARGET_EMSCRIPTEN - CORRADE_EXPECT_FAIL_IF((std::is_same::value && w != expected), + CORRADE_EXPECT_FAIL_IF((std::is_same::value && uwv->second() != expected), "Some strange problems with Double on recent Emscripten versions " "(1.36.5 worked fine, 1.37.1 worked fine on larger optimization " "levels, not on -O1, 1.37.5 works fine again)."); #endif - CORRADE_COMPARE(w, expected); + CORRADE_COMPARE(uwv->second(), expected); } } @@ -105,17 +103,14 @@ void SvdTest::decomposeRotationScaling() { Matrix4 a = Matrix4::rotationZ(35.0_degf)*Matrix4::scaling({1.5f, 2.0f, 1.0f}); - Matrix3x3 u{Magnum::NoInit}; - Vector3 w{Magnum::NoInit}; - Matrix3x3 v{Magnum::NoInit}; - std::tie(u, w, v) = Algorithms::svd(a.rotationScaling()); + Containers::Triple uwv{*Algorithms::svd(a.rotationScaling())}; - CORRADE_COMPARE(u*Matrix3x3::fromDiagonal(w)*v.transposed(), a.rotationScaling()); + CORRADE_COMPARE(uwv.first()*Matrix3x3::fromDiagonal(uwv.second())*uwv.third().transposed(), a.rotationScaling()); /* V contains flipped signs for the whole matrix, use it to fix the signs for U */ - CORRADE_COMPARE(w, (Vector3{1.5f, 2.0f, 1.0f})); - CORRADE_COMPARE(Matrix4::from(u*v.transposed(), {}), Matrix4::rotationZ(35.0_degf)); + CORRADE_COMPARE(uwv.second(), (Vector3{1.5f, 2.0f, 1.0f})); + CORRADE_COMPARE(Matrix4::from(uwv.first()*uwv.third().transposed(), {}), Matrix4::rotationZ(35.0_degf)); } void SvdTest::decomposeRotationShear() { @@ -128,17 +123,14 @@ void SvdTest::decomposeRotationShear() { /* Like above, but with order flipped, which results in a shear */ Matrix4 a = Matrix4::scaling({1.5f, 2.0f, 1.0f})*Matrix4::rotationZ(35.0_degf); - Matrix3x3 u{Magnum::NoInit}; - Vector3 w{Magnum::NoInit}; - Matrix3x3 v{Magnum::NoInit}; - std::tie(u, w, v) = Algorithms::svd(a.rotationScaling()); + Containers::Triple uwv{*Algorithms::svd(a.rotationScaling())}; - CORRADE_COMPARE(u*Matrix3x3::fromDiagonal(w)*v.transposed(), a.rotationScaling()); + CORRADE_COMPARE(uwv.first()*Matrix3x3::fromDiagonal(uwv.second())*uwv.third().transposed(), a.rotationScaling()); /* U contains a flipped sign for Z, use it to remove the sign from the transposed rotation matrix V */ - CORRADE_COMPARE(w, (Vector3{1.5f, 2.0f, 1.0f})); - CORRADE_COMPARE(Matrix4::from(u*v.transposed(), {}), Matrix4::rotationZ(35.0_degf)); + CORRADE_COMPARE(uwv.second(), (Vector3{1.5f, 2.0f, 1.0f})); + CORRADE_COMPARE(Matrix4::from(uwv.first()*uwv.third().transposed(), {}), Matrix4::rotationZ(35.0_degf)); } }}}}} diff --git a/src/singles/MagnumMath.hpp b/src/singles/MagnumMath.hpp index b69581bae..8f3aaa025 100644 --- a/src/singles/MagnumMath.hpp +++ b/src/singles/MagnumMath.hpp @@ -393,7 +393,7 @@ typedef Math::Frustum Frustumd; #include "Magnum/Math/Algorithms/GramSchmidt.h" #include "Magnum/Math/Algorithms/KahanSum.h" #include "Magnum/Math/Algorithms/Qr.h" -//#include "Magnum/Math/Algorithms/Svd.h" // TODO: uses +//#include "Magnum/Math/Algorithms/Svd.h" // TODO needs Optional and Triple #ifdef MAGNUM_MATH_GLM_INTEGRATION // {{includes}} #include "Magnum/GlmIntegration/Integration.h"