diff --git a/doc/changelog.dox b/doc/changelog.dox index 2e724ee3d..3080399f0 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -172,9 +172,6 @@ See also: @subsubsection changelog-latest-changes-math Math library -- @ref Math::Matrix3::scaling() const and @ref Math::Matrix4::scaling() const - now return a signed scaling vector, taking into account negative scaling as - well - Functions in @ref Magnum/Math/FunctionsBatch.h now accept any type that's convertible to a @ref Corrade::Containers::StridedArrayView without having to add explicit casts or template parameters @@ -314,10 +311,6 @@ See also: - The 4-argument @ref GL::DynamicAttribute constructor was not marked as @cpp explicit @ce by mistake, it's done now to enforce readability in long expressions. -- @ref Math::Matrix3::scaling() const and @ref Math::Matrix4::scaling() const - now return a signed scaling vector instead of unsigned in order to prevent - information loss. To restore the old behavior, apply @ref Math::abs() on - the result. - The @ref Magnum/Math/FunctionsBatch.h header is no longer included from @ref Magnum/Math/Functions.h for backwards compatibility in order to speed up compile times. diff --git a/src/Magnum/Math/Matrix3.h b/src/Magnum/Math/Matrix3.h index 9beb0204c..a32d83413 100644 --- a/src/Magnum/Math/Matrix3.h +++ b/src/Magnum/Math/Matrix3.h @@ -431,11 +431,11 @@ template class Matrix3: public Matrix3x3 { /** * @brief Non-uniform scaling part of the matrix * - * *Signed* length of vectors in upper-left 2x2 part of the matrix. Use - * the faster alternative @ref scalingSquared() where possible. - * Assuming the following matrix, with the upper-left 2x2 part - * represented by column vectors @f$ \boldsymbol{a} @f$ and - * @f$ \boldsymbol{b} @f$: @f[ + * Length of vectors in upper-left 2x2 part of the matrix. Use the + * faster alternative @ref scalingSquared() where possible. Assuming + * the following matrix, with the upper-left 2x2 part represented by + * column vectors @f$ \boldsymbol{a} @f$ and @f$ \boldsymbol{b} @f$: + * @f[ * \begin{pmatrix} * \color{m-warning} a_x & \color{m-warning} b_x & t_x \\ * \color{m-warning} a_y & \color{m-warning} b_y & t_y \\ @@ -447,17 +447,25 @@ template class Matrix3: public Matrix3x3 { * * the resulting scaling vector is: @f[ * \boldsymbol{s} = \begin{pmatrix} - * \operatorname{sgn}(a_x) | \boldsymbol{a} | \\ - * \operatorname{sgn}(a_y) | \boldsymbol{b} | + * | \boldsymbol{a} | \\ + * | \boldsymbol{b} | * \end{pmatrix} * @f] * + * Note that the returned vector is sign-less and the signs are instead + * contained in @ref rotation() const / @ref rotationShear() const in + * order to ensure @f$ \boldsymbol{R} \boldsymbol{S} = \boldsymbol{M} @f$ + * for @f$ \boldsymbol{R} @f$ and @f$ \boldsymbol{S} @f$ extracted out + * of @f$ \boldsymbol{M} @f$. The signs can be extracted for example by + * applying @ref Math::sign() on a @ref diagonal(), but keep in mind + * that the signs can be negative even for pure rotation matrices. + * * @see @ref scalingSquared(), @ref uniformScaling(), * @ref rotation() const, @ref Matrix4::scaling() const */ Vector2 scaling() const { - return {(*this)[0].xy().length()*T((*this)[0][0] < T(0) ? -1 : 1), - (*this)[1].xy().length()*T((*this)[1][1] < T(0) ? -1 : 1)}; + return {(*this)[0].xy().length(), + (*this)[1].xy().length()}; } /** diff --git a/src/Magnum/Math/Matrix4.h b/src/Magnum/Math/Matrix4.h index 856d320ea..11da5a499 100644 --- a/src/Magnum/Math/Matrix4.h +++ b/src/Magnum/Math/Matrix4.h @@ -685,11 +685,11 @@ template class Matrix4: public Matrix4x4 { /** * @brief Non-uniform scaling part of the matrix * - * *Signed* length of vectors in upper-left 3x3 part of the matrix. Use - * the faster alternative @ref scalingSquared() where possible. - * Assuming the following matrix, with the upper-left 3x3 part - * represented by column vectors @f$ \boldsymbol{a} @f$, - * @f$ \boldsymbol{b} @f$ and @f$ \boldsymbol{c} @f$: @f[ + * Length of vectors in upper-left 3x3 part of the matrix. Use the + * faster alternative @ref scalingSquared() where possible. Assuming + * the following matrix, with the upper-left 3x3 part represented by + * column vectors @f$ \boldsymbol{a} @f$, @f$ \boldsymbol{b} @f$ and + * @f$ \boldsymbol{c} @f$: @f[ * \begin{pmatrix} * \color{m-warning} a_x & \color{m-warning} b_x & \color{m-warning} c_x & t_x \\ * \color{m-warning} a_y & \color{m-warning} b_y & \color{m-warning} c_y & t_y \\ @@ -702,19 +702,27 @@ template class Matrix4: public Matrix4x4 { * * the resulting scaling vector is: @f[ * \boldsymbol{s} = \begin{pmatrix} - * \operatorname{sgn}(a_x) | \boldsymbol{a} | \\ - * \operatorname{sgn}(b_y) | \boldsymbol{b} | \\ - * \operatorname{sgn}(c_z) | \boldsymbol{c} | + * | \boldsymbol{a} | \\ + * | \boldsymbol{b} | \\ + * | \boldsymbol{c} | * \end{pmatrix} * @f] * + * Note that the returned vector is sign-less and the signs are instead + * contained in @ref rotation() const / @ref rotationShear() const in + * order to ensure @f$ \boldsymbol{R} \boldsymbol{S} = \boldsymbol{M} @f$ + * for @f$ \boldsymbol{R} @f$ and @f$ \boldsymbol{S} @f$ extracted out + * of @f$ \boldsymbol{M} @f$. The signs can be extracted for example by + * applying @ref Math::sign() on a @ref diagonal(), but keep in mind + * that the signs can be negative even for pure rotation matrices. + * * @see @ref scalingSquared(), @ref uniformScaling(), * @ref rotation() const, @ref Matrix3::scaling() const */ Vector3 scaling() const { - return {(*this)[0].xyz().length()*T((*this)[0][0] < T(0) ? -1 : 1), - (*this)[1].xyz().length()*T((*this)[1][1] < T(0) ? -1 : 1), - (*this)[2].xyz().length()*T((*this)[2][2] < T(0) ? -1 : 1)}; + return {(*this)[0].xyz().length(), + (*this)[1].xyz().length(), + (*this)[2].xyz().length()}; } /** diff --git a/src/Magnum/Math/Test/Matrix3Test.cpp b/src/Magnum/Math/Test/Matrix3Test.cpp index 2fb8c57ba..3f008b687 100644 --- a/src/Magnum/Math/Test/Matrix3Test.cpp +++ b/src/Magnum/Math/Test/Matrix3Test.cpp @@ -90,6 +90,7 @@ struct Matrix3Test: Corrade::TestSuite::Tester { void rotationNormalizedPart(); void rotationNormalizedPartNotOrthogonal(); void scalingPart(); + void rotationScalingPartNegative(); void uniformScalingPart(); void uniformScalingPartNotUniform(); void vectorParts(); @@ -140,6 +141,7 @@ Matrix3Test::Matrix3Test() { &Matrix3Test::rotationNormalizedPart, &Matrix3Test::rotationNormalizedPartNotOrthogonal, &Matrix3Test::scalingPart, + &Matrix3Test::rotationScalingPartNegative, &Matrix3Test::uniformScalingPart, &Matrix3Test::uniformScalingPartNotUniform, &Matrix3Test::vectorParts, @@ -526,18 +528,39 @@ void Matrix3Test::scalingPart() { Matrix3 translationRotationScaling = Matrix3::translation({2.0f, -3.0f})* Matrix3::rotation(15.0_degf)* - Matrix3::scaling({0.5f, -3.5f}); + Matrix3::scaling({0.5f, 3.5f}); - CORRADE_COMPARE(translationRotationScaling.scaling(), (Vector2{0.5f, -3.5f})); + CORRADE_COMPARE(translationRotationScaling.scaling(), (Vector2{0.5f, 3.5f})); CORRADE_COMPARE(translationRotationScaling.scalingSquared(), (Vector2{0.25f, 12.25f})); +} - Matrix3 rotationScaling = +void Matrix3Test::rotationScalingPartNegative() { + /* Large angle */ + Matrix3 largeAngle = Matrix3::rotation(215.0_degf)* - Matrix3::scaling({-0.5f, 3.5f}); - - /* Sign inverted because of the rotation */ - CORRADE_COMPARE(rotationScaling.scaling(), (Vector2{0.5f, -3.5f})); - CORRADE_COMPARE(rotationScaling.scalingSquared(), (Vector2{0.25f, 12.25f})); + Matrix3::scaling({0.5f, 3.5f}); + CORRADE_COMPARE(Matrix3::from(largeAngle.rotation(), {}), + Matrix3::rotation(215.0_degf)); + CORRADE_COMPARE(largeAngle.scaling(), (Vector2{0.5f, 3.5f})); + /* The parts should combine back to the same matrix */ + CORRADE_COMPARE( + Matrix3::from(largeAngle.rotation(), {})* + Matrix3::scaling(largeAngle.scaling()), + largeAngle); + + /* The sign gets contained in the rotation */ + Matrix3 negativeScaling = + Matrix3::rotation(15.0_degf)* + Matrix3::scaling({0.5f, -3.5f}); + CORRADE_COMPARE(Matrix3::from(negativeScaling.rotation(), {}), + Matrix3::rotation(15.0_degf)* + Matrix3::scaling(Vector2::yScale(-1))); + CORRADE_COMPARE(negativeScaling.scaling(), (Vector2{0.5f, 3.5f})); + /* The parts should combine back to the same matrix */ + CORRADE_COMPARE( + Matrix3::from(negativeScaling.rotation(), {})* + Matrix3::scaling(negativeScaling.scaling()), + negativeScaling); } void Matrix3Test::uniformScalingPart() { diff --git a/src/Magnum/Math/Test/Matrix4Test.cpp b/src/Magnum/Math/Test/Matrix4Test.cpp index 6ecf3276d..3feb71379 100644 --- a/src/Magnum/Math/Test/Matrix4Test.cpp +++ b/src/Magnum/Math/Test/Matrix4Test.cpp @@ -105,6 +105,7 @@ struct Matrix4Test: Corrade::TestSuite::Tester { void rotationNormalizedPart(); void rotationNormalizedPartNotOrthogonal(); void scalingPart(); + void rotationScalingPartNegative(); void uniformScalingPart(); void uniformScalingPartNotUniform(); void normalMatrixPart(); @@ -173,6 +174,7 @@ Matrix4Test::Matrix4Test() { &Matrix4Test::rotationNormalizedPart, &Matrix4Test::rotationNormalizedPartNotOrthogonal, &Matrix4Test::scalingPart, + &Matrix4Test::rotationScalingPartNegative, &Matrix4Test::uniformScalingPart, &Matrix4Test::uniformScalingPartNotUniform, &Matrix4Test::normalMatrixPart, @@ -760,18 +762,39 @@ void Matrix4Test::scalingPart() { Matrix4 translationRotationScaling = Matrix4::translation({2.0f, 5.0f, -3.0f})* Matrix4::rotation(-74.0_degf, Vector3(-1.0f, 2.0f, 2.0f).normalized())* - Matrix4::scaling({0.5f, -3.5f, 1.2f}); + Matrix4::scaling({0.5f, 3.5f, 1.2f}); - CORRADE_COMPARE(translationRotationScaling.scaling(), (Vector3{0.5f, -3.5f, 1.2f})); + CORRADE_COMPARE(translationRotationScaling.scaling(), (Vector3{0.5f, 3.5f, 1.2f})); CORRADE_COMPARE(translationRotationScaling.scalingSquared(), (Vector3{0.25f, 12.25f, 1.44f})); +} - Matrix4 rotationScaling = - Matrix4::rotation(-174.0_degf, Vector3(-1.0f, 2.0f, 2.0f).normalized())* - Matrix4::scaling({-0.5f, 3.5f, 1.2f}); - - /* Sign inverted because of the rotation */ - CORRADE_COMPARE(rotationScaling.scaling(), (Vector3{0.5f, -3.5f, -1.2f})); - CORRADE_COMPARE(rotationScaling.scalingSquared(), (Vector3{0.25f, 12.25f, 1.44f})); +void Matrix4Test::rotationScalingPartNegative() { + /* Large angle */ + Matrix4 largeAngle = + Matrix4::rotationY(215.0_degf)* + Matrix4::scaling({0.5f, 3.5f, 1.2f}); + CORRADE_COMPARE(Matrix4::from(largeAngle.rotation(), {}), + Matrix4::rotationY(215.0_degf)); + CORRADE_COMPARE(largeAngle.scaling(), (Vector3{0.5f, 3.5f, 1.2f})); + /* The parts should combine back to the same matrix */ + CORRADE_COMPARE( + Matrix4::from(largeAngle.rotation(), {})* + Matrix4::scaling(largeAngle.scaling()), + largeAngle); + + /* The sign gets contained in the rotation */ + Matrix4 negativeScaling = + Matrix4::rotationY(15.0_degf)* + Matrix4::scaling({0.5f, -3.5f, 1.2f}); + CORRADE_COMPARE(Matrix4::from(negativeScaling.rotation(), {}), + Matrix4::rotationY(15.0_degf)* + Matrix4::scaling(Vector3::yScale(-1))); + CORRADE_COMPARE(negativeScaling.scaling(), (Vector3{0.5f, 3.5f, 1.2f})); + /* The parts should combine back to the same matrix */ + CORRADE_COMPARE( + Matrix4::from(negativeScaling.rotation(), {})* + Matrix4::scaling(negativeScaling.scaling()), + negativeScaling); } void Matrix4Test::uniformScalingPart() {