[1/4] libcamera: geometry: Add floating-point version of Point class

19842 diff mbox series

The piecewise linear function (Pwl) class from the Raspberry Pi IPA has
its own Point class while one already exists in libcamera's geometry.h
header. The reason is because libcamera's Point is on the plane of
integer, while Raspberry Pi's is on the plane of reals.

While making this a template class might be cleaner, it was deemed to be
too intrusive of a change, and it might not feel nice to need to specify
the type from a public API point of view. Hence we introduce a FPoint
class to designate a Point class on the plane of reals.

This is in preparation for copying/moving the Pwl class from the
Raspberry Pi IPA to libipa.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 include/libcamera/geometry.h | 50 ++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

Hi Paul,

thanks for the patch.

On Fri, Apr 05, 2024 at 05:02:56PM +0900, Paul Elder wrote:
> The piecewise linear function (Pwl) class from the Raspberry Pi IPA has
> its own Point class while one already exists in libcamera's geometry.h
> header. The reason is because libcamera's Point is on the plane of
> integer, while Raspberry Pi's is on the plane of reals.
> 
> While making this a template class might be cleaner, it was deemed to be
> too intrusive of a change, and it might not feel nice to need to specify
> the type from a public API point of view. Hence we introduce a FPoint
> class to designate a Point class on the plane of reals.

Sounds reasonable to me. One thing that came to my mind is a bit
nitpicking on the name. I would name it PointF, so that the Point and
PointF types end up at similar places in alphabetical listings (docs,
type-completion etc.)

> 
> This is in preparation for copying/moving the Pwl class from the
> Raspberry Pi IPA to libipa.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  include/libcamera/geometry.h | 50 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> index d7fdbe70..7d0c0f23 100644
> --- a/include/libcamera/geometry.h
> +++ b/include/libcamera/geometry.h
> @@ -8,6 +8,7 @@
>  #pragma once
>  
>  #include <algorithm>
> +#include <cmath>
>  #include <ostream>
>  #include <string>
>  
> @@ -49,6 +50,55 @@ static inline bool operator!=(const Point &lhs, const Point &rhs)
>  
>  std::ostream &operator<<(std::ostream &out, const Point &p);
>  
> +struct FPoint {
> +	constexpr FPoint()
> +		: x(0), y(0)
> +	{
> +	}
> +
> +	constexpr FPoint(double _x, double _y)
> +		: x(_x), y(_y)
> +	{
> +	}
> +
> +	constexpr FPoint operator-(FPoint const &p) const
> +	{
> +		return FPoint(x - p.x, y - p.y);
> +	}
> +
> +	constexpr FPoint operator+(FPoint const &p) const
> +	{
> +		return FPoint(x + p.x, y + p.y);
> +	}
> +
> +	constexpr double operator%(FPoint const &p) const
> +	{
> +		return x * p.x + y * p.y;
> +	}
> +
> +	constexpr FPoint operator*(double f) const
> +	{
> +		return FPoint(x * f, y * f);
> +	}
> +
> +	constexpr FPoint operator/(double f) const
> +	{
> +		return FPoint(x / f, y / f);
> +	}
> +
> +	constexpr double len2() const
> +	{
> +		return x * x + y * y;
> +	}
> +
> +	constexpr double len() const
> +	{
> +		return std::sqrt(len2());
> +	}
> +
> +	double x, y;
> +};

As this is in the public API, we should add documentation for it. I also
saw some tests for Point in test/geometry.cpp. These should be
duplicated for the float version.

For completeness sake, the operator!= and operator<<(std::ostream &out,
const FPoint &p) should also be copied.

Best regards,
Stefan

> +
>  class Size
>  {
>  public:
> -- 
> 2.39.2
>