From e62ce4faa6370d124698934cd506566287e916a0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Sat, 16 Feb 2019 12:26:24 +0100
Subject: [PATCH] Math: fix pack() to correctly select the nearest integral
 value.

Together with an assorted set of off-by-one changes to tests involving
packing, in addition to the changes done in the previous test cleanup
commit. Now Color3 sRGB conversion rountrips correctly.
---
 doc/changelog.dox                    |  3 +++
 src/Magnum/Math/Packing.h            |  2 +-
 src/Magnum/Math/Test/ColorTest.cpp   | 24 ++++++++++-----------
 src/Magnum/Math/Test/PackingTest.cpp | 31 +++++++++++++++++-----------
 4 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/doc/changelog.dox b/doc/changelog.dox
index ba1b9164a..cb0633a59 100644
--- a/doc/changelog.dox
+++ b/doc/changelog.dox
@@ -65,6 +65,9 @@ See also:
 @subsection changelog-latest-bugfixes Bug fixes
 
 -   Fixed compilation of the @ref Vk library on 32-bit Windows
+-   @ref Math::pack() was incorrectly not selecting the nearest integral value,
+    causing @ref Math::Color3::toSrgbInt() to not roundtrip, among other
+    things.
 
 @section changelog-2019-01 2019.01
 
diff --git a/src/Magnum/Math/Packing.h b/src/Magnum/Math/Packing.h
index c12f77f70..5972847b8 100644
--- a/src/Magnum/Math/Packing.h
+++ b/src/Magnum/Math/Packing.h
@@ -135,7 +135,7 @@ template<class Integral, class FloatingPoint, UnsignedInt bits = sizeof(Integral
         "packing must be done from floating-point to integral type");
     static_assert(bits <= sizeof(Integral)*8,
         "bit count larger than size of the integral type");
-    return Integral(value*Implementation::bitMax<Integral, bits>());
+    return Integral(round(value*Implementation::bitMax<Integral, bits>()));
 }
 template<class Integral, std::size_t size, class FloatingPoint, UnsignedInt bits = sizeof(typename Integral::Type)*8> Integral pack(const Vector<size, FloatingPoint>& value) {
     static_assert(Integral::Size == size,
diff --git a/src/Magnum/Math/Test/ColorTest.cpp b/src/Magnum/Math/Test/ColorTest.cpp
index d2aafaf6c..22a3f7cc9 100644
--- a/src/Magnum/Math/Test/ColorTest.cpp
+++ b/src/Magnum/Math/Test/ColorTest.cpp
@@ -370,11 +370,11 @@ void ColorTest::constructConversion() {
 void ColorTest::constructPacking() {
     constexpr Color3 a(1.0f, 0.5f, 0.75f);
     auto b = Math::pack<Color3ub>(a);
-    CORRADE_COMPARE(b, Color3ub(255, 127, 191));
+    CORRADE_COMPARE(b, Color3ub(255, 128, 191));
 
     constexpr Color4 c(1.0f, 0.5f, 0.75f, 0.25f);
     auto d = Math::pack<Color4ub>(c);
-    CORRADE_COMPARE(d, Color4ub(255, 127, 191, 63));
+    CORRADE_COMPARE(d, Color4ub(255, 128, 191, 64));
 }
 
 void ColorTest::constructCopy() {
@@ -537,9 +537,9 @@ void ColorTest::hue() {
 
     /* Integral -- little precision loss */
     CORRADE_COMPARE(Math::Color3<UnsignedShort>::fromHsv(27.0_degf, 1.0f, 1.0f),
-        (Math::Color3<UnsignedShort>{65535, 29490, 0}));
+        (Math::Color3<UnsignedShort>{65535, 29491, 0}));
     CORRADE_COMPARE(Math::Color4<UnsignedShort>::fromHsv(27.0_degf, 1.0f, 1.0f, 15239),
-        (Math::Color4<UnsignedShort>{65535, 29490, 0, 15239}));
+        (Math::Color4<UnsignedShort>{65535, 29491, 0, 15239}));
     CORRADE_COMPARE((Math::Color3<UnsignedShort>{65535, 29490, 0}).hue(), 26.9993_degf);
     CORRADE_COMPARE((Math::Color4<UnsignedShort>{65535, 29490, 0, 15239}.hue()), 26.9993_degf);
 }
@@ -612,13 +612,13 @@ void ColorTest::hsv() {
 
     /* Integral -- little precision loss */
     CORRADE_COMPARE(Math::Color3<UnsignedShort>::fromHsv(std::make_tuple(230.0_degf, 0.749f, 0.427f)),
-        (Math::Color3<UnsignedShort>{7023, 10517, 27983}));
+        (Math::Color3<UnsignedShort>{7024, 10517, 27983}));
     CORRADE_COMPARE(Math::Color3<UnsignedShort>::fromHsv(230.0_degf, 0.749f, 0.427f),
-        (Math::Color3<UnsignedShort>{7023, 10517, 27983}));
+        (Math::Color3<UnsignedShort>{7024, 10517, 27983}));
     CORRADE_COMPARE(Math::Color4<UnsignedShort>::fromHsv(std::make_tuple(230.0_degf, 0.749f, 0.427f), 15239),
-        (Math::Color4<UnsignedShort>{7023, 10517, 27983, 15239}));
+        (Math::Color4<UnsignedShort>{7024, 10517, 27983, 15239}));
     CORRADE_COMPARE(Math::Color4<UnsignedShort>::fromHsv(230.0_degf, 0.749f, 0.427f, 15239),
-        (Math::Color4<UnsignedShort>{7023, 10517, 27983, 15239}));
+        (Math::Color4<UnsignedShort>{7024, 10517, 27983, 15239}));
 
     std::tie(hue, saturation, value) = Math::Color3<UnsignedShort>{7023, 10517, 27983}.toHsv();
     CORRADE_COMPARE(hue, 230.0_degf);
@@ -661,9 +661,9 @@ void ColorTest::fromHsvDefaultAlpha() {
 
     /* Integral */
     CORRADE_COMPARE(Math::Color4<UnsignedShort>::fromHsv(std::make_tuple(230.0_degf, 0.749f, 0.427f)),
-        (Math::Color4<UnsignedShort>{7023, 10517, 27983, 65535}));
+        (Math::Color4<UnsignedShort>{7024, 10517, 27983, 65535}));
     CORRADE_COMPARE(Math::Color4<UnsignedShort>::fromHsv(230.0_degf, 0.749f, 0.427f),
-        (Math::Color4<UnsignedShort>{7023, 10517, 27983, 65535}));
+        (Math::Color4<UnsignedShort>{7024, 10517, 27983, 65535}));
 }
 
 void ColorTest::srgb() {
@@ -715,7 +715,7 @@ void ColorTest::fromSrgbDefaultAlpha() {
 
     /* Integral */
     CORRADE_COMPARE(Math::Color4<UnsignedShort>::fromSrgb({0.1523f, 0.00125f, 0.9853f}),
-        (Math::Color4<UnsignedShort>{1319, 6, 63364, 65535}));
+        (Math::Color4<UnsignedShort>{1320, 6, 63365, 65535}));
     CORRADE_COMPARE(Math::Color4<UnsignedShort>::fromSrgb<UnsignedByte>({0xf3, 0x2a, 0x80}),
         (Math::Color4<UnsignedShort>{58737, 1517, 14146, 65535}));
 }
@@ -852,7 +852,7 @@ void ColorTest::fromXyzDefaultAlpha() {
 
     /* Integral */
     CORRADE_COMPARE(Math::Color4<UnsignedShort>::fromXyz({0.454279f, 0.413092f, 0.0607124f}),
-        (Math::Color4<UnsignedShort>{52883, 22095, 339, 65535}));
+        (Math::Color4<UnsignedShort>{52884, 22096, 340, 65535}));
 }
 
 void ColorTest::xyY() {
diff --git a/src/Magnum/Math/Test/PackingTest.cpp b/src/Magnum/Math/Test/PackingTest.cpp
index 42a0f0e93..5003eec1b 100644
--- a/src/Magnum/Math/Test/PackingTest.cpp
+++ b/src/Magnum/Math/Test/PackingTest.cpp
@@ -152,12 +152,20 @@ void PackingTest::unpackSigned() {
 }
 
 void PackingTest::packUnsigned() {
+    /* Close extremes should work too */
     CORRADE_COMPARE(Math::pack<UnsignedByte>(0.0f), 0);
+    CORRADE_COMPARE(Math::pack<UnsignedByte>(0.0000001f), 0);
     CORRADE_COMPARE(Math::pack<UnsignedByte>(0.4357f), 111);
+    CORRADE_COMPARE(Math::pack<UnsignedByte>(0.5f), 128);
     CORRADE_COMPARE(Math::pack<UnsignedByte>(1.0f), 255);
+    CORRADE_COMPARE(Math::pack<UnsignedByte>(0.9999999f), 255);
 
     CORRADE_COMPARE(Math::pack<UnsignedShort>(0.0f), 0);
-    CORRADE_COMPARE(Math::pack<UnsignedShort>(1.0f), std::numeric_limits<UnsignedShort>::max());
+    CORRADE_COMPARE(Math::pack<UnsignedShort>(0.000001f), 0);
+    CORRADE_COMPARE(Math::pack<UnsignedShort>(0.4357f), 28554);
+    CORRADE_COMPARE(Math::pack<UnsignedShort>(0.5f), 32768);
+    CORRADE_COMPARE(Math::pack<UnsignedShort>(1.0f), 65535);
+    CORRADE_COMPARE(Math::pack<UnsignedShort>(0.999999f), 65535);
 
     CORRADE_COMPARE(Math::pack<UnsignedInt>(0.0), 0);
     CORRADE_COMPARE(Math::pack<UnsignedInt>(1.0), std::numeric_limits<UnsignedInt>::max());
@@ -172,21 +180,21 @@ void PackingTest::packUnsigned() {
     }
     #endif
 
-    CORRADE_COMPARE(Math::pack<UnsignedShort>(0.33f), 21626);
+    CORRADE_COMPARE(Math::pack<UnsignedShort>(0.33f), 21627);
     CORRADE_COMPARE(Math::pack<UnsignedShort>(0.66f), 43253);
 
     /* Bits */
-    CORRADE_COMPARE((Math::pack<UnsignedShort>(0.5f)), 32767);
-    CORRADE_COMPARE((Math::pack<UnsignedShort, 14>(0.5f)), 8191);
+    CORRADE_COMPARE((Math::pack<UnsignedShort>(0.5f)), 32768);
+    CORRADE_COMPARE((Math::pack<UnsignedShort, 14>(0.5f)), 8192);
 
     /* Vector overloads */
-    CORRADE_COMPARE(Math::pack<Vector3ub>(Vector3(0.0f, 0.5f, 1.0f)), Vector3ub(0, 127, 255));
-    CORRADE_COMPARE((Math::pack<Vector3ub, 6>(Vector3(0.0f, 0.5f, 1.0f))), Vector3ub(0, 31, 63));
+    CORRADE_COMPARE(Math::pack<Vector3ub>(Vector3(0.0f, 0.5f, 1.0f)), Vector3ub(0, 128, 255));
+    CORRADE_COMPARE((Math::pack<Vector3ub, 6>(Vector3(0.0f, 0.5f, 1.0f))), Vector3ub(0, 32, 63));
 }
 
 void PackingTest::packSigned() {
     CORRADE_COMPARE(Math::pack<Byte>(-1.0f), -127);
-    CORRADE_COMPARE(Math::pack<Byte>(-0.732f), -92);
+    CORRADE_COMPARE(Math::pack<Byte>(-0.732f), -93);
     CORRADE_COMPARE(Math::pack<Byte>(0.0f), 0);
     CORRADE_COMPARE(Math::pack<Byte>(0.1357f), 17);
     CORRADE_COMPARE(Math::pack<Byte>(1.0f), 127);
@@ -219,13 +227,12 @@ void PackingTest::packSigned() {
     CORRADE_COMPARE(Math::pack<Short>(0.66f), 21626);
 
     /* Bits */
-    CORRADE_COMPARE((Math::pack<Short>(-0.5f)), -16383);
-    CORRADE_COMPARE((Math::pack<Short, 14>(-0.5f)), -4095);
+    CORRADE_COMPARE((Math::pack<Short>(-0.5f)), -16384);
+    CORRADE_COMPARE((Math::pack<Short, 14>(-0.5f)), -4096);
 
     /* Vector overloads */
-    CORRADE_COMPARE(Math::pack<Vector3b>(Vector3(0.0f, -1.0f, 0.5f)), Vector3b(0, -127, 63));
-    CORRADE_COMPARE((Math::pack<Vector3b, 6>(Vector3(0.0f, -1.0f, 0.5f))), Vector3b(0, -31, 15));
-
+    CORRADE_COMPARE(Math::pack<Vector3b>(Vector3(0.0f, -1.0f, 0.5f)), Vector3b(0, -127, 64));
+    CORRADE_COMPARE((Math::pack<Vector3b, 6>(Vector3(0.0f, -1.0f, 0.5f))), Vector3b(0, -31, 16));
 }
 
 void PackingTest::reunpackUnsinged() {