[RFC,v2,10/12] test: libipa: Add Vector class test
diff mbox series

Message ID 20241118000738.18977-11-laurent.pinchart@ideasonboard.com
State New
Headers show
Series
  • Improve linear algebra helpers in libipa
Related show

Commit Message

Laurent Pinchart Nov. 18, 2024, 12:07 a.m. UTC 
Add a unit test to exercize the API of the ipa::Vector class.

The test binary being called 'vector', implicit includes cause the
binary to be picked by '#include <vector>', causing builds to fail. Set
implicit_include_directories to false to avoid this, as done in commit
6cd849125888 ("test: Don't add current build directory to include
path").

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/ipa/libipa/meson.build |   2 +
 test/ipa/libipa/vector.cpp  | 100 ++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)
 create mode 100644 test/ipa/libipa/vector.cpp

Comments

Milan Zamazal Nov. 18, 2024, 1:27 p.m. UTC | #1 
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Add a unit test to exercize the API of the ipa::Vector class.
>
> The test binary being called 'vector', implicit includes cause the
> binary to be picked by '#include <vector>', causing builds to fail. Set
> implicit_include_directories to false to avoid this, as done in commit
> 6cd849125888 ("test: Don't add current build directory to include
> path").
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  test/ipa/libipa/meson.build |   2 +
>  test/ipa/libipa/vector.cpp  | 100 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 102 insertions(+)
>  create mode 100644 test/ipa/libipa/vector.cpp
>
> diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build
> index 4d2427dbd4e7..f9b3c46d000f 100644
> --- a/test/ipa/libipa/meson.build
> +++ b/test/ipa/libipa/meson.build
> @@ -2,11 +2,13 @@
>  
>  libipa_test = [
>      {'name': 'interpolator', 'sources': ['interpolator.cpp']},
> +    {'name': 'vector', 'sources': ['vector.cpp']},
>  ]
>  
>  foreach test : libipa_test
>      exe = executable(test['name'], test['sources'],
>                       dependencies : [libcamera_private, libipa_dep],
> +                     implicit_include_directories : false,
>                       link_with : [test_libraries],
>                       include_directories : [test_includes_internal,
>                                              '../../../src/ipa/libipa/'])
> diff --git a/test/ipa/libipa/vector.cpp b/test/ipa/libipa/vector.cpp
> new file mode 100644
> index 000000000000..5279bc8216b3
> --- /dev/null
> +++ b/test/ipa/libipa/vector.cpp
> @@ -0,0 +1,100 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2024, Ideas on Board Oy
> + *
> + * Vector tests
> + */
> +
> +#include "../src/ipa/libipa/vector.h"
> +
> +#include <cmath>
> +#include <iostream>
> +
> +#include "test.h"
> +
> +using namespace libcamera::ipa;
> +
> +#define ASSERT_EQ(a, b)							\
> +if ((a) != (b)) {							\
> +	std::cout << #a " != " #b << " (line " << __LINE__ << ")"	\
> +		  << std::endl;						\
> +	return TestFail;						\
> +}
> +
> +class VectorTest : public Test
> +{
> +protected:
> +	int run()
> +	{
> +		Vector<double, 3> v1;

My compiler (g++ 14.2.1) complains that

  ../test/ipa/libipa/vector.cpp: In member function ‘virtual int VectorTest::run()’:
  ../test/ipa/libipa/vector.cpp:18:9: error: ‘v1’ is used uninitialized [-Werror=uninitialized]
     18 | if ((a) != (b)) {                                                       \
  ../test/ipa/libipa/vector.cpp:31:17: note: in expansion of macro ‘ASSERT_EQ’
     31 |                 ASSERT_EQ(v1[0], 0.0);
        |                 ^~~~~~~~~
  ../test/ipa/libipa/vector.cpp:29:35: note: ‘v1’ declared here
     29 |                 Vector<double, 3> v1;
        |                                   ^~

Automatic initialization is apparently the point of the test, so what's wrong?

> +		ASSERT_EQ(v1[0], 0.0);
> +		ASSERT_EQ(v1[1], 0.0);
> +		ASSERT_EQ(v1[2], 0.0);
> +
> +		ASSERT_EQ(v1.length(), 0.0);
> +		ASSERT_EQ(v1.length2(), 0.0);
> +
> +		Vector<double, 3> v2{{ 1.0, 4.0, 8.0 }};
> +
> +		ASSERT_EQ(v2[0], 1.0);
> +		ASSERT_EQ(v2[1], 4.0);
> +		ASSERT_EQ(v2[2], 8.0);
> +
> +		ASSERT_EQ(v2.x(), 1.0);
> +		ASSERT_EQ(v2.y(), 4.0);
> +		ASSERT_EQ(v2.z(), 8.0);
> +
> +		ASSERT_EQ(v2.r(), 1.0);
> +		ASSERT_EQ(v2.g(), 4.0);
> +		ASSERT_EQ(v2.b(), 8.0);
> +
> +		ASSERT_EQ(v2.length2(), 81.0);
> +		ASSERT_EQ(v2.length(), 9.0);
> +		ASSERT_EQ(v2.sum(), 13.0);
> +
> +		Vector<double, 3> v3{ v2 };
> +
> +		ASSERT_EQ(v2, v3);
> +
> +		v3 = Vector<double, 3>{{ 4.0, 4.0, 4.0 }};
> +
> +		ASSERT_EQ(v2 + v3, (Vector<double, 3>{{ 5.0, 8.0, 12.0 }}));
> +		ASSERT_EQ(v2 + 4.0, (Vector<double, 3>{{ 5.0, 8.0, 12.0 }}));
> +		ASSERT_EQ(v2 - v3, (Vector<double, 3>{{ -3.0, 0.0, 4.0 }}));
> +		ASSERT_EQ(v2 - 4.0, (Vector<double, 3>{{ -3.0, 0.0, 4.0 }}));
> +		ASSERT_EQ(v2 * v3, (Vector<double, 3>{{ 4.0, 16.0, 32.0 }}));
> +		ASSERT_EQ(v2 * 4.0, (Vector<double, 3>{{ 4.0, 16.0, 32.0 }}));
> +		ASSERT_EQ(v2 / v3, (Vector<double, 3>{{ 0.25, 1.0, 2.0 }}));
> +		ASSERT_EQ(v2 / 4.0, (Vector<double, 3>{{ 0.25, 1.0, 2.0 }}));
> +
> +		ASSERT_EQ(v2.min(v3), (Vector<double, 3>{{ 1.0, 4.0, 4.0 }}));
> +		ASSERT_EQ(v2.min(4.0), (Vector<double, 3>{{ 1.0, 4.0, 4.0 }}));
> +		ASSERT_EQ(v2.max(v3), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
> +		ASSERT_EQ(v2.max(4.0), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
> +
> +		ASSERT_EQ(v2.dot(v3), 52.0);
> +
> +		v2 += v3;
> +		ASSERT_EQ(v2, (Vector<double, 3>{{ 5.0, 8.0, 12.0 }}));
> +		v2 -= v3;
> +		ASSERT_EQ(v2, (Vector<double, 3>{{ 1.0, 4.0, 8.0 }}));
> +		v2 *= v3;
> +		ASSERT_EQ(v2, (Vector<double, 3>{{ 4.0, 16.0, 32.0 }}));
> +		v2 /= v3;
> +		ASSERT_EQ(v2, (Vector<double, 3>{{ 1.0, 4.0, 8.0 }}));
> +
> +		v2 += 4.0;
> +		ASSERT_EQ(v2, (Vector<double, 3>{{ 5.0, 8.0, 12.0 }}));
> +		v2 -= 4.0;
> +		ASSERT_EQ(v2, (Vector<double, 3>{{ 1.0, 4.0, 8.0 }}));
> +		v2 *= 4.0;
> +		ASSERT_EQ(v2, (Vector<double, 3>{{ 4.0, 16.0, 32.0 }}));
> +		v2 /= 4.0;
> +		ASSERT_EQ(v2, (Vector<double, 3>{{ 1.0, 4.0, 8.0 }}));
> +
> +		return TestPass;
> +	}
> +};
> +
> +TEST_REGISTER(VectorTest)
Laurent Pinchart Nov. 18, 2024, 4:32 p.m. UTC | #2 
Hi Milan,

On Mon, Nov 18, 2024 at 02:27:59PM +0100, Milan Zamazal wrote:
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
> 
> > Add a unit test to exercize the API of the ipa::Vector class.
> >
> > The test binary being called 'vector', implicit includes cause the
> > binary to be picked by '#include <vector>', causing builds to fail. Set
> > implicit_include_directories to false to avoid this, as done in commit
> > 6cd849125888 ("test: Don't add current build directory to include
> > path").
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  test/ipa/libipa/meson.build |   2 +
> >  test/ipa/libipa/vector.cpp  | 100 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 102 insertions(+)
> >  create mode 100644 test/ipa/libipa/vector.cpp
> >
> > diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build
> > index 4d2427dbd4e7..f9b3c46d000f 100644
> > --- a/test/ipa/libipa/meson.build
> > +++ b/test/ipa/libipa/meson.build
> > @@ -2,11 +2,13 @@
> >  
> >  libipa_test = [
> >      {'name': 'interpolator', 'sources': ['interpolator.cpp']},
> > +    {'name': 'vector', 'sources': ['vector.cpp']},
> >  ]
> >  
> >  foreach test : libipa_test
> >      exe = executable(test['name'], test['sources'],
> >                       dependencies : [libcamera_private, libipa_dep],
> > +                     implicit_include_directories : false,
> >                       link_with : [test_libraries],
> >                       include_directories : [test_includes_internal,
> >                                              '../../../src/ipa/libipa/'])
> > diff --git a/test/ipa/libipa/vector.cpp b/test/ipa/libipa/vector.cpp
> > new file mode 100644
> > index 000000000000..5279bc8216b3
> > --- /dev/null
> > +++ b/test/ipa/libipa/vector.cpp
> > @@ -0,0 +1,100 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2024, Ideas on Board Oy
> > + *
> > + * Vector tests
> > + */
> > +
> > +#include "../src/ipa/libipa/vector.h"
> > +
> > +#include <cmath>
> > +#include <iostream>
> > +
> > +#include "test.h"
> > +
> > +using namespace libcamera::ipa;
> > +
> > +#define ASSERT_EQ(a, b)							\
> > +if ((a) != (b)) {							\
> > +	std::cout << #a " != " #b << " (line " << __LINE__ << ")"	\
> > +		  << std::endl;						\
> > +	return TestFail;						\
> > +}
> > +
> > +class VectorTest : public Test
> > +{
> > +protected:
> > +	int run()
> > +	{
> > +		Vector<double, 3> v1;
> 
> My compiler (g++ 14.2.1) complains that
> 
>   ../test/ipa/libipa/vector.cpp: In member function ‘virtual int VectorTest::run()’:
>   ../test/ipa/libipa/vector.cpp:18:9: error: ‘v1’ is used uninitialized [-Werror=uninitialized]
>      18 | if ((a) != (b)) {                                                       \
>   ../test/ipa/libipa/vector.cpp:31:17: note: in expansion of macro ‘ASSERT_EQ’
>      31 |                 ASSERT_EQ(v1[0], 0.0);
>         |                 ^~~~~~~~~
>   ../test/ipa/libipa/vector.cpp:29:35: note: ‘v1’ declared here
>      29 |                 Vector<double, 3> v1;
>         |                                   ^~
> 
> Automatic initialization is apparently the point of the test, so what's wrong?

I didn't notice that with gcc 13 initially, but I've now been able to
reproduce the issue.

The default constructor indeed doesn't initialize the memory. This could
be changed to initialize all entries to T{}, or we could do so
explicitly in callers where it matters.

> > +		ASSERT_EQ(v1[0], 0.0);
> > +		ASSERT_EQ(v1[1], 0.0);
> > +		ASSERT_EQ(v1[2], 0.0);
> > +
> > +		ASSERT_EQ(v1.length(), 0.0);
> > +		ASSERT_EQ(v1.length2(), 0.0);
> > +
> > +		Vector<double, 3> v2{{ 1.0, 4.0, 8.0 }};
> > +
> > +		ASSERT_EQ(v2[0], 1.0);
> > +		ASSERT_EQ(v2[1], 4.0);
> > +		ASSERT_EQ(v2[2], 8.0);
> > +
> > +		ASSERT_EQ(v2.x(), 1.0);
> > +		ASSERT_EQ(v2.y(), 4.0);
> > +		ASSERT_EQ(v2.z(), 8.0);
> > +
> > +		ASSERT_EQ(v2.r(), 1.0);
> > +		ASSERT_EQ(v2.g(), 4.0);
> > +		ASSERT_EQ(v2.b(), 8.0);
> > +
> > +		ASSERT_EQ(v2.length2(), 81.0);
> > +		ASSERT_EQ(v2.length(), 9.0);
> > +		ASSERT_EQ(v2.sum(), 13.0);
> > +
> > +		Vector<double, 3> v3{ v2 };
> > +
> > +		ASSERT_EQ(v2, v3);
> > +
> > +		v3 = Vector<double, 3>{{ 4.0, 4.0, 4.0 }};
> > +
> > +		ASSERT_EQ(v2 + v3, (Vector<double, 3>{{ 5.0, 8.0, 12.0 }}));
> > +		ASSERT_EQ(v2 + 4.0, (Vector<double, 3>{{ 5.0, 8.0, 12.0 }}));
> > +		ASSERT_EQ(v2 - v3, (Vector<double, 3>{{ -3.0, 0.0, 4.0 }}));
> > +		ASSERT_EQ(v2 - 4.0, (Vector<double, 3>{{ -3.0, 0.0, 4.0 }}));
> > +		ASSERT_EQ(v2 * v3, (Vector<double, 3>{{ 4.0, 16.0, 32.0 }}));
> > +		ASSERT_EQ(v2 * 4.0, (Vector<double, 3>{{ 4.0, 16.0, 32.0 }}));
> > +		ASSERT_EQ(v2 / v3, (Vector<double, 3>{{ 0.25, 1.0, 2.0 }}));
> > +		ASSERT_EQ(v2 / 4.0, (Vector<double, 3>{{ 0.25, 1.0, 2.0 }}));
> > +
> > +		ASSERT_EQ(v2.min(v3), (Vector<double, 3>{{ 1.0, 4.0, 4.0 }}));
> > +		ASSERT_EQ(v2.min(4.0), (Vector<double, 3>{{ 1.0, 4.0, 4.0 }}));
> > +		ASSERT_EQ(v2.max(v3), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
> > +		ASSERT_EQ(v2.max(4.0), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
> > +
> > +		ASSERT_EQ(v2.dot(v3), 52.0);
> > +
> > +		v2 += v3;
> > +		ASSERT_EQ(v2, (Vector<double, 3>{{ 5.0, 8.0, 12.0 }}));
> > +		v2 -= v3;
> > +		ASSERT_EQ(v2, (Vector<double, 3>{{ 1.0, 4.0, 8.0 }}));
> > +		v2 *= v3;
> > +		ASSERT_EQ(v2, (Vector<double, 3>{{ 4.0, 16.0, 32.0 }}));
> > +		v2 /= v3;
> > +		ASSERT_EQ(v2, (Vector<double, 3>{{ 1.0, 4.0, 8.0 }}));
> > +
> > +		v2 += 4.0;
> > +		ASSERT_EQ(v2, (Vector<double, 3>{{ 5.0, 8.0, 12.0 }}));
> > +		v2 -= 4.0;
> > +		ASSERT_EQ(v2, (Vector<double, 3>{{ 1.0, 4.0, 8.0 }}));
> > +		v2 *= 4.0;
> > +		ASSERT_EQ(v2, (Vector<double, 3>{{ 4.0, 16.0, 32.0 }}));
> > +		v2 /= 4.0;
> > +		ASSERT_EQ(v2, (Vector<double, 3>{{ 1.0, 4.0, 8.0 }}));
> > +
> > +		return TestPass;
> > +	}
> > +};
> > +
> > +TEST_REGISTER(VectorTest)

Patch
diff mbox series 

diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build
index 4d2427dbd4e7..f9b3c46d000f 100644
--- a/test/ipa/libipa/meson.build
+++ b/test/ipa/libipa/meson.build
@@ -2,11 +2,13 @@ 
 
 libipa_test = [
     {'name': 'interpolator', 'sources': ['interpolator.cpp']},
+    {'name': 'vector', 'sources': ['vector.cpp']},
 ]
 
 foreach test : libipa_test
     exe = executable(test['name'], test['sources'],
                      dependencies : [libcamera_private, libipa_dep],
+                     implicit_include_directories : false,
                      link_with : [test_libraries],
                      include_directories : [test_includes_internal,
                                             '../../../src/ipa/libipa/'])
diff --git a/test/ipa/libipa/vector.cpp b/test/ipa/libipa/vector.cpp
new file mode 100644
index 000000000000..5279bc8216b3
--- /dev/null
+++ b/test/ipa/libipa/vector.cpp
@@ -0,0 +1,100 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2024, Ideas on Board Oy
+ *
+ * Vector tests
+ */
+
+#include "../src/ipa/libipa/vector.h"
+
+#include <cmath>
+#include <iostream>
+
+#include "test.h"
+
+using namespace libcamera::ipa;
+
+#define ASSERT_EQ(a, b)							\
+if ((a) != (b)) {							\
+	std::cout << #a " != " #b << " (line " << __LINE__ << ")"	\
+		  << std::endl;						\
+	return TestFail;						\
+}
+
+class VectorTest : public Test
+{
+protected:
+	int run()
+	{
+		Vector<double, 3> v1;
+
+		ASSERT_EQ(v1[0], 0.0);
+		ASSERT_EQ(v1[1], 0.0);
+		ASSERT_EQ(v1[2], 0.0);
+
+		ASSERT_EQ(v1.length(), 0.0);
+		ASSERT_EQ(v1.length2(), 0.0);
+
+		Vector<double, 3> v2{{ 1.0, 4.0, 8.0 }};
+
+		ASSERT_EQ(v2[0], 1.0);
+		ASSERT_EQ(v2[1], 4.0);
+		ASSERT_EQ(v2[2], 8.0);
+
+		ASSERT_EQ(v2.x(), 1.0);
+		ASSERT_EQ(v2.y(), 4.0);
+		ASSERT_EQ(v2.z(), 8.0);
+
+		ASSERT_EQ(v2.r(), 1.0);
+		ASSERT_EQ(v2.g(), 4.0);
+		ASSERT_EQ(v2.b(), 8.0);
+
+		ASSERT_EQ(v2.length2(), 81.0);
+		ASSERT_EQ(v2.length(), 9.0);
+		ASSERT_EQ(v2.sum(), 13.0);
+
+		Vector<double, 3> v3{ v2 };
+
+		ASSERT_EQ(v2, v3);
+
+		v3 = Vector<double, 3>{{ 4.0, 4.0, 4.0 }};
+
+		ASSERT_EQ(v2 + v3, (Vector<double, 3>{{ 5.0, 8.0, 12.0 }}));
+		ASSERT_EQ(v2 + 4.0, (Vector<double, 3>{{ 5.0, 8.0, 12.0 }}));
+		ASSERT_EQ(v2 - v3, (Vector<double, 3>{{ -3.0, 0.0, 4.0 }}));
+		ASSERT_EQ(v2 - 4.0, (Vector<double, 3>{{ -3.0, 0.0, 4.0 }}));
+		ASSERT_EQ(v2 * v3, (Vector<double, 3>{{ 4.0, 16.0, 32.0 }}));
+		ASSERT_EQ(v2 * 4.0, (Vector<double, 3>{{ 4.0, 16.0, 32.0 }}));
+		ASSERT_EQ(v2 / v3, (Vector<double, 3>{{ 0.25, 1.0, 2.0 }}));
+		ASSERT_EQ(v2 / 4.0, (Vector<double, 3>{{ 0.25, 1.0, 2.0 }}));
+
+		ASSERT_EQ(v2.min(v3), (Vector<double, 3>{{ 1.0, 4.0, 4.0 }}));
+		ASSERT_EQ(v2.min(4.0), (Vector<double, 3>{{ 1.0, 4.0, 4.0 }}));
+		ASSERT_EQ(v2.max(v3), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
+		ASSERT_EQ(v2.max(4.0), (Vector<double, 3>{{ 4.0, 4.0, 8.0 }}));
+
+		ASSERT_EQ(v2.dot(v3), 52.0);
+
+		v2 += v3;
+		ASSERT_EQ(v2, (Vector<double, 3>{{ 5.0, 8.0, 12.0 }}));
+		v2 -= v3;
+		ASSERT_EQ(v2, (Vector<double, 3>{{ 1.0, 4.0, 8.0 }}));
+		v2 *= v3;
+		ASSERT_EQ(v2, (Vector<double, 3>{{ 4.0, 16.0, 32.0 }}));
+		v2 /= v3;
+		ASSERT_EQ(v2, (Vector<double, 3>{{ 1.0, 4.0, 8.0 }}));
+
+		v2 += 4.0;
+		ASSERT_EQ(v2, (Vector<double, 3>{{ 5.0, 8.0, 12.0 }}));
+		v2 -= 4.0;
+		ASSERT_EQ(v2, (Vector<double, 3>{{ 1.0, 4.0, 8.0 }}));
+		v2 *= 4.0;
+		ASSERT_EQ(v2, (Vector<double, 3>{{ 4.0, 16.0, 32.0 }}));
+		v2 /= 4.0;
+		ASSERT_EQ(v2, (Vector<double, 3>{{ 1.0, 4.0, 8.0 }}));
+
+		return TestPass;
+	}
+};
+
+TEST_REGISTER(VectorTest)