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

[Jacopo Mondi]

Hi Barnabás

On Wed, Sep 04, 2024 at 11:26:37AM GMT, Barnabás Pőcze wrote:
> Hi
>
>
> 2024. szeptember 3., kedd 13:39 keltezéssel, Harvey Yang <chenghaoyang at chromium.org> írta:
>
> > From: Yudhistira Erlandinata <yerlandinata at chromium.org>
> >
> > Add a Rectangle constructor that accepts two points:
> > topLeft and bottomRight.
> >
> > Signed-off-by: Yudhistira Erlandinata <yerlandinata at chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> > Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> > ---
> >  include/libcamera/geometry.h |  2 ++
> >  src/libcamera/geometry.cpp   | 14 ++++++++++++++
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> > index 3e6f0f5d7..dc56f180f 100644
> > --- a/include/libcamera/geometry.h
> > +++ b/include/libcamera/geometry.h
> > @@ -262,6 +262,8 @@ public:
> >  	{
> >  	}
> >
> > +	constexpr Rectangle(const Point &topLeft, const Point &bottomRight);
>
> Don't make this `constexpr` because it is not useful since the definition is not available.

I found references online that constexpr constuctors are implcitly
inline, is this the reason of your comment ?

However, I can't find it clearly specified in cppreference. Do you
have any pointer ?

Anyway, if inline is the reason, isn't it better to inline the
definition and maintain the constexpr specifier ?

>
>
> Regards,
> Barnabás Pőcze
>
>
> > +
> >  	int x;
> >  	int y;
> >  	unsigned int width;
> > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> > index 000151364..029b8dad2 100644
> > --- a/src/libcamera/geometry.cpp
> > +++ b/src/libcamera/geometry.cpp
> > @@ -629,6 +629,20 @@ 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
> > + */
> > +constexpr Rectangle::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);
> > +}
> > +
> >  /**
> >   * \var Rectangle::x
> >   * \brief The horizontal coordinate of the rectangle's top-left corner
> > --
> > 2.46.0.469.g59c65b2a67-goog
> >
> >