[PATCH v2 3/8] libcamera: geometry: Add Rectangle::transformedBetween()

[Jacopo Mondi]

Hi Stefan

On Mon, Nov 25, 2024 at 04:14:12PM +0100, Stefan Klug wrote:
> Handling cropping and scaling within a complicated pipeline involves
> transformations of rectangles between different coordinate systems. For
> example the full input of the dewarper (0,0)/1920x1080 might correspond
> to the rectangle (0, 243)/2592x1458 in sensor coordinates (of a
> 2592x1944 sensor). Add a function that allows the transformation of a
> rectangle defined in one reference frame (dewarper) into the coordinates
> of a second reference frame (sensor).
>
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v2:
> - Renamed mappedBetween() to transformedBetween()
> - Improved documentation
> - Collected tags
> ---
>  include/libcamera/geometry.h |  3 +++
>  src/libcamera/geometry.cpp   | 37 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
>
> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> index 9ca5865a3d0d..e5f0a843d314 100644
> --- a/include/libcamera/geometry.h
> +++ b/include/libcamera/geometry.h
> @@ -299,6 +299,9 @@ public:
>  	__nodiscard Rectangle scaledBy(const Size &numerator,
>  				       const Size &denominator) const;
>  	__nodiscard Rectangle translatedBy(const Point &point) const;
> +
> +	Rectangle transformedBetween(const Rectangle &source,
> +				     const Rectangle &target) const;
>  };
>
>  bool operator==(const Rectangle &lhs, const Rectangle &rhs);
> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> index 90ccf8c19f97..2c9308cb25ee 100644
> --- a/src/libcamera/geometry.cpp
> +++ b/src/libcamera/geometry.cpp
> @@ -837,6 +837,43 @@ Rectangle Rectangle::translatedBy(const Point &point) const
>  	return { x + point.x, y + point.y, width, height };
>  }
>
> +/**
> + * \brief Transforms a Rectangle from one reference frame to another

nit: documentation is usually in imperative form:
Transforms -> Transform

> + * \param[in] source The \a source reference frame
> + * \param[in] destination The \a destination reference frame

I'm not sure I 100% like 'frame' here. The geometry library is not
only about frames or images...


> + *
> + * The params source and destinations specify two reference frames for the same

Let's spell out terms in full in documentation.

"The \a source and \a destination parameters "

Also, I would find it cleaner if we talk about Rectangles here.

"The \a source and \a destination parameters describe two rectangles
defined in different reference systems. The Rectangle is translated
from the source reference system into the destination reference
system."


> + * data. The rectangle is translated from the source reference frame into the
> + * destination reference frame.
> + *

We can say

"The typical use case for this function is to translate a selection
rectangle specified in a reference system, in example the sensor's
pixel array, into the same rectangle re-scaled and translated into
a different reference system, in example the output frame on
which the selection rectangle is applied to."

> + * For example, consider a sensor with a resolution of 4040x2360 pixels and a
> + * assume a rectangle of (100, 100)/3840x2160 (sensorFrame) in sensor
> + * coordinates is mapped to a rectangle (0,0)/(1920,1080) (displayFrame) in
> + * display coordinates. This function can be used to transform an arbitrary
> + * rectangle from display coordinates to sensor coordinates or vice versa:

And then provide a concrete example ? What do you think ? is is too
much details ?

> + *
> + * \code{.cpp}
> + * // Bottom right quarter in sensor coordinates
> + * Rectangle sensorRect(2020, 100, 1920, 1080);
> + * displayRect = sensorRect.transformedBetween(sensorFrame, displayFrame);
> + * // displayRect is now (960, 540)/960x540
> + * sensorRect = displayRect.transformedBetween(displayFrame, sensorFrame);
> + * \endcode
> + */
> +Rectangle Rectangle::transformedBetween(const Rectangle &source,
> +					const Rectangle &destination) const
> +{
> +	Rectangle r;
> +	double sx = static_cast<double>(destination.width) / source.width;
> +	double sy = static_cast<double>(destination.height) / source.height;
> +
> +	r.x = static_cast<int>((x - source.x) * sx) + destination.x;
> +	r.y = static_cast<int>((y - source.y) * sy) + destination.y;
> +	r.width = static_cast<int>(width * sx);
> +	r.height = static_cast<int>(height * sy);

nit: I would add a blank line here

> +	return r;
> +}
> +

nits and nitpicking apart
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j

>  /**
>   * \brief Compare rectangles for equality
>   * \return True if the two rectangles are equal, false otherwise
> --
> 2.43.0
>