[PATCH 01/10] libcamera: matrix: Add cast function

[Stefan Klug]

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 at 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