[{"id":29157,"web_url":"https://patchwork.libcamera.org/comment/29157/","msgid":"","date":"2024-04-05T09:56:58","subject":"Re: [PATCH 3/4] ipa: libipa: pwl: Clean up Pwl class to match\n\tlibcamera","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Paul,\n\nOn Fri, 5 Apr 2024 at 09:03, Paul Elder wrote:\n>\n> Clean up the Pwl class copied from the Raspberry Pi IPA to align it more\n> with the libcamera style.\n>\n> Signed-off-by: Paul Elder \n> ---\n> src/ipa/libipa/pwl.cpp | 135 +++++++++++++++++++++++++++++++++--------\n> src/ipa/libipa/pwl.h | 113 ++++++++++++++--------------------\n> 2 files changed, 154 insertions(+), 94 deletions(-)\n>\n> diff --git a/src/ipa/libipa/pwl.cpp b/src/ipa/libipa/pwl.cpp\n> index 09f5d65c..58925d83 100644\n> --- a/src/ipa/libipa/pwl.cpp\n> +++ b/src/ipa/libipa/pwl.cpp\n> @@ -5,13 +5,40 @@\n> * pwl.cpp - piecewise linear functions\n> */\n>\n> +#include \"pwl.h\"\n> +\n> #include \n> #include \n> +#include \n> #include \n>\n> -#include \"pwl.h\"\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +namespace ipa {\n>\n> -int Pwl::read(const libcamera::YamlObject ¶ms)\n> +/*\n> + * \\enum Pwl::PerpType\n> + * \\brief Type of perpendicular found when inverting a piecewise linear function\n> + *\n> + * \\var None\n> + * \\brief no perpendicular found\n> + *\n> + * \\var Start\n> + * \\brief start of Pwl is closest point\n> + *\n> + * \\var End\n> + * \\brief end of Pwl is closest point\n> + *\n> + * \\var Vertex\n> + * \\brief vertex of Pwl is closest point\n> + *\n> + * \\var Perpendicular\n> + * \\brief true perpendicular found\n> + */\n> +\n> +int Pwl::readYaml(const libcamera::YamlObject ¶ms)\n> {\n> if (!params.size() || params.size() % 2)\n> return -EINVAL;\n> @@ -29,7 +56,7 @@ int Pwl::read(const libcamera::YamlObject ¶ms)\n> if (!y)\n> return -EINVAL;\n>\n> - points_.push_back(Point(*x, *y));\n> + points_.push_back(FPoint(*x, *y));\n> }\n>\n> return 0;\n> @@ -38,13 +65,13 @@ int Pwl::read(const libcamera::YamlObject ¶ms)\n> void Pwl::append(double x, double y, const double eps)\n> {\n> if (points_.empty() || points_.back().x + eps < x)\n> - points_.push_back(Point(x, y));\n> + points_.push_back(FPoint(x, y));\n> }\n>\n> void Pwl::prepend(double x, double y, const double eps)\n> {\n> if (points_.empty() || points_.front().x - eps > x)\n> - points_.insert(points_.begin(), Point(x, y));\n> + points_.insert(points_.begin(), FPoint(x, y));\n> }\n>\n> Pwl::Interval Pwl::domain() const\n> @@ -65,6 +92,19 @@ bool Pwl::empty() const\n> return points_.empty();\n> }\n>\n> +/*\n> + * \\brief Evaluate the piecewise linear function\n> + * \\param[in] x The x value to input into the function\n> + * \\param[inout] spanPtr Initial guess for span\n> + * \\param[in] updateSpan Set to true to update spanPtr\n> + *\n> + * Evaluate Pwl, optionally supplying an initial guess for the\n> + * \"span\". The \"span\" may be optionally be updated. If you want to know\n> + * the \"span\" value but don't have an initial guess you can set it to\n> + * -1.\n> + *\n> + * \\return The result of evaluating the piecewise linear function with input \\a x\n> + */\n> double Pwl::eval(double x, int *spanPtr, bool updateSpan) const\n> {\n> int span = findSpan(x, spanPtr && *spanPtr != -1 ? *spanPtr : points_.size() / 2 - 1);\n> @@ -94,16 +134,22 @@ int Pwl::findSpan(double x, int span) const\n> return span;\n> }\n>\n> -Pwl::PerpType Pwl::invert(Point const &xy, Point &perp, int &span,\n> +/*\n> + * Find perpendicular closest to xy, starting from span+1 so you can\n> + * call it repeatedly to check for multiple closest points (set span to\n> + * -1 on the first call). Also returns \"pseudo\" perpendiculars; see\n> + * PerpType enum.\n> + */\n> +Pwl::PerpType Pwl::invert(FPoint const &xy, FPoint &perp, int &span,\n> const double eps) const\n> {\n> assert(span >= -1);\n> bool prevOffEnd = false;\n> for (span = span + 1; span < (int)points_.size() - 1; span++) {\n> - Point spanVec = points_[span + 1] - points_[span];\n> + FPoint spanVec = points_[span + 1] - points_[span];\n> double t = ((xy - points_[span]) % spanVec) / spanVec.len2();\n> - if (t < -eps) /* off the start of this span */\n> - {\n> + if (t < -eps) {\n> + /* off the start of this span */\n> if (span == 0) {\n> perp = points_[span];\n> return PerpType::Start;\n> @@ -111,15 +157,15 @@ Pwl::PerpType Pwl::invert(Point const &xy, Point &perp, int &span,\n> perp = points_[span];\n> return PerpType::Vertex;\n> }\n> - } else if (t > 1 + eps) /* off the end of this span */\n> - {\n> + } else if (t > 1 + eps) {\n> + /* off the end of this span */\n> if (span == (int)points_.size() - 2) {\n> perp = points_[span + 1];\n> return PerpType::End;\n> }\n> prevOffEnd = true;\n> - } else /* a true perpendicular */\n> - {\n> + } else {\n> + /* a true perpendicular */\n> perp = points_[span] + spanVec * t;\n> return PerpType::Perpendicular;\n> }\n> @@ -127,25 +173,34 @@ Pwl::PerpType Pwl::invert(Point const &xy, Point &perp, int &span,\n> return PerpType::None;\n> }\n>\n> +/*\n> + * \\brief Compute the inverse function\n> + * \\param[out] trueInverse True of the resulting inverse is a proper/true inverse\n> + * \\param[in] eps Epsilon (optional)\n> + * Indicate if it is a proper (true) inverse, or only a best effort (e.g.\n> + * input was non-monotonic).\n> + * \\return The inverse piecewise linear function\n> + */\n> Pwl Pwl::inverse(bool *trueInverse, const double eps) const\n> {\n> bool appended = false, prepended = false, neither = false;\n> Pwl inverse;\n>\n> - for (Point const &p : points_) {\n> - if (inverse.empty())\n> + for (FPoint const &p : points_) {\n> + if (inverse.empty()) {\n> inverse.append(p.y, p.x, eps);\n> - else if (std::abs(inverse.points_.back().x - p.y) <= eps ||\n> - std::abs(inverse.points_.front().x - p.y) <= eps)\n> + } else if (std::abs(inverse.points_.back().x - p.y) <= eps ||\n> + std::abs(inverse.points_.front().x - p.y) <= eps) {\n> /* do nothing */;\n> - else if (p.y > inverse.points_.back().x) {\n> + } else if (p.y > inverse.points_.back().x) {\n> inverse.append(p.y, p.x, eps);\n> appended = true;\n> } else if (p.y < inverse.points_.front().x) {\n> inverse.prepend(p.y, p.x, eps);\n> prepended = true;\n> - } else\n> + } else {\n> neither = true;\n> + }\n> }\n>\n> /*\n> @@ -159,18 +214,25 @@ Pwl Pwl::inverse(bool *trueInverse, const double eps) const\n> return inverse;\n> }\n>\n> +/*\n> + * \\brief Compose two piecewise linear functions together\n> + * \\param[in] other The \"other\" piecewise linear function\n> + * \\param[in] eps Epsilon (optiona)\n> + * The \"this\" function is done first, and \"other\" after.\n> + * \\return The composed piecewise linear function\n> + */\n> Pwl Pwl::compose(Pwl const &other, const double eps) const\n> {\n> double thisX = points_[0].x, thisY = points_[0].y;\n> int thisSpan = 0, otherSpan = other.findSpan(thisY, 0);\n> Pwl result({ { thisX, other.eval(thisY, &otherSpan, false) } });\n> +\n> while (thisSpan != (int)points_.size() - 1) {\n> double dx = points_[thisSpan + 1].x - points_[thisSpan].x,\n> dy = points_[thisSpan + 1].y - points_[thisSpan].y;\n> if (std::abs(dy) > eps &&\n> otherSpan + 1 < (int)other.points_.size() &&\n> - points_[thisSpan + 1].y >=\n> - other.points_[otherSpan + 1].x + eps) {\n> + points_[thisSpan + 1].y >= other.points_[otherSpan + 1].x + eps) {\n> /*\n> * next control point in result will be where this\n> * function's y reaches the next span in other\n> @@ -204,18 +266,24 @@ Pwl Pwl::compose(Pwl const &other, const double eps) const\n> return result;\n> }\n>\n> +/* \\brief Apply function to (x,y) values at every control point. */\n> void Pwl::map(std::function f) const\n> {\n> for (auto &pt : points_)\n> f(pt.x, pt.y);\n> }\n>\n> +/*\n> + * \\brief Apply function to (x, y0, y1) values wherever either Pwl has a\n> + * control point.\n> + */\n> void Pwl::map2(Pwl const &pwl0, Pwl const &pwl1,\n> std::function f)\n> {\n> int span0 = 0, span1 = 0;\n> double x = std::min(pwl0.points_[0].x, pwl1.points_[0].x);\n> f(x, pwl0.eval(x, &span0, false), pwl1.eval(x, &span1, false));\n> +\n> while (span0 < (int)pwl0.points_.size() - 1 ||\n> span1 < (int)pwl1.points_.size() - 1) {\n> if (span0 == (int)pwl0.points_.size() - 1)\n> @@ -230,6 +298,12 @@ void Pwl::map2(Pwl const &pwl0, Pwl const &pwl1,\n> }\n> }\n>\n> +/*\n> + * \\brief Combine two Pwls\n> + *\n> + * Create a new Pwl where the y values are given by running f wherever either\n> + * has a knot.\n> + */\n> Pwl Pwl::combine(Pwl const &pwl0, Pwl const &pwl1,\n> std::function f,\n> const double eps)\n> @@ -241,6 +315,11 @@ Pwl Pwl::combine(Pwl const &pwl0, Pwl const &pwl1,\n> return result;\n> }\n>\n> +/*\n> + * \\brief Make \"this\" match (at least) the given domain.\n> + *\n> + * Any extension my be clipped or linear.\n> + */\n> void Pwl::matchDomain(Interval const &domain, bool clip, const double eps)\n> {\n> int span = 0;\n> @@ -258,10 +337,16 @@ Pwl &Pwl::operator*=(double d)\n> return *this;\n> }\n>\n> -void Pwl::debug(FILE *fp) const\n> +std::string Pwl::toString() const\n> {\n> - fprintf(fp, \"Pwl {\\n\");\n> + std::stringstream ss;\n> + ss << \"Pwl { \";\n> for (auto &p : points_)\n> - fprintf(fp, \"\\t(%g, %g)\\n\", p.x, p.y);\n> - fprintf(fp, \"}\\n\");\n> + ss << \"(\" << p.x << \", \" << p.y << \") \";\n> + ss << \"}\";\n> + return ss.str();\n> }\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/pwl.h b/src/ipa/libipa/pwl.h\n> index 7a6a6452..ef49e302 100644\n> --- a/src/ipa/libipa/pwl.h\n> +++ b/src/ipa/libipa/pwl.h\n> @@ -8,116 +8,91 @@\n>\n> #include \n> #include \n> +#include \n> #include \n>\n> +#include \n> +\n> #include \"libcamera/internal/yaml_parser.h\"\n>\n> +namespace libcamera {\n> +\n> +namespace ipa {\n> +\n> class Pwl\n> {\n> public:\n> + enum class PerpType {\n> + None,\n> + Start,\n> + End,\n> + Vertex,\n> + Perpendicular,\n> + };\n> +\n> struct Interval {\n> Interval(double _start, double _end)\n> - : start(_start), end(_end)\n> - {\n> - }\n> - double start, end;\n> + : start(_start), end(_end) {}\n> +\n> bool contains(double value)\n> {\n> return value >= start && value <= end;\n> }\n> - double clip(double value)\n> +\n> + double clamp(double value)\n\nTypically clamp implies a min/max range value. Since we are only\nusing a singular value here, IMO this should still be called clip.\n\nThanks,\nNaush\n\n\n> {\n> return value < start ? start\n> : (value > end ? end : value);\n> }\n> +\n> double len() const { return end - start; }\n> +\n> + double start, end;\n> };\n> - struct Point {\n> - Point() : x(0), y(0) {}\n> - Point(double _x, double _y)\n> - : x(_x), y(_y) {}\n> - double x, y;\n> - Point operator-(Point const &p) const\n> - {\n> - return Point(x - p.x, y - p.y);\n> - }\n> - Point operator+(Point const &p) const\n> - {\n> - return Point(x + p.x, y + p.y);\n> - }\n> - double operator%(Point const &p) const\n> - {\n> - return x * p.x + y * p.y;\n> - }\n> - Point operator*(double f) const { return Point(x * f, y * f); }\n> - Point operator/(double f) const { return Point(x / f, y / f); }\n> - double len2() const { return x * x + y * y; }\n> - double len() const { return sqrt(len2()); }\n> - };\n> +\n> Pwl() {}\n> - Pwl(std::vector const &points) : points_(points) {}\n> - int read(const libcamera::YamlObject ¶ms);\n> + Pwl(std::vector const &points)\n> + : points_(points) {}\n> + int readYaml(const libcamera::YamlObject ¶ms);\n> +\n> void append(double x, double y, const double eps = 1e-6);\n> void prepend(double x, double y, const double eps = 1e-6);\n> +\n> Interval domain() const;\n> Interval range() const;\n> +\n> bool empty() const;\n> - /*\n> - * Evaluate Pwl, optionally supplying an initial guess for the\n> - * \"span\". The \"span\" may be optionally be updated. If you want to know\n> - * the \"span\" value but don't have an initial guess you can set it to\n> - * -1.\n> - */\n> +\n> double eval(double x, int *spanPtr = nullptr,\n> bool updateSpan = true) const;\n> - /*\n> - * Find perpendicular closest to xy, starting from span+1 so you can\n> - * call it repeatedly to check for multiple closest points (set span to\n> - * -1 on the first call). Also returns \"pseudo\" perpendiculars; see\n> - * PerpType enum.\n> - */\n> - enum class PerpType {\n> - None, /* no perpendicular found */\n> - Start, /* start of Pwl is closest point */\n> - End, /* end of Pwl is closest point */\n> - Vertex, /* vertex of Pwl is closest point */\n> - Perpendicular /* true perpendicular found */\n> - };\n> - PerpType invert(Point const &xy, Point &perp, int &span,\n> +\n> + PerpType invert(FPoint const &xy, FPoint &perp, int &span,\n> const double eps = 1e-6) const;\n> - /*\n> - * Compute the inverse function. Indicate if it is a proper (true)\n> - * inverse, or only a best effort (e.g. input was non-monotonic).\n> - */\n> Pwl inverse(bool *trueInverse = nullptr, const double eps = 1e-6) const;\n> - /* Compose two Pwls together, doing \"this\" first and \"other\" after. */\n> Pwl compose(Pwl const &other, const double eps = 1e-6) const;\n> - /* Apply function to (x,y) values at every control point. */\n> +\n> void map(std::function f) const;\n> - /*\n> - * Apply function to (x, y0, y1) values wherever either Pwl has a\n> - * control point.\n> - */\n> +\n> static void map2(Pwl const &pwl0, Pwl const &pwl1,\n> std::function f);\n> - /*\n> - * Combine two Pwls, meaning we create a new Pwl where the y values are\n> - * given by running f wherever either has a knot.\n> - */\n> +\n> static Pwl\n> combine(Pwl const &pwl0, Pwl const &pwl1,\n> std::function f,\n> const double eps = 1e-6);\n> - /*\n> - * Make \"this\" match (at least) the given domain. Any extension my be\n> - * clipped or linear.\n> - */\n> +\n> void matchDomain(Interval const &domain, bool clip = true,\n> const double eps = 1e-6);\n> +\n> Pwl &operator*=(double d);\n> - void debug(FILE *fp = stdout) const;\n> +\n> + std::string toString() const;\n>\n> private:\n> int findSpan(double x, int span) const;\n> - std::vector points_;\n> + std::vector points_;\n> };\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> --\n> 2.39.2\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9FC01C0DA4\n\tfor ;\n\tFri, 5 Apr 2024 09:57:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A191961C20;\n\tFri, 5 Apr 2024 11:57:36 +0200 (CEST)","from mail-yw1-x112e.google.com (mail-yw1-x112e.google.com\n\t[IPv6:2607:f8b0:4864:20::112e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1F99F61C20\n\tfor ;\n\tFri, 5 Apr 2024 11:57:35 +0200 (CEST)","by mail-yw1-x112e.google.com with SMTP id\n\t00721157ae682-617d411a696so2794577b3.1\n\tfor ;\n\tFri, 05 Apr 2024 02:57:35 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"gJqAhkjU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1712311054; x=1712915854;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=CbO1B1edAoX08WJ2rawzQBWCr8koPqwQ3Ckda58dLtA=;\n\tb=gJqAhkjUxdsRJjvGMY+1UODfxjelCk7/nwtZ6SBzuhKvPz0T2R7A8QTj+ygELjPPSh\n\t6YZn4VAgkzEklWDR0/JhEeKRiVtKJIceLpA5u2//PKL1TQDbkoFhM66s5TcSwIWBqMCk\n\tD+webA4ux1l06anFZSNKrji+3Ft5pycrlYKilgWQCzEDUgC5d1vqUzMVHo7B5HJ/wp5y\n\tRF8mzOFztPnumNZw3t9MsAclKE4zIEN/r72EjUNrDTkYWQknzVe9VV3Blux0+YLb1Vb4\n\tdDXNBI9ZcIx/tPMKDGkLvp6+YkIyx9UVShbADwD9l9XW6CafiJ44jDwcTjsQeKHzvBf6\n\t18OQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1712311054; x=1712915854;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=CbO1B1edAoX08WJ2rawzQBWCr8koPqwQ3Ckda58dLtA=;\n\tb=pgfEoNQhXTlCCG9T+Q/tEqoKO9EMVmfQ7FP4IiPRo+y0aJDpfqvWTmGolzJfHSWrYV\n\th8k4ce658ODpjMdVOm+elglzAla/YGMnExJe4z5GJ3iEqWfhzq+gUUT+dAlW/wA0XpOD\n\t5Qv4m1V80Oy4GaiItkhWNhZ5RR0eKMQbZ5bnl2Etrz5zQsl6TEg5jFHZwQAVRKMA5jdh\n\tasmnvjKCsenGOavcFlOKqinfaU8Q4InKVD3XfzeO5xnQA46Jy493xVp2MryQbpX0WtET\n\tR83UpTrZeS2g5YNT53Da7ac7y/7NqFfjeaTmDP6TcTZ3z9r7f049eURVi7AHaNfxZ463\n\tXblw==","X-Gm-Message-State":"AOJu0YwbFIKSXKzMOXu3tv2IU8DWpf5d/rwr/rRCrK/j93P31e6Jn2nq\n\t/6h8gwExJknjASU5d/6tRypnbb564JnbpcET1Pm7ZuZTavF9p7rI+CUFC4GUa7/iEreIH2zGKB8\n\twTaRyo+V2BEhhJ6w+XqMtuNqOPb4Icvf6qh6hcQ==","X-Google-Smtp-Source":"AGHT+IFWWr0H/T9nIajUfO8WkVbBgGKLzQX265e6LFKSRHpS6V9OCLMMuCZMp9ny26QMNbMEEsTNoxHv+4zWXXJP1+k=","X-Received":"by 2002:a81:9c4e:0:b0:615:875:2918 with SMTP id\n\tn14-20020a819c4e000000b0061508752918mr723374ywa.44.1712311053743;\n\tFri, 05 Apr 2024 02:57:33 -0700 (PDT)","MIME-Version":"1.0","References":"<20240405080259.1806453-1-paul.elder@ideasonboard.com>\n\t<20240405080259.1806453-4-paul.elder@ideasonboard.com>","In-Reply-To":"<20240405080259.1806453-4-paul.elder@ideasonboard.com>","From":"Naushir Patuck ","Date":"Fri, 5 Apr 2024 10:56:58 +0100","Message-ID":"","Subject":"Re: [PATCH 3/4] ipa: libipa: pwl: Clean up Pwl class to match\n\tlibcamera","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29158,"web_url":"https://patchwork.libcamera.org/comment/29158/","msgid":"","date":"2024-04-05T14:37:06","subject":"Re: [PATCH 3/4] ipa: libipa: pwl: Clean up Pwl class to match\n\tlibcamera","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Naush,\n\nOn Fri, Apr 05, 2024 at 10:56:58AM +0100, Naushir Patuck wrote:\n> Hi Paul,\n> \n> On Fri, 5 Apr 2024 at 09:03, Paul Elder wrote:\n> >\n> > Clean up the Pwl class copied from the Raspberry Pi IPA to align it more\n> > with the libcamera style.\n> >\n> > Signed-off-by: Paul Elder \n> > ---\n> > src/ipa/libipa/pwl.cpp | 135 +++++++++++++++++++++++++++++++++--------\n> > src/ipa/libipa/pwl.h | 113 ++++++++++++++--------------------\n> > 2 files changed, 154 insertions(+), 94 deletions(-)\n> >\n> > diff --git a/src/ipa/libipa/pwl.cpp b/src/ipa/libipa/pwl.cpp\n> > index 09f5d65c..58925d83 100644\n> > --- a/src/ipa/libipa/pwl.cpp\n> > +++ b/src/ipa/libipa/pwl.cpp\n> > @@ -5,13 +5,40 @@\n> > * pwl.cpp - piecewise linear functions\n> > */\n> >\n> > +#include \"pwl.h\"\n> > +\n> > #include \n> > #include \n> > +#include \n> > #include \n> >\n> > -#include \"pwl.h\"\n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa {\n> >\n> > -int Pwl::read(const libcamera::YamlObject ¶ms)\n> > +/*\n> > + * \\enum Pwl::PerpType\n> > + * \\brief Type of perpendicular found when inverting a piecewise linear function\n> > + *\n> > + * \\var None\n> > + * \\brief no perpendicular found\n> > + *\n> > + * \\var Start\n> > + * \\brief start of Pwl is closest point\n> > + *\n> > + * \\var End\n> > + * \\brief end of Pwl is closest point\n> > + *\n> > + * \\var Vertex\n> > + * \\brief vertex of Pwl is closest point\n> > + *\n> > + * \\var Perpendicular\n> > + * \\brief true perpendicular found\n> > + */\n> > +\n> > +int Pwl::readYaml(const libcamera::YamlObject ¶ms)\n> > {\n> > if (!params.size() || params.size() % 2)\n> > return -EINVAL;\n> > @@ -29,7 +56,7 @@ int Pwl::read(const libcamera::YamlObject ¶ms)\n> > if (!y)\n> > return -EINVAL;\n> >\n> > - points_.push_back(Point(*x, *y));\n> > + points_.push_back(FPoint(*x, *y));\n> > }\n> >\n> > return 0;\n> > @@ -38,13 +65,13 @@ int Pwl::read(const libcamera::YamlObject ¶ms)\n> > void Pwl::append(double x, double y, const double eps)\n> > {\n> > if (points_.empty() || points_.back().x + eps < x)\n> > - points_.push_back(Point(x, y));\n> > + points_.push_back(FPoint(x, y));\n> > }\n> >\n> > void Pwl::prepend(double x, double y, const double eps)\n> > {\n> > if (points_.empty() || points_.front().x - eps > x)\n> > - points_.insert(points_.begin(), Point(x, y));\n> > + points_.insert(points_.begin(), FPoint(x, y));\n> > }\n> >\n> > Pwl::Interval Pwl::domain() const\n> > @@ -65,6 +92,19 @@ bool Pwl::empty() const\n> > return points_.empty();\n> > }\n> >\n> > +/*\n> > + * \\brief Evaluate the piecewise linear function\n> > + * \\param[in] x The x value to input into the function\n> > + * \\param[inout] spanPtr Initial guess for span\n> > + * \\param[in] updateSpan Set to true to update spanPtr\n> > + *\n> > + * Evaluate Pwl, optionally supplying an initial guess for the\n> > + * \"span\". The \"span\" may be optionally be updated. If you want to know\n> > + * the \"span\" value but don't have an initial guess you can set it to\n> > + * -1.\n> > + *\n> > + * \\return The result of evaluating the piecewise linear function with input \\a x\n> > + */\n> > double Pwl::eval(double x, int *spanPtr, bool updateSpan) const\n> > {\n> > int span = findSpan(x, spanPtr && *spanPtr != -1 ? *spanPtr : points_.size() / 2 - 1);\n> > @@ -94,16 +134,22 @@ int Pwl::findSpan(double x, int span) const\n> > return span;\n> > }\n> >\n> > -Pwl::PerpType Pwl::invert(Point const &xy, Point &perp, int &span,\n> > +/*\n> > + * Find perpendicular closest to xy, starting from span+1 so you can\n> > + * call it repeatedly to check for multiple closest points (set span to\n> > + * -1 on the first call). Also returns \"pseudo\" perpendiculars; see\n> > + * PerpType enum.\n> > + */\n> > +Pwl::PerpType Pwl::invert(FPoint const &xy, FPoint &perp, int &span,\n> > const double eps) const\n> > {\n> > assert(span >= -1);\n> > bool prevOffEnd = false;\n> > for (span = span + 1; span < (int)points_.size() - 1; span++) {\n> > - Point spanVec = points_[span + 1] - points_[span];\n> > + FPoint spanVec = points_[span + 1] - points_[span];\n> > double t = ((xy - points_[span]) % spanVec) / spanVec.len2();\n> > - if (t < -eps) /* off the start of this span */\n> > - {\n> > + if (t < -eps) {\n> > + /* off the start of this span */\n> > if (span == 0) {\n> > perp = points_[span];\n> > return PerpType::Start;\n> > @@ -111,15 +157,15 @@ Pwl::PerpType Pwl::invert(Point const &xy, Point &perp, int &span,\n> > perp = points_[span];\n> > return PerpType::Vertex;\n> > }\n> > - } else if (t > 1 + eps) /* off the end of this span */\n> > - {\n> > + } else if (t > 1 + eps) {\n> > + /* off the end of this span */\n> > if (span == (int)points_.size() - 2) {\n> > perp = points_[span + 1];\n> > return PerpType::End;\n> > }\n> > prevOffEnd = true;\n> > - } else /* a true perpendicular */\n> > - {\n> > + } else {\n> > + /* a true perpendicular */\n> > perp = points_[span] + spanVec * t;\n> > return PerpType::Perpendicular;\n> > }\n> > @@ -127,25 +173,34 @@ Pwl::PerpType Pwl::invert(Point const &xy, Point &perp, int &span,\n> > return PerpType::None;\n> > }\n> >\n> > +/*\n> > + * \\brief Compute the inverse function\n> > + * \\param[out] trueInverse True of the resulting inverse is a proper/true inverse\n> > + * \\param[in] eps Epsilon (optional)\n> > + * Indicate if it is a proper (true) inverse, or only a best effort (e.g.\n> > + * input was non-monotonic).\n> > + * \\return The inverse piecewise linear function\n> > + */\n> > Pwl Pwl::inverse(bool *trueInverse, const double eps) const\n> > {\n> > bool appended = false, prepended = false, neither = false;\n> > Pwl inverse;\n> >\n> > - for (Point const &p : points_) {\n> > - if (inverse.empty())\n> > + for (FPoint const &p : points_) {\n> > + if (inverse.empty()) {\n> > inverse.append(p.y, p.x, eps);\n> > - else if (std::abs(inverse.points_.back().x - p.y) <= eps ||\n> > - std::abs(inverse.points_.front().x - p.y) <= eps)\n> > + } else if (std::abs(inverse.points_.back().x - p.y) <= eps ||\n> > + std::abs(inverse.points_.front().x - p.y) <= eps) {\n> > /* do nothing */;\n> > - else if (p.y > inverse.points_.back().x) {\n> > + } else if (p.y > inverse.points_.back().x) {\n> > inverse.append(p.y, p.x, eps);\n> > appended = true;\n> > } else if (p.y < inverse.points_.front().x) {\n> > inverse.prepend(p.y, p.x, eps);\n> > prepended = true;\n> > - } else\n> > + } else {\n> > neither = true;\n> > + }\n> > }\n> >\n> > /*\n> > @@ -159,18 +214,25 @@ Pwl Pwl::inverse(bool *trueInverse, const double eps) const\n> > return inverse;\n> > }\n> >\n> > +/*\n> > + * \\brief Compose two piecewise linear functions together\n> > + * \\param[in] other The \"other\" piecewise linear function\n> > + * \\param[in] eps Epsilon (optiona)\n> > + * The \"this\" function is done first, and \"other\" after.\n> > + * \\return The composed piecewise linear function\n> > + */\n> > Pwl Pwl::compose(Pwl const &other, const double eps) const\n> > {\n> > double thisX = points_[0].x, thisY = points_[0].y;\n> > int thisSpan = 0, otherSpan = other.findSpan(thisY, 0);\n> > Pwl result({ { thisX, other.eval(thisY, &otherSpan, false) } });\n> > +\n> > while (thisSpan != (int)points_.size() - 1) {\n> > double dx = points_[thisSpan + 1].x - points_[thisSpan].x,\n> > dy = points_[thisSpan + 1].y - points_[thisSpan].y;\n> > if (std::abs(dy) > eps &&\n> > otherSpan + 1 < (int)other.points_.size() &&\n> > - points_[thisSpan + 1].y >=\n> > - other.points_[otherSpan + 1].x + eps) {\n> > + points_[thisSpan + 1].y >= other.points_[otherSpan + 1].x + eps) {\n> > /*\n> > * next control point in result will be where this\n> > * function's y reaches the next span in other\n> > @@ -204,18 +266,24 @@ Pwl Pwl::compose(Pwl const &other, const double eps) const\n> > return result;\n> > }\n> >\n> > +/* \\brief Apply function to (x,y) values at every control point. */\n> > void Pwl::map(std::function f) const\n> > {\n> > for (auto &pt : points_)\n> > f(pt.x, pt.y);\n> > }\n> >\n> > +/*\n> > + * \\brief Apply function to (x, y0, y1) values wherever either Pwl has a\n> > + * control point.\n> > + */\n> > void Pwl::map2(Pwl const &pwl0, Pwl const &pwl1,\n> > std::function f)\n> > {\n> > int span0 = 0, span1 = 0;\n> > double x = std::min(pwl0.points_[0].x, pwl1.points_[0].x);\n> > f(x, pwl0.eval(x, &span0, false), pwl1.eval(x, &span1, false));\n> > +\n> > while (span0 < (int)pwl0.points_.size() - 1 ||\n> > span1 < (int)pwl1.points_.size() - 1) {\n> > if (span0 == (int)pwl0.points_.size() - 1)\n> > @@ -230,6 +298,12 @@ void Pwl::map2(Pwl const &pwl0, Pwl const &pwl1,\n> > }\n> > }\n> >\n> > +/*\n> > + * \\brief Combine two Pwls\n> > + *\n> > + * Create a new Pwl where the y values are given by running f wherever either\n> > + * has a knot.\n> > + */\n> > Pwl Pwl::combine(Pwl const &pwl0, Pwl const &pwl1,\n> > std::function f,\n> > const double eps)\n> > @@ -241,6 +315,11 @@ Pwl Pwl::combine(Pwl const &pwl0, Pwl const &pwl1,\n> > return result;\n> > }\n> >\n> > +/*\n> > + * \\brief Make \"this\" match (at least) the given domain.\n> > + *\n> > + * Any extension my be clipped or linear.\n> > + */\n> > void Pwl::matchDomain(Interval const &domain, bool clip, const double eps)\n> > {\n> > int span = 0;\n> > @@ -258,10 +337,16 @@ Pwl &Pwl::operator*=(double d)\n> > return *this;\n> > }\n> >\n> > -void Pwl::debug(FILE *fp) const\n> > +std::string Pwl::toString() const\n> > {\n> > - fprintf(fp, \"Pwl {\\n\");\n> > + std::stringstream ss;\n> > + ss << \"Pwl { \";\n> > for (auto &p : points_)\n> > - fprintf(fp, \"\\t(%g, %g)\\n\", p.x, p.y);\n> > - fprintf(fp, \"}\\n\");\n> > + ss << \"(\" << p.x << \", \" << p.y << \") \";\n> > + ss << \"}\";\n> > + return ss.str();\n> > }\n> > +\n> > +} /* namespace ipa */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/libipa/pwl.h b/src/ipa/libipa/pwl.h\n> > index 7a6a6452..ef49e302 100644\n> > --- a/src/ipa/libipa/pwl.h\n> > +++ b/src/ipa/libipa/pwl.h\n> > @@ -8,116 +8,91 @@\n> >\n> > #include \n> > #include \n> > +#include \n> > #include \n> >\n> > +#include \n> > +\n> > #include \"libcamera/internal/yaml_parser.h\"\n> >\n> > +namespace libcamera {\n> > +\n> > +namespace ipa {\n> > +\n> > class Pwl\n> > {\n> > public:\n> > + enum class PerpType {\n> > + None,\n> > + Start,\n> > + End,\n> > + Vertex,\n> > + Perpendicular,\n> > + };\n> > +\n> > struct Interval {\n> > Interval(double _start, double _end)\n> > - : start(_start), end(_end)\n> > - {\n> > - }\n> > - double start, end;\n> > + : start(_start), end(_end) {}\n> > +\n> > bool contains(double value)\n> > {\n> > return value >= start && value <= end;\n> > }\n> > - double clip(double value)\n> > +\n> > + double clamp(double value)\n> \n> Typically clamp implies a min/max range value. Since we are only\n> using a singular value here, IMO this should still be called clip.\n\nI was under the impression that the interval itself made up the min and max\nrange values, which is why I changed it to clamp. I started seeing clip\nused elsewhere too though and I suppose it's not that big of a deal so\nI'll change it back to clip (except I have patches coming in a few\nminutes that depend on this so they'll still have clamp).\n\n\nThanks,\n\nPaul\n\n> \n> \n> > {\n> > return value < start ? start\n> > : (value > end ? end : value);\n> > }\n> > +\n> > double len() const { return end - start; }\n> > +\n> > + double start, end;\n> > };\n> > - struct Point {\n> > - Point() : x(0), y(0) {}\n> > - Point(double _x, double _y)\n> > - : x(_x), y(_y) {}\n> > - double x, y;\n> > - Point operator-(Point const &p) const\n> > - {\n> > - return Point(x - p.x, y - p.y);\n> > - }\n> > - Point operator+(Point const &p) const\n> > - {\n> > - return Point(x + p.x, y + p.y);\n> > - }\n> > - double operator%(Point const &p) const\n> > - {\n> > - return x * p.x + y * p.y;\n> > - }\n> > - Point operator*(double f) const { return Point(x * f, y * f); }\n> > - Point operator/(double f) const { return Point(x / f, y / f); }\n> > - double len2() const { return x * x + y * y; }\n> > - double len() const { return sqrt(len2()); }\n> > - };\n> > +\n> > Pwl() {}\n> > - Pwl(std::vector const &points) : points_(points) {}\n> > - int read(const libcamera::YamlObject ¶ms);\n> > + Pwl(std::vector const &points)\n> > + : points_(points) {}\n> > + int readYaml(const libcamera::YamlObject ¶ms);\n> > +\n> > void append(double x, double y, const double eps = 1e-6);\n> > void prepend(double x, double y, const double eps = 1e-6);\n> > +\n> > Interval domain() const;\n> > Interval range() const;\n> > +\n> > bool empty() const;\n> > - /*\n> > - * Evaluate Pwl, optionally supplying an initial guess for the\n> > - * \"span\". The \"span\" may be optionally be updated. If you want to know\n> > - * the \"span\" value but don't have an initial guess you can set it to\n> > - * -1.\n> > - */\n> > +\n> > double eval(double x, int *spanPtr = nullptr,\n> > bool updateSpan = true) const;\n> > - /*\n> > - * Find perpendicular closest to xy, starting from span+1 so you can\n> > - * call it repeatedly to check for multiple closest points (set span to\n> > - * -1 on the first call). Also returns \"pseudo\" perpendiculars; see\n> > - * PerpType enum.\n> > - */\n> > - enum class PerpType {\n> > - None, /* no perpendicular found */\n> > - Start, /* start of Pwl is closest point */\n> > - End, /* end of Pwl is closest point */\n> > - Vertex, /* vertex of Pwl is closest point */\n> > - Perpendicular /* true perpendicular found */\n> > - };\n> > - PerpType invert(Point const &xy, Point &perp, int &span,\n> > +\n> > + PerpType invert(FPoint const &xy, FPoint &perp, int &span,\n> > const double eps = 1e-6) const;\n> > - /*\n> > - * Compute the inverse function. Indicate if it is a proper (true)\n> > - * inverse, or only a best effort (e.g. input was non-monotonic).\n> > - */\n> > Pwl inverse(bool *trueInverse = nullptr, const double eps = 1e-6) const;\n> > - /* Compose two Pwls together, doing \"this\" first and \"other\" after. */\n> > Pwl compose(Pwl const &other, const double eps = 1e-6) const;\n> > - /* Apply function to (x,y) values at every control point. */\n> > +\n> > void map(std::function f) const;\n> > - /*\n> > - * Apply function to (x, y0, y1) values wherever either Pwl has a\n> > - * control point.\n> > - */\n> > +\n> > static void map2(Pwl const &pwl0, Pwl const &pwl1,\n> > std::function f);\n> > - /*\n> > - * Combine two Pwls, meaning we create a new Pwl where the y values are\n> > - * given by running f wherever either has a knot.\n> > - */\n> > +\n> > static Pwl\n> > combine(Pwl const &pwl0, Pwl const &pwl1,\n> > std::function f,\n> > const double eps = 1e-6);\n> > - /*\n> > - * Make \"this\" match (at least) the given domain. Any extension my be\n> > - * clipped or linear.\n> > - */\n> > +\n> > void matchDomain(Interval const &domain, bool clip = true,\n> > const double eps = 1e-6);\n> > +\n> > Pwl &operator*=(double d);\n> > - void debug(FILE *fp = stdout) const;\n> > +\n> > + std::string toString() const;\n> >\n> > private:\n> > int findSpan(double x, int span) const;\n> > - std::vector points_;\n> > + std::vector points_;\n> > };\n> > +\n> > +} /* namespace ipa */\n> > +\n> > +} /* namespace libcamera */\n> > --\n> > 2.39.2\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A85FDBD16B\n\tfor ;\n\tFri, 5 Apr 2024 14:37:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A552961C2F;\n\tFri, 5 Apr 2024 16:37:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4997461C29\n\tfor ;\n\tFri, 5 Apr 2024 16:37:16 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4CD7E22A;\n\tFri, 5 Apr 2024 16:36:36 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RGloG4Or\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1712327797;\n\tbh=0roD2vVjKX0bimLevGDiZfJEUE3x7Pc2qRdReq9M5QQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RGloG4OrKyMlCZ3OGIySrQKhUV/882d8Nn2PzPKsKCHL5uYJkC8sJ6RkqoyaZ0cGQ\n\tsXyI5nhMe4mutXUivL3pA1QYmu2FzD5Jj2qqLuBO7FTk7MqS0yHH/J3R/TGXF3BfcB\n\t3/L0c9LTh/EQNkymNsa2XZoLitd+7DPNXe3uW3Rc=","Date":"Fri, 5 Apr 2024 23:37:06 +0900","From":"Paul Elder ","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 3/4] ipa: libipa: pwl: Clean up Pwl class to match\n\tlibcamera","Message-ID":"","References":"<20240405080259.1806453-1-paul.elder@ideasonboard.com>\n\t<20240405080259.1806453-4-paul.elder@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29159,"web_url":"https://patchwork.libcamera.org/comment/29159/","msgid":"","date":"2024-04-05T15:09:06","subject":"Re: [PATCH 3/4] ipa: libipa: pwl: Clean up Pwl class to match\n\tlibcamera","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Paul,\n\nOn Fri, 5 Apr 2024 at 15:37, Paul Elder wrote:\n>\n> Hi Naush,\n>\n> On Fri, Apr 05, 2024 at 10:56:58AM +0100, Naushir Patuck wrote:\n> > Hi Paul,\n> >\n> > On Fri, 5 Apr 2024 at 09:03, Paul Elder wrote:\n> > >\n> > > Clean up the Pwl class copied from the Raspberry Pi IPA to align it more\n> > > with the libcamera style.\n> > >\n> > > Signed-off-by: Paul Elder \n> > > ---\n> > > src/ipa/libipa/pwl.cpp | 135 +++++++++++++++++++++++++++++++++--------\n> > > src/ipa/libipa/pwl.h | 113 ++++++++++++++--------------------\n> > > 2 files changed, 154 insertions(+), 94 deletions(-)\n> > >\n> > > diff --git a/src/ipa/libipa/pwl.cpp b/src/ipa/libipa/pwl.cpp\n> > > index 09f5d65c..58925d83 100644\n> > > --- a/src/ipa/libipa/pwl.cpp\n> > > +++ b/src/ipa/libipa/pwl.cpp\n> > > @@ -5,13 +5,40 @@\n> > > * pwl.cpp - piecewise linear functions\n> > > */\n> > >\n> > > +#include \"pwl.h\"\n> > > +\n> > > #include \n> > > #include \n> > > +#include \n> > > #include \n> > >\n> > > -#include \"pwl.h\"\n> > > +#include \n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +namespace ipa {\n> > >\n> > > -int Pwl::read(const libcamera::YamlObject ¶ms)\n> > > +/*\n> > > + * \\enum Pwl::PerpType\n> > > + * \\brief Type of perpendicular found when inverting a piecewise linear function\n> > > + *\n> > > + * \\var None\n> > > + * \\brief no perpendicular found\n> > > + *\n> > > + * \\var Start\n> > > + * \\brief start of Pwl is closest point\n> > > + *\n> > > + * \\var End\n> > > + * \\brief end of Pwl is closest point\n> > > + *\n> > > + * \\var Vertex\n> > > + * \\brief vertex of Pwl is closest point\n> > > + *\n> > > + * \\var Perpendicular\n> > > + * \\brief true perpendicular found\n> > > + */\n> > > +\n> > > +int Pwl::readYaml(const libcamera::YamlObject ¶ms)\n> > > {\n> > > if (!params.size() || params.size() % 2)\n> > > return -EINVAL;\n> > > @@ -29,7 +56,7 @@ int Pwl::read(const libcamera::YamlObject ¶ms)\n> > > if (!y)\n> > > return -EINVAL;\n> > >\n> > > - points_.push_back(Point(*x, *y));\n> > > + points_.push_back(FPoint(*x, *y));\n> > > }\n> > >\n> > > return 0;\n> > > @@ -38,13 +65,13 @@ int Pwl::read(const libcamera::YamlObject ¶ms)\n> > > void Pwl::append(double x, double y, const double eps)\n> > > {\n> > > if (points_.empty() || points_.back().x + eps < x)\n> > > - points_.push_back(Point(x, y));\n> > > + points_.push_back(FPoint(x, y));\n> > > }\n> > >\n> > > void Pwl::prepend(double x, double y, const double eps)\n> > > {\n> > > if (points_.empty() || points_.front().x - eps > x)\n> > > - points_.insert(points_.begin(), Point(x, y));\n> > > + points_.insert(points_.begin(), FPoint(x, y));\n> > > }\n> > >\n> > > Pwl::Interval Pwl::domain() const\n> > > @@ -65,6 +92,19 @@ bool Pwl::empty() const\n> > > return points_.empty();\n> > > }\n> > >\n> > > +/*\n> > > + * \\brief Evaluate the piecewise linear function\n> > > + * \\param[in] x The x value to input into the function\n> > > + * \\param[inout] spanPtr Initial guess for span\n> > > + * \\param[in] updateSpan Set to true to update spanPtr\n> > > + *\n> > > + * Evaluate Pwl, optionally supplying an initial guess for the\n> > > + * \"span\". The \"span\" may be optionally be updated. If you want to know\n> > > + * the \"span\" value but don't have an initial guess you can set it to\n> > > + * -1.\n> > > + *\n> > > + * \\return The result of evaluating the piecewise linear function with input \\a x\n> > > + */\n> > > double Pwl::eval(double x, int *spanPtr, bool updateSpan) const\n> > > {\n> > > int span = findSpan(x, spanPtr && *spanPtr != -1 ? *spanPtr : points_.size() / 2 - 1);\n> > > @@ -94,16 +134,22 @@ int Pwl::findSpan(double x, int span) const\n> > > return span;\n> > > }\n> > >\n> > > -Pwl::PerpType Pwl::invert(Point const &xy, Point &perp, int &span,\n> > > +/*\n> > > + * Find perpendicular closest to xy, starting from span+1 so you can\n> > > + * call it repeatedly to check for multiple closest points (set span to\n> > > + * -1 on the first call). Also returns \"pseudo\" perpendiculars; see\n> > > + * PerpType enum.\n> > > + */\n> > > +Pwl::PerpType Pwl::invert(FPoint const &xy, FPoint &perp, int &span,\n> > > const double eps) const\n> > > {\n> > > assert(span >= -1);\n> > > bool prevOffEnd = false;\n> > > for (span = span + 1; span < (int)points_.size() - 1; span++) {\n> > > - Point spanVec = points_[span + 1] - points_[span];\n> > > + FPoint spanVec = points_[span + 1] - points_[span];\n> > > double t = ((xy - points_[span]) % spanVec) / spanVec.len2();\n> > > - if (t < -eps) /* off the start of this span */\n> > > - {\n> > > + if (t < -eps) {\n> > > + /* off the start of this span */\n> > > if (span == 0) {\n> > > perp = points_[span];\n> > > return PerpType::Start;\n> > > @@ -111,15 +157,15 @@ Pwl::PerpType Pwl::invert(Point const &xy, Point &perp, int &span,\n> > > perp = points_[span];\n> > > return PerpType::Vertex;\n> > > }\n> > > - } else if (t > 1 + eps) /* off the end of this span */\n> > > - {\n> > > + } else if (t > 1 + eps) {\n> > > + /* off the end of this span */\n> > > if (span == (int)points_.size() - 2) {\n> > > perp = points_[span + 1];\n> > > return PerpType::End;\n> > > }\n> > > prevOffEnd = true;\n> > > - } else /* a true perpendicular */\n> > > - {\n> > > + } else {\n> > > + /* a true perpendicular */\n> > > perp = points_[span] + spanVec * t;\n> > > return PerpType::Perpendicular;\n> > > }\n> > > @@ -127,25 +173,34 @@ Pwl::PerpType Pwl::invert(Point const &xy, Point &perp, int &span,\n> > > return PerpType::None;\n> > > }\n> > >\n> > > +/*\n> > > + * \\brief Compute the inverse function\n> > > + * \\param[out] trueInverse True of the resulting inverse is a proper/true inverse\n> > > + * \\param[in] eps Epsilon (optional)\n> > > + * Indicate if it is a proper (true) inverse, or only a best effort (e.g.\n> > > + * input was non-monotonic).\n> > > + * \\return The inverse piecewise linear function\n> > > + */\n> > > Pwl Pwl::inverse(bool *trueInverse, const double eps) const\n> > > {\n> > > bool appended = false, prepended = false, neither = false;\n> > > Pwl inverse;\n> > >\n> > > - for (Point const &p : points_) {\n> > > - if (inverse.empty())\n> > > + for (FPoint const &p : points_) {\n> > > + if (inverse.empty()) {\n> > > inverse.append(p.y, p.x, eps);\n> > > - else if (std::abs(inverse.points_.back().x - p.y) <= eps ||\n> > > - std::abs(inverse.points_.front().x - p.y) <= eps)\n> > > + } else if (std::abs(inverse.points_.back().x - p.y) <= eps ||\n> > > + std::abs(inverse.points_.front().x - p.y) <= eps) {\n> > > /* do nothing */;\n> > > - else if (p.y > inverse.points_.back().x) {\n> > > + } else if (p.y > inverse.points_.back().x) {\n> > > inverse.append(p.y, p.x, eps);\n> > > appended = true;\n> > > } else if (p.y < inverse.points_.front().x) {\n> > > inverse.prepend(p.y, p.x, eps);\n> > > prepended = true;\n> > > - } else\n> > > + } else {\n> > > neither = true;\n> > > + }\n> > > }\n> > >\n> > > /*\n> > > @@ -159,18 +214,25 @@ Pwl Pwl::inverse(bool *trueInverse, const double eps) const\n> > > return inverse;\n> > > }\n> > >\n> > > +/*\n> > > + * \\brief Compose two piecewise linear functions together\n> > > + * \\param[in] other The \"other\" piecewise linear function\n> > > + * \\param[in] eps Epsilon (optiona)\n> > > + * The \"this\" function is done first, and \"other\" after.\n> > > + * \\return The composed piecewise linear function\n> > > + */\n> > > Pwl Pwl::compose(Pwl const &other, const double eps) const\n> > > {\n> > > double thisX = points_[0].x, thisY = points_[0].y;\n> > > int thisSpan = 0, otherSpan = other.findSpan(thisY, 0);\n> > > Pwl result({ { thisX, other.eval(thisY, &otherSpan, false) } });\n> > > +\n> > > while (thisSpan != (int)points_.size() - 1) {\n> > > double dx = points_[thisSpan + 1].x - points_[thisSpan].x,\n> > > dy = points_[thisSpan + 1].y - points_[thisSpan].y;\n> > > if (std::abs(dy) > eps &&\n> > > otherSpan + 1 < (int)other.points_.size() &&\n> > > - points_[thisSpan + 1].y >=\n> > > - other.points_[otherSpan + 1].x + eps) {\n> > > + points_[thisSpan + 1].y >= other.points_[otherSpan + 1].x + eps) {\n> > > /*\n> > > * next control point in result will be where this\n> > > * function's y reaches the next span in other\n> > > @@ -204,18 +266,24 @@ Pwl Pwl::compose(Pwl const &other, const double eps) const\n> > > return result;\n> > > }\n> > >\n> > > +/* \\brief Apply function to (x,y) values at every control point. */\n> > > void Pwl::map(std::function f) const\n> > > {\n> > > for (auto &pt : points_)\n> > > f(pt.x, pt.y);\n> > > }\n> > >\n> > > +/*\n> > > + * \\brief Apply function to (x, y0, y1) values wherever either Pwl has a\n> > > + * control point.\n> > > + */\n> > > void Pwl::map2(Pwl const &pwl0, Pwl const &pwl1,\n> > > std::function f)\n> > > {\n> > > int span0 = 0, span1 = 0;\n> > > double x = std::min(pwl0.points_[0].x, pwl1.points_[0].x);\n> > > f(x, pwl0.eval(x, &span0, false), pwl1.eval(x, &span1, false));\n> > > +\n> > > while (span0 < (int)pwl0.points_.size() - 1 ||\n> > > span1 < (int)pwl1.points_.size() - 1) {\n> > > if (span0 == (int)pwl0.points_.size() - 1)\n> > > @@ -230,6 +298,12 @@ void Pwl::map2(Pwl const &pwl0, Pwl const &pwl1,\n> > > }\n> > > }\n> > >\n> > > +/*\n> > > + * \\brief Combine two Pwls\n> > > + *\n> > > + * Create a new Pwl where the y values are given by running f wherever either\n> > > + * has a knot.\n> > > + */\n> > > Pwl Pwl::combine(Pwl const &pwl0, Pwl const &pwl1,\n> > > std::function f,\n> > > const double eps)\n> > > @@ -241,6 +315,11 @@ Pwl Pwl::combine(Pwl const &pwl0, Pwl const &pwl1,\n> > > return result;\n> > > }\n> > >\n> > > +/*\n> > > + * \\brief Make \"this\" match (at least) the given domain.\n> > > + *\n> > > + * Any extension my be clipped or linear.\n> > > + */\n> > > void Pwl::matchDomain(Interval const &domain, bool clip, const double eps)\n> > > {\n> > > int span = 0;\n> > > @@ -258,10 +337,16 @@ Pwl &Pwl::operator*=(double d)\n> > > return *this;\n> > > }\n> > >\n> > > -void Pwl::debug(FILE *fp) const\n> > > +std::string Pwl::toString() const\n> > > {\n> > > - fprintf(fp, \"Pwl {\\n\");\n> > > + std::stringstream ss;\n> > > + ss << \"Pwl { \";\n> > > for (auto &p : points_)\n> > > - fprintf(fp, \"\\t(%g, %g)\\n\", p.x, p.y);\n> > > - fprintf(fp, \"}\\n\");\n> > > + ss << \"(\" << p.x << \", \" << p.y << \") \";\n> > > + ss << \"}\";\n> > > + return ss.str();\n> > > }\n> > > +\n> > > +} /* namespace ipa */\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/ipa/libipa/pwl.h b/src/ipa/libipa/pwl.h\n> > > index 7a6a6452..ef49e302 100644\n> > > --- a/src/ipa/libipa/pwl.h\n> > > +++ b/src/ipa/libipa/pwl.h\n> > > @@ -8,116 +8,91 @@\n> > >\n> > > #include \n> > > #include \n> > > +#include \n> > > #include \n> > >\n> > > +#include \n> > > +\n> > > #include \"libcamera/internal/yaml_parser.h\"\n> > >\n> > > +namespace libcamera {\n> > > +\n> > > +namespace ipa {\n> > > +\n> > > class Pwl\n> > > {\n> > > public:\n> > > + enum class PerpType {\n> > > + None,\n> > > + Start,\n> > > + End,\n> > > + Vertex,\n> > > + Perpendicular,\n> > > + };\n> > > +\n> > > struct Interval {\n> > > Interval(double _start, double _end)\n> > > - : start(_start), end(_end)\n> > > - {\n> > > - }\n> > > - double start, end;\n> > > + : start(_start), end(_end) {}\n> > > +\n> > > bool contains(double value)\n> > > {\n> > > return value >= start && value <= end;\n> > > }\n> > > - double clip(double value)\n> > > +\n> > > + double clamp(double value)\n> >\n> > Typically clamp implies a min/max range value. Since we are only\n> > using a singular value here, IMO this should still be called clip.\n>\n> I was under the impression that the interval itself made up the min and max\n> range values, which is why I changed it to clamp. I started seeing clip\n> used elsewhere too though and I suppose it's not that big of a deal so\n> I'll change it back to clip (except I have patches coming in a few\n> minutes that depend on this so they'll still have clamp).\n\nYou are right, this is a clamp operation, I just didn't read the code\nright. I'd be happy to change the clip to clamp like you've already\ndone.\n\nRegards,\nNaush\n\n>\n>\n> Thanks,\n>\n> Paul\n>\n> >\n> >\n> > > {\n> > > return value < start ? start\n> > > : (value > end ? end : value);\n> > > }\n> > > +\n> > > double len() const { return end - start; }\n> > > +\n> > > + double start, end;\n> > > };\n> > > - struct Point {\n> > > - Point() : x(0), y(0) {}\n> > > - Point(double _x, double _y)\n> > > - : x(_x), y(_y) {}\n> > > - double x, y;\n> > > - Point operator-(Point const &p) const\n> > > - {\n> > > - return Point(x - p.x, y - p.y);\n> > > - }\n> > > - Point operator+(Point const &p) const\n> > > - {\n> > > - return Point(x + p.x, y + p.y);\n> > > - }\n> > > - double operator%(Point const &p) const\n> > > - {\n> > > - return x * p.x + y * p.y;\n> > > - }\n> > > - Point operator*(double f) const { return Point(x * f, y * f); }\n> > > - Point operator/(double f) const { return Point(x / f, y / f); }\n> > > - double len2() const { return x * x + y * y; }\n> > > - double len() const { return sqrt(len2()); }\n> > > - };\n> > > +\n> > > Pwl() {}\n> > > - Pwl(std::vector const &points) : points_(points) {}\n> > > - int read(const libcamera::YamlObject ¶ms);\n> > > + Pwl(std::vector const &points)\n> > > + : points_(points) {}\n> > > + int readYaml(const libcamera::YamlObject ¶ms);\n> > > +\n> > > void append(double x, double y, const double eps = 1e-6);\n> > > void prepend(double x, double y, const double eps = 1e-6);\n> > > +\n> > > Interval domain() const;\n> > > Interval range() const;\n> > > +\n> > > bool empty() const;\n> > > - /*\n> > > - * Evaluate Pwl, optionally supplying an initial guess for the\n> > > - * \"span\". The \"span\" may be optionally be updated. If you want to know\n> > > - * the \"span\" value but don't have an initial guess you can set it to\n> > > - * -1.\n> > > - */\n> > > +\n> > > double eval(double x, int *spanPtr = nullptr,\n> > > bool updateSpan = true) const;\n> > > - /*\n> > > - * Find perpendicular closest to xy, starting from span+1 so you can\n> > > - * call it repeatedly to check for multiple closest points (set span to\n> > > - * -1 on the first call). Also returns \"pseudo\" perpendiculars; see\n> > > - * PerpType enum.\n> > > - */\n> > > - enum class PerpType {\n> > > - None, /* no perpendicular found */\n> > > - Start, /* start of Pwl is closest point */\n> > > - End, /* end of Pwl is closest point */\n> > > - Vertex, /* vertex of Pwl is closest point */\n> > > - Perpendicular /* true perpendicular found */\n> > > - };\n> > > - PerpType invert(Point const &xy, Point &perp, int &span,\n> > > +\n> > > + PerpType invert(FPoint const &xy, FPoint &perp, int &span,\n> > > const double eps = 1e-6) const;\n> > > - /*\n> > > - * Compute the inverse function. Indicate if it is a proper (true)\n> > > - * inverse, or only a best effort (e.g. input was non-monotonic).\n> > > - */\n> > > Pwl inverse(bool *trueInverse = nullptr, const double eps = 1e-6) const;\n> > > - /* Compose two Pwls together, doing \"this\" first and \"other\" after. */\n> > > Pwl compose(Pwl const &other, const double eps = 1e-6) const;\n> > > - /* Apply function to (x,y) values at every control point. */\n> > > +\n> > > void map(std::function f) const;\n> > > - /*\n> > > - * Apply function to (x, y0, y1) values wherever either Pwl has a\n> > > - * control point.\n> > > - */\n> > > +\n> > > static void map2(Pwl const &pwl0, Pwl const &pwl1,\n> > > std::function f);\n> > > - /*\n> > > - * Combine two Pwls, meaning we create a new Pwl where the y values are\n> > > - * given by running f wherever either has a knot.\n> > > - */\n> > > +\n> > > static Pwl\n> > > combine(Pwl const &pwl0, Pwl const &pwl1,\n> > > std::function f,\n> > > const double eps = 1e-6);\n> > > - /*\n> > > - * Make \"this\" match (at least) the given domain. Any extension my be\n> > > - * clipped or linear.\n> > > - */\n> > > +\n> > > void matchDomain(Interval const &domain, bool clip = true,\n> > > const double eps = 1e-6);\n> > > +\n> > > Pwl &operator*=(double d);\n> > > - void debug(FILE *fp = stdout) const;\n> > > +\n> > > + std::string toString() const;\n> > >\n> > > private:\n> > > int findSpan(double x, int span) const;\n> > > - std::vector points_;\n> > > + std::vector points_;\n> > > };\n> > > +\n> > > +} /* namespace ipa */\n> > > +\n> > > +} /* namespace libcamera */\n> > > --\n> > > 2.39.2\n> > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E7E3DBD16B\n\tfor ;\n\tFri, 5 Apr 2024 15:09:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0E8C861C33;\n\tFri, 5 Apr 2024 17:09:47 +0200 (CEST)","from mail-yb1-xb30.google.com (mail-yb1-xb30.google.com\n\t[IPv6:2607:f8b0:4864:20::b30])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 062DB61C29\n\tfor ;\n\tFri, 5 Apr 2024 17:09:44 +0200 (CEST)","by mail-yb1-xb30.google.com with SMTP id\n\t3f1490d57ef6-dc23bf7e5aaso2517107276.0\n\tfor ;\n\tFri, 05 Apr 2024 08:09:44 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"MzZ5krQ6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1712329784; x=1712934584;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=NmsIm7JXmOquygdtBYwh3+CqQthKrOzz6UBeoAF9/Zc=;\n\tb=MzZ5krQ6zzjwRzLeEMK1sRUjEXcmVw+/wI+oA+K/NYNUNPYildHtCwjeCC5KCJkhND\n\tQo4aeXgQKAb4F69m/gv7XYjJVPlwCfyWzvzY/nZ+w8mZscgzN0N+h/0mrct+aIJONfsQ\n\tjMgmXx1nur89/9M1mhSMAywooUTjbxs97MPgesUJfOAqCMIjd+R1npHK988APxzcZyns\n\t6X9fXQqQQlaEGugo+ozs/O3in7g74MoKSxjxBecN0CwtvrGwUYIHB7RtivVM+16xQ57r\n\tRNFdjZUNrWvpRoSK8qPWy7L1PCDJI03OtBbZ36jIpP67orPiwUwvlkzgJkh5M0QfqZ2e\n\t03lA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1712329784; x=1712934584;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=NmsIm7JXmOquygdtBYwh3+CqQthKrOzz6UBeoAF9/Zc=;\n\tb=S8VfCHu9WE++Btk5YmbO5JHOlOPG1xPPBeImAGGtWNQ52gnZPI2BUkCNOMf9UKgqZH\n\tcoCuEXzwVqhbISy29x6z3G37IWKbo7ML6GWBseBXUmPI6twOqXhwaNnA9njlnz0Mx3GE\n\t8bcKCB/XRtJurtEGiEK2ZpL7JzKFjRwXafBKaax9ctAAcljF5AFSgkg4NAhR8bd/GXRU\n\toLmZ4nZVcB69HNGmLxR2z1mvwPDiU/5YpPq8eolLaVCncGPes2FGLv5YCdq7+DzgJqXr\n\tcF4R1CEdxRyW1HZb8vbz/WuJk6UjAUq4+hrvvzF01blbmTA6qkQG/avM7f8VD0slM6aj\n\tT9ew==","X-Gm-Message-State":"AOJu0YzTD6U5d3EeUOK3Pd/8gBvvk74RE8IZaKhrfMW1u/RP70gaAcms\n\t7iYnC+KMNmJA15SyhHomRbA63RXCh5/6zaFigb/Z5YsGJgpgpuBijSm78QVCvqsebg6eb0Zmg04\n\t2TUSa12m8q2z/zSNTd7GgmDpEQAv3IsMCO27qww==","X-Google-Smtp-Source":"AGHT+IE0o2tFe5DKPT9EffBQvD/8RB2bjQKmveu7Y95tyA/ZkiPllt3thU0pR62PW0ocbhPU2sb2RePAlYbAoDMHNGg=","X-Received":"by 2002:a25:ac58:0:b0:dc6:c510:df6b with SMTP id\n\tr24-20020a25ac58000000b00dc6c510df6bmr1617180ybd.55.1712329783540;\n\tFri, 05 Apr 2024 08:09:43 -0700 (PDT)","MIME-Version":"1.0","References":"<20240405080259.1806453-1-paul.elder@ideasonboard.com>\n\t<20240405080259.1806453-4-paul.elder@ideasonboard.com>\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Fri, 5 Apr 2024 16:09:06 +0100","Message-ID":"","Subject":"Re: [PATCH 3/4] ipa: libipa: pwl: Clean up Pwl class to match\n\tlibcamera","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29226,"web_url":"https://patchwork.libcamera.org/comment/29226/","msgid":"<20240415124631.7f34tjtb2yu5yxze@jasper>","date":"2024-04-15T12:46:31","subject":"Re: [PATCH 3/4] ipa: libipa: pwl: Clean up Pwl class to match\n\tlibcamera","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Paul,\n\nthanks for the patch.\n\nOn Fri, Apr 05, 2024 at 05:02:58PM +0900, Paul Elder wrote:\n> Clean up the Pwl class copied from the Raspberry Pi IPA to align it more\n> with the libcamera style.\n> \n> Signed-off-by: Paul Elder \n> ---\n> src/ipa/libipa/pwl.cpp | 135 +++++++++++++++++++++++++++++++++--------\n> src/ipa/libipa/pwl.h | 113 ++++++++++++++--------------------\n> 2 files changed, 154 insertions(+), 94 deletions(-)\n> \n> diff --git a/src/ipa/libipa/pwl.cpp b/src/ipa/libipa/pwl.cpp\n> index 09f5d65c..58925d83 100644\n> --- a/src/ipa/libipa/pwl.cpp\n> +++ b/src/ipa/libipa/pwl.cpp\n> @@ -5,13 +5,40 @@\n> * pwl.cpp - piecewise linear functions\n\nIf we are serious with the copyright, this would be the chance to add\nIdeas on Board :-)\n\n> */\n> \n> +#include \"pwl.h\"\n> +\n> #include \n> #include \n> +#include \n> #include \n> \n> -#include \"pwl.h\"\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +namespace ipa {\n> \n> -int Pwl::read(const libcamera::YamlObject ¶ms)\n> +/*\n> + * \\enum Pwl::PerpType\n> + * \\brief Type of perpendicular found when inverting a piecewise linear function\n> + *\n> + * \\var None\n> + * \\brief no perpendicular found\n> + *\n> + * \\var Start\n> + * \\brief start of Pwl is closest point\n> + *\n> + * \\var End\n> + * \\brief end of Pwl is closest point\n> + *\n> + * \\var Vertex\n> + * \\brief vertex of Pwl is closest point\n> + *\n> + * \\var Perpendicular\n> + * \\brief true perpendicular found\n> + */\n> +\n> +int Pwl::readYaml(const libcamera::YamlObject ¶ms)\n> {\n> \tif (!params.size() || params.size() % 2)\n> \t\treturn -EINVAL;\n> @@ -29,7 +56,7 @@ int Pwl::read(const libcamera::YamlObject ¶ms)\n> \t\tif (!y)\n> \t\t\treturn -EINVAL;\n> \n> -\t\tpoints_.push_back(Point(*x, *y));\n> +\t\tpoints_.push_back(FPoint(*x, *y));\n> \t}\n> \n> \treturn 0;\n> @@ -38,13 +65,13 @@ int Pwl::read(const libcamera::YamlObject ¶ms)\n> void Pwl::append(double x, double y, const double eps)\n> {\n> \tif (points_.empty() || points_.back().x + eps < x)\n> -\t\tpoints_.push_back(Point(x, y));\n> +\t\tpoints_.push_back(FPoint(x, y));\n> }\n> \n> void Pwl::prepend(double x, double y, const double eps)\n> {\n> \tif (points_.empty() || points_.front().x - eps > x)\n> -\t\tpoints_.insert(points_.begin(), Point(x, y));\n> +\t\tpoints_.insert(points_.begin(), FPoint(x, y));\n> }\n> \n> Pwl::Interval Pwl::domain() const\n> @@ -65,6 +92,19 @@ bool Pwl::empty() const\n> \treturn points_.empty();\n> }\n> \n> +/*\n> + * \\brief Evaluate the piecewise linear function\n> + * \\param[in] x The x value to input into the function\n> + * \\param[inout] spanPtr Initial guess for span\n> + * \\param[in] updateSpan Set to true to update spanPtr\n> + *\n> + * Evaluate Pwl, optionally supplying an initial guess for the\n> + * \"span\". The \"span\" may be optionally be updated. If you want to know\n> + * the \"span\" value but don't have an initial guess you can set it to\n> + * -1.\n> + *\n> + * \\return The result of evaluating the piecewise linear function with input \\a x\n\nQuestion to the native speakers: How would you phrase that? To me \"...\nevaluate pwl at position x ...\" sound more natural.\n\n> + */\n> double Pwl::eval(double x, int *spanPtr, bool updateSpan) const\n> {\n> \tint span = findSpan(x, spanPtr && *spanPtr != -1 ? *spanPtr : points_.size() / 2 - 1);\n> @@ -94,16 +134,22 @@ int Pwl::findSpan(double x, int span) const\n> \treturn span;\n> }\n> \n> -Pwl::PerpType Pwl::invert(Point const &xy, Point &perp, int &span,\n> +/*\n\nDocs for the params is missing.\n\n> + * Find perpendicular closest to xy, starting from span+1 so you can\n> + * call it repeatedly to check for multiple closest points (set span to\n> + * -1 on the first call). Also returns \"pseudo\" perpendiculars; see\n> + * PerpType enum.\n> + */\n> +Pwl::PerpType Pwl::invert(FPoint const &xy, FPoint &perp, int &span,\n> \t\t\t const double eps) const\n> {\n> \tassert(span >= -1);\n> \tbool prevOffEnd = false;\n> \tfor (span = span + 1; span < (int)points_.size() - 1; span++) {\n> -\t\tPoint spanVec = points_[span + 1] - points_[span];\n> +\t\tFPoint spanVec = points_[span + 1] - points_[span];\n> \t\tdouble t = ((xy - points_[span]) % spanVec) / spanVec.len2();\n> -\t\tif (t < -eps) /* off the start of this span */\n> -\t\t{\n> +\t\tif (t < -eps) {\n> +\t\t\t/* off the start of this span */\n> \t\t\tif (span == 0) {\n> \t\t\t\tperp = points_[span];\n> \t\t\t\treturn PerpType::Start;\n> @@ -111,15 +157,15 @@ Pwl::PerpType Pwl::invert(Point const &xy, Point &perp, int &span,\n> \t\t\t\tperp = points_[span];\n> \t\t\t\treturn PerpType::Vertex;\n> \t\t\t}\n> -\t\t} else if (t > 1 + eps) /* off the end of this span */\n> -\t\t{\n> +\t\t} else if (t > 1 + eps) {\n> +\t\t\t/* off the end of this span */\n> \t\t\tif (span == (int)points_.size() - 2) {\n> \t\t\t\tperp = points_[span + 1];\n> \t\t\t\treturn PerpType::End;\n> \t\t\t}\n> \t\t\tprevOffEnd = true;\n> -\t\t} else /* a true perpendicular */\n> -\t\t{\n> +\t\t} else {\n> +\t\t\t/* a true perpendicular */\n> \t\t\tperp = points_[span] + spanVec * t;\n> \t\t\treturn PerpType::Perpendicular;\n> \t\t}\n> @@ -127,25 +173,34 @@ Pwl::PerpType Pwl::invert(Point const &xy, Point &perp, int &span,\n> \treturn PerpType::None;\n> }\n> \n> +/*\n> + * \\brief Compute the inverse function\n> + * \\param[out] trueInverse True of the resulting inverse is a proper/true inverse\n\ns/of/if/ , maybe s/resulting inverse/result/\n\n> + * \\param[in] eps Epsilon (optional)\n> + * Indicate if it is a proper (true) inverse, or only a best effort (e.g.\n> + * input was non-monotonic).\n\nI believe there should be empty lines around the description.\n\n> + * \\return The inverse piecewise linear function\n> + */\n> Pwl Pwl::inverse(bool *trueInverse, const double eps) const\n> {\n> \tbool appended = false, prepended = false, neither = false;\n> \tPwl inverse;\n> \n> -\tfor (Point const &p : points_) {\n> -\t\tif (inverse.empty())\n> +\tfor (FPoint const &p : points_) {\n> +\t\tif (inverse.empty()) {\n> \t\t\tinverse.append(p.y, p.x, eps);\n> -\t\telse if (std::abs(inverse.points_.back().x - p.y) <= eps ||\n> -\t\t\t std::abs(inverse.points_.front().x - p.y) <= eps)\n> +\t\t} else if (std::abs(inverse.points_.back().x - p.y) <= eps ||\n> +\t\t\t std::abs(inverse.points_.front().x - p.y) <= eps) {\n> \t\t\t/* do nothing */;\n> -\t\telse if (p.y > inverse.points_.back().x) {\n> +\t\t} else if (p.y > inverse.points_.back().x) {\n> \t\t\tinverse.append(p.y, p.x, eps);\n> \t\t\tappended = true;\n> \t\t} else if (p.y < inverse.points_.front().x) {\n> \t\t\tinverse.prepend(p.y, p.x, eps);\n> \t\t\tprepended = true;\n> -\t\t} else\n> +\t\t} else {\n> \t\t\tneither = true;\n> +\t\t}\n> \t}\n> \n> \t/*\n> @@ -159,18 +214,25 @@ Pwl Pwl::inverse(bool *trueInverse, const double eps) const\n> \treturn inverse;\n> }\n> \n> +/*\n> + * \\brief Compose two piecewise linear functions together\n> + * \\param[in] other The \"other\" piecewise linear function\n> + * \\param[in] eps Epsilon (optiona)\n\nempty line\n\n> + * The \"this\" function is done first, and \"other\" after.\n\nempty line\n\n> + * \\return The composed piecewise linear function\n> + */\n> Pwl Pwl::compose(Pwl const &other, const double eps) const\n> {\n> \tdouble thisX = points_[0].x, thisY = points_[0].y;\n> \tint thisSpan = 0, otherSpan = other.findSpan(thisY, 0);\n> \tPwl result({ { thisX, other.eval(thisY, &otherSpan, false) } });\n> +\n> \twhile (thisSpan != (int)points_.size() - 1) {\n> \t\tdouble dx = points_[thisSpan + 1].x - points_[thisSpan].x,\n> \t\t dy = points_[thisSpan + 1].y - points_[thisSpan].y;\n> \t\tif (std::abs(dy) > eps &&\n> \t\t otherSpan + 1 < (int)other.points_.size() &&\n> -\t\t points_[thisSpan + 1].y >=\n> -\t\t\t other.points_[otherSpan + 1].x + eps) {\n> +\t\t points_[thisSpan + 1].y >= other.points_[otherSpan + 1].x + eps) {\n> \t\t\t/*\n> \t\t\t * next control point in result will be where this\n> \t\t\t * function's y reaches the next span in other\n> @@ -204,18 +266,24 @@ Pwl Pwl::compose(Pwl const &other, const double eps) const\n> \treturn result;\n> }\n> \n> +/* \\brief Apply function to (x,y) values at every control point. */\n> void Pwl::map(std::function f) const\n> {\n> \tfor (auto &pt : points_)\n> \t\tf(pt.x, pt.y);\n> }\n> \n> +/*\n> + * \\brief Apply function to (x, y0, y1) values wherever either Pwl has a\n> + * control point.\n> + */\n> void Pwl::map2(Pwl const &pwl0, Pwl const &pwl1,\n> \t std::function f)\n> {\n> \tint span0 = 0, span1 = 0;\n> \tdouble x = std::min(pwl0.points_[0].x, pwl1.points_[0].x);\n> \tf(x, pwl0.eval(x, &span0, false), pwl1.eval(x, &span1, false));\n> +\n> \twhile (span0 < (int)pwl0.points_.size() - 1 ||\n> \t span1 < (int)pwl1.points_.size() - 1) {\n> \t\tif (span0 == (int)pwl0.points_.size() - 1)\n> @@ -230,6 +298,12 @@ void Pwl::map2(Pwl const &pwl0, Pwl const &pwl1,\n> \t}\n> }\n> \n> +/*\n> + * \\brief Combine two Pwls\n> + *\n> + * Create a new Pwl where the y values are given by running f wherever either\n> + * has a knot.\n> + */\n> Pwl Pwl::combine(Pwl const &pwl0, Pwl const &pwl1,\n> \t\t std::function f,\n> \t\t const double eps)\n> @@ -241,6 +315,11 @@ Pwl Pwl::combine(Pwl const &pwl0, Pwl const &pwl1,\n> \treturn result;\n> }\n> \n> +/*\n> + * \\brief Make \"this\" match (at least) the given domain.\n> + *\n\nWithout looking at the source, I would not understand what this function does.\n\n> + * Any extension my be clipped or linear.\n\ns/my/may/ \n\n> + */\n> void Pwl::matchDomain(Interval const &domain, bool clip, const double eps)\n> {\n> \tint span = 0;\n> @@ -258,10 +337,16 @@ Pwl &Pwl::operator*=(double d)\n> \treturn *this;\n> }\n> \n> -void Pwl::debug(FILE *fp) const\n> +std::string Pwl::toString() const\n\nIn similar cases we overload the << operator. Is there any particular\nreason not to do that here?\n\n> {\n> -\tfprintf(fp, \"Pwl {\\n\");\n> +\tstd::stringstream ss;\n> +\tss << \"Pwl { \";\n> \tfor (auto &p : points_)\n> -\t\tfprintf(fp, \"\\t(%g, %g)\\n\", p.x, p.y);\n> -\tfprintf(fp, \"}\\n\");\n> +\t\tss << \"(\" << p.x << \", \" << p.y << \") \";\n> +\tss << \"}\";\n> +\treturn ss.str();\n> }\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/pwl.h b/src/ipa/libipa/pwl.h\n> index 7a6a6452..ef49e302 100644\n> --- a/src/ipa/libipa/pwl.h\n> +++ b/src/ipa/libipa/pwl.h\n> @@ -8,116 +8,91 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> \n> +#include \n> +\n> #include \"libcamera/internal/yaml_parser.h\"\n> \n> +namespace libcamera {\n> +\n> +namespace ipa {\n> +\n> class Pwl\n> {\n> public:\n> +\tenum class PerpType {\n> +\t\tNone,\n> +\t\tStart,\n> +\t\tEnd,\n> +\t\tVertex,\n> +\t\tPerpendicular,\n> +\t};\n> +\n> \tstruct Interval {\n> \t\tInterval(double _start, double _end)\n> -\t\t\t: start(_start), end(_end)\n> -\t\t{\n> -\t\t}\n> -\t\tdouble start, end;\n> +\t\t\t: start(_start), end(_end) {}\n> +\n> \t\tbool contains(double value)\n> \t\t{\n> \t\t\treturn value >= start && value <= end;\n> \t\t}\n> -\t\tdouble clip(double value)\n> +\n> +\t\tdouble clamp(double value)\n> \t\t{\n> \t\t\treturn value < start ? start\n> \t\t\t\t\t : (value > end ? end : value);\n> \t\t}\n> +\n> \t\tdouble len() const { return end - start; }\n> +\n> +\t\tdouble start, end;\n> \t};\n> -\tstruct Point {\n> -\t\tPoint() : x(0), y(0) {}\n> -\t\tPoint(double _x, double _y)\n> -\t\t\t: x(_x), y(_y) {}\n> -\t\tdouble x, y;\n> -\t\tPoint operator-(Point const &p) const\n> -\t\t{\n> -\t\t\treturn Point(x - p.x, y - p.y);\n> -\t\t}\n> -\t\tPoint operator+(Point const &p) const\n> -\t\t{\n> -\t\t\treturn Point(x + p.x, y + p.y);\n> -\t\t}\n> -\t\tdouble operator%(Point const &p) const\n> -\t\t{\n> -\t\t\treturn x * p.x + y * p.y;\n> -\t\t}\n> -\t\tPoint operator*(double f) const { return Point(x * f, y * f); }\n> -\t\tPoint operator/(double f) const { return Point(x / f, y / f); }\n> -\t\tdouble len2() const { return x * x + y * y; }\n> -\t\tdouble len() const { return sqrt(len2()); }\n> -\t};\n> +\n> \tPwl() {}\n> -\tPwl(std::vector const &points) : points_(points) {}\n> -\tint read(const libcamera::YamlObject ¶ms);\n> +\tPwl(std::vector const &points)\n> +\t\t: points_(points) {}\n> +\tint readYaml(const libcamera::YamlObject ¶ms);\n> +\n> \tvoid append(double x, double y, const double eps = 1e-6);\n> \tvoid prepend(double x, double y, const double eps = 1e-6);\n> +\n> \tInterval domain() const;\n> \tInterval range() const;\n> +\n> \tbool empty() const;\n> -\t/*\n> -\t * Evaluate Pwl, optionally supplying an initial guess for the\n> -\t * \"span\". The \"span\" may be optionally be updated. If you want to know\n> -\t * the \"span\" value but don't have an initial guess you can set it to\n> -\t * -1.\n> -\t */\n> +\n> \tdouble eval(double x, int *spanPtr = nullptr,\n> \t\t bool updateSpan = true) const;\n> -\t/*\n> -\t * Find perpendicular closest to xy, starting from span+1 so you can\n> -\t * call it repeatedly to check for multiple closest points (set span to\n> -\t * -1 on the first call). Also returns \"pseudo\" perpendiculars; see\n> -\t * PerpType enum.\n> -\t */\n> -\tenum class PerpType {\n> -\t\tNone, /* no perpendicular found */\n> -\t\tStart, /* start of Pwl is closest point */\n> -\t\tEnd, /* end of Pwl is closest point */\n> -\t\tVertex, /* vertex of Pwl is closest point */\n> -\t\tPerpendicular /* true perpendicular found */\n> -\t};\n> -\tPerpType invert(Point const &xy, Point &perp, int &span,\n> +\n> +\tPerpType invert(FPoint const &xy, FPoint &perp, int &span,\n> \t\t\tconst double eps = 1e-6) const;\n> -\t/*\n> -\t * Compute the inverse function. Indicate if it is a proper (true)\n> -\t * inverse, or only a best effort (e.g. input was non-monotonic).\n> -\t */\n> \tPwl inverse(bool *trueInverse = nullptr, const double eps = 1e-6) const;\n> -\t/* Compose two Pwls together, doing \"this\" first and \"other\" after. */\n> \tPwl compose(Pwl const &other, const double eps = 1e-6) const;\n> -\t/* Apply function to (x,y) values at every control point. */\n> +\n> \tvoid map(std::function f) const;\n> -\t/*\n> -\t * Apply function to (x, y0, y1) values wherever either Pwl has a\n> -\t * control point.\n> -\t */\n> +\n> \tstatic void map2(Pwl const &pwl0, Pwl const &pwl1,\n> \t\t\t std::function f);\n> -\t/*\n> -\t * Combine two Pwls, meaning we create a new Pwl where the y values are\n> -\t * given by running f wherever either has a knot.\n> -\t */\n> +\n> \tstatic Pwl\n> \tcombine(Pwl const &pwl0, Pwl const &pwl1,\n> \t\tstd::function f,\n> \t\tconst double eps = 1e-6);\n> -\t/*\n> -\t * Make \"this\" match (at least) the given domain. Any extension my be\n> -\t * clipped or linear.\n> -\t */\n> +\n> \tvoid matchDomain(Interval const &domain, bool clip = true,\n> \t\t\t const double eps = 1e-6);\n> +\n> \tPwl &operator*=(double d);\n> -\tvoid debug(FILE *fp = stdout) const;\n> +\n> +\tstd::string toString() const;\n> \n> private:\n> \tint findSpan(double x, int span) const;\n> -\tstd::vector points_;\n> +\tstd::vector points_;\n> };\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n\n\nI feel bad to say it, but a few initial tests would be gerat, as it\nlands in public API :-)\n\nCheers,\nStefan\n\n> -- \n> 2.39.2\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4DDDDBE08B\n\tfor ;\n\tMon, 15 Apr 2024 12:46:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7510763352;\n\tMon, 15 Apr 2024 14:46:35 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F075563339\n\tfor ;\n\tMon, 15 Apr 2024 14:46:33 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7a0d:dd2e:881a:db83])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 496047E4;\n\tMon, 15 Apr 2024 14:45:48 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Kw+0kCmY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713185148;\n\tbh=g6kzfq0i9HWYg3h76DAlpaZY24C07+pmH9WP35TfwZs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Kw+0kCmY/Mdqq7fycjAiGXlTYGN7qrla/7YpZBrEjj9uNTt638O20ENg/fSeIfv6v\n\tcrJWi+EdTRkXm2M2OPNFdWrAChtnsLcd0QGnnvk56xuxvJWodoDHyej+YOnxh1thVE\n\tCwawHAOo0dOt3UDHPPuSljsZiCHC+VjQppE5jG54=","Date":"Mon, 15 Apr 2024 14:46:31 +0200","From":"Stefan Klug ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 3/4] ipa: libipa: pwl: Clean up Pwl class to match\n\tlibcamera","Message-ID":"<20240415124631.7f34tjtb2yu5yxze@jasper>","References":"<20240405080259.1806453-1-paul.elder@ideasonboard.com>\n\t<20240405080259.1806453-4-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240405080259.1806453-4-paul.elder@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]