[v3,02/16] libcamera: matrix: Make most functions constexpr
diff mbox series

Message ID 20250403154925.382973-3-stefan.klug@ideasonboard.com
State Accepted
Headers show
Series
  • Some rkisp1 awb improvements
Related show

Commit Message

Stefan Klug April 3, 2025, 3:49 p.m. UTC 
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_ uninitialized
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@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---

Changes in v2:
- Added this patch

Changes in v3:
- Added comment on data_ initializer
---
 include/libcamera/internal/matrix.h | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

Comments

Kieran Bingham May 2, 2025, 7:41 a.m. UTC | #1 
Quoting Stefan Klug (2025-04-03 16:49:07)
> 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_ uninitialized
> 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@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> ---
> 
> Changes in v2:
> - Added this patch
> 
> Changes in v3:
> - Added comment on data_ initializer
> ---
>  include/libcamera/internal/matrix.h | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h
> index b9c3d41ef855..512c1162c3bc 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 };
>         }
> @@ -88,7 +87,12 @@ public:
>         }
>  
>  private:
> -       std::array<T, Rows * Cols> data_;
> +       /*
> +        * \todo The initializer is only necessary for the constructor to be
> +        * constexpr in C++17. Remove the initializer as soon as we are on
> +        * C++20.
> +        */
> +       std::array<T, Rows * Cols> data_ = {};
>  };
>  
>  #ifndef __DOXYGEN__
> @@ -141,7 +145,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;
>  
> -- 
> 2.43.0
>
Paul Elder May 7, 2025, 3:39 p.m. UTC | #2 
On Thu, Apr 03, 2025 at 05:49:07PM +0200, 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_ uninitialized
> 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@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> 
> ---
> 
> Changes in v2:
> - Added this patch
> 
> Changes in v3:
> - Added comment on data_ initializer
> ---
>  include/libcamera/internal/matrix.h | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h
> index b9c3d41ef855..512c1162c3bc 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 };
>  	}
> @@ -88,7 +87,12 @@ public:
>  	}
>  
>  private:
> -	std::array<T, Rows * Cols> data_;
> +	/*
> +	 * \todo The initializer is only necessary for the constructor to be
> +	 * constexpr in C++17. Remove the initializer as soon as we are on
> +	 * C++20.
> +	 */
> +	std::array<T, Rows * Cols> data_ = {};
>  };
>  
>  #ifndef __DOXYGEN__
> @@ -141,7 +145,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;
>  
> -- 
> 2.43.0
>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h
index b9c3d41ef855..512c1162c3bc 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 };
 	}
@@ -88,7 +87,12 @@  public:
 	}
 
 private:
-	std::array<T, Rows * Cols> data_;
+	/*
+	 * \todo The initializer is only necessary for the constructor to be
+	 * constexpr in C++17. Remove the initializer as soon as we are on
+	 * C++20.
+	 */
+	std::array<T, Rows * Cols> data_ = {};
 };
 
 #ifndef __DOXYGEN__
@@ -141,7 +145,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;