[{"id":30946,"web_url":"https://patchwork.libcamera.org/comment/30946/","msgid":"","date":"2024-08-28T12:10:38","subject":"Re: [PATCH v2 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\n\nOn Fri, Aug 23, 2024 at 02:29:08PM GMT, Harvey Yang wrote:\n> From: Yudhistira Erlandinata \n>\n> Add a Rectangle constructor that accepts two points:\n> topLeft and bottomRight.\n>\n> BUG=b:308714092\n> TEST=emerge-geralt libcamera-mtkisp7\n\nPlease drop them, not related to libcamera\n\n>\n> Signed-off-by: Yudhistira Erlandinata \n> Co-developed-by: Harvey Yang \n> ---\n> include/libcamera/geometry.h | 10 ++++++++++\n> src/libcamera/geometry.cpp | 11 +++++++++--\n> 2 files changed, 19 insertions(+), 2 deletions(-)\n>\n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index 3e6f0f5d7..978322a22 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -8,7 +8,9 @@\n> #pragma once\n>\n> #include \n> +#include \n> #include \n> +#include \n\nWhat is this for ?\n\n> #include \n>\n> #include \n> @@ -262,6 +264,14 @@ public:\n> \t{\n> \t}\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\nassert() is fine, however we have our own ASSERT() defined in log.h\nwhich can be conditionally disabled. The rest of the codebase uses\nthat.\n\n> +\t}\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..86385ad35 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -5,13 +5,13 @@\n> * Geometry-related structures\n> */\n>\n> -#include \n> -\n> #include \n> #include \n>\n> #include \n>\n> +#include \n> +\n\nwhy ?\n\n> /**\n> * \\file geometry.h\n> * \\brief Data structures related to geometric objects\n> @@ -629,6 +629,13 @@ 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> +\n\nthe rest looks good\n\n> /**\n> * \\var Rectangle::x\n> * \\brief The horizontal coordinate of the rectangle's top-left corner\n> --\n> 2.46.0.295.g3b9ea8a38a-goog\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 ADFA2C323E\n\tfor ;\n\tWed, 28 Aug 2024 12:10:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B8C99633CD;\n\tWed, 28 Aug 2024 14:10:43 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7C6CD61903\n\tfor ;\n\tWed, 28 Aug 2024 14:10:42 +0200 (CEST)","from ideasonboard.com (mob-5-90-141-165.net.vodafone.it\n\t[5.90.141.165])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 92E4E220;\n\tWed, 28 Aug 2024 14:09:34 +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=\"ndybFeLz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1724846974;\n\tbh=Fr0LjU+jvT+XreQYbqvbh19gtbkId5JAQUw0L+J/tZo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ndybFeLzBsC8HC6Y/0PHSJyE806y3BCu5rYxkP+tfs+Fac118ryt//jRfxvW4K8ec\n\tP+9IkSZ0wT4PVz7BNiw/r1NiBEK8WZfkXWDm9C69HfX3pjInGhuRiGddzjPZS1UQd+\n\tSfPBjbr3WWes7KiouUD/IW7rEjltr9hV7b4pxaO4=","Date":"Wed, 28 Aug 2024 14:10:38 +0200","From":"Jacopo Mondi ","To":"Harvey Yang ","Cc":"libcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ","Subject":"Re: [PATCH v2 1/3] libcamera: Add rectangle two-point constructor","Message-ID":"","References":"<20240823143205.2668765-1-chenghaoyang@google.com>\n\t<20240823143205.2668765-2-chenghaoyang@google.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240823143205.2668765-2-chenghaoyang@google.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30997,"web_url":"https://patchwork.libcamera.org/comment/30997/","msgid":"","date":"2024-08-30T21:04:21","subject":"Re: [PATCH v2 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 Jacopo,\n\nOn Wed, Aug 28, 2024 at 2:10 PM Jacopo Mondi \nwrote:\n\n> Hi\n>\n> On Fri, Aug 23, 2024 at 02:29:08PM GMT, Harvey Yang wrote:\n> > From: Yudhistira Erlandinata \n> >\n> > Add a Rectangle constructor that accepts two points:\n> > topLeft and bottomRight.\n> >\n> > BUG=b:308714092\n> > TEST=emerge-geralt libcamera-mtkisp7\n>\n> Please drop them, not related to libcamera\n>\nRemoved.\n\n\n>\n> >\n> > Signed-off-by: Yudhistira Erlandinata \n> > Co-developed-by: Harvey Yang \n> > ---\n> > include/libcamera/geometry.h | 10 ++++++++++\n> > src/libcamera/geometry.cpp | 11 +++++++++--\n> > 2 files changed, 19 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > index 3e6f0f5d7..978322a22 100644\n> > --- a/include/libcamera/geometry.h\n> > +++ b/include/libcamera/geometry.h\n> > @@ -8,7 +8,9 @@\n> > #pragma once\n> >\n> > #include \n> > +#include \n> > #include \n> > +#include \n>\n> What is this for ?\n>\nRemoved.\n\n>\n> > #include \n> >\n> > #include \n> > @@ -262,6 +264,14 @@ public:\n> > {\n> > }\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> assert() is fine, however we have our own ASSERT() defined in log.h\n> which can be conditionally disabled. The rest of the codebase uses\n> that.\n>\nRight, thanks! Done.\n\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..86385ad35 100644\n> > --- a/src/libcamera/geometry.cpp\n> > +++ b/src/libcamera/geometry.cpp\n> > @@ -5,13 +5,13 @@\n> > * Geometry-related structures\n> > */\n> >\n> > -#include \n> > -\n> > #include \n> > #include \n> >\n> > #include \n> >\n> > +#include \n> > +\n>\n> why ?\n>\nMy linter went crazy... Reverted.\n\n\n>\n> > /**\n> > * \\file geometry.h\n> > * \\brief Data structures related to geometric objects\n> > @@ -629,6 +629,13 @@ 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> > +\n>\n> the rest looks good\n>\n> > /**\n> > * \\var Rectangle::x\n> > * \\brief The horizontal coordinate of the rectangle's top-left corner\n> > --\n> > 2.46.0.295.g3b9ea8a38a-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 C3B01C323E\n\tfor ;\n\tFri, 30 Aug 2024 21:04:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 77E1563466;\n\tFri, 30 Aug 2024 23:04:35 +0200 (CEST)","from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com\n\t[IPv6:2a00:1450:4864:20::22f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4635E61E4A\n\tfor ;\n\tFri, 30 Aug 2024 23:04:33 +0200 (CEST)","by mail-lj1-x22f.google.com with SMTP id\n\t38308e7fff4ca-2f4f5dbd93bso20318911fa.2\n\tfor ;\n\tFri, 30 Aug 2024 14:04:33 -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=\"SKnEvWrh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1725051872; x=1725656672;\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=g/uKtIhRRlUF8+z4yemUwT2ieqh+8VDF/zd7o5hXaJA=;\n\tb=SKnEvWrhc+SUPgzwRYf84NSyi0NwrBZEAdcyYZbLZ4UJ3Pm90tlXOI67k0ys+DBj2G\n\trIcKhUsWxzO7eWROW9yEq6pM8+3J2STVSMZtb7Wf6xhQGyOZOheyeOdQ5ufVYuvri+5f\n\tBzgkkvA4dVLhDIH+ddpEYWIleza1MjqyETOSc=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1725051872; x=1725656672;\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=g/uKtIhRRlUF8+z4yemUwT2ieqh+8VDF/zd7o5hXaJA=;\n\tb=wU8iLT79XvRtbl6JjVMSKXqlVczNqeZ6xjoG4mgH/9eOhNOSn2OB47cdToxdjP3lMb\n\tSyGrVoF+r2gcJZcwMlljRYSU53lzHXRIKGF+T7juqAtBG3m0v32G2z9yoc7yzJKecC6n\n\teggib6pG6Y9GBrFjD9AJyYF1ZjwDfjoUIR5xOF1ZTwYrET2QWflMzdXC34jw/H3e4ZCa\n\tpnQAzUJOhpkBJ6KWVeymZBYF882FlIMhHDchYQ1xgj3oRonP4EhQIR7jg4eY9MtkmGky\n\tNKo1zgRsGp4SS7j/2Lc1aAMxd9c0ll30XYVMNuO4ezZLlHdPeSjOpkILffPjUJlSMrBd\n\tG7/A==","X-Gm-Message-State":"AOJu0YxESFv5OhZs1jp07YP6flAf5j61SGxxlQO80WM9JboO/tFvD3OG\n\tJBSWZJ4cv9n6/QdibHhPGhLjrLZlKOjSy8hdWzKgxnSE4sOYANSD26tvj05qS7LLTpTbZUqxK/h\n\tOyNvOo/RlsMfOnWNaHZBYBqr47Wu1jQCjZeL/DRI2Itd8f1ngwLCU","X-Google-Smtp-Source":"AGHT+IEEi2Tcujv8NJWBNNL34EIb9NG0Y8gZu0VMgQoNeKcqHVM/+9hXAPTIEFtyfdQmO1ZZ7vmCDskNsBz7tXWs7Ow=","X-Received":"by 2002:a2e:a9a1:0:b0:2ef:2cdb:5053 with SMTP id\n\t38308e7fff4ca-2f6104cedf5mr58597361fa.37.1725051872432;\n\tFri, 30 Aug 2024 14:04:32 -0700 (PDT)","MIME-Version":"1.0","References":"<20240823143205.2668765-1-chenghaoyang@google.com>\n\t<20240823143205.2668765-2-chenghaoyang@google.com>\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Fri, 30 Aug 2024 23:04:21 +0200","Message-ID":"","Subject":"Re: [PATCH v2 1/3] libcamera: Add rectangle two-point constructor","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ","Content-Type":"multipart/alternative; boundary=\"00000000000013717e0620ecf0d0\"","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\" "}}]