#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

@@ -5,6 +5,7 @@
 
 #include "math/Vector2.h"
 #include "math/Vector3.h"
+#include "math/Matrix3.h"
 #include "math/Matrix4.h"
 #include <vector>
 
@@ -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.

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();

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;

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]) &&

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;
 };

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;
     }
 }
 

radiantcore/brush/TextureProjection.h

@@ -39,21 +39,20 @@ 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);
 
     // Aligns this texture to the given edge of the winding
     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);
+};

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)

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";