diff options
author | Aras Pranckevicius <aras@nesnausk.org> | 2022-04-17 21:59:55 +0300 |
---|---|---|
committer | Aras Pranckevicius <aras@nesnausk.org> | 2022-04-17 21:59:55 +0300 |
commit | a3eb4027c2383827b9f5beed709c54c53c7d6d20 (patch) | |
tree | f2b73785a7e54190fd22598659f71fb019b7030d /source/blender | |
parent | b32cb0266c61abb333906dc6848d252f6f4dbd20 (diff) |
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
Diffstat (limited to 'source/blender')
5 files changed, 43 insertions, 195 deletions
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 ¶ms, 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 coordinates_[spline_index][vertex_index].data(); - } - - int get_nurbs_degree(const int spline_index) const - { - return degrees_[spline_index]; - } - - int total_spline_control_points(const int spline_index) const - { - return control_points_[spline_index]; - } -}; - struct OBJExportParamsDefault { OBJExportParams params; OBJExportParamsDefault() @@ -103,48 +37,4 @@ struct OBJExportParamsDefault { } }; -const std::vector<std::vector<array_float_3>> coordinates_NurbsCurve{ - {{6.94742, 0.000000, 0.000000}, - {7.44742, 0.000000, -1.000000}, - {9.44742, 0.000000, -1.000000}, - {9.94742, 0.000000, 0.000000}}}; -const std::vector<std::vector<array_float_3>> coordinates_NurbsCircle{ - {{11.463165, 0.000000, 1.000000}, - {10.463165, 0.000000, 1.000000}, - {10.463165, 0.000000, 0.000000}, - {10.463165, 0.000000, -1.000000}, - {11.463165, 0.000000, -1.000000}, - {12.463165, 0.000000, -1.000000}, - {12.463165, 0.000000, 0.000000}, - {12.463165, 0.000000, 1.000000}}}; -const std::vector<std::vector<array_float_3>> coordinates_NurbsPathCurve{ - {{13.690557, 0.000000, 0.000000}, - {14.690557, 0.000000, 0.000000}, - {15.690557, 0.000000, 0.000000}, - {16.690557, 0.000000, 0.000000}, - {17.690557, 0.000000, 0.000000}}, - {{14.192808, 0.000000, 0.000000}, - {14.692808, 0.000000, -1.000000}, - {16.692808, 0.000000, -1.000000}, - {17.192808, 0.000000, 0.000000}}}; - -const std::map<std::string, std::unique_ptr<NurbsObject>> all_nurbs_truth = []() { - std::map<std::string, std::unique_ptr<NurbsObject>> all_nurbs; - all_nurbs.emplace( - "NurbsCurve", - /* Name, coordinates, degrees of splines, control points of splines. */ - std::make_unique<NurbsObject>( - "NurbsCurve", coordinates_NurbsCurve, std::vector<int>{3}, std::vector<int>{4})); - all_nurbs.emplace( - "NurbsCircle", - std::make_unique<NurbsObject>( - "NurbsCircle", coordinates_NurbsCircle, std::vector<int>{3}, std::vector<int>{11})); - /* This is actually an Object containing a NurbsPath and a NurbsCurve spline. */ - all_nurbs.emplace("NurbsPathCurve", - std::make_unique<NurbsObject>("NurbsPathCurve", - coordinates_NurbsPathCurve, - std::vector<int>{3, 3}, - std::vector<int>{5, 4})); - return all_nurbs; -}(); } // namespace blender::io::obj 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 9dd9e7c1a37..611e0cbb209 100644 --- a/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc +++ b/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc @@ -444,6 +444,7 @@ TEST_F(obj_importer_test, import_all_objects) float3(5, 1, 1), float3(0, 0, 1), float2(0.654526f, 0.579873f)}, + {"OBNurbsCircle.001", OB_MESH, 4, 4, 0, 0, float3(2, -3, 0), float3(3, -2, 0)}, {"OBSurface", OB_MESH, 256, |