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

[Barnabás Pőcze]

Hi


2024. szeptember 5., csütörtök 10:35 keltezéssel, Jacopo Mondi <jacopo.mondi at ideasonboard.com> írta:

> 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 ?

Yes.


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

  "A constexpr specifier used in a function or static data member(since C++17) declaration implies inline."
  -- https://en.cppreference.com/w/cpp/language/constexpr

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

That is an option as well.


Regards,
Barnabás Pőcze