[libcamera-devel] [PATCH v3 08/10] test: Add unit test for Transform and Orientation
David Plowman
david.plowman at raspberrypi.com
Thu Jul 20 12:06:54 CEST 2023
Hi Jacopo
Thanks for these tests.
On Tue, 18 Jul 2023 at 11:52, Jacopo Mondi
<jacopo.mondi at ideasonboard.com> wrote:
>
> Add a unit test for Transform and Orientation to validate the
> implementation of the operations between the two types.
>
> In particular, test that:
>
> o1 / o2 = t
> o2 * t = o1
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> ---
> include/libcamera/transform.h | 7 +
> test/meson.build | 1 +
> test/transform.cpp | 335 ++++++++++++++++++++++++++++++++++
> 3 files changed, 343 insertions(+)
> create mode 100644 test/transform.cpp
>
> diff --git a/include/libcamera/transform.h b/include/libcamera/transform.h
> index 847a1f94713c..833b50d46fd0 100644
> --- a/include/libcamera/transform.h
> +++ b/include/libcamera/transform.h
> @@ -16,13 +16,20 @@ enum class Orientation;
> enum class Transform : int {
> Identity = 0,
> Rot0 = Identity,
> +
> HFlip = 1,
> +
> VFlip = 2,
> +
> HVFlip = HFlip | VFlip,
> Rot180 = HVFlip,
> +
> Transpose = 4,
> +
> Rot270 = HFlip | Transpose,
> +
> Rot90 = VFlip | Transpose,
> +
Didn't I see another commit removing these extra blank lines again?
Not that I mind...!
> Rot180Transpose = HFlip | VFlip | Transpose
> };
>
> diff --git a/test/meson.build b/test/meson.build
> index b227be818419..189e1428485a 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -46,6 +46,7 @@ public_tests = [
> {'name': 'public-api', 'sources': ['public-api.cpp']},
> {'name': 'signal', 'sources': ['signal.cpp']},
> {'name': 'span', 'sources': ['span.cpp']},
> + {'name': 'transform', 'sources': ['transform.cpp']},
> ]
>
> internal_tests = [
> diff --git a/test/transform.cpp b/test/transform.cpp
> new file mode 100644
> index 000000000000..f79a1a892d16
> --- /dev/null
> +++ b/test/transform.cpp
> @@ -0,0 +1,335 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2023, Ideas On Board Oy
> + *
> + * transform.cpp - Transform and Orientation tests
> + */
> +
> +#include <iostream>
> +
> +#include <libcamera/orientation.h>
> +#include <libcamera/transform.h>
> +
> +#include "test.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +
> +class TransformTest : public Test
> +{
> +protected:
> + int run();
> +};
> +
> +int TransformTest::run()
> +{
> + /*
> + * RotationTestEntry collects two Orientation and one Transform that
> + * gets combined to validate that (o1 / o2 = T) and (o1 = o2 * T)
> + *
> + * o1 / o2 = t computes the Transform to apply to o2 to obtain o1
> + * o2 * t = o1 combines o2 with t by applying o2 first then t
> + *
> + * The comments on the (most complex) transform show how applying to
> + * an image with orientation o2 the Transform t allows to obtain o1.
> + *
> + * The image with basic rotation0 is assumed to be:
> + *
> + * AB
> + * CD
> + *
> + * And the Transform operators are:
> + *
> + * V = vertical flip
> + * H = horizontal flip
> + * T = transpose
> + *
> + * the operator '* (T|V)' applies V first then T.
Should we mention H here as well? Again, this is so minor I'm not at
all bothered...!
> + */
> + struct RotationTestEntry {
> + Orientation o1;
> + Orientation o2;
> + Transform t;
> + } testEntries[] = {
> + /* Test identities transforms first. */
> + {
> + Orientation::rotate0, Orientation::rotate0,
> + Transform::Identity,
> + },
> + {
> + Orientation::rotate0Flip, Orientation::rotate0Flip,
> + Transform::Identity,
> + },
> + {
> + Orientation::rotate180, Orientation::rotate180,
> + Transform::Identity,
> + },
> + {
> + Orientation::rotate180Flip, Orientation::rotate180Flip,
> + Transform::Identity,
> + },
> + {
> + Orientation::rotate90, Orientation::rotate90,
> + Transform::Identity,
> + },
> + {
> + Orientation::rotate90Flip, Orientation::rotate90Flip,
> + Transform::Identity,
> + },
> + {
> + Orientation::rotate270, Orientation::rotate270,
> + Transform::Identity,
> + },
> + {
> + Orientation::rotate270Flip, Orientation::rotate270Flip,
> + Transform::Identity,
> + },
> + /*
> + * Combine 0 and 180 degrees rotation as they're the most common
> + * ones.
> + */
> + {
> + /*
> + * o2 t o1
> + * --------------------------
> + * CD * (H|V) = BA AB
> + * BA CD CD
> + */
> + Orientation::rotate0, Orientation::rotate180,
> + Transform::Rot180,
> + },
> + {
> + Orientation::rotate180, Orientation::rotate180,
> + Transform::Identity
Didn't we do this one already? Not that there's any harm in doing it again!
> + },
> + {
> + /*
> + * o2 t o1
> + * --------------------------
> + * AB * (H|V) = CD DC
> + * CD AB BA
> + */
> + Orientation::rotate180, Orientation::rotate0,
> + Transform::Rot180
> + },
> + /* Test that transpositions are handled correctly. */
> + {
> + /*
> + * o2 t o1
> + * --------------------------
> + * AB * (T|V) = CD CA
> + * CD AB DB
> + */
> + Orientation::rotate90, Orientation::rotate0,
> + Transform::Rot90,
> + },
> + {
> + /*
> + * o2 t o1
> + * --------------------------
> + * CA * (T|H) = AC AB
> + * DB BD CD
> + */
> + Orientation::rotate0, Orientation::rotate90,
> + Transform::Rot270,
> + },
> + {
> + /*
> + * o2 t o1
> + * --------------------------
> + * AB * (T|H) = BA BD
> + * CD DC AC
> + */
> + Orientation::rotate270, Orientation::rotate0,
> + Transform::Rot270,
> + },
> + {
> + /*
> + * o2 t o1
> + * --------------------------
> + * BD * (T|V) = AC AB
> + * AC BD CD
> + */
> + Orientation::rotate0, Orientation::rotate270,
> + Transform::Rot90,
> + },
> + {
> + /*
> + * o2 t o1
> + * --------------------------
> + * CD * (T|H) = DC DA
> + * BA AB CB
> + */
> + Orientation::rotate90, Orientation::rotate180,
> + Transform::Rot270,
> + },
> + {
> + /*
> + * o2 t o1
> + * --------------------------
> + * DA * (T|V) = CB CD
> + * CB DA BA
> + */
> + Orientation::rotate180, Orientation::rotate90,
> + Transform::Rot90,
> + },
> + {
> + /*
> + * o2 t o1
> + * --------------------------
> + * CD * (T|V) = BA BC
> + * BA CD AD
> + */
> + Orientation::rotate270, Orientation::rotate180,
> + Transform::Rot90,
> + },
> + {
> + /*
> + * o2 t o1
> + * --------------------------
> + * BC * (T|H) = CB CD
> + * AD DA BA
> + */
> + Orientation::rotate180, Orientation::rotate270,
> + Transform::Rot270,
> + },
> + {
> + /*
> + * o2 t o1
> + * --------------------------
> + * DA * (V|H) = AD BC
> + * CB BC AD
> + */
> + Orientation::rotate270, Orientation::rotate90,
> + Transform::Rot180,
> + },
> + /* Test that mirroring is handled correctly. */
> + {
> + Orientation::rotate0, Orientation::rotate0Flip,
> + Transform::HFlip
> + },
> + {
> + Orientation::rotate0Flip, Orientation::rotate0,
> + Transform::HFlip
> + },
> + {
> + Orientation::rotate180, Orientation::rotate180Flip,
> + Transform::HFlip
> + },
> + {
> + Orientation::rotate180Flip, Orientation::rotate180,
> + Transform::HFlip
> + },
> + {
> + Orientation::rotate90, Orientation::rotate90Flip,
> + Transform::HFlip
> + },
> + {
> + Orientation::rotate90Flip, Orientation::rotate90,
> + Transform::HFlip
> + },
> + {
> + Orientation::rotate270, Orientation::rotate270Flip,
> + Transform::HFlip
> + },
> + {
> + Orientation::rotate270Flip, Orientation::rotate270,
> + Transform::HFlip
> + },
> + {
> + Orientation::rotate0, Orientation::rotate0Flip,
> + Transform::HFlip
> + },
> + /*
> + * More exotic transforms which include Transpositions and
> + * mirroring.
> + */
> + {
> + /*
> + * o2 t o1
> + * ------------------
> + * BC * (V) = AD
> + * AD BC
> + */
> + Orientation::rotate90Flip, Orientation::rotate270,
> + Transform::VFlip,
> + },
> + {
> + /*
> + * o2 t o1
> + * ------------------
> + * CB * (T) = CD
> + * DA BA
> + */
> + Orientation::rotate180, Orientation::rotate270Flip,
> + Transform::Transpose,
> + },
> + {
> + /*
> + * o2 t o1
> + * ------------------
> + * AD * (T) = AB
> + * BC DC
> + */
> + Orientation::rotate0, Orientation::rotate90Flip,
> + Transform::Transpose,
> + },
> + {
> + /*
> + * o2 t o1
> + * ------------------
> + * AD * (V) = BC
> + * BC AD
> + */
> + Orientation::rotate270, Orientation::rotate90Flip,
> + Transform::VFlip,
> + },
> + {
> + /*
> + * o2 t o1
> + * ------------------
> + * DA * (V) = CB
> + * CB DA
> + */
> + Orientation::rotate270Flip, Orientation::rotate90,
> + Transform::VFlip,
> + },
> + {
> + /*
> + * o2 t o1
> + * --------------------------
> + * CB * (V|H) = BC AD
> + * DA AD BC
> + */
> + Orientation::rotate90Flip, Orientation::rotate270Flip,
> + Transform::Rot180,
> + },
> + };
> +
> + for (const auto &entry : testEntries) {
> + Transform transform = entry.o1 / entry.o2;
> + cout << "Testing: " << entry.o1 << " / " << entry.o2
> + << " = " << transformToString(transform) << endl;
> + if (transform != entry.t) {
> + cerr << "Failed to validate: " << entry.o1
> + << " / " << entry.o2
> + << " = " << transformToString(entry.t) << endl;
> + cerr << "Got back: "
> + << transformToString(transform) << endl;
> + return TestFail;
> + }
> +
> + Orientation adjusted = entry.o2 * entry.t;
> + if (adjusted != entry.o1) {
> + cerr << "Failed to validate: " << entry.o2
> + << " * " << transformToString(entry.t)
> + << " = " << entry.o1 << endl;
> + cerr << "Got back: " << adjusted << endl;
> + return TestFail;
> + }
> + }
All looks good to me, I think there's probably plenty of coverage here
so that you'd notice if anything got broken.
Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
Thanks!
David
> +
> + return TestPass;
> +}
> +
> +TEST_REGISTER(TransformTest)
> --
> 2.40.1
>
More information about the libcamera-devel
mailing list