[{"id":29222,"web_url":"https://patchwork.libcamera.org/comment/29222/","msgid":"<20240415102622.p6m3whj2mkxm32iu@jasper>","date":"2024-04-15T10:26:22","subject":"Re: [PATCH 1/4] libcamera: geometry: Add floating-point version of\n\tPoint class","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Paul,\n\nthanks for the patch.\n\nOn Fri, Apr 05, 2024 at 05:02:56PM +0900, Paul Elder wrote:\n> The piecewise linear function (Pwl) class from the Raspberry Pi IPA has\n> its own Point class while one already exists in libcamera's geometry.h\n> header. The reason is because libcamera's Point is on the plane of\n> integer, while Raspberry Pi's is on the plane of reals.\n> \n> While making this a template class might be cleaner, it was deemed to be\n> too intrusive of a change, and it might not feel nice to need to specify\n> the type from a public API point of view. Hence we introduce a FPoint\n> class to designate a Point class on the plane of reals.\n\nSounds reasonable to me. One thing that came to my mind is a bit\nnitpicking on the name. I would name it PointF, so that the Point and\nPointF types end up at similar places in alphabetical listings (docs,\ntype-completion etc.)\n\n> \n> This is in preparation for copying/moving the Pwl class from the\n> Raspberry Pi IPA to libipa.\n> \n> Signed-off-by: Paul Elder \n> ---\n> include/libcamera/geometry.h | 50 ++++++++++++++++++++++++++++++++++++\n> 1 file changed, 50 insertions(+)\n> \n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index d7fdbe70..7d0c0f23 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n> \n> #include \n> +#include \n> #include \n> #include \n> \n> @@ -49,6 +50,55 @@ static inline bool operator!=(const Point &lhs, const Point &rhs)\n> \n> std::ostream &operator<<(std::ostream &out, const Point &p);\n> \n> +struct FPoint {\n> +\tconstexpr FPoint()\n> +\t\t: x(0), y(0)\n> +\t{\n> +\t}\n> +\n> +\tconstexpr FPoint(double _x, double _y)\n> +\t\t: x(_x), y(_y)\n> +\t{\n> +\t}\n> +\n> +\tconstexpr FPoint operator-(FPoint const &p) const\n> +\t{\n> +\t\treturn FPoint(x - p.x, y - p.y);\n> +\t}\n> +\n> +\tconstexpr FPoint operator+(FPoint const &p) const\n> +\t{\n> +\t\treturn FPoint(x + p.x, y + p.y);\n> +\t}\n> +\n> +\tconstexpr double operator%(FPoint const &p) const\n> +\t{\n> +\t\treturn x * p.x + y * p.y;\n> +\t}\n> +\n> +\tconstexpr FPoint operator*(double f) const\n> +\t{\n> +\t\treturn FPoint(x * f, y * f);\n> +\t}\n> +\n> +\tconstexpr FPoint operator/(double f) const\n> +\t{\n> +\t\treturn FPoint(x / f, y / f);\n> +\t}\n> +\n> +\tconstexpr double len2() const\n> +\t{\n> +\t\treturn x * x + y * y;\n> +\t}\n> +\n> +\tconstexpr double len() const\n> +\t{\n> +\t\treturn std::sqrt(len2());\n> +\t}\n> +\n> +\tdouble x, y;\n> +};\n\nAs this is in the public API, we should add documentation for it. I also\nsaw some tests for Point in test/geometry.cpp. These should be\nduplicated for the float version.\n\nFor completeness sake, the operator!= and operator<<(std::ostream &out,\nconst FPoint &p) should also be copied.\n\nBest regards,\nStefan\n\n> +\n> class Size\n> {\n> public:\n> -- \n> 2.39.2\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 8E638BE08B\n\tfor ;\n\tMon, 15 Apr 2024 10:26:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 746B463352;\n\tMon, 15 Apr 2024 12:26:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5F4F663339\n\tfor ;\n\tMon, 15 Apr 2024 12:26:26 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7a0d:dd2e:881a:db83])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C2017236;\n\tMon, 15 Apr 2024 12:25:40 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"K+ZlnmhW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713176740;\n\tbh=Y60pLfG3x2CCYENapUwP8+rlphHIreL3Zv1Q/kdvlGU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=K+ZlnmhWlJDbDgwnI2BzN3NVm+IjW8xC/a/6VmMUkd1AkITTJ6IAhHTsd1g+/eWe5\n\t5ieC3mymm+0HFpGaKcL9yPWWIbc6fi9ctPHEPWmhu29xYQ1sW/0qnTSvfYFmiIhjE9\n\t+R8ipGacZhYy/lPh50GFtwNF/W97Va6kt7KKAuos=","Date":"Mon, 15 Apr 2024 12:26:22 +0200","From":"Stefan Klug ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/4] libcamera: geometry: Add floating-point version of\n\tPoint class","Message-ID":"<20240415102622.p6m3whj2mkxm32iu@jasper>","References":"<20240405080259.1806453-1-paul.elder@ideasonboard.com>\n\t<20240405080259.1806453-2-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240405080259.1806453-2-paul.elder@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]