[{"id":31090,"web_url":"https://patchwork.libcamera.org/comment/31090/","msgid":"","date":"2024-09-04T11:26:37","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang írta:\n\n> From: Yudhistira Erlandinata \n> \n> Add a Rectangle constructor that accepts two points:\n> topLeft and bottomRight.\n> \n> Signed-off-by: Yudhistira Erlandinata \n> Co-developed-by: Harvey Yang \n> Reviewed-by: Jacopo Mondi \n> ---\n> include/libcamera/geometry.h | 2 ++\n> src/libcamera/geometry.cpp | 14 ++++++++++++++\n> 2 files changed, 16 insertions(+)\n> \n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index 3e6f0f5d7..dc56f180f 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -262,6 +262,8 @@ public:\n> \t{\n> \t}\n> \n> +\tconstexpr Rectangle(const Point &topLeft, const Point &bottomRight);\n\nDon't make this `constexpr` because it is not useful since the definition is not available.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> +\n> \tint x;\n> \tint y;\n> \tunsigned int width;\n> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> index 000151364..029b8dad2 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -629,6 +629,20 @@ std::ostream &operator<<(std::ostream &out, const SizeRange &sr)\n> * \\param[in] size The desired Rectangle size\n> */\n> \n> +/**\n> + * \\fn Rectangle::Rectangle(const Point &topLeft, const Point &bottomRight)\n> + * \\brief Construct a Rectangle with the two given points\n> + * \\param[in] topLeft The top-left corner\n> + * \\param[in] bottomRight The bottom-right corner\n> + */\n> +constexpr Rectangle::Rectangle(const Point &topLeft, const Point &bottomRight)\n> +\t: x(topLeft.x), y(topLeft.y),\n> +\t width(bottomRight.x - x),\n> +\t height(bottomRight.y - y)\n> +{\n> +\tASSERT(bottomRight.x >= x && bottomRight.y >= y);\n> +}\n> +\n> /**\n> * \\var Rectangle::x\n> * \\brief The horizontal coordinate of the rectangle's top-left corner\n> --\n> 2.46.0.469.g59c65b2a67-goog\n> \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 80B94C324C\n\tfor ;\n\tWed, 4 Sep 2024 11:26:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9507C634E4;\n\tWed, 4 Sep 2024 13:26:43 +0200 (CEST)","from mail-4316.protonmail.ch (mail-4316.protonmail.ch\n\t[185.70.43.16])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0C3E618FE\n\tfor ;\n\tWed, 4 Sep 2024 13:26:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"IgmOE5VT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1725449201; x=1725708401;\n\tbh=ggcJAW4W+QO4DiKqM1uYS3O3KfNyT/1bQ5eeYLqC818=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=IgmOE5VTmtuCsg2eT7PSr2aSQYalKPKjg6d9n15A3Fil4qD7DFTJ6NuSN6b8XZPql\n\tl/CSe+Piotgf8YYepv0ojFn423J0Eaox0Exgele7RSqvBG7o+pqIhwsjE0xtWmL/ut\n\tnWQS6GBca4HNqVPHWzXu3HzCEBknxkgdnx2jxzW+bMpyVp6+Hb0WE74vN+q/tmAUMo\n\t5XxCzVLXpmKP4WbmrkytvohIMlumZItVs+ylH6L73KOLSlXyMDEcJtGgFpmsoejD3I\n\tnPRwBSonDacTy4/1wcJpW0qHLZVvpYPI4lonzFgFO75+SM62xkdYQ6A876Utn0fW5W\n\tjCb/Ifl5aw92w==","Date":"Wed, 04 Sep 2024 11:26:37 +0000","To":"Harvey Yang ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org,\n\tYudhistira Erlandinata ,\n\tJacopo Mondi ","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","Message-ID":"","In-Reply-To":"<20240903104018.3289112-2-chenghaoyang@chromium.org>","References":"<20240903104018.3289112-1-chenghaoyang@chromium.org>\n\t<20240903104018.3289112-2-chenghaoyang@chromium.org>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"4443ba0f57601faff10b94c2752d9525c0b3e631","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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":31091,"web_url":"https://patchwork.libcamera.org/comment/31091/","msgid":"","date":"2024-09-05T04:11:32","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Thanks Barnabás,\n\nUpdated on gitlab:\nhttps://gitlab.freedesktop.org/chenghaoyang/libcamera/-/pipelines/1264782\nWill be included in v5.\n\nBR,\nHarvey\n\nOn Wed, Sep 4, 2024 at 7:26 PM Barnabás Pőcze wrote:\n\n> Hi\n>\n>\n> 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang <\n> chenghaoyang@chromium.org> írta:\n>\n> > From: Yudhistira Erlandinata \n> >\n> > Add a Rectangle constructor that accepts two points:\n> > topLeft and bottomRight.\n> >\n> > Signed-off-by: Yudhistira Erlandinata \n> > Co-developed-by: Harvey Yang \n> > Reviewed-by: Jacopo Mondi \n> > ---\n> > include/libcamera/geometry.h | 2 ++\n> > src/libcamera/geometry.cpp | 14 ++++++++++++++\n> > 2 files changed, 16 insertions(+)\n> >\n> > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > index 3e6f0f5d7..dc56f180f 100644\n> > --- a/include/libcamera/geometry.h\n> > +++ b/include/libcamera/geometry.h\n> > @@ -262,6 +262,8 @@ public:\n> > {\n> > }\n> >\n> > + constexpr Rectangle(const Point &topLeft, const Point\n> &bottomRight);\n>\n> Don't make this `constexpr` because it is not useful since the definition\n> is not available.\n>\n>\n> Regards,\n> Barnabás Pőcze\n>\n>\n> > +\n> > int x;\n> > int y;\n> > unsigned int width;\n> > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> > index 000151364..029b8dad2 100644\n> > --- a/src/libcamera/geometry.cpp\n> > +++ b/src/libcamera/geometry.cpp\n> > @@ -629,6 +629,20 @@ std::ostream &operator<<(std::ostream &out, const\n> SizeRange &sr)\n> > * \\param[in] size The desired Rectangle size\n> > */\n> >\n> > +/**\n> > + * \\fn Rectangle::Rectangle(const Point &topLeft, const Point\n> &bottomRight)\n> > + * \\brief Construct a Rectangle with the two given points\n> > + * \\param[in] topLeft The top-left corner\n> > + * \\param[in] bottomRight The bottom-right corner\n> > + */\n> > +constexpr Rectangle::Rectangle(const Point &topLeft, const Point\n> &bottomRight)\n> > + : x(topLeft.x), y(topLeft.y),\n> > + width(bottomRight.x - x),\n> > + height(bottomRight.y - y)\n> > +{\n> > + ASSERT(bottomRight.x >= x && bottomRight.y >= y);\n> > +}\n> > +\n> > /**\n> > * \\var Rectangle::x\n> > * \\brief The horizontal coordinate of the rectangle's top-left corner\n> > --\n> > 2.46.0.469.g59c65b2a67-goog\n> >\n> >\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 CE9E1BF415\n\tfor ;\n\tThu, 5 Sep 2024 04:11:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A3E4A634E4;\n\tThu, 5 Sep 2024 06:11:47 +0200 (CEST)","from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com\n\t[IPv6:2a00:1450:4864:20::22e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 64C82618FA\n\tfor ;\n\tThu, 5 Sep 2024 06:11:45 +0200 (CEST)","by mail-lj1-x22e.google.com with SMTP id\n\t38308e7fff4ca-2f50966c448so3188021fa.2\n\tfor ;\n\tWed, 04 Sep 2024 21:11:45 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"TVrB8Fvf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1725509504; x=1726114304;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=JzLysJOEDjKbgoOPByy+4CKA6OmILADYsxr/VaAuZmA=;\n\tb=TVrB8FvfMZNOCVZeqzaPAlTuPkL59hm1206ZXt2WzZBfnTs38IwTjqq59XwsHvqR+4\n\tSfbxwNoX5zgpaGKAslPtKfd5gaQbcdb35yKtKjPPF/5+50keSmpU9TaKLZ+utTiLJSQp\n\tFWHgAbiVRK0VUCARalg/p2yrj0ZZNMtgzRlmg=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1725509504; x=1726114304;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=JzLysJOEDjKbgoOPByy+4CKA6OmILADYsxr/VaAuZmA=;\n\tb=KZRaWRGzIMD1X1Wm5ttSRTOFZ/6FAzOCesy94GCIMVFyjwvElifWZV/vPHfdNOE7s6\n\tgJlRmIb3cZYnOWCJgkX8UWQ1h4WWktvCQkeAVBKZdpMsf2/J2jUtvkzF6SsmpiNkD2v7\n\t8JVb97AidPwqyLnrC94F72RCMmH9L9a24ky9c24iRgmFi9Ch/kqOlp+CmVmYBi67zPKR\n\tbZoLsApMF5ltfLRqi7SXJZROuSCxmsNkaXqMzo+raNmARm2apMmzeKISDcHS/VLliWsz\n\tJF9hnekdIFK18VS/NBVU2flQvPmFkkhEojUnls4Q7NkCl34ZkSGKqs5UCS0vh2qT7qsQ\n\tw9/Q==","X-Gm-Message-State":"AOJu0Yw5JBqlHNizNA6ZgR7fBnNekc6++uub5XmVM4A0vx2YxCIfqh0r\n\t+YixtvmsAEVrdCa+QKAlnoMMJWtjR+aJlvkZkU9oTvD/p9AEjB0ejBZEsaxCxpxvW8/uXJ+i40m\n\tEJzI37+CRuBvPyCcrANjbH707woBONrDyusCP","X-Google-Smtp-Source":"AGHT+IFZKVXiqd/MUY5BMvtb8KLvQfMXJf0cquZQET0lo7SNCaBpCm4oKCzRC/24D8FHls/h3fGXXwa91Il/9H7Mlu8=","X-Received":"by 2002:a2e:a9a1:0:b0:2f3:d008:a54e with SMTP id\n\t38308e7fff4ca-2f6104f2859mr162135711fa.36.1725509503976;\n\tWed, 04 Sep 2024 21:11:43 -0700 (PDT)","MIME-Version":"1.0","References":"<20240903104018.3289112-1-chenghaoyang@chromium.org>\n\t<20240903104018.3289112-2-chenghaoyang@chromium.org>\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Thu, 5 Sep 2024 12:11:32 +0800","Message-ID":"","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ,\n\tJacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"0000000000000aa38a0621577d31\"","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":31092,"web_url":"https://patchwork.libcamera.org/comment/31092/","msgid":"<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>","date":"2024-09-05T08:35:06","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Wed, Sep 04, 2024 at 11:26:37AM GMT, Barnabás Pőcze wrote:\n> Hi\n>\n>\n> 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang írta:\n>\n> > From: Yudhistira Erlandinata \n> >\n> > Add a Rectangle constructor that accepts two points:\n> > topLeft and bottomRight.\n> >\n> > Signed-off-by: Yudhistira Erlandinata \n> > Co-developed-by: Harvey Yang \n> > Reviewed-by: Jacopo Mondi \n> > ---\n> > include/libcamera/geometry.h | 2 ++\n> > src/libcamera/geometry.cpp | 14 ++++++++++++++\n> > 2 files changed, 16 insertions(+)\n> >\n> > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > index 3e6f0f5d7..dc56f180f 100644\n> > --- a/include/libcamera/geometry.h\n> > +++ b/include/libcamera/geometry.h\n> > @@ -262,6 +262,8 @@ public:\n> > \t{\n> > \t}\n> >\n> > +\tconstexpr Rectangle(const Point &topLeft, const Point &bottomRight);\n>\n> Don't make this `constexpr` because it is not useful since the definition is not available.\n\nI found references online that constexpr constuctors are implcitly\ninline, is this the reason of your comment ?\n\nHowever, I can't find it clearly specified in cppreference. Do you\nhave any pointer ?\n\nAnyway, if inline is the reason, isn't it better to inline the\ndefinition and maintain the constexpr specifier ?\n\n>\n>\n> Regards,\n> Barnabás Pőcze\n>\n>\n> > +\n> > \tint x;\n> > \tint y;\n> > \tunsigned int width;\n> > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> > index 000151364..029b8dad2 100644\n> > --- a/src/libcamera/geometry.cpp\n> > +++ b/src/libcamera/geometry.cpp\n> > @@ -629,6 +629,20 @@ std::ostream &operator<<(std::ostream &out, const SizeRange &sr)\n> > * \\param[in] size The desired Rectangle size\n> > */\n> >\n> > +/**\n> > + * \\fn Rectangle::Rectangle(const Point &topLeft, const Point &bottomRight)\n> > + * \\brief Construct a Rectangle with the two given points\n> > + * \\param[in] topLeft The top-left corner\n> > + * \\param[in] bottomRight The bottom-right corner\n> > + */\n> > +constexpr Rectangle::Rectangle(const Point &topLeft, const Point &bottomRight)\n> > +\t: x(topLeft.x), y(topLeft.y),\n> > +\t width(bottomRight.x - x),\n> > +\t height(bottomRight.y - y)\n> > +{\n> > +\tASSERT(bottomRight.x >= x && bottomRight.y >= y);\n> > +}\n> > +\n> > /**\n> > * \\var Rectangle::x\n> > * \\brief The horizontal coordinate of the rectangle's top-left corner\n> > --\n> > 2.46.0.469.g59c65b2a67-goog\n> >\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 39BF9C324C\n\tfor ;\n\tThu, 5 Sep 2024 08:35:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2E3B9634A6;\n\tThu, 5 Sep 2024 10:35:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E7BE63471\n\tfor ;\n\tThu, 5 Sep 2024 10:35:10 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C9B033E6;\n\tThu, 5 Sep 2024 10:33: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=\"cA8LHnio\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725525236;\n\tbh=SoOnoflKq4OPTUhArKsMM36VQQz6rO0oLpAHCTzB51g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cA8LHnioeRn6EXwncstvgPALIIyEEw3UllztKcAMXebPJdEft9D82iipzRAV33nRF\n\t9AezGWmjgmpjEw+7+OpSr6DSb8eZ6aWqe1RA7k5xMhLqLasLXN/CBWma5VJpS6MI6B\n\tcrAGUlCkQR6qs6y8CZVx9VtyK9B3nCHzifyCEAVc=","Date":"Thu, 5 Sep 2024 10:35:06 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Harvey Yang , \n\tlibcamera-devel@lists.libcamera.org,\n\tYudhistira Erlandinata , \n\tJacopo Mondi ","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","Message-ID":"<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>","References":"<20240903104018.3289112-1-chenghaoyang@chromium.org>\n\t<20240903104018.3289112-2-chenghaoyang@chromium.org>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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":31233,"web_url":"https://patchwork.libcamera.org/comment/31233/","msgid":"","date":"2024-09-15T17:27:45","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. szeptember 5., csütörtök 10:35 keltezéssel, Jacopo Mondi írta:\n\n> Hi Barnabás\n> \n> On Wed, Sep 04, 2024 at 11:26:37AM GMT, Barnabás Pőcze wrote:\n> > Hi\n> >\n> >\n> > 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang írta:\n> >\n> > > From: Yudhistira Erlandinata \n> > >\n> > > Add a Rectangle constructor that accepts two points:\n> > > topLeft and bottomRight.\n> > >\n> > > Signed-off-by: Yudhistira Erlandinata \n> > > Co-developed-by: Harvey Yang \n> > > Reviewed-by: Jacopo Mondi \n> > > ---\n> > > include/libcamera/geometry.h | 2 ++\n> > > src/libcamera/geometry.cpp | 14 ++++++++++++++\n> > > 2 files changed, 16 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > > index 3e6f0f5d7..dc56f180f 100644\n> > > --- a/include/libcamera/geometry.h\n> > > +++ b/include/libcamera/geometry.h\n> > > @@ -262,6 +262,8 @@ public:\n> > > \t{\n> > > \t}\n> > >\n> > > +\tconstexpr Rectangle(const Point &topLeft, const Point &bottomRight);\n> >\n> > Don't make this `constexpr` because it is not useful since the definition is not available.\n> \n> I found references online that constexpr constuctors are implcitly\n> inline, is this the reason of your comment ?\n\nYes.\n\n\n> \n> However, I can't find it clearly specified in cppreference. Do you\n> have any pointer ?\n\n \"A constexpr specifier used in a function or static data member(since C++17) declaration implies inline.\"\n -- https://en.cppreference.com/w/cpp/language/constexpr\n\n> \n> Anyway, if inline is the reason, isn't it better to inline the\n> definition and maintain the constexpr specifier ?\n> [...]\n\nThat is an option as well.\n\n\nRegards,\nBarnabás Pőcze","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 C06DEC3257\n\tfor ;\n\tSun, 15 Sep 2024 17:27:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2BAD7634FC;\n\tSun, 15 Sep 2024 19:27:53 +0200 (CEST)","from mail-40134.protonmail.ch (mail-40134.protonmail.ch\n\t[185.70.40.134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5A85E634F4\n\tfor ;\n\tSun, 15 Sep 2024 19:27:51 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"USvrxIFj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1726421270; x=1726680470;\n\tbh=jBi8AN1HXnl0Zo0V5Nwh/LB16qCAz1abIX4zIJHBcN0=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=USvrxIFjxU32GOjSKGSbyy/7D3VJyYQntedIzY6hRCSLcne/KRFlt++veQKOZK9XQ\n\ty4ZwigTWvLXJUDcc3oJKzCWBVSEK5vK1BsxMVkPHmVgm3ahh266DtBYgv6vttGszJi\n\tXJpdZoEkQiEkIuvHDnLKKN4lp6R9BE2RaW6peN4p6LCjjHzEHbsfbcrAH2LWFqZCLH\n\tKzNJJDNd7ZMEOaWyEmLgOsU+sTQyn77l9ikr2uS1gnCFAg9Ya53aMnxoeFwPxDt0HQ\n\toYWLCbYAER3ZP+JBHqpMcWXFsNnnpzReIPFp6pEwtjdT3EoTbvV6MJdvZG1iFhRRvj\n\txw/p57nEX5NnA==","Date":"Sun, 15 Sep 2024 17:27:45 +0000","To":"Jacopo Mondi ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org,\n\tYudhistira Erlandinata ","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","Message-ID":"","In-Reply-To":"<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>","References":"<20240903104018.3289112-1-chenghaoyang@chromium.org>\n\t<20240903104018.3289112-2-chenghaoyang@chromium.org>\n\t\n\t<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"81ef96e70d243152929fc1f4fb2bdef3e68feda8","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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":31234,"web_url":"https://patchwork.libcamera.org/comment/31234/","msgid":"","date":"2024-09-16T04:34:54","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Barnabás and Jacopo,\n\nOn Mon, Sep 16, 2024 at 1:27 AM Barnabás Pőcze wrote:\n\n> Hi\n>\n>\n> 2024. szeptember 5., csütörtök 10:35 keltezéssel, Jacopo Mondi <\n> jacopo.mondi@ideasonboard.com> írta:\n>\n> > Hi Barnabás\n> >\n> > On Wed, Sep 04, 2024 at 11:26:37AM GMT, Barnabás Pőcze wrote:\n> > > Hi\n> > >\n> > >\n> > > 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang <\n> chenghaoyang@chromium.org> írta:\n> > >\n> > > > From: Yudhistira Erlandinata \n> > > >\n> > > > Add a Rectangle constructor that accepts two points:\n> > > > topLeft and bottomRight.\n> > > >\n> > > > Signed-off-by: Yudhistira Erlandinata \n> > > > Co-developed-by: Harvey Yang \n> > > > Reviewed-by: Jacopo Mondi \n> > > > ---\n> > > > include/libcamera/geometry.h | 2 ++\n> > > > src/libcamera/geometry.cpp | 14 ++++++++++++++\n> > > > 2 files changed, 16 insertions(+)\n> > > >\n> > > > diff --git a/include/libcamera/geometry.h\n> b/include/libcamera/geometry.h\n> > > > index 3e6f0f5d7..dc56f180f 100644\n> > > > --- a/include/libcamera/geometry.h\n> > > > +++ b/include/libcamera/geometry.h\n> > > > @@ -262,6 +262,8 @@ public:\n> > > > {\n> > > > }\n> > > >\n> > > > + constexpr Rectangle(const Point &topLeft, const Point\n> &bottomRight);\n> > >\n> > > Don't make this `constexpr` because it is not useful since the\n> definition is not available.\n> >\n> > I found references online that constexpr constuctors are implcitly\n> > inline, is this the reason of your comment ?\n>\n> Yes.\n>\n>\n> >\n> > However, I can't find it clearly specified in cppreference. Do you\n> > have any pointer ?\n>\n> \"A constexpr specifier used in a function or static data member(since\n> C++17) declaration implies inline.\"\n> -- https://en.cppreference.com/w/cpp/language/constexpr\n>\n> >\n> > Anyway, if inline is the reason, isn't it better to inline the\n> > definition and maintain the constexpr specifier ?\n> > [...]\n>\n> That is an option as well.\n>\n\nIIUC, you're suggesting to put the definition of the new c'tor\nback in the header file? As we use `ASSERT` in this c'tor\ndefinition, there will be a compile error if we do that:\n```\n\nIn file included from ../include/libcamera/base/log.h:12,\n\n from ../include/libcamera/geometry.h:16,\n\n from ../include/libcamera/controls.h:21,\n\n from ../include/libcamera/camera.h:22,\n\n from ../src/apps/common/dng_writer.h:13,\n\n from ../src/apps/common/dng_writer.cpp:8:\n\n../include/libcamera/base/private.h:21:2: error: #error \"Private headers\nmust not be included in the libcamera API\"\n\n 21 | #error \"Private headers must not be included in the libcamera API\"\n```\n\nBR,\nHarvey\n\n\n>\n> Regards,\n> Barnabás Pőcze\n>\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 5689AC324C\n\tfor ;\n\tMon, 16 Sep 2024 04:35:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 389D8634FC;\n\tMon, 16 Sep 2024 06:35:08 +0200 (CEST)","from mail-lj1-x232.google.com (mail-lj1-x232.google.com\n\t[IPv6:2a00:1450:4864:20::232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C3FA6618E3\n\tfor ;\n\tMon, 16 Sep 2024 06:35:06 +0200 (CEST)","by mail-lj1-x232.google.com with SMTP id\n\t38308e7fff4ca-2f75129b3a3so41499561fa.2\n\tfor ;\n\tSun, 15 Sep 2024 21:35:06 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"FpaGtDii\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1726461306; x=1727066106;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=EELWLEtJEhS3G/FzAkQ5/YRB2/6B8ks2ttM6FMp2i2c=;\n\tb=FpaGtDiigLp+kPcSctEDj5Po4RQjGCE0k5Va1Uh8BaAqJjfpSdp7kFMw3RrxfNkMH4\n\tCgpnqeZDyOEFVXNWqBkRhIQXC3YfjYjd1MCDLEwxbpA0ItCkY3F2Li3L8IUx7s3BzcaT\n\tbuqvMGAdcxHZwhDDvEFLrASl02ivcYhtnWON8=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1726461306; x=1727066106;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=EELWLEtJEhS3G/FzAkQ5/YRB2/6B8ks2ttM6FMp2i2c=;\n\tb=OuyN3vKTipTcm0lZeAaO7uJnu5JsAoAO67H5iHHGvYJ1A7DLsBDRR/WonXbTjfefRX\n\t6Bw9nu/Sq4dgccTv9YeFA9zktwzluYgWeMbzRAHX8ZK9Ahgv8TVFQRdIw162/smg8VRI\n\tXXpAKc+5SByMxMVWpiCLlupTj6IxotKPEGIotnegh7KX1e/GmD6tJJdzh24U4jTBRHtC\n\tiq0ROwEsyidsgBtKvEiZRyNVa406wI1eDG67nParF9hhIwLYc6+Esxnj1gTfsZxguzfF\n\tRe/ERt4R+P0AaiJStzTKawG13fm1SCZo79smn5j3GaAJK4wCWFSpKiSmhXX5jkQ/9hwO\n\tkpxw==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCUcNbWoqLF5ohiftihaN8bsbMatr57IFM1w/kQZNaab5a+qfbm81lWyTJ1PLi4yuYWXprprze4ESdAunr518+o=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YzcN4xUq8y/7Gr1Ou2Kk5c10H2dsJClj8xsCtsngBk4HiiGi0Wy\n\tDHAxOZCGrsRA0Jl6t5g4LPZDwvFjGXuI3SyZQGK4GtPv/1mSN4zlRjH5gnqSFrHR/2jQ8v6gTTX\n\tkKrLpRMSeg0ori9k06rmpLa0zQLUvEWo1UWXr","X-Google-Smtp-Source":"AGHT+IHGEyTaRtobwzL+myiitmMfy75a1sSZy9h4h03EAf9Kc3cjHcibgUcTRuZy5YYCEtMFFmy8Q+wVuUtaqq2aUIY=","X-Received":"by 2002:a2e:130a:0:b0:2f2:9a2e:c257 with SMTP id\n\t38308e7fff4ca-2f787f588c3mr54004961fa.41.1726461305256;\n\tSun, 15 Sep 2024 21:35:05 -0700 (PDT)","MIME-Version":"1.0","References":"<20240903104018.3289112-1-chenghaoyang@chromium.org>\n\t<20240903104018.3289112-2-chenghaoyang@chromium.org>\n\t\n\t<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Mon, 16 Sep 2024 12:34:54 +0800","Message-ID":"","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ","Content-Type":"multipart/alternative; boundary=\"000000000000d19e04062235185a\"","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":31235,"web_url":"https://patchwork.libcamera.org/comment/31235/","msgid":"","date":"2024-09-16T07:24:23","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hello\n\nOn Mon, Sep 16, 2024 at 12:34:54PM GMT, Cheng-Hao Yang wrote:\n> Hi Barnabás and Jacopo,\n>\n> On Mon, Sep 16, 2024 at 1:27 AM Barnabás Pőcze wrote:\n>\n> > Hi\n> >\n> >\n> > 2024. szeptember 5., csütörtök 10:35 keltezéssel, Jacopo Mondi <\n> > jacopo.mondi@ideasonboard.com> írta:\n> >\n> > > Hi Barnabás\n> > >\n> > > On Wed, Sep 04, 2024 at 11:26:37AM GMT, Barnabás Pőcze wrote:\n> > > > Hi\n> > > >\n> > > >\n> > > > 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang <\n> > chenghaoyang@chromium.org> írta:\n> > > >\n> > > > > From: Yudhistira Erlandinata \n> > > > >\n> > > > > Add a Rectangle constructor that accepts two points:\n> > > > > topLeft and bottomRight.\n> > > > >\n> > > > > Signed-off-by: Yudhistira Erlandinata \n> > > > > Co-developed-by: Harvey Yang \n> > > > > Reviewed-by: Jacopo Mondi \n> > > > > ---\n> > > > > include/libcamera/geometry.h | 2 ++\n> > > > > src/libcamera/geometry.cpp | 14 ++++++++++++++\n> > > > > 2 files changed, 16 insertions(+)\n> > > > >\n> > > > > diff --git a/include/libcamera/geometry.h\n> > b/include/libcamera/geometry.h\n> > > > > index 3e6f0f5d7..dc56f180f 100644\n> > > > > --- a/include/libcamera/geometry.h\n> > > > > +++ b/include/libcamera/geometry.h\n> > > > > @@ -262,6 +262,8 @@ public:\n> > > > > {\n> > > > > }\n> > > > >\n> > > > > + constexpr Rectangle(const Point &topLeft, const Point\n> > &bottomRight);\n> > > >\n> > > > Don't make this `constexpr` because it is not useful since the\n> > definition is not available.\n> > >\n> > > I found references online that constexpr constuctors are implcitly\n> > > inline, is this the reason of your comment ?\n> >\n> > Yes.\n> >\n> >\n> > >\n> > > However, I can't find it clearly specified in cppreference. Do you\n> > > have any pointer ?\n> >\n> > \"A constexpr specifier used in a function or static data member(since\n> > C++17) declaration implies inline.\"\n> > -- https://en.cppreference.com/w/cpp/language/constexpr\n> >\n\n\nOne day I'll lear to read properly\n\n> > >\n> > > Anyway, if inline is the reason, isn't it better to inline the\n> > > definition and maintain the constexpr specifier ?\n> > > [...]\n> >\n> > That is an option as well.\n> >\n>\n> IIUC, you're suggesting to put the definition of the new c'tor\n> back in the header file? As we use `ASSERT` in this c'tor\n> definition, there will be a compile error if we do that:\n> ```\n>\n> In file included from ../include/libcamera/base/log.h:12,\n>\n> from ../include/libcamera/geometry.h:16,\n>\n> from ../include/libcamera/controls.h:21,\n>\n> from ../include/libcamera/camera.h:22,\n>\n> from ../src/apps/common/dng_writer.h:13,\n>\n> from ../src/apps/common/dng_writer.cpp:8:\n>\n> ../include/libcamera/base/private.h:21:2: error: #error \"Private headers\n> must not be included in the libcamera API\"\n>\n> 21 | #error \"Private headers must not be included in the libcamera API\"\n> ```\n\nI see. log.h is private, and the fact ASSERT is not exposed in the\npublic API makes me think aborting execution on an invalid user input\nis probably not the best idea ? That's maybe the reason why ASSERT is not\nexposed to the public API ?\n\nI'll ask others what they think about this\n>\n> BR,\n> Harvey\n>\n>\n> >\n> > Regards,\n> > Barnabás Pőcze\n> >\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 E99D6C324C\n\tfor ;\n\tMon, 16 Sep 2024 07:24:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CFF57634FC;\n\tMon, 16 Sep 2024 09:24:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3F653618F0\n\tfor ;\n\tMon, 16 Sep 2024 09:24:28 +0200 (CEST)","from ideasonboard.com (unknown [193.34.101.21])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2C588497;\n\tMon, 16 Sep 2024 09:23:05 +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=\"qxYCZapS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726471385;\n\tbh=VqsHnygbnrh4TDRt9zNrORi6fHcJzEfm6Lgg9pXlf6k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qxYCZapSmCaBfcLRO6oh0ZuBOsVhzzR51Kxe5GgGBpTm3rtB3DEg6KsX+frsA+P8J\n\tXngfr8nbGGGbo3blPvf15vVzkpHyhx0EF/SkunDMfJF31j5oxsPMYrce34VbGXiQLK\n\tKtbAJvpR6BCX8/hf1ZoyfbKSl6B7TobGrIRcM0Fk=","Date":"Mon, 16 Sep 2024 09:24:23 +0200","From":"Jacopo Mondi ","To":"Cheng-Hao Yang ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tJacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org, Yudhistira Erlandinata\n\t","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","Message-ID":"","References":"<20240903104018.3289112-1-chenghaoyang@chromium.org>\n\t<20240903104018.3289112-2-chenghaoyang@chromium.org>\n\t\n\t<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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":31272,"web_url":"https://patchwork.libcamera.org/comment/31272/","msgid":"<4ofyijzjlfqdyuq2sz5sbjeao3dmibwvkttdgkxsvikyd66px6@u6lq2nzjdyoi>","date":"2024-09-19T11:27:09","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Harvey\n\nOn Mon, Sep 16, 2024 at 09:24:23AM GMT, Jacopo Mondi wrote:\n> Hello\n>\n> On Mon, Sep 16, 2024 at 12:34:54PM GMT, Cheng-Hao Yang wrote:\n> > Hi Barnabás and Jacopo,\n> >\n> > On Mon, Sep 16, 2024 at 1:27 AM Barnabás Pőcze wrote:\n> >\n> > > Hi\n> > >\n> > >\n> > > 2024. szeptember 5., csütörtök 10:35 keltezéssel, Jacopo Mondi <\n> > > jacopo.mondi@ideasonboard.com> írta:\n> > >\n> > > > Hi Barnabás\n> > > >\n> > > > On Wed, Sep 04, 2024 at 11:26:37AM GMT, Barnabás Pőcze wrote:\n> > > > > Hi\n> > > > >\n> > > > >\n> > > > > 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang <\n> > > chenghaoyang@chromium.org> írta:\n> > > > >\n> > > > > > From: Yudhistira Erlandinata \n> > > > > >\n> > > > > > Add a Rectangle constructor that accepts two points:\n> > > > > > topLeft and bottomRight.\n> > > > > >\n> > > > > > Signed-off-by: Yudhistira Erlandinata \n> > > > > > Co-developed-by: Harvey Yang \n> > > > > > Reviewed-by: Jacopo Mondi \n> > > > > > ---\n> > > > > > include/libcamera/geometry.h | 2 ++\n> > > > > > src/libcamera/geometry.cpp | 14 ++++++++++++++\n> > > > > > 2 files changed, 16 insertions(+)\n> > > > > >\n> > > > > > diff --git a/include/libcamera/geometry.h\n> > > b/include/libcamera/geometry.h\n> > > > > > index 3e6f0f5d7..dc56f180f 100644\n> > > > > > --- a/include/libcamera/geometry.h\n> > > > > > +++ b/include/libcamera/geometry.h\n> > > > > > @@ -262,6 +262,8 @@ public:\n> > > > > > {\n> > > > > > }\n> > > > > >\n> > > > > > + constexpr Rectangle(const Point &topLeft, const Point\n> > > &bottomRight);\n> > > > >\n> > > > > Don't make this `constexpr` because it is not useful since the\n> > > definition is not available.\n> > > >\n> > > > I found references online that constexpr constuctors are implcitly\n> > > > inline, is this the reason of your comment ?\n> > >\n> > > Yes.\n> > >\n> > >\n> > > >\n> > > > However, I can't find it clearly specified in cppreference. Do you\n> > > > have any pointer ?\n> > >\n> > > \"A constexpr specifier used in a function or static data member(since\n> > > C++17) declaration implies inline.\"\n> > > -- https://en.cppreference.com/w/cpp/language/constexpr\n> > >\n>\n>\n> One day I'll lear to read properly\n>\n> > > >\n> > > > Anyway, if inline is the reason, isn't it better to inline the\n> > > > definition and maintain the constexpr specifier ?\n> > > > [...]\n> > >\n> > > That is an option as well.\n> > >\n> >\n> > IIUC, you're suggesting to put the definition of the new c'tor\n> > back in the header file? As we use `ASSERT` in this c'tor\n> > definition, there will be a compile error if we do that:\n> > ```\n> >\n> > In file included from ../include/libcamera/base/log.h:12,\n> >\n> > from ../include/libcamera/geometry.h:16,\n> >\n> > from ../include/libcamera/controls.h:21,\n> >\n> > from ../include/libcamera/camera.h:22,\n> >\n> > from ../src/apps/common/dng_writer.h:13,\n> >\n> > from ../src/apps/common/dng_writer.cpp:8:\n> >\n> > ../include/libcamera/base/private.h:21:2: error: #error \"Private headers\n> > must not be included in the libcamera API\"\n> >\n> > 21 | #error \"Private headers must not be included in the libcamera API\"\n> > ```\n>\n> I see. log.h is private, and the fact ASSERT is not exposed in the\n> public API makes me think aborting execution on an invalid user input\n> is probably not the best idea ? That's maybe the reason why ASSERT is not\n> exposed to the public API ?\n>\n\nAssuming the top-left corner is always defined as the point with the\nsmaller 'x' and the higher 'y' whatever the reference system the\nrectangle is placed in:\n\nIn example:\n\n(0,0)\n -------------------------------->\n |\n | -------------------\n | | |\n | | |\n | x------------------\n |\n V\n\nOr:\n\n ^\n |\n |\n | x------------------\n | | |\n | | |\n | -------------------\n |\n |\n -------------------------------->\n(0,0)\n\nThe assertion you have introduced in your proposed constructor\n\n\tconstexpr Rectangle(const Point &topLeft, const Point &bottomRight)\n\t\t: x(topLeft.x), y(topLeft.y),\n\t\t width(bottomRight.x - x),\n\t\t height(bottomRight.y - y)\n\t{\n\t\tASSERT(bottomRight.x >= x && bottomRight.y >= y);\n\t}\n\nis wrong as bottomRight.y shall always be smaller than topLeft.y\n\nHas this code been ever run ?\n\n> I'll ask others what they think about this\n\nAs they're smarter than me, they made me notice that you should rather\ndefine a constructor with Point1 and Point2 and construct a Rectangle\nthat spans between these two points defined using the existing\nRectangle constructor as:\n\n\tconstexpr Rectangle(const Point &point1, const Point &point2)\n : Rectangle(std::min(point1.x, point2.x), std::max(point1.y, point2.y),\n std::max(point1.x, point2.x) - std::min(point1.x, point2.x),\n std::max(point1.y, point2.y) - std::min(point1.y, point2.y))\n\t{\n\t}\n\nPlease make sure to add proper documentation for this new constructor.\n\nYou can also add the following snippet to test/geometry.cpp\n\n\t\tPoint topLeft(3, 30);\n\t\tPoint bottomRight(30, 3);\n\t\tPoint topRight(30, 30);\n\t\tPoint bottomLeft(3, 3);\n\t\tRectangle rect1(topLeft, bottomRight);\n\t\tRectangle rect2(topRight, bottomLeft);\n\t\tRectangle rect3(bottomRight, topLeft);\n\t\tRectangle rect4(bottomLeft, topRight);\n\n\t\tif (rect1 != rect2 || rect1 != rect3 || rect1 != rect4) {\n\t\t\tcout << \"Point-from-point construction failed\" << endl;\n\t\t\treturn TestFail;\n\t\t}\n\nThanks\n j\n\n> >\n> > BR,\n> > Harvey\n> >\n> >\n> > >\n> > > Regards,\n> > > Barnabás Pőcze\n> > >\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 47ECCC324C\n\tfor ;\n\tThu, 19 Sep 2024 11:27:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2799A634FC;\n\tThu, 19 Sep 2024 13:27:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E219C618E7\n\tfor ;\n\tThu, 19 Sep 2024 13:27:13 +0200 (CEST)","from ideasonboard.com (unknown [83.68.141.146])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4D99A2C5;\n\tThu, 19 Sep 2024 13:25:50 +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=\"HHICeDGj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726745150;\n\tbh=rYgqaTzGr8us1gOQ3ze0H0Eu+c8n8q/sLqbjzm/WZOY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HHICeDGjb8gW6Xxi8CM5sBtTI1jyaSKXXp8l9CdRuzjhDtOEN8BRe4iHK+mLmJC9K\n\tCli6GRA2MzcxoC2P5bSsQVcVkwLRUQgWvH90NyWiSIPerGvjDoXb970Vl4lmAh+qFe\n\t7OfBaHH6o8sE5Mu9U5fVfgPq/gENoTdIfeZkwmeQ=","Date":"Thu, 19 Sep 2024 13:27:09 +0200","From":"Jacopo Mondi ","To":"Jacopo Mondi ","Cc":"Cheng-Hao Yang , =?utf-8?q?Barnab=C3=A1s_P?=\n\t=?utf-8?b?xZFjemU=?= ,\n\tlibcamera-devel@lists.libcamera.org, Yudhistira Erlandinata\n\t","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","Message-ID":"<4ofyijzjlfqdyuq2sz5sbjeao3dmibwvkttdgkxsvikyd66px6@u6lq2nzjdyoi>","References":"<20240903104018.3289112-1-chenghaoyang@chromium.org>\n\t<20240903104018.3289112-2-chenghaoyang@chromium.org>\n\t\n\t<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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":31274,"web_url":"https://patchwork.libcamera.org/comment/31274/","msgid":"","date":"2024-09-19T14:15:14","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":148,"url":"https://patchwork.libcamera.org/api/people/148/","name":"Cheng-Hao Yang","email":"chenghaoyang@google.com"},"content":"Hi Jacopo,\n\nOn Thu, Sep 19, 2024 at 7:27 PM Jacopo Mondi \nwrote:\n\n> Hi Harvey\n>\n> On Mon, Sep 16, 2024 at 09:24:23AM GMT, Jacopo Mondi wrote:\n> > Hello\n> >\n> > On Mon, Sep 16, 2024 at 12:34:54PM GMT, Cheng-Hao Yang wrote:\n> > > Hi Barnabás and Jacopo,\n> > >\n> > > On Mon, Sep 16, 2024 at 1:27 AM Barnabás Pőcze \n> wrote:\n> > >\n> > > > Hi\n> > > >\n> > > >\n> > > > 2024. szeptember 5., csütörtök 10:35 keltezéssel, Jacopo Mondi <\n> > > > jacopo.mondi@ideasonboard.com> írta:\n> > > >\n> > > > > Hi Barnabás\n> > > > >\n> > > > > On Wed, Sep 04, 2024 at 11:26:37AM GMT, Barnabás Pőcze wrote:\n> > > > > > Hi\n> > > > > >\n> > > > > >\n> > > > > > 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang <\n> > > > chenghaoyang@chromium.org> írta:\n> > > > > >\n> > > > > > > From: Yudhistira Erlandinata \n> > > > > > >\n> > > > > > > Add a Rectangle constructor that accepts two points:\n> > > > > > > topLeft and bottomRight.\n> > > > > > >\n> > > > > > > Signed-off-by: Yudhistira Erlandinata <\n> yerlandinata@chromium.org>\n> > > > > > > Co-developed-by: Harvey Yang \n> > > > > > > Reviewed-by: Jacopo Mondi \n> > > > > > > ---\n> > > > > > > include/libcamera/geometry.h | 2 ++\n> > > > > > > src/libcamera/geometry.cpp | 14 ++++++++++++++\n> > > > > > > 2 files changed, 16 insertions(+)\n> > > > > > >\n> > > > > > > diff --git a/include/libcamera/geometry.h\n> > > > b/include/libcamera/geometry.h\n> > > > > > > index 3e6f0f5d7..dc56f180f 100644\n> > > > > > > --- a/include/libcamera/geometry.h\n> > > > > > > +++ b/include/libcamera/geometry.h\n> > > > > > > @@ -262,6 +262,8 @@ public:\n> > > > > > > {\n> > > > > > > }\n> > > > > > >\n> > > > > > > + constexpr Rectangle(const Point &topLeft, const Point\n> > > > &bottomRight);\n> > > > > >\n> > > > > > Don't make this `constexpr` because it is not useful since the\n> > > > definition is not available.\n> > > > >\n> > > > > I found references online that constexpr constuctors are implcitly\n> > > > > inline, is this the reason of your comment ?\n> > > >\n> > > > Yes.\n> > > >\n> > > >\n> > > > >\n> > > > > However, I can't find it clearly specified in cppreference. Do you\n> > > > > have any pointer ?\n> > > >\n> > > > \"A constexpr specifier used in a function or static data\n> member(since\n> > > > C++17) declaration implies inline.\"\n> > > > -- https://en.cppreference.com/w/cpp/language/constexpr\n> > > >\n> >\n> >\n> > One day I'll lear to read properly\n> >\n> > > > >\n> > > > > Anyway, if inline is the reason, isn't it better to inline the\n> > > > > definition and maintain the constexpr specifier ?\n> > > > > [...]\n> > > >\n> > > > That is an option as well.\n> > > >\n> > >\n> > > IIUC, you're suggesting to put the definition of the new c'tor\n> > > back in the header file? As we use `ASSERT` in this c'tor\n> > > definition, there will be a compile error if we do that:\n> > > ```\n> > >\n> > > In file included from ../include/libcamera/base/log.h:12,\n> > >\n> > > from ../include/libcamera/geometry.h:16,\n> > >\n> > > from ../include/libcamera/controls.h:21,\n> > >\n> > > from ../include/libcamera/camera.h:22,\n> > >\n> > > from ../src/apps/common/dng_writer.h:13,\n> > >\n> > > from ../src/apps/common/dng_writer.cpp:8:\n> > >\n> > > ../include/libcamera/base/private.h:21:2: error: #error \"Private\n> headers\n> > > must not be included in the libcamera API\"\n> > >\n> > > 21 | #error \"Private headers must not be included in the libcamera\n> API\"\n> > > ```\n> >\n> > I see. log.h is private, and the fact ASSERT is not exposed in the\n> > public API makes me think aborting execution on an invalid user input\n> > is probably not the best idea ? That's maybe the reason why ASSERT is not\n> > exposed to the public API ?\n> >\n>\n> Assuming the top-left corner is always defined as the point with the\n> smaller 'x' and the higher 'y' whatever the reference system the\n> rectangle is placed in:\n>\n> In example:\n>\n> (0,0)\n> -------------------------------->\n> |\n> | -------------------\n> | | |\n> | | |\n> | x------------------\n> |\n> V\n>\n> Or:\n>\n> ^\n> |\n> |\n> | x------------------\n> | | |\n> | | |\n> | -------------------\n> |\n> |\n> -------------------------------->\n> (0,0)\n>\n> The assertion you have introduced in your proposed constructor\n>\n> constexpr Rectangle(const Point &topLeft, const Point &bottomRight)\n> : x(topLeft.x), y(topLeft.y),\n> width(bottomRight.x - x),\n> height(bottomRight.y - y)\n> {\n> ASSERT(bottomRight.x >= x && bottomRight.y >= y);\n> }\n>\n> is wrong as bottomRight.y shall always be smaller than topLeft.y\n>\n> Has this code been ever run ?\n>\n\nSorry for the confusion, but the `topLeft` point is actually somewhere\nlike this:\n\n(0,0)\n -------------------------------->\n |\n | x-----------------\n | | |\n | | |\n | -------------------\n |\n V\n\n`top-left` is (0, 0) in Android metadata's description [1], and it seems\nthat\nit's also in libcamera's description to indicate a point with smaller x and\ny coordinates [2].\n\n[1]:\nhttps://android.googlesource.com/platform/system/media/+/refs/heads/main/camera/docs/docs.html#33019\n[2]:\nhttps://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/geometry.cpp#n639\n\n\n>\n> > I'll ask others what they think about this\n>\n> As they're smarter than me, they made me notice that you should rather\n> define a constructor with Point1 and Point2 and construct a Rectangle\n> that spans between these two points defined using the existing\n> Rectangle constructor as:\n>\n> constexpr Rectangle(const Point &point1, const Point &point2)\n> : Rectangle(std::min(point1.x, point2.x),\n> std::max(point1.y, point2.y),\n> std::max(point1.x, point2.x) -\n> std::min(point1.x, point2.x),\n> std::max(point1.y, point2.y) -\n> std::min(point1.y, point2.y))\n> {\n> }\n>\n\nWe originally were trying to avoid adding this powerful c'tor that allows\nevery\ncombination, as libcamera's Rectangle API seems to prefer indicating the\n`topLeft` point in c'tors [3].\n\nAre you sure that the span is what we prefer?\n\n[3]:\nhttps://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/geometry.cpp#n609\n\n\n>\n> Please make sure to add proper documentation for this new constructor.\n>\n> You can also add the following snippet to test/geometry.cpp\n>\n> Point topLeft(3, 30);\n> Point bottomRight(30, 3);\n> Point topRight(30, 30);\n> Point bottomLeft(3, 3);\n> Rectangle rect1(topLeft, bottomRight);\n> Rectangle rect2(topRight, bottomLeft);\n> Rectangle rect3(bottomRight, topLeft);\n> Rectangle rect4(bottomLeft, topRight);\n>\n> if (rect1 != rect2 || rect1 != rect3 || rect1 != rect4) {\n> cout << \"Point-from-point construction failed\" <<\n> endl;\n> return TestFail;\n> }\n\nThanks\n> j\n>\n> > >\n> > > BR,\n> > > Harvey\n> > >\n> > >\n> > > >\n> > > > Regards,\n> > > > Barnabás Pőcze\n> > > >\n> > > >\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 F3CE9C324C\n\tfor ;\n\tThu, 19 Sep 2024 14:15:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D87F4634FC;\n\tThu, 19 Sep 2024 16:15:55 +0200 (CEST)","from mail-lf1-x131.google.com (mail-lf1-x131.google.com\n\t[IPv6:2a00:1450:4864:20::131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 891C9618E7\n\tfor ;\n\tThu, 19 Sep 2024 16:15:53 +0200 (CEST)","by mail-lf1-x131.google.com with SMTP id\n\t2adb3069b0e04-53661d95508so51957e87.1\n\tfor ;\n\tThu, 19 Sep 2024 07:15:53 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"jgn52jCj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=google.com; s=20230601; t=1726755352; x=1727360152;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=eV0NhVB0FtTCQ9LNF9odKvmkGI2Gc+eba1DxFLdqR00=;\n\tb=jgn52jCj6EkS5MuCFqRrjp60rNMY6h9lBN6q53PbotkzoMer2tG+zBoFc4yBDVq5gl\n\tmFa/+7UJ7uJK8TwpfbK/qpWrukA1AA1ucHQX1LZJVfvA8drcBnkHYftx8018ZlsthpTI\n\tYkq0Zn95gkyjtqN7p6tLz0Nw0AhbC8PJLfvY2ii07WwqDcNO+6ZwjJqMrKiDik+BKP7B\n\txM6TFpbV5oSb8SK0okksIYoT6SltfilTzse8ct5l8K3vwtljBZrViOebOB3lvK7y//nG\n\tb7FOC2S0x2gldw3w30+xpebHFvCfwe9OqNxWrWj/z0xBpA38OmiUeDL1WAwzftXFOA/3\n\tQBNg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1726755352; x=1727360152;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=eV0NhVB0FtTCQ9LNF9odKvmkGI2Gc+eba1DxFLdqR00=;\n\tb=rMq7YOcpMi4XFBi14h65Iw1NGhklDzEpkwJHGh8vifIL4kutWYAgMaRNGQVuFqHkK/\n\tR7KlLJfoY6RK3jMAuWH/RHZjQ2QNttMTpuwtA3eKrbsImiy5ckBjkgJXUonrId1q1SWH\n\tkdOpqyyR34JJAK6sd5aPA/uJl8uDWGT6WFYk93ioFUO9VNkLsOMAwiznYzzyEtfozjND\n\tSDXBtDXhFyRBgsVRnngiQN7OSJTuB84obJYdtiu6bOU8bO6e8ph3kQz3IUYjo6fCO6Iq\n\tWXVEW89PvpCPg0ElkxtJnfxRsh4HvCMI0A/h2XwLiAmbQgTk5xs53n1gikxXqz0qGY+A\n\twDgg==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCXg9HEBsehr9A63f+Jn92U+KcIr+VQ+80vg40gG/C5cLFXJDsWhfhbDy/EHgNRbBQyJVA6CptHLLDdOJiu2M/c=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YzxnckWue+eJe5asY62oE85ZYEHkvUOduhb/e455DzWJFhJnjwn\n\tc8vsIIOv1GQ5kGyuD8uARdfmkxzwatMiJtw9I4xcyO/eXr99QFmge5Atzo7tX1SQcn3mvKBxwME\n\tbcsfJXP9kO8I6M47tBnxpY76DRqwL3h1l3xhGweU/ikU2EszNtDBf","X-Google-Smtp-Source":"AGHT+IE+Ns77XbZd1a7LejuwdZIAYnwDD4uiopELJgAH0bNNDSyvWvgrDsfCOnTOWEbEDh15JoVESduX2k1F5SODbkY=","X-Received":"by 2002:a05:6512:1281:b0:52e:934c:1cc0 with SMTP id\n\t2adb3069b0e04-536a7c2e01bmr308133e87.7.1726755352173; Thu, 19 Sep 2024\n\t07:15:52 -0700 (PDT)","MIME-Version":"1.0","References":"<20240903104018.3289112-1-chenghaoyang@chromium.org>\n\t<20240903104018.3289112-2-chenghaoyang@chromium.org>\n\t\n\t<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>\n\t\n\t\n\t\n\t<4ofyijzjlfqdyuq2sz5sbjeao3dmibwvkttdgkxsvikyd66px6@u6lq2nzjdyoi>","In-Reply-To":"<4ofyijzjlfqdyuq2sz5sbjeao3dmibwvkttdgkxsvikyd66px6@u6lq2nzjdyoi>","From":"Cheng-Hao Yang ","Date":"Thu, 19 Sep 2024 22:15:14 +0800","Message-ID":"","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","To":"Jacopo Mondi ","Cc":"Cheng-Hao Yang , =?utf-8?q?Barnab=C3=A1s_P?=\n\t=?utf-8?b?xZFjemU=?= ,\n\tlibcamera-devel@lists.libcamera.org, Yudhistira Erlandinata\n\t","Content-Type":"multipart/alternative; boundary=\"0000000000006226d40622798f18\"","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":31293,"web_url":"https://patchwork.libcamera.org/comment/31293/","msgid":"","date":"2024-09-23T08:32:01","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Harvey\n\nOn Thu, Sep 19, 2024 at 10:15:14PM GMT, Cheng-Hao Yang wrote:\n> Hi Jacopo,\n>\n> On Thu, Sep 19, 2024 at 7:27 PM Jacopo Mondi \n> wrote:\n>\n> > Hi Harvey\n> >\n> > On Mon, Sep 16, 2024 at 09:24:23AM GMT, Jacopo Mondi wrote:\n> > > Hello\n> > >\n> > > On Mon, Sep 16, 2024 at 12:34:54PM GMT, Cheng-Hao Yang wrote:\n> > > > Hi Barnabás and Jacopo,\n> > > >\n> > > > On Mon, Sep 16, 2024 at 1:27 AM Barnabás Pőcze \n> > wrote:\n> > > >\n> > > > > Hi\n> > > > >\n> > > > >\n> > > > > 2024. szeptember 5., csütörtök 10:35 keltezéssel, Jacopo Mondi <\n> > > > > jacopo.mondi@ideasonboard.com> írta:\n> > > > >\n> > > > > > Hi Barnabás\n> > > > > >\n> > > > > > On Wed, Sep 04, 2024 at 11:26:37AM GMT, Barnabás Pőcze wrote:\n> > > > > > > Hi\n> > > > > > >\n> > > > > > >\n> > > > > > > 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang <\n> > > > > chenghaoyang@chromium.org> írta:\n> > > > > > >\n> > > > > > > > From: Yudhistira Erlandinata \n> > > > > > > >\n> > > > > > > > Add a Rectangle constructor that accepts two points:\n> > > > > > > > topLeft and bottomRight.\n> > > > > > > >\n> > > > > > > > Signed-off-by: Yudhistira Erlandinata <\n> > yerlandinata@chromium.org>\n> > > > > > > > Co-developed-by: Harvey Yang \n> > > > > > > > Reviewed-by: Jacopo Mondi \n> > > > > > > > ---\n> > > > > > > > include/libcamera/geometry.h | 2 ++\n> > > > > > > > src/libcamera/geometry.cpp | 14 ++++++++++++++\n> > > > > > > > 2 files changed, 16 insertions(+)\n> > > > > > > >\n> > > > > > > > diff --git a/include/libcamera/geometry.h\n> > > > > b/include/libcamera/geometry.h\n> > > > > > > > index 3e6f0f5d7..dc56f180f 100644\n> > > > > > > > --- a/include/libcamera/geometry.h\n> > > > > > > > +++ b/include/libcamera/geometry.h\n> > > > > > > > @@ -262,6 +262,8 @@ public:\n> > > > > > > > {\n> > > > > > > > }\n> > > > > > > >\n> > > > > > > > + constexpr Rectangle(const Point &topLeft, const Point\n> > > > > &bottomRight);\n> > > > > > >\n> > > > > > > Don't make this `constexpr` because it is not useful since the\n> > > > > definition is not available.\n> > > > > >\n> > > > > > I found references online that constexpr constuctors are implcitly\n> > > > > > inline, is this the reason of your comment ?\n> > > > >\n> > > > > Yes.\n> > > > >\n> > > > >\n> > > > > >\n> > > > > > However, I can't find it clearly specified in cppreference. Do you\n> > > > > > have any pointer ?\n> > > > >\n> > > > > \"A constexpr specifier used in a function or static data\n> > member(since\n> > > > > C++17) declaration implies inline.\"\n> > > > > -- https://en.cppreference.com/w/cpp/language/constexpr\n> > > > >\n> > >\n> > >\n> > > One day I'll lear to read properly\n> > >\n> > > > > >\n> > > > > > Anyway, if inline is the reason, isn't it better to inline the\n> > > > > > definition and maintain the constexpr specifier ?\n> > > > > > [...]\n> > > > >\n> > > > > That is an option as well.\n> > > > >\n> > > >\n> > > > IIUC, you're suggesting to put the definition of the new c'tor\n> > > > back in the header file? As we use `ASSERT` in this c'tor\n> > > > definition, there will be a compile error if we do that:\n> > > > ```\n> > > >\n> > > > In file included from ../include/libcamera/base/log.h:12,\n> > > >\n> > > > from ../include/libcamera/geometry.h:16,\n> > > >\n> > > > from ../include/libcamera/controls.h:21,\n> > > >\n> > > > from ../include/libcamera/camera.h:22,\n> > > >\n> > > > from ../src/apps/common/dng_writer.h:13,\n> > > >\n> > > > from ../src/apps/common/dng_writer.cpp:8:\n> > > >\n> > > > ../include/libcamera/base/private.h:21:2: error: #error \"Private\n> > headers\n> > > > must not be included in the libcamera API\"\n> > > >\n> > > > 21 | #error \"Private headers must not be included in the libcamera\n> > API\"\n> > > > ```\n> > >\n> > > I see. log.h is private, and the fact ASSERT is not exposed in the\n> > > public API makes me think aborting execution on an invalid user input\n> > > is probably not the best idea ? That's maybe the reason why ASSERT is not\n> > > exposed to the public API ?\n> > >\n> >\n> > Assuming the top-left corner is always defined as the point with the\n> > smaller 'x' and the higher 'y' whatever the reference system the\n> > rectangle is placed in:\n> >\n> > In example:\n> >\n> > (0,0)\n> > -------------------------------->\n> > |\n> > | -------------------\n> > | | |\n> > | | |\n> > | x------------------\n> > |\n> > V\n> >\n> > Or:\n> >\n> > ^\n> > |\n> > |\n> > | x------------------\n> > | | |\n> > | | |\n> > | -------------------\n> > |\n> > |\n> > -------------------------------->\n> > (0,0)\n> >\n> > The assertion you have introduced in your proposed constructor\n> >\n> > constexpr Rectangle(const Point &topLeft, const Point &bottomRight)\n> > : x(topLeft.x), y(topLeft.y),\n> > width(bottomRight.x - x),\n> > height(bottomRight.y - y)\n> > {\n> > ASSERT(bottomRight.x >= x && bottomRight.y >= y);\n> > }\n> >\n> > is wrong as bottomRight.y shall always be smaller than topLeft.y\n> >\n> > Has this code been ever run ?\n> >\n>\n> Sorry for the confusion, but the `topLeft` point is actually somewhere\n> like this:\n>\n> (0,0)\n> -------------------------------->\n> |\n> | x-----------------\n> | | |\n> | | |\n> | -------------------\n> |\n> V\n>\n> `top-left` is (0, 0) in Android metadata's description [1], and it seems\n> that\n> it's also in libcamera's description to indicate a point with smaller x and\n> y coordinates [2].\n>\n> [1]:\n> https://android.googlesource.com/platform/system/media/+/refs/heads/main/camera/docs/docs.html#33019\n> [2]:\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/geometry.cpp#n639\n\nIs this because of\n\n/**\n * \\fn Rectangle::Rectangle(const Size &size)\n * \\brief Construct a Rectangle of \\a size with its top left corner located\n * at (0,0)\n\nThis clearly doesn't match this case\n\n ^\n |\n |\n | x------------------\n | | |\n | | |\n | -------------------\n |\n |\n -------------------------------->\n (0,0)\n\nMaybe we have been a bit short-sighted and only assumed the blelow\nreference system had to be taken into account.\n\n (0,0)\n -------------------------------->\n |\n | x-----------------\n | | |\n | | |\n | -------------------\n |\n V\n\nPersonally I would remove this assumption in our doc and define the\ntop-left corner as the point with smaller x and largest y\n\nOpinions from libcamera people ?\n\n>\n>\n> >\n> > > I'll ask others what they think about this\n> >\n> > As they're smarter than me, they made me notice that you should rather\n> > define a constructor with Point1 and Point2 and construct a Rectangle\n> > that spans between these two points defined using the existing\n> > Rectangle constructor as:\n> >\n> > constexpr Rectangle(const Point &point1, const Point &point2)\n> > : Rectangle(std::min(point1.x, point2.x),\n> > std::max(point1.y, point2.y),\n> > std::max(point1.x, point2.x) -\n> > std::min(point1.x, point2.x),\n> > std::max(point1.y, point2.y) -\n> > std::min(point1.y, point2.y))\n> > {\n> > }\n> >\n>\n> We originally were trying to avoid adding this powerful c'tor that allows\n> every\n> combination, as libcamera's Rectangle API seems to prefer indicating the\n> `topLeft` point in c'tors [3].\n>\n> Are you sure that the span is what we prefer?\n>\n> [3]:\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/geometry.cpp#n609\n>\n>\n> >\n> > Please make sure to add proper documentation for this new constructor.\n> >\n> > You can also add the following snippet to test/geometry.cpp\n> >\n> > Point topLeft(3, 30);\n> > Point bottomRight(30, 3);\n> > Point topRight(30, 30);\n> > Point bottomLeft(3, 3);\n> > Rectangle rect1(topLeft, bottomRight);\n> > Rectangle rect2(topRight, bottomLeft);\n> > Rectangle rect3(bottomRight, topLeft);\n> > Rectangle rect4(bottomLeft, topRight);\n> >\n> > if (rect1 != rect2 || rect1 != rect3 || rect1 != rect4) {\n> > cout << \"Point-from-point construction failed\" <<\n> > endl;\n> > return TestFail;\n> > }\n>\n> Thanks\n> > j\n> >\n> > > >\n> > > > BR,\n> > > > Harvey\n> > > >\n> > > >\n> > > > >\n> > > > > Regards,\n> > > > > Barnabás Pőcze\n> > > > >\n> > > > >\n> >\n>\n>\n> --\n> BR,\n> Harvey Yang","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 7AAC8C0F1B\n\tfor ;\n\tMon, 23 Sep 2024 08:32:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 209F86350D;\n\tMon, 23 Sep 2024 10:32:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4410D6037E\n\tfor ;\n\tMon, 23 Sep 2024 10:32:08 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F3A632E0;\n\tMon, 23 Sep 2024 10:30:41 +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=\"cGB91IH0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727080242;\n\tbh=UC+tLzAhoW/ZBKBJ0zEnkvKdVK5O3bl+CLWd17SQ9nI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cGB91IH0Eo+IxsFRNHqsxfeEUWFX5A6tzWfP6Pqgbb34ZjVPAIaSeWk+5C4ey+q9h\n\tRje0eAaOcRXuSXh99OLkMjzA3avOX3MCG+xN6LYyq+jkqNFs5IGvQhoLmyIGPCyTwP\n\tVeSNucre30CBKdOuPCiXbOpTgQcvGtkVofr3+4XI=","Date":"Mon, 23 Sep 2024 10:32:01 +0200","From":"Jacopo Mondi ","To":"Cheng-Hao Yang ","Cc":"Jacopo Mondi , Cheng-Hao Yang\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t, libcamera-devel@lists.libcamera.org,\n\tYudhistira Erlandinata ","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","Message-ID":"","References":"<20240903104018.3289112-1-chenghaoyang@chromium.org>\n\t<20240903104018.3289112-2-chenghaoyang@chromium.org>\n\t\n\t<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>\n\t\n\t\n\t\n\t<4ofyijzjlfqdyuq2sz5sbjeao3dmibwvkttdgkxsvikyd66px6@u6lq2nzjdyoi>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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":31294,"web_url":"https://patchwork.libcamera.org/comment/31294/","msgid":"<172708093143.129190.2641190310931920372@ping.linuxembedded.co.uk>","date":"2024-09-23T08:42:11","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Cheng-Hao Yang (2024-09-19 15:15:14)\n> Hi Jacopo,\n> \n> On Thu, Sep 19, 2024 at 7:27 PM Jacopo Mondi \n> wrote:\n> \n> > Hi Harvey\n> >\n> > On Mon, Sep 16, 2024 at 09:24:23AM GMT, Jacopo Mondi wrote:\n> > > Hello\n> > >\n> > > On Mon, Sep 16, 2024 at 12:34:54PM GMT, Cheng-Hao Yang wrote:\n> > > > Hi Barnabás and Jacopo,\n> > > >\n> > > > On Mon, Sep 16, 2024 at 1:27 AM Barnabás Pőcze \n> > wrote:\n> > > >\n> > > > > Hi\n> > > > >\n> > > > >\n> > > > > 2024. szeptember 5., csütörtök 10:35 keltezéssel, Jacopo Mondi <\n> > > > > jacopo.mondi@ideasonboard.com> írta:\n> > > > >\n> > > > > > Hi Barnabás\n> > > > > >\n> > > > > > On Wed, Sep 04, 2024 at 11:26:37AM GMT, Barnabás Pőcze wrote:\n> > > > > > > Hi\n> > > > > > >\n> > > > > > >\n> > > > > > > 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang <\n> > > > > chenghaoyang@chromium.org> írta:\n> > > > > > >\n> > > > > > > > From: Yudhistira Erlandinata \n> > > > > > > >\n> > > > > > > > Add a Rectangle constructor that accepts two points:\n> > > > > > > > topLeft and bottomRight.\n> > > > > > > >\n> > > > > > > > Signed-off-by: Yudhistira Erlandinata <\n> > yerlandinata@chromium.org>\n> > > > > > > > Co-developed-by: Harvey Yang \n> > > > > > > > Reviewed-by: Jacopo Mondi \n> > > > > > > > ---\n> > > > > > > > include/libcamera/geometry.h | 2 ++\n> > > > > > > > src/libcamera/geometry.cpp | 14 ++++++++++++++\n> > > > > > > > 2 files changed, 16 insertions(+)\n> > > > > > > >\n> > > > > > > > diff --git a/include/libcamera/geometry.h\n> > > > > b/include/libcamera/geometry.h\n> > > > > > > > index 3e6f0f5d7..dc56f180f 100644\n> > > > > > > > --- a/include/libcamera/geometry.h\n> > > > > > > > +++ b/include/libcamera/geometry.h\n> > > > > > > > @@ -262,6 +262,8 @@ public:\n> > > > > > > > {\n> > > > > > > > }\n> > > > > > > >\n> > > > > > > > + constexpr Rectangle(const Point &topLeft, const Point\n> > > > > &bottomRight);\n> > > > > > >\n> > > > > > > Don't make this `constexpr` because it is not useful since the\n> > > > > definition is not available.\n> > > > > >\n> > > > > > I found references online that constexpr constuctors are implcitly\n> > > > > > inline, is this the reason of your comment ?\n> > > > >\n> > > > > Yes.\n> > > > >\n> > > > >\n> > > > > >\n> > > > > > However, I can't find it clearly specified in cppreference. Do you\n> > > > > > have any pointer ?\n> > > > >\n> > > > > \"A constexpr specifier used in a function or static data\n> > member(since\n> > > > > C++17) declaration implies inline.\"\n> > > > > -- https://en.cppreference.com/w/cpp/language/constexpr\n> > > > >\n> > >\n> > >\n> > > One day I'll lear to read properly\n> > >\n> > > > > >\n> > > > > > Anyway, if inline is the reason, isn't it better to inline the\n> > > > > > definition and maintain the constexpr specifier ?\n> > > > > > [...]\n> > > > >\n> > > > > That is an option as well.\n> > > > >\n> > > >\n> > > > IIUC, you're suggesting to put the definition of the new c'tor\n> > > > back in the header file? As we use `ASSERT` in this c'tor\n> > > > definition, there will be a compile error if we do that:\n> > > > ```\n> > > >\n> > > > In file included from ../include/libcamera/base/log.h:12,\n> > > >\n> > > > from ../include/libcamera/geometry.h:16,\n> > > >\n> > > > from ../include/libcamera/controls.h:21,\n> > > >\n> > > > from ../include/libcamera/camera.h:22,\n> > > >\n> > > > from ../src/apps/common/dng_writer.h:13,\n> > > >\n> > > > from ../src/apps/common/dng_writer.cpp:8:\n> > > >\n> > > > ../include/libcamera/base/private.h:21:2: error: #error \"Private\n> > headers\n> > > > must not be included in the libcamera API\"\n> > > >\n> > > > 21 | #error \"Private headers must not be included in the libcamera\n> > API\"\n> > > > ```\n> > >\n> > > I see. log.h is private, and the fact ASSERT is not exposed in the\n> > > public API makes me think aborting execution on an invalid user input\n> > > is probably not the best idea ? That's maybe the reason why ASSERT is not\n> > > exposed to the public API ?\n\nIndeed, we that error means we really can't use that feature in a place\nthat's in public headers.\n\n> > >\n> >\n> > Assuming the top-left corner is always defined as the point with the\n> > smaller 'x' and the higher 'y' whatever the reference system the\n> > rectangle is placed in:\n> >\n> > In example:\n> >\n> > (0,0)\n> > -------------------------------->\n> > |\n> > | -------------------\n> > | | |\n> > | | |\n> > | x------------------\n> > |\n> > V\n> >\n> > Or:\n> >\n> > ^\n> > |\n> > |\n> > | x------------------\n> > | | |\n> > | | |\n> > | -------------------\n> > |\n> > |\n> > -------------------------------->\n> > (0,0)\n> >\n> > The assertion you have introduced in your proposed constructor\n> >\n> > constexpr Rectangle(const Point &topLeft, const Point &bottomRight)\n> > : x(topLeft.x), y(topLeft.y),\n> > width(bottomRight.x - x),\n> > height(bottomRight.y - y)\n> > {\n> > ASSERT(bottomRight.x >= x && bottomRight.y >= y);\n> > }\n> >\n> > is wrong as bottomRight.y shall always be smaller than topLeft.y\n> >\n> > Has this code been ever run ?\n> >\n> \n> Sorry for the confusion, but the `topLeft` point is actually somewhere\n> like this:\n> \n> (0,0)\n> -------------------------------->\n> |\n> | x-----------------\n> | | |\n> | | |\n> | -------------------\n> |\n> V\n> \n> `top-left` is (0, 0) in Android metadata's description [1], and it seems\n> that\n> it's also in libcamera's description to indicate a point with smaller x and\n> y coordinates [2].\n> \n> [1]:\n> https://android.googlesource.com/platform/system/media/+/refs/heads/main/camera/docs/docs.html#33019\n> [2]:\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/geometry.cpp#n639\n> \n\nTop left being 0,0 is how I understand coordinates on images too. Of\ncourse for maths it's usually bottom left ...\n\nPerhaps we need to do better for documenting our expectations on origin point coordinates ....\n\n\n\n> \n> >\n> > > I'll ask others what they think about this\n> >\n> > As they're smarter than me, they made me notice that you should rather\n> > define a constructor with Point1 and Point2 and construct a Rectangle\n> > that spans between these two points defined using the existing\n> > Rectangle constructor as:\n> >\n> > constexpr Rectangle(const Point &point1, const Point &point2)\n> > : Rectangle(std::min(point1.x, point2.x),\n> > std::max(point1.y, point2.y),\n> > std::max(point1.x, point2.x) -\n> > std::min(point1.x, point2.x),\n> > std::max(point1.y, point2.y) -\n> > std::min(point1.y, point2.y))\n> > {\n> > }\n> >\n> \n> We originally were trying to avoid adding this powerful c'tor that allows\n> every\n> combination, as libcamera's Rectangle API seems to prefer indicating the\n> `topLeft` point in c'tors [3].\n> \n> Are you sure that the span is what we prefer?\n> \n> [3]:\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/geometry.cpp#n609\n> \n> \n\nIf only a single point is given, then indeed it has to be defined at\nwhich location it is (hence [3]). But if a rectangle is produced from\ntwo (opposing corner) points, then the dimensions can be clearly\nobtained so I do think constructing from two points with the min/max\nhelpers is a suitable way to go here without needing any assertsions.\n\n> >\n> > Please make sure to add proper documentation for this new constructor.\n> >\n> > You can also add the following snippet to test/geometry.cpp\n> >\n> > Point topLeft(3, 30);\n> > Point bottomRight(30, 3);\n> > Point topRight(30, 30);\n> > Point bottomLeft(3, 3);\n> > Rectangle rect1(topLeft, bottomRight);\n> > Rectangle rect2(topRight, bottomLeft);\n> > Rectangle rect3(bottomRight, topLeft);\n> > Rectangle rect4(bottomLeft, topRight);\n> >\n> > if (rect1 != rect2 || rect1 != rect3 || rect1 != rect4) {\n> > cout << \"Point-from-point construction failed\" <<\n> > endl;\n> > return TestFail;\n> > }\n\nAnd looks like a good addition to codify it all.\n\n--\nKieran\n\n\n> \n> Thanks\n> > j\n> >\n> > > >\n> > > > BR,\n> > > > Harvey\n> > > >\n> > > >\n> > > > >\n> > > > > Regards,\n> > > > > Barnabás Pőcze\n> > > > >\n> > > > >\n> >\n> \n> \n> -- \n> BR,\n> Harvey Yang","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 200C4C3257\n\tfor ;\n\tMon, 23 Sep 2024 08:42:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D22C76350D;\n\tMon, 23 Sep 2024 10:42:16 +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 E9DAB6037E\n\tfor ;\n\tMon, 23 Sep 2024 10:42:14 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DFD7D2E0;\n\tMon, 23 Sep 2024 10:40:48 +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=\"PJ0p4G/J\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727080849;\n\tbh=CZO2NpC1MTfTkGx0Fek5sAx+GLlWXjaNzf7EARV0zPs=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=PJ0p4G/JEFyi5L9dGC6HJ4WGE7Oal1c7WrSuh1tj0eMCv3X/2eYi/+1okGN3JVSEY\n\thZGQZvUFqtJJDwKhLoviXTJsljY9m8xbeu7lGWu1rsgTpQFnwtXRATWgyh/42dG0DO\n\tMblWA8CWGMx2ZBcJPa3jAnBKHnFgxCRL14VD/kHs=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20240903104018.3289112-1-chenghaoyang@chromium.org>\n\t<20240903104018.3289112-2-chenghaoyang@chromium.org>\n\t\n\t<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>\n\t\n\t\n\t\n\t<4ofyijzjlfqdyuq2sz5sbjeao3dmibwvkttdgkxsvikyd66px6@u6lq2nzjdyoi>\n\t","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","From":"Kieran Bingham ","Cc":"Cheng-Hao Yang , =?utf-8?q?Barnab=C3=A1s_P?=\n\t=?utf-8?b?xZFjemU=?= ,\n\tlibcamera-devel@lists.libcamera.org, Yudhistira Erlandinata\n\t","To":"Cheng-Hao Yang ,\n\tJacopo Mondi ","Date":"Mon, 23 Sep 2024 09:42:11 +0100","Message-ID":"<172708093143.129190.2641190310931920372@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":31295,"web_url":"https://patchwork.libcamera.org/comment/31295/","msgid":"<172708128719.129190.16780326770350068542@ping.linuxembedded.co.uk>","date":"2024-09-23T08:48:07","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2024-09-23 09:32:01)\n> Hi Harvey\n> \n> On Thu, Sep 19, 2024 at 10:15:14PM GMT, Cheng-Hao Yang wrote:\n> > Hi Jacopo,\n> >\n> > On Thu, Sep 19, 2024 at 7:27 PM Jacopo Mondi \n> > wrote:\n> >\n> > > Hi Harvey\n> > >\n> > > On Mon, Sep 16, 2024 at 09:24:23AM GMT, Jacopo Mondi wrote:\n> > > > Hello\n> > > >\n> > > > On Mon, Sep 16, 2024 at 12:34:54PM GMT, Cheng-Hao Yang wrote:\n> > > > > Hi Barnabás and Jacopo,\n> > > > >\n> > > > > On Mon, Sep 16, 2024 at 1:27 AM Barnabás Pőcze \n> > > wrote:\n> > > > >\n> > > > > > Hi\n> > > > > >\n> > > > > >\n> > > > > > 2024. szeptember 5., csütörtök 10:35 keltezéssel, Jacopo Mondi <\n> > > > > > jacopo.mondi@ideasonboard.com> írta:\n> > > > > >\n> > > > > > > Hi Barnabás\n> > > > > > >\n> > > > > > > On Wed, Sep 04, 2024 at 11:26:37AM GMT, Barnabás Pőcze wrote:\n> > > > > > > > Hi\n> > > > > > > >\n> > > > > > > >\n> > > > > > > > 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang <\n> > > > > > chenghaoyang@chromium.org> írta:\n> > > > > > > >\n> > > > > > > > > From: Yudhistira Erlandinata \n> > > > > > > > >\n> > > > > > > > > Add a Rectangle constructor that accepts two points:\n> > > > > > > > > topLeft and bottomRight.\n> > > > > > > > >\n> > > > > > > > > Signed-off-by: Yudhistira Erlandinata <\n> > > yerlandinata@chromium.org>\n> > > > > > > > > Co-developed-by: Harvey Yang \n> > > > > > > > > Reviewed-by: Jacopo Mondi \n> > > > > > > > > ---\n> > > > > > > > > include/libcamera/geometry.h | 2 ++\n> > > > > > > > > src/libcamera/geometry.cpp | 14 ++++++++++++++\n> > > > > > > > > 2 files changed, 16 insertions(+)\n> > > > > > > > >\n> > > > > > > > > diff --git a/include/libcamera/geometry.h\n> > > > > > b/include/libcamera/geometry.h\n> > > > > > > > > index 3e6f0f5d7..dc56f180f 100644\n> > > > > > > > > --- a/include/libcamera/geometry.h\n> > > > > > > > > +++ b/include/libcamera/geometry.h\n> > > > > > > > > @@ -262,6 +262,8 @@ public:\n> > > > > > > > > {\n> > > > > > > > > }\n> > > > > > > > >\n> > > > > > > > > + constexpr Rectangle(const Point &topLeft, const Point\n> > > > > > &bottomRight);\n> > > > > > > >\n> > > > > > > > Don't make this `constexpr` because it is not useful since the\n> > > > > > definition is not available.\n> > > > > > >\n> > > > > > > I found references online that constexpr constuctors are implcitly\n> > > > > > > inline, is this the reason of your comment ?\n> > > > > >\n> > > > > > Yes.\n> > > > > >\n> > > > > >\n> > > > > > >\n> > > > > > > However, I can't find it clearly specified in cppreference. Do you\n> > > > > > > have any pointer ?\n> > > > > >\n> > > > > > \"A constexpr specifier used in a function or static data\n> > > member(since\n> > > > > > C++17) declaration implies inline.\"\n> > > > > > -- https://en.cppreference.com/w/cpp/language/constexpr\n> > > > > >\n> > > >\n> > > >\n> > > > One day I'll lear to read properly\n> > > >\n> > > > > > >\n> > > > > > > Anyway, if inline is the reason, isn't it better to inline the\n> > > > > > > definition and maintain the constexpr specifier ?\n> > > > > > > [...]\n> > > > > >\n> > > > > > That is an option as well.\n> > > > > >\n> > > > >\n> > > > > IIUC, you're suggesting to put the definition of the new c'tor\n> > > > > back in the header file? As we use `ASSERT` in this c'tor\n> > > > > definition, there will be a compile error if we do that:\n> > > > > ```\n> > > > >\n> > > > > In file included from ../include/libcamera/base/log.h:12,\n> > > > >\n> > > > > from ../include/libcamera/geometry.h:16,\n> > > > >\n> > > > > from ../include/libcamera/controls.h:21,\n> > > > >\n> > > > > from ../include/libcamera/camera.h:22,\n> > > > >\n> > > > > from ../src/apps/common/dng_writer.h:13,\n> > > > >\n> > > > > from ../src/apps/common/dng_writer.cpp:8:\n> > > > >\n> > > > > ../include/libcamera/base/private.h:21:2: error: #error \"Private\n> > > headers\n> > > > > must not be included in the libcamera API\"\n> > > > >\n> > > > > 21 | #error \"Private headers must not be included in the libcamera\n> > > API\"\n> > > > > ```\n> > > >\n> > > > I see. log.h is private, and the fact ASSERT is not exposed in the\n> > > > public API makes me think aborting execution on an invalid user input\n> > > > is probably not the best idea ? That's maybe the reason why ASSERT is not\n> > > > exposed to the public API ?\n> > > >\n> > >\n> > > Assuming the top-left corner is always defined as the point with the\n> > > smaller 'x' and the higher 'y' whatever the reference system the\n> > > rectangle is placed in:\n> > >\n> > > In example:\n> > >\n> > > (0,0)\n> > > -------------------------------->\n> > > |\n> > > | -------------------\n> > > | | |\n> > > | | |\n> > > | x------------------\n> > > |\n> > > V\n> > >\n> > > Or:\n> > >\n> > > ^\n> > > |\n> > > |\n> > > | x------------------\n> > > | | |\n> > > | | |\n> > > | -------------------\n> > > |\n> > > |\n> > > -------------------------------->\n> > > (0,0)\n> > >\n> > > The assertion you have introduced in your proposed constructor\n> > >\n> > > constexpr Rectangle(const Point &topLeft, const Point &bottomRight)\n> > > : x(topLeft.x), y(topLeft.y),\n> > > width(bottomRight.x - x),\n> > > height(bottomRight.y - y)\n> > > {\n> > > ASSERT(bottomRight.x >= x && bottomRight.y >= y);\n> > > }\n> > >\n> > > is wrong as bottomRight.y shall always be smaller than topLeft.y\n> > >\n> > > Has this code been ever run ?\n> > >\n> >\n> > Sorry for the confusion, but the `topLeft` point is actually somewhere\n> > like this:\n> >\n> > (0,0)\n> > -------------------------------->\n> > |\n> > | x-----------------\n> > | | |\n> > | | |\n> > | -------------------\n> > |\n> > V\n> >\n> > `top-left` is (0, 0) in Android metadata's description [1], and it seems\n> > that\n> > it's also in libcamera's description to indicate a point with smaller x and\n> > y coordinates [2].\n> >\n> > [1]:\n> > https://android.googlesource.com/platform/system/media/+/refs/heads/main/camera/docs/docs.html#33019\n> > [2]:\n> > https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/geometry.cpp#n639\n> \n> Is this because of\n> \n> /**\n> * \\fn Rectangle::Rectangle(const Size &size)\n> * \\brief Construct a Rectangle of \\a size with its top left corner located\n> * at (0,0)\n> \n> This clearly doesn't match this case\n> \n> ^\n> |\n> |\n> | x------------------\n> | | |\n> | | |\n> | -------------------\n> |\n> |\n> -------------------------------->\n> (0,0)\n> \n> Maybe we have been a bit short-sighted and only assumed the blelow\n> reference system had to be taken into account.\n> \n> (0,0)\n> -------------------------------->\n> |\n> | x-----------------\n> | | |\n> | | |\n> | -------------------\n> |\n> V\n> \n> Personally I would remove this assumption in our doc and define the\n> top-left corner as the point with smaller x and largest y\n> \n> Opinions from libcamera people ?\n\nAha, I would define top left as 0,0 :-)\n\nI've just opened up GIMP which also uses top left origin.\n\nI expect other image applications would also honour this - so worth\nchecking a few ...\n\nFor me - even though this is 'geometry' ... it's *image processing*\ngeometry.\n\n--\nKieran","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 94F2CC0F1B\n\tfor ;\n\tMon, 23 Sep 2024 08:48:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 010326350D;\n\tMon, 23 Sep 2024 10:48:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CC1C76037E\n\tfor ;\n\tMon, 23 Sep 2024 10:48:09 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A134F2E0;\n\tMon, 23 Sep 2024 10:46:43 +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=\"YDGe8aSM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727081203;\n\tbh=SQx00t1m1zQPSLi+Tjdjhk8sx9cPV90JK85TPYuEaIo=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=YDGe8aSMRWq2VzEtWsEjnZHQHPerg4yu5T0/Jwass+CIyiQPXM9FsawyQOFXrJkqV\n\tt89ARKAZ6wiloVbH+P63/P6PVGaZnEya2V5En0VTGlcGoVoy62NCasIss8fIOur9GH\n\tyGrGSEUpBnCBkpyB5cwe+lU5vovTwms+MfI5N+t8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20240903104018.3289112-1-chenghaoyang@chromium.org>\n\t<20240903104018.3289112-2-chenghaoyang@chromium.org>\n\t\n\t<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>\n\t\n\t\n\t\n\t<4ofyijzjlfqdyuq2sz5sbjeao3dmibwvkttdgkxsvikyd66px6@u6lq2nzjdyoi>\n\t\n\t","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","From":"Kieran Bingham ","Cc":"Jacopo Mondi , Cheng-Hao Yang\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t, libcamera-devel@lists.libcamera.org,\n\tYudhistira Erlandinata ","To":"Cheng-Hao Yang ,\n\tJacopo Mondi ","Date":"Mon, 23 Sep 2024 09:48:07 +0100","Message-ID":"<172708128719.129190.16780326770350068542@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":31299,"web_url":"https://patchwork.libcamera.org/comment/31299/","msgid":"","date":"2024-09-23T09:20:07","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Mon, Sep 23, 2024 at 09:48:07AM GMT, Kieran Bingham wrote:\n> Quoting Jacopo Mondi (2024-09-23 09:32:01)\n> > Hi Harvey\n> >\n> > On Thu, Sep 19, 2024 at 10:15:14PM GMT, Cheng-Hao Yang wrote:\n> > > Hi Jacopo,\n> > >\n> > > On Thu, Sep 19, 2024 at 7:27 PM Jacopo Mondi \n> > > wrote:\n> > >\n> > > > Hi Harvey\n> > > >\n> > > > On Mon, Sep 16, 2024 at 09:24:23AM GMT, Jacopo Mondi wrote:\n> > > > > Hello\n> > > > >\n> > > > > On Mon, Sep 16, 2024 at 12:34:54PM GMT, Cheng-Hao Yang wrote:\n> > > > > > Hi Barnabás and Jacopo,\n> > > > > >\n> > > > > > On Mon, Sep 16, 2024 at 1:27 AM Barnabás Pőcze \n> > > > wrote:\n> > > > > >\n> > > > > > > Hi\n> > > > > > >\n> > > > > > >\n> > > > > > > 2024. szeptember 5., csütörtök 10:35 keltezéssel, Jacopo Mondi <\n> > > > > > > jacopo.mondi@ideasonboard.com> írta:\n> > > > > > >\n> > > > > > > > Hi Barnabás\n> > > > > > > >\n> > > > > > > > On Wed, Sep 04, 2024 at 11:26:37AM GMT, Barnabás Pőcze wrote:\n> > > > > > > > > Hi\n> > > > > > > > >\n> > > > > > > > >\n> > > > > > > > > 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang <\n> > > > > > > chenghaoyang@chromium.org> írta:\n> > > > > > > > >\n> > > > > > > > > > From: Yudhistira Erlandinata \n> > > > > > > > > >\n> > > > > > > > > > Add a Rectangle constructor that accepts two points:\n> > > > > > > > > > topLeft and bottomRight.\n> > > > > > > > > >\n> > > > > > > > > > Signed-off-by: Yudhistira Erlandinata <\n> > > > yerlandinata@chromium.org>\n> > > > > > > > > > Co-developed-by: Harvey Yang \n> > > > > > > > > > Reviewed-by: Jacopo Mondi \n> > > > > > > > > > ---\n> > > > > > > > > > include/libcamera/geometry.h | 2 ++\n> > > > > > > > > > src/libcamera/geometry.cpp | 14 ++++++++++++++\n> > > > > > > > > > 2 files changed, 16 insertions(+)\n> > > > > > > > > >\n> > > > > > > > > > diff --git a/include/libcamera/geometry.h\n> > > > > > > b/include/libcamera/geometry.h\n> > > > > > > > > > index 3e6f0f5d7..dc56f180f 100644\n> > > > > > > > > > --- a/include/libcamera/geometry.h\n> > > > > > > > > > +++ b/include/libcamera/geometry.h\n> > > > > > > > > > @@ -262,6 +262,8 @@ public:\n> > > > > > > > > > {\n> > > > > > > > > > }\n> > > > > > > > > >\n> > > > > > > > > > + constexpr Rectangle(const Point &topLeft, const Point\n> > > > > > > &bottomRight);\n> > > > > > > > >\n> > > > > > > > > Don't make this `constexpr` because it is not useful since the\n> > > > > > > definition is not available.\n> > > > > > > >\n> > > > > > > > I found references online that constexpr constuctors are implcitly\n> > > > > > > > inline, is this the reason of your comment ?\n> > > > > > >\n> > > > > > > Yes.\n> > > > > > >\n> > > > > > >\n> > > > > > > >\n> > > > > > > > However, I can't find it clearly specified in cppreference. Do you\n> > > > > > > > have any pointer ?\n> > > > > > >\n> > > > > > > \"A constexpr specifier used in a function or static data\n> > > > member(since\n> > > > > > > C++17) declaration implies inline.\"\n> > > > > > > -- https://en.cppreference.com/w/cpp/language/constexpr\n> > > > > > >\n> > > > >\n> > > > >\n> > > > > One day I'll lear to read properly\n> > > > >\n> > > > > > > >\n> > > > > > > > Anyway, if inline is the reason, isn't it better to inline the\n> > > > > > > > definition and maintain the constexpr specifier ?\n> > > > > > > > [...]\n> > > > > > >\n> > > > > > > That is an option as well.\n> > > > > > >\n> > > > > >\n> > > > > > IIUC, you're suggesting to put the definition of the new c'tor\n> > > > > > back in the header file? As we use `ASSERT` in this c'tor\n> > > > > > definition, there will be a compile error if we do that:\n> > > > > > ```\n> > > > > >\n> > > > > > In file included from ../include/libcamera/base/log.h:12,\n> > > > > >\n> > > > > > from ../include/libcamera/geometry.h:16,\n> > > > > >\n> > > > > > from ../include/libcamera/controls.h:21,\n> > > > > >\n> > > > > > from ../include/libcamera/camera.h:22,\n> > > > > >\n> > > > > > from ../src/apps/common/dng_writer.h:13,\n> > > > > >\n> > > > > > from ../src/apps/common/dng_writer.cpp:8:\n> > > > > >\n> > > > > > ../include/libcamera/base/private.h:21:2: error: #error \"Private\n> > > > headers\n> > > > > > must not be included in the libcamera API\"\n> > > > > >\n> > > > > > 21 | #error \"Private headers must not be included in the libcamera\n> > > > API\"\n> > > > > > ```\n> > > > >\n> > > > > I see. log.h is private, and the fact ASSERT is not exposed in the\n> > > > > public API makes me think aborting execution on an invalid user input\n> > > > > is probably not the best idea ? That's maybe the reason why ASSERT is not\n> > > > > exposed to the public API ?\n> > > > >\n> > > >\n> > > > Assuming the top-left corner is always defined as the point with the\n> > > > smaller 'x' and the higher 'y' whatever the reference system the\n> > > > rectangle is placed in:\n> > > >\n> > > > In example:\n> > > >\n> > > > (0,0)\n> > > > -------------------------------->\n> > > > |\n> > > > | -------------------\n> > > > | | |\n> > > > | | |\n> > > > | x------------------\n> > > > |\n> > > > V\n> > > >\n> > > > Or:\n> > > >\n> > > > ^\n> > > > |\n> > > > |\n> > > > | x------------------\n> > > > | | |\n> > > > | | |\n> > > > | -------------------\n> > > > |\n> > > > |\n> > > > -------------------------------->\n> > > > (0,0)\n> > > >\n> > > > The assertion you have introduced in your proposed constructor\n> > > >\n> > > > constexpr Rectangle(const Point &topLeft, const Point &bottomRight)\n> > > > : x(topLeft.x), y(topLeft.y),\n> > > > width(bottomRight.x - x),\n> > > > height(bottomRight.y - y)\n> > > > {\n> > > > ASSERT(bottomRight.x >= x && bottomRight.y >= y);\n> > > > }\n> > > >\n> > > > is wrong as bottomRight.y shall always be smaller than topLeft.y\n> > > >\n> > > > Has this code been ever run ?\n> > > >\n> > >\n> > > Sorry for the confusion, but the `topLeft` point is actually somewhere\n> > > like this:\n> > >\n> > > (0,0)\n> > > -------------------------------->\n> > > |\n> > > | x-----------------\n> > > | | |\n> > > | | |\n> > > | -------------------\n> > > |\n> > > V\n> > >\n> > > `top-left` is (0, 0) in Android metadata's description [1], and it seems\n> > > that\n> > > it's also in libcamera's description to indicate a point with smaller x and\n> > > y coordinates [2].\n> > >\n> > > [1]:\n> > > https://android.googlesource.com/platform/system/media/+/refs/heads/main/camera/docs/docs.html#33019\n> > > [2]:\n> > > https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/geometry.cpp#n639\n> >\n> > Is this because of\n> >\n> > /**\n> > * \\fn Rectangle::Rectangle(const Size &size)\n> > * \\brief Construct a Rectangle of \\a size with its top left corner located\n> > * at (0,0)\n> >\n> > This clearly doesn't match this case\n> >\n> > ^\n> > |\n> > |\n> > | x------------------\n> > | | |\n> > | | |\n> > | -------------------\n> > |\n> > |\n> > -------------------------------->\n> > (0,0)\n> >\n> > Maybe we have been a bit short-sighted and only assumed the blelow\n> > reference system had to be taken into account.\n> >\n> > (0,0)\n> > -------------------------------->\n> > |\n> > | x-----------------\n> > | | |\n> > | | |\n> > | -------------------\n> > |\n> > V\n> >\n> > Personally I would remove this assumption in our doc and define the\n> > top-left corner as the point with smaller x and largest y\n> >\n> > Opinions from libcamera people ?\n>\n> Aha, I would define top left as 0,0 :-)\n>\n> I've just opened up GIMP which also uses top left origin.\n>\n> I expect other image applications would also honour this - so worth\n> checking a few ...\n>\n> For me - even though this is 'geometry' ... it's *image processing*\n> geometry.\n>\n\nThe Rectangle class documentation says already\n\n * The measure unit of the rectangle coordinates and size, as well as the\n * reference point from which the Rectangle::x and Rectangle::y displacements\n * refers to, are defined by the context were rectangle is used.\n\nWhich seems to suggest that no assumptions on the reference system are\nmade in the class, but however we also have one constructor that is\ndocumented as\n\n * \\brief Construct a Rectangle of \\a size with its top left corner located\n * at (0,0)\n\nWhich, in the below example suggestes \"top-left\" is actually visually\nbottom-right.\n\n\n ^\n |\n |\n | -------------------\n | | |\n | | |\n | X------------------\n |\n |\n -------------------------------->\n (0,0)\n\nIf it is instead assumed that \"top-left\" is is the visual top-left\nregardless of the coordinate system so we should clarify what\ndirections do we increment x and y when give a constructor like\nRectangle({x,y}, w, h) as in this case we grow both x and y\n\n (0,0)\n -------------------------------->\n |\n | x------------------\n | | |\n | | |\n | -------------------\n |\n V\n\nwhile in this we increase x and decrement y\n ^\n |\n |\n | x------------------\n | | |\n | | |\n | -------------------\n |\n |\n -------------------------------->\n (0,0)\n\nbut it would be totally legit to construct the same rectangle as\n\n ^ -------------------\n | | |\n | | |\n | x------------------\n |\n |\n |\n |\n |\n -------------------------------->\n (0,0)\n\nTo remove ambiguities I would rather redefine top-left as \"origin\"\npoint defined as the point wiht the lower x and lower y of the\nrectangle and clarify that we always increase width and height\nfrom there when constructing Rectangle({x,y}, w, h)\n\n (0,0)\n -------------------------------->\n |\n | x------------------\n | | |\n | | |\n | -------------------\n |\n V\n\n ^\n |\n |\n | -------------------\n | | |\n | | |\n | x------------------\n |\n |\n -------------------------------->\n (0,0)\n\nAnd now define the newly constructor proposed by Harvey as the\ntwo-point constructur I suggested.\n\nHope I'm not overthinking this.\n\n> --\n> Kieran","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 B7830C3257\n\tfor ;\n\tMon, 23 Sep 2024 09:20:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5CA546350F;\n\tMon, 23 Sep 2024 11:20:12 +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 75E4B63500\n\tfor ;\n\tMon, 23 Sep 2024 11:20:10 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 15943670;\n\tMon, 23 Sep 2024 11:18:44 +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=\"TmHoM9Bb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727083124;\n\tbh=fIwWKi9nHHeo0Pe8KNvct6GeeSoMoRuKtGIipdfxrx4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TmHoM9Bbea3AcdsEqRUWL4pP+UgnYpVrv8ANzvODAB7rR0F5n3eFGDKmzqlaFoNCB\n\teYiBcf9Bin75oaalQfPd58dMy5YBCnc/HKBFn0Lonu80tBF06xHXX2rMpX/fcodXZh\n\tR5jjloqDgCGY+QbTnYuZyLaGt8qOhj+IimsWCvy8=","Date":"Mon, 23 Sep 2024 11:20:07 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"Cheng-Hao Yang , Jacopo Mondi\n\t, Cheng-Hao Yang\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t, libcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","Message-ID":"","References":"<20240903104018.3289112-2-chenghaoyang@chromium.org>\n\t\n\t<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>\n\t\n\t\n\t\n\t<4ofyijzjlfqdyuq2sz5sbjeao3dmibwvkttdgkxsvikyd66px6@u6lq2nzjdyoi>\n\t\n\t\n\t<172708128719.129190.16780326770350068542@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<172708128719.129190.16780326770350068542@ping.linuxembedded.co.uk>","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":31300,"web_url":"https://patchwork.libcamera.org/comment/31300/","msgid":"","date":"2024-09-23T09:44:32","subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","submitter":{"id":148,"url":"https://patchwork.libcamera.org/api/people/148/","name":"Cheng-Hao Yang","email":"chenghaoyang@google.com"},"content":"Hi Jacopo and Kieran,\n\nUploaded v5. Please take another look.\n\nOn Mon, Sep 23, 2024 at 5:20 PM Jacopo Mondi \nwrote:\n\n> Hi Kieran\n>\n> On Mon, Sep 23, 2024 at 09:48:07AM GMT, Kieran Bingham wrote:\n> > Quoting Jacopo Mondi (2024-09-23 09:32:01)\n> > > Hi Harvey\n> > >\n> > > On Thu, Sep 19, 2024 at 10:15:14PM GMT, Cheng-Hao Yang wrote:\n> > > > Hi Jacopo,\n> > > >\n> > > > On Thu, Sep 19, 2024 at 7:27 PM Jacopo Mondi <\n> jacopo.mondi@ideasonboard.com>\n> > > > wrote:\n> > > >\n> > > > > Hi Harvey\n> > > > >\n> > > > > On Mon, Sep 16, 2024 at 09:24:23AM GMT, Jacopo Mondi wrote:\n> > > > > > Hello\n> > > > > >\n> > > > > > On Mon, Sep 16, 2024 at 12:34:54PM GMT, Cheng-Hao Yang wrote:\n> > > > > > > Hi Barnabás and Jacopo,\n> > > > > > >\n> > > > > > > On Mon, Sep 16, 2024 at 1:27 AM Barnabás Pőcze <\n> pobrn@protonmail.com>\n> > > > > wrote:\n> > > > > > >\n> > > > > > > > Hi\n> > > > > > > >\n> > > > > > > >\n> > > > > > > > 2024. szeptember 5., csütörtök 10:35 keltezéssel, Jacopo\n> Mondi <\n> > > > > > > > jacopo.mondi@ideasonboard.com> írta:\n> > > > > > > >\n> > > > > > > > > Hi Barnabás\n> > > > > > > > >\n> > > > > > > > > On Wed, Sep 04, 2024 at 11:26:37AM GMT, Barnabás Pőcze\n> wrote:\n> > > > > > > > > > Hi\n> > > > > > > > > >\n> > > > > > > > > >\n> > > > > > > > > > 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang\n> <\n> > > > > > > > chenghaoyang@chromium.org> írta:\n> > > > > > > > > >\n> > > > > > > > > > > From: Yudhistira Erlandinata <\n> yerlandinata@chromium.org>\n> > > > > > > > > > >\n> > > > > > > > > > > Add a Rectangle constructor that accepts two points:\n> > > > > > > > > > > topLeft and bottomRight.\n> > > > > > > > > > >\n> > > > > > > > > > > Signed-off-by: Yudhistira Erlandinata <\n> > > > > yerlandinata@chromium.org>\n> > > > > > > > > > > Co-developed-by: Harvey Yang <\n> chenghaoyang@chromium.org>\n> > > > > > > > > > > Reviewed-by: Jacopo Mondi <\n> jacopo.mondi@ideasonboard.com>\n> > > > > > > > > > > ---\n> > > > > > > > > > > include/libcamera/geometry.h | 2 ++\n> > > > > > > > > > > src/libcamera/geometry.cpp | 14 ++++++++++++++\n> > > > > > > > > > > 2 files changed, 16 insertions(+)\n> > > > > > > > > > >\n> > > > > > > > > > > diff --git a/include/libcamera/geometry.h\n> > > > > > > > b/include/libcamera/geometry.h\n> > > > > > > > > > > index 3e6f0f5d7..dc56f180f 100644\n> > > > > > > > > > > --- a/include/libcamera/geometry.h\n> > > > > > > > > > > +++ b/include/libcamera/geometry.h\n> > > > > > > > > > > @@ -262,6 +262,8 @@ public:\n> > > > > > > > > > > {\n> > > > > > > > > > > }\n> > > > > > > > > > >\n> > > > > > > > > > > + constexpr Rectangle(const Point &topLeft, const Point\n> > > > > > > > &bottomRight);\n> > > > > > > > > >\n> > > > > > > > > > Don't make this `constexpr` because it is not useful\n> since the\n> > > > > > > > definition is not available.\n> > > > > > > > >\n> > > > > > > > > I found references online that constexpr constuctors are\n> implcitly\n> > > > > > > > > inline, is this the reason of your comment ?\n> > > > > > > >\n> > > > > > > > Yes.\n> > > > > > > >\n> > > > > > > >\n> > > > > > > > >\n> > > > > > > > > However, I can't find it clearly specified in\n> cppreference. Do you\n> > > > > > > > > have any pointer ?\n> > > > > > > >\n> > > > > > > > \"A constexpr specifier used in a function or static data\n> > > > > member(since\n> > > > > > > > C++17) declaration implies inline.\"\n> > > > > > > > -- https://en.cppreference.com/w/cpp/language/constexpr\n> > > > > > > >\n> > > > > >\n> > > > > >\n> > > > > > One day I'll lear to read properly\n> > > > > >\n> > > > > > > > >\n> > > > > > > > > Anyway, if inline is the reason, isn't it better to inline\n> the\n> > > > > > > > > definition and maintain the constexpr specifier ?\n> > > > > > > > > [...]\n> > > > > > > >\n> > > > > > > > That is an option as well.\n> > > > > > > >\n> > > > > > >\n> > > > > > > IIUC, you're suggesting to put the definition of the new c'tor\n> > > > > > > back in the header file? As we use `ASSERT` in this c'tor\n> > > > > > > definition, there will be a compile error if we do that:\n> > > > > > > ```\n> > > > > > >\n> > > > > > > In file included from ../include/libcamera/base/log.h:12,\n> > > > > > >\n> > > > > > > from ../include/libcamera/geometry.h:16,\n> > > > > > >\n> > > > > > > from ../include/libcamera/controls.h:21,\n> > > > > > >\n> > > > > > > from ../include/libcamera/camera.h:22,\n> > > > > > >\n> > > > > > > from ../src/apps/common/dng_writer.h:13,\n> > > > > > >\n> > > > > > > from ../src/apps/common/dng_writer.cpp:8:\n> > > > > > >\n> > > > > > > ../include/libcamera/base/private.h:21:2: error: #error\n> \"Private\n> > > > > headers\n> > > > > > > must not be included in the libcamera API\"\n> > > > > > >\n> > > > > > > 21 | #error \"Private headers must not be included in the\n> libcamera\n> > > > > API\"\n> > > > > > > ```\n> > > > > >\n> > > > > > I see. log.h is private, and the fact ASSERT is not exposed in\n> the\n> > > > > > public API makes me think aborting execution on an invalid user\n> input\n> > > > > > is probably not the best idea ? That's maybe the reason why\n> ASSERT is not\n> > > > > > exposed to the public API ?\n> > > > > >\n> > > > >\n> > > > > Assuming the top-left corner is always defined as the point with\n> the\n> > > > > smaller 'x' and the higher 'y' whatever the reference system the\n> > > > > rectangle is placed in:\n> > > > >\n> > > > > In example:\n> > > > >\n> > > > > (0,0)\n> > > > > -------------------------------->\n> > > > > |\n> > > > > | -------------------\n> > > > > | | |\n> > > > > | | |\n> > > > > | x------------------\n> > > > > |\n> > > > > V\n> > > > >\n> > > > > Or:\n> > > > >\n> > > > > ^\n> > > > > |\n> > > > > |\n> > > > > | x------------------\n> > > > > | | |\n> > > > > | | |\n> > > > > | -------------------\n> > > > > |\n> > > > > |\n> > > > > -------------------------------->\n> > > > > (0,0)\n> > > > >\n> > > > > The assertion you have introduced in your proposed constructor\n> > > > >\n> > > > > constexpr Rectangle(const Point &topLeft, const Point\n> &bottomRight)\n> > > > > : x(topLeft.x), y(topLeft.y),\n> > > > > width(bottomRight.x - x),\n> > > > > height(bottomRight.y - y)\n> > > > > {\n> > > > > ASSERT(bottomRight.x >= x && bottomRight.y >= y);\n> > > > > }\n> > > > >\n> > > > > is wrong as bottomRight.y shall always be smaller than topLeft.y\n> > > > >\n> > > > > Has this code been ever run ?\n> > > > >\n> > > >\n> > > > Sorry for the confusion, but the `topLeft` point is actually\n> somewhere\n> > > > like this:\n> > > >\n> > > > (0,0)\n> > > > -------------------------------->\n> > > > |\n> > > > | x-----------------\n> > > > | | |\n> > > > | | |\n> > > > | -------------------\n> > > > |\n> > > > V\n> > > >\n> > > > `top-left` is (0, 0) in Android metadata's description [1], and it\n> seems\n> > > > that\n> > > > it's also in libcamera's description to indicate a point with\n> smaller x and\n> > > > y coordinates [2].\n> > > >\n> > > > [1]:\n> > > >\n> https://android.googlesource.com/platform/system/media/+/refs/heads/main/camera/docs/docs.html#33019\n> > > > [2]:\n> > > >\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/geometry.cpp#n639\n> > >\n> > > Is this because of\n> > >\n> > > /**\n> > > * \\fn Rectangle::Rectangle(const Size &size)\n> > > * \\brief Construct a Rectangle of \\a size with its top left corner\n> located\n> > > * at (0,0)\n> > >\n> > > This clearly doesn't match this case\n> > >\n> > > ^\n> > > |\n> > > |\n> > > | x------------------\n> > > | | |\n> > > | | |\n> > > | -------------------\n> > > |\n> > > |\n> > > -------------------------------->\n> > > (0,0)\n> > >\n> > > Maybe we have been a bit short-sighted and only assumed the blelow\n> > > reference system had to be taken into account.\n> > >\n> > > (0,0)\n> > > -------------------------------->\n> > > |\n> > > | x-----------------\n> > > | | |\n> > > | | |\n> > > | -------------------\n> > > |\n> > > V\n> > >\n> > > Personally I would remove this assumption in our doc and define the\n> > > top-left corner as the point with smaller x and largest y\n> > >\n> > > Opinions from libcamera people ?\n> >\n> > Aha, I would define top left as 0,0 :-)\n> >\n> > I've just opened up GIMP which also uses top left origin.\n> >\n> > I expect other image applications would also honour this - so worth\n> > checking a few ...\n> >\n> > For me - even though this is 'geometry' ... it's *image processing*\n> > geometry.\n> >\n>\n> The Rectangle class documentation says already\n>\n> * The measure unit of the rectangle coordinates and size, as well as the\n> * reference point from which the Rectangle::x and Rectangle::y\n> displacements\n> * refers to, are defined by the context were rectangle is used.\n>\n> Which seems to suggest that no assumptions on the reference system are\n> made in the class, but however we also have one constructor that is\n> documented as\n>\n> * \\brief Construct a Rectangle of \\a size with its top left corner located\n> * at (0,0)\n>\n> Which, in the below example suggestes \"top-left\" is actually visually\n> bottom-right.\n>\n\nnit: bottom-left\n\n\n>\n>\n> ^\n> |\n> |\n> | -------------------\n> | | |\n> | | |\n> | X------------------\n> |\n> |\n> -------------------------------->\n> (0,0)\n>\n> If it is instead assumed that \"top-left\" is is the visual top-left\n> regardless of the coordinate system so we should clarify what\n> directions do we increment x and y when give a constructor like\n> Rectangle({x,y}, w, h) as in this case we grow both x and y\n\n\n> (0,0)\n> -------------------------------->\n> |\n> | x------------------\n> | | |\n> | | |\n> | -------------------\n> |\n> V\n>\n\nI think this constructor creates a Rectangle like this:\n\n (0,0)\n -------------------------------->\n | |\n | |\n | |\n | |\n | ------------------(Size.width, Size.height)\n |\n V\n\nWhich is a rectangle surrounded by (0, 0) and\n(Size.width, Size.height).\n\n\n> while in this we increase x and decrement y\n> ^\n> |\n> |\n> | x------------------\n> | | |\n> | | |\n> | -------------------\n> |\n> |\n> -------------------------------->\n> (0,0)\n>\n> but it would be totally legit to construct the same rectangle as\n>\n> ^ -------------------\n> | | |\n> | | |\n> | x------------------\n> |\n> |\n> |\n> |\n> |\n> -------------------------------->\n> (0,0)\n>\n> To remove ambiguities I would rather redefine top-left as \"origin\"\n> point defined as the point wiht the lower x and lower y of the\n> rectangle and clarify that we always increase width and height\n> from there when constructing Rectangle({x,y}, w, h)\n>\n> (0,0)\n> -------------------------------->\n> |\n> | x------------------\n> | | |\n> | | |\n> | -------------------\n> |\n> V\n>\n> ^\n> |\n> |\n> | -------------------\n> | | |\n> | | |\n> | x------------------\n> |\n> |\n> -------------------------------->\n> (0,0)\n>\n> And now define the newly constructor proposed by Harvey as the\n> two-point constructur I suggested.\n>\n\nUpdated it to be two diagonal points. I've updated the variable names\nto align with the original documentation, which `topLeft` means the one\nwith the minimum x and y coordinates.\nLet me know what's the better variable names instead though, if we\nredefine `top-left` as the origin point `(0, 0)`.\n(Or just adjust it for me before accepting the patches, if that's the last\nissue in question).\n\nThanks!\n\n\n>\n> Hope I'm not overthinking this.\n>\n> > --\n> > Kieran\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 9A376C0F1B\n\tfor ;\n\tMon, 23 Sep 2024 09:45:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 888F36350D;\n\tMon, 23 Sep 2024 11:45:11 +0200 (CEST)","from mail-lf1-x130.google.com (mail-lf1-x130.google.com\n\t[IPv6:2a00:1450:4864:20::130])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B7A636037E\n\tfor ;\n\tMon, 23 Sep 2024 11:45:09 +0200 (CEST)","by mail-lf1-x130.google.com with SMTP id\n\t2adb3069b0e04-53661d95508so14471e87.1\n\tfor ;\n\tMon, 23 Sep 2024 02:45:09 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"JOdacWOH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=google.com; s=20230601; t=1727084709; x=1727689509;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=1p9SemiY4O8iiGr1BWH2wJ5m0dPnBlpsWol8XsFaQSI=;\n\tb=JOdacWOHuslaOA//e/JyTmLSpDlsqSpApKkynUx9avf52GtgH1wUVCDXnhxXtxCHsO\n\tPmwcFyAZEDRljDgORMsBB5dc/1iZBCLNtMfUHb4IgLhuj/CGRxCOMc+kIbS6NF+S1uqI\n\tkAVK3WbXCfR3YfTMynpnPSwnjxN3Q093QCwg1CgoUMyhjPCvmrHZhL2hwSUqYPeQeOgz\n\tWuXZyNGPKuM8sK1Nc0+nfgXOy7qaBhZEJCC5XHY6+/7Ij9uPS52gmzrKh2lEKHJQHypF\n\tIbH1veIXm/qGsMpP7FQFUwSyKhbmiKA8lDgP2skR84ZHc8BVeQgUZpSy28o/ir4Ldzej\n\tyEbw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1727084709; x=1727689509;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=1p9SemiY4O8iiGr1BWH2wJ5m0dPnBlpsWol8XsFaQSI=;\n\tb=grji2W2uubtdOrfkTb0R6/4dZYNE/6vqDdC6t+jZLUTpb1fbyu4PHkaYTIq06HJQNN\n\tWPtcIvZMDXdHZ7F7BFHMbfPJW2GOO7HdrKEvn5agbJxcpi0QkujzNsIx3kPyxirQultk\n\tsYv60BfKnruggMhqIPQ+QywAnriakFXn5ZVYs0YzxxpLqRnxoUqExyyV94Cc+XdJXRyk\n\tvkzH8TU1gQ2taC+xE9KBLRbyN+YBZy+valRNHLIgQ8kTqqLXdw7GSu/cTLyZYgnp3Z1l\n\tdBDcsg4Lr08QkkEu6WIJ+Lk4QWAzSDIi92vC/cVsLcu8U7VUoIWuOZ7/d18VIAA9MzLW\n\tMZUw==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVIz+rCl4rUWBszZ6koSb2/eTnzdkH+qtbdViQBpDhLHUUDgJmeLk0Q/2OJ08Vf2ZMG9QptGs913HqjTAL65Sc=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YxY6XTD/Xattw830QtgXbjB1CrRAIxq3RVsC4sGWakhCQPVA9RP\n\tCfVmdz5KwrJLBlH6gFSa5dGWPfMjctsUe3tzvqzQRo0JfedGcoklcZYvQmH0X09EsfzCBJyl4gM\n\tdGX2cTlJX0lnT/5rCiC6fKhWwX5XHss99cCOvktg71kI/mkANeZ3e","X-Google-Smtp-Source":"AGHT+IFk+l4jtyol4gp03YowSZ1mmn+9pVof4TpWVua253lKbMSaKi6JEe5BRP3dxS76719nm0PqLK+qRTTX9H3zz3w=","X-Received":"by 2002:a05:6512:3f1d:b0:535:68b2:9589 with SMTP id\n\t2adb3069b0e04-5374adc4e10mr428564e87.2.1727084708369; Mon, 23 Sep 2024\n\t02:45:08 -0700 (PDT)","MIME-Version":"1.0","References":"<20240903104018.3289112-2-chenghaoyang@chromium.org>\n\t\n\t<7da4aqw6cmb6grwz2uitdjlao4x2o6lpxwjqn3uyasmc3dtbdg@zse6c2m7afnw>\n\t\n\t\n\t\n\t<4ofyijzjlfqdyuq2sz5sbjeao3dmibwvkttdgkxsvikyd66px6@u6lq2nzjdyoi>\n\t\n\t\n\t<172708128719.129190.16780326770350068542@ping.linuxembedded.co.uk>\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Mon, 23 Sep 2024 17:44:32 +0800","Message-ID":"","Subject":"Re: [PATCH v4 1/3] libcamera: Add rectangle two-point constructor","To":"Jacopo Mondi ","Cc":"Kieran Bingham , Cheng-Hao Yang\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t, libcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ","Content-Type":"multipart/alternative; boundary=\"0000000000008b14d30622c63e85\"","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\" "}}]