[PATCH v2 04/17] libcamera: vector: Add a Span based constructor

[Laurent Pinchart]

On Mon, Mar 31, 2025 at 06:05:28PM +0100, Kieran Bingham wrote:
> Quoting Stefan Klug (2025-03-19 16:11:09)
> > Now that Matrix has a Span based constructor, add the same for Vector.
> 
> Well, with that logic, someone else has a yacht - so I deserve one ...
> :-)
> 
> I assume Vectors deserve span based constructors because it benefits
> them directly?

A similar commit message to 03/17 would be better indeed.

All my comments on 03/17 apply here too.

> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > 
> > ---
> > 
> > Changes in v2:
> > - Added this patch
> > ---
> >  include/libcamera/internal/vector.h | 12 ++++++++----
> >  src/libcamera/vector.cpp            |  8 ++++++++
> >  2 files changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h
> > index a67a09474204..66cc5ac988c2 100644
> > --- a/include/libcamera/internal/vector.h
> > +++ b/include/libcamera/internal/vector.h
> > @@ -40,10 +40,14 @@ public:
> >                 data_.fill(scalar);
> >         }
> >  
> > -       constexpr Vector(const std::array<T, Rows> &data)
> > +       Vector(const std::array<T, Rows> &data)
> >         {
> > -               for (unsigned int i = 0; i < Rows; i++)
> > -                       data_[i] = data[i];
> > +               std::copy(data.begin(), data.end(), data_.begin());
> > +       }
> 
> This loses a constexpr ... what's the downside here? At least that
> should be highlighted/discussed in the commit message.
> 
> > +
> > +       Vector(const Span<const T, Rows> &data)
> > +       {
> > +               std::copy(data.begin(), data.end(), data_.begin());
> >         }
> >  
> >         const T &operator[](size_t i) const
> > @@ -285,7 +289,7 @@ private:
> >                 return *this;
> >         }
> >  
> > -       std::array<T, Rows> data_;
> > +       std::array<T, Rows> data_{};
> 
> Is this just for initialisation ?
>
> >  };
> >  
> >  template<typename T>
> > diff --git a/src/libcamera/vector.cpp b/src/libcamera/vector.cpp
> > index 85ca2208245a..435f2fc62f8b 100644
> > --- a/src/libcamera/vector.cpp
> > +++ b/src/libcamera/vector.cpp
> > @@ -44,6 +44,14 @@ LOG_DEFINE_CATEGORY(Vector)
> >   * The size of \a data must be equal to the dimension size Rows of the vector.
> >   */
> >  
> > +/**
> > + * \fn Vector::Vector(const Span<const T, Rows> &data)
> > + * \brief Construct vector from supplied data
> > + * \param data Data from which to construct a vector
> > + *
> > + * The size of \a data must be equal to the dimension size Rows of the vector.
> > + */
> > +
> >  /**
> >   * \fn T Vector::operator[](size_t i) const
> >   * \brief Index to an element in the vector

-- 
Regards,

Laurent Pinchart