[PATCH v9 2/5] libcamera: geometry: Add two-point Rectangle constructor

[Jacopo Mondi]

Hi Barnabás

On Mon, Sep 30, 2024 at 10:50:30PM GMT, Barnabás Pőcze wrote:
> Hi
>
>
> 2024. szeptember 30., hétfő 21:59 keltezéssel, Jacopo Mondi <jacopo.mondi at ideasonboard.com> írta:
>
> > From: Yudhistira Erlandinata <yerlandinata at chromium.org>
> >
> > Add a constructor to the Rectangle class that accepts two points.
> >
> > The constructed Rectangle spans all the space between the two given
> > points.
> >
> > 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>
> > Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> > ---
> >  include/libcamera/geometry.h |  7 +++++
> >  src/libcamera/geometry.cpp   | 58 ++++++++++++++++++++++++++++++++++++
> >  test/geometry.cpp            | 14 +++++++++
> >  3 files changed, 79 insertions(+)
> >
> > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> > index 3e6f0f5d7fab..2cc25f1facd9 100644
> > --- a/include/libcamera/geometry.h
> > +++ b/include/libcamera/geometry.h
> > @@ -262,6 +262,13 @@ public:
> >  	{
> >  	}
> >
> > +	constexpr Rectangle(const Point &point1, const Point &point2)
> > +		: Rectangle(std::min(point1.x, point2.x), std::min(point1.y, point2.y),
> > +			    std::max(point1.x, point2.x) - std::min(point1.x, point2.x),
> > +			    std::max(point1.y, point2.y) - std::min(point1.y, point2.y))
>
> The subtraction can lead to under/overflow. I believe simply casting to
> `unsigned` avoids that, e.g.:
>
>   unsigned(std::max(point1.x, point2.x)) - unsigned(std::min(point1.x, point2.x))
>
> Although I don't foresee it causing any issues, since the fix is easy,
> I think it's worth doing.

Is this because we're subtracting two signed values and assign the
result to an unsigned ?

It shouldn't be an issue, but better safe than sorry, so I'll happily
take your suggestion in.

Thanks
  j

>
>
> Regards,
> Barnabás Pőcze
>
> > +	{
> > +	}
> > +
> >  	int x;
> >  	int y;
> >  	unsigned int width;
> > [...]