[PATCH v2 1/3] libcamera: Add rectangle two-point constructor

Cheng-Hao Yang chenghaoyang at chromium.org
Fri Aug 30 23:04:21 CEST 2024


Thanks Jacopo,

On Wed, Aug 28, 2024 at 2:10 PM Jacopo Mondi <jacopo.mondi at ideasonboard.com>
wrote:

> Hi
>
> On Fri, Aug 23, 2024 at 02:29:08PM GMT, Harvey Yang wrote:
> > From: Yudhistira Erlandinata <yerlandinata at chromium.org>
> >
> > Add a Rectangle constructor that accepts two points:
> > topLeft and bottomRight.
> >
> > BUG=b:308714092
> > TEST=emerge-geralt libcamera-mtkisp7
>
> Please drop them, not related to libcamera
>
Removed.


>
> >
> > Signed-off-by: Yudhistira Erlandinata <yerlandinata at chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> > ---
> >  include/libcamera/geometry.h | 10 ++++++++++
> >  src/libcamera/geometry.cpp   | 11 +++++++++--
> >  2 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> > index 3e6f0f5d7..978322a22 100644
> > --- a/include/libcamera/geometry.h
> > +++ b/include/libcamera/geometry.h
> > @@ -8,7 +8,9 @@
> >  #pragma once
> >
> >  #include <algorithm>
> > +#include <assert.h>
> >  #include <ostream>
> > +#include <stdlib.h>
>
> What is this for ?
>
Removed.

>
> >  #include <string>
> >
> >  #include <libcamera/base/compiler.h>
> > @@ -262,6 +264,14 @@ public:
> >       {
> >       }
> >
> > +     constexpr Rectangle(const Point &topLeft, const Point &bottomRight)
> > +             : x(topLeft.x), y(topLeft.y),
> > +               width(bottomRight.x - x),
> > +               height(bottomRight.y - y)
> > +     {
> > +             assert(bottomRight.x >= x && bottomRight.y >= y);
>
> assert() is fine, however we have our own ASSERT() defined in log.h
> which can be conditionally disabled. The rest of the codebase uses
> that.
>
Right, thanks! Done.

>
> > +     }
> > +
> >       int x;
> >       int y;
> >       unsigned int width;
> > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> > index 000151364..86385ad35 100644
> > --- a/src/libcamera/geometry.cpp
> > +++ b/src/libcamera/geometry.cpp
> > @@ -5,13 +5,13 @@
> >   * Geometry-related structures
> >   */
> >
> > -#include <libcamera/geometry.h>
> > -
> >  #include <sstream>
> >  #include <stdint.h>
> >
> >  #include <libcamera/base/log.h>
> >
> > +#include <libcamera/geometry.h>
> > +
>
> why ?
>
My linter went crazy... Reverted.


>
> >  /**
> >   * \file geometry.h
> >   * \brief Data structures related to geometric objects
> > @@ -629,6 +629,13 @@ std::ostream &operator<<(std::ostream &out, const
> SizeRange &sr)
> >   * \param[in] size The desired Rectangle size
> >   */
> >
> > +/**
> > + * \fn Rectangle::Rectangle(const Point &topLeft, const Point
> &bottomRight)
> > + * \brief Construct a Rectangle with the two given points
> > + * \param[in] topLeft The top-left corner
> > + * \param[in] bottomRight The bottom-right corner
> > + */
> > +
>
> the rest looks good
>
> >  /**
> >   * \var Rectangle::x
> >   * \brief The horizontal coordinate of the rectangle's top-left corner
> > --
> > 2.46.0.295.g3b9ea8a38a-goog
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20240830/371511b8/attachment.htm>


More information about the libcamera-devel mailing list