[{"id":32854,"web_url":"https://patchwork.libcamera.org/comment/32854/","msgid":"<20241217213440.GI23470@pendragon.ideasonboard.com>","date":"2024-12-17T21:34:40","subject":"Re: [PATCH v4 03/20] libcamera: geometry: Add\n\tRectangle::transformedBetween()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Dec 16, 2024 at 04:40:43PM +0100, Stefan Klug wrote:\n> Handling cropping and scaling within a complicated pipeline involves\n> transformations of rectangles between different coordinate systems. For\n> example the full input of the dewarper (0,0)/1920x1080 might correspond\n> to the rectangle (0, 243)/2592x1458 in sensor coordinates (of a\n> 2592x1944 sensor). Add a function that allows the transformation of a\n> rectangle defined in one reference frame (dewarper) into the coordinates\n> of a second reference frame (sensor).\n> \n> Signed-off-by: Stefan Klug \n> Reviewed-by: Paul Elder \n> Reviewed-by: Jacopo Mondi \n> \n> ---\n> Changes in v3:\n> - Improved documentation\n> - Added testcase\n> \n> Changes in v2:\n> - Renamed mappedBetween() to transformedBetween()\n> - Improved documentation\n> - Collected tags\n> ---\n> include/libcamera/geometry.h | 3 +++\n> src/libcamera/geometry.cpp | 49 ++++++++++++++++++++++++++++++++++++\n> test/geometry.cpp | 11 ++++++++\n> 3 files changed, 63 insertions(+)\n> \n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index 9ca5865a3d0d..e5f0a843d314 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -299,6 +299,9 @@ public:\n> \t__nodiscard Rectangle scaledBy(const Size &numerator,\n> \t\t\t\t const Size &denominator) const;\n> \t__nodiscard Rectangle translatedBy(const Point &point) const;\n> +\n> +\tRectangle transformedBetween(const Rectangle &source,\n> +\t\t\t\t const Rectangle &target) const;\n> };\n> \n> bool operator==(const Rectangle &lhs, const Rectangle &rhs);\n> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> index 90ccf8c19f97..81cc8cd538a0 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -837,6 +837,55 @@ Rectangle Rectangle::translatedBy(const Point &point) const\n> \treturn { x + point.x, y + point.y, width, height };\n> }\n> \n> +/**\n> + * \\brief Transform a Rectangle from one reference rectangle to another\n> + * \\param[in] source The \\a source reference rectangle\n> + * \\param[in] destination The \\a destination reference rectangle\n> + *\n> + * The \\a source and \\a destination parameters describe two rectangles defined\n> + * in different reference systems. The Rectangle is translated from the source\n\ns/translated/transformed/\n\n> + * reference system into the destination reference system.\n> + *\n> + * The typical use case for this function is to translate a selection rectangle\n> + * specified in a reference system, in example the sensor's pixel array, into\n\ns/in example/for example/ (or \"for instance\")\n\n> + * the same rectangle re-scaled and translated into a different reference\n> + * system, in example the output frame on which the selection rectangle is\n\nSame here.\n\n> + * applied to.\n\ns/ to// (or s/on which/which/ in the previous line)\n\n> + *\n> + * For example, consider a sensor with a resolution of 4040x2360 pixels and a\n\nI think there's a stray \"a\" at the end of the line.\n\n> + * assume a rectangle of (100, 100)/3840x2160 (sensorFrame) in sensor\n> + * coordinates is mapped to a rectangle (0,0)/(1920,1080) (displayFrame) in\n> + * display coordinates. This function can be used to transform an arbitrary\n> + * rectangle from display coordinates to sensor coordinates or vice versa:\n> + *\n> + * \\code{.cpp}\n> + * Rectangle sensorReference(100, 100, 3840, 2160);\n> + * Rectangle displayReference(0, 0, 1920, 1080);\n> + *\n> + * // Bottom right quarter in sensor coordinates\n> + * Rectangle sensorRect(2020, 100, 1920, 1080);\n> + * displayRect = sensorRect.transformedBetween(sensorReference, displayReference);\n> + * // displayRect is now (960, 540)/960x540\n> + *\n> + * // Transformation back to sensor coordinates\n> + * sensorRect = displayRect.transformedBetween(displayReference, sensorReference);\n> + * \\endcode\n\nMissing \\return\n\n> + */\n> +Rectangle Rectangle::transformedBetween(const Rectangle &source,\n> +\t\t\t\t\tconst Rectangle &destination) const\n> +{\n> +\tRectangle r;\n> +\tdouble sx = static_cast(destination.width) / source.width;\n> +\tdouble sy = static_cast(destination.height) / source.height;\n> +\n> +\tr.x = static_cast((x - source.x) * sx) + destination.x;\n> +\tr.y = static_cast((y - source.y) * sy) + destination.y;\n> +\tr.width = static_cast(width * sx);\n> +\tr.height = static_cast(height * sy);\n\nwidth and height are unsigned, so static_cast is more\nappropriate.\n\nIs rounding down instead of rounding to the closest value the best\noption ?\n\n> +\n> +\treturn r;\n> +}\n> +\n\nI'm wondering if the Rectangle API isn't getting a bit too complicated.\nSomething like https://doc.qt.io/qt-6/qtransform.html may be more\nappropriate. No need to act on this now.\n\n> /**\n> * \\brief Compare rectangles for equality\n> * \\return True if the two rectangles are equal, false otherwise\n> diff --git a/test/geometry.cpp b/test/geometry.cpp\n> index 5760fa3c885a..11df043b733b 100644\n> --- a/test/geometry.cpp\n> +++ b/test/geometry.cpp\n> @@ -495,6 +495,17 @@ protected:\n> \t\t\treturn TestFail;\n> \t\t}\n> \n> +\t\tRectangle f1 = Rectangle(100, 200, 3000, 2000);\n> +\t\tRectangle f2 = Rectangle(200, 300, 1500, 1000);\n\nYou could have written\n\n\t\tRectangle f1(100, 200, 3000, 2000);\n\t\tRectangle f2(200, 300, 1500, 1000);\n\n> +\t\t/* Bottom right quarter of the corresponding frames. */\n> +\t\tRectangle r1 = Rectangle(100 + 1500, 200 + 1000, 1500, 1000);\n> +\t\tRectangle r2 = Rectangle(200 + 750, 300 + 500, 750, 500);\n\nSame here. No big deal.\n\n> +\t\tif (r1.transformedBetween(f1, f2) != r2 ||\n> +\t\t r2.transformedBetween(f2, f1) != r1) {\n> +\t\t\tcout << \"Rectangle::transformedBetween() test failed\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> \t\treturn TestPass;\n> \t}\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 6002DBD1F1\n\tfor ;\n\tTue, 17 Dec 2024 21:34:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9ED4E68059;\n\tTue, 17 Dec 2024 22:34:44 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 872F467FD3\n\tfor ;\n\tTue, 17 Dec 2024 22:34:42 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C05543E;\n\tTue, 17 Dec 2024 22:34:04 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"SrvhjkWz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734471245;\n\tbh=ogICOnx98iyVlx0eDdRYFnSSHx3hPj9Lk4Rcy8PAcLc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SrvhjkWzN0ZUe1b1+wUsFtuMzoWAAVpr2qB2TsQRVfC6cmzzVhhHGk9tyccJG+m+3\n\toX2Daat2nSc2luQnALjDcbW64i3SVedhdnbNqmf8h0WlxTKogX5r1BPRyU9hzyNErB\n\twbo3lLNlCws7OEiQufLqmil07ICEMerh/P7n5W0w=","Date":"Tue, 17 Dec 2024 23:34:40 +0200","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org,\n\tPaul Elder ,\n\tJacopo Mondi ","Subject":"Re: [PATCH v4 03/20] libcamera: geometry: Add\n\tRectangle::transformedBetween()","Message-ID":"<20241217213440.GI23470@pendragon.ideasonboard.com>","References":"<20241216154124.203650-1-stefan.klug@ideasonboard.com>\n\t<20241216154124.203650-4-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241216154124.203650-4-stefan.klug@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\" "}}]