[{"id":33824,"web_url":"https://patchwork.libcamera.org/comment/33824/","msgid":"<20250401005957.GP14432@pendragon.ideasonboard.com>","date":"2025-04-01T00:59:57","subject":"Re: [PATCH v2 02/17] libcamera: matrix: Make most functions\n\tconstexpr","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Stefan,\n\nThank you for the patch.\n\nOn Wed, Mar 19, 2025 at 05:11:07PM +0100, Stefan Klug wrote:\n> By zero-initializing the data_ member we can make most functions\n> constexpr which will come in handy in upcoming patches. Note that this\n> is due to C++17. In C++20 we will be able to leave data_ unintialized\n\ns/unintialized/uninitialized/\n\nGiven that we only operate on small matrices I think this is acceptable.\nPlease add a todo comment to indicate that initialization of data should\nbe removed with C++20.\n\n> for constexpr. The Matrix(std::array) version of the constructor can\n> not be constexpr because std::copy only became constexpr in C++20.\n> \n> Signed-off-by: Stefan Klug \n> \n> ---\n> \n> Changes in v2:\n> - Added this patch\n> ---\n> include/libcamera/internal/matrix.h | 15 +++++++--------\n> 1 file changed, 7 insertions(+), 8 deletions(-)\n> \n> diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h\n> index 8399be583f28..2e7929e9060c 100644\n> --- a/include/libcamera/internal/matrix.h\n> +++ b/include/libcamera/internal/matrix.h\n> @@ -25,9 +25,8 @@ class Matrix\n> \tstatic_assert(std::is_arithmetic_v, \"Matrix type must be arithmetic\");\n> \n> public:\n> -\tMatrix()\n> +\tconstexpr Matrix()\n> \t{\n> -\t\tdata_.fill(static_cast(0));\n> \t}\n> \n> \tMatrix(const std::array &data)\n> @@ -35,7 +34,7 @@ public:\n> \t\tstd::copy(data.begin(), data.end(), data_.begin());\n> \t}\n> \n> -\tstatic Matrix identity()\n> +\tstatic constexpr Matrix identity()\n> \t{\n> \t\tMatrix ret;\n> \t\tfor (size_t i = 0; i < std::min(Rows, Cols); i++)\n> @@ -63,14 +62,14 @@ public:\n> \t\treturn out.str();\n> \t}\n> \n> -\tSpan data() const { return data_; }\n> +\tconstexpr Span data() const { return data_; }\n> \n> -\tSpan operator[](size_t i) const\n> +\tconstexpr Span operator[](size_t i) const\n> \t{\n> \t\treturn Span{ &data_.data()[i * Cols], Cols };\n> \t}\n> \n> -\tSpan operator[](size_t i)\n> +\tconstexpr Span operator[](size_t i)\n> \t{\n> \t\treturn Span{ &data_.data()[i * Cols], Cols };\n> \t}\n> @@ -85,7 +84,7 @@ public:\n> \t}\n> \n> private:\n> -\tstd::array data_;\n> +\tstd::array data_{};\n\nWe usually write\n\n\tstd::array data_ = {};\n\nWith that and the comment,\n\nReviewed-by: Laurent Pinchart \n\n> };\n> \n> template\n> @@ -130,7 +129,7 @@ constexpr Matrix operator*(const Matrix &m1, const Matrix<\n> }\n> \n> template\n> -Matrix operator+(const Matrix &m1, const Matrix &m2)\n> +constexpr Matrix operator+(const Matrix &m1, const Matrix &m2)\n> {\n> \tMatrix result;\n>","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 B66AFC323E\n\tfor ;\n\tTue, 1 Apr 2025 01:00:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9C6E468986;\n\tTue, 1 Apr 2025 03:00:23 +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 37A1262C66\n\tfor ;\n\tTue, 1 Apr 2025 03:00:22 +0200 (CEST)","from pendragon.ideasonboard.com (85-76-147-224-nat.elisa-mobile.fi\n\t[85.76.147.224])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3A3D2250;\n\tTue, 1 Apr 2025 02:58:29 +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=\"mMzFJNbV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743469110;\n\tbh=mCuUF6PMsgmR19PNso+TZleTi17zvxUbaPXruBbusps=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mMzFJNbVCklWPEMpgo52GGC6RMUXs+Tm0bQZ8BReqkjpdt+rQvGKjUjooOV5cUT6f\n\t99eZbN6ZuzLyhVLJXFKjw1Y95gcW1185uwW5QXjDBBpuye9vrKluH3rqfvGEWg//bB\n\tBp4EO+Tv4nAKJkfcnddWgm02afZMUfiEZkfiOjEM=","Date":"Tue, 1 Apr 2025 03:59:57 +0300","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 02/17] libcamera: matrix: Make most functions\n\tconstexpr","Message-ID":"<20250401005957.GP14432@pendragon.ideasonboard.com>","References":"<20250319161152.63625-1-stefan.klug@ideasonboard.com>\n\t<20250319161152.63625-3-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250319161152.63625-3-stefan.klug@ideasonboard.com>","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\" "}},{"id":33827,"web_url":"https://patchwork.libcamera.org/comment/33827/","msgid":"<20250401010824.GA20312@pendragon.ideasonboard.com>","date":"2025-04-01T01:08:24","subject":"Re: [PATCH v2 02/17] libcamera: matrix: Make most functions\n\tconstexpr","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Apr 01, 2025 at 03:59:57AM +0300, Laurent Pinchart wrote:\n> Hi Stefan,\n> \n> Thank you for the patch.\n> \n> On Wed, Mar 19, 2025 at 05:11:07PM +0100, Stefan Klug wrote:\n> > By zero-initializing the data_ member we can make most functions\n> > constexpr which will come in handy in upcoming patches. Note that this\n> > is due to C++17. In C++20 we will be able to leave data_ unintialized\n> \n> s/unintialized/uninitialized/\n> \n> Given that we only operate on small matrices I think this is acceptable.\n> Please add a todo comment to indicate that initialization of data should\n> be removed with C++20.\n> \n> > for constexpr. The Matrix(std::array) version of the constructor can\n> > not be constexpr because std::copy only became constexpr in C++20.\n\nActually, would open-coding std::copy allow making the constructor\nconstexpr ? If subsequent patches should be updated accordingly.\n\n> > Signed-off-by: Stefan Klug \n> > \n> > ---\n> > \n> > Changes in v2:\n> > - Added this patch\n> > ---\n> > include/libcamera/internal/matrix.h | 15 +++++++--------\n> > 1 file changed, 7 insertions(+), 8 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h\n> > index 8399be583f28..2e7929e9060c 100644\n> > --- a/include/libcamera/internal/matrix.h\n> > +++ b/include/libcamera/internal/matrix.h\n> > @@ -25,9 +25,8 @@ class Matrix\n> > \tstatic_assert(std::is_arithmetic_v, \"Matrix type must be arithmetic\");\n> > \n> > public:\n> > -\tMatrix()\n> > +\tconstexpr Matrix()\n> > \t{\n> > -\t\tdata_.fill(static_cast(0));\n> > \t}\n> > \n> > \tMatrix(const std::array &data)\n> > @@ -35,7 +34,7 @@ public:\n> > \t\tstd::copy(data.begin(), data.end(), data_.begin());\n> > \t}\n> > \n> > -\tstatic Matrix identity()\n> > +\tstatic constexpr Matrix identity()\n> > \t{\n> > \t\tMatrix ret;\n> > \t\tfor (size_t i = 0; i < std::min(Rows, Cols); i++)\n> > @@ -63,14 +62,14 @@ public:\n> > \t\treturn out.str();\n> > \t}\n> > \n> > -\tSpan data() const { return data_; }\n> > +\tconstexpr Span data() const { return data_; }\n> > \n> > -\tSpan operator[](size_t i) const\n> > +\tconstexpr Span operator[](size_t i) const\n> > \t{\n> > \t\treturn Span{ &data_.data()[i * Cols], Cols };\n> > \t}\n> > \n> > -\tSpan operator[](size_t i)\n> > +\tconstexpr Span operator[](size_t i)\n> > \t{\n> > \t\treturn Span{ &data_.data()[i * Cols], Cols };\n> > \t}\n> > @@ -85,7 +84,7 @@ public:\n> > \t}\n> > \n> > private:\n> > -\tstd::array data_;\n> > +\tstd::array data_{};\n> \n> We usually write\n> \n> \tstd::array data_ = {};\n> \n> With that and the comment,\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > };\n> > \n> > template\n> > @@ -130,7 +129,7 @@ constexpr Matrix operator*(const Matrix &m1, const Matrix<\n> > }\n> > \n> > template\n> > -Matrix operator+(const Matrix &m1, const Matrix &m2)\n> > +constexpr Matrix operator+(const Matrix &m1, const Matrix &m2)\n> > {\n> > \tMatrix result;\n> >","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 6EF5BC323E\n\tfor ;\n\tTue, 1 Apr 2025 01:08:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4606868985;\n\tTue, 1 Apr 2025 03:08:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2B8C362C66\n\tfor ;\n\tTue, 1 Apr 2025 03:08:49 +0200 (CEST)","from pendragon.ideasonboard.com (85-76-147-224-nat.elisa-mobile.fi\n\t[85.76.147.224])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 12390564;\n\tTue, 1 Apr 2025 03:06:56 +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=\"KPqn0c3a\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743469617;\n\tbh=seb9+1J+s9R7T2PQu5sjGkYIca8hlnkt98LuXkP040k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KPqn0c3akFaMqxSAk7xcMXkSqZS/T7lHRbrdKvx0OVFY13I8ICD1Ge+3z1aafXNQT\n\t+zdZIQpeJNPEGVEOVnxSDI/BNKeT1V1FYs8Amwi4hBkAoCn3Nn2PivPejieMyGcrOy\n\tRv+Elfo3EOlwrSUP3XAmUYTB4DKeTDi/fEafuMsQ=","Date":"Tue, 1 Apr 2025 04:08:24 +0300","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 02/17] libcamera: matrix: Make most functions\n\tconstexpr","Message-ID":"<20250401010824.GA20312@pendragon.ideasonboard.com>","References":"<20250319161152.63625-1-stefan.klug@ideasonboard.com>\n\t<20250319161152.63625-3-stefan.klug@ideasonboard.com>\n\t<20250401005957.GP14432@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250401005957.GP14432@pendragon.ideasonboard.com>","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\" "}},{"id":33850,"web_url":"https://patchwork.libcamera.org/comment/33850/","msgid":"<5litsybba5l7swnltuok5wrirrwxzgyjkpabu2fhxswqbbrmfb@4opkw5ijnfyc>","date":"2025-04-01T13:50:41","subject":"Re: [PATCH v2 02/17] libcamera: matrix: Make most functions\n\tconstexpr","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"On Tue, Apr 01, 2025 at 04:08:24AM +0300, Laurent Pinchart wrote:\n> On Tue, Apr 01, 2025 at 03:59:57AM +0300, Laurent Pinchart wrote:\n> > Hi Stefan,\n> > \n> > Thank you for the patch.\n> > \n> > On Wed, Mar 19, 2025 at 05:11:07PM +0100, Stefan Klug wrote:\n> > > By zero-initializing the data_ member we can make most functions\n> > > constexpr which will come in handy in upcoming patches. Note that this\n> > > is due to C++17. In C++20 we will be able to leave data_ unintialized\n> > \n> > s/unintialized/uninitialized/\n> > \n> > Given that we only operate on small matrices I think this is acceptable.\n> > Please add a todo comment to indicate that initialization of data should\n> > be removed with C++20.\n> > \n> > > for constexpr. The Matrix(std::array) version of the constructor can\n> > > not be constexpr because std::copy only became constexpr in C++20.\n> \n> Actually, would open-coding std::copy allow making the constructor\n> constexpr ? If subsequent patches should be updated accordingly.\n\nI believe yes. I didn't bother to do that as we don't have any user of\nthat constructor and it is unclear what comes first. A constexpr usage\nof that constructor or us switching to C++20. While I write that: What\nabout leaving the constexpr here as we have other places where we label\nthings as constexpr that are actually not constexpr. So the intent would\nbe clear, just that it doesn't work in C++17 :-)\n\nBest regards,\nStefan\n\n> \n> > > Signed-off-by: Stefan Klug \n> > > \n> > > ---\n> > > \n> > > Changes in v2:\n> > > - Added this patch\n> > > ---\n> > > include/libcamera/internal/matrix.h | 15 +++++++--------\n> > > 1 file changed, 7 insertions(+), 8 deletions(-)\n> > > \n> > > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h\n> > > index 8399be583f28..2e7929e9060c 100644\n> > > --- a/include/libcamera/internal/matrix.h\n> > > +++ b/include/libcamera/internal/matrix.h\n> > > @@ -25,9 +25,8 @@ class Matrix\n> > > \tstatic_assert(std::is_arithmetic_v, \"Matrix type must be arithmetic\");\n> > > \n> > > public:\n> > > -\tMatrix()\n> > > +\tconstexpr Matrix()\n> > > \t{\n> > > -\t\tdata_.fill(static_cast(0));\n> > > \t}\n> > > \n> > > \tMatrix(const std::array &data)\n> > > @@ -35,7 +34,7 @@ public:\n> > > \t\tstd::copy(data.begin(), data.end(), data_.begin());\n> > > \t}\n> > > \n> > > -\tstatic Matrix identity()\n> > > +\tstatic constexpr Matrix identity()\n> > > \t{\n> > > \t\tMatrix ret;\n> > > \t\tfor (size_t i = 0; i < std::min(Rows, Cols); i++)\n> > > @@ -63,14 +62,14 @@ public:\n> > > \t\treturn out.str();\n> > > \t}\n> > > \n> > > -\tSpan data() const { return data_; }\n> > > +\tconstexpr Span data() const { return data_; }\n> > > \n> > > -\tSpan operator[](size_t i) const\n> > > +\tconstexpr Span operator[](size_t i) const\n> > > \t{\n> > > \t\treturn Span{ &data_.data()[i * Cols], Cols };\n> > > \t}\n> > > \n> > > -\tSpan operator[](size_t i)\n> > > +\tconstexpr Span operator[](size_t i)\n> > > \t{\n> > > \t\treturn Span{ &data_.data()[i * Cols], Cols };\n> > > \t}\n> > > @@ -85,7 +84,7 @@ public:\n> > > \t}\n> > > \n> > > private:\n> > > -\tstd::array data_;\n> > > +\tstd::array data_{};\n> > \n> > We usually write\n> > \n> > \tstd::array data_ = {};\n> > \n> > With that and the comment,\n> > \n> > Reviewed-by: Laurent Pinchart \n> > \n> > > };\n> > > \n> > > template\n> > > @@ -130,7 +129,7 @@ constexpr Matrix operator*(const Matrix &m1, const Matrix<\n> > > }\n> > > \n> > > template\n> > > -Matrix operator+(const Matrix &m1, const Matrix &m2)\n> > > +constexpr Matrix operator+(const Matrix &m1, const Matrix &m2)\n> > > {\n> > > \tMatrix result;\n> > > \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 2E7EAC323E\n\tfor ;\n\tTue, 1 Apr 2025 13:50:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2C84368981;\n\tTue, 1 Apr 2025 15:50:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7016068947\n\tfor ;\n\tTue, 1 Apr 2025 15:50:44 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:14c7:4fcc:495b:719f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 302D33E;\n\tTue, 1 Apr 2025 15:48:52 +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=\"F6WH5a4Y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743515332;\n\tbh=XsUhNBm5+C2uZF/c/uOluoX2x7hm8k9SoGuxGlxYCX0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=F6WH5a4YcpIR5RyHkz+0g5o+yQwZlhbRWezGyTOqeHqaT21IlI7q2XqySu3PxCBor\n\tiQ2dknZDV2OzVeLMPDIWfza0OAqS90+xBrxERjWbec1y4a3gyOoJogKWeMXt6F1e+D\n\taRkSCc4CsVQqWHWN5OajoQ5xYqoq1eOMdZtajWsk=","Date":"Tue, 1 Apr 2025 15:50:41 +0200","From":"Stefan Klug ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 02/17] libcamera: matrix: Make most functions\n\tconstexpr","Message-ID":"<5litsybba5l7swnltuok5wrirrwxzgyjkpabu2fhxswqbbrmfb@4opkw5ijnfyc>","References":"<20250319161152.63625-1-stefan.klug@ideasonboard.com>\n\t<20250319161152.63625-3-stefan.klug@ideasonboard.com>\n\t<20250401005957.GP14432@pendragon.ideasonboard.com>\n\t<20250401010824.GA20312@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250401010824.GA20312@pendragon.ideasonboard.com>","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\" "}},{"id":33851,"web_url":"https://patchwork.libcamera.org/comment/33851/","msgid":"<20250401135356.GD11469@pendragon.ideasonboard.com>","date":"2025-04-01T13:53:56","subject":"Re: [PATCH v2 02/17] libcamera: matrix: Make most functions\n\tconstexpr","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Apr 01, 2025 at 03:50:41PM +0200, Stefan Klug wrote:\n> On Tue, Apr 01, 2025 at 04:08:24AM +0300, Laurent Pinchart wrote:\n> > On Tue, Apr 01, 2025 at 03:59:57AM +0300, Laurent Pinchart wrote:\n> > > Hi Stefan,\n> > > \n> > > Thank you for the patch.\n> > > \n> > > On Wed, Mar 19, 2025 at 05:11:07PM +0100, Stefan Klug wrote:\n> > > > By zero-initializing the data_ member we can make most functions\n> > > > constexpr which will come in handy in upcoming patches. Note that this\n> > > > is due to C++17. In C++20 we will be able to leave data_ unintialized\n> > > \n> > > s/unintialized/uninitialized/\n> > > \n> > > Given that we only operate on small matrices I think this is acceptable.\n> > > Please add a todo comment to indicate that initialization of data should\n> > > be removed with C++20.\n> > > \n> > > > for constexpr. The Matrix(std::array) version of the constructor can\n> > > > not be constexpr because std::copy only became constexpr in C++20.\n> > \n> > Actually, would open-coding std::copy allow making the constructor\n> > constexpr ? If subsequent patches should be updated accordingly.\n> \n> I believe yes. I didn't bother to do that as we don't have any user of\n> that constructor and it is unclear what comes first. A constexpr usage\n> of that constructor or us switching to C++20. While I write that: What\n> about leaving the constexpr here as we have other places where we label\n> things as constexpr that are actually not constexpr. So the intent would\n> be clear, just that it doesn't work in C++17 :-)\n\nWorks for me. Whoever uses the constructor first in C++17 will have to\ndeal with the situation :-)\n\n> > > > Signed-off-by: Stefan Klug \n> > > > \n> > > > ---\n> > > > \n> > > > Changes in v2:\n> > > > - Added this patch\n> > > > ---\n> > > > include/libcamera/internal/matrix.h | 15 +++++++--------\n> > > > 1 file changed, 7 insertions(+), 8 deletions(-)\n> > > > \n> > > > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h\n> > > > index 8399be583f28..2e7929e9060c 100644\n> > > > --- a/include/libcamera/internal/matrix.h\n> > > > +++ b/include/libcamera/internal/matrix.h\n> > > > @@ -25,9 +25,8 @@ class Matrix\n> > > > \tstatic_assert(std::is_arithmetic_v, \"Matrix type must be arithmetic\");\n> > > > \n> > > > public:\n> > > > -\tMatrix()\n> > > > +\tconstexpr Matrix()\n> > > > \t{\n> > > > -\t\tdata_.fill(static_cast(0));\n> > > > \t}\n> > > > \n> > > > \tMatrix(const std::array &data)\n> > > > @@ -35,7 +34,7 @@ public:\n> > > > \t\tstd::copy(data.begin(), data.end(), data_.begin());\n> > > > \t}\n> > > > \n> > > > -\tstatic Matrix identity()\n> > > > +\tstatic constexpr Matrix identity()\n> > > > \t{\n> > > > \t\tMatrix ret;\n> > > > \t\tfor (size_t i = 0; i < std::min(Rows, Cols); i++)\n> > > > @@ -63,14 +62,14 @@ public:\n> > > > \t\treturn out.str();\n> > > > \t}\n> > > > \n> > > > -\tSpan data() const { return data_; }\n> > > > +\tconstexpr Span data() const { return data_; }\n> > > > \n> > > > -\tSpan operator[](size_t i) const\n> > > > +\tconstexpr Span operator[](size_t i) const\n> > > > \t{\n> > > > \t\treturn Span{ &data_.data()[i * Cols], Cols };\n> > > > \t}\n> > > > \n> > > > -\tSpan operator[](size_t i)\n> > > > +\tconstexpr Span operator[](size_t i)\n> > > > \t{\n> > > > \t\treturn Span{ &data_.data()[i * Cols], Cols };\n> > > > \t}\n> > > > @@ -85,7 +84,7 @@ public:\n> > > > \t}\n> > > > \n> > > > private:\n> > > > -\tstd::array data_;\n> > > > +\tstd::array data_{};\n> > > \n> > > We usually write\n> > > \n> > > \tstd::array data_ = {};\n> > > \n> > > With that and the comment,\n> > > \n> > > Reviewed-by: Laurent Pinchart \n> > > \n> > > > };\n> > > > \n> > > > template\n> > > > @@ -130,7 +129,7 @@ constexpr Matrix operator*(const Matrix &m1, const Matrix<\n> > > > }\n> > > > \n> > > > template\n> > > > -Matrix operator+(const Matrix &m1, const Matrix &m2)\n> > > > +constexpr Matrix operator+(const Matrix &m1, const Matrix &m2)\n> > > > {\n> > > > \tMatrix result;\n> > > >","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 36A6CC3213\n\tfor ;\n\tTue, 1 Apr 2025 13:54:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E64C068962;\n\tTue, 1 Apr 2025 15:54:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 18B1568947\n\tfor ;\n\tTue, 1 Apr 2025 15:54:21 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D06E6741;\n\tTue, 1 Apr 2025 15:52:28 +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=\"BeaLEtZ4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743515549;\n\tbh=25NywHmEqE7z7eLD5Rw7etMnIoDa/e8V/5DfszWREms=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BeaLEtZ4Lmv8Pmd5MZNb33hZRWVDwLAcTLmy+9DRmXjpNMVjO088Tq0zdgSeGDdwp\n\tpPtm5S7q6r2wDolz8kSHJ72H2m/DS07/ilTYdKdj6iJXn1QJ2LM64tvgf+r31xC2mF\n\t7XxvtGQcLNSn1g4w+Npzf8l+eWzujMW5yMlfybWU=","Date":"Tue, 1 Apr 2025 16:53:56 +0300","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 02/17] libcamera: matrix: Make most functions\n\tconstexpr","Message-ID":"<20250401135356.GD11469@pendragon.ideasonboard.com>","References":"<20250319161152.63625-1-stefan.klug@ideasonboard.com>\n\t<20250319161152.63625-3-stefan.klug@ideasonboard.com>\n\t<20250401005957.GP14432@pendragon.ideasonboard.com>\n\t<20250401010824.GA20312@pendragon.ideasonboard.com>\n\t<5litsybba5l7swnltuok5wrirrwxzgyjkpabu2fhxswqbbrmfb@4opkw5ijnfyc>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<5litsybba5l7swnltuok5wrirrwxzgyjkpabu2fhxswqbbrmfb@4opkw5ijnfyc>","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\" "}}]