From 7612f446b5f6e09f23f9702afea48c176276828a Mon Sep 17 00:00:00 2001 From: Tom Poole Date: Wed, 27 Jul 2022 15:52:49 +0100 Subject: [PATCH] Matrix3D: Fixed an ordering bug in the multiplication operator --- BREAKING-CHANGES.txt | 32 +++++++++++++++ examples/GUI/OpenGLAppDemo.h | 6 +-- examples/GUI/OpenGLDemo.h | 4 +- modules/juce_opengl/geometry/juce_Matrix3D.h | 42 ++++++++++---------- 4 files changed, 58 insertions(+), 26 deletions(-) diff --git a/BREAKING-CHANGES.txt b/BREAKING-CHANGES.txt index 90568f220f..3d5728c04b 100644 --- a/BREAKING-CHANGES.txt +++ b/BREAKING-CHANGES.txt @@ -1,6 +1,38 @@ JUCE breaking changes ===================== +Develop +======= + +Change +------ +The Matrix3D (Vector3D vector) constructor has been replaced with an +explicit static Matrix3D fromTranslation (Vector3D vector) function, and a +bug in the behaviour of the multipication operator that reversed the order of +operations has been addressed. + +Possible Issues +--------------- +Code using the old constructor will not compile. Code that relied upon the order +of multiplication operations will return different results. + +Workaround +---------- +Code that was using the old constructor must use the new static function. Code +that relied on the order of multiplication operations will need to have the +order of the arguments reversed. With the old code A * B was returning BA rather +than AB. + +Rationale +--------- +Previously a matrix multipled by a vector would return a matrix, rather than a +vector, as the multiplied-by vector would be automatically converted into a +matrix during the operation. Removing the converting constructor makes +everything much more explicit and there is no confusion about dimensionality. +The current multiplication routine also included a bug where A * B resulted in +BA rather than AB, which needed to be addressed. + + Version 7.0.0 ============= diff --git a/examples/GUI/OpenGLAppDemo.h b/examples/GUI/OpenGLAppDemo.h index d1b923acdb..ee5cbd6581 100644 --- a/examples/GUI/OpenGLAppDemo.h +++ b/examples/GUI/OpenGLAppDemo.h @@ -93,10 +93,10 @@ public: Matrix3D getViewMatrix() const { - Matrix3D viewMatrix ({ 0.0f, 0.0f, -10.0f }); - Matrix3D rotationMatrix = viewMatrix.rotation ({ -0.3f, 5.0f * std::sin ((float) getFrameCounter() * 0.01f), 0.0f }); + auto viewMatrix = Matrix3D::fromTranslation ({ 0.0f, 0.0f, -10.0f }); + auto rotationMatrix = viewMatrix.rotation ({ -0.3f, 5.0f * std::sin ((float) getFrameCounter() * 0.01f), 0.0f }); - return rotationMatrix * viewMatrix; + return viewMatrix * rotationMatrix; } void render() override diff --git a/examples/GUI/OpenGLDemo.h b/examples/GUI/OpenGLDemo.h index c1bd9cba2f..8718acdf3c 100644 --- a/examples/GUI/OpenGLDemo.h +++ b/examples/GUI/OpenGLDemo.h @@ -893,10 +893,10 @@ public: { const ScopedLock lock (mutex); - auto viewMatrix = draggableOrientation.getRotationMatrix() * Vector3D (0.0f, 1.0f, -10.0f); + auto viewMatrix = Matrix3D::fromTranslation ({ 0.0f, 1.0f, -10.0f }) * draggableOrientation.getRotationMatrix(); auto rotationMatrix = Matrix3D::rotation ({ rotation, rotation, -0.3f }); - return rotationMatrix * viewMatrix; + return viewMatrix * rotationMatrix; } void setTexture (OpenGLUtils::DemoTexture* t) diff --git a/modules/juce_opengl/geometry/juce_Matrix3D.h b/modules/juce_opengl/geometry/juce_Matrix3D.h index eb65213609..0bd2862eaf 100644 --- a/modules/juce_opengl/geometry/juce_Matrix3D.h +++ b/modules/juce_opengl/geometry/juce_Matrix3D.h @@ -88,12 +88,12 @@ public: } /** Creates a matrix from a 3D vector translation. */ - Matrix3D (Vector3D vector) noexcept + static Matrix3D fromTranslation (Vector3D vector) noexcept { - mat[0] = Type (1); mat[1] = 0; mat[2] = 0; mat[3] = 0; - mat[4] = 0; mat[5] = Type (1); mat[6] = 0; mat[7] = 0; - mat[8] = 0; mat[9] = 0; mat[10] = Type (1); mat[11] = 0; - mat[12] = vector.x; mat[13] = vector.y; mat[14] = vector.z; mat[15] = Type (1); + return { Type (1), 0, 0, 0, + 0, Type (1), 0, 0, + 0, 0, Type (1), 0, + vector.x, vector.y, vector.z, Type (1) }; } /** Returns a new matrix from the given frustum values. */ @@ -129,22 +129,22 @@ public: { auto&& m2 = other.mat; - return { mat[0] * m2[0] + mat[1] * m2[4] + mat[2] * m2[8] + mat[3] * m2[12], - mat[0] * m2[1] + mat[1] * m2[5] + mat[2] * m2[9] + mat[3] * m2[13], - mat[0] * m2[2] + mat[1] * m2[6] + mat[2] * m2[10] + mat[3] * m2[14], - mat[0] * m2[3] + mat[1] * m2[7] + mat[2] * m2[11] + mat[3] * m2[15], - mat[4] * m2[0] + mat[5] * m2[4] + mat[6] * m2[8] + mat[7] * m2[12], - mat[4] * m2[1] + mat[5] * m2[5] + mat[6] * m2[9] + mat[7] * m2[13], - mat[4] * m2[2] + mat[5] * m2[6] + mat[6] * m2[10] + mat[7] * m2[14], - mat[4] * m2[3] + mat[5] * m2[7] + mat[6] * m2[11] + mat[7] * m2[15], - mat[8] * m2[0] + mat[9] * m2[4] + mat[10] * m2[8] + mat[11] * m2[12], - mat[8] * m2[1] + mat[9] * m2[5] + mat[10] * m2[9] + mat[11] * m2[13], - mat[8] * m2[2] + mat[9] * m2[6] + mat[10] * m2[10] + mat[11] * m2[14], - mat[8] * m2[3] + mat[9] * m2[7] + mat[10] * m2[11] + mat[11] * m2[15], - mat[12] * m2[0] + mat[13] * m2[4] + mat[14] * m2[8] + mat[15] * m2[12], - mat[12] * m2[1] + mat[13] * m2[5] + mat[14] * m2[9] + mat[15] * m2[13], - mat[12] * m2[2] + mat[13] * m2[6] + mat[14] * m2[10] + mat[15] * m2[14], - mat[12] * m2[3] + mat[13] * m2[7] + mat[14] * m2[11] + mat[15] * m2[15] }; + return { mat[0] * m2[0] + mat[4] * m2[1] + mat[8] * m2[2] + mat[12] * m2[3], + mat[1] * m2[0] + mat[5] * m2[1] + mat[9] * m2[2] + mat[13] * m2[3], + mat[2] * m2[0] + mat[6] * m2[1] + mat[10] * m2[2] + mat[14] * m2[3], + mat[3] * m2[0] + mat[7] * m2[1] + mat[11] * m2[2] + mat[15] * m2[3], + mat[0] * m2[4] + mat[4] * m2[5] + mat[8] * m2[6] + mat[12] * m2[7], + mat[1] * m2[4] + mat[5] * m2[5] + mat[9] * m2[6] + mat[13] * m2[7], + mat[2] * m2[4] + mat[6] * m2[5] + mat[10] * m2[6] + mat[14] * m2[7], + mat[3] * m2[4] + mat[7] * m2[5] + mat[11] * m2[6] + mat[15] * m2[7], + mat[0] * m2[8] + mat[4] * m2[9] + mat[8] * m2[10] + mat[12] * m2[11], + mat[1] * m2[8] + mat[5] * m2[9] + mat[9] * m2[10] + mat[13] * m2[11], + mat[2] * m2[8] + mat[6] * m2[9] + mat[10] * m2[10] + mat[14] * m2[11], + mat[3] * m2[8] + mat[7] * m2[9] + mat[11] * m2[10] + mat[15] * m2[11], + mat[0] * m2[12] + mat[4] * m2[13] + mat[8] * m2[14] + mat[12] * m2[15], + mat[1] * m2[12] + mat[5] * m2[13] + mat[9] * m2[14] + mat[13] * m2[15], + mat[2] * m2[12] + mat[6] * m2[13] + mat[10] * m2[14] + mat[14] * m2[15], + mat[3] * m2[12] + mat[7] * m2[13] + mat[11] * m2[14] + mat[15] * m2[15] }; } /** The 4x4 matrix values. These are stored in the standard OpenGL order. */