[libcamera-devel] [PATCH v3 08/10] test: Add unit test for Transform and Orientation
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Mon Jul 24 08:59:21 CEST 2023
Hi David
On Thu, Jul 20, 2023 at 11:06:54AM +0100, David Plowman via libcamera-devel wrote:
> 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...!
This clearly shouldn't be here
>
> > 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...!
>
Well, this was just an example to specify the operator application
order
> > + */
> > + 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!
>
Ah yes, I'll drop
> > + },
> > + {
> > + /*
> > + * 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
j
>
> Thanks!
> David
>
> > +
> > + return TestPass;
> > +}
> > +
> > +TEST_REGISTER(TransformTest)
> > --
> > 2.40.1
> >
More information about the libcamera-devel
mailing list