[{"id":32365,"web_url":"https://patchwork.libcamera.org/comment/32365/","msgid":"<uifqfcjras3vac72upsyp2kmoatjbeguc5zugurpoa3dnvcbbq@4tteqjso7noy>","date":"2024-11-25T18:20:46","subject":"Re: [PATCH v2 3/8] libcamera: geometry: Add\n\tRectangle::transformedBetween()","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Mon, Nov 25, 2024 at 04:14:12PM +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 <stefan.klug@ideasonboard.com>\n> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n>\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   | 37 ++++++++++++++++++++++++++++++++++++\n>  2 files changed, 40 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..2c9308cb25ee 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -837,6 +837,43 @@ Rectangle Rectangle::translatedBy(const Point &point) const\n>  \treturn { x + point.x, y + point.y, width, height };\n>  }\n>\n> +/**\n> + * \\brief Transforms a Rectangle from one reference frame to another\n\nnit: documentation is usually in imperative form:\nTransforms -> Transform\n\n> + * \\param[in] source The \\a source reference frame\n> + * \\param[in] destination The \\a destination reference frame\n\nI'm not sure I 100% like 'frame' here. The geometry library is not\nonly about frames or images...\n\n\n> + *\n> + * The params source and destinations specify two reference frames for the same\n\nLet's spell out terms in full in documentation.\n\n\"The \\a source and \\a destination parameters \"\n\nAlso, I would find it cleaner if we talk about Rectangles here.\n\n\"The \\a source and \\a destination parameters describe two rectangles\ndefined in different reference systems. The Rectangle is translated\nfrom the source reference system into the destination reference\nsystem.\"\n\n\n> + * data. The rectangle is translated from the source reference frame into the\n> + * destination reference frame.\n> + *\n\nWe can say\n\n\"The typical use case for this function is to translate a selection\nrectangle specified in a reference system, in example the sensor's\npixel array, into the same rectangle re-scaled and translated into\na different reference system, in example the output frame on\nwhich the selection rectangle is applied to.\"\n\n> + * For example, consider a sensor with a resolution of 4040x2360 pixels and a\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\nAnd then provide a concrete example ? What do you think ? is is too\nmuch details ?\n\n> + *\n> + * \\code{.cpp}\n> + * // Bottom right quarter in sensor coordinates\n> + * Rectangle sensorRect(2020, 100, 1920, 1080);\n> + * displayRect = sensorRect.transformedBetween(sensorFrame, displayFrame);\n> + * // displayRect is now (960, 540)/960x540\n> + * sensorRect = displayRect.transformedBetween(displayFrame, sensorFrame);\n> + * \\endcode\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<double>(destination.width) / source.width;\n> +\tdouble sy = static_cast<double>(destination.height) / source.height;\n> +\n> +\tr.x = static_cast<int>((x - source.x) * sx) + destination.x;\n> +\tr.y = static_cast<int>((y - source.y) * sy) + destination.y;\n> +\tr.width = static_cast<int>(width * sx);\n> +\tr.height = static_cast<int>(height * sy);\n\nnit: I would add a blank line here\n\n> +\treturn r;\n> +}\n> +\n\nnits and nitpicking apart\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nThanks\n  j\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":"<libcamera-devel-bounces@lists.libcamera.org>","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 4C7D7BD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Nov 2024 18:20:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 96F8F65F81;\n\tMon, 25 Nov 2024 19:20:51 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 200B665F74\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Nov 2024 19:20:50 +0100 (CET)","from ideasonboard.com (mob-5-90-139-188.net.vodafone.it\n\t[5.90.139.188])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 487FD6B5;\n\tMon, 25 Nov 2024 19:20:28 +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=\"Bt7+RuRG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732558828;\n\tbh=tnhwfnqQ3uf1rJDBcGlfuwiUMKVAyNyUEl0sxPU+HuQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Bt7+RuRGkLj33qEZdG2zauhHuLr35xGBcFI+DdHhditwYcC2wwfWzsp/43Lwgnih2\n\tTWpjVxbe6jdFi+4V9Xg1rqJTvRVLxA45t8Btznv1k+Qeo37QAXpcOa7rFY4uoyG5nP\n\tGG4muYDk3JsBa3r7DONgW12BdtKu0aLZGZ5TUOKU=","Date":"Mon, 25 Nov 2024 19:20:46 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, \n\tPaul Elder <paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v2 3/8] libcamera: geometry: Add\n\tRectangle::transformedBetween()","Message-ID":"<uifqfcjras3vac72upsyp2kmoatjbeguc5zugurpoa3dnvcbbq@4tteqjso7noy>","References":"<20241125151430.2437285-1-stefan.klug@ideasonboard.com>\n\t<20241125151430.2437285-4-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241125151430.2437285-4-stefan.klug@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32369,"web_url":"https://patchwork.libcamera.org/comment/32369/","msgid":"<6hkgpzgw2k5x2cd5kexaaxfyj7wmy3sbzvlbaetf4wlsi477kt@ye5nd2pa7zfg>","date":"2024-11-25T19:28:27","subject":"Re: [PATCH v2 3/8] libcamera: geometry: Add\n\tRectangle::transformedBetween()","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Ups, I think you'll need a test for this though, even a simple one\nthat tests the example you have made in the documentation..\n\nOn Mon, Nov 25, 2024 at 07:20:46PM +0100, Jacopo Mondi wrote:\n> Hi Stefan\n>\n> On Mon, Nov 25, 2024 at 04:14:12PM +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 <stefan.klug@ideasonboard.com>\n> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> >\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   | 37 ++++++++++++++++++++++++++++++++++++\n> >  2 files changed, 40 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..2c9308cb25ee 100644\n> > --- a/src/libcamera/geometry.cpp\n> > +++ b/src/libcamera/geometry.cpp\n> > @@ -837,6 +837,43 @@ Rectangle Rectangle::translatedBy(const Point &point) const\n> >  \treturn { x + point.x, y + point.y, width, height };\n> >  }\n> >\n> > +/**\n> > + * \\brief Transforms a Rectangle from one reference frame to another\n>\n> nit: documentation is usually in imperative form:\n> Transforms -> Transform\n>\n> > + * \\param[in] source The \\a source reference frame\n> > + * \\param[in] destination The \\a destination reference frame\n>\n> I'm not sure I 100% like 'frame' here. The geometry library is not\n> only about frames or images...\n>\n>\n> > + *\n> > + * The params source and destinations specify two reference frames for the same\n>\n> Let's spell out terms in full in documentation.\n>\n> \"The \\a source and \\a destination parameters \"\n>\n> Also, I would find it cleaner if we talk about Rectangles here.\n>\n> \"The \\a source and \\a destination parameters describe two rectangles\n> defined in different reference systems. The Rectangle is translated\n> from the source reference system into the destination reference\n> system.\"\n>\n>\n> > + * data. The rectangle is translated from the source reference frame into the\n> > + * destination reference frame.\n> > + *\n>\n> We can say\n>\n> \"The typical use case for this function is to translate a selection\n> rectangle specified in a reference system, in example the sensor's\n> pixel array, into the same rectangle re-scaled and translated into\n> a different reference system, in example the output frame on\n> which the selection rectangle is applied to.\"\n>\n> > + * For example, consider a sensor with a resolution of 4040x2360 pixels and a\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> And then provide a concrete example ? What do you think ? is is too\n> much details ?\n>\n> > + *\n> > + * \\code{.cpp}\n> > + * // Bottom right quarter in sensor coordinates\n> > + * Rectangle sensorRect(2020, 100, 1920, 1080);\n> > + * displayRect = sensorRect.transformedBetween(sensorFrame, displayFrame);\n> > + * // displayRect is now (960, 540)/960x540\n> > + * sensorRect = displayRect.transformedBetween(displayFrame, sensorFrame);\n> > + * \\endcode\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<double>(destination.width) / source.width;\n> > +\tdouble sy = static_cast<double>(destination.height) / source.height;\n> > +\n> > +\tr.x = static_cast<int>((x - source.x) * sx) + destination.x;\n> > +\tr.y = static_cast<int>((y - source.y) * sy) + destination.y;\n> > +\tr.width = static_cast<int>(width * sx);\n> > +\tr.height = static_cast<int>(height * sy);\n>\n> nit: I would add a blank line here\n>\n> > +\treturn r;\n> > +}\n> > +\n>\n> nits and nitpicking apart\n> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>\n> Thanks\n>   j\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":"<libcamera-devel-bounces@lists.libcamera.org>","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 AFB94BD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Nov 2024 19:28:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6FCAE66034;\n\tMon, 25 Nov 2024 20:28:33 +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 0E4F965F66\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Nov 2024 20:28:32 +0100 (CET)","from ideasonboard.com (mob-5-90-139-188.net.vodafone.it\n\t[5.90.139.188])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 300B14AD;\n\tMon, 25 Nov 2024 20:28:10 +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=\"Kr9TTaPI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732562890;\n\tbh=PzSFU7FzGdJbkGA8pILb0TvnyEfEq752Aydt/KSJDJY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Kr9TTaPItOTu4HVIcRY/oi0PHBxrbecNAWs43WeD4k3mHGogMLdaeygFUPsLBmduV\n\tNOpoFZ9q9hG+XAE6oaPKeWh1c8mY8E7P1mpryLxrk09UftGVE9fEs4gyOUORsuohtI\n\tjX4mmDwhZPnJ177Kxz2n/nUaX6jdPpk3y37L8Bqw=","Date":"Mon, 25 Nov 2024 20:28:27 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org,\n\tPaul Elder <paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v2 3/8] libcamera: geometry: Add\n\tRectangle::transformedBetween()","Message-ID":"<6hkgpzgw2k5x2cd5kexaaxfyj7wmy3sbzvlbaetf4wlsi477kt@ye5nd2pa7zfg>","References":"<20241125151430.2437285-1-stefan.klug@ideasonboard.com>\n\t<20241125151430.2437285-4-stefan.klug@ideasonboard.com>\n\t<uifqfcjras3vac72upsyp2kmoatjbeguc5zugurpoa3dnvcbbq@4tteqjso7noy>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<uifqfcjras3vac72upsyp2kmoatjbeguc5zugurpoa3dnvcbbq@4tteqjso7noy>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32424,"web_url":"https://patchwork.libcamera.org/comment/32424/","msgid":"<gob6znng2pnu6xicid5ayfn5rsw7d3eyxrc4qbgn743in4kbbt@uybcqyo2msmd>","date":"2024-11-28T12:58:08","subject":"Re: [PATCH v2 3/8] libcamera: geometry: Add\n\tRectangle::transformedBetween()","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Nov 25, 2024 at 08:28:27PM +0100, Jacopo Mondi wrote:\n> Ups, I think you'll need a test for this though, even a simple one\n> that tests the example you have made in the documentation..\n\nRight that was forgotten in v2, now added for v3.\n\n> \n> On Mon, Nov 25, 2024 at 07:20:46PM +0100, Jacopo Mondi wrote:\n> > Hi Stefan\n> >\n> > On Mon, Nov 25, 2024 at 04:14:12PM +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 <stefan.klug@ideasonboard.com>\n> > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> > >\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   | 37 ++++++++++++++++++++++++++++++++++++\n> > >  2 files changed, 40 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..2c9308cb25ee 100644\n> > > --- a/src/libcamera/geometry.cpp\n> > > +++ b/src/libcamera/geometry.cpp\n> > > @@ -837,6 +837,43 @@ Rectangle Rectangle::translatedBy(const Point &point) const\n> > >  \treturn { x + point.x, y + point.y, width, height };\n> > >  }\n> > >\n> > > +/**\n> > > + * \\brief Transforms a Rectangle from one reference frame to another\n> >\n> > nit: documentation is usually in imperative form:\n> > Transforms -> Transform\n> >\n> > > + * \\param[in] source The \\a source reference frame\n> > > + * \\param[in] destination The \\a destination reference frame\n> >\n> > I'm not sure I 100% like 'frame' here. The geometry library is not\n> > only about frames or images...\n> >\n> >\n> > > + *\n> > > + * The params source and destinations specify two reference frames for the same\n> >\n> > Let's spell out terms in full in documentation.\n> >\n> > \"The \\a source and \\a destination parameters \"\n> >\n> > Also, I would find it cleaner if we talk about Rectangles here.\n> >\n> > \"The \\a source and \\a destination parameters describe two rectangles\n> > defined in different reference systems. The Rectangle is translated\n> > from the source reference system into the destination reference\n> > system.\"\n> >\n> >\n> > > + * data. The rectangle is translated from the source reference frame into the\n> > > + * destination reference frame.\n> > > + *\n> >\n> > We can say\n> >\n> > \"The typical use case for this function is to translate a selection\n> > rectangle specified in a reference system, in example the sensor's\n> > pixel array, into the same rectangle re-scaled and translated into\n> > a different reference system, in example the output frame on\n> > which the selection rectangle is applied to.\"\n> >\n> > > + * For example, consider a sensor with a resolution of 4040x2360 pixels and a\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> > And then provide a concrete example ? What do you think ? is is too\n> > much details ?\n\nThanks for the suggestions. I took all of them. Let's see how you like\nv3.\n\n> >\n> > > + *\n> > > + * \\code{.cpp}\n> > > + * // Bottom right quarter in sensor coordinates\n> > > + * Rectangle sensorRect(2020, 100, 1920, 1080);\n> > > + * displayRect = sensorRect.transformedBetween(sensorFrame, displayFrame);\n> > > + * // displayRect is now (960, 540)/960x540\n> > > + * sensorRect = displayRect.transformedBetween(displayFrame, sensorFrame);\n> > > + * \\endcode\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<double>(destination.width) / source.width;\n> > > +\tdouble sy = static_cast<double>(destination.height) / source.height;\n> > > +\n> > > +\tr.x = static_cast<int>((x - source.x) * sx) + destination.x;\n> > > +\tr.y = static_cast<int>((y - source.y) * sy) + destination.y;\n> > > +\tr.width = static_cast<int>(width * sx);\n> > > +\tr.height = static_cast<int>(height * sy);\n> >\n> > nit: I would add a blank line here\n> >\n\nDone.\n\nCheers,\nStefan\n\n> > > +\treturn r;\n> > > +}\n> > > +\n> >\n> > nits and nitpicking apart\n> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> >\n> > Thanks\n> >   j\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":"<libcamera-devel-bounces@lists.libcamera.org>","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 0C2B9BDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Nov 2024 12:58:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 08AED65F67;\n\tThu, 28 Nov 2024 13:58:12 +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 8B85565898\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Nov 2024 13:58:10 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:22e0:a94:b035:820])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 967B759D;\n\tThu, 28 Nov 2024 13:57:46 +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=\"jPcqQ6zN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732798666;\n\tbh=5hKXFD1HsZydVZAbPWViK+sxW0PWRn6y+siW19J7+VE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jPcqQ6zN/Ix5s1pmz7qjQ3cu4yydRQEz4luvQ9Hw6q8vjWWlUqDAhESeXUOLo30YG\n\tknVVfHr32kZ8Jt8blvmnFTAP5WqWZxRp8tsaL/ZAXtDVVpATi69nMv8U7V8rt0dQMY\n\tJ+sy8yfZQ+2Y5/qh3nJbLcCeYUKmmFKXfXMVH6Nc=","Date":"Thu, 28 Nov 2024 13:58:08 +0100","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, \n\tPaul Elder <paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v2 3/8] libcamera: geometry: Add\n\tRectangle::transformedBetween()","Message-ID":"<gob6znng2pnu6xicid5ayfn5rsw7d3eyxrc4qbgn743in4kbbt@uybcqyo2msmd>","References":"<20241125151430.2437285-1-stefan.klug@ideasonboard.com>\n\t<20241125151430.2437285-4-stefan.klug@ideasonboard.com>\n\t<uifqfcjras3vac72upsyp2kmoatjbeguc5zugurpoa3dnvcbbq@4tteqjso7noy>\n\t<6hkgpzgw2k5x2cd5kexaaxfyj7wmy3sbzvlbaetf4wlsi477kt@ye5nd2pa7zfg>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<6hkgpzgw2k5x2cd5kexaaxfyj7wmy3sbzvlbaetf4wlsi477kt@ye5nd2pa7zfg>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]