[v2,1/2] libcamera: matrix: Add a transpose() function

28239 diff mbox series

The Matrix class has a member function to invert a matrix, but no
function to transpose it. We already have one open-coded transpose
operation in the software ISP implementation, and more would likely be
added. Add a transpose() member function to the Matrix class to cover
this need.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:

- Add and use nonc-const data()
- Add unit test
---
 include/libcamera/internal/matrix.h | 14 ++++++++++++
 src/libcamera/matrix.cpp            | 34 ++++++++++++++++++++++++++++-
 test/matrix.cpp                     |  9 ++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)

2026. 09. 14. 9:21 keltezéssel, Laurent Pinchart írta:
> The Matrix class has a member function to invert a matrix, but no
> function to transpose it. We already have one open-coded transpose
> operation in the software ISP implementation, and more would likely be
> added. Add a transpose() member function to the Matrix class to cover
> this need.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since v1:
> 
> - Add and use nonc-const data()
> - Add unit test
> ---
>   include/libcamera/internal/matrix.h | 14 ++++++++++++
>   src/libcamera/matrix.cpp            | 34 ++++++++++++++++++++++++++++-
>   test/matrix.cpp                     |  9 ++++++++
>   3 files changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h
> index f74cda103c72..221e3b517626 100644
> --- a/include/libcamera/internal/matrix.h
> +++ b/include/libcamera/internal/matrix.h
> @@ -24,6 +24,9 @@ LOG_DECLARE_CATEGORY(Matrix)
>   template<typename T>
>   bool matrixInvert(std::span<const T> dataIn, std::span<T> dataOut, unsigned int dim,
>   		  std::span<T> scratchBuffer, std::span<unsigned int> swapBuffer);
> +template<typename T>
> +void matrixTranspose(std::span<const T> dataIn, std::span<T> dataOut,
> +		     unsigned int rows, unsigned int cols);
>   #endif /* __DOXYGEN__ */
>   
>   template<typename T, unsigned int Rows, unsigned int Cols>
> @@ -76,6 +79,8 @@ public:
>   
>   	constexpr std::span<const T, Rows * Cols> data() const { return data_; }
>   
> +	constexpr std::span<T, Rows * Cols> data() { return data_; }
> +
>   	constexpr std::span<const T, Cols> operator[](size_t i) const
>   	{
>   		return std::span<const T, Cols>{ &data_.data()[i * Cols], Cols };
> @@ -115,6 +120,15 @@ public:
>   		return inverse;
>   	}
>   
> +	Matrix<T, Cols, Rows> transpose() const
> +	{
> +		Matrix<T, Cols, Rows> transposed;
> +		matrixTranspose(std::span<const T>(data_),
> +				std::span<T>(transposed.data()),
> +				Rows, Cols);
> +		return transposed;
> +	}

I also made an attempt after my reply, but in the end decided not to send it.
I had:

	template<typename U = T>
	[[nodiscard]]
	constexpr Matrix<U, Cols, Rows> transpose() const
	{
		static_assert(std::is_convertible_v<T, U>);

which had a separate template parameter for doing a conversion during the transposition.
That seemed somewhat useful at essentially no extra cost to the operation or template
complexity itself.

But in any case, the only thing I'm not so sure about here is that the operation is
not part of the matrix template. Considering the number of users, the existing functions
of the matrix and vector types, and the usual matrix sizes (<= 3x3), I think I would just
have it inline.


> +
>   private:
>   	/*
>   	 * \todo The initializer is only necessary for the constructor to be
> diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp
> index 9cb0885b3b54..e600a598c057 100644
> --- a/src/libcamera/matrix.cpp
> +++ b/src/libcamera/matrix.cpp
> @@ -69,7 +69,7 @@ LOG_DEFINE_CATEGORY(Matrix)
>    */
>   
>   /**
> - * \fn Matrix::data()
> + * \fn Matrix::data() const
>    * \brief Access the matrix data as a linear array
>    *
>    * Access the contents of the matrix as a one-dimensional linear array of
> @@ -79,6 +79,11 @@ LOG_DEFINE_CATEGORY(Matrix)
>    * \return A span referencing the matrix data as a linear array
>    */
>   
> +/**
> + * \fn Matrix::data()
> + * \copydoc Matrix::data() const
> + */
> +
>   /**
>    * \fn std::span<const T, Cols> Matrix::operator[](size_t i) const
>    * \brief Index to a row in the matrix
> @@ -107,6 +112,16 @@ LOG_DEFINE_CATEGORY(Matrix)
>    * \return The inverse of the matrix
>    */
>   
> +/**
> + * \fn Matrix::transpose() const
> + * \brief Compute the transpose of the matrix
> + *
> + * This function computes the transpose of the matrix. It is only implemented
> + * for matrices of float and double types.
> + *
> + * \return The transpose of the matrix
> + */
> +
>   /**
>    * \fn Matrix::operator[](size_t i)
>    * \copydoc Matrix::operator[](size_t i) const
> @@ -309,6 +324,23 @@ template bool matrixInvert<double>(std::span<const double> data, std::span<doubl
>   				   unsigned int dim, std::span<double> scratchBuffer,
>   				   std::span<unsigned int> swapBuffer);
>   
> +template<typename T>
> +void matrixTranspose(std::span<const T> dataIn, std::span<T> dataOut,
> +		     unsigned int rows, unsigned int cols)
> +{
> +	for (unsigned int row = 0; row < rows; ++row) {
> +		for (unsigned int col = 0; col < cols; ++col)
> +			dataOut[col * rows + row] = dataIn[row * cols + col];
> +	}
> +}
> +
> +template void matrixTranspose(std::span<const float> dataIn,
> +			      std::span<float> dataOut,
> +			      unsigned int rows, unsigned int cols);
> +template void matrixTranspose(std::span<const double> dataIn,
> +			      std::span<double> dataOut,
> +			      unsigned int rows, unsigned int cols);
> +
>   /*
>    * The value node shall be a list of numerical values. Its size shall be equal
>    * to the product of the number of rows and columns of the matrix (Rows x
> diff --git a/test/matrix.cpp b/test/matrix.cpp
> index 4afae2da7866..4504f6c96185 100644
> --- a/test/matrix.cpp
> +++ b/test/matrix.cpp
> @@ -46,6 +46,15 @@ protected:
>   		ASSERT_EQ(m5[1][0], 0.0);
>   		ASSERT_EQ(m5[1][1], 1.0);
>   
> +		Matrix<float, 2, 3> m6({ 1.0, 2.0, 3.0, 4.0, 5.0, 6.0 });
> +		Matrix<float, 3, 2> m7 = m6.transpose();

Given https://gitlab.freedesktop.org/camera/libcamera/-/work_items/348
I think it would be best to avoid floats in tests where not really required.
Especially since there is also a float-double mismatch here.


> +		ASSERT_EQ(m7[0][0], 1.0);
> +		ASSERT_EQ(m7[0][1], 4.0);
> +		ASSERT_EQ(m7[1][0], 2.0);
> +		ASSERT_EQ(m7[1][1], 5.0);
> +		ASSERT_EQ(m7[2][0], 3.0);
> +		ASSERT_EQ(m7[2][1], 6.0);
> +
>   		return TestPass;
>   	}
>   };

On Mon, Sep 14, 2026 at 09:32:33AM +0200, Barnabás Pőcze wrote:
> 2026. 09. 14. 9:21 keltezéssel, Laurent Pinchart írta:
> > The Matrix class has a member function to invert a matrix, but no
> > function to transpose it. We already have one open-coded transpose
> > operation in the software ISP implementation, and more would likely be
> > added. Add a transpose() member function to the Matrix class to cover
> > this need.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v1:
> > 
> > - Add and use nonc-const data()
> > - Add unit test
> > ---
> >   include/libcamera/internal/matrix.h | 14 ++++++++++++
> >   src/libcamera/matrix.cpp            | 34 ++++++++++++++++++++++++++++-
> >   test/matrix.cpp                     |  9 ++++++++
> >   3 files changed, 56 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h
> > index f74cda103c72..221e3b517626 100644
> > --- a/include/libcamera/internal/matrix.h
> > +++ b/include/libcamera/internal/matrix.h
> > @@ -24,6 +24,9 @@ LOG_DECLARE_CATEGORY(Matrix)
> >   template<typename T>
> >   bool matrixInvert(std::span<const T> dataIn, std::span<T> dataOut, unsigned int dim,
> >   		  std::span<T> scratchBuffer, std::span<unsigned int> swapBuffer);
> > +template<typename T>
> > +void matrixTranspose(std::span<const T> dataIn, std::span<T> dataOut,
> > +		     unsigned int rows, unsigned int cols);
> >   #endif /* __DOXYGEN__ */
> >   
> >   template<typename T, unsigned int Rows, unsigned int Cols>
> > @@ -76,6 +79,8 @@ public:
> >   
> >   	constexpr std::span<const T, Rows * Cols> data() const { return data_; }
> >   
> > +	constexpr std::span<T, Rows * Cols> data() { return data_; }
> > +
> >   	constexpr std::span<const T, Cols> operator[](size_t i) const
> >   	{
> >   		return std::span<const T, Cols>{ &data_.data()[i * Cols], Cols };
> > @@ -115,6 +120,15 @@ public:
> >   		return inverse;
> >   	}
> >   
> > +	Matrix<T, Cols, Rows> transpose() const
> > +	{
> > +		Matrix<T, Cols, Rows> transposed;
> > +		matrixTranspose(std::span<const T>(data_),
> > +				std::span<T>(transposed.data()),
> > +				Rows, Cols);
> > +		return transposed;
> > +	}
> 
> I also made an attempt after my reply, but in the end decided not to send it.
> I had:
> 
> 	template<typename U = T>
> 	[[nodiscard]]
> 	constexpr Matrix<U, Cols, Rows> transpose() const
> 	{
> 		static_assert(std::is_convertible_v<T, U>);
> 
> which had a separate template parameter for doing a conversion during the transposition.
> That seemed somewhat useful at essentially no extra cost to the operation or template
> complexity itself.
> 
> But in any case, the only thing I'm not so sure about here is that the operation is
> not part of the matrix template. Considering the number of users, the existing functions
> of the matrix and vector types, and the usual matrix sizes (<= 3x3), I think I would just
> have it inline.

I've considered both options and didn't have a strong preference, so
I'll switch to inline.

> > +
> >   private:
> >   	/*
> >   	 * \todo The initializer is only necessary for the constructor to be
> > diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp
> > index 9cb0885b3b54..e600a598c057 100644
> > --- a/src/libcamera/matrix.cpp
> > +++ b/src/libcamera/matrix.cpp
> > @@ -69,7 +69,7 @@ LOG_DEFINE_CATEGORY(Matrix)
> >    */
> >   
> >   /**
> > - * \fn Matrix::data()
> > + * \fn Matrix::data() const
> >    * \brief Access the matrix data as a linear array
> >    *
> >    * Access the contents of the matrix as a one-dimensional linear array of
> > @@ -79,6 +79,11 @@ LOG_DEFINE_CATEGORY(Matrix)
> >    * \return A span referencing the matrix data as a linear array
> >    */
> >   
> > +/**
> > + * \fn Matrix::data()
> > + * \copydoc Matrix::data() const
> > + */
> > +
> >   /**
> >    * \fn std::span<const T, Cols> Matrix::operator[](size_t i) const
> >    * \brief Index to a row in the matrix
> > @@ -107,6 +112,16 @@ LOG_DEFINE_CATEGORY(Matrix)
> >    * \return The inverse of the matrix
> >    */
> >   
> > +/**
> > + * \fn Matrix::transpose() const
> > + * \brief Compute the transpose of the matrix
> > + *
> > + * This function computes the transpose of the matrix. It is only implemented
> > + * for matrices of float and double types.
> > + *
> > + * \return The transpose of the matrix
> > + */
> > +
> >   /**
> >    * \fn Matrix::operator[](size_t i)
> >    * \copydoc Matrix::operator[](size_t i) const
> > @@ -309,6 +324,23 @@ template bool matrixInvert<double>(std::span<const double> data, std::span<doubl
> >   				   unsigned int dim, std::span<double> scratchBuffer,
> >   				   std::span<unsigned int> swapBuffer);
> >   
> > +template<typename T>
> > +void matrixTranspose(std::span<const T> dataIn, std::span<T> dataOut,
> > +		     unsigned int rows, unsigned int cols)
> > +{
> > +	for (unsigned int row = 0; row < rows; ++row) {
> > +		for (unsigned int col = 0; col < cols; ++col)
> > +			dataOut[col * rows + row] = dataIn[row * cols + col];
> > +	}
> > +}
> > +
> > +template void matrixTranspose(std::span<const float> dataIn,
> > +			      std::span<float> dataOut,
> > +			      unsigned int rows, unsigned int cols);
> > +template void matrixTranspose(std::span<const double> dataIn,
> > +			      std::span<double> dataOut,
> > +			      unsigned int rows, unsigned int cols);
> > +
> >   /*
> >    * The value node shall be a list of numerical values. Its size shall be equal
> >    * to the product of the number of rows and columns of the matrix (Rows x
> > diff --git a/test/matrix.cpp b/test/matrix.cpp
> > index 4afae2da7866..4504f6c96185 100644
> > --- a/test/matrix.cpp
> > +++ b/test/matrix.cpp
> > @@ -46,6 +46,15 @@ protected:
> >   		ASSERT_EQ(m5[1][0], 0.0);
> >   		ASSERT_EQ(m5[1][1], 1.0);
> >   
> > +		Matrix<float, 2, 3> m6({ 1.0, 2.0, 3.0, 4.0, 5.0, 6.0 });
> > +		Matrix<float, 3, 2> m7 = m6.transpose();
> 
> Given https://gitlab.freedesktop.org/camera/libcamera/-/work_items/348
> I think it would be best to avoid floats in tests where not really required.
> Especially since there is also a float-double mismatch here.

The rest of the unit test uses floats, but I agree we can convert to
double. Or did you mean this should use int ?

> > +		ASSERT_EQ(m7[0][0], 1.0);
> > +		ASSERT_EQ(m7[0][1], 4.0);
> > +		ASSERT_EQ(m7[1][0], 2.0);
> > +		ASSERT_EQ(m7[1][1], 5.0);
> > +		ASSERT_EQ(m7[2][0], 3.0);
> > +		ASSERT_EQ(m7[2][1], 6.0);
> > +
> >   		return TestPass;
> >   	}
> >   };