[{"id":12654,"web_url":"https://patchwork.libcamera.org/comment/12654/","msgid":"<20200923091554.jrzypoigkzfq6mb3@uno.localdomain>","date":"2020-09-23T09:15:54","subject":"Re: [libcamera-devel] [PATCH 4/6] libcamera: Add geometry helper\n\tfunctions","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n\nOn Tue, Sep 22, 2020 at 11:03:58AM +0100, David Plowman wrote:\n> These functions are aimed at making it easier to calculate cropping\n> rectangles, particularly in order to implement digital zoom.\n>\n> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  include/libcamera/geometry.h |  20 ++++++\n>  src/libcamera/geometry.cpp   | 131 +++++++++++++++++++++++++++++++++++\n>  2 files changed, 151 insertions(+)\n>\n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index 02fb63c..0447ee3 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -13,6 +13,8 @@\n>\n>  namespace libcamera {\n>\n> +class Rectangle;\n> +\n>  class Size\n>  {\n>  public:\n> @@ -93,8 +95,17 @@ public:\n>  \t\t\tstd::max(height, expand.height)\n>  \t\t};\n>  \t}\n> +\n> +\tSize alignedDownToAspectRatio(const Size &ratio) const;\n> +\tSize alignedUpToAspectRatio(const Size &ratio) const;\n> +\n> +\tRectangle centredTo(const Rectangle &region,\n> +\t\t\t    int offsetX = 0, int offsetY = 0) const;\n>  };\n>\n> +Size operator*(const Size &size, float f);\n> +Size operator/(const Size &size, float f);\n> +\n>  bool operator==(const Size &lhs, const Size &rhs);\n>  bool operator<(const Size &lhs, const Size &rhs);\n>\n> @@ -176,6 +187,11 @@ public:\n>  \t{\n>  \t}\n>\n> +\tconstexpr explicit Rectangle(const Size &size)\n> +\t\t: x(0), y(0), width(size.width), height(size.height)\n> +\t{\n> +\t}\n> +\n>  \tint x;\n>  \tint y;\n>  \tunsigned int width;\n> @@ -183,6 +199,10 @@ public:\n>\n>  \tbool isNull() const { return !width && !height; }\n>  \tconst std::string toString() const;\n> +\n> +\tSize size() const;\n> +\n> +\tRectangle boundedTo(const Rectangle &boundary) 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 b12e1a6..0c058dd 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -143,6 +143,90 @@ const std::string Size::toString() const\n>   * height of this size and the \\a expand size\n>   */\n>\n> +/**\n> + * \\brief Align down to the aspect ratio given by \\a ratio\n> + * \\param[in] ratio The size whose aspect ratio to align down to\n> + * \\return A Size whose width and height are equal to the width and height\n> + * of this Size aligned down to the aspect ratio of \\a ratio\n> + */\n> +Size Size::alignedDownToAspectRatio(const Size &ratio) const\n> +{\n> +\tSize result;\n\nresult is not used, doesn't the compiler complain ?\n\n> +\n> +\tuint64_t ratio1 = static_cast<uint64_t>(width) *\n> +\t\t\t  static_cast<uint64_t>(ratio.height);\n> +\tuint64_t ratio2 = static_cast<uint64_t>(ratio.width) *\n> +\t\t\t  static_cast<uint64_t>(height);\n> +\n> +\tif (ratio1 > ratio2)\n> +\t\treturn { static_cast<unsigned int>(ratio2 / ratio.height), height };\n> +\telse\n> +\t\treturn { width, static_cast<unsigned int>(ratio1 / ratio.width) };\n> +}\n> +\n> +/**\n> + * \\brief Align up to the aspect ratio given by \\a ratio\n> + * \\param[in] ratio The size whose aspect ratio to align up to\n> + * \\return A Size whose width and height are equal to the width and height\n> + * of this Size aligned up to the aspect ratio of \\a ratio\n> + */\n> +Size Size::alignedUpToAspectRatio(const Size &ratio) const\n> +{\n> +\tSize result;\n> +\n> +\tuint64_t ratio1 = static_cast<uint64_t>(width) *\n> +\t\t\t  static_cast<uint64_t>(ratio.height);\n> +\tuint64_t ratio2 = static_cast<uint64_t>(ratio.width) *\n> +\t\t\t  static_cast<uint64_t>(height);\n> +\n> +\tif (ratio1 < ratio2)\n> +\t\treturn { static_cast<unsigned int>(ratio2 / ratio.height), height };\n> +\telse\n> +\t\treturn { width, static_cast<unsigned int>(ratio1 / ratio.width) };\n> +}\n> +\n> +/**\n> + * \\brief Centre a rectangle of this size within another rectangular region,\n> + * with optional offsets\n> + * \\param[in] region The rectangular region relative to which the returned\n> + * rectangle can be position\n> + * \\param[in] offsetX The X offset of the mid-point of the returned rectangle\n> + * relative to the mid-point of the region\n> + * \\param[in] offsetY The Y offset of the mid-point of the returned rectangle\n> + * relative to the mid-point of the region\n> + *\n> + * A Rectangle of this object's size is positioned within the Rectangle\n> + * given by \\a region. It is positioned so that its mid-point coincides\n> + * with the mid-point of \\a region, and is then further offset by the\n> + * values \\a offsetX and \\a offsetY.\n> + *\n> + * \\return A Rectangle with the horizontal and vertical sizes of\n> + * this Size instance, centred with offsets within a region\n> + */\n> +Rectangle Size::centredTo(const Rectangle &region, int offsetX, int offsetY) const\n> +{\n> +\tint x = (region.width - width) / 2 + region.x + offsetX;\n> +\tint y = (region.height - height) / 2 + region.y + offsetY;\n> +\n> +\treturn Rectangle(x, y, width, height);\n> +}\n> +\n> +/**\n> + * \\brief Scale size up by the given factor\n\nDoesn't doxygen complain for a missing \\return statement ?\n\n> + */\n> +Size operator*(const Size &size, float f)\n> +{\n> +\treturn Size(size.width * f, size.height * f);\n> +}\n> +\n> +/**\n> + * \\brief Scale size down by the given factor\n\nHere too\n\n> + */\n> +Size operator/(const Size &size, float f)\n> +{\n> +\treturn Size(size.width / f, size.height / f);\n> +}\n> +\n>  /**\n>   * \\brief Compare sizes for equality\n>   * \\return True if the two sizes are equal, false otherwise\n> @@ -365,6 +449,12 @@ bool operator==(const SizeRange &lhs, const SizeRange &rhs)\n>   * \\param[in] height The height\n>   */\n>\n> +/**\n> + * \\fn Rectangle::Rectangle(const Size &size)\n> + * \\brief Construct a Rectangle with zero offsets and the given size\n\nnit: \"the given \\a size\"\n\n> + * \\param[in] size The desired Rectangle size\n> + */\n> +\n>  /**\n>   * \\var Rectangle::x\n>   * \\brief The horizontal coordinate of the rectangle's top-left corner\n> @@ -404,6 +494,47 @@ const std::string Rectangle::toString() const\n>  \treturn ss.str();\n>  }\n>\n> +/**\n> + * \\brief Retrieve the size of this rectangle\n> + * \\return A Size reporting the Rectangle horizontal and vertical sizes\n> + */\n> +Size Rectangle::size() const\n> +{\n> +\treturn Size(width, height);\n> +}\n> +\n> +/**\n> + * \\brief Bound a Rectangle so as not to exceeed another Rectangle\n> + * \\param[in] boundary The limit that the returned Rectangle will not exceed\n> + *\n> + * The Rectangle is bounded so that it does not exceeed the given \\a boundary.\n> + * This process involves translating the Rectangle if any of its edges\n> + * lie beyond \\a boundary, so that those edges then lie along the boundary\n> + * instead.\n> + *\n> + * If either width or height are larger than \\a bounary, then the returned\n> + * Rectangle is clipped to be no larger. But other than this, the\n> + * Rectangle is not clipped or reduced in size, merely translated.\n> + *\n> + * We note that this is not a conventional Rectangle intersection function.\n> + *\n> + * \\return A Rectangle that does not extend beyond a boundary Rectangle\n> + */\n> +Rectangle Rectangle::boundedTo(const Rectangle &boundary) const\n> +{\n> +\tRectangle result(*this);\n> +\n> +\tresult.width = std::min(result.width, boundary.width);\n> +\tresult.x = std::clamp<int>(result.x, boundary.x,\n> +\t\t\t\t   boundary.x + boundary.width - result.width);\n> +\n> +\tresult.height = std::min(result.height, boundary.height);\n> +\tresult.y = std::clamp<int>(result.y, boundary.y,\n> +\t\t\t\t   boundary.y + boundary.height - result.height);\n> +\n> +\treturn result;\n> +}\n> +\n\nnit aparts, very nice!\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\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.20.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 09E35C3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Sep 2020 09:12:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5C94262FDC;\n\tWed, 23 Sep 2020 11:12:03 +0200 (CEST)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 25E3060363\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Sep 2020 11:12:02 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 8C84EC000A;\n\tWed, 23 Sep 2020 09:12:01 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 23 Sep 2020 11:15:54 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<20200923091554.jrzypoigkzfq6mb3@uno.localdomain>","References":"<20200922100400.30766-1-david.plowman@raspberrypi.com>\n\t<20200922100400.30766-5-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200922100400.30766-5-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 4/6] libcamera: Add geometry helper\n\tfunctions","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]