[PATCH v2 02/17] libcamera: matrix: Make most functions constexpr

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Wed, Mar 19, 2025 at 05:11:07PM +0100, Stefan Klug wrote:
> By zero-initializing the data_ member we can make most functions
> constexpr which will come in handy in upcoming patches.  Note that this
> is due to C++17. In C++20 we will be able to leave data_ unintialized

s/unintialized/uninitialized/

Given that we only operate on small matrices I think this is acceptable.
Please add a todo comment to indicate that initialization of data should
be removed with C++20.

> for constexpr.  The Matrix(std::array) version of the constructor can
> not be constexpr because std::copy only became constexpr in C++20.
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> 
> ---
> 
> Changes in v2:
> - Added this patch
> ---
>  include/libcamera/internal/matrix.h | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h
> index 8399be583f28..2e7929e9060c 100644
> --- a/include/libcamera/internal/matrix.h
> +++ b/include/libcamera/internal/matrix.h
> @@ -25,9 +25,8 @@ class Matrix
>  	static_assert(std::is_arithmetic_v<T>, "Matrix type must be arithmetic");
>  
>  public:
> -	Matrix()
> +	constexpr Matrix()
>  	{
> -		data_.fill(static_cast<T>(0));
>  	}
>  
>  	Matrix(const std::array<T, Rows * Cols> &data)
> @@ -35,7 +34,7 @@ public:
>  		std::copy(data.begin(), data.end(), data_.begin());
>  	}
>  
> -	static Matrix identity()
> +	static constexpr Matrix identity()
>  	{
>  		Matrix ret;
>  		for (size_t i = 0; i < std::min(Rows, Cols); i++)
> @@ -63,14 +62,14 @@ public:
>  		return out.str();
>  	}
>  
> -	Span<const T, Rows * Cols> data() const { return data_; }
> +	constexpr Span<const T, Rows * Cols> data() const { return data_; }
>  
> -	Span<const T, Cols> operator[](size_t i) const
> +	constexpr Span<const T, Cols> operator[](size_t i) const
>  	{
>  		return Span<const T, Cols>{ &data_.data()[i * Cols], Cols };
>  	}
>  
> -	Span<T, Cols> operator[](size_t i)
> +	constexpr Span<T, Cols> operator[](size_t i)
>  	{
>  		return Span<T, Cols>{ &data_.data()[i * Cols], Cols };
>  	}
> @@ -85,7 +84,7 @@ public:
>  	}
>  
>  private:
> -	std::array<T, Rows * Cols> data_;
> +	std::array<T, Rows * Cols> data_{};

We usually write

	std::array<T, Rows * Cols> data_ = {};

With that and the comment,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  };
>  
>  template<typename T, typename U, unsigned int Rows, unsigned int Cols>
> @@ -130,7 +129,7 @@ constexpr Matrix<T, R1, C2> operator*(const Matrix<T, R1, C1> &m1, const Matrix<
>  }
>  
>  template<typename T, unsigned int Rows, unsigned int Cols>
> -Matrix<T, Rows, Cols> operator+(const Matrix<T, Rows, Cols> &m1, const Matrix<T, Rows, Cols> &m2)
> +constexpr Matrix<T, Rows, Cols> operator+(const Matrix<T, Rows, Cols> &m1, const Matrix<T, Rows, Cols> &m2)
>  {
>  	Matrix<T, Rows, Cols> result;
>  

-- 
Regards,

Laurent Pinchart