[v3,04/16] libcamera: vector: Add a Span based constructor

23120 diff mbox series

When one wants to create a Vector from existing data, currently the only
way is via std::array. Add a Span based constructor to allow creation
from std::vectors and alike.

While at it, replace the manual loop with std::copy.

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

---

Changes in v2:
- Added this patch

Changes in v3:
- Improved commit message
- Added constexpr to the contructor
- Pass Span by value
- Removed initializer of data_ member which was only needed for the
  default constructor to be actually constexpr. But Vector is not used
as constexpr anywhere, so we can leave that as is (This will fix itself
with C++20).
---
 include/libcamera/internal/vector.h | 8 ++++++--
 src/libcamera/vector.cpp            | 8 ++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

Quoting Stefan Klug (2025-04-03 16:49:09)
> When one wants to create a Vector from existing data, currently the only
> way is via std::array. Add a Span based constructor to allow creation
> from std::vectors and alike.
> 
> While at it, replace the manual loop with std::copy.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
> 
> Changes in v2:
> - Added this patch
> 
> Changes in v3:
> - Improved commit message
> - Added constexpr to the contructor
> - Pass Span by value
> - Removed initializer of data_ member which was only needed for the
>   default constructor to be actually constexpr. But Vector is not used
> as constexpr anywhere, so we can leave that as is (This will fix itself
> with C++20).
> ---
>  include/libcamera/internal/vector.h | 8 ++++++--
>  src/libcamera/vector.cpp            | 8 ++++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h
> index a67a09474204..4e9ef1ee6853 100644
> --- a/include/libcamera/internal/vector.h
> +++ b/include/libcamera/internal/vector.h
> @@ -42,8 +42,12 @@ public:
>  
>         constexpr 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());
> +       }
> +
> +       constexpr Vector(const Span<const T, Rows> data)
> +       {
> +               std::copy(data.begin(), data.end(), data_.begin());
>         }
>  
>         const T &operator[](size_t i) const
> diff --git a/src/libcamera/vector.cpp b/src/libcamera/vector.cpp
> index 85ca2208245a..5567d5b8defb 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
> -- 
> 2.43.0
>

On Thu, Apr 03, 2025 at 05:49:09PM +0200, Stefan Klug wrote:
> When one wants to create a Vector from existing data, currently the only
> way is via std::array. Add a Span based constructor to allow creation
> from std::vectors and alike.
> 
> While at it, replace the manual loop with std::copy.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> 
> ---
> 
> Changes in v2:
> - Added this patch
> 
> Changes in v3:
> - Improved commit message
> - Added constexpr to the contructor
> - Pass Span by value
> - Removed initializer of data_ member which was only needed for the
>   default constructor to be actually constexpr. But Vector is not used
> as constexpr anywhere, so we can leave that as is (This will fix itself
> with C++20).
> ---
>  include/libcamera/internal/vector.h | 8 ++++++--
>  src/libcamera/vector.cpp            | 8 ++++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h
> index a67a09474204..4e9ef1ee6853 100644
> --- a/include/libcamera/internal/vector.h
> +++ b/include/libcamera/internal/vector.h
> @@ -42,8 +42,12 @@ public:
>  
>  	constexpr 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());
> +	}
> +
> +	constexpr Vector(const Span<const T, Rows> data)
> +	{
> +		std::copy(data.begin(), data.end(), data_.begin());
>  	}
>  
>  	const T &operator[](size_t i) const
> diff --git a/src/libcamera/vector.cpp b/src/libcamera/vector.cpp
> index 85ca2208245a..5567d5b8defb 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
> -- 
> 2.43.0
>