[PATCH v2 1/4] libcamera: geometry: Add floating-point version of Point class
Stefan Klug
stefan.klug at ideasonboard.com
Thu May 2 21:18:56 CEST 2024
Hi Paul,
thanks for the patch.
On Fri, Apr 26, 2024 at 04:36:09PM +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.
nit: s/integer/integers/
>
> 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
nit: s/FPoint/PointF/
> 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 at ideasonboard.com>
> ---
> Changes in v2:
> - renamed FPoint to PointF
> - add documentation
> - add tests
> ---
> include/libcamera/geometry.h | 65 +++++++
> src/libcamera/geometry.cpp | 123 +++++++++++-
> test/geometry.cpp | 355 +++++++++++++++++++++++++++++++++++
> 3 files changed, 542 insertions(+), 1 deletion(-)
>
> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> index d7fdbe70..6aef1f39 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,70 @@ static inline bool operator!=(const Point &lhs, const Point &rhs)
>
> std::ostream &operator<<(std::ostream &out, const Point &p);
>
> +struct PointF {
> + constexpr PointF()
> + : x(0), y(0)
> + {
> + }
> +
> + constexpr PointF(double _x, double _y)
> + : x(_x), y(_y)
> + {
> + }
> +
> + constexpr PointF operator-() const
> + {
> + return PointF{ -x, -y };
> + }
> +
> + constexpr PointF operator-(const PointF &p) const
> + {
> + return PointF(x - p.x, y - p.y);
> + }
> +
> + constexpr PointF operator+(const PointF &p) const
> + {
> + return PointF(x + p.x, y + p.y);
> + }
> +
> + constexpr double operator%(const PointF &p) const
> + {
> + return x * p.x + y * p.y;
> + }
This operator implementation is unexpected to me. Is that a common usage
for % ? I would need to look that up, whilst a dot() function would
convey the intend immediately (at least to me).
With or without that changed:
Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
Cheers,
Stefan
> +
> + constexpr PointF operator*(double f) const
> + {
> + return PointF(x * f, y * f);
> + }
> +
> + constexpr PointF operator/(double f) const
> + {
> + return PointF(x / f, y / f);
> + }
> +
> + constexpr double len2() const
> + {
> + return x * x + y * y;
> + }
> +
> + constexpr double len() const
> + {
> + return std::sqrt(len2());
> + }
> +
> + const std::string toString() const;
> +
> + double x, y;
> +};
> +
> +bool operator==(const PointF &lhs, const PointF &rhs);
> +static inline bool operator!=(const PointF &lhs, const PointF &rhs)
> +{
> + return !(lhs == rhs);
> +}
> +
> +std::ostream &operator<<(std::ostream &out, const PointF &p);
> +
> class Size
> {
> public:
> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> index 8d85b758..90b81282 100644
> --- a/src/libcamera/geometry.cpp
> +++ b/src/libcamera/geometry.cpp
> @@ -21,7 +21,7 @@ namespace libcamera {
>
> /**
> * \class Point
> - * \brief Describe a point in two-dimensional space
> + * \brief Describe a point in two-dimensional integer space
> *
> * The Point structure defines a point in two-dimensional space with integer
> * precision. The coordinates of a Point may be negative as well as positive.
> @@ -94,6 +94,127 @@ std::ostream &operator<<(std::ostream &out, const Point &p)
> return out;
> }
>
> +/**
> + * \class PointF
> + * \brief Describe a point in two-dimensional real space
> + *
> + * The Point structure defines a point in two-dimensional space with double
> + * precision. The coordinates of a Point may be negative as well as positive.
> + *
> + * This class exists separately from Point to not require all users of the
> + * Point class to have to use template parameters to specify a type.
> + */
> +
> +/**
> + * \fn PointF::PointF()
> + * \copydoc libcamera::Point::Point
> + */
> +
> +/**
> + * \fn PointF::PointF(double _x, double _y)
> + * \brief Construct a PointF at given \a _x and \a _y values
> + * \param[in] _x The x-coordinate
> + * \param[in] _y The y-coordinate
> + */
> +
> +/**
> + * \var PointF::x
> + * \copydoc libcamera::Point::x
> + */
> +
> +/**
> + * \var PointF::y
> + * \copydoc libcamera::Point::y
> + */
> +
> +/**
> + * \fn constexpr PointF PointF::operator-() const
> + * \copydoc libcamera::Point::operator-
> + */
> +
> +/**
> + * \fn constexpr PointF PointF::operator-(PointF const &p) const
> + * \brief Subtract one point from another, as if they were vectors
> + * \param[in] p The other point
> + * \return The difference of p from this point
> + */
> +
> +/**
> + * \fn PointF::operator+()
> + * \brief Add two points together, as if they were vectors
> + * \param[in] p The other point
> + * \return The sum of the two points
> + */
> +
> +/**
> + * \fn PointF::operator%()
> + * \brief Compute the dot product, treating the points as vectors
> + * \param[in] p The other point
> + * \return The dot product of the two points
> + */
> +
> +/**
> + * \fn PointF::operator*()
> + * \brief Scale up the point, as if it were a vector
> + * \param[in] f The factor
> + * \return The scaled point
> + */
> +
> +/**
> + * \fn PointF::operator/()
> + * \brief Scale down the point, as if it were a vector
> + * \param[in] f The factor
> + * \return The scaled point
> + */
> +
> +/**
> + * \fn PointF::len2()
> + * \brief Get the squared length of the point, as if it were a vector
> + * \return The squared length of the point
> + */
> +
> +/**
> + * \fn PointF::len()
> + * \brief Get the length of the point, as if it were a vector
> + * \return The length of the point
> + */
> +
> +/**
> + * \copydoc Point::toString()
> + */
> +const std::string PointF::toString() const
> +{
> + std::stringstream ss;
> + ss << *this;
> +
> + return ss.str();
> +}
> +
> +/**
> + * \copydoc operator==(const Point &lhs, const Point &rhs)
> + */
> +bool operator==(const PointF &lhs, const PointF &rhs)
> +{
> + return lhs.x == rhs.x && lhs.y == rhs.y;
> +}
> +
> +/**
> + * \fn bool operator!=(const Point &lhs, const Point &rhs)
> + * \copydoc libcamera::Point::operator!=
> + */
> +
> +/**
> + * \brief Insert a text representation of a PointF into an output stream
> + * \param[in] out The output stream
> + * \param[in] p The point
> + * \return The output stream \a out
> + */
> +std::ostream &operator<<(std::ostream &out, const PointF &p)
> +{
> + out << "(" << p.x << ", " << p.y << ")";
> + return out;
> +}
> +
> /**
> * \class Size
> * \brief Describe a two-dimensional size
> diff --git a/test/geometry.cpp b/test/geometry.cpp
> index 008d51ea..7f1f5b1a 100644
> --- a/test/geometry.cpp
> +++ b/test/geometry.cpp
> @@ -33,6 +33,53 @@ protected:
> return true;
> }
>
> + template<typename T, typename U, typename V>
> + bool compareF(const T &lhs, const U &rhs,
> + V (PointF::*op)(const U &) const,
> + const char *opName, V expect)
> + {
> + V result = (lhs.*op)(rhs);
> +
> + if (result != expect) {
> + cout << lhs << opName << " " << rhs
> + << "test failed" << std::endl;
> + return false;
> + }
> +
> + return true;
> + }
> +
> + template<typename T, typename U, typename V>
> + bool compareFScale(const T &lhs, const U &rhs,
> + V (PointF::*op)(U) const,
> + const char *opName, V expect)
> + {
> + V result = (lhs.*op)(rhs);
> +
> + if (result != expect) {
> + cout << lhs << opName << " " << rhs
> + << "test failed" << std::endl;
> + return false;
> + }
> +
> + return true;
> + }
> +
> + template<typename T>
> + bool compareFLen(const T &lhs, double (PointF::*op)() const,
> + const char *opName, double expect)
> + {
> + double result = (lhs.*op)();
> +
> + if (result != expect) {
> + cout << lhs << opName
> + << "test failed" << std::endl;
> + return false;
> + }
> +
> + return true;
> + }
> +
> int run()
> {
> /*
> @@ -88,6 +135,314 @@ protected:
> return TestFail;
> }
>
> + /*
> + * PointF tests
> + */
> +
> + /* Equality */
> + if (!compare(PointF(50.1, 100.1), PointF(50.1, 100.1), &operator==, "==", true))
> + return TestFail;
> +
> + if (!compare(PointF(-50.1, 100.1), PointF(-50.1, 100.1), &operator==, "==", true))
> + return TestFail;
> +
> + if (!compare(PointF(50.1, -100.1), PointF(50.1, -100.1), &operator==, "==", true))
> + return TestFail;
> +
> + if (!compare(PointF(-50.1, -100.1), PointF(-50.1, -100.1), &operator==, "==", true))
> + return TestFail;
> +
> + if (!compare(PointF(-50.1, -100.0), PointF(-50.1, -100), &operator==, "==", true))
> + return TestFail;
> +
> + /* Inequality */
> + if (!compare(PointF(50.1, 100.1), PointF(50.1, 100.2), &operator!=, "!=", true))
> + return TestFail;
> +
> + if (!compare(PointF(-50.1, 100.1), PointF(-50.1, 100.01), &operator!=, "!=", true))
> + return TestFail;
> +
> + if (!compare(PointF(-50.1, 100.1), PointF(-50.1, 100.1), &operator!=, "!=", false))
> + return TestFail;
> +
> + if (!compare(PointF(50.1, -100.1), PointF(50.2, -100.1), &operator!=, "!=", true))
> + return TestFail;
> +
> + if (!compare(PointF(-50.1, -100.1), PointF(-50.01, -100.0), &operator!=, "!=", true))
> + return TestFail;
> +
> + if (!compare(PointF(-50.1, 100.1), PointF(50.1, 100.1), &operator!=, "!=", true))
> + return TestFail;
> +
> + if (!compare(PointF(50.1, -100.1), PointF(50.1, 100.1), &operator!=, "!=", true))
> + return TestFail;
> +
> + if (!compare(PointF(-50.1, -100.1), PointF(-50.1, -100.1), &operator!=, "!=", false))
> + return TestFail;
> +
> + /* Negation */
> + if (PointF(50.1, 100.1) != -PointF(-50.1, -100.1) ||
> + PointF(50.1, 100.1) == -PointF(50.1, -100.1) ||
> + PointF(50.1, 100.1) == -PointF(-50.1, 100.1)) {
> + cout << "PointF negation test failed" << endl;
> + return TestFail;
> + }
> +
> + typedef PointF (PointF::*pointfOp)(const PointF &) const;
> + typedef double (PointF::*pointfDotProd)(const PointF &) const;
> + typedef PointF (PointF::*pointfScale)(double) const;
> + typedef double (PointF::*pointfLen)() const;
> +
> + /* Subtraction */
> + if (!compareF<PointF, PointF, PointF>(PointF(50.1, 100.1), PointF(50.1, 100.1),
> + static_cast<pointfOp>(&PointF::operator-), "-",
> + PointF(0, 0)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(-50.1, 100.1), PointF(-50.1, 100.1),
> + static_cast<pointfOp>(&PointF::operator-), "-",
> + PointF(0, 0)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(50.1, -100.1), PointF(50.1, -100.1),
> + static_cast<pointfOp>(&PointF::operator-), "-",
> + PointF(0, 0)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(-50.1, -100.1), PointF(-50.1, -100.1),
> + static_cast<pointfOp>(&PointF::operator-), "-",
> + PointF(0, 0)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(50.1, 100.1), PointF(-50.1, 100.1),
> + static_cast<pointfOp>(&PointF::operator-), "-",
> + PointF(100.2, 0)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(50.1, 100.1), PointF(50.1, -100.1),
> + static_cast<pointfOp>(&PointF::operator-), "-",
> + PointF(0, 200.2)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(50.1, 100.1), PointF(-50.1, -100.1),
> + static_cast<pointfOp>(&PointF::operator-), "-",
> + PointF(100.2, 200.2)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(-50.1, 100.1), PointF(50.1, 100.1),
> + static_cast<pointfOp>(&PointF::operator-), "-",
> + PointF(-100.2, 0)))
> + return TestFail;
> +
> + /* Addition */
> + if (!compareF<PointF, PointF, PointF>(PointF(50.1, 100.1), PointF(50.1, 100.1),
> + static_cast<pointfOp>(&PointF::operator+), "+",
> + PointF(100.2, 200.2)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(-50.1, 100.1), PointF(-50.1, 100.1),
> + static_cast<pointfOp>(&PointF::operator+), "+",
> + PointF(-100.2, 200.2)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(50.1, -100.1), PointF(50.1, -100.1),
> + static_cast<pointfOp>(&PointF::operator+), "+",
> + PointF(100.2, -200.2)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(-50.1, -100), PointF(-50.1, -100.1),
> + static_cast<pointfOp>(&PointF::operator+), "+",
> + PointF(-100.2, -200.1)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(50.1, 100.1), PointF(-50.1, 100.1),
> + static_cast<pointfOp>(&PointF::operator+), "+",
> + PointF(0, 200.2)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(50.1, 100.0), PointF(50.1, -100.1),
> + static_cast<pointfOp>(&PointF::operator+), "+",
> + PointF(100.2, -0.09999999999999432)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(50.1, 100.1), PointF(-50.1, -100.1),
> + static_cast<pointfOp>(&PointF::operator+), "+",
> + PointF(0, 0)))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, PointF>(PointF(-50.1, 100.1), PointF(50.1, 100.1),
> + static_cast<pointfOp>(&PointF::operator+), "+",
> + PointF(0, 200.2)))
> + return TestFail;
> +
> + /* Dot product */
> + if (!compareF<PointF, PointF, double>(PointF(50.1, 100.1), PointF(50.1, 100.1),
> + static_cast<pointfDotProd>(&PointF::operator%), "%",
> + 12530.019999999999))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, double>(PointF(50.1, 100.1), PointF(50.1, -100.1),
> + static_cast<pointfDotProd>(&PointF::operator%), "%",
> + -7509.999999999998))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, double>(PointF(50.1, 100.1), PointF(-50.1, 100.1),
> + static_cast<pointfDotProd>(&PointF::operator%), "%",
> + 7509.999999999998))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, double>(PointF(50.1, 100.1), PointF(-50.1, -100.1),
> + static_cast<pointfDotProd>(&PointF::operator%), "%",
> + -12530.019999999999))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, double>(PointF(-50.1, 100.1), PointF(-50.1, 100.1),
> + static_cast<pointfDotProd>(&PointF::operator%), "%",
> + 12530.019999999999))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, double>(PointF(50.1, -100.1), PointF(50.1, -100.1),
> + static_cast<pointfDotProd>(&PointF::operator%), "%",
> + 12530.019999999999))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, double>(PointF(-50.1, -100.1), PointF(-50.1, -100.1),
> + static_cast<pointfDotProd>(&PointF::operator%), "%",
> + 12530.019999999999))
> + return TestFail;
> +
> + if (!compareF<PointF, PointF, double>(PointF(0, 0), PointF(0, 0),
> + static_cast<pointfDotProd>(&PointF::operator%), "%",
> + 0))
> + return TestFail;
> +
> +
> + /* Scaling up */
> + if (!compareFScale<PointF, double, PointF>(PointF(10.5, -100.1), 0,
> + static_cast<pointfScale>(&PointF::operator*), "*",
> + PointF(0, 0)))
> + return TestFail;
> +
> + if (!compareFScale<PointF, double, PointF>(PointF(10.5, -100.1), 1,
> + static_cast<pointfScale>(&PointF::operator*), "*",
> + PointF(10.5, -100.1)))
> + return TestFail;
> +
> + if (!compareFScale<PointF, double, PointF>(PointF(10.5, -100.1), 1.0,
> + static_cast<pointfScale>(&PointF::operator*), "*",
> + PointF(10.5, -100.1)))
> + return TestFail;
> +
> + if (!compareFScale<PointF, double, PointF>(PointF(10.5, -100.1), -4.2,
> + static_cast<pointfScale>(&PointF::operator*), "*",
> + PointF(-44.1, 420.42)))
> + return TestFail;
> +
> + if (!compareFScale<PointF, double, PointF>(PointF(10.5, -100.1), 4.2,
> + static_cast<pointfScale>(&PointF::operator*), "*",
> + PointF(44.1, -420.42)))
> + return TestFail;
> +
> + if (!compareFScale<PointF, double, PointF>(PointF(0, -100.1), 4.2,
> + static_cast<pointfScale>(&PointF::operator*), "*",
> + PointF(0, -420.42)))
> + return TestFail;
> +
> + if (!compareFScale<PointF, double, PointF>(PointF(-50.1, -100.1), 4.2,
> + static_cast<pointfScale>(&PointF::operator*), "*",
> + PointF(-210.42000000000002, -420.42)))
> + return TestFail;
> +
> + /* Scaling down */
> + if (!compareFScale<PointF, double, PointF>(PointF(10.5, -100.1), 1,
> + static_cast<pointfScale>(&PointF::operator/), "/",
> + PointF(10.5, -100.1)))
> + return TestFail;
> +
> + if (!compareFScale<PointF, double, PointF>(PointF(10.5, -100.1), 1.0,
> + static_cast<pointfScale>(&PointF::operator/), "/",
> + PointF(10.5, -100.1)))
> + return TestFail;
> +
> + if (!compareFScale<PointF, double, PointF>(PointF(10.5, -100.1), -4.2,
> + static_cast<pointfScale>(&PointF::operator/), "/",
> + PointF(-2.5, 23.833333333333332)))
> + return TestFail;
> +
> + if (!compareFScale<PointF, double, PointF>(PointF(10.5, -100.1), 4.2,
> + static_cast<pointfScale>(&PointF::operator/), "/",
> + PointF(2.5, -23.833333333333332)))
> + return TestFail;
> +
> + if (!compareFScale<PointF, double, PointF>(PointF(0, -100.1), 4.2,
> + static_cast<pointfScale>(&PointF::operator/), "/",
> + PointF(0, -23.833333333333332)))
> + return TestFail;
> +
> + if (!compareFScale<PointF, double, PointF>(PointF(-50.1, -100.1), 4.2,
> + static_cast<pointfScale>(&PointF::operator/), "/",
> + PointF(-11.928571428571429, -23.833333333333332)))
> + return TestFail;
> +
> +
> + /* Squared length */
> + if (!compareFLen<PointF>(PointF(0, 0),
> + static_cast<pointfLen>(&PointF::len2), "len2",
> + 0))
> + return TestFail;
> +
> + if (!compareFLen<PointF>(PointF(10.4, 0),
> + static_cast<pointfLen>(&PointF::len2), "len2",
> + 108.16000000000001))
> + return TestFail;
> +
> + if (!compareFLen<PointF>(PointF(10.4, 50.1),
> + static_cast<pointfLen>(&PointF::len2), "len2",
> + 2618.17))
> + return TestFail;
> +
> + if (!compareFLen<PointF>(PointF(-10.4, 50.1),
> + static_cast<pointfLen>(&PointF::len2), "len2",
> + 2618.17))
> + return TestFail;
> +
> + if (!compareFLen<PointF>(PointF(-10.4, -50.1),
> + static_cast<pointfLen>(&PointF::len2), "len2",
> + 2618.17))
> + return TestFail;
> +
> + /* Length */
> + if (!compareFLen<PointF>(PointF(0, 0),
> + static_cast<pointfLen>(&PointF::len), "len",
> + 0))
> + return TestFail;
> +
> + if (!compareFLen<PointF>(PointF(10.4, 0),
> + static_cast<pointfLen>(&PointF::len), "len",
> + 10.4))
> + return TestFail;
> +
> + if (!compareFLen<PointF>(PointF(10.4, 50.1),
> + static_cast<pointfLen>(&PointF::len), "len",
> + 51.16805644149483))
> + return TestFail;
> +
> + if (!compareFLen<PointF>(PointF(-10.4, 50.1),
> + static_cast<pointfLen>(&PointF::len), "len",
> + 51.16805644149483))
> + return TestFail;
> +
> + if (!compareFLen<PointF>(PointF(-10.4, -50.1),
> + static_cast<pointfLen>(&PointF::len), "len",
> + 51.16805644149483))
> + return TestFail;
> +
> + /* Default constructor */
> + if (PointF() != PointF(0, 0)) {
> + cout << "Default constructor test failed" << endl;
> + return TestFail;
> + }
> +
> /*
> * Size tests
> */
> --
> 2.39.2
>
More information about the libcamera-devel
mailing list