[Bf-blender-cvs] [a3eb4027c23] master: Fix T97095: export of Poly curves, export crash when object contains multiple curve types

Aras Pranckevicius noreply at git.blender.org
Sun Apr 17 21:01:01 CEST 2022


Commit: a3eb4027c2383827b9f5beed709c54c53c7d6d20
Author: Aras Pranckevicius
Date:   Sun Apr 17 21:59:55 2022 +0300
Branches: master
https://developer.blender.org/rBa3eb4027c2383827b9f5beed709c54c53c7d6d20

Fix T97095: export of Poly curves, export crash when object contains multiple curve types

- Was not exporting "Poly" curves at all,
- Had a crash when a single object contains multiple curves of different types -- it had a check for "is this nurbs compatible?" only for the first curve, and then proceeded to treat the other curves as nurbs as well, without checking for validity.

Fixed both issues by doing the same logic as in the old python exporter:
- Poly curves are supported,
- Treat object as "nurbs compatible" only if all the curves within it are nurbs compatible.

Added test coverage in the gtest suite. While at it, made "all_curves" test use the "golden obj file template" style test, instead of a manually coded test that checks intermediate objects but does not check the final exported result.

Reviewed By: Howard Trickey
Differential Revision: https://developer.blender.org/D14611

===================================================================

M	source/blender/io/wavefront_obj/exporter/obj_export_nurbs.cc
M	source/blender/io/wavefront_obj/exporter/obj_exporter.cc
M	source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc
M	source/blender/io/wavefront_obj/tests/obj_exporter_tests.hh
M	source/blender/io/wavefront_obj/tests/obj_importer_tests.cc

===================================================================

diff --git a/source/blender/io/wavefront_obj/exporter/obj_export_nurbs.cc b/source/blender/io/wavefront_obj/exporter/obj_export_nurbs.cc
index 17069c18295..c247048ce13 100644
--- a/source/blender/io/wavefront_obj/exporter/obj_export_nurbs.cc
+++ b/source/blender/io/wavefront_obj/exporter/obj_export_nurbs.cc
@@ -72,12 +72,12 @@ float3 OBJCurve::vertex_coordinates(const int spline_index,
 int OBJCurve::total_spline_control_points(const int spline_index) const
 {
   const Nurb *const nurb = static_cast<Nurb *>(BLI_findlink(&export_curve_->nurb, spline_index));
-  const int r_nurbs_degree = nurb->orderu - 1;
+  int degree = nurb->type == CU_POLY ? 1 : nurb->orderu - 1;
   /* Total control points = Number of points in the curve (+ degree of the
    * curve if it is cyclic). */
   int r_tot_control_points = nurb->pntsv * nurb->pntsu;
   if (nurb->flagu & CU_NURB_CYCLIC) {
-    r_tot_control_points += r_nurbs_degree;
+    r_tot_control_points += degree;
   }
   return r_tot_control_points;
 }
@@ -85,7 +85,7 @@ int OBJCurve::total_spline_control_points(const int spline_index) const
 int OBJCurve::get_nurbs_degree(const int spline_index) const
 {
   const Nurb *const nurb = static_cast<Nurb *>(BLI_findlink(&export_curve_->nurb, spline_index));
-  return nurb->orderu - 1;
+  return nurb->type == CU_POLY ? 1 : nurb->orderu - 1;
 }
 
 short OBJCurve::get_nurbs_flagu(const int spline_index) const
diff --git a/source/blender/io/wavefront_obj/exporter/obj_exporter.cc b/source/blender/io/wavefront_obj/exporter/obj_exporter.cc
index 584d8ae4ec0..78b709c884a 100644
--- a/source/blender/io/wavefront_obj/exporter/obj_exporter.cc
+++ b/source/blender/io/wavefront_obj/exporter/obj_exporter.cc
@@ -68,6 +68,17 @@ static void print_exception_error(const std::system_error &ex)
             << std::endl;
 }
 
+static bool is_curve_nurbs_compatible(const Nurb *nurb)
+{
+  while (nurb) {
+    if (nurb->type == CU_BEZIER || nurb->pntsv != 1) {
+      return false;
+    }
+    nurb = nurb->next;
+  }
+  return true;
+}
+
 /**
  * Filter supported objects from the Scene.
  *
@@ -104,27 +115,13 @@ filter_supported_objects(Depsgraph *depsgraph, const OBJExportParams &export_par
           }
           break;
         }
-        switch (nurb->type) {
-          case CU_NURBS:
-            if (export_params.export_curves_as_nurbs) {
-              /* Export in parameter form: control points. */
-              r_exportable_nurbs.append(
-                  std::make_unique<OBJCurve>(depsgraph, export_params, object));
-            }
-            else {
-              /* Export in mesh form: edges and vertices. */
-              r_exportable_meshes.append(
-                  std::make_unique<OBJMesh>(depsgraph, export_params, object));
-            }
-            break;
-          case CU_BEZIER:
-            /* Always export in mesh form: edges and vertices. */
-            r_exportable_meshes.append(
-                std::make_unique<OBJMesh>(depsgraph, export_params, object));
-            break;
-          default:
-            /* Other curve types are not supported. */
-            break;
+        if (export_params.export_curves_as_nurbs && is_curve_nurbs_compatible(nurb)) {
+          /* Export in parameter form: control points. */
+          r_exportable_nurbs.append(std::make_unique<OBJCurve>(depsgraph, export_params, object));
+        }
+        else {
+          /* Export in mesh form: edges and vertices. */
+          r_exportable_meshes.append(std::make_unique<OBJMesh>(depsgraph, export_params, object));
         }
         break;
       }
diff --git a/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc b/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc
index d80e76fd965..f74bfc155dd 100644
--- a/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc
+++ b/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc
@@ -48,7 +48,6 @@ class obj_exporter_test : public BlendfileLoadingBaseTest {
 };
 
 const std::string all_objects_file = "io_tests/blend_scene/all_objects.blend";
-const std::string all_curve_objects_file = "io_tests/blend_scene/all_curves.blend";
 
 TEST_F(obj_exporter_test, filter_objects_curves_as_mesh)
 {
@@ -58,7 +57,7 @@ TEST_F(obj_exporter_test, filter_objects_curves_as_mesh)
     return;
   }
   auto [objmeshes, objcurves]{filter_supported_objects(depsgraph, _export.params)};
-  EXPECT_EQ(objmeshes.size(), 19);
+  EXPECT_EQ(objmeshes.size(), 20);
   EXPECT_EQ(objcurves.size(), 0);
 }
 
@@ -72,7 +71,7 @@ TEST_F(obj_exporter_test, filter_objects_curves_as_nurbs)
   _export.params.export_curves_as_nurbs = true;
   auto [objmeshes, objcurves]{filter_supported_objects(depsgraph, _export.params)};
   EXPECT_EQ(objmeshes.size(), 18);
-  EXPECT_EQ(objcurves.size(), 2);
+  EXPECT_EQ(objcurves.size(), 3);
 }
 
 TEST_F(obj_exporter_test, filter_objects_selected)
@@ -111,64 +110,6 @@ TEST(obj_exporter_utils, append_positive_frame_to_filename)
   EXPECT_EQ_ARRAY(path_with_frame, path_truth, BLI_strlen_utf8(path_truth));
 }
 
-TEST_F(obj_exporter_test, curve_nurbs_points)
-{
-  if (!load_file_and_depsgraph(all_curve_objects_file)) {
-    ADD_FAILURE();
-    return;
-  }
-
-  OBJExportParamsDefault _export;
-  _export.params.export_curves_as_nurbs = true;
-  auto [objmeshes_unused, objcurves]{filter_supported_objects(depsgraph, _export.params)};
-
-  for (auto &objcurve : objcurves) {
-    if (all_nurbs_truth.count(objcurve->get_curve_name()) != 1) {
-      ADD_FAILURE();
-      return;
-    }
-    const NurbsObject *const nurbs_truth = all_nurbs_truth.at(objcurve->get_curve_name()).get();
-    EXPECT_EQ(objcurve->total_splines(), nurbs_truth->total_splines());
-    for (int spline_index : IndexRange(objcurve->total_splines())) {
-      EXPECT_EQ(objcurve->total_spline_vertices(spline_index),
-                nurbs_truth->total_spline_vertices(spline_index));
-      EXPECT_EQ(objcurve->get_nurbs_degree(spline_index),
-                nurbs_truth->get_nurbs_degree(spline_index));
-      EXPECT_EQ(objcurve->total_spline_control_points(spline_index),
-                nurbs_truth->total_spline_control_points(spline_index));
-    }
-  }
-}
-
-TEST_F(obj_exporter_test, curve_coordinates)
-{
-  if (!load_file_and_depsgraph(all_curve_objects_file)) {
-    ADD_FAILURE();
-    return;
-  }
-
-  OBJExportParamsDefault _export;
-  _export.params.export_curves_as_nurbs = true;
-  auto [objmeshes_unused, objcurves]{filter_supported_objects(depsgraph, _export.params)};
-
-  for (auto &objcurve : objcurves) {
-    if (all_nurbs_truth.count(objcurve->get_curve_name()) != 1) {
-      ADD_FAILURE();
-      return;
-    }
-    const NurbsObject *const nurbs_truth = all_nurbs_truth.at(objcurve->get_curve_name()).get();
-    EXPECT_EQ(objcurve->total_splines(), nurbs_truth->total_splines());
-    for (int spline_index : IndexRange(objcurve->total_splines())) {
-      for (int vertex_index : IndexRange(objcurve->total_spline_vertices(spline_index))) {
-        EXPECT_V3_NEAR(objcurve->vertex_coordinates(
-                           spline_index, vertex_index, _export.params.scaling_factor),
-                       nurbs_truth->vertex_coordinates(spline_index, vertex_index),
-                       0.000001f);
-      }
-    }
-  }
-}
-
 static std::unique_ptr<OBJWriter> init_writer(const OBJExportParams &params,
                                               const std::string out_filepath)
 {
@@ -517,6 +458,25 @@ TEST_F(obj_exporter_regression_test, suzanne_all_data)
                                _export.params);
 }
 
+TEST_F(obj_exporter_regression_test, all_curves)
+{
+  OBJExportParamsDefault _export;
+  _export.params.export_materials = false;
+  compare_obj_export_to_golden(
+      "io_tests/blend_scene/all_curves.blend", "io_tests/obj/all_curves.obj", "", _export.params);
+}
+
+TEST_F(obj_exporter_regression_test, all_curves_as_nurbs)
+{
+  OBJExportParamsDefault _export;
+  _export.params.export_materials = false;
+  _export.params.export_curves_as_nurbs = true;
+  compare_obj_export_to_golden("io_tests/blend_scene/all_curves.blend",
+                               "io_tests/obj/all_curves_as_nurbs.obj",
+                               "",
+                               _export.params);
+}
+
 TEST_F(obj_exporter_regression_test, all_objects)
 {
   OBJExportParamsDefault _export;
diff --git a/source/blender/io/wavefront_obj/tests/obj_exporter_tests.hh b/source/blender/io/wavefront_obj/tests/obj_exporter_tests.hh
index 42660bbbe56..6a821e0b1bf 100644
--- a/source/blender/io/wavefront_obj/tests/obj_exporter_tests.hh
+++ b/source/blender/io/wavefront_obj/tests/obj_exporter_tests.hh
@@ -1,77 +1,11 @@
 /* SPDX-License-Identifier: Apache-2.0 */
 
-/**
- * This file contains default values for several items like
- * vertex coordinates, export parameters, MTL values etc.
- */
-
 #pragma once
 
-#include <array>
-#include <gtest/gtest.h>
-#include <string>
-#include <vector>
-
 #include "IO_wavefront_obj.h"
 
 namespace blender::io::obj {
 
-using array_float_3 = std::array<float, 3>;
-
-/**
- * This matches #OBJCurve's member functions, except that all the numbers and names are known
- * constants. Used to store expected values of NURBS curves objects.
- */
-class NurbsObject {
- private:
-  std::string nurbs_name_;
-  /* The indices in these vectors are spline indices. */
-  std::vector<std::vector<array_float_3>> coordinates_;
-  std::vector<int> degrees_;
-  std::vector<int> control_points_;
-
- public:
-  NurbsObject(const std::string nurbs_name,
-              const std::vector<std::vector<array_float_3>> coordinates,
-              const std::vector<int> degrees,
-              const std::vector<int> control_points)
-      : nurbs_name_(nurbs_name),
-        coordinates_(coordinates),
-        degrees_(degrees),
-        control_points_(control_points)
-  {
-  }
-
-  int total_splines() const
-  {
-    return coordinates_.size();
-  }
-
-  int total_spline_vertices(const int spline_index) const
-  {
-    if (spline_index >= coordinates_.size()) {
-      ADD_FAILURE();
-      return 0;
-    }
-    return coordinates_[spline_index].size();
-  }
-
-  const float *vertex_coordinates(const int spline_index, const int vertex_index) const
-  {
-    return coordinat

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list