[{"id":32211,"web_url":"https://patchwork.libcamera.org/comment/32211/","msgid":"<8734jokdxu.fsf@redhat.com>","date":"2024-11-18T10:56:45","subject":"Re: [RFC PATCH v2 01/12] ipa: libipa: vector: Add mutable x(), y()\n\tand z() accessors","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:\n\n> The x(), y() and z() functions of the Vector class are convenience\n> accessors for the first, second and third element of the vector\n> respectively, meant to improve readability of class users when a vector\n> represents coordinates in 1D, 2D or 3D space. Those accessors are\n> limited to immutable access to the vector elements, as they return a\n> copy. Extend the API with mutable accessors.\n>\n> The immutable accessors are modified to return a reference to the vector\n> elements instead of a copy for consistency. As they are inline\n> functions, this should make no difference in terms of performance as the\n> compiler can perform the same optimizations in their case.\n>\n> While at it, reorder functions to declare operators before other member\n> functions, to be consistent with the usual coding style.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Milan Zamazal <mzamazal@redhat.com>\n\n> ---\n>  src/ipa/libipa/vector.cpp | 51 +++++++++++++++++++++++++--------------\n>  src/ipa/libipa/vector.h   | 49 +++++++++++++++++++------------------\n>  2 files changed, 58 insertions(+), 42 deletions(-)\n>\n> diff --git a/src/ipa/libipa/vector.cpp b/src/ipa/libipa/vector.cpp\n> index bd00b01961d5..8a39bfd27f90 100644\n> --- a/src/ipa/libipa/vector.cpp\n> +++ b/src/ipa/libipa/vector.cpp\n> @@ -52,24 +52,6 @@ namespace ipa {\n>   * \\copydoc Vector::operator[](size_t i) const\n>   */\n>  \n> -/**\n> - * \\fn Vector::x()\n> - * \\brief Convenience function to access the first element of the vector\n> - * \\return The first element of the vector\n> - */\n> -\n> -/**\n> - * \\fn Vector::y()\n> - * \\brief Convenience function to access the second element of the vector\n> - * \\return The second element of the vector\n> - */\n> -\n> -/**\n> - * \\fn Vector::z()\n> - * \\brief Convenience function to access the third element of the vector\n> - * \\return The third element of the vector\n> - */\n> -\n>  /**\n>   * \\fn Vector::operator-() const\n>   * \\brief Negate a Vector by negating both all of its coordinates\n> @@ -111,6 +93,39 @@ namespace ipa {\n>   * \\return The vector divided by \\a factor\n>   */\n>  \n> +/**\n> + * \\fn T &Vector::x()\n> + * \\brief Convenience function to access the first element of the vector\n> + * \\return The first element of the vector\n> + */\n> +\n> +/**\n> + * \\fn T &Vector::y()\n> + * \\brief Convenience function to access the second element of the vector\n> + * \\return The second element of the vector\n> + */\n> +\n> +/**\n> + * \\fn T &Vector::z()\n> + * \\brief Convenience function to access the third element of the vector\n> + * \\return The third element of the vector\n> + */\n> +\n> +/**\n> + * \\fn constexpr const T &Vector::x() const\n> + * \\copydoc Vector::x()\n> + */\n> +\n> +/**\n> + * \\fn constexpr const T &Vector::y() const\n> + * \\copydoc Vector::y()\n> + */\n> +\n> +/**\n> + * \\fn constexpr const T &Vector::z() const\n> + * \\copydoc Vector::z()\n> + */\n> +\n>  /**\n>   * \\fn Vector::length2()\n>   * \\brief Get the squared length of the vector\n> diff --git a/src/ipa/libipa/vector.h b/src/ipa/libipa/vector.h\n> index 8612a06a2ab2..1b11a34deee4 100644\n> --- a/src/ipa/libipa/vector.h\n> +++ b/src/ipa/libipa/vector.h\n> @@ -53,30 +53,6 @@ public:\n>  \t\treturn data_[i];\n>  \t}\n>  \n> -#ifndef __DOXYGEN__\n> -\ttemplate<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 1>>\n> -#endif /* __DOXYGEN__ */\n> -\tconstexpr T x() const\n> -\t{\n> -\t\treturn data_[0];\n> -\t}\n> -\n> -#ifndef __DOXYGEN__\n> -\ttemplate<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 2>>\n> -#endif /* __DOXYGEN__ */\n> -\tconstexpr T y() const\n> -\t{\n> -\t\treturn data_[1];\n> -\t}\n> -\n> -#ifndef __DOXYGEN__\n> -\ttemplate<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 3>>\n> -#endif /* __DOXYGEN__ */\n> -\tconstexpr T z() const\n> -\t{\n> -\t\treturn data_[2];\n> -\t}\n> -\n>  \tconstexpr Vector<T, Rows> operator-() const\n>  \t{\n>  \t\tVector<T, Rows> ret;\n> @@ -125,6 +101,31 @@ public:\n>  \t\treturn ret;\n>  \t}\n>  \n> +#ifndef __DOXYGEN__\n> +\ttemplate<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 1>>\n> +#endif /* __DOXYGEN__ */\n> +\tconstexpr const T &x() const { return data_[0]; }\n> +#ifndef __DOXYGEN__\n> +\ttemplate<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 2>>\n> +#endif /* __DOXYGEN__ */\n> +\tconstexpr const T &y() const { return data_[1]; }\n> +#ifndef __DOXYGEN__\n> +\ttemplate<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 3>>\n> +#endif /* __DOXYGEN__ */\n> +\tconstexpr const T &z() const { return data_[2]; }\n> +#ifndef __DOXYGEN__\n> +\ttemplate<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 1>>\n> +#endif /* __DOXYGEN__ */\n> +\tT &x() { return data_[0]; }\n> +#ifndef __DOXYGEN__\n> +\ttemplate<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 2>>\n> +#endif /* __DOXYGEN__ */\n> +\tT &y() { return data_[1]; }\n> +#ifndef __DOXYGEN__\n> +\ttemplate<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 3>>\n> +#endif /* __DOXYGEN__ */\n> +\tT &z() { return data_[2]; }\n> +\n>  \tconstexpr double length2() const\n>  \t{\n>  \t\tdouble ret = 0;","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 67A79C32DD\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 18 Nov 2024 10:56:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4A350658C3;\n\tMon, 18 Nov 2024 11:56:55 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 845F4618BC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 18 Nov 2024 11:56:53 +0100 (CET)","from mail-ej1-f71.google.com (mail-ej1-f71.google.com\n\t[209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-607-aZemBxbTOai5TCvQ-AE_2g-1; Mon, 18 Nov 2024 05:56:50 -0500","by mail-ej1-f71.google.com with SMTP id\n\ta640c23a62f3a-a9e0574854dso262729566b.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 18 Nov 2024 02:56:49 -0800 (PST)","from nuthatch ([2a00:102a:400a:489a:ffe9:aa41:63cd:fc2b])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-aa20e086adfsm526807466b.197.2024.11.18.02.56.47\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 18 Nov 2024 02:56:47 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"LPY68ktj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1731927412;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=ZuQ5syKbeyV8zogBa5tgDe4JqusH8si5I2rHUMzX3fA=;\n\tb=LPY68ktjQMELSME5ff5N/+1N25254c2TbB+U+G2Fe2QyE2ch2zZkr00MJIJMbFiMoi04SO\n\tsjtl/iutum/lsMHmpIqxjAOkVyECEXJzpnZeDRkPVOPsAnqhzz2RTRsD8BvouGEjHmTDcD\n\t5ImmgYO0QVzS7U+VW7uXvWTaHpgz4oA=","X-MC-Unique":"aZemBxbTOai5TCvQ-AE_2g-1","X-Mimecast-MFC-AGG-ID":"aZemBxbTOai5TCvQ-AE_2g","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1731927408; x=1732532208;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=ZuQ5syKbeyV8zogBa5tgDe4JqusH8si5I2rHUMzX3fA=;\n\tb=q7w++XtFEh2CoV/XXV2ThClwefiWFI/AfEMXeNmoXjahJQext3UWctjFCh2ELKU/vC\n\tyHZlRFtCU6Zw6IQjLDUQXp9OaLYmGkakEiVcBjuZQbdvS4HSV1V0/CwkYZpAt7RQoXDB\n\tBVdwIpMy2U3/VWe54ZEhUM0myBExjRBsNhSIjbu8hKhVw79HNvZnI4uYOjBVVL+2HjVw\n\tR+ZGd7tVh+qwraGAUR8HKroQpXk/iMmrzoffjMkj87QhUjeOu4mLhXk5s8Gk484OTMcY\n\t9G1OcB0vHpb8GMBdZTEQykV3YlJ0ZWEB6PeT/Q7ejhlRtZBSlW0uCMXTWHwy0Lr2kQfv\n\tjPfg==","X-Gm-Message-State":"AOJu0YxMWc/DsphWiDz6Yy+Xy80Ugu9pyWqfRcGSwQQlClAuytHfao6C\n\trS5eLI79LxjCNd8KbC2LA31dNHF0NDYvnGAjMz0VgoRNrrKEz0lkdM4+M0TFrhEStE017nKw2PS\n\t+7BIiv+C6L+siQIrh1cPaYNYq40kkyMPgEXsv3/YMbDDS4YfFM7tEYRapNJCtbCa8SwoHuCpOHt\n\tl3mSCML5rMzxvKCXpDA/XZQ8PzeYA3g2QZDHVmZDlIPwv1sSfiNq1vfCA=","X-Received":["by 2002:a17:907:2d0a:b0:a99:e82a:87ee with SMTP id\n\ta640c23a62f3a-aa48354d154mr1255996566b.57.1731927408411; \n\tMon, 18 Nov 2024 02:56:48 -0800 (PST)","by 2002:a17:907:2d0a:b0:a99:e82a:87ee with SMTP id\n\ta640c23a62f3a-aa48354d154mr1255994266b.57.1731927408057; \n\tMon, 18 Nov 2024 02:56:48 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IF4TyqZG5ACAZMOEPkfPXe1fBRZnjBtFqW9fAPJjmgddMYnyyCgvBRZRfEFBHexgvh1lqH+4A==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 01/12] ipa: libipa: vector: Add mutable x(), y()\n\tand z() accessors","In-Reply-To":"<20241118000738.18977-2-laurent.pinchart@ideasonboard.com>\n\t(Laurent Pinchart's message of \"Mon, 18 Nov 2024 02:07:27 +0200\")","References":"<20241118000738.18977-1-laurent.pinchart@ideasonboard.com>\n\t<20241118000738.18977-2-laurent.pinchart@ideasonboard.com>","Date":"Mon, 18 Nov 2024 11:56:45 +0100","Message-ID":"<8734jokdxu.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"BcmaqFf9sjcczAeRfBZ9WP6uYgrcKz9Vyp7xBJJki4s_1731927409","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]