[{"id":27591,"web_url":"https://patchwork.libcamera.org/comment/27591/","msgid":"","date":"2023-07-20T10:06:54","subject":"Re: [libcamera-devel] [PATCH v3 08/10] test: Add unit test for\n\tTransform and Orientation","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for these tests.\n\nOn Tue, 18 Jul 2023 at 11:52, Jacopo Mondi\n wrote:\n>\n> Add a unit test for Transform and Orientation to validate the\n> implementation of the operations between the two types.\n>\n> In particular, test that:\n>\n> o1 / o2 = t\n> o2 * t = o1\n>\n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/transform.h | 7 +\n> test/meson.build | 1 +\n> test/transform.cpp | 335 ++++++++++++++++++++++++++++++++++\n> 3 files changed, 343 insertions(+)\n> create mode 100644 test/transform.cpp\n>\n> diff --git a/include/libcamera/transform.h b/include/libcamera/transform.h\n> index 847a1f94713c..833b50d46fd0 100644\n> --- a/include/libcamera/transform.h\n> +++ b/include/libcamera/transform.h\n> @@ -16,13 +16,20 @@ enum class Orientation;\n> enum class Transform : int {\n> Identity = 0,\n> Rot0 = Identity,\n> +\n> HFlip = 1,\n> +\n> VFlip = 2,\n> +\n> HVFlip = HFlip | VFlip,\n> Rot180 = HVFlip,\n> +\n> Transpose = 4,\n> +\n> Rot270 = HFlip | Transpose,\n> +\n> Rot90 = VFlip | Transpose,\n> +\n\nDidn't I see another commit removing these extra blank lines again?\nNot that I mind...!\n\n> Rot180Transpose = HFlip | VFlip | Transpose\n> };\n>\n> diff --git a/test/meson.build b/test/meson.build\n> index b227be818419..189e1428485a 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -46,6 +46,7 @@ public_tests = [\n> {'name': 'public-api', 'sources': ['public-api.cpp']},\n> {'name': 'signal', 'sources': ['signal.cpp']},\n> {'name': 'span', 'sources': ['span.cpp']},\n> + {'name': 'transform', 'sources': ['transform.cpp']},\n> ]\n>\n> internal_tests = [\n> diff --git a/test/transform.cpp b/test/transform.cpp\n> new file mode 100644\n> index 000000000000..f79a1a892d16\n> --- /dev/null\n> +++ b/test/transform.cpp\n> @@ -0,0 +1,335 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2023, Ideas On Board Oy\n> + *\n> + * transform.cpp - Transform and Orientation tests\n> + */\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"test.h\"\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +class TransformTest : public Test\n> +{\n> +protected:\n> + int run();\n> +};\n> +\n> +int TransformTest::run()\n> +{\n> + /*\n> + * RotationTestEntry collects two Orientation and one Transform that\n> + * gets combined to validate that (o1 / o2 = T) and (o1 = o2 * T)\n> + *\n> + * o1 / o2 = t computes the Transform to apply to o2 to obtain o1\n> + * o2 * t = o1 combines o2 with t by applying o2 first then t\n> + *\n> + * The comments on the (most complex) transform show how applying to\n> + * an image with orientation o2 the Transform t allows to obtain o1.\n> + *\n> + * The image with basic rotation0 is assumed to be:\n> + *\n> + * AB\n> + * CD\n> + *\n> + * And the Transform operators are:\n> + *\n> + * V = vertical flip\n> + * H = horizontal flip\n> + * T = transpose\n> + *\n> + * the operator '* (T|V)' applies V first then T.\n\nShould we mention H here as well? Again, this is so minor I'm not at\nall bothered...!\n\n> + */\n> + struct RotationTestEntry {\n> + Orientation o1;\n> + Orientation o2;\n> + Transform t;\n> + } testEntries[] = {\n> + /* Test identities transforms first. */\n> + {\n> + Orientation::rotate0, Orientation::rotate0,\n> + Transform::Identity,\n> + },\n> + {\n> + Orientation::rotate0Flip, Orientation::rotate0Flip,\n> + Transform::Identity,\n> + },\n> + {\n> + Orientation::rotate180, Orientation::rotate180,\n> + Transform::Identity,\n> + },\n> + {\n> + Orientation::rotate180Flip, Orientation::rotate180Flip,\n> + Transform::Identity,\n> + },\n> + {\n> + Orientation::rotate90, Orientation::rotate90,\n> + Transform::Identity,\n> + },\n> + {\n> + Orientation::rotate90Flip, Orientation::rotate90Flip,\n> + Transform::Identity,\n> + },\n> + {\n> + Orientation::rotate270, Orientation::rotate270,\n> + Transform::Identity,\n> + },\n> + {\n> + Orientation::rotate270Flip, Orientation::rotate270Flip,\n> + Transform::Identity,\n> + },\n> + /*\n> + * Combine 0 and 180 degrees rotation as they're the most common\n> + * ones.\n> + */\n> + {\n> + /*\n> + * o2 t o1\n> + * --------------------------\n> + * CD * (H|V) = BA AB\n> + * BA CD CD\n> + */\n> + Orientation::rotate0, Orientation::rotate180,\n> + Transform::Rot180,\n> + },\n> + {\n> + Orientation::rotate180, Orientation::rotate180,\n> + Transform::Identity\n\nDidn't we do this one already? Not that there's any harm in doing it again!\n\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * --------------------------\n> + * AB * (H|V) = CD DC\n> + * CD AB BA\n> + */\n> + Orientation::rotate180, Orientation::rotate0,\n> + Transform::Rot180\n> + },\n> + /* Test that transpositions are handled correctly. */\n> + {\n> + /*\n> + * o2 t o1\n> + * --------------------------\n> + * AB * (T|V) = CD CA\n> + * CD AB DB\n> + */\n> + Orientation::rotate90, Orientation::rotate0,\n> + Transform::Rot90,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * --------------------------\n> + * CA * (T|H) = AC AB\n> + * DB BD CD\n> + */\n> + Orientation::rotate0, Orientation::rotate90,\n> + Transform::Rot270,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * --------------------------\n> + * AB * (T|H) = BA BD\n> + * CD DC AC\n> + */\n> + Orientation::rotate270, Orientation::rotate0,\n> + Transform::Rot270,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * --------------------------\n> + * BD * (T|V) = AC AB\n> + * AC BD CD\n> + */\n> + Orientation::rotate0, Orientation::rotate270,\n> + Transform::Rot90,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * --------------------------\n> + * CD * (T|H) = DC DA\n> + * BA AB CB\n> + */\n> + Orientation::rotate90, Orientation::rotate180,\n> + Transform::Rot270,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * --------------------------\n> + * DA * (T|V) = CB CD\n> + * CB DA BA\n> + */\n> + Orientation::rotate180, Orientation::rotate90,\n> + Transform::Rot90,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * --------------------------\n> + * CD * (T|V) = BA BC\n> + * BA CD AD\n> + */\n> + Orientation::rotate270, Orientation::rotate180,\n> + Transform::Rot90,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * --------------------------\n> + * BC * (T|H) = CB CD\n> + * AD DA BA\n> + */\n> + Orientation::rotate180, Orientation::rotate270,\n> + Transform::Rot270,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * --------------------------\n> + * DA * (V|H) = AD BC\n> + * CB BC AD\n> + */\n> + Orientation::rotate270, Orientation::rotate90,\n> + Transform::Rot180,\n> + },\n> + /* Test that mirroring is handled correctly. */\n> + {\n> + Orientation::rotate0, Orientation::rotate0Flip,\n> + Transform::HFlip\n> + },\n> + {\n> + Orientation::rotate0Flip, Orientation::rotate0,\n> + Transform::HFlip\n> + },\n> + {\n> + Orientation::rotate180, Orientation::rotate180Flip,\n> + Transform::HFlip\n> + },\n> + {\n> + Orientation::rotate180Flip, Orientation::rotate180,\n> + Transform::HFlip\n> + },\n> + {\n> + Orientation::rotate90, Orientation::rotate90Flip,\n> + Transform::HFlip\n> + },\n> + {\n> + Orientation::rotate90Flip, Orientation::rotate90,\n> + Transform::HFlip\n> + },\n> + {\n> + Orientation::rotate270, Orientation::rotate270Flip,\n> + Transform::HFlip\n> + },\n> + {\n> + Orientation::rotate270Flip, Orientation::rotate270,\n> + Transform::HFlip\n> + },\n> + {\n> + Orientation::rotate0, Orientation::rotate0Flip,\n> + Transform::HFlip\n> + },\n> + /*\n> + * More exotic transforms which include Transpositions and\n> + * mirroring.\n> + */\n> + {\n> + /*\n> + * o2 t o1\n> + * ------------------\n> + * BC * (V) = AD\n> + * AD BC\n> + */\n> + Orientation::rotate90Flip, Orientation::rotate270,\n> + Transform::VFlip,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * ------------------\n> + * CB * (T) = CD\n> + * DA BA\n> + */\n> + Orientation::rotate180, Orientation::rotate270Flip,\n> + Transform::Transpose,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * ------------------\n> + * AD * (T) = AB\n> + * BC DC\n> + */\n> + Orientation::rotate0, Orientation::rotate90Flip,\n> + Transform::Transpose,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * ------------------\n> + * AD * (V) = BC\n> + * BC AD\n> + */\n> + Orientation::rotate270, Orientation::rotate90Flip,\n> + Transform::VFlip,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * ------------------\n> + * DA * (V) = CB\n> + * CB DA\n> + */\n> + Orientation::rotate270Flip, Orientation::rotate90,\n> + Transform::VFlip,\n> + },\n> + {\n> + /*\n> + * o2 t o1\n> + * --------------------------\n> + * CB * (V|H) = BC AD\n> + * DA AD BC\n> + */\n> + Orientation::rotate90Flip, Orientation::rotate270Flip,\n> + Transform::Rot180,\n> + },\n> + };\n> +\n> + for (const auto &entry : testEntries) {\n> + Transform transform = entry.o1 / entry.o2;\n> + cout << \"Testing: \" << entry.o1 << \" / \" << entry.o2\n> + << \" = \" << transformToString(transform) << endl;\n> + if (transform != entry.t) {\n> + cerr << \"Failed to validate: \" << entry.o1\n> + << \" / \" << entry.o2\n> + << \" = \" << transformToString(entry.t) << endl;\n> + cerr << \"Got back: \"\n> + << transformToString(transform) << endl;\n> + return TestFail;\n> + }\n> +\n> + Orientation adjusted = entry.o2 * entry.t;\n> + if (adjusted != entry.o1) {\n> + cerr << \"Failed to validate: \" << entry.o2\n> + << \" * \" << transformToString(entry.t)\n> + << \" = \" << entry.o1 << endl;\n> + cerr << \"Got back: \" << adjusted << endl;\n> + return TestFail;\n> + }\n> + }\n\nAll looks good to me, I think there's probably plenty of coverage here\nso that you'd notice if anything got broken.\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> +\n> + return TestPass;\n> +}\n> +\n> +TEST_REGISTER(TransformTest)\n> --\n> 2.40.1\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 D3966BDC71\n\tfor ;\n\tThu, 20 Jul 2023 10:07:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2763C628C0;\n\tThu, 20 Jul 2023 12:07:08 +0200 (CEST)","from mail-qv1-xf2f.google.com (mail-qv1-xf2f.google.com\n\t[IPv6:2607:f8b0:4864:20::f2f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9B30D61E2A\n\tfor ;\n\tThu, 20 Jul 2023 12:07:06 +0200 (CEST)","by mail-qv1-xf2f.google.com with SMTP id\n\t6a1803df08f44-636801fada1so4778306d6.3\n\tfor ;\n\tThu, 20 Jul 2023 03:07:06 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1689847628;\n\tbh=hMxOqERVuMhApdCjg6FdM40p1SHJroSyB6Dfpbf2oYg=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=3OoC13Q3cawGDT45t+DsgzUarnTcRzEgqUfkcZVUHdzN9iOykJEwPWjk1okksYV3J\n\tuzaU3iethDvD4kBPWA97rvVsWjeVyeyc9vlmpRMn8kErEZrpGBzdqtDtwd41zCOJxr\n\tQIrz5CjxE9gSH69SYlOuoJqapAQPIjrf2FvaqnJjw5M2fcd3oLfiGqB2nTg0Gfxh1Q\n\tURr8R7k7kNgQoCRkBTrqNQ7GiEXxEaXtI8yQOD0OA4rAw7uyy8UWey/yWHYDcT6210\n\tWD22BNzzhgd4e8sQMMXoq46oAtNHfEG8TacbHSzop5D7/cjE5YBA47lLoyPcC2Fl+c\n\tVDI/f2cZG5jhQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1689847625; x=1690452425;\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=g1jn4Zt0HAlnDhSaaOWrQ4CDm4YGMCIpmYlfXB2KF0E=;\n\tb=JfARHN6uU5rwYZwEiulL3yp+XT/+zgN9quv+ALCcShKifnEj4cuNg9P3GwgGxWvxtk\n\tSslNe14kq/mOYOGtJcJ2A8pWmMh7Yrn3ec9VEX67T7MGnLRW4h+tVOkpnnEnTbIrv5qM\n\tKARt0NzHRZgMvBktUYT0GyCQIumCFP6WiqKFIR8Gkhg45B5OzVqwmnuoMkl7wb8LEREN\n\tBieeVnRee6R8JxK45PT5qaCT7p6IWnbfyhciN3a+yL9J0rBOuX2ZQ5xFbfYMwX41NLvB\n\tU8uTar/7RpWQvip+hk9N2SMp0XSpi0AwUDaQQvMS1WK05PxWw4OyUiCeheg558ztoyxH\n\tD6vA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"JfARHN6u\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1689847625; x=1690452425;\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=g1jn4Zt0HAlnDhSaaOWrQ4CDm4YGMCIpmYlfXB2KF0E=;\n\tb=T03ZM64AWbF198GHGl1rcMsm4kwuBkCdAnYPo/gvsSD6/d5U/5TuRuXYHp8l4jxfnN\n\tWxpiDC3kRTNXnw84to7XC5K5y1H4bJjvtokiimVAsUWOrM+d0mc8xqi/5fJpS4VYNuRI\n\tFr29YeKbp0XtE6XApQvtVeebJlcQ7XtgMhfhZvVaA2YYgNOMWi/y4jXRB+l+yQ0y9CzL\n\tq68MG29ip7iikunQe88vYZDtetkPGGHfD0saaPOFl/hTmwRjMt2fjVRb5Zt1frYNT5K2\n\tiH96fEzrnTUediBQOPPes1PrEGFABNt44HM/4OD2XKPsIUXNgBhASdzh3rEm0dNV6xsy\n\tTSpg==","X-Gm-Message-State":"ABy/qLbk7yggIhg5YXSUSzmV+rFf9BVzuIvAlD26OaV5Qv8msty801JN\n\tHEwjUOChSy/CSA5oFro6U+09ktUWfrmOuv33lLY0AQ==","X-Google-Smtp-Source":"APBJJlEfUVTZcX9kCrf5Tt4SofpmdmmALW0NiLw7h0dnGuzDhcoAjMOdkaQMutjOMsGVrepUSqubHRYWb9y4CHAf9Q8=","X-Received":"by 2002:a0c:ab58:0:b0:630:7d0:56f4 with SMTP id\n\ti24-20020a0cab58000000b0063007d056f4mr1496697qvb.49.1689847625310;\n\tThu, 20 Jul 2023 03:07:05 -0700 (PDT)","MIME-Version":"1.0","References":"<20230718105210.83558-1-jacopo.mondi@ideasonboard.com>\n\t<20230718105210.83558-9-jacopo.mondi@ideasonboard.com>","In-Reply-To":"<20230718105210.83558-9-jacopo.mondi@ideasonboard.com>","Date":"Thu, 20 Jul 2023 11:06:54 +0100","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 08/10] test: Add unit test for\n\tTransform and Orientation","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","From":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27596,"web_url":"https://patchwork.libcamera.org/comment/27596/","msgid":"","date":"2023-07-24T06:59:21","subject":"Re: [libcamera-devel] [PATCH v3 08/10] test: Add unit test for\n\tTransform and Orientation","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi David\n\nOn Thu, Jul 20, 2023 at 11:06:54AM +0100, David Plowman via libcamera-devel wrote:\n> Hi Jacopo\n>\n> Thanks for these tests.\n>\n> On Tue, 18 Jul 2023 at 11:52, Jacopo Mondi\n> wrote:\n> >\n> > Add a unit test for Transform and Orientation to validate the\n> > implementation of the operations between the two types.\n> >\n> > In particular, test that:\n> >\n> > o1 / o2 = t\n> > o2 * t = o1\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > include/libcamera/transform.h | 7 +\n> > test/meson.build | 1 +\n> > test/transform.cpp | 335 ++++++++++++++++++++++++++++++++++\n> > 3 files changed, 343 insertions(+)\n> > create mode 100644 test/transform.cpp\n> >\n> > diff --git a/include/libcamera/transform.h b/include/libcamera/transform.h\n> > index 847a1f94713c..833b50d46fd0 100644\n> > --- a/include/libcamera/transform.h\n> > +++ b/include/libcamera/transform.h\n> > @@ -16,13 +16,20 @@ enum class Orientation;\n> > enum class Transform : int {\n> > Identity = 0,\n> > Rot0 = Identity,\n> > +\n> > HFlip = 1,\n> > +\n> > VFlip = 2,\n> > +\n> > HVFlip = HFlip | VFlip,\n> > Rot180 = HVFlip,\n> > +\n> > Transpose = 4,\n> > +\n> > Rot270 = HFlip | Transpose,\n> > +\n> > Rot90 = VFlip | Transpose,\n> > +\n>\n> Didn't I see another commit removing these extra blank lines again?\n> Not that I mind...!\n\nThis clearly shouldn't be here\n\n>\n> > Rot180Transpose = HFlip | VFlip | Transpose\n> > };\n> >\n> > diff --git a/test/meson.build b/test/meson.build\n> > index b227be818419..189e1428485a 100644\n> > --- a/test/meson.build\n> > +++ b/test/meson.build\n> > @@ -46,6 +46,7 @@ public_tests = [\n> > {'name': 'public-api', 'sources': ['public-api.cpp']},\n> > {'name': 'signal', 'sources': ['signal.cpp']},\n> > {'name': 'span', 'sources': ['span.cpp']},\n> > + {'name': 'transform', 'sources': ['transform.cpp']},\n> > ]\n> >\n> > internal_tests = [\n> > diff --git a/test/transform.cpp b/test/transform.cpp\n> > new file mode 100644\n> > index 000000000000..f79a1a892d16\n> > --- /dev/null\n> > +++ b/test/transform.cpp\n> > @@ -0,0 +1,335 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2023, Ideas On Board Oy\n> > + *\n> > + * transform.cpp - Transform and Orientation tests\n> > + */\n> > +\n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \"test.h\"\n> > +\n> > +using namespace std;\n> > +using namespace libcamera;\n> > +\n> > +class TransformTest : public Test\n> > +{\n> > +protected:\n> > + int run();\n> > +};\n> > +\n> > +int TransformTest::run()\n> > +{\n> > + /*\n> > + * RotationTestEntry collects two Orientation and one Transform that\n> > + * gets combined to validate that (o1 / o2 = T) and (o1 = o2 * T)\n> > + *\n> > + * o1 / o2 = t computes the Transform to apply to o2 to obtain o1\n> > + * o2 * t = o1 combines o2 with t by applying o2 first then t\n> > + *\n> > + * The comments on the (most complex) transform show how applying to\n> > + * an image with orientation o2 the Transform t allows to obtain o1.\n> > + *\n> > + * The image with basic rotation0 is assumed to be:\n> > + *\n> > + * AB\n> > + * CD\n> > + *\n> > + * And the Transform operators are:\n> > + *\n> > + * V = vertical flip\n> > + * H = horizontal flip\n> > + * T = transpose\n> > + *\n> > + * the operator '* (T|V)' applies V first then T.\n>\n> Should we mention H here as well? Again, this is so minor I'm not at\n> all bothered...!\n>\n\nWell, this was just an example to specify the operator application\norder\n\n> > + */\n> > + struct RotationTestEntry {\n> > + Orientation o1;\n> > + Orientation o2;\n> > + Transform t;\n> > + } testEntries[] = {\n> > + /* Test identities transforms first. */\n> > + {\n> > + Orientation::rotate0, Orientation::rotate0,\n> > + Transform::Identity,\n> > + },\n> > + {\n> > + Orientation::rotate0Flip, Orientation::rotate0Flip,\n> > + Transform::Identity,\n> > + },\n> > + {\n> > + Orientation::rotate180, Orientation::rotate180,\n> > + Transform::Identity,\n> > + },\n> > + {\n> > + Orientation::rotate180Flip, Orientation::rotate180Flip,\n> > + Transform::Identity,\n> > + },\n> > + {\n> > + Orientation::rotate90, Orientation::rotate90,\n> > + Transform::Identity,\n> > + },\n> > + {\n> > + Orientation::rotate90Flip, Orientation::rotate90Flip,\n> > + Transform::Identity,\n> > + },\n> > + {\n> > + Orientation::rotate270, Orientation::rotate270,\n> > + Transform::Identity,\n> > + },\n> > + {\n> > + Orientation::rotate270Flip, Orientation::rotate270Flip,\n> > + Transform::Identity,\n> > + },\n> > + /*\n> > + * Combine 0 and 180 degrees rotation as they're the most common\n> > + * ones.\n> > + */\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * --------------------------\n> > + * CD * (H|V) = BA AB\n> > + * BA CD CD\n> > + */\n> > + Orientation::rotate0, Orientation::rotate180,\n> > + Transform::Rot180,\n> > + },\n> > + {\n> > + Orientation::rotate180, Orientation::rotate180,\n> > + Transform::Identity\n>\n> Didn't we do this one already? Not that there's any harm in doing it again!\n>\n\nAh yes, I'll drop\n\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * --------------------------\n> > + * AB * (H|V) = CD DC\n> > + * CD AB BA\n> > + */\n> > + Orientation::rotate180, Orientation::rotate0,\n> > + Transform::Rot180\n> > + },\n> > + /* Test that transpositions are handled correctly. */\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * --------------------------\n> > + * AB * (T|V) = CD CA\n> > + * CD AB DB\n> > + */\n> > + Orientation::rotate90, Orientation::rotate0,\n> > + Transform::Rot90,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * --------------------------\n> > + * CA * (T|H) = AC AB\n> > + * DB BD CD\n> > + */\n> > + Orientation::rotate0, Orientation::rotate90,\n> > + Transform::Rot270,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * --------------------------\n> > + * AB * (T|H) = BA BD\n> > + * CD DC AC\n> > + */\n> > + Orientation::rotate270, Orientation::rotate0,\n> > + Transform::Rot270,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * --------------------------\n> > + * BD * (T|V) = AC AB\n> > + * AC BD CD\n> > + */\n> > + Orientation::rotate0, Orientation::rotate270,\n> > + Transform::Rot90,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * --------------------------\n> > + * CD * (T|H) = DC DA\n> > + * BA AB CB\n> > + */\n> > + Orientation::rotate90, Orientation::rotate180,\n> > + Transform::Rot270,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * --------------------------\n> > + * DA * (T|V) = CB CD\n> > + * CB DA BA\n> > + */\n> > + Orientation::rotate180, Orientation::rotate90,\n> > + Transform::Rot90,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * --------------------------\n> > + * CD * (T|V) = BA BC\n> > + * BA CD AD\n> > + */\n> > + Orientation::rotate270, Orientation::rotate180,\n> > + Transform::Rot90,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * --------------------------\n> > + * BC * (T|H) = CB CD\n> > + * AD DA BA\n> > + */\n> > + Orientation::rotate180, Orientation::rotate270,\n> > + Transform::Rot270,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * --------------------------\n> > + * DA * (V|H) = AD BC\n> > + * CB BC AD\n> > + */\n> > + Orientation::rotate270, Orientation::rotate90,\n> > + Transform::Rot180,\n> > + },\n> > + /* Test that mirroring is handled correctly. */\n> > + {\n> > + Orientation::rotate0, Orientation::rotate0Flip,\n> > + Transform::HFlip\n> > + },\n> > + {\n> > + Orientation::rotate0Flip, Orientation::rotate0,\n> > + Transform::HFlip\n> > + },\n> > + {\n> > + Orientation::rotate180, Orientation::rotate180Flip,\n> > + Transform::HFlip\n> > + },\n> > + {\n> > + Orientation::rotate180Flip, Orientation::rotate180,\n> > + Transform::HFlip\n> > + },\n> > + {\n> > + Orientation::rotate90, Orientation::rotate90Flip,\n> > + Transform::HFlip\n> > + },\n> > + {\n> > + Orientation::rotate90Flip, Orientation::rotate90,\n> > + Transform::HFlip\n> > + },\n> > + {\n> > + Orientation::rotate270, Orientation::rotate270Flip,\n> > + Transform::HFlip\n> > + },\n> > + {\n> > + Orientation::rotate270Flip, Orientation::rotate270,\n> > + Transform::HFlip\n> > + },\n> > + {\n> > + Orientation::rotate0, Orientation::rotate0Flip,\n> > + Transform::HFlip\n> > + },\n> > + /*\n> > + * More exotic transforms which include Transpositions and\n> > + * mirroring.\n> > + */\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * ------------------\n> > + * BC * (V) = AD\n> > + * AD BC\n> > + */\n> > + Orientation::rotate90Flip, Orientation::rotate270,\n> > + Transform::VFlip,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * ------------------\n> > + * CB * (T) = CD\n> > + * DA BA\n> > + */\n> > + Orientation::rotate180, Orientation::rotate270Flip,\n> > + Transform::Transpose,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * ------------------\n> > + * AD * (T) = AB\n> > + * BC DC\n> > + */\n> > + Orientation::rotate0, Orientation::rotate90Flip,\n> > + Transform::Transpose,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * ------------------\n> > + * AD * (V) = BC\n> > + * BC AD\n> > + */\n> > + Orientation::rotate270, Orientation::rotate90Flip,\n> > + Transform::VFlip,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * ------------------\n> > + * DA * (V) = CB\n> > + * CB DA\n> > + */\n> > + Orientation::rotate270Flip, Orientation::rotate90,\n> > + Transform::VFlip,\n> > + },\n> > + {\n> > + /*\n> > + * o2 t o1\n> > + * --------------------------\n> > + * CB * (V|H) = BC AD\n> > + * DA AD BC\n> > + */\n> > + Orientation::rotate90Flip, Orientation::rotate270Flip,\n> > + Transform::Rot180,\n> > + },\n> > + };\n> > +\n> > + for (const auto &entry : testEntries) {\n> > + Transform transform = entry.o1 / entry.o2;\n> > + cout << \"Testing: \" << entry.o1 << \" / \" << entry.o2\n> > + << \" = \" << transformToString(transform) << endl;\n> > + if (transform != entry.t) {\n> > + cerr << \"Failed to validate: \" << entry.o1\n> > + << \" / \" << entry.o2\n> > + << \" = \" << transformToString(entry.t) << endl;\n> > + cerr << \"Got back: \"\n> > + << transformToString(transform) << endl;\n> > + return TestFail;\n> > + }\n> > +\n> > + Orientation adjusted = entry.o2 * entry.t;\n> > + if (adjusted != entry.o1) {\n> > + cerr << \"Failed to validate: \" << entry.o2\n> > + << \" * \" << transformToString(entry.t)\n> > + << \" = \" << entry.o1 << endl;\n> > + cerr << \"Got back: \" << adjusted << endl;\n> > + return TestFail;\n> > + }\n> > + }\n>\n> All looks good to me, I think there's probably plenty of coverage here\n> so that you'd notice if anything got broken.\n>\n> Reviewed-by: David Plowman \n\nThanks\n j\n\n>\n> Thanks!\n> David\n>\n> > +\n> > + return TestPass;\n> > +}\n> > +\n> > +TEST_REGISTER(TransformTest)\n> > --\n> > 2.40.1\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 ADB7BBDB13\n\tfor ;\n\tMon, 24 Jul 2023 06:59:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 088E0628C0;\n\tMon, 24 Jul 2023 08:59:31 +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 B2E1661E20\n\tfor ;\n\tMon, 24 Jul 2023 08:59:25 +0200 (CEST)","from ideasonboard.com (mob-5-91-20-233.net.vodafone.it\n\t[5.91.20.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B2356C67;\n\tMon, 24 Jul 2023 08:58:27 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1690181971;\n\tbh=kRmSh3JLEhny1kkpR3TN4EPW8eNPGqNalM1zxBFLLyc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Na+BR2dZavDLEjphy9w5RmwNVCMF7EXSd1bHGJMQvLL4/WMoQkhuYAVWa3cFciWoV\n\tqzNwDq7GsolLS33i+Hv/uak75ppcgevm475+UDVrMa62e0BUiWMqy9tN82Muf82MGr\n\tU0c54M8iNYEFF4zqszAFkWPCsYvMEpWU7fCKtDSU4LkFYu4eOM9gzEBlX+dXY0pUED\n\tNKA02JmSWzmaWvQBIYJHoPGbgO+PY0vFNQyHVHxipfrlQAlk3b+L/F77zVGVYDMY52\n\th4CCtDSrU9mLjfn0V933eRsBj5Q6NXJ7sFamkxyS1FizLcUurkEzLgg8ln4q2Ah7wM\n\tpI+SvLybsKsWg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1690181908;\n\tbh=kRmSh3JLEhny1kkpR3TN4EPW8eNPGqNalM1zxBFLLyc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=m8guaHPd1DZDvQ6kbLNWGw+RG8jVIs1z7CxzNhki1HJ2sWkQJZZimVE6VvtQrnIRg\n\tdTLY6/wJaZXcwQRfWZs5l3A+bEKhovpGFsSoR+EAOVZOTEtmIoNApWNhydL8q4kS7t\n\t7ogLSXbycTL/CMiNxn0F8UfTQhWrxyFDtxDihE8M="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"m8guaHPd\"; dkim-atps=neutral","Date":"Mon, 24 Jul 2023 08:59:21 +0200","To":"David Plowman ","Message-ID":"","References":"<20230718105210.83558-1-jacopo.mondi@ideasonboard.com>\n\t<20230718105210.83558-9-jacopo.mondi@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3 08/10] test: Add unit test for\n\tTransform and Orientation","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]