From cd6f156d5c044966acf7047e8c103b7d138ec66b Mon Sep 17 00:00:00 2001 From: codereader Date: Fri, 8 Oct 2021 20:11:59 +0200 Subject: [PATCH] #5773: Change IFace interface to return a Matrix3 instance instead of Matrix4 for the texture projection (which has only 6 nontrivial components anyway). --- include/ibrush.h | 8 ++-- radiantcore/brush/Face.cpp | 6 +-- radiantcore/brush/Face.h | 4 +- radiantcore/brush/TextureMatrix.cpp | 12 +++++- radiantcore/brush/TextureMatrix.h | 3 ++ radiantcore/brush/TextureProjection.cpp | 56 ++++++++++++------------- radiantcore/brush/TextureProjection.h | 12 +++--- test/Brush.cpp | 17 ++++---- test/TextureManipulation.cpp | 4 +- 9 files changed, 68 insertions(+), 54 deletions(-) diff --git a/include/ibrush.h b/include/ibrush.h index 3b90e5f900..d780c669f3 100644 --- a/include/ibrush.h +++ b/include/ibrush.h @@ -5,6 +5,7 @@ #include "math/Vector2.h" #include "math/Vector3.h" +#include "math/Matrix3.h" #include "math/Matrix4.h" #include @@ -186,11 +187,12 @@ class IFace virtual Matrix4 getTexDefMatrix() const = 0; /** - * The matrix used to project world coordinates to U/V space. + * The matrix used to project world coordinates to U/V space, after the winding vertices + * have been transformed to this face's axis base system. */ - virtual Matrix4 getProjectionMatrix() = 0; + virtual Matrix3 getProjectionMatrix() = 0; - virtual void setProjectionMatrix(const Matrix4& projection) = 0; + virtual void setProjectionMatrix(const Matrix3& projection) = 0; // Constructs the texture projection matrix from the given (world) vertex and texture coords. // Three vertices and their UV coordinates are enough to construct the texdef. diff --git a/radiantcore/brush/Face.cpp b/radiantcore/brush/Face.cpp index 8db4e20e5e..c696b7cfff 100644 --- a/radiantcore/brush/Face.cpp +++ b/radiantcore/brush/Face.cpp @@ -446,12 +446,12 @@ TextureProjection& Face::getProjection() return _texdef; } -Matrix4 Face::getProjectionMatrix() +Matrix3 Face::getProjectionMatrix() { - return getProjection().getTransform(); + return getProjection().getMatrix(); } -void Face::setProjectionMatrix(const Matrix4& projection) +void Face::setProjectionMatrix(const Matrix3& projection) { getProjection().setTransform(projection); texdefChanged(); diff --git a/radiantcore/brush/Face.h b/radiantcore/brush/Face.h index e6e642f9b1..2cce488685 100644 --- a/radiantcore/brush/Face.h +++ b/radiantcore/brush/Face.h @@ -203,8 +203,8 @@ class Face : Matrix4 getTexDefMatrix() const; - Matrix4 getProjectionMatrix() override; - void setProjectionMatrix(const Matrix4& projection) override; + Matrix3 getProjectionMatrix() override; + void setProjectionMatrix(const Matrix3& projection) override; SurfaceShader& getFaceShader(); const SurfaceShader& getFaceShader() const; diff --git a/radiantcore/brush/TextureMatrix.cpp b/radiantcore/brush/TextureMatrix.cpp index 8ab22c87f1..d78f412224 100644 --- a/radiantcore/brush/TextureMatrix.cpp +++ b/radiantcore/brush/TextureMatrix.cpp @@ -145,7 +145,8 @@ void TextureMatrix::normalise(float width, float height) { * As the member variables already ARE the matrix * components, they are just copied into the right places. */ -Matrix4 TextureMatrix::getTransform() const { +Matrix4 TextureMatrix::getTransform() const +{ // Initialise the return value with the identity matrix Matrix4 transform = Matrix4::getIdentity(); @@ -160,6 +161,15 @@ Matrix4 TextureMatrix::getTransform() const { return transform; } +Matrix3 TextureMatrix::getMatrix3() const +{ + return Matrix3::byRows( + coords[0][0], coords[0][1], coords[0][2], + coords[1][0], coords[1][1], coords[1][2], + 0, 0, 1 + ); +} + bool TextureMatrix::isSane() const { return !std::isnan(coords[0][0]) && !std::isinf(coords[0][0]) && diff --git a/radiantcore/brush/TextureMatrix.h b/radiantcore/brush/TextureMatrix.h index c903726786..5362499f83 100644 --- a/radiantcore/brush/TextureMatrix.h +++ b/radiantcore/brush/TextureMatrix.h @@ -84,6 +84,9 @@ struct TextureMatrix */ Matrix4 getTransform() const; + // Returns the Matrix3 form of this instance + Matrix3 getMatrix3() const; + // Checks if any of the matrix components are NaN or INF (in which case the matrix is not sane) bool isSane() const; }; diff --git a/radiantcore/brush/TextureProjection.cpp b/radiantcore/brush/TextureProjection.cpp index 6b835c5f9f..c71aa260ec 100644 --- a/radiantcore/brush/TextureProjection.cpp +++ b/radiantcore/brush/TextureProjection.cpp @@ -68,29 +68,33 @@ void TextureProjection::setTransform(const Matrix4& transform) } } -/* greebo: Returns the transformation matrix from the - * texture definitions members. - */ -Matrix4 TextureProjection::getTransform() const { +Matrix4 TextureProjection::getTransform() const +{ return matrix.getTransform(); } +Matrix3 TextureProjection::getMatrix() const +{ + return matrix.getMatrix3(); +} + void TextureProjection::shift(double s, double t) { matrix.shift(s, t); } -// Normalise projection for a given texture width and height. -void TextureProjection::normalise(float width, float height) { +void TextureProjection::normalise(float width, float height) +{ matrix.normalise(width, height); } // Fits a texture to a brush face void TextureProjection::fitTexture(std::size_t width, std::size_t height, - const Vector3& normal, const Winding& w, + const Vector3& normal, const Winding& winding, float s_repeat, float t_repeat) { - if (w.size() < 3) { + if (winding.size() < 3) + { return; } @@ -100,16 +104,15 @@ void TextureProjection::fitTexture(std::size_t width, std::size_t height, // the current texture transform Matrix4 local2tex = st2tex; { - Matrix4 xyz2st; - xyz2st = getBasisTransformForNormal(normal); + Matrix4 xyz2st = getBasisTransformForNormal(normal); local2tex.multiplyBy(xyz2st); } // the bounds of the current texture transform AABB bounds; - for (Winding::const_iterator i = w.begin(); i != w.end(); ++i) { - Vector3 texcoord = local2tex.transformPoint(i->vertex); - bounds.includePoint(texcoord); + for (const auto& vertex : winding) + { + bounds.includePoint(local2tex.transformPoint(vertex.vertex)); } bounds.origin.z() = 0; bounds.extents.z() = 1; @@ -225,16 +228,11 @@ Matrix4 TextureProjection::getWorldToTexture(const Vector3& normal, const Matrix return local2tex; } -/* greebo: This method calculates the texture coordinates for the brush winding vertices - * via matrix operations and stores the results into the Winding vertices (together with the - * tangent and bitangent vectors) - * - * Note: The matrix localToWorld is basically useless at the moment, as it is the identity matrix for faces, and this method - * gets called on face operations only... */ -void TextureProjection::emitTextureCoordinates(Winding& w, const Vector3& normal, const Matrix4& localToWorld) const { - - // Quit, if we have less than three points (degenerate brushes?) - if (w.size() < 3) { +void TextureProjection::emitTextureCoordinates(Winding& winding, const Vector3& normal, const Matrix4& localToWorld) const +{ + // Quit if we have less than three points (degenerate brushes?) + if (winding.size() < 3) + { return; } @@ -265,17 +263,17 @@ void TextureProjection::emitTextureCoordinates(Winding& w, const Vector3& normal // Cycle through the winding vertices and apply the texture transformation matrix // onto each of them. - for (Winding::iterator i = w.begin(); i != w.end(); ++i) + for (auto& vertex : winding) { - Vector3 texcoord = local2tex.transformPoint(i->vertex); + Vector3 texcoord = local2tex.transformPoint(vertex.vertex); // Store the s,t coordinates into the winding texcoord vector - i->texcoord[0] = texcoord[0]; - i->texcoord[1] = texcoord[1]; + vertex.texcoord[0] = texcoord[0]; + vertex.texcoord[1] = texcoord[1]; // Save the tangent and bitangent vectors, they are the same for all the face vertices - i->tangent = tangent; - i->bitangent = bitangent; + vertex.tangent = tangent; + vertex.bitangent = bitangent; } } diff --git a/radiantcore/brush/TextureProjection.h b/radiantcore/brush/TextureProjection.h index e03f0b686e..3f4b4cdb0f 100644 --- a/radiantcore/brush/TextureProjection.h +++ b/radiantcore/brush/TextureProjection.h @@ -39,13 +39,12 @@ class TextureProjection void setTransform(const Matrix3& transform); void setTransform(const Matrix4& transform); Matrix4 getTransform() const; + Matrix3 getMatrix() const; // s and t are texture coordinates, not pixels void shift(double s, double t); - // Normalise projection for a given texture width and height. - void normalise(float width, float height); - +public: // Fits a texture to a brush face void fitTexture(std::size_t width, std::size_t height, const Vector3& normal, const Winding& w, float s_repeat, float t_repeat); @@ -53,7 +52,7 @@ class TextureProjection void alignTexture(IFace::AlignEdge align, const Winding& winding); // greebo: Saves the texture definitions into the brush winding points - void emitTextureCoordinates(Winding& w, const Vector3& normal, const Matrix4& localToWorld) const; + void emitTextureCoordinates(Winding& winding, const Vector3& normal, const Matrix4& localToWorld) const; // Calculates the UV coords of a single point Vector2 getTextureCoordsForVertex(const Vector3& point, const Vector3& normal, const Matrix4& localToWorld) const; @@ -64,4 +63,7 @@ class TextureProjection // Calculate the texture projection for the desired set of UVs and XYZ void calculateFromPoints(const Vector3 points[3], const Vector2 uvs[3], const Vector3& normal); -}; // class TextureProjection +private: + // Normalise projection for a given texture width and height. + void normalise(float width, float height); +}; diff --git a/test/Brush.cpp b/test/Brush.cpp index 70269ea1f2..ee5c2189dd 100644 --- a/test/Brush.cpp +++ b/test/Brush.cpp @@ -56,17 +56,16 @@ class BrushTest: public RadiantTest } }; -inline bool isSane(const Matrix4& matrix) +inline bool isSane(double value) { - for (std::size_t i = 0; i < 15; ++i) - { - if (std::isnan(matrix[i]) || std::isinf(matrix[i])) - { - return false; - } - } + return !std::isnan(value) && !std::isinf(value); +} - return true; +inline bool isSane(const Matrix3& matrix) +{ + return isSane(matrix.xx()) && isSane(matrix.xy()) && isSane(matrix.xz()) && + isSane(matrix.yx()) && isSane(matrix.yy()) && isSane(matrix.yz()) && + isSane(matrix.zx()) && isSane(matrix.zy()) && isSane(matrix.zz()); } TEST_F(BrushTest, FitTextureWithZeroScale) diff --git a/test/TextureManipulation.cpp b/test/TextureManipulation.cpp index 640b86fa5e..a1cdfdfdda 100644 --- a/test/TextureManipulation.cpp +++ b/test/TextureManipulation.cpp @@ -637,8 +637,8 @@ TEST_F(TextureManipulationTest, FaceGetTexelScale) // A 1024 texture width fit to a 512px wide face: texel scale = 2 // A 512 texture height fit to a 512px wide face: texel scale = 1 - auto expectedTexelScaleX = textureWidth / 512; - auto expectedTexelScaleY = textureHeight / 512; + auto expectedTexelScaleX = textureWidth / 512.0; + auto expectedTexelScaleY = textureHeight / 512.0; EXPECT_NEAR(texelScale.x(), expectedTexelScaleX, 0.01) << "Texel scale X is off"; EXPECT_NEAR(texelScale.y(), expectedTexelScaleY, 0.01) << "Texel scale Y is off";