[{"id":5399,"web_url":"https://patchwork.libcamera.org/comment/5399/","msgid":"<1338d183-9168-c8d7-8523-6e51cbbdfd48@uajain.com>","date":"2020-06-25T04:31:21","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Laurent,\n\nThanks for the patch.\n\nOn 6/25/20 6:53 AM, Laurent Pinchart wrote:\n> It's common for code to check if a size is null. Add a helper function\n> to do so.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> Changes since v1:\n>\n> - Add test\n> ---\n> include/libcamera/geometry.h | 1 +\n> src/libcamera/geometry.cpp | 6 ++++++\n> test/geometry.cpp | 10 ++++++++++\n> 3 files changed, 17 insertions(+)\n>\n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index edda42cf34cc..7d4b8bcfe3d8 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -41,6 +41,7 @@ struct Size {\n> \tunsigned int width;\n> \tunsigned int height;\n> \n> +\tbool isNull() const { return !width && !height; }\n> \tconst std::string toString() const;\n> };\n> \n> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> index fd78cf2c0ab7..24c44fb43acf 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -107,6 +107,12 @@ bool operator==(const Rectangle &lhs, const Rectangle &rhs)\n> * \\brief The Size height\n> */\n> \n> +/**\n> + * \\fn bool Size::isNull() const\n> + * \\brief Check if the size is null\n> + * \\return True if both the width and height are 0, or false otherwise\n> + */\n> +\n> /**\n> * \\brief Assemble and return a string describing the size\n> * \\return A string describing the size\n> diff --git a/test/geometry.cpp b/test/geometry.cpp\n> index 27e65565d7c6..904ad92c9448 100644\n> --- a/test/geometry.cpp\n> +++ b/test/geometry.cpp\n> @@ -36,6 +36,16 @@ protected:\n> \n> \tint run()\n> \t{\n> +\t\tif (!Size().isNull() || !Size(0, 0).isNull()) {\n> +\t\t\tcout << \"Null size incorrectly reported as not null\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (Size(0, 100).isNull() || Size(100, 0).isNull() || Size(100, 100).isNull()) {\n> +\t\t\tcout << \"Non-null size incorrectly reported as null\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> \t\t/* Test Size equality and inequality. */\n> \t\tif (!compare(Size(100, 100), Size(100, 100), &operator==, \"==\", true))\n> \t\t\treturn TestFail;\nLooks good to me.\n\nReviewed-by: Umang Jain ","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 2429FC0101\n\tfor ;\n\tThu, 25 Jun 2020 04:31:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8879C609A5;\n\tThu, 25 Jun 2020 06:31:24 +0200 (CEST)","from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B7414603B5\n\tfor ;\n\tThu, 25 Jun 2020 06:31:22 +0200 (CEST)","by filterdrecv-p3mdw1-6dd9dd6d45-7kcmw with SMTP id\n\tfilterdrecv-p3mdw1-6dd9dd6d45-7kcmw-19-5EF42899-6\n\t2020-06-25 04:31:21.273383526 +0000 UTC m=+556126.407612459","from mail.uajain.com (unknown)\n\tby ismtpd0004p1maa1.sendgrid.net (SG) with ESMTP\n\tid s7beQBLQSCqlDiiKmuUyeQ Thu, 25 Jun 2020 04:31:20.901 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"gN6heeGr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com;\n\th=subject:references:from:mime-version:in-reply-to:to:content-type:\n\tcontent-transfer-encoding;\n\ts=s1; bh=vB23qAvWt58hw0yvhxjjQDymXmX+P80DVUzLFoEQfiM=;\n\tb=gN6heeGrk6g2yqYv9prVdtvVjoJLWAA+w5u6pEX00rjfeWoExWAZmJRqH+gCubPSLCfr\n\tsNB7k20t4HrD8AkpsmRb0bFemmTyIkKPkUlEWXuw7s5A1M9z4U6Zo89cg508KlCOqFgtu+\n\txKY6Fq7uJskNK8UGy+nEx3qUM59daNEqo=","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>","From":"Umang Jain ","Message-ID":"<1338d183-9168-c8d7-8523-6e51cbbdfd48@uajain.com>","Date":"Thu, 25 Jun 2020 04:31:21 +0000 (UTC)","Mime-Version":"1.0","In-Reply-To":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>","X-SG-EID":"1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPcpFdq5YgiOsiT/6hp/evDR/BRUTq8lVmkCptvuEaoxHdrUKj8IiG0xKkI+CEucToPcdGrUnmJO17LG6vMxCQ1fevEKkJvsins8fbi/xbekgQpNp5D6d0fUyLa/fqyRQu0WA9UuA12LNuSmajdgIqznHxOhYYF6lpXdY/ApPi78rsUv+db9F6iztw/KQJ0YMr7Xh/Nihi9iih6a7tAEQvRfw==","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":5404,"web_url":"https://patchwork.libcamera.org/comment/5404/","msgid":"<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>","date":"2020-06-25T07:37:39","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Thu, Jun 25, 2020 at 04:23:30AM +0300, Laurent Pinchart wrote:\n> It's common for code to check if a size is null. Add a helper function\n> to do so.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> Changes since v1:\n>\n> - Add test\n> ---\n> include/libcamera/geometry.h | 1 +\n> src/libcamera/geometry.cpp | 6 ++++++\n> test/geometry.cpp | 10 ++++++++++\n> 3 files changed, 17 insertions(+)\n>\n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index edda42cf34cc..7d4b8bcfe3d8 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -41,6 +41,7 @@ struct Size {\n> \tunsigned int width;\n> \tunsigned int height;\n>\n> +\tbool isNull() const { return !width && !height; }\n\nIs isNull() a good name ? We have isValid() for PixelFormat() should\nwe stay consistent ?\n\nOtherwise\nReviewed-by: Jacopo Mondi \n\n> \tconst std::string toString() const;\n> };\n>\n> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> index fd78cf2c0ab7..24c44fb43acf 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -107,6 +107,12 @@ bool operator==(const Rectangle &lhs, const Rectangle &rhs)\n> * \\brief The Size height\n> */\n>\n> +/**\n> + * \\fn bool Size::isNull() const\n> + * \\brief Check if the size is null\n> + * \\return True if both the width and height are 0, or false otherwise\n> + */\n> +\n> /**\n> * \\brief Assemble and return a string describing the size\n> * \\return A string describing the size\n> diff --git a/test/geometry.cpp b/test/geometry.cpp\n> index 27e65565d7c6..904ad92c9448 100644\n> --- a/test/geometry.cpp\n> +++ b/test/geometry.cpp\n> @@ -36,6 +36,16 @@ protected:\n>\n> \tint run()\n> \t{\n> +\t\tif (!Size().isNull() || !Size(0, 0).isNull()) {\n> +\t\t\tcout << \"Null size incorrectly reported as not null\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (Size(0, 100).isNull() || Size(100, 0).isNull() || Size(100, 100).isNull()) {\n> +\t\t\tcout << \"Non-null size incorrectly reported as null\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> \t\t/* Test Size equality and inequality. */\n> \t\tif (!compare(Size(100, 100), Size(100, 100), &operator==, \"==\", true))\n> \t\t\treturn TestFail;\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 2303FC0101\n\tfor ;\n\tThu, 25 Jun 2020 07:34:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 83188609B3;\n\tThu, 25 Jun 2020 09:34:13 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 74335609A3\n\tfor ;\n\tThu, 25 Jun 2020 09:34:12 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id EEF44E0002;\n\tThu, 25 Jun 2020 07:34:11 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 25 Jun 2020 09:37:39 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":5416,"web_url":"https://patchwork.libcamera.org/comment/5416/","msgid":"<20200625085705.GB5865@pendragon.ideasonboard.com>","date":"2020-06-25T08:57:05","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Jun 25, 2020 at 09:37:39AM +0200, Jacopo Mondi wrote:\n> On Thu, Jun 25, 2020 at 04:23:30AM +0300, Laurent Pinchart wrote:\n> > It's common for code to check if a size is null. Add a helper function\n> > to do so.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Changes since v1:\n> >\n> > - Add test\n> > ---\n> > include/libcamera/geometry.h | 1 +\n> > src/libcamera/geometry.cpp | 6 ++++++\n> > test/geometry.cpp | 10 ++++++++++\n> > 3 files changed, 17 insertions(+)\n> >\n> > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > index edda42cf34cc..7d4b8bcfe3d8 100644\n> > --- a/include/libcamera/geometry.h\n> > +++ b/include/libcamera/geometry.h\n> > @@ -41,6 +41,7 @@ struct Size {\n> > \tunsigned int width;\n> > \tunsigned int height;\n> >\n> > +\tbool isNull() const { return !width && !height; }\n> \n> Is isNull() a good name ? We have isValid() for PixelFormat() should\n> we stay consistent ?\n\nI initially had isEmpty(), and I've taken inspiration from QRect the\ndefines isNull() as !width && !height (or rather width <= 0 && height <=\n0, but our width and height are unsigned), and isEmpty() as !width ||\n!height.\n\n> Otherwise\n> Reviewed-by: Jacopo Mondi \n> \n> > \tconst std::string toString() const;\n> > };\n> >\n> > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> > index fd78cf2c0ab7..24c44fb43acf 100644\n> > --- a/src/libcamera/geometry.cpp\n> > +++ b/src/libcamera/geometry.cpp\n> > @@ -107,6 +107,12 @@ bool operator==(const Rectangle &lhs, const Rectangle &rhs)\n> > * \\brief The Size height\n> > */\n> >\n> > +/**\n> > + * \\fn bool Size::isNull() const\n> > + * \\brief Check if the size is null\n> > + * \\return True if both the width and height are 0, or false otherwise\n> > + */\n> > +\n> > /**\n> > * \\brief Assemble and return a string describing the size\n> > * \\return A string describing the size\n> > diff --git a/test/geometry.cpp b/test/geometry.cpp\n> > index 27e65565d7c6..904ad92c9448 100644\n> > --- a/test/geometry.cpp\n> > +++ b/test/geometry.cpp\n> > @@ -36,6 +36,16 @@ protected:\n> >\n> > \tint run()\n> > \t{\n> > +\t\tif (!Size().isNull() || !Size(0, 0).isNull()) {\n> > +\t\t\tcout << \"Null size incorrectly reported as not null\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (Size(0, 100).isNull() || Size(100, 0).isNull() || Size(100, 100).isNull()) {\n> > +\t\t\tcout << \"Non-null size incorrectly reported as null\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > \t\t/* Test Size equality and inequality. */\n> > \t\tif (!compare(Size(100, 100), Size(100, 100), &operator==, \"==\", true))\n> > \t\t\treturn TestFail;","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 919B7C0101\n\tfor ;\n\tThu, 25 Jun 2020 08:57:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2BB4F609C3;\n\tThu, 25 Jun 2020 10:57:08 +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 5AD83609A5\n\tfor ;\n\tThu, 25 Jun 2020 10:57:07 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CE6EA521;\n\tThu, 25 Jun 2020 10:57:06 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VXa6EsdV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593075427;\n\tbh=Wj+N+Jq1ESOSvfpNfgpkygCivM++aXmKgY7OQk/zeVA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VXa6EsdVnFDfbI1fo02VEASEUFP7Vf8124S5YQm5yN5Vpl2QwopiaR2qv5E46TmB7\n\tNwjedjJbqM0//0m0pJnUJBYIFgR5le50PZDHagtvurA4SPW7CnAsy+Ht8NlKVTcWhy\n\t1TxqcCg9jeroFW17dgeVYc6Qj0vgBGHmITnnoCjw=","Date":"Thu, 25 Jun 2020 11:57:05 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200625085705.GB5865@pendragon.ideasonboard.com>","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>\n\t<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":5418,"web_url":"https://patchwork.libcamera.org/comment/5418/","msgid":"<20200625090733.j3eeq5bb3rhr632m@uno.localdomain>","date":"2020-06-25T09:07:33","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Jun 25, 2020 at 11:57:05AM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Thu, Jun 25, 2020 at 09:37:39AM +0200, Jacopo Mondi wrote:\n> > On Thu, Jun 25, 2020 at 04:23:30AM +0300, Laurent Pinchart wrote:\n> > > It's common for code to check if a size is null. Add a helper function\n> > > to do so.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > Changes since v1:\n> > >\n> > > - Add test\n> > > ---\n> > > include/libcamera/geometry.h | 1 +\n> > > src/libcamera/geometry.cpp | 6 ++++++\n> > > test/geometry.cpp | 10 ++++++++++\n> > > 3 files changed, 17 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > > index edda42cf34cc..7d4b8bcfe3d8 100644\n> > > --- a/include/libcamera/geometry.h\n> > > +++ b/include/libcamera/geometry.h\n> > > @@ -41,6 +41,7 @@ struct Size {\n> > > \tunsigned int width;\n> > > \tunsigned int height;\n> > >\n> > > +\tbool isNull() const { return !width && !height; }\n> >\n> > Is isNull() a good name ? We have isValid() for PixelFormat() should\n> > we stay consistent ?\n>\n> I initially had isEmpty(), and I've taken inspiration from QRect the\n> defines isNull() as !width && !height (or rather width <= 0 && height <=\n> 0, but our width and height are unsigned), and isEmpty() as !width ||\n> !height.\n>\n\nI was more concerned about the fact we have a number of classes using\nisValid() already, and consistency with the library is important,\notherwise it's a constant \"go to look at the class definition\", much\nsimilar to when we had Rectangle::width and Size::w\n\ninclude/libcamera/file_descriptor.h: bool isValid() const { return fd_ != nullptr; }\ninclude/libcamera/internal/formats.h: bool isValid() const { return format.isValid(); }\ninclude/libcamera/internal/ipa_module.h: bool isValid() const;\ninclude/libcamera/internal/ipa_proxy.h: bool isValid() const { return valid_; }\ninclude/libcamera/internal/pub_key.h: bool isValid() const { return valid_; }\ninclude/libcamera/internal/v4l2_pixelformat.h: bool isValid() const { return fourcc_ != 0; }\ninclude/libcamera/pixel_format.h: constexpr bool isValid() const { return fourcc_ != 0; }\n\nFor the record we have one isEmpty()\nnclude/libcamera/internal/formats.h: bool isEmpty() const;\n\nbut no isNull(). I would avoid introducing another term for a similar,\nif not the same, concept.\n\nThanks\n j\n\n> > Otherwise\n> > Reviewed-by: Jacopo Mondi \n> >\n> > > \tconst std::string toString() const;\n> > > };\n> > >\n> > > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> > > index fd78cf2c0ab7..24c44fb43acf 100644\n> > > --- a/src/libcamera/geometry.cpp\n> > > +++ b/src/libcamera/geometry.cpp\n> > > @@ -107,6 +107,12 @@ bool operator==(const Rectangle &lhs, const Rectangle &rhs)\n> > > * \\brief The Size height\n> > > */\n> > >\n> > > +/**\n> > > + * \\fn bool Size::isNull() const\n> > > + * \\brief Check if the size is null\n> > > + * \\return True if both the width and height are 0, or false otherwise\n> > > + */\n> > > +\n> > > /**\n> > > * \\brief Assemble and return a string describing the size\n> > > * \\return A string describing the size\n> > > diff --git a/test/geometry.cpp b/test/geometry.cpp\n> > > index 27e65565d7c6..904ad92c9448 100644\n> > > --- a/test/geometry.cpp\n> > > +++ b/test/geometry.cpp\n> > > @@ -36,6 +36,16 @@ protected:\n> > >\n> > > \tint run()\n> > > \t{\n> > > +\t\tif (!Size().isNull() || !Size(0, 0).isNull()) {\n> > > +\t\t\tcout << \"Null size incorrectly reported as not null\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > +\t\tif (Size(0, 100).isNull() || Size(100, 0).isNull() || Size(100, 100).isNull()) {\n> > > +\t\t\tcout << \"Non-null size incorrectly reported as null\" << endl;\n> > > +\t\t\treturn TestFail;\n> > > +\t\t}\n> > > +\n> > > \t\t/* Test Size equality and inequality. */\n> > > \t\tif (!compare(Size(100, 100), Size(100, 100), &operator==, \"==\", true))\n> > > \t\t\treturn TestFail;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0B2DEC0100\n\tfor ;\n\tThu, 25 Jun 2020 09:04:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA440609C3;\n\tThu, 25 Jun 2020 11:04:06 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4A8DB609A5\n\tfor ;\n\tThu, 25 Jun 2020 11:04:06 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 74688240018;\n\tThu, 25 Jun 2020 09:04:05 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 25 Jun 2020 11:07:33 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200625090733.j3eeq5bb3rhr632m@uno.localdomain>","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>\n\t<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>\n\t<20200625085705.GB5865@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200625085705.GB5865@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10850,"web_url":"https://patchwork.libcamera.org/comment/10850/","msgid":"<20200625090739.GD5865@pendragon.ideasonboard.com>","date":"2020-06-25T09:07:39","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Jun 25, 2020 at 11:07:33AM +0200, Jacopo Mondi wrote:\n> On Thu, Jun 25, 2020 at 11:57:05AM +0300, Laurent Pinchart wrote:\n> > On Thu, Jun 25, 2020 at 09:37:39AM +0200, Jacopo Mondi wrote:\n> > > On Thu, Jun 25, 2020 at 04:23:30AM +0300, Laurent Pinchart wrote:\n> > > > It's common for code to check if a size is null. Add a helper function\n> > > > to do so.\n> > > >\n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > Changes since v1:\n> > > >\n> > > > - Add test\n> > > > ---\n> > > > include/libcamera/geometry.h | 1 +\n> > > > src/libcamera/geometry.cpp | 6 ++++++\n> > > > test/geometry.cpp | 10 ++++++++++\n> > > > 3 files changed, 17 insertions(+)\n> > > >\n> > > > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > > > index edda42cf34cc..7d4b8bcfe3d8 100644\n> > > > --- a/include/libcamera/geometry.h\n> > > > +++ b/include/libcamera/geometry.h\n> > > > @@ -41,6 +41,7 @@ struct Size {\n> > > > \tunsigned int width;\n> > > > \tunsigned int height;\n> > > >\n> > > > +\tbool isNull() const { return !width && !height; }\n> > >\n> > > Is isNull() a good name ? We have isValid() for PixelFormat() should\n> > > we stay consistent ?\n> >\n> > I initially had isEmpty(), and I've taken inspiration from QRect the\n> > defines isNull() as !width && !height (or rather width <= 0 && height <=\n> > 0, but our width and height are unsigned), and isEmpty() as !width ||\n> > !height.\n> \n> I was more concerned about the fact we have a number of classes using\n> isValid() already, and consistency with the library is important,\n> otherwise it's a constant \"go to look at the class definition\", much\n> similar to when we had Rectangle::width and Size::w\n> \n> include/libcamera/file_descriptor.h: bool isValid() const { return fd_ != nullptr; }\n> include/libcamera/internal/formats.h: bool isValid() const { return format.isValid(); }\n> include/libcamera/internal/ipa_module.h: bool isValid() const;\n> include/libcamera/internal/ipa_proxy.h: bool isValid() const { return valid_; }\n> include/libcamera/internal/pub_key.h: bool isValid() const { return valid_; }\n> include/libcamera/internal/v4l2_pixelformat.h: bool isValid() const { return fourcc_ != 0; }\n> include/libcamera/pixel_format.h: constexpr bool isValid() const { return fourcc_ != 0; }\n> \n> For the record we have one isEmpty()\n> nclude/libcamera/internal/formats.h: bool isEmpty() const;\n> \n> but no isNull(). I would avoid introducing another term for a similar,\n> if not the same, concept.\n\nThat boils down to the question of is a Size(0, 0) \"invalid\" ?\n\n> > > Otherwise\n> > > Reviewed-by: Jacopo Mondi \n> > >\n> > > > \tconst std::string toString() const;\n> > > > };\n> > > >\n> > > > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> > > > index fd78cf2c0ab7..24c44fb43acf 100644\n> > > > --- a/src/libcamera/geometry.cpp\n> > > > +++ b/src/libcamera/geometry.cpp\n> > > > @@ -107,6 +107,12 @@ bool operator==(const Rectangle &lhs, const Rectangle &rhs)\n> > > > * \\brief The Size height\n> > > > */\n> > > >\n> > > > +/**\n> > > > + * \\fn bool Size::isNull() const\n> > > > + * \\brief Check if the size is null\n> > > > + * \\return True if both the width and height are 0, or false otherwise\n> > > > + */\n> > > > +\n> > > > /**\n> > > > * \\brief Assemble and return a string describing the size\n> > > > * \\return A string describing the size\n> > > > diff --git a/test/geometry.cpp b/test/geometry.cpp\n> > > > index 27e65565d7c6..904ad92c9448 100644\n> > > > --- a/test/geometry.cpp\n> > > > +++ b/test/geometry.cpp\n> > > > @@ -36,6 +36,16 @@ protected:\n> > > >\n> > > > \tint run()\n> > > > \t{\n> > > > +\t\tif (!Size().isNull() || !Size(0, 0).isNull()) {\n> > > > +\t\t\tcout << \"Null size incorrectly reported as not null\" << endl;\n> > > > +\t\t\treturn TestFail;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\tif (Size(0, 100).isNull() || Size(100, 0).isNull() || Size(100, 100).isNull()) {\n> > > > +\t\t\tcout << \"Non-null size incorrectly reported as null\" << endl;\n> > > > +\t\t\treturn TestFail;\n> > > > +\t\t}\n> > > > +\n> > > > \t\t/* Test Size equality and inequality. */\n> > > > \t\tif (!compare(Size(100, 100), Size(100, 100), &operator==, \"==\", true))\n> > > > \t\t\treturn TestFail;","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 A5A64C0109\n\tfor ;\n\tThu, 25 Jun 2020 09:07:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2242F609B3;\n\tThu, 25 Jun 2020 11:07:42 +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 975CF609A3\n\tfor ;\n\tThu, 25 Jun 2020 11:07:41 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 11065521;\n\tThu, 25 Jun 2020 11:07:40 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VQ+SVTqh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593076061;\n\tbh=lWdtNe2AsSE62ap/7rT0BvpJ1TLL59W0hTofdeqYcZQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VQ+SVTqhDAC61IOhrkVpyteZJ1B5cZWXRPM/PKk6upmibr0qNF20FS/QzjI3w2oJ/\n\tsG+Pm8rDlFU7c24uqWktj9OQ8wA4r859ZuQQUupZTHXDszg3/2UbvCr3Wp83jmj2c4\n\tI9WhQ+PeP/rwP1NUGrIi97mZ9bimjyMTfA+SxvCQ=","Date":"Thu, 25 Jun 2020 12:07:39 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200625090739.GD5865@pendragon.ideasonboard.com>","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>\n\t<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>\n\t<20200625085705.GB5865@pendragon.ideasonboard.com>\n\t<20200625090733.j3eeq5bb3rhr632m@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200625090733.j3eeq5bb3rhr632m@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10852,"web_url":"https://patchwork.libcamera.org/comment/10852/","msgid":"<20200625092617.ciw3rdifn63lmsht@uno.localdomain>","date":"2020-06-25T09:26:17","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Jun 25, 2020 at 12:07:39PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Thu, Jun 25, 2020 at 11:07:33AM +0200, Jacopo Mondi wrote:\n> > On Thu, Jun 25, 2020 at 11:57:05AM +0300, Laurent Pinchart wrote:\n> > > On Thu, Jun 25, 2020 at 09:37:39AM +0200, Jacopo Mondi wrote:\n> > > > On Thu, Jun 25, 2020 at 04:23:30AM +0300, Laurent Pinchart wrote:\n> > > > > It's common for code to check if a size is null. Add a helper function\n> > > > > to do so.\n> > > > >\n> > > > > Signed-off-by: Laurent Pinchart \n> > > > > ---\n> > > > > Changes since v1:\n> > > > >\n> > > > > - Add test\n> > > > > ---\n> > > > > include/libcamera/geometry.h | 1 +\n> > > > > src/libcamera/geometry.cpp | 6 ++++++\n> > > > > test/geometry.cpp | 10 ++++++++++\n> > > > > 3 files changed, 17 insertions(+)\n> > > > >\n> > > > > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > > > > index edda42cf34cc..7d4b8bcfe3d8 100644\n> > > > > --- a/include/libcamera/geometry.h\n> > > > > +++ b/include/libcamera/geometry.h\n> > > > > @@ -41,6 +41,7 @@ struct Size {\n> > > > > \tunsigned int width;\n> > > > > \tunsigned int height;\n> > > > >\n> > > > > +\tbool isNull() const { return !width && !height; }\n> > > >\n> > > > Is isNull() a good name ? We have isValid() for PixelFormat() should\n> > > > we stay consistent ?\n> > >\n> > > I initially had isEmpty(), and I've taken inspiration from QRect the\n> > > defines isNull() as !width && !height (or rather width <= 0 && height <=\n> > > 0, but our width and height are unsigned), and isEmpty() as !width ||\n> > > !height.\n> >\n> > I was more concerned about the fact we have a number of classes using\n> > isValid() already, and consistency with the library is important,\n> > otherwise it's a constant \"go to look at the class definition\", much\n> > similar to when we had Rectangle::width and Size::w\n> >\n> > include/libcamera/file_descriptor.h: bool isValid() const { return fd_ != nullptr; }\n> > include/libcamera/internal/formats.h: bool isValid() const { return format.isValid(); }\n> > include/libcamera/internal/ipa_module.h: bool isValid() const;\n> > include/libcamera/internal/ipa_proxy.h: bool isValid() const { return valid_; }\n> > include/libcamera/internal/pub_key.h: bool isValid() const { return valid_; }\n> > include/libcamera/internal/v4l2_pixelformat.h: bool isValid() const { return fourcc_ != 0; }\n> > include/libcamera/pixel_format.h: constexpr bool isValid() const { return fourcc_ != 0; }\n> >\n> > For the record we have one isEmpty()\n> > nclude/libcamera/internal/formats.h: bool isEmpty() const;\n> >\n> > but no isNull(). I would avoid introducing another term for a similar,\n> > if not the same, concept.\n>\n> That boils down to the question of is a Size(0, 0) \"invalid\" ?\n\nSorry about this, but how would you define a \"valid\" size ? To me\nwhatever size that has -both- width and height > 0 is valid.\nEverything else is not. I don't really see a need for a distinction\nbetween a \"valid\" (w || h) and a \"!null\" (or zero) one (w && h).\n\nA size with any of its members set to 0 is not valid. Is this too\nharsh ?\n\n>\n> > > > Otherwise\n> > > > Reviewed-by: Jacopo Mondi \n> > > >\n> > > > > \tconst std::string toString() const;\n> > > > > };\n> > > > >\n> > > > > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> > > > > index fd78cf2c0ab7..24c44fb43acf 100644\n> > > > > --- a/src/libcamera/geometry.cpp\n> > > > > +++ b/src/libcamera/geometry.cpp\n> > > > > @@ -107,6 +107,12 @@ bool operator==(const Rectangle &lhs, const Rectangle &rhs)\n> > > > > * \\brief The Size height\n> > > > > */\n> > > > >\n> > > > > +/**\n> > > > > + * \\fn bool Size::isNull() const\n> > > > > + * \\brief Check if the size is null\n> > > > > + * \\return True if both the width and height are 0, or false otherwise\n> > > > > + */\n> > > > > +\n> > > > > /**\n> > > > > * \\brief Assemble and return a string describing the size\n> > > > > * \\return A string describing the size\n> > > > > diff --git a/test/geometry.cpp b/test/geometry.cpp\n> > > > > index 27e65565d7c6..904ad92c9448 100644\n> > > > > --- a/test/geometry.cpp\n> > > > > +++ b/test/geometry.cpp\n> > > > > @@ -36,6 +36,16 @@ protected:\n> > > > >\n> > > > > \tint run()\n> > > > > \t{\n> > > > > +\t\tif (!Size().isNull() || !Size(0, 0).isNull()) {\n> > > > > +\t\t\tcout << \"Null size incorrectly reported as not null\" << endl;\n> > > > > +\t\t\treturn TestFail;\n> > > > > +\t\t}\n> > > > > +\n> > > > > +\t\tif (Size(0, 100).isNull() || Size(100, 0).isNull() || Size(100, 100).isNull()) {\n> > > > > +\t\t\tcout << \"Non-null size incorrectly reported as null\" << endl;\n> > > > > +\t\t\treturn TestFail;\n> > > > > +\t\t}\n> > > > > +\n> > > > > \t\t/* Test Size equality and inequality. */\n> > > > > \t\tif (!compare(Size(100, 100), Size(100, 100), &operator==, \"==\", true))\n> > > > > \t\t\treturn TestFail;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C0F18C0109\n\tfor ;\n\tThu, 25 Jun 2020 09:22:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 37EC0609C4;\n\tThu, 25 Jun 2020 11:22:51 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 36295609A3\n\tfor ;\n\tThu, 25 Jun 2020 11:22:50 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id B97272000B;\n\tThu, 25 Jun 2020 09:22:49 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 25 Jun 2020 11:26:17 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200625092617.ciw3rdifn63lmsht@uno.localdomain>","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>\n\t<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>\n\t<20200625085705.GB5865@pendragon.ideasonboard.com>\n\t<20200625090733.j3eeq5bb3rhr632m@uno.localdomain>\n\t<20200625090739.GD5865@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200625090739.GD5865@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10853,"web_url":"https://patchwork.libcamera.org/comment/10853/","msgid":"<20200625095115.GF5865@pendragon.ideasonboard.com>","date":"2020-06-25T09:51:15","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Jun 25, 2020 at 11:26:17AM +0200, Jacopo Mondi wrote:\n> On Thu, Jun 25, 2020 at 12:07:39PM +0300, Laurent Pinchart wrote:\n> > On Thu, Jun 25, 2020 at 11:07:33AM +0200, Jacopo Mondi wrote:\n> >> On Thu, Jun 25, 2020 at 11:57:05AM +0300, Laurent Pinchart wrote:\n> >>> On Thu, Jun 25, 2020 at 09:37:39AM +0200, Jacopo Mondi wrote:\n> >>>> On Thu, Jun 25, 2020 at 04:23:30AM +0300, Laurent Pinchart wrote:\n> >>>>> It's common for code to check if a size is null. Add a helper function\n> >>>>> to do so.\n> >>>>>\n> >>>>> Signed-off-by: Laurent Pinchart \n> >>>>> ---\n> >>>>> Changes since v1:\n> >>>>>\n> >>>>> - Add test\n> >>>>> ---\n> >>>>> include/libcamera/geometry.h | 1 +\n> >>>>> src/libcamera/geometry.cpp | 6 ++++++\n> >>>>> test/geometry.cpp | 10 ++++++++++\n> >>>>> 3 files changed, 17 insertions(+)\n> >>>>>\n> >>>>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> >>>>> index edda42cf34cc..7d4b8bcfe3d8 100644\n> >>>>> --- a/include/libcamera/geometry.h\n> >>>>> +++ b/include/libcamera/geometry.h\n> >>>>> @@ -41,6 +41,7 @@ struct Size {\n> >>>>> \tunsigned int width;\n> >>>>> \tunsigned int height;\n> >>>>>\n> >>>>> +\tbool isNull() const { return !width && !height; }\n> >>>>\n> >>>> Is isNull() a good name ? We have isValid() for PixelFormat() should\n> >>>> we stay consistent ?\n> >>>\n> >>> I initially had isEmpty(), and I've taken inspiration from QRect the\n> >>> defines isNull() as !width && !height (or rather width <= 0 && height <=\n> >>> 0, but our width and height are unsigned), and isEmpty() as !width ||\n> >>> !height.\n> >>\n> >> I was more concerned about the fact we have a number of classes using\n> >> isValid() already, and consistency with the library is important,\n> >> otherwise it's a constant \"go to look at the class definition\", much\n> >> similar to when we had Rectangle::width and Size::w\n> >>\n> >> include/libcamera/file_descriptor.h: bool isValid() const { return fd_ != nullptr; }\n> >> include/libcamera/internal/formats.h: bool isValid() const { return format.isValid(); }\n> >> include/libcamera/internal/ipa_module.h: bool isValid() const;\n> >> include/libcamera/internal/ipa_proxy.h: bool isValid() const { return valid_; }\n> >> include/libcamera/internal/pub_key.h: bool isValid() const { return valid_; }\n> >> include/libcamera/internal/v4l2_pixelformat.h: bool isValid() const { return fourcc_ != 0; }\n> >> include/libcamera/pixel_format.h: constexpr bool isValid() const { return fourcc_ != 0; }\n> >>\n> >> For the record we have one isEmpty()\n> >> nclude/libcamera/internal/formats.h: bool isEmpty() const;\n> >>\n> >> but no isNull(). I would avoid introducing another term for a similar,\n> >> if not the same, concept.\n> >\n> > That boils down to the question of is a Size(0, 0) \"invalid\" ?\n> \n> Sorry about this, but how would you define a \"valid\" size ? To me\n> whatever size that has -both- width and height > 0 is valid.\n> Everything else is not. I don't really see a need for a distinction\n> between a \"valid\" (w || h) and a \"!null\" (or zero) one (w && h).\n> \n> A size with any of its members set to 0 is not valid. Is this too\n> harsh ?\n\nThat wasn't the question :-) My point is that the name \"valid\" doesn't\nseem to be a good candidate to me here, I wouldn't call a size that has\na width or height (or both) equal to 0 as \"invalid\". It's a\nzero/null/empty size, but not intrinsicly invalid. Whether such a size\ncan be valid for a given usage depends on the usage, but isn't a\nproperty of the Size class in my opinion.\n\n> >>>> Otherwise\n> >>>> Reviewed-by: Jacopo Mondi \n> >>>>\n> >>>>> \tconst std::string toString() const;\n> >>>>> };\n> >>>>>\n> >>>>> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> >>>>> index fd78cf2c0ab7..24c44fb43acf 100644\n> >>>>> --- a/src/libcamera/geometry.cpp\n> >>>>> +++ b/src/libcamera/geometry.cpp\n> >>>>> @@ -107,6 +107,12 @@ bool operator==(const Rectangle &lhs, const Rectangle &rhs)\n> >>>>> * \\brief The Size height\n> >>>>> */\n> >>>>>\n> >>>>> +/**\n> >>>>> + * \\fn bool Size::isNull() const\n> >>>>> + * \\brief Check if the size is null\n> >>>>> + * \\return True if both the width and height are 0, or false otherwise\n> >>>>> + */\n> >>>>> +\n> >>>>> /**\n> >>>>> * \\brief Assemble and return a string describing the size\n> >>>>> * \\return A string describing the size\n> >>>>> diff --git a/test/geometry.cpp b/test/geometry.cpp\n> >>>>> index 27e65565d7c6..904ad92c9448 100644\n> >>>>> --- a/test/geometry.cpp\n> >>>>> +++ b/test/geometry.cpp\n> >>>>> @@ -36,6 +36,16 @@ protected:\n> >>>>>\n> >>>>> \tint run()\n> >>>>> \t{\n> >>>>> +\t\tif (!Size().isNull() || !Size(0, 0).isNull()) {\n> >>>>> +\t\t\tcout << \"Null size incorrectly reported as not null\" << endl;\n> >>>>> +\t\t\treturn TestFail;\n> >>>>> +\t\t}\n> >>>>> +\n> >>>>> +\t\tif (Size(0, 100).isNull() || Size(100, 0).isNull() || Size(100, 100).isNull()) {\n> >>>>> +\t\t\tcout << \"Non-null size incorrectly reported as null\" << endl;\n> >>>>> +\t\t\treturn TestFail;\n> >>>>> +\t\t}\n> >>>>> +\n> >>>>> \t\t/* Test Size equality and inequality. */\n> >>>>> \t\tif (!compare(Size(100, 100), Size(100, 100), &operator==, \"==\", true))\n> >>>>> \t\t\treturn TestFail;","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 5E7E7C0100\n\tfor ;\n\tThu, 25 Jun 2020 09:51:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E875D609A5;\n\tThu, 25 Jun 2020 11:51:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 63A6A609A3\n\tfor ;\n\tThu, 25 Jun 2020 11:51:17 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C7D1F521;\n\tThu, 25 Jun 2020 11:51:16 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"j6IWL9qz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593078677;\n\tbh=nxjTmiBX+f+njnvNvoNEtfiSrfQ7uP9egA2/ZHpS8o4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=j6IWL9qzDpPhDw7ZFB949mCzJnZNMjlqBJCU6TLzrHVOnlj4DbK6aUjlEAZbap7Bn\n\tFWxJQSU4i3myhF2posKG+8WlsK9uBsXJuJno3UKhNW9YvzGXbssk4Q1oyzXJ/XJDEU\n\t7zkztD5lS2GOgzcAdFJ+ghk5KMF7ZphJmvPZ62zU=","Date":"Thu, 25 Jun 2020 12:51:15 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200625095115.GF5865@pendragon.ideasonboard.com>","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>\n\t<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>\n\t<20200625085705.GB5865@pendragon.ideasonboard.com>\n\t<20200625090733.j3eeq5bb3rhr632m@uno.localdomain>\n\t<20200625090739.GD5865@pendragon.ideasonboard.com>\n\t<20200625092617.ciw3rdifn63lmsht@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200625092617.ciw3rdifn63lmsht@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10855,"web_url":"https://patchwork.libcamera.org/comment/10855/","msgid":"<20200625101743.tui7e5tjslwgcdci@uno.localdomain>","date":"2020-06-25T10:17:43","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"On Thu, Jun 25, 2020 at 12:51:15PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Thu, Jun 25, 2020 at 11:26:17AM +0200, Jacopo Mondi wrote:\n> > On Thu, Jun 25, 2020 at 12:07:39PM +0300, Laurent Pinchart wrote:\n> > > On Thu, Jun 25, 2020 at 11:07:33AM +0200, Jacopo Mondi wrote:\n> > >> On Thu, Jun 25, 2020 at 11:57:05AM +0300, Laurent Pinchart wrote:\n> > >>> On Thu, Jun 25, 2020 at 09:37:39AM +0200, Jacopo Mondi wrote:\n> > >>>> On Thu, Jun 25, 2020 at 04:23:30AM +0300, Laurent Pinchart wrote:\n> > >>>>> It's common for code to check if a size is null. Add a helper function\n> > >>>>> to do so.\n> > >>>>>\n> > >>>>> Signed-off-by: Laurent Pinchart \n> > >>>>> ---\n> > >>>>> Changes since v1:\n> > >>>>>\n> > >>>>> - Add test\n> > >>>>> ---\n> > >>>>> include/libcamera/geometry.h | 1 +\n> > >>>>> src/libcamera/geometry.cpp | 6 ++++++\n> > >>>>> test/geometry.cpp | 10 ++++++++++\n> > >>>>> 3 files changed, 17 insertions(+)\n> > >>>>>\n> > >>>>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > >>>>> index edda42cf34cc..7d4b8bcfe3d8 100644\n> > >>>>> --- a/include/libcamera/geometry.h\n> > >>>>> +++ b/include/libcamera/geometry.h\n> > >>>>> @@ -41,6 +41,7 @@ struct Size {\n> > >>>>> \tunsigned int width;\n> > >>>>> \tunsigned int height;\n> > >>>>>\n> > >>>>> +\tbool isNull() const { return !width && !height; }\n> > >>>>\n> > >>>> Is isNull() a good name ? We have isValid() for PixelFormat() should\n> > >>>> we stay consistent ?\n> > >>>\n> > >>> I initially had isEmpty(), and I've taken inspiration from QRect the\n> > >>> defines isNull() as !width && !height (or rather width <= 0 && height <=\n> > >>> 0, but our width and height are unsigned), and isEmpty() as !width ||\n> > >>> !height.\n> > >>\n> > >> I was more concerned about the fact we have a number of classes using\n> > >> isValid() already, and consistency with the library is important,\n> > >> otherwise it's a constant \"go to look at the class definition\", much\n> > >> similar to when we had Rectangle::width and Size::w\n> > >>\n> > >> include/libcamera/file_descriptor.h: bool isValid() const { return fd_ != nullptr; }\n> > >> include/libcamera/internal/formats.h: bool isValid() const { return format.isValid(); }\n> > >> include/libcamera/internal/ipa_module.h: bool isValid() const;\n> > >> include/libcamera/internal/ipa_proxy.h: bool isValid() const { return valid_; }\n> > >> include/libcamera/internal/pub_key.h: bool isValid() const { return valid_; }\n> > >> include/libcamera/internal/v4l2_pixelformat.h: bool isValid() const { return fourcc_ != 0; }\n> > >> include/libcamera/pixel_format.h: constexpr bool isValid() const { return fourcc_ != 0; }\n> > >>\n> > >> For the record we have one isEmpty()\n> > >> nclude/libcamera/internal/formats.h: bool isEmpty() const;\n> > >>\n> > >> but no isNull(). I would avoid introducing another term for a similar,\n> > >> if not the same, concept.\n> > >\n> > > That boils down to the question of is a Size(0, 0) \"invalid\" ?\n> >\n> > Sorry about this, but how would you define a \"valid\" size ? To me\n> > whatever size that has -both- width and height > 0 is valid.\n> > Everything else is not. I don't really see a need for a distinction\n> > between a \"valid\" (w || h) and a \"!null\" (or zero) one (w && h).\n> >\n> > A size with any of its members set to 0 is not valid. Is this too\n> > harsh ?\n>\n> That wasn't the question :-) My point is that the name \"valid\" doesn't\n> seem to be a good candidate to me here, I wouldn't call a size that has\n> a width or height (or both) equal to 0 as \"invalid\". It's a\n> zero/null/empty size, but not intrinsicly invalid. Whether such a size\n> can be valid for a given usage depends on the usage, but isn't a\n> property of the Size class in my opinion.\n>\n\nAs you wish. This is going into the philosophical territory. To me a\nSize with any of its member set to 0 is empty, null, invalid or\nwhatever as I don't see a need to distinguish between an \"empty\" a\n\"null\" or a \"valid\" size. I just wanted to use the most widespread\nname we have to avoid having to look it up every time, which is just\nannoying. That said, up to you :)\n\nThanks\n j","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 E7F5FC0100\n\tfor ;\n\tThu, 25 Jun 2020 10:14:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 775A1609C2;\n\tThu, 25 Jun 2020 12:14:17 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F1D87609A5\n\tfor ;\n\tThu, 25 Jun 2020 12:14:15 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 761CD100005;\n\tThu, 25 Jun 2020 10:14:15 +0000 (UTC)"],"Date":"Thu, 25 Jun 2020 12:17:43 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200625101743.tui7e5tjslwgcdci@uno.localdomain>","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>\n\t<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>\n\t<20200625085705.GB5865@pendragon.ideasonboard.com>\n\t<20200625090733.j3eeq5bb3rhr632m@uno.localdomain>\n\t<20200625090739.GD5865@pendragon.ideasonboard.com>\n\t<20200625092617.ciw3rdifn63lmsht@uno.localdomain>\n\t<20200625095115.GF5865@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200625095115.GF5865@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10856,"web_url":"https://patchwork.libcamera.org/comment/10856/","msgid":"<55341149-45a4-6aa7-a02f-6ffe7a4293a1@ideasonboard.com>","date":"2020-06-25T10:50:18","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Ohhhh a bikeshedding thread :-D\n\n/me jumps in ...\n\nOn 25/06/2020 11:17, Jacopo Mondi wrote:\n> On Thu, Jun 25, 2020 at 12:51:15PM +0300, Laurent Pinchart wrote:\n>> Hi Jacopo,\n>>\n>> On Thu, Jun 25, 2020 at 11:26:17AM +0200, Jacopo Mondi wrote:\n>>> On Thu, Jun 25, 2020 at 12:07:39PM +0300, Laurent Pinchart wrote:\n>>>> On Thu, Jun 25, 2020 at 11:07:33AM +0200, Jacopo Mondi wrote:\n>>>>> On Thu, Jun 25, 2020 at 11:57:05AM +0300, Laurent Pinchart wrote:\n>>>>>> On Thu, Jun 25, 2020 at 09:37:39AM +0200, Jacopo Mondi wrote:\n>>>>>>> On Thu, Jun 25, 2020 at 04:23:30AM +0300, Laurent Pinchart wrote:\n>>>>>>>> It's common for code to check if a size is null. Add a helper function\n>>>>>>>> to do so.\n>>>>>>>>\n>>>>>>>> Signed-off-by: Laurent Pinchart \n>>>>>>>> ---\n>>>>>>>> Changes since v1:\n>>>>>>>>\n>>>>>>>> - Add test\n>>>>>>>> ---\n>>>>>>>> include/libcamera/geometry.h | 1 +\n>>>>>>>> src/libcamera/geometry.cpp | 6 ++++++\n>>>>>>>> test/geometry.cpp | 10 ++++++++++\n>>>>>>>> 3 files changed, 17 insertions(+)\n>>>>>>>>\n>>>>>>>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n>>>>>>>> index edda42cf34cc..7d4b8bcfe3d8 100644\n>>>>>>>> --- a/include/libcamera/geometry.h\n>>>>>>>> +++ b/include/libcamera/geometry.h\n>>>>>>>> @@ -41,6 +41,7 @@ struct Size {\n>>>>>>>> \tunsigned int width;\n>>>>>>>> \tunsigned int height;\n>>>>>>>>\n>>>>>>>> +\tbool isNull() const { return !width && !height; }\n>>>>>>>\n>>>>>>> Is isNull() a good name ? We have isValid() for PixelFormat() should\n>>>>>>> we stay consistent ?\n>>>>>>\n>>>>>> I initially had isEmpty(), and I've taken inspiration from QRect the\n>>>>>> defines isNull() as !width && !height (or rather width <= 0 && height <=\n>>>>>> 0, but our width and height are unsigned), and isEmpty() as !width ||\n>>>>>> !height.\n>>>>>\n>>>>> I was more concerned about the fact we have a number of classes using\n>>>>> isValid() already, and consistency with the library is important,\n>>>>> otherwise it's a constant \"go to look at the class definition\", much\n>>>>> similar to when we had Rectangle::width and Size::w\n>>>>>\n>>>>> include/libcamera/file_descriptor.h: bool isValid() const { return fd_ != nullptr; }\n>>>>> include/libcamera/internal/formats.h: bool isValid() const { return format.isValid(); }\n>>>>> include/libcamera/internal/ipa_module.h: bool isValid() const;\n>>>>> include/libcamera/internal/ipa_proxy.h: bool isValid() const { return valid_; }\n>>>>> include/libcamera/internal/pub_key.h: bool isValid() const { return valid_; }\n>>>>> include/libcamera/internal/v4l2_pixelformat.h: bool isValid() const { return fourcc_ != 0; }\n>>>>> include/libcamera/pixel_format.h: constexpr bool isValid() const { return fourcc_ != 0; }\n>>>>>\n>>>>> For the record we have one isEmpty()\n>>>>> nclude/libcamera/internal/formats.h: bool isEmpty() const;\n>>>>>\n>>>>> but no isNull(). I would avoid introducing another term for a similar,\n>>>>> if not the same, concept.\n>>>>\n>>>> That boils down to the question of is a Size(0, 0) \"invalid\" ?\n>>>\n>>> Sorry about this, but how would you define a \"valid\" size ? To me\n>>> whatever size that has -both- width and height > 0 is valid.\n>>> Everything else is not. I don't really see a need for a distinction\n>>> between a \"valid\" (w || h) and a \"!null\" (or zero) one (w && h).\n>>>\n>>> A size with any of its members set to 0 is not valid. Is this too\n>>> harsh ?\n>>\n>> That wasn't the question :-) My point is that the name \"valid\" doesn't\n>> seem to be a good candidate to me here, I wouldn't call a size that has\n>> a width or height (or both) equal to 0 as \"invalid\". It's a\n>> zero/null/empty size, but not intrinsicly invalid. Whether such a size\n>> can be valid for a given usage depends on the usage, but isn't a\n>> property of the Size class in my opinion.\n>>\n> \n> As you wish. This is going into the philosophical territory. To me a\n> Size with any of its member set to 0 is empty, null, invalid or\n> whatever as I don't see a need to distinguish between an \"empty\" a\n> \"null\" or a \"valid\" size. I just wanted to use the most widespread\n> name we have to avoid having to look it up every time, which is just\n> annoying. That said, up to you :)\n\n\nPersonally I would say that a size of 0 is 'valid'. Maybe it depends on\ncontext, but I can attest that the size of my cherry-bakewell is now 0\nin both width, height (and depth):\n\n if (cherry-bakewell.size == Size(0, 0)) {\n\tstd::cout << \"Was it yummy?\" << std::endl;\n } else {\n\tstd::cout << \"C'mon - eat up...\" << std::endl;\n }\n\nisNull, isZero, would both be fine with me (I'd actually use isZero) But\nfor some reason - not isEmpty().\n\nI don't think you'd have an empty size, as the 'size' is not a container.\n\n--\nKieran\n\n\n\n> \n> Thanks\n> j\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\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 14629C0109\n\tfor ;\n\tThu, 25 Jun 2020 10:50:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 785B5609C2;\n\tThu, 25 Jun 2020 12:50:24 +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 65959609A5\n\tfor ;\n\tThu, 25 Jun 2020 12:50:22 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 94990521;\n\tThu, 25 Jun 2020 12:50:21 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"jUIXsS8s\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593082221;\n\tbh=PYOKr9bjXqt6iwH0JcelUBFl4oSEleWUaJWoVWgjjfU=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=jUIXsS8sql00a2lM2u2eozdkL37FkHHn4rzzzrncgTyL+QvbnI5fqPNbnCOZ0EabI\n\t6ouQaM6tLrAHBcpcySqDL+y0/z7PuEY/KbHy9W8RKuN1GbUSpCSdvMNMzgj9qGPTZW\n\tKD/HGZ4PQcK0z8jzCEQw/TDjuDD6uGH94SGcJTK4=","To":"Jacopo Mondi ,\n\tLaurent Pinchart ","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>\n\t<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>\n\t<20200625085705.GB5865@pendragon.ideasonboard.com>\n\t<20200625090733.j3eeq5bb3rhr632m@uno.localdomain>\n\t<20200625090739.GD5865@pendragon.ideasonboard.com>\n\t<20200625092617.ciw3rdifn63lmsht@uno.localdomain>\n\t<20200625095115.GF5865@pendragon.ideasonboard.com>\n\t<20200625101743.tui7e5tjslwgcdci@uno.localdomain>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<55341149-45a4-6aa7-a02f-6ffe7a4293a1@ideasonboard.com>","Date":"Thu, 25 Jun 2020 11:50:18 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200625101743.tui7e5tjslwgcdci@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10857,"web_url":"https://patchwork.libcamera.org/comment/10857/","msgid":"<20200625105114.GA275666@oden.dyn.berto.se>","date":"2020-06-25T10:51:14","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2020-06-25 04:23:30 +0300, Laurent Pinchart wrote:\n> It's common for code to check if a size is null. Add a helper function\n> to do so.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> Changes since v1:\n> \n> - Add test\n> ---\n> include/libcamera/geometry.h | 1 +\n> src/libcamera/geometry.cpp | 6 ++++++\n> test/geometry.cpp | 10 ++++++++++\n> 3 files changed, 17 insertions(+)\n> \n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index edda42cf34cc..7d4b8bcfe3d8 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -41,6 +41,7 @@ struct Size {\n> \tunsigned int width;\n> \tunsigned int height;\n> \n> +\tbool isNull() const { return !width && !height; }\n> \tconst std::string toString() const;\n> };\n> \n> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> index fd78cf2c0ab7..24c44fb43acf 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -107,6 +107,12 @@ bool operator==(const Rectangle &lhs, const Rectangle &rhs)\n> * \\brief The Size height\n> */\n> \n> +/**\n> + * \\fn bool Size::isNull() const\n> + * \\brief Check if the size is null\n> + * \\return True if both the width and height are 0, or false otherwise\n> + */\n> +\n> /**\n> * \\brief Assemble and return a string describing the size\n> * \\return A string describing the size\n> diff --git a/test/geometry.cpp b/test/geometry.cpp\n> index 27e65565d7c6..904ad92c9448 100644\n> --- a/test/geometry.cpp\n> +++ b/test/geometry.cpp\n> @@ -36,6 +36,16 @@ protected:\n> \n> \tint run()\n> \t{\n> +\t\tif (!Size().isNull() || !Size(0, 0).isNull()) {\n> +\t\t\tcout << \"Null size incorrectly reported as not null\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (Size(0, 100).isNull() || Size(100, 0).isNull() || Size(100, 100).isNull()) {\n> +\t\t\tcout << \"Non-null size incorrectly reported as null\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> \t\t/* Test Size equality and inequality. */\n> \t\tif (!compare(Size(100, 100), Size(100, 100), &operator==, \"==\", true))\n> \t\t\treturn TestFail;\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 62D79C0100\n\tfor ;\n\tThu, 25 Jun 2020 10:51:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 305B8609C2;\n\tThu, 25 Jun 2020 12:51:18 +0200 (CEST)","from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 57FFF609A5\n\tfor ;\n\tThu, 25 Jun 2020 12:51:16 +0200 (CEST)","by mail-lj1-x243.google.com with SMTP id h19so5927828ljg.13\n\tfor ;\n\tThu, 25 Jun 2020 03:51:16 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tk21sm2055843ljk.121.2020.06.25.03.51.14\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 25 Jun 2020 03:51:14 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"pseEf61e\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=VL0p/I2szlGw0OoOVi4TaMM26l96JF7m7Ty5ywRaYtM=;\n\tb=pseEf61ewRDitPi+b9KJlIKe0sonr/9R+l/PimX7Xi/Fp1RcgNHx2NrIpMnq+vx2gd\n\tGmCPqRaY0vriBKFpqMzanATuHqUUFoqjBzb8Sa7m3MbJp9btFakS26kFAb7xAvIacs0Q\n\tuvrV9H9xPfo+Fw9ye3GeaxfDPbamJI4Moqk6YMLNofVEKpmX4LnrhRbciDstMifGE7wf\n\tpFLpSgtkZMnZ0k/D3bD2zuQll2qycyhfKv8SitDxG2U3W/a+Qw50H9GSuAjM1TNEYyil\n\tjPeKqVHLrZKzIDqUB7WNp1HgnOFP0q86XrJeFRHTOtUh7tex/8T4GijsYbzxaBdoTu/J\n\tUVxw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=VL0p/I2szlGw0OoOVi4TaMM26l96JF7m7Ty5ywRaYtM=;\n\tb=sJGQvV2Wj3vvC6IFoNQkuBb4X3v+C88MrRhFTKUzbVpSLGV+wqo7tVMekiGDsFR4i0\n\tyjpWzIsDZzN2ZJ9HYKIk8VhjZWDrCqDEdPNKVwDSlT5sUVuzmdkymlSnWkU31R4W38pK\n\t907Oj5QKP3KxMIcTL46bLUw17mqKCsr40fntLj9pdT11zM99Aujg7HicmjagPnx3Jp+c\n\tU8ve09gIgH6o5nH9UjY+Cir76K9FfcBOPjXX225hxNNErYEgmb7l+EGXLUviDygYLk5z\n\t0zQm7TAXN2CnsmBxJYnTUkvDglFjtGFbtPaeYDfB+QvEHseK0WZeQUaRjNd0b30MygFB\n\tuiPQ==","X-Gm-Message-State":"AOAM53104f5aXSGxCTbREhvMrjfUboZzgDbO9Fyu8VrRBxLuQZw5Azif\n\tlN97D7VNBGJFdZTRQWai6vZ+eQ==","X-Google-Smtp-Source":"ABdhPJy9yXfcgvrncikX2yEASJLDP6yTMMGs3xYO6mNi+hFtDGR23qLRJTmIoP+2IEhC1iu6axkwdQ==","X-Received":"by 2002:a2e:751a:: with SMTP id\n\tq26mr15202636ljc.258.1593082275455; \n\tThu, 25 Jun 2020 03:51:15 -0700 (PDT)","Date":"Thu, 25 Jun 2020 12:51:14 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"<20200625105114.GA275666@oden.dyn.berto.se>","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10868,"web_url":"https://patchwork.libcamera.org/comment/10868/","msgid":"<20200626010224.GO5865@pendragon.ideasonboard.com>","date":"2020-06-26T01:02:24","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Jun 25, 2020 at 11:50:18AM +0100, Kieran Bingham wrote:\n> Ohhhh a bikeshedding thread :-D\n> \n> /me jumps in ...\n\n:-)\n\n> On 25/06/2020 11:17, Jacopo Mondi wrote:\n> > On Thu, Jun 25, 2020 at 12:51:15PM +0300, Laurent Pinchart wrote:\n> >> On Thu, Jun 25, 2020 at 11:26:17AM +0200, Jacopo Mondi wrote:\n> >>> On Thu, Jun 25, 2020 at 12:07:39PM +0300, Laurent Pinchart wrote:\n> >>>> On Thu, Jun 25, 2020 at 11:07:33AM +0200, Jacopo Mondi wrote:\n> >>>>> On Thu, Jun 25, 2020 at 11:57:05AM +0300, Laurent Pinchart wrote:\n> >>>>>> On Thu, Jun 25, 2020 at 09:37:39AM +0200, Jacopo Mondi wrote:\n> >>>>>>> On Thu, Jun 25, 2020 at 04:23:30AM +0300, Laurent Pinchart wrote:\n> >>>>>>>> It's common for code to check if a size is null. Add a helper function\n> >>>>>>>> to do so.\n> >>>>>>>>\n> >>>>>>>> Signed-off-by: Laurent Pinchart \n> >>>>>>>> ---\n> >>>>>>>> Changes since v1:\n> >>>>>>>>\n> >>>>>>>> - Add test\n> >>>>>>>> ---\n> >>>>>>>> include/libcamera/geometry.h | 1 +\n> >>>>>>>> src/libcamera/geometry.cpp | 6 ++++++\n> >>>>>>>> test/geometry.cpp | 10 ++++++++++\n> >>>>>>>> 3 files changed, 17 insertions(+)\n> >>>>>>>>\n> >>>>>>>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> >>>>>>>> index edda42cf34cc..7d4b8bcfe3d8 100644\n> >>>>>>>> --- a/include/libcamera/geometry.h\n> >>>>>>>> +++ b/include/libcamera/geometry.h\n> >>>>>>>> @@ -41,6 +41,7 @@ struct Size {\n> >>>>>>>> \tunsigned int width;\n> >>>>>>>> \tunsigned int height;\n> >>>>>>>>\n> >>>>>>>> +\tbool isNull() const { return !width && !height; }\n> >>>>>>>\n> >>>>>>> Is isNull() a good name ? We have isValid() for PixelFormat() should\n> >>>>>>> we stay consistent ?\n> >>>>>>\n> >>>>>> I initially had isEmpty(), and I've taken inspiration from QRect the\n> >>>>>> defines isNull() as !width && !height (or rather width <= 0 && height <=\n> >>>>>> 0, but our width and height are unsigned), and isEmpty() as !width ||\n> >>>>>> !height.\n> >>>>>\n> >>>>> I was more concerned about the fact we have a number of classes using\n> >>>>> isValid() already, and consistency with the library is important,\n> >>>>> otherwise it's a constant \"go to look at the class definition\", much\n> >>>>> similar to when we had Rectangle::width and Size::w\n> >>>>>\n> >>>>> include/libcamera/file_descriptor.h: bool isValid() const { return fd_ != nullptr; }\n> >>>>> include/libcamera/internal/formats.h: bool isValid() const { return format.isValid(); }\n> >>>>> include/libcamera/internal/ipa_module.h: bool isValid() const;\n> >>>>> include/libcamera/internal/ipa_proxy.h: bool isValid() const { return valid_; }\n> >>>>> include/libcamera/internal/pub_key.h: bool isValid() const { return valid_; }\n> >>>>> include/libcamera/internal/v4l2_pixelformat.h: bool isValid() const { return fourcc_ != 0; }\n> >>>>> include/libcamera/pixel_format.h: constexpr bool isValid() const { return fourcc_ != 0; }\n> >>>>>\n> >>>>> For the record we have one isEmpty()\n> >>>>> nclude/libcamera/internal/formats.h: bool isEmpty() const;\n> >>>>>\n> >>>>> but no isNull(). I would avoid introducing another term for a similar,\n> >>>>> if not the same, concept.\n> >>>>\n> >>>> That boils down to the question of is a Size(0, 0) \"invalid\" ?\n> >>>\n> >>> Sorry about this, but how would you define a \"valid\" size ? To me\n> >>> whatever size that has -both- width and height > 0 is valid.\n> >>> Everything else is not. I don't really see a need for a distinction\n> >>> between a \"valid\" (w || h) and a \"!null\" (or zero) one (w && h).\n> >>>\n> >>> A size with any of its members set to 0 is not valid. Is this too\n> >>> harsh ?\n> >>\n> >> That wasn't the question :-) My point is that the name \"valid\" doesn't\n> >> seem to be a good candidate to me here, I wouldn't call a size that has\n> >> a width or height (or both) equal to 0 as \"invalid\". It's a\n> >> zero/null/empty size, but not intrinsicly invalid. Whether such a size\n> >> can be valid for a given usage depends on the usage, but isn't a\n> >> property of the Size class in my opinion.\n> > \n> > As you wish. This is going into the philosophical territory. To me a\n> > Size with any of its member set to 0 is empty, null, invalid or\n> > whatever as I don't see a need to distinguish between an \"empty\" a\n> > \"null\" or a \"valid\" size.\n\nI'm not sure it's philosophical, but it's clearly a semantical issue. I\ndon't like using \"invalid\" (or \"valid\") in this context, as having 0\nwidth or height doesn't make the size intrinsically invalid. A\nPixelFormat with a 0 fourcc, on the other hand, is invalid, because 0\nis not a valid fourcc.\n\n> > I just wanted to use the most widespread\n> > name we have to avoid having to look it up every time, which is just\n> > annoying. That said, up to you :)\n\nThat's an interesting point. Based on my (limited) experience with Qt,\nMFC and .net frameworks (in another life), I believe that applying the\nsame term throughout the API to incrzase consistency isn't a rule that\nshould be blindly followed. Of course consistency is overall encouraged,\nbut I've found it was much more difficult to work with the MFC or .net\nGUI frameworks than with Qt because the first would pick a set of\nconcepts and apply them to everything, even when they were not the best\nfit, while the second would try to pick the right concept tailored to\neach problem at hand. There is a wider range of names in Qt, but when I\nneeded a function in a class, my first guess for its name was usually\nright. It was a long time ago, so I don't have any specific example in\nmind, but if I had to make one, let's say you have a widget and you need\nto set its internal margins. Qt would have a setMargins() function,\nwhile other frameworks would use something like setLayoutProperties()\nthat would be present in all GUI classes, and take a list of properties\nas a map from property ID to property value. The function would be\nconsistently used through the API, but you would have to look up every\ntime which properties a particular widget would accept.\n\nThis is a completely made up example, but working with Qt really\nimpressed me in how the API felt natural to use. Since that day I've\nstrived to achieve the same level of quality in APIs (and have surely\nlargely failed :-)). This is why, in this case, I don't think that\nisValid() would be a good choice.\n\n> Personally I would say that a size of 0 is 'valid'. Maybe it depends on\n> context, but I can attest that the size of my cherry-bakewell is now 0\n> in both width, height (and depth):\n> \n> if (cherry-bakewell.size == Size(0, 0)) {\n> \tstd::cout << \"Was it yummy?\" << std::endl;\n> } else {\n> \tstd::cout << \"C'mon - eat up...\" << std::endl;\n> }\n> \n> isNull, isZero, would both be fine with me (I'd actually use isZero) But\n> for some reason - not isEmpty().\n> \n> I don't think you'd have an empty size, as the 'size' is not a container.\n\nisEmpty() is a function that would indeed be more applicable to the\nRectangle class for instance.","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 E9269C0109\n\tfor ;\n\tFri, 26 Jun 2020 01:02:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 78A31609C5;\n\tFri, 26 Jun 2020 03:02:27 +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 6937A603BA\n\tfor ;\n\tFri, 26 Jun 2020 03:02:26 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CBC1072E;\n\tFri, 26 Jun 2020 03:02:25 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"CZNWx21z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593133346;\n\tbh=cqohNk6WUaCex1ey4Ictc16zxDiBK6vxO94r2ACfc1o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CZNWx21zboj/ZCSqfld9ibjnWKPKEygiCQXxbt7lIR2907We8E8DRyOye64apsirB\n\te77dtpqbO2/ZGGNCcaTKKFJyOfy+crrNKqQVW3vZxOyX2B47Ark9JGuXnPoXYfsrLG\n\tkhqTAiDCH4GzvfGrlPhJfqHlI3dScjjrKNLgx91o=","Date":"Fri, 26 Jun 2020 04:02:24 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20200626010224.GO5865@pendragon.ideasonboard.com>","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>\n\t<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>\n\t<20200625085705.GB5865@pendragon.ideasonboard.com>\n\t<20200625090733.j3eeq5bb3rhr632m@uno.localdomain>\n\t<20200625090739.GD5865@pendragon.ideasonboard.com>\n\t<20200625092617.ciw3rdifn63lmsht@uno.localdomain>\n\t<20200625095115.GF5865@pendragon.ideasonboard.com>\n\t<20200625101743.tui7e5tjslwgcdci@uno.localdomain>\n\t<55341149-45a4-6aa7-a02f-6ffe7a4293a1@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<55341149-45a4-6aa7-a02f-6ffe7a4293a1@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10869,"web_url":"https://patchwork.libcamera.org/comment/10869/","msgid":"<20200626012505.GB477866@oden.dyn.berto.se>","date":"2020-06-26T01:25:05","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"On 2020-06-25 11:50:18 +0100, Kieran Bingham wrote:\n> Ohhhh a bikeshedding thread :-D\n\nI don't like bikeshedding, but I would like for this functionality to be \nmerged, so here is my suggestions :-)\n\nI like isNull(). As Laurent points out isValid() implies we can have an \ninvalid size. isEmpty() makes me think of containers and makes little \nsens for me in this context.\n\n> \n> /me jumps in ...\n> \n> On 25/06/2020 11:17, Jacopo Mondi wrote:\n> > On Thu, Jun 25, 2020 at 12:51:15PM +0300, Laurent Pinchart wrote:\n> >> Hi Jacopo,\n> >>\n> >> On Thu, Jun 25, 2020 at 11:26:17AM +0200, Jacopo Mondi wrote:\n> >>> On Thu, Jun 25, 2020 at 12:07:39PM +0300, Laurent Pinchart wrote:\n> >>>> On Thu, Jun 25, 2020 at 11:07:33AM +0200, Jacopo Mondi wrote:\n> >>>>> On Thu, Jun 25, 2020 at 11:57:05AM +0300, Laurent Pinchart wrote:\n> >>>>>> On Thu, Jun 25, 2020 at 09:37:39AM +0200, Jacopo Mondi wrote:\n> >>>>>>> On Thu, Jun 25, 2020 at 04:23:30AM +0300, Laurent Pinchart wrote:\n> >>>>>>>> It's common for code to check if a size is null. Add a helper function\n> >>>>>>>> to do so.\n> >>>>>>>>\n> >>>>>>>> Signed-off-by: Laurent Pinchart \n> >>>>>>>> ---\n> >>>>>>>> Changes since v1:\n> >>>>>>>>\n> >>>>>>>> - Add test\n> >>>>>>>> ---\n> >>>>>>>> include/libcamera/geometry.h | 1 +\n> >>>>>>>> src/libcamera/geometry.cpp | 6 ++++++\n> >>>>>>>> test/geometry.cpp | 10 ++++++++++\n> >>>>>>>> 3 files changed, 17 insertions(+)\n> >>>>>>>>\n> >>>>>>>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> >>>>>>>> index edda42cf34cc..7d4b8bcfe3d8 100644\n> >>>>>>>> --- a/include/libcamera/geometry.h\n> >>>>>>>> +++ b/include/libcamera/geometry.h\n> >>>>>>>> @@ -41,6 +41,7 @@ struct Size {\n> >>>>>>>> \tunsigned int width;\n> >>>>>>>> \tunsigned int height;\n> >>>>>>>>\n> >>>>>>>> +\tbool isNull() const { return !width && !height; }\n> >>>>>>>\n> >>>>>>> Is isNull() a good name ? We have isValid() for PixelFormat() should\n> >>>>>>> we stay consistent ?\n> >>>>>>\n> >>>>>> I initially had isEmpty(), and I've taken inspiration from QRect the\n> >>>>>> defines isNull() as !width && !height (or rather width <= 0 && height <=\n> >>>>>> 0, but our width and height are unsigned), and isEmpty() as !width ||\n> >>>>>> !height.\n> >>>>>\n> >>>>> I was more concerned about the fact we have a number of classes using\n> >>>>> isValid() already, and consistency with the library is important,\n> >>>>> otherwise it's a constant \"go to look at the class definition\", much\n> >>>>> similar to when we had Rectangle::width and Size::w\n> >>>>>\n> >>>>> include/libcamera/file_descriptor.h: bool isValid() const { return fd_ != nullptr; }\n> >>>>> include/libcamera/internal/formats.h: bool isValid() const { return format.isValid(); }\n> >>>>> include/libcamera/internal/ipa_module.h: bool isValid() const;\n> >>>>> include/libcamera/internal/ipa_proxy.h: bool isValid() const { return valid_; }\n> >>>>> include/libcamera/internal/pub_key.h: bool isValid() const { return valid_; }\n> >>>>> include/libcamera/internal/v4l2_pixelformat.h: bool isValid() const { return fourcc_ != 0; }\n> >>>>> include/libcamera/pixel_format.h: constexpr bool isValid() const { return fourcc_ != 0; }\n> >>>>>\n> >>>>> For the record we have one isEmpty()\n> >>>>> nclude/libcamera/internal/formats.h: bool isEmpty() const;\n> >>>>>\n> >>>>> but no isNull(). I would avoid introducing another term for a similar,\n> >>>>> if not the same, concept.\n> >>>>\n> >>>> That boils down to the question of is a Size(0, 0) \"invalid\" ?\n> >>>\n> >>> Sorry about this, but how would you define a \"valid\" size ? To me\n> >>> whatever size that has -both- width and height > 0 is valid.\n> >>> Everything else is not. I don't really see a need for a distinction\n> >>> between a \"valid\" (w || h) and a \"!null\" (or zero) one (w && h).\n> >>>\n> >>> A size with any of its members set to 0 is not valid. Is this too\n> >>> harsh ?\n> >>\n> >> That wasn't the question :-) My point is that the name \"valid\" doesn't\n> >> seem to be a good candidate to me here, I wouldn't call a size that has\n> >> a width or height (or both) equal to 0 as \"invalid\". It's a\n> >> zero/null/empty size, but not intrinsicly invalid. Whether such a size\n> >> can be valid for a given usage depends on the usage, but isn't a\n> >> property of the Size class in my opinion.\n> >>\n> > \n> > As you wish. This is going into the philosophical territory. To me a\n> > Size with any of its member set to 0 is empty, null, invalid or\n> > whatever as I don't see a need to distinguish between an \"empty\" a\n> > \"null\" or a \"valid\" size. I just wanted to use the most widespread\n> > name we have to avoid having to look it up every time, which is just\n> > annoying. That said, up to you :)\n> \n> \n> Personally I would say that a size of 0 is 'valid'. Maybe it depends on\n> context, but I can attest that the size of my cherry-bakewell is now 0\n> in both width, height (and depth):\n> \n> if (cherry-bakewell.size == Size(0, 0)) {\n> \tstd::cout << \"Was it yummy?\" << std::endl;\n> } else {\n> \tstd::cout << \"C'mon - eat up...\" << std::endl;\n> }\n> \n> isNull, isZero, would both be fine with me (I'd actually use isZero) But\n> for some reason - not isEmpty().\n> \n> I don't think you'd have an empty size, as the 'size' is not a container.\n> \n> --\n> Kieran\n> \n> \n> \n> > \n> > Thanks\n> > j\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n> > \n> \n> -- \n> Regards\n> --\n> Kieran\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 1ED9BC2E65\n\tfor ;\n\tFri, 26 Jun 2020 01:25:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8CE11609C7;\n\tFri, 26 Jun 2020 03:25:09 +0200 (CEST)","from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B1404603BA\n\tfor ;\n\tFri, 26 Jun 2020 03:25:07 +0200 (CEST)","by mail-lf1-x142.google.com with SMTP id g2so4279336lfb.0\n\tfor ;\n\tThu, 25 Jun 2020 18:25:07 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tg142sm6358829lfd.41.2020.06.25.18.25.05\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 25 Jun 2020 18:25:05 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"t69ElPjV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=h0YlUZu9hOgQuST+ibl7z8xr+fPz2xd7p9e+IYE6CnU=;\n\tb=t69ElPjVKvnaS9uprc3uIUlcY9kLjXkWjXDtGtwRF4XlFXEScRHt2ojnBVxjpIOIv5\n\t/0HQTJPXbuplypJKdza3Rt3s71gUdfCwWwPqh9xpS4rDmGg+CAc+mLWUI94U/LjPnCqx\n\tLvqD2XDyoq4mTbfuo7d8SbyGersLdTyR/o1oMua0YsNXKLGZiTGDEmg/r1MLAahEMrwx\n\tIWFxiQw8QoNG+psWe9IhpM5YFdgaCfIL4Cja97EUS8/TcM4/JsTY1M5aQdI9uA19jtAC\n\to8nuj1QD/UEvf4kIZxZu6v+79XWWTceZ5dnZ9/hQp5mRX6vk0IiSkLzvPvwdZXgVM8RU\n\tKOyw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=h0YlUZu9hOgQuST+ibl7z8xr+fPz2xd7p9e+IYE6CnU=;\n\tb=Ad7Lg4EDa7/FXlZAalfqrXdRqqUtulIRl+uHIVRWJ52CHYpjU8XX7ZnI9XplQIJI7B\n\tYJybPaIJL2STp+A80KdcU0edW2qkHC0uLendruqHJCi0gMt5+ZsWya4fEQsWCVG57rNi\n\tkI3P58v9h6PARwaYXg43op5Uvc+1RFkUffBeJKf7TRCVjfvzN65InUPMvkQmikmwzYfY\n\t0XKPthwn9FfAjmr6EFoy8B1I0PpuDd+mih56DKU+T8dme33EToexp/z1H3ATSsDrcrGz\n\t7bsFXUHMtMzTYp3K3mslsZ/dblUywn7wEwCcjmkPNmNhjPg6ZMjY/mEnR8tcrD/g/Z4P\n\tmq5A==","X-Gm-Message-State":"AOAM5333eJofK4QYR/6KQ1vHV9qvPoxSRCnHx3ci8YQcU+aJmtVpUDFm\n\trc/BSlx5Vt21Ouwge8zaGUKBSA==","X-Google-Smtp-Source":"ABdhPJy5V4uKzp5oeSUfBwrO6BlanT+/8wvQr4QjKPLsbCH//hwJXAdey7o3wF+1NIq13LBsiCvvWA==","X-Received":"by 2002:a19:ecc:: with SMTP id 195mr421678lfo.71.1593134706699; \n\tThu, 25 Jun 2020 18:25:06 -0700 (PDT)","Date":"Fri, 26 Jun 2020 03:25:05 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Kieran Bingham ","Message-ID":"<20200626012505.GB477866@oden.dyn.berto.se>","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>\n\t<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>\n\t<20200625085705.GB5865@pendragon.ideasonboard.com>\n\t<20200625090733.j3eeq5bb3rhr632m@uno.localdomain>\n\t<20200625090739.GD5865@pendragon.ideasonboard.com>\n\t<20200625092617.ciw3rdifn63lmsht@uno.localdomain>\n\t<20200625095115.GF5865@pendragon.ideasonboard.com>\n\t<20200625101743.tui7e5tjslwgcdci@uno.localdomain>\n\t<55341149-45a4-6aa7-a02f-6ffe7a4293a1@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<55341149-45a4-6aa7-a02f-6ffe7a4293a1@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10876,"web_url":"https://patchwork.libcamera.org/comment/10876/","msgid":"<20200626015047.GX5865@pendragon.ideasonboard.com>","date":"2020-06-26T01:50:47","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Jun 26, 2020 at 03:25:05AM +0200, Niklas Söderlund wrote:\n> On 2020-06-25 11:50:18 +0100, Kieran Bingham wrote:\n> > Ohhhh a bikeshedding thread :-D\n> \n> I don't like bikeshedding, but I would like for this functionality to be \n> merged, so here is my suggestions :-)\n> \n> I like isNull(). As Laurent points out isValid() implies we can have an \n> invalid size. isEmpty() makes me think of containers and makes little \n> sens for me in this context.\n\nI'd rather go for isNull() as well (I'm possibly influenced by Qt here).\nIf we later need a function to test width == 0 || height == 0 in\naddition to width == 0 && height == 0 we'll bikeshed the name for that\nnew function again :-)\n\n> > /me jumps in ...\n> > \n> > On 25/06/2020 11:17, Jacopo Mondi wrote:\n> > > On Thu, Jun 25, 2020 at 12:51:15PM +0300, Laurent Pinchart wrote:\n> > >> Hi Jacopo,\n> > >>\n> > >> On Thu, Jun 25, 2020 at 11:26:17AM +0200, Jacopo Mondi wrote:\n> > >>> On Thu, Jun 25, 2020 at 12:07:39PM +0300, Laurent Pinchart wrote:\n> > >>>> On Thu, Jun 25, 2020 at 11:07:33AM +0200, Jacopo Mondi wrote:\n> > >>>>> On Thu, Jun 25, 2020 at 11:57:05AM +0300, Laurent Pinchart wrote:\n> > >>>>>> On Thu, Jun 25, 2020 at 09:37:39AM +0200, Jacopo Mondi wrote:\n> > >>>>>>> On Thu, Jun 25, 2020 at 04:23:30AM +0300, Laurent Pinchart wrote:\n> > >>>>>>>> It's common for code to check if a size is null. Add a helper function\n> > >>>>>>>> to do so.\n> > >>>>>>>>\n> > >>>>>>>> Signed-off-by: Laurent Pinchart \n> > >>>>>>>> ---\n> > >>>>>>>> Changes since v1:\n> > >>>>>>>>\n> > >>>>>>>> - Add test\n> > >>>>>>>> ---\n> > >>>>>>>> include/libcamera/geometry.h | 1 +\n> > >>>>>>>> src/libcamera/geometry.cpp | 6 ++++++\n> > >>>>>>>> test/geometry.cpp | 10 ++++++++++\n> > >>>>>>>> 3 files changed, 17 insertions(+)\n> > >>>>>>>>\n> > >>>>>>>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > >>>>>>>> index edda42cf34cc..7d4b8bcfe3d8 100644\n> > >>>>>>>> --- a/include/libcamera/geometry.h\n> > >>>>>>>> +++ b/include/libcamera/geometry.h\n> > >>>>>>>> @@ -41,6 +41,7 @@ struct Size {\n> > >>>>>>>> \tunsigned int width;\n> > >>>>>>>> \tunsigned int height;\n> > >>>>>>>>\n> > >>>>>>>> +\tbool isNull() const { return !width && !height; }\n> > >>>>>>>\n> > >>>>>>> Is isNull() a good name ? We have isValid() for PixelFormat() should\n> > >>>>>>> we stay consistent ?\n> > >>>>>>\n> > >>>>>> I initially had isEmpty(), and I've taken inspiration from QRect the\n> > >>>>>> defines isNull() as !width && !height (or rather width <= 0 && height <=\n> > >>>>>> 0, but our width and height are unsigned), and isEmpty() as !width ||\n> > >>>>>> !height.\n> > >>>>>\n> > >>>>> I was more concerned about the fact we have a number of classes using\n> > >>>>> isValid() already, and consistency with the library is important,\n> > >>>>> otherwise it's a constant \"go to look at the class definition\", much\n> > >>>>> similar to when we had Rectangle::width and Size::w\n> > >>>>>\n> > >>>>> include/libcamera/file_descriptor.h: bool isValid() const { return fd_ != nullptr; }\n> > >>>>> include/libcamera/internal/formats.h: bool isValid() const { return format.isValid(); }\n> > >>>>> include/libcamera/internal/ipa_module.h: bool isValid() const;\n> > >>>>> include/libcamera/internal/ipa_proxy.h: bool isValid() const { return valid_; }\n> > >>>>> include/libcamera/internal/pub_key.h: bool isValid() const { return valid_; }\n> > >>>>> include/libcamera/internal/v4l2_pixelformat.h: bool isValid() const { return fourcc_ != 0; }\n> > >>>>> include/libcamera/pixel_format.h: constexpr bool isValid() const { return fourcc_ != 0; }\n> > >>>>>\n> > >>>>> For the record we have one isEmpty()\n> > >>>>> nclude/libcamera/internal/formats.h: bool isEmpty() const;\n> > >>>>>\n> > >>>>> but no isNull(). I would avoid introducing another term for a similar,\n> > >>>>> if not the same, concept.\n> > >>>>\n> > >>>> That boils down to the question of is a Size(0, 0) \"invalid\" ?\n> > >>>\n> > >>> Sorry about this, but how would you define a \"valid\" size ? To me\n> > >>> whatever size that has -both- width and height > 0 is valid.\n> > >>> Everything else is not. I don't really see a need for a distinction\n> > >>> between a \"valid\" (w || h) and a \"!null\" (or zero) one (w && h).\n> > >>>\n> > >>> A size with any of its members set to 0 is not valid. Is this too\n> > >>> harsh ?\n> > >>\n> > >> That wasn't the question :-) My point is that the name \"valid\" doesn't\n> > >> seem to be a good candidate to me here, I wouldn't call a size that has\n> > >> a width or height (or both) equal to 0 as \"invalid\". It's a\n> > >> zero/null/empty size, but not intrinsicly invalid. Whether such a size\n> > >> can be valid for a given usage depends on the usage, but isn't a\n> > >> property of the Size class in my opinion.\n> > >>\n> > > \n> > > As you wish. This is going into the philosophical territory. To me a\n> > > Size with any of its member set to 0 is empty, null, invalid or\n> > > whatever as I don't see a need to distinguish between an \"empty\" a\n> > > \"null\" or a \"valid\" size. I just wanted to use the most widespread\n> > > name we have to avoid having to look it up every time, which is just\n> > > annoying. That said, up to you :)\n> > \n> > \n> > Personally I would say that a size of 0 is 'valid'. Maybe it depends on\n> > context, but I can attest that the size of my cherry-bakewell is now 0\n> > in both width, height (and depth):\n> > \n> > if (cherry-bakewell.size == Size(0, 0)) {\n> > \tstd::cout << \"Was it yummy?\" << std::endl;\n> > } else {\n> > \tstd::cout << \"C'mon - eat up...\" << std::endl;\n> > }\n> > \n> > isNull, isZero, would both be fine with me (I'd actually use isZero) But\n> > for some reason - not isEmpty().\n> > \n> > I don't think you'd have an empty size, as the 'size' is not a container.","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 5FCD2C2E65\n\tfor ;\n\tFri, 26 Jun 2020 01:50:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2FEEF609C6;\n\tFri, 26 Jun 2020 03:50:51 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9FEB1609C3\n\tfor ;\n\tFri, 26 Jun 2020 03:50:49 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3350672E;\n\tFri, 26 Jun 2020 03:50:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"I6Nv1cpG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593136249;\n\tbh=PshL2derxWLnXyMHwCvmZNa0cWjsbohp+MES6+T2k3s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=I6Nv1cpGqUQhO5j3VlTJ729UWloVhxuLqf+4u51CRFXeyVJK6+9R4W9Fma5PpNUWW\n\tkre0XPGuPS01IlcuYOZJOdcd7N3fIGcrB2656DqyJ9qxUt5o9LUhXmGgHzxOUMUGVN\n\tBEIBf1nGNiN+v9cfqf0qhmljnCxGA6g/+tSwpOsw=","Date":"Fri, 26 Jun 2020 04:50:47 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200626015047.GX5865@pendragon.ideasonboard.com>","References":"<20200625012330.7639-1-laurent.pinchart@ideasonboard.com>\n\t<20200625073739.75v2mgdq76j4jc5b@uno.localdomain>\n\t<20200625085705.GB5865@pendragon.ideasonboard.com>\n\t<20200625090733.j3eeq5bb3rhr632m@uno.localdomain>\n\t<20200625090739.GD5865@pendragon.ideasonboard.com>\n\t<20200625092617.ciw3rdifn63lmsht@uno.localdomain>\n\t<20200625095115.GF5865@pendragon.ideasonboard.com>\n\t<20200625101743.tui7e5tjslwgcdci@uno.localdomain>\n\t<55341149-45a4-6aa7-a02f-6ffe7a4293a1@ideasonboard.com>\n\t<20200626012505.GB477866@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200626012505.GB477866@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: geometry: Add isNull()\n\tfunction to Size class","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]