[{"id":32331,"web_url":"https://patchwork.libcamera.org/comment/32331/","msgid":"","date":"2024-11-21T11:45:47","subject":"Re: [PATCH 3/7] libcamera: geometry: Add Rectangle::mappedBetween()","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Wed, Nov 20, 2024 at 09:57:42AM +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\nLet me try to clear my thoughts on this point, I've gone through\nre-scaling ScalerCrops too many times.\n\nCurrently the way all pipline handlers do this is\n\n\tRectangle ispCrop = nativeCrop.translatedBy(-sensor.topLeft());\n\tispCrop.scaleBy(display.size(), sensor.size());\n\nwith:\n nativeCrop = The crop rectangles expressed on the sensor\n native pixel array (controls::ScalerCrop)\n sensor = The sensor analog crop rectangle\n display = The destination reference system\n\nThe assumption is that nativeCrop is always contained in the\nanalogCrop, as per the definition of the ScalerCrop control and of\nScalerCropMaximum.\n\nGood news is that\n\n\tRectangle sensor = { 0, 243, 2592, 1458 };\n\tRectangle display = { 0, 0, 1920, 1080 };\n\tRectangle nativeCrop = { 400, 400, 640, 480 };\n\n\tRectangle scaled = nativeCrop.mappedBetween(sensor, display);\n\n\tcout << \"Scaled: \" << scaled << endl;\n\n\tRectangle ispCrop = nativeCrop.translatedBy(-sensor.topLeft());\n\tispCrop.scaleBy(display.size(), sensor.size());\n\n\tcout << \"ISP Crop: \" << ispCrop << endl;\n\nGive the same result\n\nScaled: (296, 116)/474x355\nISP Crop: (296, 116)/474x355\n\n>\n> Signed-off-by: Stefan Klug \n> ---\n> include/libcamera/geometry.h | 2 ++\n> src/libcamera/geometry.cpp | 33 +++++++++++++++++++++++++++++++++\n> 2 files changed, 35 insertions(+)\n>\n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index 9ca5865a3d0d..8c5f34f4dec1 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -299,6 +299,8 @@ 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 mappedBetween(const Rectangle &source, 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..7e29fe35801e 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -837,6 +837,39 @@ Rectangle Rectangle::translatedBy(const Point &point) const\n> \treturn { x + point.x, y + point.y, width, height };\n> }\n>\n> +/**\n> + * \\brief Maps a Rectangle from one reference frame to another\n> + * \\param[in] source The source reference frame\n> + * \\param[in] destination The destination reference frame\n> + *\n> + * The params source and destinations specify two reference frames for the same\n> + * data. The rectangle is translated from the source reference frame into the\n> + * destination reference frame.\n> + *\n> + * For example, consider a sensor with a resolution of 2592x1458 pixels and a\n> + * assume a rectangle of (0, 243)/2592x1458 (sensorFrame) in sensor coordinates\n> + * is mapped to a rectangle (0,0)/(1920,1080) (displayFrame) in display\n> + * coordinates. This function can be used to transform an arbitrary rectangle\n> + * from display coordinates to sensor coordinates or vice versa:\n> + *\n> + * \\code{.cpp}\n> + * displayRect = sensorRect.mappedBetween(sensorFrame, displayFrame);\n> + * \\endcode\n> + */\n> +Rectangle Rectangle::mappedBetween(const Rectangle &source,\n> +\t\t\t\t const Rectangle &destination) const\n> +{\n\nLooking at the code of the two examples above, the sequence\n\n\tRectangle ispCrop = nativeCrop.translatedBy(-sensor.topLeft());\n\tispCrop.scaleBy(display.size(), sensor.size());\n\ntranslates to\n\n\tr.x = x + destination.x;\n r.y = y + destination.y;\n\n\tr.x = static_cast(r.x) * sx;\n\tr.y = static_cast(r.y) * sy;\n\twidth = static_cast(width) * sx;\n\theight = static_cast(height) * sy;\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> +\treturn r;\n\nWhich is equivalent to your code here if not for the '+ destination.x'\nand '+ destination.y' in:\n\n\tr.x = static_cast((x - source.x) * sx) + destination.x;\n\tr.y = static_cast((y - source.y) * sy) + destination.y;\n\nNow, Rectangle::scaleBy() takes two sizes instead of rectangles, so it\nassumes that the top-left point of the destination is not relevant.\nIs it in the case you want to support ?\n\nAlso, can\n\n\tr.x = static_cast((x - source.x) * sx) + destination.x;\n\tr.y = static_cast((y - source.y) * sy) + destination.y;\n\nresult in any negative value ?\n\n\n> +}\n> +\n> /**\n> * \\brief Compare rectangles for equality\n> * \\return True if the two rectangles are equal, false otherwise\n> --\n> 2.43.0\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 2CD6DC32F9\n\tfor ;\n\tThu, 21 Nov 2024 11:45:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9407465FC1;\n\tThu, 21 Nov 2024 12:45:52 +0100 (CET)","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 1B29B65F9F\n\tfor ;\n\tThu, 21 Nov 2024 12:45:51 +0100 (CET)","from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it\n\t[93.46.82.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3F21B670;\n\tThu, 21 Nov 2024 12:45:32 +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=\"SJEaOe2e\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732189532;\n\tbh=DWLheKlL1ylJGGHbWQIbBVZNVTb36LRdDAnoUr/gKZ4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SJEaOe2erfagKhhmR3++xtKJ52zH4EF7iLqpla1JDIsQxjLFi/EDYZBl/nEngvMe8\n\t9cguSMTy9/4zMy7gYhqnQJ2fGXhZGvVFehTcGrz1mdJiy4PRc+zgiilUKF5nr44WEH\n\tqbzvB7y1fxWiFlSuwTrHCeBP1Q04YaXfzYpE4drA=","Date":"Thu, 21 Nov 2024 12:45:47 +0100","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 3/7] libcamera: geometry: Add Rectangle::mappedBetween()","Message-ID":"","References":"<20241120085753.1993436-1-stefan.klug@ideasonboard.com>\n\t<20241120085753.1993436-4-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241120085753.1993436-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\" "}}]