| Message ID | 20250217100203.297894-2-stefan.klug@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Stefan, Thank you for the patch. On Mon, Feb 17, 2025 at 11:01:42AM +0100, Stefan Klug wrote: > The libcamera code base uses matrices and vectors of type float and > double. Add a cast function to easily convert between different > underlying types. Should we standardize on one of the floating point precisions to avoid casts ? > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > include/libcamera/internal/matrix.h | 11 +++++++++++ > src/libcamera/matrix.cpp | 10 ++++++++++ > 2 files changed, 21 insertions(+) > > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h > index a055e6926c94..ca81dcda93c4 100644 > --- a/include/libcamera/internal/matrix.h > +++ b/include/libcamera/internal/matrix.h > @@ -90,6 +90,17 @@ public: > return *this; > } > > + template<typename T2> > + Matrix<T2, Rows, Cols> cast() const I wonder if we should use a "copy" constructor instead of a cast() function, to make it clear a copy iw made. > + { > + Matrix<T2, Rows, Cols> ret; > + for (unsigned int i = 0; i < Rows; i++) > + for (unsigned int j = 0; j < Cols; j++) > + ret[i][j] = static_cast<T2>((*this)[i][j]); You can operator on data_ directly, with a single loop. > + > + return ret; > + } > + > private: > std::array<T, Rows * Cols> data_; > }; > diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp > index e7e027225666..91a3f16405a3 100644 > --- a/src/libcamera/matrix.cpp > +++ b/src/libcamera/matrix.cpp > @@ -77,6 +77,16 @@ LOG_DEFINE_CATEGORY(Matrix) > * \return Row \a i from the matrix, as a Span > */ > > +/** > + * \fn template<typename T2> Matrix<T2, Rows, Cols> Matrix::cast() const > + * \brief Cast the matrix to a different type > + * > + * This function returns a new matrix with the same size and values but a > + * different type. > + * > + * \return The new matrix > + */ > + > /** > * \fn Matrix::operator[](size_t i) > * \copydoc Matrix::operator[](size_t i) const
Hi Laurent, Thank you for the review. On Mon, Feb 17, 2025 at 01:42:52PM +0200, Laurent Pinchart wrote: > Hi Stefan, > > Thank you for the patch. > > On Mon, Feb 17, 2025 at 11:01:42AM +0100, Stefan Klug wrote: > > The libcamera code base uses matrices and vectors of type float and > > double. Add a cast function to easily convert between different > > underlying types. > > Should we standardize on one of the floating point precisions to avoid > casts ? I thought about that before. But grepping through the source, I don't really see that. All controls are in float (which is fine). For calculations I tend to go for double. These might work in float also, but I don't think it's worth forcing the whole project. > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > include/libcamera/internal/matrix.h | 11 +++++++++++ > > src/libcamera/matrix.cpp | 10 ++++++++++ > > 2 files changed, 21 insertions(+) > > > > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h > > index a055e6926c94..ca81dcda93c4 100644 > > --- a/include/libcamera/internal/matrix.h > > +++ b/include/libcamera/internal/matrix.h > > @@ -90,6 +90,17 @@ public: > > return *this; > > } > > > > + template<typename T2> > > + Matrix<T2, Rows, Cols> cast() const > > I wonder if we should use a "copy" constructor instead of a cast() > function, to make it clear a copy iw made. I tried that before. I guess it's a matter of taste and in the end I'm fine with either way. The cast function was just easier to type and you don't have to repeat the dimensions. e.g.: v = Matrix<double, 3, 3>(m) * a; vs v = m.cast<double>() * a; > > > + { > > + Matrix<T2, Rows, Cols> ret; > > + for (unsigned int i = 0; i < Rows; i++) > > + for (unsigned int j = 0; j < Cols; j++) > > + ret[i][j] = static_cast<T2>((*this)[i][j]); > > You can operator on data_ directly, with a single loop. In the "copy" constructor case, yes. Not in this case because ret.data_ is private from within this class. Best regards, Stefan > > > + > > + return ret; > > + } > > + > > private: > > std::array<T, Rows * Cols> data_; > > }; > > diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp > > index e7e027225666..91a3f16405a3 100644 > > --- a/src/libcamera/matrix.cpp > > +++ b/src/libcamera/matrix.cpp > > @@ -77,6 +77,16 @@ LOG_DEFINE_CATEGORY(Matrix) > > * \return Row \a i from the matrix, as a Span > > */ > > > > +/** > > + * \fn template<typename T2> Matrix<T2, Rows, Cols> Matrix::cast() const > > + * \brief Cast the matrix to a different type > > + * > > + * This function returns a new matrix with the same size and values but a > > + * different type. > > + * > > + * \return The new matrix > > + */ > > + > > /** > > * \fn Matrix::operator[](size_t i) > > * \copydoc Matrix::operator[](size_t i) const > > -- > Regards, > > Laurent Pinchart
Hi 2025. február 17., hétfő 16:02 keltezéssel, Stefan Klug <stefan.klug@ideasonboard.com> írta: > Hi Laurent, > > Thank you for the review. > > On Mon, Feb 17, 2025 at 01:42:52PM +0200, Laurent Pinchart wrote: > > Hi Stefan, > > > > Thank you for the patch. > > > > On Mon, Feb 17, 2025 at 11:01:42AM +0100, Stefan Klug wrote: > > > The libcamera code base uses matrices and vectors of type float and > > > double. Add a cast function to easily convert between different > > > underlying types. > > > > Should we standardize on one of the floating point precisions to avoid > > casts ? > > I thought about that before. But grepping through the source, I don't > really see that. All controls are in float (which is fine). For > calculations I tend to go for double. These might work in float also, > but I don't think it's worth forcing the whole project. > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > --- > > > include/libcamera/internal/matrix.h | 11 +++++++++++ > > > src/libcamera/matrix.cpp | 10 ++++++++++ > > > 2 files changed, 21 insertions(+) > > > > > > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h > > > index a055e6926c94..ca81dcda93c4 100644 > > > --- a/include/libcamera/internal/matrix.h > > > +++ b/include/libcamera/internal/matrix.h > > > @@ -90,6 +90,17 @@ public: > > > return *this; > > > } > > > > > > + template<typename T2> > > > + Matrix<T2, Rows, Cols> cast() const > > > > I wonder if we should use a "copy" constructor instead of a cast() > > function, to make it clear a copy iw made. > > I tried that before. I guess it's a matter of taste and in the end I'm > fine with either way. The cast function was just easier to type and you > don't have to repeat the dimensions. > > e.g.: > > v = Matrix<double, 3, 3>(m) * a; > > vs > > v = m.cast<double>() * a; > > > > > > + { > > > + Matrix<T2, Rows, Cols> ret; > > > + for (unsigned int i = 0; i < Rows; i++) > > > + for (unsigned int j = 0; j < Cols; j++) > > > + ret[i][j] = static_cast<T2>((*this)[i][j]); > > > > You can operator on data_ directly, with a single loop. > > In the "copy" constructor case, yes. Not in this case because ret.data_ is > private from within this class. But this is a non-static member function, so it has access to all members of the class. The visibility rules apply to the type, not to the instance, e.g. even a static member function `T::f()` can access arbitrary members of an object of type T. Regards, Barnabás Pőcze > > Best regards, > Stefan > > > > > > + > > > + return ret; > > > + } > > > + > > > private: > > > std::array<T, Rows * Cols> data_; > > > }; > > > diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp > > > index e7e027225666..91a3f16405a3 100644 > > > --- a/src/libcamera/matrix.cpp > > > +++ b/src/libcamera/matrix.cpp > > > @@ -77,6 +77,16 @@ LOG_DEFINE_CATEGORY(Matrix) > > > * \return Row \a i from the matrix, as a Span > > > */ > > > > > > +/** > > > + * \fn template<typename T2> Matrix<T2, Rows, Cols> Matrix::cast() const > > > + * \brief Cast the matrix to a different type > > > + * > > > + * This function returns a new matrix with the same size and values but a > > > + * different type. > > > + * > > > + * \return The new matrix > > > + */ > > > + > > > /** > > > * \fn Matrix::operator[](size_t i) > > > * \copydoc Matrix::operator[](size_t i) const > > > > -- > > Regards, > > > > Laurent Pinchart
Hi Barnabás, On Mon, Feb 17, 2025 at 03:13:47PM +0000, Barnabás Pőcze wrote: > Hi > > > 2025. február 17., hétfő 16:02 keltezéssel, Stefan Klug <stefan.klug@ideasonboard.com> írta: > > > Hi Laurent, > > > > Thank you for the review. > > > > On Mon, Feb 17, 2025 at 01:42:52PM +0200, Laurent Pinchart wrote: > > > Hi Stefan, > > > > > > Thank you for the patch. > > > > > > On Mon, Feb 17, 2025 at 11:01:42AM +0100, Stefan Klug wrote: > > > > The libcamera code base uses matrices and vectors of type float and > > > > double. Add a cast function to easily convert between different > > > > underlying types. > > > > > > Should we standardize on one of the floating point precisions to avoid > > > casts ? > > > > I thought about that before. But grepping through the source, I don't > > really see that. All controls are in float (which is fine). For > > calculations I tend to go for double. These might work in float also, > > but I don't think it's worth forcing the whole project. > > > > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > > --- > > > > include/libcamera/internal/matrix.h | 11 +++++++++++ > > > > src/libcamera/matrix.cpp | 10 ++++++++++ > > > > 2 files changed, 21 insertions(+) > > > > > > > > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h > > > > index a055e6926c94..ca81dcda93c4 100644 > > > > --- a/include/libcamera/internal/matrix.h > > > > +++ b/include/libcamera/internal/matrix.h > > > > @@ -90,6 +90,17 @@ public: > > > > return *this; > > > > } > > > > > > > > + template<typename T2> > > > > + Matrix<T2, Rows, Cols> cast() const > > > > > > I wonder if we should use a "copy" constructor instead of a cast() > > > function, to make it clear a copy iw made. > > > > I tried that before. I guess it's a matter of taste and in the end I'm > > fine with either way. The cast function was just easier to type and you > > don't have to repeat the dimensions. > > > > e.g.: > > > > v = Matrix<double, 3, 3>(m) * a; > > > > vs > > > > v = m.cast<double>() * a; > > > > > > > > > + { > > > > + Matrix<T2, Rows, Cols> ret; > > > > + for (unsigned int i = 0; i < Rows; i++) > > > > + for (unsigned int j = 0; j < Cols; j++) > > > > + ret[i][j] = static_cast<T2>((*this)[i][j]); > > > > > > You can operator on data_ directly, with a single loop. > > > > In the "copy" constructor case, yes. Not in this case because ret.data_ is > > private from within this class. > > But this is a non-static member function, so it has access to all members of the class. > The visibility rules apply to the type, not to the instance, e.g. even a static > member function `T::f()` can access arbitrary members of an object of type T. Yes, but in this case T::cast() would try to access T2::data_. The class types are different and so the visibility rule applies. I tried with a friend declaration but got nowhere (I'm happy to learn how that would look like) and settled for the 2D loop to get it done :-) Best regards, Stefan > > > Regards, > Barnabás Pőcze > > > > > > Best regards, > > Stefan > > > > > > > > > + > > > > + return ret; > > > > + } > > > > + > > > > private: > > > > std::array<T, Rows * Cols> data_; > > > > }; > > > > diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp > > > > index e7e027225666..91a3f16405a3 100644 > > > > --- a/src/libcamera/matrix.cpp > > > > +++ b/src/libcamera/matrix.cpp > > > > @@ -77,6 +77,16 @@ LOG_DEFINE_CATEGORY(Matrix) > > > > * \return Row \a i from the matrix, as a Span > > > > */ > > > > > > > > +/** > > > > + * \fn template<typename T2> Matrix<T2, Rows, Cols> Matrix::cast() const > > > > + * \brief Cast the matrix to a different type > > > > + * > > > > + * This function returns a new matrix with the same size and values but a > > > > + * different type. > > > > + * > > > > + * \return The new matrix > > > > + */ > > > > + > > > > /** > > > > * \fn Matrix::operator[](size_t i) > > > > * \copydoc Matrix::operator[](size_t i) const > > > > > > -- > > > Regards, > > > > > > Laurent Pinchart
On Mon, Feb 17, 2025 at 04:31:26PM +0100, Stefan Klug wrote: > Hi Barnabás, > > On Mon, Feb 17, 2025 at 03:13:47PM +0000, Barnabás Pőcze wrote: > > Hi > > > > > > 2025. február 17., hétfő 16:02 keltezéssel, Stefan Klug <stefan.klug@ideasonboard.com> írta: > > > > > Hi Laurent, > > > > > > Thank you for the review. > > > > > > On Mon, Feb 17, 2025 at 01:42:52PM +0200, Laurent Pinchart wrote: > > > > Hi Stefan, > > > > > > > > Thank you for the patch. > > > > > > > > On Mon, Feb 17, 2025 at 11:01:42AM +0100, Stefan Klug wrote: > > > > > The libcamera code base uses matrices and vectors of type float and > > > > > double. Add a cast function to easily convert between different > > > > > underlying types. > > > > > > > > Should we standardize on one of the floating point precisions to avoid > > > > casts ? > > > > > > I thought about that before. But grepping through the source, I don't > > > really see that. All controls are in float (which is fine). For > > > calculations I tend to go for double. These might work in float also, > > > but I don't think it's worth forcing the whole project. > > > > > > > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > > > --- > > > > > include/libcamera/internal/matrix.h | 11 +++++++++++ > > > > > src/libcamera/matrix.cpp | 10 ++++++++++ > > > > > 2 files changed, 21 insertions(+) > > > > > > > > > > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h > > > > > index a055e6926c94..ca81dcda93c4 100644 > > > > > --- a/include/libcamera/internal/matrix.h > > > > > +++ b/include/libcamera/internal/matrix.h > > > > > @@ -90,6 +90,17 @@ public: > > > > > return *this; > > > > > } > > > > > > > > > > + template<typename T2> > > > > > + Matrix<T2, Rows, Cols> cast() const > > > > > > > > I wonder if we should use a "copy" constructor instead of a cast() > > > > function, to make it clear a copy iw made. > > > > > > I tried that before. I guess it's a matter of taste and in the end I'm > > > fine with either way. The cast function was just easier to type and you > > > don't have to repeat the dimensions. > > > > > > e.g.: > > > > > > v = Matrix<double, 3, 3>(m) * a; > > > > > > vs > > > > > > v = m.cast<double>() * a; > > > > > > > > > > > > + { > > > > > + Matrix<T2, Rows, Cols> ret; > > > > > + for (unsigned int i = 0; i < Rows; i++) > > > > > + for (unsigned int j = 0; j < Cols; j++) > > > > > + ret[i][j] = static_cast<T2>((*this)[i][j]); > > > > > > > > You can operator on data_ directly, with a single loop. > > > > > > In the "copy" constructor case, yes. Not in this case because ret.data_ is > > > private from within this class. > > > > But this is a non-static member function, so it has access to all members of the class. > > The visibility rules apply to the type, not to the instance, e.g. even a static > > member function `T::f()` can access arbitrary members of an object of type T. > > Yes, but in this case T::cast() would try to access T2::data_. The class > types are different and so the visibility rule applies. I tried with a friend > declaration but got nowhere (I'm happy to learn how that would look > like) and settled for the 2D loop to get it done :-) > > Best regards, > Stefan Maybe we should add a non-const data() function... > > > > > > > Regards, > > Barnabás Pőcze > > > > > > > > > > Best regards, > > > Stefan > > > > > > > > > > > > + > > > > > + return ret; > > > > > + } > > > > > + > > > > > private: > > > > > std::array<T, Rows * Cols> data_; > > > > > }; > > > > > diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp > > > > > index e7e027225666..91a3f16405a3 100644 > > > > > --- a/src/libcamera/matrix.cpp > > > > > +++ b/src/libcamera/matrix.cpp > > > > > @@ -77,6 +77,16 @@ LOG_DEFINE_CATEGORY(Matrix) > > > > > * \return Row \a i from the matrix, as a Span > > > > > */ > > > > > > > > > > +/** > > > > > + * \fn template<typename T2> Matrix<T2, Rows, Cols> Matrix::cast() const > > > > > + * \brief Cast the matrix to a different type > > > > > + * > > > > > + * This function returns a new matrix with the same size and values but a > > > > > + * different type. > > > > > + * > > > > > + * \return The new matrix > > > > > + */ > > > > > + > > > > > /** > > > > > * \fn Matrix::operator[](size_t i) > > > > > * \copydoc Matrix::operator[](size_t i) const > > > > > > > > -- > > > > Regards, > > > > > > > > Laurent Pinchart
Hi 2025. február 17., hétfő 16:33 keltezéssel, Stefan Klug <stefan.klug@ideasonboard.com> írta: > On Mon, Feb 17, 2025 at 04:31:26PM +0100, Stefan Klug wrote: > > Hi Barnabás, > > > > On Mon, Feb 17, 2025 at 03:13:47PM +0000, Barnabás Pőcze wrote: > > > Hi > > > > > > > > > 2025. február 17., hétfő 16:02 keltezéssel, Stefan Klug <stefan.klug@ideasonboard.com> írta: > > > > > > > Hi Laurent, > > > > > > > > Thank you for the review. > > > > > > > > On Mon, Feb 17, 2025 at 01:42:52PM +0200, Laurent Pinchart wrote: > > > > > Hi Stefan, > > > > > > > > > > Thank you for the patch. > > > > > > > > > > On Mon, Feb 17, 2025 at 11:01:42AM +0100, Stefan Klug wrote: > > > > > > The libcamera code base uses matrices and vectors of type float and > > > > > > double. Add a cast function to easily convert between different > > > > > > underlying types. > > > > > > > > > > Should we standardize on one of the floating point precisions to avoid > > > > > casts ? > > > > > > > > I thought about that before. But grepping through the source, I don't > > > > really see that. All controls are in float (which is fine). For > > > > calculations I tend to go for double. These might work in float also, > > > > but I don't think it's worth forcing the whole project. > > > > > > > > > > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > > > > --- > > > > > > include/libcamera/internal/matrix.h | 11 +++++++++++ > > > > > > src/libcamera/matrix.cpp | 10 ++++++++++ > > > > > > 2 files changed, 21 insertions(+) > > > > > > > > > > > > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h > > > > > > index a055e6926c94..ca81dcda93c4 100644 > > > > > > --- a/include/libcamera/internal/matrix.h > > > > > > +++ b/include/libcamera/internal/matrix.h > > > > > > @@ -90,6 +90,17 @@ public: > > > > > > return *this; > > > > > > } > > > > > > > > > > > > + template<typename T2> > > > > > > + Matrix<T2, Rows, Cols> cast() const > > > > > > > > > > I wonder if we should use a "copy" constructor instead of a cast() > > > > > function, to make it clear a copy iw made. > > > > > > > > I tried that before. I guess it's a matter of taste and in the end I'm > > > > fine with either way. The cast function was just easier to type and you > > > > don't have to repeat the dimensions. > > > > > > > > e.g.: > > > > > > > > v = Matrix<double, 3, 3>(m) * a; > > > > > > > > vs > > > > > > > > v = m.cast<double>() * a; > > > > > > > > > > > > > > > + { > > > > > > + Matrix<T2, Rows, Cols> ret; > > > > > > + for (unsigned int i = 0; i < Rows; i++) > > > > > > + for (unsigned int j = 0; j < Cols; j++) > > > > > > + ret[i][j] = static_cast<T2>((*this)[i][j]); > > > > > > > > > > You can operator on data_ directly, with a single loop. > > > > > > > > In the "copy" constructor case, yes. Not in this case because ret.data_ is > > > > private from within this class. > > > > > > But this is a non-static member function, so it has access to all members of the class. > > > The visibility rules apply to the type, not to the instance, e.g. even a static > > > member function `T::f()` can access arbitrary members of an object of type T. > > > > Yes, but in this case T::cast() would try to access T2::data_. The class > > types are different and so the visibility rule applies. I tried with a friend > > declaration but got nowhere (I'm happy to learn how that would look > > like) and settled for the 2D loop to get it done :-) Ahh, sorry, you're right, the types are indeed different... should've checked twice. A friend declaration could look like this: template<typename, unsigned int, unsigned int> friend class Matrix; I'm not sure if it is possible to make it more restrictive (i.e. require same column/row count). > > > > Best regards, > > Stefan > > Maybe we should add a non-const data() function... Since there is already an operator[] that provides mutable view, I think that makes sense. Regards, Barnabás Pőcze > > > > > > > > > > > > Regards, > > > Barnabás Pőcze > > > > > > > > > > > > > > Best regards, > > > > Stefan > > > > > > > > > > > > > > > + > > > > > > + return ret; > > > > > > + } > > > > > > + > > > > > > private: > > > > > > std::array<T, Rows * Cols> data_; > > > > > > }; > > > > > > diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp > > > > > > index e7e027225666..91a3f16405a3 100644 > > > > > > --- a/src/libcamera/matrix.cpp > > > > > > +++ b/src/libcamera/matrix.cpp > > > > > > @@ -77,6 +77,16 @@ LOG_DEFINE_CATEGORY(Matrix) > > > > > > * \return Row \a i from the matrix, as a Span > > > > > > */ > > > > > > > > > > > > +/** > > > > > > + * \fn template<typename T2> Matrix<T2, Rows, Cols> Matrix::cast() const > > > > > > + * \brief Cast the matrix to a different type > > > > > > + * > > > > > > + * This function returns a new matrix with the same size and values but a > > > > > > + * different type. > > > > > > + * > > > > > > + * \return The new matrix > > > > > > + */ > > > > > > + > > > > > > /** > > > > > > * \fn Matrix::operator[](size_t i) > > > > > > * \copydoc Matrix::operator[](size_t i) const > > > > > > > > > > -- > > > > > Regards, > > > > > > > > > > Laurent Pinchart
On Mon, Feb 17, 2025 at 04:02:55PM +0100, Stefan Klug wrote: > On Mon, Feb 17, 2025 at 01:42:52PM +0200, Laurent Pinchart wrote: > > On Mon, Feb 17, 2025 at 11:01:42AM +0100, Stefan Klug wrote: > > > The libcamera code base uses matrices and vectors of type float and > > > double. Add a cast function to easily convert between different > > > underlying types. > > > > Should we standardize on one of the floating point precisions to avoid > > casts ? > > I thought about that before. But grepping through the source, I don't > really see that. All controls are in float (which is fine). For > calculations I tend to go for double. These might work in float also, > but I don't think it's worth forcing the whole project. > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > --- > > > include/libcamera/internal/matrix.h | 11 +++++++++++ > > > src/libcamera/matrix.cpp | 10 ++++++++++ > > > 2 files changed, 21 insertions(+) > > > > > > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h > > > index a055e6926c94..ca81dcda93c4 100644 > > > --- a/include/libcamera/internal/matrix.h > > > +++ b/include/libcamera/internal/matrix.h > > > @@ -90,6 +90,17 @@ public: > > > return *this; > > > } > > > > > > + template<typename T2> > > > + Matrix<T2, Rows, Cols> cast() const > > > > I wonder if we should use a "copy" constructor instead of a cast() > > function, to make it clear a copy iw made. > > I tried that before. I guess it's a matter of taste and in the end I'm > fine with either way. The cast function was just easier to type and you > don't have to repeat the dimensions. > > e.g.: > > v = Matrix<double, 3, 3>(m) * a; > > vs > > v = m.cast<double>() * a; The thing that bothers me a bit with cast() is that the name implies it's just a cast, while a new object is constructed, and data copied. I do agree the syntax is a bit nicer though. This being said, static_cast<> can also end up constructing a new instance, and copying data, so maybe using cast() would be fine. Unless there's a better name (and I can't think of one at the moment) I'm OK with a cast() function. We should really minimize its usage though, and try to standardize on float vs. double as much as possible, at least internally. > > > + { > > > + Matrix<T2, Rows, Cols> ret; > > > + for (unsigned int i = 0; i < Rows; i++) > > > + for (unsigned int j = 0; j < Cols; j++) > > > + ret[i][j] = static_cast<T2>((*this)[i][j]); > > > > You can operator on data_ directly, with a single loop. > > In the "copy" constructor case, yes. Not in this case because ret.data_ is > private from within this class. > > > > + > > > + return ret; > > > + } > > > + > > > private: > > > std::array<T, Rows * Cols> data_; > > > }; > > > diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp > > > index e7e027225666..91a3f16405a3 100644 > > > --- a/src/libcamera/matrix.cpp > > > +++ b/src/libcamera/matrix.cpp > > > @@ -77,6 +77,16 @@ LOG_DEFINE_CATEGORY(Matrix) > > > * \return Row \a i from the matrix, as a Span > > > */ > > > > > > +/** > > > + * \fn template<typename T2> Matrix<T2, Rows, Cols> Matrix::cast() const > > > + * \brief Cast the matrix to a different type > > > + * > > > + * This function returns a new matrix with the same size and values but a > > > + * different type. > > > + * > > > + * \return The new matrix > > > + */ > > > + > > > /** > > > * \fn Matrix::operator[](size_t i) > > > * \copydoc Matrix::operator[](size_t i) const
diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h index a055e6926c94..ca81dcda93c4 100644 --- a/include/libcamera/internal/matrix.h +++ b/include/libcamera/internal/matrix.h @@ -90,6 +90,17 @@ public: return *this; } + template<typename T2> + Matrix<T2, Rows, Cols> cast() const + { + Matrix<T2, Rows, Cols> ret; + for (unsigned int i = 0; i < Rows; i++) + for (unsigned int j = 0; j < Cols; j++) + ret[i][j] = static_cast<T2>((*this)[i][j]); + + return ret; + } + private: std::array<T, Rows * Cols> data_; }; diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp index e7e027225666..91a3f16405a3 100644 --- a/src/libcamera/matrix.cpp +++ b/src/libcamera/matrix.cpp @@ -77,6 +77,16 @@ LOG_DEFINE_CATEGORY(Matrix) * \return Row \a i from the matrix, as a Span */ +/** + * \fn template<typename T2> Matrix<T2, Rows, Cols> Matrix::cast() const + * \brief Cast the matrix to a different type + * + * This function returns a new matrix with the same size and values but a + * different type. + * + * \return The new matrix + */ + /** * \fn Matrix::operator[](size_t i) * \copydoc Matrix::operator[](size_t i) const
The libcamera code base uses matrices and vectors of type float and double. Add a cast function to easily convert between different underlying types. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- include/libcamera/internal/matrix.h | 11 +++++++++++ src/libcamera/matrix.cpp | 10 ++++++++++ 2 files changed, 21 insertions(+)