| Message ID | 20260508191941.9DB451EA006C@mailuser.phl.internal |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On Fri, May 08, 2026 at 01:19:25PM -0600, Javier Tia wrote: > Add operator>> and operator>>= for right-shifting all elements by a > scalar shift amount. Both operate element-wise: operator>> returns a > new Vector with each element shifted, operator>>= shifts in place and > returns a reference to *this. > > The motivating use case is SwStatsCpu::finishFrame(), which right-shifts > three individual components of the RGB sum by the same sumShift_ value. > With these operators that becomes a single sum_ >>= sumShift_ expression. > > Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Javier Tia <floss@jetm.me> > --- > include/libcamera/internal/vector.h | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h > index ed7490e1..475bc522 100644 > --- a/include/libcamera/internal/vector.h > +++ b/include/libcamera/internal/vector.h > @@ -111,6 +111,14 @@ public: > return apply(*this, scalar, std::divides<>{}); > } > > + constexpr Vector operator>>(unsigned int shift) const > + { > + Vector result; > + std::transform(data_.begin(), data_.end(), result.data_.begin(), > + [shift](T v) { return v >> shift; }); > + return result; What's the reason to implement this operator differently from the other ones ? > + } > + > Vector &operator+=(const Vector &other) > { > return apply(other, [](T a, T b) { return a + b; }); > @@ -151,6 +159,13 @@ public: > return apply(scalar, [](T a, T b) { return a / b; }); > } > > + Vector &operator>>=(unsigned int shift) > + { > + std::for_each(data_.begin(), data_.end(), > + [shift](T &v) { v >>= shift; }); > + return *this; > + } > + > constexpr Vector min(const Vector &other) const > { > return apply(*this, other, [](T a, T b) { return std::min(a, b); });
On Fri, May 08, 2026 at 11:01:21PM +0300, Laurent Pinchart wrote: > On Fri, May 08, 2026 at 01:19:25PM -0600, Javier Tia wrote: > > Add operator>> and operator>>= for right-shifting all elements by a > > scalar shift amount. Both operate element-wise: operator>> returns a > > new Vector with each element shifted, operator>>= shifts in place and > > returns a reference to *this. > > > > The motivating use case is SwStatsCpu::finishFrame(), which right-shifts > > three individual components of the RGB sum by the same sumShift_ value. > > With these operators that becomes a single sum_ >>= sumShift_ expression. > > > > Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Signed-off-by: Javier Tia <floss@jetm.me> > > --- > > include/libcamera/internal/vector.h | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h > > index ed7490e1..475bc522 100644 > > --- a/include/libcamera/internal/vector.h > > +++ b/include/libcamera/internal/vector.h > > @@ -111,6 +111,14 @@ public: > > return apply(*this, scalar, std::divides<>{}); > > } > > > > + constexpr Vector operator>>(unsigned int shift) const > > + { > > + Vector result; > > + std::transform(data_.begin(), data_.end(), result.data_.begin(), > > + [shift](T v) { return v >> shift; }); > > + return result; > > What's the reason to implement this operator differently from the other > ones ? Also, please compile libcamera with documentation=enabled and doc_werror=true. > > + } > > + > > Vector &operator+=(const Vector &other) > > { > > return apply(other, [](T a, T b) { return a + b; }); > > @@ -151,6 +159,13 @@ public: > > return apply(scalar, [](T a, T b) { return a / b; }); > > } > > > > + Vector &operator>>=(unsigned int shift) > > + { > > + std::for_each(data_.begin(), data_.end(), > > + [shift](T &v) { v >>= shift; }); > > + return *this; > > + } > > + > > constexpr Vector min(const Vector &other) const > > { > > return apply(*this, other, [](T a, T b) { return std::min(a, b); });
Hi Laurent, On Fri, May 08, 2026 at 11:03:01PM +0300, Laurent Pinchart wrote: > On Fri, May 08, 2026 at 11:01:21PM +0300, Laurent Pinchart wrote: > > > + constexpr Vector operator>>(unsigned int shift) const > > > + { > > > + Vector result; > > > + std::transform(data_.begin(), data_.end(), result.data_.begin(), > > > + [shift](T v) { return v >> shift; }); > > > + return result; > > > > What's the reason to implement this operator differently from the other > > ones ? No good reason - I should have used apply() from the start. The shift amount is unsigned int rather than T, which is why I sidestepped apply() initially, but the cleaner fix is to template the scalar apply() overloads on the scalar type so they accept any integral type while existing call sites keep deducing U as T. In v2 both operators are one-liners on top of apply(): constexpr Vector operator>>(unsigned int shift) const { return apply(*this, shift, [](T a, unsigned int b) { return a >> b; }); } Vector &operator>>=(unsigned int shift) { return apply(shift, [](T a, unsigned int b) { return a >> b; }); } > Also, please compile libcamera with documentation=enabled and > doc_werror=true. Done. v2 adds \fn blocks for both operators in src/libcamera/vector.cpp next to the other arithmetic operator entries, and the build passes with -Ddocumentation=enabled -Ddoc_werror=true. Sending v2 momentarily. Best, Javier
On Fri, May 08, 2026 at 02:41:10PM -0600, Javier Tia wrote: > Hi Laurent, > > On Fri, May 08, 2026 at 11:03:01PM +0300, Laurent Pinchart wrote: > > On Fri, May 08, 2026 at 11:01:21PM +0300, Laurent Pinchart wrote: > > > > + constexpr Vector operator>>(unsigned int shift) const > > > > + { > > > > + Vector result; > > > > + std::transform(data_.begin(), data_.end(), result.data_.begin(), > > > > + [shift](T v) { return v >> shift; }); > > > > + return result; > > > > > > What's the reason to implement this operator differently from the other > > > ones ? > > No good reason - I should have used apply() from the start. The shift > amount is unsigned int rather than T, which is why I sidestepped apply() > initially, Ah that's a good point. > but the cleaner fix is to template the scalar apply() > overloads on the scalar type so they accept any integral type while > existing call sites keep deducing U as T. Hmmm... OK let's see how it goes. This also makes me think we may want a static_assert in these two new operators to ensure that T is an integer. > In v2 both operators are one-liners on top of apply(): > > constexpr Vector operator>>(unsigned int shift) const > { > return apply(*this, shift, [](T a, unsigned int b) { return a >> b; }); > } > > Vector &operator>>=(unsigned int shift) > { > return apply(shift, [](T a, unsigned int b) { return a >> b; }); > } > > > Also, please compile libcamera with documentation=enabled and > > doc_werror=true. > > Done. v2 adds \fn blocks for both operators in src/libcamera/vector.cpp > next to the other arithmetic operator entries, and the build passes > with -Ddocumentation=enabled -Ddoc_werror=true. > > Sending v2 momentarily.
Hi Laurent, On Fri, May 08, 2026 at 11:57:19PM +0300, Laurent Pinchart wrote: > Hmmm... OK let's see how it goes. This also makes me think we may want a > static_assert in these two new operators to ensure that T is an integer. Good call. v2 1/2 adds static_assert(std::is_integral_v<T>) in both operator>> and operator>>= with a message pointing at the operator.
diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h index ed7490e1..475bc522 100644 --- a/include/libcamera/internal/vector.h +++ b/include/libcamera/internal/vector.h @@ -111,6 +111,14 @@ public: return apply(*this, scalar, std::divides<>{}); } + constexpr Vector operator>>(unsigned int shift) const + { + Vector result; + std::transform(data_.begin(), data_.end(), result.data_.begin(), + [shift](T v) { return v >> shift; }); + return result; + } + Vector &operator+=(const Vector &other) { return apply(other, [](T a, T b) { return a + b; }); @@ -151,6 +159,13 @@ public: return apply(scalar, [](T a, T b) { return a / b; }); } + Vector &operator>>=(unsigned int shift) + { + std::for_each(data_.begin(), data_.end(), + [shift](T &v) { v >>= shift; }); + return *this; + } + constexpr Vector min(const Vector &other) const { return apply(*this, other, [](T a, T b) { return std::min(a, b); });
Add operator>> and operator>>= for right-shifting all elements by a scalar shift amount. Both operate element-wise: operator>> returns a new Vector with each element shifted, operator>>= shifts in place and returns a reference to *this. The motivating use case is SwStatsCpu::finishFrame(), which right-shifts three individual components of the RGB sum by the same sumShift_ value. With these operators that becomes a single sum_ >>= sumShift_ expression. Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Javier Tia <floss@jetm.me> --- include/libcamera/internal/vector.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+)