[{"id":38849,"web_url":"https://patchwork.libcamera.org/comment/38849/","msgid":"","date":"2026-05-11T09:18:36","subject":"Re: [PATCH v2 1/2] libcamera: libipa: Add right-shift operators to\n\tVector","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2026. 05. 08. 23:35 keltezéssel, Javier Tia írta:\n> Add operator>> and operator>>= for right-shifting all elements by a\n> scalar shift amount. Both operate element-wise: operator>> returns a\n> new Vector with each element shifted, operator>>= shifts in place and\n> returns a reference to *this.\n> \n> The motivating use case is SwStatsCpu::finishFrame(), which right-shifts\n> three individual components of the RGB sum by the same sumShift_ value.\n> With these operators that becomes a single sum_ >>= sumShift_ expression.\n> \n> The private apply() helpers for non-mutating and mutating scalar\n> operations are templated on the scalar type so the existing operators\n> keep working with T while shift can pass an unsigned int. A\n> static_assert in each shift operator restricts them to integer element\n> types since right-shift is undefined on floating-point Vectors.\n\nUnfortunately `std::is_integral_v == true`, but I think we should ignore that.\n\n\n> \n> Suggested-by: Barnabás Pőcze \n> Suggested-by: Laurent Pinchart \n> Signed-off-by: Javier Tia \n> ---\n> include/libcamera/internal/vector.h | 22 ++++++++++++++++++----\n> src/libcamera/vector.cpp | 14 ++++++++++++++\n> 2 files changed, 32 insertions(+), 4 deletions(-)\n> \n> diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h\n> index ed7490e1..e3ea723d 100644\n> --- a/include/libcamera/internal/vector.h\n> +++ b/include/libcamera/internal/vector.h\n> @@ -111,6 +111,13 @@ public:\n> \t\treturn apply(*this, scalar, std::divides<>{});\n> \t}\n> \n> +\tconstexpr Vector operator>>(unsigned int shift) const\n> +\t{\n> +\t\tstatic_assert(std::is_integral_v,\n> +\t\t\t \"Vector::operator>> requires an integer element type\");\n> +\t\treturn apply(*this, shift, [](T a, unsigned int b) { return a >> b; });\n> +\t}\n> +\n> \tVector &operator+=(const Vector &other)\n> \t{\n> \t\treturn apply(other, [](T a, T b) { return a + b; });\n> @@ -151,6 +158,13 @@ public:\n> \t\treturn apply(scalar, [](T a, T b) { return a / b; });\n> \t}\n> \n> +\tVector &operator>>=(unsigned int shift)\n> +\t{\n> +\t\tstatic_assert(std::is_integral_v,\n> +\t\t\t \"Vector::operator>>= requires an integer element type\");\n> +\t\treturn apply(shift, [](T a, unsigned int b) { return a >> b; });\n> +\t}\n> +\n> \tconstexpr Vector min(const Vector &other) const\n> \t{\n> \t\treturn apply(*this, other, [](T a, T b) { return std::min(a, b); });\n> @@ -260,8 +274,8 @@ private:\n> \t\treturn result;\n> \t}\n> \n> -\ttemplate\n> -\tstatic constexpr Vector apply(const Vector &lhs, T rhs, BinaryOp op)\n> +\ttemplate\n> +\tstatic constexpr Vector apply(const Vector &lhs, U rhs, BinaryOp op)\n> \t{\n> \t\tVector result;\n> \t\tstd::transform(lhs.data_.begin(), lhs.data_.end(),\n> @@ -281,8 +295,8 @@ private:\n> \t\treturn *this;\n> \t}\n> \n> -\ttemplate\n> -\tVector &apply(T scalar, BinaryOp op)\n> +\ttemplate\n> +\tVector &apply(U scalar, BinaryOp op)\n> \t{\n> \t\tstd::for_each(data_.begin(), data_.end(),\n> \t\t\t [&op, scalar](T &v) { v = op(v, scalar); });\n> diff --git a/src/libcamera/vector.cpp b/src/libcamera/vector.cpp\n> index 86b9f9bb..d3b7e4ab 100644\n> --- a/src/libcamera/vector.cpp\n> +++ b/src/libcamera/vector.cpp\n> @@ -126,6 +126,20 @@ LOG_DEFINE_CATEGORY(Vector)\n> * \\return The element-wise division of this vector by \\a scalar\n> */\n> \n> +/**\n> + * \\fn Vector::operator>>(unsigned int shift) const\n> + * \\brief Right-shift each element of this vector by \\a shift bits\n> + * \\param[in] shift The shift amount\n> + * \\return A new vector with each element right-shifted by \\a shift\n> + */\n> +\n> +/**\n> + * \\fn Vector::operator>>=(unsigned int shift)\n> + * \\brief Right-shift each element of this vector by \\a shift bits in place\n> + * \\param[in] shift The shift amount\n> + * \\return This vector\n> + */\n\nI think it would be best to preserve the order, i.e. moving the `=` variant to\nbe after `operator/=`. Otherwise the code looks ok to me. Maybe adding a couple\ntests to `test/vector.cpp` would be nice.\n\nReviewed-by: Barnabás Pőcze \n\n\n> +\n> /**\n> * \\fn Vector::operator+=(Vector const &other)\n> * \\brief Add \\a other element-wise to this vector","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 64BE3BDB1C\n\tfor ;\n\tMon, 11 May 2026 09:18:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 198E76271A;\n\tMon, 11 May 2026 11:18:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D7B5862010\n\tfor ;\n\tMon, 11 May 2026 11:18:39 +0200 (CEST)","from [192.168.33.37] (185.182.215.166.nat.pool.zt.hu\n\t[185.182.215.166])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D390A73B;\n\tMon, 11 May 2026 11:18:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XzXIIUGr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778491113;\n\tbh=AObmR6vpXXtoiNRvm25zEo9jrkkvyxvXORdTYjxp+wA=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=XzXIIUGr2bxZ5Xi+uDZqmrtu4can+f+1ZiYOPxRbhPkbT4U0v3RLXu1bUSCafvqDu\n\tofYnrDClQxZvzOFbz5mX9qjTqltiiwQdEuBVmlwL3tNX4IzxooThXPCtS0j3CcfQqT\n\tWnkBwC2MNrcGM8acxuSnHxaeKbieHsHtC6je6H6k=","Message-ID":"","Date":"Mon, 11 May 2026 11:18:36 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 1/2] libcamera: libipa: Add right-shift operators to\n\tVector","To":"Javier Tia , libcamera-devel@lists.libcamera.org","Cc":"mzamazal@redhat.com, kieran.bingham@ideasonboard.com,\n\tlaurent.pinchart@ideasonboard.com, robert.mader@collabora.com","References":"<177827984964.786021.2242634090011461397@jetm.me>\n\t<20260508223733.DA0831EA006C@mailuser.phl.internal>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260508223733.DA0831EA006C@mailuser.phl.internal>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]