[v4,03/20] libcamera: geometry: Add Rectangle::transformedBetween()

22349 diff mbox series

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@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

---
Changes in v3:
- Improved documentation
- Added testcase

Changes in v2:
- Renamed mappedBetween() to transformedBetween()
- Improved documentation
- Collected tags
---
 include/libcamera/geometry.h |  3 +++
 src/libcamera/geometry.cpp   | 49 ++++++++++++++++++++++++++++++++++++
 test/geometry.cpp            | 11 ++++++++
 3 files changed, 63 insertions(+)

On Mon, Dec 16, 2024 at 04:40:43PM +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@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> ---
> Changes in v3:
> - Improved documentation
> - Added testcase
> 
> Changes in v2:
> - Renamed mappedBetween() to transformedBetween()
> - Improved documentation
> - Collected tags
> ---
>  include/libcamera/geometry.h |  3 +++
>  src/libcamera/geometry.cpp   | 49 ++++++++++++++++++++++++++++++++++++
>  test/geometry.cpp            | 11 ++++++++
>  3 files changed, 63 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..81cc8cd538a0 100644
> --- a/src/libcamera/geometry.cpp
> +++ b/src/libcamera/geometry.cpp
> @@ -837,6 +837,55 @@ Rectangle Rectangle::translatedBy(const Point &point) const
>  	return { x + point.x, y + point.y, width, height };
>  }
>  
> +/**
> + * \brief Transform a Rectangle from one reference rectangle to another
> + * \param[in] source The \a source reference rectangle
> + * \param[in] destination The \a destination reference rectangle
> + *
> + * The \a source and \a destination parameters describe two rectangles defined
> + * in different reference systems. The Rectangle is translated from the source

s/translated/transformed/

> + * reference system into the destination reference system.
> + *
> + * 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

s/in example/for example/ (or "for instance")

> + * the same rectangle re-scaled and translated into a different reference
> + * system, in example the output frame on which the selection rectangle is

Same here.

> + * applied to.

s/ to// (or s/on which/which/ in the previous line)

> + *
> + * For example, consider a sensor with a resolution of 4040x2360 pixels and a

I think there's a stray "a" at the end of the line.

> + * 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:
> + *
> + * \code{.cpp}
> + * Rectangle sensorReference(100, 100, 3840, 2160);
> + * Rectangle displayReference(0, 0, 1920, 1080);
> + *
> + * // Bottom right quarter in sensor coordinates
> + * Rectangle sensorRect(2020, 100, 1920, 1080);
> + * displayRect = sensorRect.transformedBetween(sensorReference, displayReference);
> + * // displayRect is now (960, 540)/960x540
> + *
> + * // Transformation back to sensor coordinates
> + * sensorRect = displayRect.transformedBetween(displayReference, sensorReference);
> + * \endcode

Missing \return

> + */
> +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);

width and height are unsigned, so static_cast<unsigned int> is more
appropriate.

Is rounding down instead of rounding to the closest value the best
option ?

> +
> +	return r;
> +}
> +

I'm wondering if the Rectangle API isn't getting a bit too complicated.
Something like https://doc.qt.io/qt-6/qtransform.html may be more
appropriate. No need to act on this now.

>  /**
>   * \brief Compare rectangles for equality
>   * \return True if the two rectangles are equal, false otherwise
> diff --git a/test/geometry.cpp b/test/geometry.cpp
> index 5760fa3c885a..11df043b733b 100644
> --- a/test/geometry.cpp
> +++ b/test/geometry.cpp
> @@ -495,6 +495,17 @@ protected:
>  			return TestFail;
>  		}
>  
> +		Rectangle f1 = Rectangle(100, 200, 3000, 2000);
> +		Rectangle f2 = Rectangle(200, 300, 1500, 1000);

You could have written

		Rectangle f1(100, 200, 3000, 2000);
		Rectangle f2(200, 300, 1500, 1000);

> +		/* Bottom right quarter of the corresponding frames. */
> +		Rectangle r1 = Rectangle(100 + 1500, 200 + 1000, 1500, 1000);
> +		Rectangle r2 = Rectangle(200 + 750, 300 + 500, 750, 500);

Same here. No big deal.

> +		if (r1.transformedBetween(f1, f2) != r2 ||
> +		    r2.transformedBetween(f2, f1) != r1) {
> +			cout << "Rectangle::transformedBetween() test failed" << endl;
> +			return TestFail;
> +		}
> +
>  		return TestPass;
>  	}
>  };