[PATCH 3/7] libcamera: geometry: Add Rectangle::mappedBetween()
Stefan Klug
stefan.klug at ideasonboard.com
Thu Nov 21 14:50:11 CET 2024
Hi Jacopo,
Thank you for the review.
On Thu, Nov 21, 2024 at 12:45:47PM +0100, Jacopo Mondi wrote:
> Hi Stefan
>
> On Wed, Nov 20, 2024 at 09:57:42AM +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).
>
> Let me try to clear my thoughts on this point, I've gone through
> re-scaling ScalerCrops too many times.
>
> Currently the way all pipline handlers do this is
>
> Rectangle ispCrop = nativeCrop.translatedBy(-sensor.topLeft());
> ispCrop.scaleBy(display.size(), sensor.size());
>
> with:
> nativeCrop = The crop rectangles expressed on the sensor
> native pixel array (controls::ScalerCrop)
> sensor = The sensor analog crop rectangle
> display = The destination reference system
>
> The assumption is that nativeCrop is always contained in the
> analogCrop, as per the definition of the ScalerCrop control and of
> ScalerCropMaximum.
>
> Good news is that
>
> Rectangle sensor = { 0, 243, 2592, 1458 };
> Rectangle display = { 0, 0, 1920, 1080 };
> Rectangle nativeCrop = { 400, 400, 640, 480 };
>
> Rectangle scaled = nativeCrop.mappedBetween(sensor, display);
>
> cout << "Scaled: " << scaled << endl;
>
> Rectangle ispCrop = nativeCrop.translatedBy(-sensor.topLeft());
> ispCrop.scaleBy(display.size(), sensor.size());
>
> cout << "ISP Crop: " << ispCrop << endl;
>
> Give the same result
>
> Scaled: (296, 116)/474x355
> ISP Crop: (296, 116)/474x355
>
> >
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> > include/libcamera/geometry.h | 2 ++
> > src/libcamera/geometry.cpp | 33 +++++++++++++++++++++++++++++++++
> > 2 files changed, 35 insertions(+)
> >
> > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> > index 9ca5865a3d0d..8c5f34f4dec1 100644
> > --- a/include/libcamera/geometry.h
> > +++ b/include/libcamera/geometry.h
> > @@ -299,6 +299,8 @@ public:
> > __nodiscard Rectangle scaledBy(const Size &numerator,
> > const Size &denominator) const;
> > __nodiscard Rectangle translatedBy(const Point &point) const;
> > +
> > + Rectangle mappedBetween(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..7e29fe35801e 100644
> > --- a/src/libcamera/geometry.cpp
> > +++ b/src/libcamera/geometry.cpp
> > @@ -837,6 +837,39 @@ Rectangle Rectangle::translatedBy(const Point &point) const
> > return { x + point.x, y + point.y, width, height };
> > }
> >
> > +/**
> > + * \brief Maps a Rectangle from one reference frame to another
> > + * \param[in] source The source reference frame
> > + * \param[in] destination The destination reference frame
> > + *
> > + * The params source and destinations specify two reference frames for the same
> > + * data. The rectangle is translated from the source reference frame into the
> > + * destination reference frame.
> > + *
> > + * For example, consider a sensor with a resolution of 2592x1458 pixels and a
> > + * assume a rectangle of (0, 243)/2592x1458 (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}
> > + * displayRect = sensorRect.mappedBetween(sensorFrame, displayFrame);
> > + * \endcode
> > + */
> > +Rectangle Rectangle::mappedBetween(const Rectangle &source,
> > + const Rectangle &destination) const
> > +{
>
> Looking at the code of the two examples above, the sequence
>
> Rectangle ispCrop = nativeCrop.translatedBy(-sensor.topLeft());
> ispCrop.scaleBy(display.size(), sensor.size());
>
> translates to
>
> r.x = x + destination.x;
> r.y = y + destination.y;
Was this meant to be source.x/y? As this stems from
translatedBy(-sensor.topLeft()).
>
> r.x = static_cast<int64_t>(r.x) * sx;
> r.y = static_cast<int64_t>(r.y) * sy;
> width = static_cast<uint64_t>(width) * sx;
> height = static_cast<uint64_t>(height) * sy;
>
> > + 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);
> > + return r;
>
> Which is equivalent to your code here if not for the '+ destination.x'
> and '+ destination.y' in:
>
> r.x = static_cast<int>((x - source.x) * sx) + destination.x;
> r.y = static_cast<int>((y - source.y) * sy) + destination.y;
>
> Now, Rectangle::scaleBy() takes two sizes instead of rectangles, so it
> assumes that the top-left point of the destination is not relevant.
> Is it in the case you want to support ?
The + destination.x/y makes the function generic and allows to map in
both directions. So to continue from your example above, you can do:
Rectangle scaled = nativeCrop.mappedBetween(sensor, display);
Rectangle native = scaled.mappedBetween(display, sensor);
This is helpful when mapping back the scaledCrop into sensor coordinates
for the metadata calculation. For my brain it is easier to think in
reference frames/coordinate systems than to manually get the sequence of
transformations in the right order. I also thought about representing
that as a matrix transformation. That felt like an overkill, but might
come in handy when our processing chains get longer.
>
> Also, can
>
> r.x = static_cast<int>((x - source.x) * sx) + destination.x;
> r.y = static_cast<int>((y - source.y) * sy) + destination.y;
>
> result in any negative value ?
Yes that is possible for badly defined rectangles. But as x and y are
signed ints I don't see an issue here.
Best regards,
Stefan
>
>
> > +}
> > +
> > /**
> > * \brief Compare rectangles for equality
> > * \return True if the two rectangles are equal, false otherwise
> > --
> > 2.43.0
> >
More information about the libcamera-devel
mailing list