Separate const and modifying Matrix4 column accessors

Distinguish the methods which return a modifiable reference to columns by
appending 'Ref' to the method name, i.e. xCol3Ref(), tColRef() etc. Calling
code is updated to use these versions where a reference is *required*, either
because it is assigned to as an lvalue or returned by const reference to a
higher-level function.

The remaining xCol3(), yCol3() etc methods now return by value. Changing a
method from returning a reference to returning a value is potentially
dangerous, because any calling code which continues to treat the returned value
as if it were a reference (e.g. assigning to it) will do the wrong thing, and
neither G++ nor CLang seem to be able to warn about this. Hopefully the
combination of manual searching along with unit tests has ensured that there
are no remaining uses of xCol3() which should be xCol3Ref().

libs/Transformable.h

@@ -195,9 +195,9 @@ class Transformable :
 	static Matrix4 getMatrixForComponents(const Vector3& translation, const Quaternion& rotation, const Vector3& scale)
 	{
 		Matrix4 result(Matrix4::getRotationQuantised(rotation));
-		result.xCol3() *= scale.x();
-		result.yCol3() *= scale.y();
-		result.zCol3() *= scale.z();
+		result.xCol3Ref() *= scale.x();
+		result.yCol3Ref() *= scale.y();
+		result.zCol3Ref() *= scale.z();
 		result.tx() = translation.x();
 		result.ty() = translation.y();
 		result.tz() = translation.z();

libs/math/Matrix4.h

@@ -211,35 +211,35 @@ class alignas(16) Matrix4
      * Return columns of the matrix as vectors.
      * \{
      */
-    Vector3& xCol3()
+    Vector3& xCol3Ref()
     {
         return reinterpret_cast<Vector3&>(xx());
     }
-    const Vector3& xCol3() const
+    Vector3 xCol3() const
     {
         return reinterpret_cast<const Vector3&>(xx());
     }
-    Vector3& yCol3()
+    Vector3& yCol3Ref()
     {
         return reinterpret_cast<Vector3&>(yx());
     }
-    const Vector3& yCol3() const
+    Vector3 yCol3() const
     {
         return reinterpret_cast<const Vector3&>(yx());
     }
-    Vector3& zCol3()
+    Vector3& zCol3Ref()
     {
         return reinterpret_cast<Vector3&>(zx());
     }
-    const Vector3& zCol3() const
+    Vector3 zCol3() const
     {
         return reinterpret_cast<const Vector3&>(zx());
     }
-    Vector4& tCol()
+    Vector4& tColRef()
     {
         return reinterpret_cast<Vector4&>(tx());
     }
-    const Vector4& tCol() const
+    Vector4 tCol() const
     {
         return reinterpret_cast<const Vector4&>(tx());
     }
@@ -407,7 +407,7 @@ class alignas(16) Matrix4
     Handedness getHandedness() const;
 
     /// Return the 3-element translation component of this matrix
-    const Vector3& translation() const
+    Vector3 translation() const
     {
         return tCol().getVector3();
     }

libs/pivot.h

@@ -12,10 +12,10 @@ inline void billboard_viewplaneOriented(Matrix4& rotation, const Matrix4& world2
   Vector3 x(world2screen.xCol3().getNormalised());
   Vector3 y(world2screen.yCol3().getNormalised());
   Vector3 z(world2screen.zCol3().getNormalised());
-  rotation.yCol3() = Vector3(x.y(), y.y(), z.y());
-  rotation.zCol3() = -Vector3(x.z(), y.z(), z.z());
-  rotation.xCol3() = rotation.yCol3().cross(rotation.zCol3()).getNormalised();
-  rotation.yCol3() = rotation.zCol3().cross(rotation.xCol3());
+  rotation.yCol3Ref() = Vector3(x.y(), y.y(), z.y());
+  rotation.zCol3Ref() = -Vector3(x.z(), y.z(), z.z());
+  rotation.xCol3Ref() = rotation.yCol3().cross(rotation.zCol3()).getNormalised();
+  rotation.yCol3Ref() = rotation.zCol3().cross(rotation.xCol3());
 #else
   Matrix4 screen2world(matrix4_full_inverse(world2screen));
 
@@ -54,10 +54,10 @@ inline void billboard_viewpointOriented(Matrix4& rotation, const Matrix4& world2
 
 #if 1
   rotation = Matrix4::getIdentity();
-  rotation.yCol3() = screen2world.yCol3().getNormalised();
-  rotation.zCol3() = -screen2world.zCol3().getNormalised();
-  rotation.xCol3() = rotation.yCol3().cross(rotation.zCol3()).getNormalised();
-  rotation.yCol3() = rotation.zCol3().cross(rotation.xCol3());
+  rotation.yCol3Ref() = screen2world.yCol3().getNormalised();
+  rotation.zCol3Ref() = -screen2world.zCol3().getNormalised();
+  rotation.xCol3Ref() = rotation.yCol3().cross(rotation.zCol3()).getNormalised();
+  rotation.yCol3Ref() = rotation.zCol3().cross(rotation.xCol3());
 #else
   Vector3 near_(
       matrix4_transformed_vector4(

radiantcore/brush/TextureProjection.cpp

@@ -90,7 +90,7 @@ Matrix4 TextureProjection::getBasisForNormal(const Vector3& normal) const {
     Matrix4 basis;
 
     basis = Matrix4::getIdentity();
-    ComputeAxisBase(normal, basis.xCol3(), basis.yCol3());
+    ComputeAxisBase(normal, basis.xCol3Ref(), basis.yCol3Ref());
     basis.zCol3() = normal;
 
     // At this point the basis matrix contains three lines that are

radiantcore/particles/ParticleNode.cpp

@@ -49,9 +49,9 @@ Matrix4 ParticleNode::localToParent() const
 		_local2Parent = parent->localToWorld();
 
 		// compensate the parent rotation only
-		_local2Parent.tCol().x() = 0;
-		_local2Parent.tCol().y() = 0;
-		_local2Parent.tCol().z() = 0;
+		_local2Parent.tColRef().x() = 0;
+		_local2Parent.tColRef().y() = 0;
+		_local2Parent.tColRef().z() = 0;
 
 		_local2Parent.invert();
 	}
@@ -95,7 +95,7 @@ void ParticleNode::update(const VolumeTest& viewVolume) const
 {
 	// Get the view rotation and cancel out the translation part
 	Matrix4 viewRotation = viewVolume.GetModelview();
-	viewRotation.tCol() = Vector4(0,0,0,1);
+	viewRotation.tColRef() = Vector4(0,0,0,1);
 
 	// Get the main direction of our parent entity
 	_renderableParticle->setMainDirection(_renderEntity->getDirection());

radiantcore/selection/ManipulationPivot.cpp

@@ -58,7 +58,7 @@ const Vector3& ManipulationPivot::getVector3()
 		updateFromSelection();
 	}
 
-	return _pivot2World.tCol().getVector3();
+	return _pivot2World.tColRef().getVector3();
 }
 
 void ManipulationPivot::setFromMatrix(const Matrix4& newPivot2World)
@@ -112,7 +112,7 @@ void ManipulationPivot::applyTranslation(const Vector3& translation)
 	if (_snapPivotToGrid)
 	{
 		// The resulting pivot should be grid-snapped
-		_pivot2World.tCol().getVector3().snap(GlobalGrid().getGridSize());
+		_pivot2World.tColRef().getVector3().snap(GlobalGrid().getGridSize());
 	}
 }
 

radiantcore/selection/manipulators/RotateManipulator.cpp

@@ -100,8 +100,8 @@ void RotateManipulator::updateCircleTransforms()
     if(_circleX_visible)
     {
       _local2worldX = Matrix4::getIdentity();
-      _local2worldX.yCol3() = g_vector3_axis_x.cross(localViewpoint).getNormalised();
-      _local2worldX.zCol3() = _local2worldX.xCol3().cross(
+      _local2worldX.yCol3Ref() = g_vector3_axis_x.cross(localViewpoint).getNormalised();
+      _local2worldX.zCol3Ref() = _local2worldX.xCol3().cross(
         											_local2worldX.yCol3()).getNormalised();
 	  _local2worldX.premultiplyBy(_pivot2World._worldSpace);
     }
@@ -110,8 +110,8 @@ void RotateManipulator::updateCircleTransforms()
     if(_circleY_visible)
     {
       _local2worldY = Matrix4::getIdentity();
-      _local2worldY.zCol3() = g_vector3_axis_y.cross(localViewpoint).getNormalised();
-      _local2worldY.xCol3() = _local2worldY.yCol3().cross(
+      _local2worldY.zCol3Ref() = g_vector3_axis_y.cross(localViewpoint).getNormalised();
+      _local2worldY.xCol3Ref() = _local2worldY.yCol3().cross(
       													_local2worldY.zCol3()).getNormalised();
       _local2worldY.premultiplyBy(_pivot2World._worldSpace);
     }
@@ -120,8 +120,8 @@ void RotateManipulator::updateCircleTransforms()
     if(_circleZ_visible)
     {
       _local2worldZ = Matrix4::getIdentity();
-      _local2worldZ.xCol3() = g_vector3_axis_z.cross(localViewpoint).getNormalised();
-      _local2worldZ.yCol3() = _local2worldZ.zCol3().cross(
+      _local2worldZ.xCol3Ref() = g_vector3_axis_z.cross(localViewpoint).getNormalised();
+      _local2worldZ.yCol3Ref() = _local2worldZ.zCol3().cross(
       												_local2worldZ.xCol3()).getNormalised();
 	  _local2worldZ.premultiplyBy(_pivot2World._worldSpace);
     }

test/math/Matrix4.cpp

@@ -128,10 +128,10 @@ TEST(MatrixTest, AccessMatrixColumnVectors)
     EXPECT_EQ(m.translation(), Vector3(-5, 13, 14));
 
     // Write column values
-    m.xCol3() = Vector3(0.1, 0.2, 0.3);
-    m.yCol3() = Vector3(0.5, 0.6, 0.7);
-    m.zCol3() = Vector3(0.9, 1.0, 1.1);
-    m.tCol() = Vector4(1.3, 1.4, 1.5, 1.6);
+    m.xCol3Ref() = Vector3(0.1, 0.2, 0.3);
+    m.yCol3Ref() = Vector3(0.5, 0.6, 0.7);
+    m.zCol3Ref() = Vector3(0.9, 1.0, 1.1);
+    m.tColRef() = Vector4(1.3, 1.4, 1.5, 1.6);
     EXPECT_EQ(m, Matrix4::byColumns(0.1, 0.2, 0.3, 26,
                                     0.5, 0.6, 0.7, -100,
                                     0.9, 1.0, 1.1, 0.5,