[{"id":34977,"web_url":"https://patchwork.libcamera.org/comment/34977/","msgid":"<175311820497.50296.7386672516877998226@ping.linuxembedded.co.uk>","date":"2025-07-21T17:16:44","subject":"Re: [PATCH 1/2] libcamera: geometry: Add Rectangle::croppedBy","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2025-07-18 15:44:55)\n> Let's add a new Rectangle method that returns the coordinates of the\n> central area of the original rectangle cropped by given\n> numerator/denominator fraction. This is useful to specify a limited\n> area of the image to operate on, for example computing statistics only\n> from a reduced area for speedup.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> include/libcamera/geometry.h | 2 ++\n> src/libcamera/geometry.cpp | 18 ++++++++++++++++++\n> 2 files changed, 20 insertions(+)\n> \n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index f322e3d5b..603c9a117 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -297,6 +297,8 @@ public:\n> [[nodiscard]] Rectangle scaledBy(const Size &numerator,\n> const Size &denominator) const;\n> [[nodiscard]] Rectangle translatedBy(const Point &point) const;\n> + [[nodiscard]] Rectangle croppedBy(const unsigned int numerator,\n> + const unsigned int denominator) const;\n> \n> Rectangle transformedBetween(const Rectangle &source,\n> const Rectangle &target) const;\n> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> index 81cc8cd53..8c1b156c5 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -837,6 +837,24 @@ Rectangle Rectangle::translatedBy(const Point &point) const\n> return { x + point.x, y + point.y, width, height };\n> }\n> \n> +/**\n> + * \\brief Return a central area crop of the rectangle\n> + * \\param[in] numerator The numerator of the crop factor\n> + * \\param[in] denominator The denominator of the crop factor\n> + *\n> + * The rectangle of the central part of the original rectangle is computed, of\n> + * numerator/denominator times the original width and height.\n> + *\n> + * \\return The cropped Rectangle coordinates\n> + */\n> +Rectangle Rectangle::croppedBy(const unsigned int numerator,\n> + const unsigned int denominator) const\n> +{\n> + const unsigned int w = numerator * width / denominator;\n> + const unsigned int h = numerator * height / denominator;\n\nIn Rectangle::scaledBy the numbers are cast to 64 bits to ensure that\noverflows don't happen. I suspect we should do the same here...\n\nOr use that call as a helper ?\n\n\n> + return Rectangle((width - w) / 2, (height - h) / 2, w, h);\n\nAlso scaledBy accounts for the x, y coordinates too. We can't always\nassume that a rectangle being cropped starts at 0,0!\n\nFor example the incoming Rectangle you're croppping might already have a\ncrop applied to represent the active pixels of a sensor array...\n\n> +}\n\nI wonder if implementing this as :\n\nRectangle Rectangle::croppedBy(const unsigned int numerator,\n const unsigned int denominator) const\n{\n\tSize cropped = this.size() * numerator / denominator);\n\treturn cropped.centeredTo(this.center());\n}\n\n\nwould solve the above concerns. Actually I don't think that would\nprotect against overflow ... but maybe it's not a big deal (yet?) So\nmaybe inlining the Size cropped still makes sense.\n\nWhich makes it more like:\n\nRectangle Rectangle::croppedBy(const unsigned int numerator,\n const unsigned int denominator) const\n{\n\tunsigned int w = static_cast(width) * numerator / denominator;\n\tunsigned int h = static_cast(height) * numerator / denominator;\n\n\treturn Size(w, h).centeredTo(center());\n\n}\n\n?\n\nPerhaps we should also add both:\n\n croppedBy() and cropBy()\n\nthough to have the version that modifies in place? \n\nFinally, any additions to geometry.cpp likely need associated tests in\ntest/geometry.cpp\n\n--\nKieran\n\n\n\n> +\n> /**\n> * \\brief Transform a Rectangle from one reference rectangle to another\n> * \\param[in] source The \\a source reference rectangle\n> -- \n> 2.50.1\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 8302DC3237\n\tfor ;\n\tMon, 21 Jul 2025 17:16:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D7DE68FB1;\n\tMon, 21 Jul 2025 19:16:49 +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 B25BF68FB1\n\tfor ;\n\tMon, 21 Jul 2025 19:16:47 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7DBB57939;\n\tMon, 21 Jul 2025 19:16:10 +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=\"UkCukJb2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753118170;\n\tbh=IyzFUKiubcDNZnjLjfh2hBctPQ1d83xl6bCRO1Z4UEI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=UkCukJb2PuRa8GAN7oMnjWgiyBYcNrd1736ORcEx5JeBWAz7S+muP/VxZ6yk5tMVH\n\tQp6Fcd5AUbGhAyRCXmt4/SCOEb7NnZcMKaB2dMSAxqXqe8UgCHSvrTjA8ZfYEXaKeQ\n\tJvUpluKje/FR8rywm03u6IdG5tXljt3MblLhK+lI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250718144456.58625-1-mzamazal@redhat.com>","References":"<20250718144456.58625-1-mzamazal@redhat.com>","Subject":"Re: [PATCH 1/2] libcamera: geometry: Add Rectangle::croppedBy","From":"Kieran Bingham ","Cc":"Milan Zamazal ,\n\tBryan O'Donoghue ","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Date":"Mon, 21 Jul 2025 18:16:44 +0100","Message-ID":"<175311820497.50296.7386672516877998226@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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\" "}},{"id":34991,"web_url":"https://patchwork.libcamera.org/comment/34991/","msgid":"<85o6tdbcb4.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-07-21T18:58:55","subject":"Re: [PATCH 1/2] libcamera: geometry: Add Rectangle::croppedBy","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Kieran,\n\nthank you for review.\n\nKieran Bingham writes:\n\n> Quoting Milan Zamazal (2025-07-18 15:44:55)\n>> Let's add a new Rectangle method that returns the coordinates of the\n>> central area of the original rectangle cropped by given\n>\n>> numerator/denominator fraction. This is useful to specify a limited\n>> area of the image to operate on, for example computing statistics only\n>> from a reduced area for speedup.\n>> \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> include/libcamera/geometry.h | 2 ++\n>> src/libcamera/geometry.cpp | 18 ++++++++++++++++++\n>> 2 files changed, 20 insertions(+)\n>> \n>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n>> index f322e3d5b..603c9a117 100644\n>> --- a/include/libcamera/geometry.h\n>> +++ b/include/libcamera/geometry.h\n>> @@ -297,6 +297,8 @@ public:\n>> [[nodiscard]] Rectangle scaledBy(const Size &numerator,\n>> const Size &denominator) const;\n>> [[nodiscard]] Rectangle translatedBy(const Point &point) const;\n>> + [[nodiscard]] Rectangle croppedBy(const unsigned int numerator,\n>> + const unsigned int denominator) const;\n>> \n>> Rectangle transformedBetween(const Rectangle &source,\n>> const Rectangle &target) const;\n>> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n>> index 81cc8cd53..8c1b156c5 100644\n>> --- a/src/libcamera/geometry.cpp\n>> +++ b/src/libcamera/geometry.cpp\n>> @@ -837,6 +837,24 @@ Rectangle Rectangle::translatedBy(const Point &point) const\n>> return { x + point.x, y + point.y, width, height };\n>> }\n>> \n>> +/**\n>> + * \\brief Return a central area crop of the rectangle\n>> + * \\param[in] numerator The numerator of the crop factor\n>> + * \\param[in] denominator The denominator of the crop factor\n>> + *\n>> + * The rectangle of the central part of the original rectangle is computed, of\n>> + * numerator/denominator times the original width and height.\n>> + *\n>> + * \\return The cropped Rectangle coordinates\n>> + */\n>> +Rectangle Rectangle::croppedBy(const unsigned int numerator,\n>> + const unsigned int denominator) const\n>> +{\n>> + const unsigned int w = numerator * width / denominator;\n>> + const unsigned int h = numerator * height / denominator;\n>\n> In Rectangle::scaledBy the numbers are cast to 64 bits to ensure that\n> overflows don't happen. I suspect we should do the same here...\n>\n> Or use that call as a helper ?\n>\n>\n>> + return Rectangle((width - w) / 2, (height - h) / 2, w, h);\n>\n> Also scaledBy accounts for the x, y coordinates too. We can't always\n> assume that a rectangle being cropped starts at 0,0!\n>\n> For example the incoming Rectangle you're croppping might already have a\n> crop applied to represent the active pixels of a sensor array...\n>\n>> +}\n>\n> I wonder if implementing this as :\n>\n> Rectangle Rectangle::croppedBy(const unsigned int numerator,\n> const unsigned int denominator) const\n> {\n> \tSize cropped = this.size() * numerator / denominator);\n> \treturn cropped.centeredTo(this.center());\n> }\n>\n>\n> would solve the above concerns. Actually I don't think that would\n> protect against overflow ... but maybe it's not a big deal (yet?) So\n> maybe inlining the Size cropped still makes sense.\n>\n> Which makes it more like:\n>\n> Rectangle Rectangle::croppedBy(const unsigned int numerator,\n> const unsigned int denominator) const\n> {\n> \tunsigned int w = static_cast(width) * numerator / denominator;\n> \tunsigned int h = static_cast(height) * numerator / denominator;\n>\n> \treturn Size(w, h).centeredTo(center());\n>\n> }\n>\n> ?\n\nThis version looks safe and right, I'll use it in v2, thank you for the\nsuggestion.\n\n> Perhaps we should also add both:\n>\n> croppedBy() and cropBy()\n>\n> though to have the version that modifies in place? \n\nWe don't have currently use for the in-place-modification version,\nshould it be added anyway?\n\n> Finally, any additions to geometry.cpp likely need associated tests in\n> test/geometry.cpp\n\nI'll add them.\n\n> --\n> Kieran\n>\n>\n>\n>> +\n>> /**\n>> * \\brief Transform a Rectangle from one reference rectangle to another\n>> * \\param[in] source The \\a source reference rectangle\n>> -- \n>> 2.50.1\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 34AE6BDCC1\n\tfor ;\n\tMon, 21 Jul 2025 18:59:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6917869008;\n\tMon, 21 Jul 2025 20:59:02 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 28F4168FB1\n\tfor ;\n\tMon, 21 Jul 2025 20:59:01 +0200 (CEST)","from mail-wr1-f71.google.com (mail-wr1-f71.google.com\n\t[209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-445-UzAWhkYJMBW_al1_ni9CQg-1; Mon, 21 Jul 2025 14:58:58 -0400","by mail-wr1-f71.google.com with SMTP id\n\tffacd0b85a97d-3a50816cc58so1654834f8f.3\n\tfor ;\n\tMon, 21 Jul 2025 11:58:58 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-3b61ca315basm11211424f8f.39.2025.07.21.11.58.55\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 21 Jul 2025 11:58:56 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"XwHSJ9FX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1753124340;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=vzyz/G838QWgbPcjyBmYiI5Nexo7KuBfetxFQMww+po=;\n\tb=XwHSJ9FXccDnHoz8+Mxf4R7jvbKlu908Km0Xis6hozNIXqqdMpheueh1p91uu9NMlpLEoe\n\t3WJ5CmMAeWtEpqoni4VhIsrhGM+02lJasJeF3DjFkS/o9stUFy3O49t0RnMnNMXsXNeMeK\n\tGog7dMl3do57lGIHRKXIXNwteGhYlNE=","X-MC-Unique":"UzAWhkYJMBW_al1_ni9CQg-1","X-Mimecast-MFC-AGG-ID":"UzAWhkYJMBW_al1_ni9CQg_1753124337","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1753124337; x=1753729137;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=vzyz/G838QWgbPcjyBmYiI5Nexo7KuBfetxFQMww+po=;\n\tb=S+XRop/IRGGXkMd1B5YWJCXohQePCW4Eh1z1WsGgn8gPAwsVi9Tr44qw9PuYkHsCS9\n\tQnBreQ5RKLKK19PZMduY2McVdfVHr8XNMuMP21DTPvYSAruF36Oan1t4TpIWWqPMvmZh\n\tj3ly3GEsVxLIsHyazHm79/D3au7MBaZWx0fdntrtdktAr7KhGCSseOsOsrM2UtWHAYWL\n\ty1RUCYUvCNUfpLt4R98D8gSXVpGNls27USu9+W52wp533ptEFdOM7vwe469hf9KKn1nZ\n\twdeQUVzoHMehaRsP/AGnLE4qihqyCeezpyNeFNu1ZhvSPro5YJps1YLxmCcW9GNQSY3/\n\tEIGA==","X-Gm-Message-State":"AOJu0YxVr4wC/uC3N9fV/8ymDkABAWiX5xxBRf7uKGEgsSbCnkW0WuAG\n\tceFZzK5VzxNfEh35l1v0+wTf60QWFDDGbWf1ncTQgsCuULrACTrwFdHVIq4J3OjgzJ0muLPni0d\n\t9/uJDPEQwgLKus+tzI4eQ8T8TdPrs3Og3C9l3V6GhjtT8Hcz2ug4oxAEI0mZuD2jFNt1a16WSXD\n\tA=","X-Gm-Gg":"ASbGncsI8TpnCJrSlFK+3J+rvXUQbMbTZRAwb6QqlouJYMPNJIVc3Pc5m92aeLR8+5W\n\tjt5QEpZ93+mRr2Th9/Q9BjxEjF8imkigfIRv5jxhtS8+yAEvLGcN+arOEj0hqMM6K1AoK78DXO8\n\tMMRPrJGJ4282Fq8V6Hpd99/OeYFeKRZPU7n+m6RpHsan6XY1t51sz2rJqCW4uk9t54Ch+C9R5I/\n\t/vFGVzNObWxFdsDIufNIaomxc+wxALVUvjpRUL1w6agZ26NdgC24ueKSH8ePgHKRP2Z9Qo/VS8S\n\tiZ/XwySYject88OVghqc0TrY45QcZ3Z4SFc6nuGB/yPQbbPDLtitgdNu6qfIBflcWyorLNzhppl\n\t+mJMh7IRpmm4z012f","X-Received":["by 2002:a05:6000:2c01:b0:3a4:fa6a:9189 with SMTP id\n\tffacd0b85a97d-3b613e9831cmr12410885f8f.31.1753124337100; \n\tMon, 21 Jul 2025 11:58:57 -0700 (PDT)","by 2002:a05:6000:2c01:b0:3a4:fa6a:9189 with SMTP id\n\tffacd0b85a97d-3b613e9831cmr12410871f8f.31.1753124336623; \n\tMon, 21 Jul 2025 11:58:56 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IF7Z8TRwCxHti83INiwv7rzew558Ne48QSUUKkcQfaL+HFs1uZIPz8Sc3IXgqCY4p+OgQfKqg==","From":"Milan Zamazal ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, Bryan O'Donoghue\n\t","Subject":"Re: [PATCH 1/2] libcamera: geometry: Add Rectangle::croppedBy","In-Reply-To":"<175311820497.50296.7386672516877998226@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Mon, 21 Jul 2025 18:16:44 +0100\")","References":"<20250718144456.58625-1-mzamazal@redhat.com>\n\t<175311820497.50296.7386672516877998226@ping.linuxembedded.co.uk>","Date":"Mon, 21 Jul 2025 20:58:55 +0200","Message-ID":"<85o6tdbcb4.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"lKhVkTl-iwajMR7euvASaFS0f4HsQO5Sys12JCjK85g_1753124337","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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\" "}},{"id":34997,"web_url":"https://patchwork.libcamera.org/comment/34997/","msgid":"<175312572936.50296.6171601308711792598@ping.linuxembedded.co.uk>","date":"2025-07-21T19:22:09","subject":"Re: [PATCH 1/2] libcamera: geometry: Add Rectangle::croppedBy","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2025-07-21 19:58:55)\n> Hi Kieran,\n> \n> thank you for review.\n> \n> Kieran Bingham writes:\n> \n> > Quoting Milan Zamazal (2025-07-18 15:44:55)\n> >> Let's add a new Rectangle method that returns the coordinates of the\n> >> central area of the original rectangle cropped by given\n> >\n> >> numerator/denominator fraction. This is useful to specify a limited\n> >> area of the image to operate on, for example computing statistics only\n> >> from a reduced area for speedup.\n> >> \n> >> Signed-off-by: Milan Zamazal \n> >> ---\n> >> include/libcamera/geometry.h | 2 ++\n> >> src/libcamera/geometry.cpp | 18 ++++++++++++++++++\n> >> 2 files changed, 20 insertions(+)\n> >> \n> >> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> >> index f322e3d5b..603c9a117 100644\n> >> --- a/include/libcamera/geometry.h\n> >> +++ b/include/libcamera/geometry.h\n> >> @@ -297,6 +297,8 @@ public:\n> >> [[nodiscard]] Rectangle scaledBy(const Size &numerator,\n> >> const Size &denominator) const;\n> >> [[nodiscard]] Rectangle translatedBy(const Point &point) const;\n> >> + [[nodiscard]] Rectangle croppedBy(const unsigned int numerator,\n> >> + const unsigned int denominator) const;\n> >> \n> >> Rectangle transformedBetween(const Rectangle &source,\n> >> const Rectangle &target) const;\n> >> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> >> index 81cc8cd53..8c1b156c5 100644\n> >> --- a/src/libcamera/geometry.cpp\n> >> +++ b/src/libcamera/geometry.cpp\n> >> @@ -837,6 +837,24 @@ Rectangle Rectangle::translatedBy(const Point &point) const\n> >> return { x + point.x, y + point.y, width, height };\n> >> }\n> >> \n> >> +/**\n> >> + * \\brief Return a central area crop of the rectangle\n> >> + * \\param[in] numerator The numerator of the crop factor\n> >> + * \\param[in] denominator The denominator of the crop factor\n> >> + *\n> >> + * The rectangle of the central part of the original rectangle is computed, of\n> >> + * numerator/denominator times the original width and height.\n> >> + *\n> >> + * \\return The cropped Rectangle coordinates\n> >> + */\n> >> +Rectangle Rectangle::croppedBy(const unsigned int numerator,\n> >> + const unsigned int denominator) const\n> >> +{\n> >> + const unsigned int w = numerator * width / denominator;\n> >> + const unsigned int h = numerator * height / denominator;\n> >\n> > In Rectangle::scaledBy the numbers are cast to 64 bits to ensure that\n> > overflows don't happen. I suspect we should do the same here...\n> >\n> > Or use that call as a helper ?\n> >\n> >\n> >> + return Rectangle((width - w) / 2, (height - h) / 2, w, h);\n> >\n> > Also scaledBy accounts for the x, y coordinates too. We can't always\n> > assume that a rectangle being cropped starts at 0,0!\n> >\n> > For example the incoming Rectangle you're croppping might already have a\n> > crop applied to represent the active pixels of a sensor array...\n> >\n> >> +}\n> >\n> > I wonder if implementing this as :\n> >\n> > Rectangle Rectangle::croppedBy(const unsigned int numerator,\n> > const unsigned int denominator) const\n> > {\n> > Size cropped = this.size() * numerator / denominator);\n> > return cropped.centeredTo(this.center());\n> > }\n> >\n> >\n> > would solve the above concerns. Actually I don't think that would\n> > protect against overflow ... but maybe it's not a big deal (yet?) So\n> > maybe inlining the Size cropped still makes sense.\n> >\n> > Which makes it more like:\n> >\n> > Rectangle Rectangle::croppedBy(const unsigned int numerator,\n> > const unsigned int denominator) const\n> > {\n> > unsigned int w = static_cast(width) * numerator / denominator;\n> > unsigned int h = static_cast(height) * numerator / denominator;\n> >\n> > return Size(w, h).centeredTo(center());\n> >\n> > }\n> >\n> > ?\n> \n> This version looks safe and right, I'll use it in v2, thank you for the\n> suggestion.\n> \n> > Perhaps we should also add both:\n> >\n> > croppedBy() and cropBy()\n> >\n> > though to have the version that modifies in place? \n> \n> We don't have currently use for the in-place-modification version,\n> should it be added anyway?\n\nIf they're not needed at the moment it's fine not to add them. And\nprobably simpler for now. It's just the symmetry with the\nscaledBy/scaleBy I was comparing against.\n\n> \n> > Finally, any additions to geometry.cpp likely need associated tests in\n> > test/geometry.cpp\n> \n> I'll add them.\n\nThanks.\n\nKieran\n\n> \n> > --\n> > Kieran\n> >\n> >\n> >\n> >> +\n> >> /**\n> >> * \\brief Transform a Rectangle from one reference rectangle to another\n> >> * \\param[in] source The \\a source reference rectangle\n> >> -- \n> >> 2.50.1\n> >>\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 904A0BDCC1\n\tfor ;\n\tMon, 21 Jul 2025 19:22:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9D82A69013;\n\tMon, 21 Jul 2025 21:22:13 +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 0983D68FB1\n\tfor ;\n\tMon, 21 Jul 2025 21:22:12 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DCCDF7950;\n\tMon, 21 Jul 2025 21:21:34 +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=\"KQVoNlqb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753125694;\n\tbh=bVfuQ9fzKzgaGQsxmYXAW6qmONOYw4estDANl6Hd4DE=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=KQVoNlqbaOkWBRSZhXYSxAxPGCLUzv7OAvn362cFbGpIcmNNYyao4PpvY5Mi4baVU\n\tWQeG+naR66PPeM1skvFL4sfgQAbGgeYHMKUmV3QV6iG+ZVl40I2AYNlJuXV35d4SEf\n\tYxTd6sPotD4IOzFJbF6sJKxeyjkeShdzBQKL1gmA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<85o6tdbcb4.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20250718144456.58625-1-mzamazal@redhat.com>\n\t<175311820497.50296.7386672516877998226@ping.linuxembedded.co.uk>\n\t<85o6tdbcb4.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Subject":"Re: [PATCH 1/2] libcamera: geometry: Add Rectangle::croppedBy","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tBryan O'Donoghue ","To":"Milan Zamazal ","Date":"Mon, 21 Jul 2025 20:22:09 +0100","Message-ID":"<175312572936.50296.6171601308711792598@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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\" "}}]