[4/4] libcamera: vector: Add clamp() member function
diff mbox series

Message ID 20260621002305.3763752-5-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: vector: Add clamp() function
Related show

Commit Message

Laurent Pinchart June 21, 2026, 12:23 a.m. UTC 
From: Kieran Bingham <kieran.bingham@ideasonboard.com>

Provide a member function that performs a std::clamp() given a scalar
high and low values to clamp to.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since Kieran's version:

- Use apply()
- Fix typos in commit message
---
 include/libcamera/internal/vector.h | 7 +++++++
 src/libcamera/vector.cpp            | 8 ++++++++
 test/vector.cpp                     | 1 +
 3 files changed, 16 insertions(+)

Comments

Kieran Bingham June 21, 2026, 12:20 p.m. UTC | #1 
Quoting Laurent Pinchart (2026-06-21 01:23:05)
> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Provide a member function that performs a std::clamp() given a scalar
> high and low values to clamp to.
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since Kieran's version:
> 
> - Use apply()
> - Fix typos in commit message
> ---
>  include/libcamera/internal/vector.h | 7 +++++++
>  src/libcamera/vector.cpp            | 8 ++++++++
>  test/vector.cpp                     | 1 +
>  3 files changed, 16 insertions(+)
> 
> diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h
> index 7c90d6542352..0270651a0ef7 100644
> --- a/include/libcamera/internal/vector.h
> +++ b/include/libcamera/internal/vector.h
> @@ -185,6 +185,13 @@ public:
>                 return apply(*this, [](T a, T b) -> T { return std::max(a, b); }, scalar);
>         }
>  
> +       constexpr Vector clamp(T low, T high) const
> +       {
> +               return apply(*this,
> +                            [](T v, T lo, T hi) -> T { return std::clamp(v, lo, hi); },
> +                            low, high);

Erm ... reviewed-by/approved-by of the changes to my patch?

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

> +       }
> +
>         constexpr T dot(const Vector<T, Rows> &other) const
>         {
>                 T ret = 0;
> diff --git a/src/libcamera/vector.cpp b/src/libcamera/vector.cpp
> index 47b3d1af36d9..a135ab498e17 100644
> --- a/src/libcamera/vector.cpp
> +++ b/src/libcamera/vector.cpp
> @@ -224,6 +224,14 @@ LOG_DEFINE_CATEGORY(Vector)
>   * \return The element-wise maximum of this vector and \a scalar
>   */
>  
> +/**
> + * \fn Vector::clamp(T low, T high) const
> + * \brief Clamp the vector element-wise between \a low and \a high
> + * \param[in] low The lower limit
> + * \param[in] high The upper limit
> + * \return A vector with each element clamped between \a low and \a high
> + */
> +
>  /**
>   * \fn Vector::dot(const Vector<T, Rows> &other) const
>   * \brief Compute the dot product
> diff --git a/test/vector.cpp b/test/vector.cpp
> index 4ff908e8f682..2f3d97d93dcc 100644
> --- a/test/vector.cpp
> +++ b/test/vector.cpp
> @@ -72,6 +72,7 @@ protected:
>                 ASSERT_EQ(v2.min(4.0), (Vector<double, 3>{{ 1.0, 4.0, 4.0 }}));
>                 ASSERT_EQ(v2.max(v3), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
>                 ASSERT_EQ(v2.max(4.0), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
> +               ASSERT_EQ(v2.clamp(2.0, 6.0), (Vector<double, 3>{{ 2.0, 4.0, 6.0 }}));
>  
>                 ASSERT_EQ(v2.dot(v3), 52.0);
>  
> -- 
> Regards,
> 
> Laurent Pinchart
>
Barnabás Pőcze June 22, 2026, 7:54 a.m. UTC | #2 
2026. 06. 21. 2:23 keltezéssel, Laurent Pinchart írta:
> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Provide a member function that performs a std::clamp() given a scalar
> high and low values to clamp to.
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since Kieran's version:
> 
> - Use apply()
> - Fix typos in commit message
> ---
>   include/libcamera/internal/vector.h | 7 +++++++
>   src/libcamera/vector.cpp            | 8 ++++++++
>   test/vector.cpp                     | 1 +
>   3 files changed, 16 insertions(+)
> 
> diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h
> index 7c90d6542352..0270651a0ef7 100644
> --- a/include/libcamera/internal/vector.h
> +++ b/include/libcamera/internal/vector.h
> @@ -185,6 +185,13 @@ public:
>   		return apply(*this, [](T a, T b) -> T { return std::max(a, b); }, scalar);
>   	}
>   
> +	constexpr Vector clamp(T low, T high) const
> +	{
> +		return apply(*this,
> +			     [](T v, T lo, T hi) -> T { return std::clamp(v, lo, hi); },

An alternative approach would be to capture the extra arguments in the lambda,
and then there is no need for supporting *any* scalar arguments. I guess it
mainly depends on how important it is to be able to use the same operations for
both the scalar and vector operations. On a related note, I'm wondering why
`operator{+,-,/,*}=` do not use `std::{plus,minus,...}` ?

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


> +			     low, high);
> +	}
> +
>   	constexpr T dot(const Vector<T, Rows> &other) const
>   	{
>   		T ret = 0;
> diff --git a/src/libcamera/vector.cpp b/src/libcamera/vector.cpp
> index 47b3d1af36d9..a135ab498e17 100644
> --- a/src/libcamera/vector.cpp
> +++ b/src/libcamera/vector.cpp
> @@ -224,6 +224,14 @@ LOG_DEFINE_CATEGORY(Vector)
>    * \return The element-wise maximum of this vector and \a scalar
>    */
>   
> +/**
> + * \fn Vector::clamp(T low, T high) const
> + * \brief Clamp the vector element-wise between \a low and \a high
> + * \param[in] low The lower limit
> + * \param[in] high The upper limit
> + * \return A vector with each element clamped between \a low and \a high
> + */
> +
>   /**
>    * \fn Vector::dot(const Vector<T, Rows> &other) const
>    * \brief Compute the dot product
> diff --git a/test/vector.cpp b/test/vector.cpp
> index 4ff908e8f682..2f3d97d93dcc 100644
> --- a/test/vector.cpp
> +++ b/test/vector.cpp
> @@ -72,6 +72,7 @@ protected:
>   		ASSERT_EQ(v2.min(4.0), (Vector<double, 3>{{ 1.0, 4.0, 4.0 }}));
>   		ASSERT_EQ(v2.max(v3), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
>   		ASSERT_EQ(v2.max(4.0), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
> +		ASSERT_EQ(v2.clamp(2.0, 6.0), (Vector<double, 3>{{ 2.0, 4.0, 6.0 }}));
>   
>   		ASSERT_EQ(v2.dot(v3), 52.0);
>
Laurent Pinchart June 22, 2026, 9:17 a.m. UTC | #3 
On Mon, Jun 22, 2026 at 09:54:09AM +0200, Barnabás Pőcze wrote:
> 2026. 06. 21. 2:23 keltezéssel, Laurent Pinchart írta:
> > From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> > Provide a member function that performs a std::clamp() given a scalar
> > high and low values to clamp to.
> > 
> > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since Kieran's version:
> > 
> > - Use apply()
> > - Fix typos in commit message
> > ---
> >   include/libcamera/internal/vector.h | 7 +++++++
> >   src/libcamera/vector.cpp            | 8 ++++++++
> >   test/vector.cpp                     | 1 +
> >   3 files changed, 16 insertions(+)
> > 
> > diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h
> > index 7c90d6542352..0270651a0ef7 100644
> > --- a/include/libcamera/internal/vector.h
> > +++ b/include/libcamera/internal/vector.h
> > @@ -185,6 +185,13 @@ public:
> >   		return apply(*this, [](T a, T b) -> T { return std::max(a, b); }, scalar);
> >   	}
> >   
> > +	constexpr Vector clamp(T low, T high) const
> > +	{
> > +		return apply(*this,
> > +			     [](T v, T lo, T hi) -> T { return std::clamp(v, lo, hi); },
> 
> An alternative approach would be to capture the extra arguments in the lambda,
> and then there is no need for supporting *any* scalar arguments. I guess it
> mainly depends on how important it is to be able to use the same operations for
> both the scalar and vector operations.

True, we could have done that, but then we won't be able to used
std::plus and the other similar objects provided by the standard
library.

I don't mind much personally, I don't think it makes a big difference
for now, but I'll try to keep it in mind for future work on the Vector
class.

> On a related note, I'm wondering why
> `operator{+,-,/,*}=` do not use `std::{plus,minus,...}` ?

I don't think there's a specific reason. I'll send a patch.

> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> > +			     low, high);
> > +	}
> > +
> >   	constexpr T dot(const Vector<T, Rows> &other) const
> >   	{
> >   		T ret = 0;
> > diff --git a/src/libcamera/vector.cpp b/src/libcamera/vector.cpp
> > index 47b3d1af36d9..a135ab498e17 100644
> > --- a/src/libcamera/vector.cpp
> > +++ b/src/libcamera/vector.cpp
> > @@ -224,6 +224,14 @@ LOG_DEFINE_CATEGORY(Vector)
> >    * \return The element-wise maximum of this vector and \a scalar
> >    */
> >   
> > +/**
> > + * \fn Vector::clamp(T low, T high) const
> > + * \brief Clamp the vector element-wise between \a low and \a high
> > + * \param[in] low The lower limit
> > + * \param[in] high The upper limit
> > + * \return A vector with each element clamped between \a low and \a high
> > + */
> > +
> >   /**
> >    * \fn Vector::dot(const Vector<T, Rows> &other) const
> >    * \brief Compute the dot product
> > diff --git a/test/vector.cpp b/test/vector.cpp
> > index 4ff908e8f682..2f3d97d93dcc 100644
> > --- a/test/vector.cpp
> > +++ b/test/vector.cpp
> > @@ -72,6 +72,7 @@ protected:
> >   		ASSERT_EQ(v2.min(4.0), (Vector<double, 3>{{ 1.0, 4.0, 4.0 }}));
> >   		ASSERT_EQ(v2.max(v3), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
> >   		ASSERT_EQ(v2.max(4.0), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
> > +		ASSERT_EQ(v2.clamp(2.0, 6.0), (Vector<double, 3>{{ 2.0, 4.0, 6.0 }}));
> >   
> >   		ASSERT_EQ(v2.dot(v3), 52.0);
> >

Patch
diff mbox series 

diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h
index 7c90d6542352..0270651a0ef7 100644
--- a/include/libcamera/internal/vector.h
+++ b/include/libcamera/internal/vector.h
@@ -185,6 +185,13 @@  public:
 		return apply(*this, [](T a, T b) -> T { return std::max(a, b); }, scalar);
 	}
 
+	constexpr Vector clamp(T low, T high) const
+	{
+		return apply(*this,
+			     [](T v, T lo, T hi) -> T { return std::clamp(v, lo, hi); },
+			     low, high);
+	}
+
 	constexpr T dot(const Vector<T, Rows> &other) const
 	{
 		T ret = 0;
diff --git a/src/libcamera/vector.cpp b/src/libcamera/vector.cpp
index 47b3d1af36d9..a135ab498e17 100644
--- a/src/libcamera/vector.cpp
+++ b/src/libcamera/vector.cpp
@@ -224,6 +224,14 @@  LOG_DEFINE_CATEGORY(Vector)
  * \return The element-wise maximum of this vector and \a scalar
  */
 
+/**
+ * \fn Vector::clamp(T low, T high) const
+ * \brief Clamp the vector element-wise between \a low and \a high
+ * \param[in] low The lower limit
+ * \param[in] high The upper limit
+ * \return A vector with each element clamped between \a low and \a high
+ */
+
 /**
  * \fn Vector::dot(const Vector<T, Rows> &other) const
  * \brief Compute the dot product
diff --git a/test/vector.cpp b/test/vector.cpp
index 4ff908e8f682..2f3d97d93dcc 100644
--- a/test/vector.cpp
+++ b/test/vector.cpp
@@ -72,6 +72,7 @@  protected:
 		ASSERT_EQ(v2.min(4.0), (Vector<double, 3>{{ 1.0, 4.0, 4.0 }}));
 		ASSERT_EQ(v2.max(v3), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
 		ASSERT_EQ(v2.max(4.0), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
+		ASSERT_EQ(v2.clamp(2.0, 6.0), (Vector<double, 3>{{ 2.0, 4.0, 6.0 }}));
 
 		ASSERT_EQ(v2.dot(v3), 52.0);