ObjImporter: support negative indices.

What the hell, why even is that a thing? Especially given it's never
actually using the feature in a way that would make the file smaller.

doc/changelog.dox

@@ -1097,6 +1097,7 @@ See also:
     consistent with @ref Trade::SceneData::findFieldId().
 -   Added @ref Trade::MeshData::attributeId() that returns ID of an attribute
     in a set of attributes of the same name
+-   @relativeref{Trade,ObjImporter} now supports negative indices
 -   Added @ref Trade::TextureType::Texture1DArray,
     @relativeref{Trade::TextureType,Texture2DArray} and
     @relativeref{Trade::TextureType,CubeMapArray} in order to be able to

src/MagnumPlugins/ObjImporter/ObjImporter.cpp

@@ -26,6 +26,7 @@
 
 #include "ObjImporter.h"
 
+#include <climits>
 #include <unordered_map>
 #include <Corrade/Containers/GrowableArray.h>
 #include <Corrade/Containers/Optional.h>
@@ -109,7 +110,7 @@ inline bool parseFloat(const char* const errorPrefix, const Containers::StringVi
     return true;
 }
 
-inline bool parseUnsignedInt(const char* const errorPrefix, const Containers::StringView string, UnsignedInt& out) {
+inline bool parseInt(const char* const errorPrefix, const Containers::StringView string, Int& out) {
     /** @todo replace with something that can parse non-null-terminated stuff,
         then drop this "too long" error */
     char buffer[128];
@@ -122,17 +123,17 @@ inline bool parseUnsignedInt(const char* const errorPrefix, const Containers::St
     std::memcpy(buffer, string.data(), size);
     buffer[size] = '\0';
     char* end;
-    /* Not using strtoul() here as on Windows it's 32-bit and we wouldn't be
+    /* Not using strtol() here as on Windows it's 32-bit and we wouldn't be
        able to detect overflows */
     /** @todo replace with something that can report errors in a non-insane
         way */
-    const std::uint64_t outLong = std::strtoull(buffer, &end, 10);
+    const std::int64_t outLong = std::strtoll(buffer, &end, 10);
     if(!string || std::size_t(end - buffer) != size) {
         Error{} << errorPrefix << "invalid integer literal" << string;
         return false;
     }
-    if(outLong > ~std::uint32_t{}) {
+    if(outLong < INT_MIN || outLong > INT_MAX) {
-        Error{} << errorPrefix << "too large integer literal" << string;
+        Error{} << errorPrefix << "too small or large integer literal" << string;
         return false;
     }
 
@@ -399,9 +400,14 @@ Containers::Optional<MeshData> ObjImporter::doMesh(const UnsignedInt id, Unsigne
 
                 /* The number before first slash is a position index */
                 const Containers::StringView foundSlash1 = indexTuple.findOr('/', indexTuple.end());
-                if(!parseUnsignedInt("Trade::ObjImporter::mesh():", indexTuple.prefix(foundSlash1.begin()), data[i][0]))
+                Int index;
+                if(!parseInt("Trade::ObjImporter::mesh():", indexTuple.prefix(foundSlash1.begin()), index))
                     return {};
-                data[i][0] -= mesh.positionIndexOffset;
+                /* If the number is negative, it counts from the end (-1 is
+                   the last known position at this point, counting from 1) */
+                if(index < 0)
+                    index += positions.size() + 1;
+                data[i][0] = index - mesh.positionIndexOffset;
 
                 /* If there was a slash, next is a texture coordinate or
                    empty */
@@ -409,18 +415,28 @@ Containers::Optional<MeshData> ObjImporter::doMesh(const UnsignedInt id, Unsigne
                     indexTuple = indexTuple.suffix(foundSlash1.end());
                     const Containers::StringView foundSlash2 = indexTuple.findOr('/', indexTuple.end());
                     if(!foundSlash2 || foundSlash2.begin() != indexTuple.begin()) {
-                        if(!parseUnsignedInt("Trade::ObjImporter::mesh():", indexTuple.prefix(foundSlash2.begin()), data[i][1]))
+                        if(!parseInt("Trade::ObjImporter::mesh():", indexTuple.prefix(foundSlash2.begin()), index))
                             return {};
-                        data[i][1] -= mesh.textureCoordinateIndexOffset;
+                        /* If the number is negative, it counts from the end
+                           (-1 is the last known texture coordinate at this
+                           point, counting from 1) */
+                        if(index < 0)
+                            index += textureCoordinates.size() + 1;
+                        data[i][1] = index - mesh.textureCoordinateIndexOffset;
                         ++textureCoordinateIndexCount;
                     }
 
                     /* If there was a second slash, last is a normal */
                     if(foundSlash2) {
                         indexTuple = indexTuple.suffix(foundSlash2.end());
-                        if(!parseUnsignedInt("Trade::ObjImporter::mesh():", indexTuple, data[i][2]))
+                        if(!parseInt("Trade::ObjImporter::mesh():", indexTuple, index))
                             return {};
-                        data[i][2] -= mesh.normalIndexOffset;
+                        /* If the number is negative, it counts from the end
+                           (-1 is the last known normal at this point, counting
+                           from 1) */
+                        if(index < 0)
+                            index += normals.size() + 1;
+                        data[i][2] = index - mesh.normalIndexOffset;
                         ++normalIndexCount;
                     }
                 }

src/MagnumPlugins/ObjImporter/ObjImporter.h

@@ -105,6 +105,11 @@ Meshes are imported as @ref MeshPrimitive::Triangles with
 positions with optional @ref VertexFormat::Vector3 normals and
 @ref VertexFormat::Vector2 texture coordinates, if present in the source file.
 
+Negative indices (where @cpp -1 @ce is the last position / texture coordinate
+/ normal known at given point in the file, @cpp -2 @ce is the second-to-last,
+etc.), produced for example by 3ds Max or [Mineways](http://mineways.com), are
+supported.
+
 Polygons (quads etc.) and material properties are currently not supported.
 */
 class MAGNUM_OBJIMPORTER_EXPORT ObjImporter: public AbstractImporter {

src/MagnumPlugins/ObjImporter/Test/CMakeLists.txt

@@ -60,6 +60,7 @@ corrade_add_test(ObjImporterTest ObjImporterTest.cpp
         mesh-named-first-unnamed.obj
         mesh-named-first-unnamed-index-first.obj
         mesh-named.obj
+        mesh-negative-indices.obj
         mesh-normals.obj
         mesh-positions-optional-coordinate.obj
         mesh-primitive-lines.obj

src/MagnumPlugins/ObjImporter/Test/ObjImporterTest.cpp

@@ -56,6 +56,8 @@ struct ObjImporterTest: TestSuite::Tester {
     void meshNormals();
     void meshTextureCoordinatesNormals();
 
+    void meshNegativeIndices();
+
     void meshIgnoredKeyword();
 
     void meshNamed();
@@ -120,7 +122,8 @@ const struct {
 } InvalidNumbersData[]{
     {"too long float literal", "too long numeric literal 1234.567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567"},
     {"too long integer literal", "too long numeric literal 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678"},
-    {"too large integer literal", "too large integer literal 4294967296"},
+    {"too small integer literal", "too small or large integer literal -2147483649"},
+    {"too large integer literal", "too small or large integer literal 2147483648"},
     {"invalid float literal", "invalid floating-point literal bleh"},
     {"invalid integer literal", "invalid integer literal bleh"},
     {"position index out of range", "index 3 out of range for 1 vertices"},
@@ -195,6 +198,8 @@ ObjImporterTest::ObjImporterTest() {
               &ObjImporterTest::meshNormals,
               &ObjImporterTest::meshTextureCoordinatesNormals,
 
+              &ObjImporterTest::meshNegativeIndices,
+
               &ObjImporterTest::meshIgnoredKeyword,
 
               &ObjImporterTest::meshNamed});
@@ -448,6 +453,62 @@ void ObjImporterTest::meshTextureCoordinatesNormals() {
         TestSuite::Compare::Container);
 }
 
+void ObjImporterTest::meshNegativeIndices() {
+    Containers::Pointer<AbstractImporter> importer = _manager.instantiate("ObjImporter");
+    CORRADE_VERIFY(importer->openFile(Utility::Path::join(OBJIMPORTER_TEST_DIR, "mesh-negative-indices.obj")));
+    CORRADE_COMPARE(importer->meshCount(), 1);
+
+    const Containers::Optional<MeshData> data = importer->mesh(0);
+    CORRADE_VERIFY(data);
+    CORRADE_COMPARE(data->primitive(), MeshPrimitive::Lines);
+    CORRADE_COMPARE(data->attributeCount(), 3);
+    CORRADE_COMPARE_AS(data->attribute<Vector3>(MeshAttribute::Position),
+        Containers::arrayView<Vector3>({
+            {0.5f, 2.0f, 3.0f},
+            {0.0f, 1.5f, 1.0f},
+            {0.5f, 2.0f, 3.0f},
+            {0.0f, 1.5f, 1.0f},
+            {0.0f, 1.5f, 1.0f},
+            /* The first 5 elements should be the same as in
+               meshTextureCoordinatesNormals() */
+            {2.0f, 1.5f, 0.0f},
+            {1.5f, 0.0f, 2.0f}
+        }), TestSuite::Compare::Container);
+    CORRADE_COMPARE_AS(data->attribute<Vector2>(MeshAttribute::TextureCoordinates),
+        Containers::arrayView<Vector2>({
+            {1.0f, 0.5f},
+            {1.0f, 0.5f},
+            {0.5f, 1.0f},
+            {0.5f, 1.0f},
+            {0.5f, 1.0f},
+            /* The first 5 elements should be the same as in
+               meshTextureCoordinatesNormals() */
+            {0.5f, 1.0f},
+            {0.0f, 0.5f}
+        }), TestSuite::Compare::Container);
+    CORRADE_COMPARE_AS(data->attribute<Vector3>(MeshAttribute::Normal),
+        Containers::arrayView<Vector3>({
+            {1.0f, 0.5f, 3.5f},
+            {0.5f, 1.0f, 0.5f},
+            {0.5f, 1.0f, 0.5f},
+            {1.0f, 0.5f, 3.5f},
+            {0.5f, 1.0f, 0.5f},
+            /* The first 5 elements should be the same as in
+               meshTextureCoordinatesNormals() */
+            {0.5f, 1.0f, 0.5f},
+            {1.0f, 0.5f, 3.5f}
+        }), TestSuite::Compare::Container);
+    CORRADE_VERIFY(data->isIndexed());
+    CORRADE_COMPARE(data->indexType(), MeshIndexType::UnsignedInt);
+    CORRADE_COMPARE_AS(data->indices<UnsignedInt>(),
+        Containers::arrayView<UnsignedInt>({
+            0, 1, 2, 3, 1, 0, 4, 2,
+            /* The first 8 elements should be the same as in
+               meshTextureCoordinatesNormals() */
+            5, 6
+        }), TestSuite::Compare::Container);
+}
+
 void ObjImporterTest::meshIgnoredKeyword() {
     Containers::Pointer<AbstractImporter> importer = _manager.instantiate("ObjImporter");
     CORRADE_VERIFY(importer->openFile(Utility::Path::join(OBJIMPORTER_TEST_DIR, "mesh-ignored-keyword.obj")));

src/MagnumPlugins/ObjImporter/Test/invalid-numbers.obj

@@ -4,8 +4,11 @@ v 1234.5678901234567890123456789012345678901234567890123456789012345678901234567
 o too long integer literal
 p 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
 
+o too small integer literal
+p -2147483649
+
 o too large integer literal
-p 4294967296
+p 2147483648
 
 o invalid float literal
 v 1 bleh 2

src/MagnumPlugins/ObjImporter/Test/mesh-negative-indices.obj

@@ -0,0 +1,35 @@
+# OBJ files produced by http://mineways.com have this, for example the Rungholt
+# scene from the McGuire archive (https://casual-effects.com/data/) has this.
+# Not sure what's the point of having such a feature or even using such a
+# feature, since the file starts with
+#
+#   f -53570/-200/-18 -53569/-199/-18 -53568/-198/-18 -53567/-197/-18
+#
+# and then the numbers just decrease going forward. I would see a point when
+# the face and vertex declarations were interleaved -- to make the indices
+# shorter -- but not like this. Honestly this looks like once upon a time,
+# someone just randomly flipped signs until the export-import cycle started
+# making sense, and then the other importers had to follow suit?! The same
+# insanity is exported by 3dsMax: https://github.com/CGAL/cgal/issues/4062
+
+v 0.5 2 3
+v 0 1.5 1
+vt 1 0.5
+vt 0.5 1
+vn 1 0.5 3.5
+vn 0.5 1 0.5
+
+# -2 is 1, -1 is 2 for all
+l -2/-2/-2 -1/-2/-1
+l -2/-1/-1 -1/-1/-2
+
+v 1.5 0 2
+v 2 1.5 0
+vt 0 0.5
+
+# -4 is 1, -3 is 2, -2 is 3, -1 is 4 for positions
+# -3 is 1, -2 is 2, -1 is 3 for texture coordinates
+# -2 is 1, -1 is 2 for normals
+l -3/-3/-1 -4/-3/-2
+l -3/-2/-1 -4/-2/-1
+l -1/-2/-1 -2/-1/-2