[v3,4/9] libcamera: internal: matrix: Replace vector with array in constructor
diff mbox series

Message ID 20241120142807.2093301-5-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • Move Matrix class from libipa to libcamera
Related show

Commit Message

Stefan Klug Nov. 20, 2024, 2:27 p.m. UTC 
The Matrix constructor that takes a std::vector is meant and only used
to initialize a Matrix from an initializer list. Using a std::vector is
problematic for two reasons. First, it requires constructing a vector,
copying the data from the initializer list, which is an expensive
operation. Then, the vector size can't be verified at compile time,
making the constructor unsafe.

The first issue could be solved by replacing the vector with a
std::initializer_list or a Span. The second issue would require checking
the initializer list size with a static assertion, or restricting usage
of the constructor to fixed-extent spans. Unfortunately, even if the
size of initializer lists is always known at compile time, the
std::initializer_list::size() function is a compile-time constant only
for constant initializer lists. Using a span would work better, but
construction of a fixed extent span from an initializer list must be
explicit, making the API cumbersome.

We can solve all those issues by passing an std::array to the
constructor. Construction of an array from an initializer list can be
implicit and doesn't involve a copy, and the array size is a template
parameter and therefore guaranteed to be a compile-time constant.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---
Changes in v3:
- Collect tags
- Update commit msg
---
 include/libcamera/internal/matrix.h | 2 +-
 src/libcamera/matrix.cpp            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Patch
diff mbox series 

diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h
index 3701d0ee980b..7a71028c473a 100644
--- a/include/libcamera/internal/matrix.h
+++ b/include/libcamera/internal/matrix.h
@@ -33,7 +33,7 @@  public:
 		data_.fill(static_cast<T>(0));
 	}
 
-	Matrix(const std::vector<T> &data)
+	Matrix(const std::array<T, Rows * Cols> &data)
 	{
 		std::copy(data.begin(), data.end(), data_.begin());
 	}
diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp
index 55359aa206ee..4d95a19bfbb9 100644
--- a/src/libcamera/matrix.cpp
+++ b/src/libcamera/matrix.cpp
@@ -32,7 +32,7 @@  LOG_DEFINE_CATEGORY(Matrix)
  */
 
 /**
- * \fn Matrix::Matrix(const std::vector<T> &data)
+ * \fn Matrix::Matrix(const std::array<T, Rows * Cols> &data)
  * \brief Construct a matrix from supplied data
  * \param[in] data Data from which to construct a matrix
  *