[PATCH] libcamera: geometry: Rename Rectangle's top-left corner
Umang Jain
umang.jain at ideasonboard.com
Thu Sep 26 10:45:01 CEST 2024
Hi Jacopo
On 26/09/24 1:25 pm, Jacopo Mondi wrote:
> Hi Umang
>
> On Wed, Sep 25, 2024 at 12:06:05PM GMT, Umang Jain wrote:
>> Hi Jacopo
>>
>> Thank you for the patch
>>
>> On 24/09/24 6:15 pm, Jacopo Mondi wrote:
>>> The libcamera::Rectangle class should allow defining rectangles
>>> regardless of the reference system where an instance is used in.
>>>
>>> However the class documentation assumes the reference system origin
>>> point to be in the visual top-left side and that rectangles are
>>> constructed by specifying the top-left corner position and sizes.
>>>
>>> However this is ambiguous when using a different reference system as,
>>> in example, constructing a rectangle (x, y, w, h) in the following
>>> scenario, with the origin corner in the top-left position, would require
>>> a negative decrement along the y-axis.
>>>
>>> ^
>>> |
>>> y| o------------------
>>> | | | h
>>> | | |
>>> | -------------------
>>> | w
>>> ------------------------------->
>>> (0,0) x
>>>
>>> To remove ambiguities, drop references to the visual top-left corner and
>>> define the rectangle's origin corner as the corner closer to the
>>> reference system origin point, and specify the horizontal and vertical
>>> dimensions always increment along their respective axis, as in the
>>> following example.
>>>
>>> ^
>>> |
>>> | -------------------
>>> | ^ | h
>>> | | |
>>> y| o---->-------------
>>> | w
>>> ------------------------------->
>>> (0,0) x
>>>
>>> (0,0) x
>>> ------------------------------>
>>> | w
>>> y| o---->-------------
>>> | | | h
>>> | v |
>>> | -------------------
>>> |
>>> V
>>>
>>> Also rename for consistency the Rectangle::topLeft() function to
>>> Rectangle::origin() and update its in-tree users.
>>>
>>> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>>> ---
>>> include/libcamera/geometry.h | 2 +-
>>> src/ipa/ipu3/algorithms/af.cpp | 2 +-
>>> src/libcamera/geometry.cpp | 85 ++++++++++++++-----
>>> .../pipeline/rpi/common/pipeline_base.cpp | 4 +-
>>> src/py/libcamera/py_geometry.cpp | 2 +-
>>> test/geometry.cpp | 8 +-
>>> 6 files changed, 75 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
>>> index 3e6f0f5d7fab..07cb0a7866fe 100644
>>> --- a/include/libcamera/geometry.h
>>> +++ b/include/libcamera/geometry.h
>>> @@ -277,7 +277,7 @@ public:
>>> return { width, height };
>>> }
>>>
>>> - Point topLeft() const
>>> + Point origin() const
>>> {
>>> return { x, y };
>>> }
>>> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
>>> index cf68fb59c49b..1b707e9dd9ab 100644
>>> --- a/src/ipa/ipu3/algorithms/af.cpp
>>> +++ b/src/ipa/ipu3/algorithms/af.cpp
>>> @@ -155,7 +155,7 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>>> * - Return the AF ROI as metadata in the Request
>>> */
>>> Rectangle roi = gridSize.centeredTo(bds.center());
>>> - Point start = roi.topLeft();
>>> + Point start = roi.origin();
>>>
>>> /* x_start and y_start should be even */
>>> grid.x_start = utils::alignDown(start.x, 2);
>>> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
>>> index 000151364c7f..45ba81a99920 100644
>>> --- a/src/libcamera/geometry.cpp
>>> +++ b/src/libcamera/geometry.cpp
>>> @@ -593,11 +593,39 @@ std::ostream &operator<<(std::ostream &out, const SizeRange &sr)
>>> * \brief Describe a rectangle's position and dimensions
>>> *
>>> * Rectangles are used to identify an area of an image. They are specified by
>>> - * the coordinates of top-left corner and their horizontal and vertical size.
>>> + * the coordinates of the corner closer to the reference system's origin point
>>> + * and by horizontal and vertical increments from there.
>>> *
>>> - * The measure unit of the rectangle coordinates and size, as well as the
>>> - * reference point from which the Rectangle::x and Rectangle::y displacements
>>> - * refers to, are defined by the context were rectangle is used.
>>> + * The corner closer to the reference system's origin point is the rectangle's
>>> + * origin corner.
>>> + *
>>> + * \verbatim
>>> +
>>> + o = origin corner
>>> +
>>> + ^
>>> + |
>>> + | -------------------
>>> + | ^ |
>>> + | | |
>>> + | o---->-------------
>>> + |
>>> + ------------------------------->
>>> + (0,0)
>>> +
>>> + (0,0)
>>> + ------------------------------->
>>> + |
>>> + | o---->-------------
>>> + | | |
>>> + | v |
>>> + | -------------------
>>> + |
>>> + V
>>> + \endverbatim
>>> + *
>>> + * The measure unit of the rectangle coordinates and size are defined by the
>>> + * context were the rectangle is used.
>>> */
>>>
>>> /**
>>> @@ -608,35 +636,50 @@ std::ostream &operator<<(std::ostream &out, const SizeRange &sr)
>>> /**
>>> * \fn Rectangle::Rectangle(int x, int y, const Size &size)
>>> * \brief Construct a Rectangle with the given position and size
>>> - * \param[in] x The horizontal coordinate of the top-left corner
>>> - * \param[in] y The vertical coordinate of the top-left corner
>>> + * \param[in] x The horizontal coordinate of the origin corner
>>> + * \param[in] y The vertical coordinate of the origin corner
>>> * \param[in] size The size
>>> + *
>>> + * The rectangle's origin corner is the corner closer to the reference system
>>> + * origin point (0, 0).
>>> */
>>>
>>> /**
>>> * \fn Rectangle::Rectangle(int x, int y, unsigned int width, unsigned int height)
>>> * \brief Construct a Rectangle with the given position and size
>>> - * \param[in] x The horizontal coordinate of the top-left corner
>>> - * \param[in] y The vertical coordinate of the top-left corner
>>> + * \param[in] x The horizontal coordinate of the origin corner
>>> + * \param[in] y The vertical coordinate of the origin corner
>>> * \param[in] width The width
>>> * \param[in] height The height
>>> + *
>>> + * The rectangle's origin corner is the corner closer to the reference system
>>> + * origin point (0, 0).
>>> */
>>>
>>> /**
>>> * \fn Rectangle::Rectangle(const Size &size)
>>> - * \brief Construct a Rectangle of \a size with its top left corner located
>>> + * \brief Construct a Rectangle of \a size with its origin corner located
>>> * at (0,0)
>>> * \param[in] size The desired Rectangle size
>>> + *
>>> + * The rectangle's origin corner is the corner closer to the reference system
>>> + * origin point (0, 0).
>>> */
>>>
>>> /**
>>> * \var Rectangle::x
>>> - * \brief The horizontal coordinate of the rectangle's top-left corner
>>> + * \brief The horizontal coordinate of the rectangle's origin corner
>>> + *
>>> + * The rectangle's origin corner is the corner closer to the reference system
>>> + * origin point (0, 0).
>>> */
>>>
>>> /**
>>> * \var Rectangle::y
>>> - * \brief The vertical coordinate of the rectangle's top-left corner
>>> + * \brief The vertical coordinate of the rectangle's origin corner
>>> + *
>>> + * The rectangle's origin corner is the corner closer to the reference system
>>> + * origin point (0, 0).
>>> */
>>>
>>> /**
>>> @@ -683,9 +726,13 @@ Point Rectangle::center() const
>>> */
>>>
>>> /**
>>> - * \fn Point Rectangle::topLeft() const
>>> - * \brief Retrieve the coordinates of the top left corner of this Rectangle
>>> - * \return The Rectangle's top left corner
>>> + * \fn Point Rectangle::origin() const
>>> + * \brief Retrieve the coordinates of the origin corner of this Rectangle
>>> + *
>>> + * The rectangle's origin corner is the corner closer to the reference system
>>> + * origin point (0, 0).
>>> + *
>>> + * \return The Rectangle's origin corner
>> I think you can drop "corner" from "origin corner"
>>
>> Probably throughout the patch:
>> s/origin corner/origin/
>>
> I considered that, but as a "rectangle origin" is not a well-defined
> term in geometry I prefer to specify it's a corner. Is this ok ?
Maybe "rectangle origin vertex" ?
Just a suggestion. Keep what you think is best!
>
>> Otherwise, LGTM
>>
>> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> Thanks
> j
>
>>> */
>>>
>>> /**
>>> @@ -740,15 +787,15 @@ Rectangle &Rectangle::translateBy(const Point &point)
>>> */
>>> Rectangle Rectangle::boundedTo(const Rectangle &bound) const
>>> {
>>> - int topLeftX = std::max(x, bound.x);
>>> - int topLeftY = std::max(y, bound.y);
>>> + int originX = std::max(x, bound.x);
>>> + int originY = std::max(y, bound.y);
>>> int bottomRightX = std::min<int>(x + width, bound.x + bound.width);
>>> int bottomRightY = std::min<int>(y + height, bound.y + bound.height);
>>>
>>> - unsigned int newWidth = std::max(bottomRightX - topLeftX, 0);
>>> - unsigned int newHeight = std::max(bottomRightY - topLeftY, 0);
>>> + unsigned int newWidth = std::max(bottomRightX - originX, 0);
>>> + unsigned int newHeight = std::max(bottomRightY - originY, 0);
>>>
>>> - return { topLeftX, topLeftY, newWidth, newHeight };
>>> + return { originX, originY, newWidth, newHeight };
>>> }
>>>
>>> /**
>>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>>> index 3041fd1ed9fd..8d2440026689 100644
>>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>>> @@ -1289,7 +1289,7 @@ Rectangle CameraData::scaleIspCrop(const Rectangle &ispCrop) const
>>> */
>>> Rectangle nativeCrop = ispCrop.scaledBy(sensorInfo_.analogCrop.size(),
>>> sensorInfo_.outputSize);
>>> - nativeCrop.translateBy(sensorInfo_.analogCrop.topLeft());
>>> + nativeCrop.translateBy(sensorInfo_.analogCrop.origin());
>>> return nativeCrop;
>>> }
>>>
>>> @@ -1303,7 +1303,7 @@ void CameraData::applyScalerCrop(const ControlList &controls)
>>> nativeCrop = { 0, 0, 1, 1 };
>>>
>>> /* Create a version of the crop scaled to ISP (camera mode) pixels. */
>>> - Rectangle ispCrop = nativeCrop.translatedBy(-sensorInfo_.analogCrop.topLeft());
>>> + Rectangle ispCrop = nativeCrop.translatedBy(-sensorInfo_.analogCrop.origin());
>>> ispCrop.scaleBy(sensorInfo_.outputSize, sensorInfo_.analogCrop.size());
>>>
>>> /*
>>> diff --git a/src/py/libcamera/py_geometry.cpp b/src/py/libcamera/py_geometry.cpp
>>> index c7e303609427..8dee365b25dd 100644
>>> --- a/src/py/libcamera/py_geometry.cpp
>>> +++ b/src/py/libcamera/py_geometry.cpp
>>> @@ -105,7 +105,7 @@ void init_py_geometry(py::module &m)
>>> .def_property_readonly("is_null", &Rectangle::isNull)
>>> .def_property_readonly("center", &Rectangle::center)
>>> .def_property_readonly("size", &Rectangle::size)
>>> - .def_property_readonly("topLeft", &Rectangle::topLeft)
>>> + .def_property_readonly("origin", &Rectangle::origin)
>>> .def("scale_by", &Rectangle::scaleBy)
>>> .def("translate_by", &Rectangle::translateBy)
>>> .def("bounded_to", &Rectangle::boundedTo)
>>> diff --git a/test/geometry.cpp b/test/geometry.cpp
>>> index 64169206ad16..a4ee6f6e167a 100644
>>> --- a/test/geometry.cpp
>>> +++ b/test/geometry.cpp
>>> @@ -363,16 +363,16 @@ protected:
>>> return TestFail;
>>> }
>>>
>>> - /* Rectangle::size(), Rectangle::topLeft() and Rectangle::center() tests */
>>> + /* Rectangle::size(), Rectangle::origin() and Rectangle::center() tests */
>>> if (Rectangle(-1, -2, 3, 4).size() != Size(3, 4) ||
>>> Rectangle(0, 0, 100000, 200000).size() != Size(100000, 200000)) {
>>> cout << "Rectangle::size() test failed" << endl;
>>> return TestFail;
>>> }
>>>
>>> - if (Rectangle(1, 2, 3, 4).topLeft() != Point(1, 2) ||
>>> - Rectangle(-1, -2, 3, 4).topLeft() != Point(-1, -2)) {
>>> - cout << "Rectangle::topLeft() test failed" << endl;
>>> + if (Rectangle(1, 2, 3, 4).origin() != Point(1, 2) ||
>>> + Rectangle(-1, -2, 3, 4).origin() != Point(-1, -2)) {
>>> + cout << "Rectangle::origin() test failed" << endl;
>>> return TestFail;
>>> }
>>>
>>> --
>>> 2.46.1
>>>
More information about the libcamera-devel
mailing list