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

[Jacopo Mondi]

Ups, I think you'll need a test for this though, even a simple one
that tests the example you have made in the documentation..

On Mon, Nov 25, 2024 at 07:20:46PM +0100, Jacopo Mondi wrote:
> 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
> >