From cfe828b452e6b0c94219b4cd8d60757ea0645aa3 Mon Sep 17 00:00:00 2001
From: Martin Heistermann <mheistermann@noreply.localhost>
Date: Tue, 14 Feb 2023 16:30:50 +0100
Subject: [PATCH] OBJ: Support polylines with more than 2 vertices.

The OBJ spec (page B1-17) allows "l" entries to specify
polylines with more than 2 vertices, optionally with texture
coordinates.

Previously, only the first 2 vertices of each polyline
were read and added as loose edges, failing when texture
coordinates were present.

This adds support for proper polylines, reading but ignoring
texture coordinates.

Pull Request #104503
---
 .../importer/obj_import_file_reader.cc        | 76 +++++++++++++++----
 .../wavefront_obj/tests/obj_importer_tests.cc |  9 +++
 2 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc b/source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
index f0ae807b018..edd46ccd690 100644
--- a/source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
+++ b/source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
@@ -152,21 +152,67 @@ static void geom_add_uv_vertex(const char *p, const char *end, GlobalVertices &r
   r_global_vertices.uv_vertices.append(uv);
 }
 
-static void geom_add_edge(Geometry *geom,
-                          const char *p,
-                          const char *end,
-                          GlobalVertices &r_global_vertices)
+/**
+ * Parse vertex index and transform to non-negative, zero-based.
+ * Sets r_index to the index or INT32_MAX on error.
+ * Index is transformed and bounds-checked using n_vertices,
+ * which specifies the number of vertices that have been read before.
+ * Returns updated p.
+ */
+static const char *parse_vertex_index(const char *p, const char *end, size_t n_elems, int &r_index)
 {
-  int edge_v1, edge_v2;
-  p = parse_int(p, end, -1, edge_v1);
-  p = parse_int(p, end, -1, edge_v2);
-  /* Always keep stored indices non-negative and zero-based. */
-  edge_v1 += edge_v1 < 0 ? r_global_vertices.vertices.size() : -1;
-  edge_v2 += edge_v2 < 0 ? r_global_vertices.vertices.size() : -1;
-  BLI_assert(edge_v1 >= 0 && edge_v2 >= 0);
-  geom->edges_.append({uint(edge_v1), uint(edge_v2)});
-  geom->track_vertex_index(edge_v1);
-  geom->track_vertex_index(edge_v2);
+  p = parse_int(p, end, INT32_MAX, r_index, false);
+  if (r_index != INT32_MAX) {
+    r_index += r_index < 0 ? n_elems : -1;
+    if (r_index < 0 || r_index >= n_elems) {
+      fprintf(stderr, "Invalid vertex index %i (valid range [0, %zu))\n", r_index, n_elems);
+      r_index = INT32_MAX;
+    }
+  }
+  return p;
+}
+
+/**
+ * Parse a polyline and add its line segments as loose edges.
+ * We support the following polyline specifications:
+ *  - "l v1/vt1 v2/vt2 ..."
+ *  - "l v1 v2 ..."
+ *  If a line only has one vertex (technically not allowed by the spec),
+ *  no line is created, but the vertex will be added to
+ *  the mesh even if it is unconnected.
+ */
+static void geom_add_polyline(Geometry *geom,
+                              const char *p,
+                              const char *end,
+                              GlobalVertices &r_global_vertices)
+{
+  int last_vertex_index;
+  p = drop_whitespace(p, end);
+  p = parse_vertex_index(p, end, r_global_vertices.vertices.size(), last_vertex_index);
+
+  if (last_vertex_index == INT32_MAX) {
+    fprintf(stderr, "Skipping invalid OBJ polyline.\n");
+    return;
+  }
+  geom->track_vertex_index(last_vertex_index);
+
+  while (p < end) {
+    int vertex_index;
+
+    /* Lines can contain texture coordinate indices, just ignore them. */
+    p = drop_non_whitespace(p, end);
+    /* Skip whitespace to get to the next vertex. */
+    p = drop_whitespace(p, end);
+
+    p = parse_vertex_index(p, end, r_global_vertices.vertices.size(), vertex_index);
+    if (vertex_index == INT32_MAX) {
+      break;
+    }
+
+    geom->edges_.append({uint(last_vertex_index), uint(vertex_index)});
+    geom->track_vertex_index(vertex_index);
+    last_vertex_index = vertex_index;
+  }
 }
 
 static void geom_add_polygon(Geometry *geom,
@@ -548,7 +594,7 @@ void OBJParser::parse(Vector<std::unique_ptr<Geometry>> &r_all_geometries,
       }
       /* Faces. */
       else if (parse_keyword(p, end, "l")) {
-        geom_add_edge(curr_geom, p, end, r_global_vertices);
+        geom_add_polyline(curr_geom, p, end, r_global_vertices);
       }
       /* Objects. */
       else if (parse_keyword(p, end, "o")) {
diff --git a/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc b/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc
index f15687e7bef..b2ba10a7abd 100644
--- a/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc
+++ b/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc
@@ -869,4 +869,13 @@ TEST_F(obj_importer_test, import_split_options_none)
   import_and_check("split_options.obj", expect, std::size(expect), 0);
 }
 
+TEST_F(obj_importer_test, import_polylines)
+{
+  Expectation expect[] = {
+      {"OBCube", OB_MESH, 8, 12, 6, 24, float3(1, 1, -1), float3(-1, 1, 1)},
+      {"OBpolylines", OB_MESH, 13, 8, 0, 0, float3(1, 0, 0), float3(.7, .7, 2)},
+  };
+  import_and_check("polylines.obj", expect, std::size(expect), 0);
+}
+
 }  // namespace blender::io::obj