| Message ID | 20241118150528.1856797-4-stefan.klug@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Stefan, Thank you for the patch. On Mon, Nov 18, 2024 at 04:05:06PM +0100, Stefan Klug wrote: > Use a Span for construction to be more flexible. Additionally add an > assert for the correct size of the span. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > include/libcamera/internal/matrix.h | 3 ++- > src/libcamera/matrix.cpp | 10 ++++++++++ > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h > index b82d33f98658..28ea68a55af9 100644 > --- a/include/libcamera/internal/matrix.h > +++ b/include/libcamera/internal/matrix.h > @@ -39,8 +39,9 @@ public: > std::copy(data.begin(), data.end(), data_.begin()); > } > > - Matrix(const std::vector<T> &data) > + Matrix(const Span<T> &data) A span is certainly better than a vector. > { > + ASSERT(data.size() == Rows * Cols); It would be nice to drop this constructor in favour of an std::array constructor, as indicated in my review of 2/4. I haven't checked if that would be possible. > std::copy(data.begin(), data.end(), data_.begin()); > } > > diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp > index c3ed54895b22..bb9fd3003996 100644 > --- a/src/libcamera/matrix.cpp > +++ b/src/libcamera/matrix.cpp > @@ -31,6 +31,16 @@ LOG_DEFINE_CATEGORY(Matrix) > * \brief Construct a zero matrix > */ > > +/** > + * \fn Matrix::Matrix(std::initializer_list<T> data) > + * \brief Construct a matrix from supplied data > + * \param[in] data Data from which to construct a matrix > + * > + * \a data is a one-dimensional vector and will be turned into a matrix in > + * row-major order. The size of \a data must be equal to the product of the > + * number of rows and columns of the matrix (Rows x Cols). > + */ This is unrelated to the commit message. > + > /** > * \fn Matrix::Matrix(const Span<T> &data) And this was added in 2/4. I think something went wrong at some point during a split & rebase. > * \brief Construct a matrix from supplied data
diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h index b82d33f98658..28ea68a55af9 100644 --- a/include/libcamera/internal/matrix.h +++ b/include/libcamera/internal/matrix.h @@ -39,8 +39,9 @@ public: std::copy(data.begin(), data.end(), data_.begin()); } - Matrix(const std::vector<T> &data) + Matrix(const Span<T> &data) { + ASSERT(data.size() == Rows * Cols); std::copy(data.begin(), data.end(), data_.begin()); } diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp index c3ed54895b22..bb9fd3003996 100644 --- a/src/libcamera/matrix.cpp +++ b/src/libcamera/matrix.cpp @@ -31,6 +31,16 @@ LOG_DEFINE_CATEGORY(Matrix) * \brief Construct a zero matrix */ +/** + * \fn Matrix::Matrix(std::initializer_list<T> data) + * \brief Construct a matrix from supplied data + * \param[in] data Data from which to construct a matrix + * + * \a data is a one-dimensional vector and will be turned into a matrix in + * row-major order. The size of \a data must be equal to the product of the + * number of rows and columns of the matrix (Rows x Cols). + */ + /** * \fn Matrix::Matrix(const Span<T> &data) * \brief Construct a matrix from supplied data
Use a Span for construction to be more flexible. Additionally add an assert for the correct size of the span. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- include/libcamera/internal/matrix.h | 3 ++- src/libcamera/matrix.cpp | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-)