From db409c21939e3b1849ad10ff3bd5619c87006aff Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Fri, 18 Nov 2022 17:24:10 +0000 Subject: [PATCH 01/19] This patch addresses the feature request T102376 Differential Revision: https://developer.blender.org/D16545 --- source/blender/io/usd/CMakeLists.txt | 2 + .../io/usd/intern/usd_hierarchy_iterator.cc | 11 +- .../io/usd/intern/usd_writer_curves.cc | 368 ++++++++++++++++++ .../blender/io/usd/intern/usd_writer_curves.h | 47 +++ 4 files changed, 427 insertions(+), 1 deletion(-) create mode 100644 source/blender/io/usd/intern/usd_writer_curves.cc create mode 100644 source/blender/io/usd/intern/usd_writer_curves.h diff --git a/source/blender/io/usd/CMakeLists.txt b/source/blender/io/usd/CMakeLists.txt index f6d2b3f4548..7a076abcdd0 100644 --- a/source/blender/io/usd/CMakeLists.txt +++ b/source/blender/io/usd/CMakeLists.txt @@ -74,6 +74,7 @@ set(SRC intern/usd_hierarchy_iterator.cc intern/usd_writer_abstract.cc intern/usd_writer_camera.cc + intern/usd_writer_curves.cc intern/usd_writer_hair.cc intern/usd_writer_light.cc intern/usd_writer_material.cc @@ -103,6 +104,7 @@ set(SRC intern/usd_hierarchy_iterator.h intern/usd_writer_abstract.h intern/usd_writer_camera.h + intern/usd_writer_curves.h intern/usd_writer_hair.h intern/usd_writer_light.h intern/usd_writer_material.h diff --git a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc index 5c23742fe50..60379a33f0e 100644 --- a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc +++ b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc @@ -5,6 +5,7 @@ #include "usd_hierarchy_iterator.h" #include "usd_writer_abstract.h" #include "usd_writer_camera.h" +#include "usd_writer_curves.h" #include "usd_writer_hair.h" #include "usd_writer_light.h" #include "usd_writer_mesh.h" @@ -16,6 +17,7 @@ #include +#include "BKE_curve_legacy_convert.hh" #include "BKE_duplilist.h" #include "BLI_assert.h" @@ -105,12 +107,19 @@ AbstractHierarchyWriter *USDHierarchyIterator::create_data_writer(const Hierarch case OB_MBALL: data_writer = new USDMetaballWriter(usd_export_context); break; + case OB_CURVES_LEGACY: { + auto legacy_curve = static_cast(context->object->data); + auto curves = bke::curve_legacy_to_curves(*legacy_curve); + data_writer = new USDCurvesWriter(usd_export_context, curves); + } break; + case OB_CURVES: + data_writer = new USDCurvesWriter(usd_export_context); + break; case OB_VOLUME: data_writer = new USDVolumeWriter(usd_export_context); break; case OB_EMPTY: - case OB_CURVES_LEGACY: case OB_SURF: case OB_FONT: case OB_SPEAKER: diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc new file mode 100644 index 00000000000..9394e63c583 --- /dev/null +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -0,0 +1,368 @@ +/* + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * The Original Code is Copyright (C) 2019 Blender Foundation. + * All rights reserved. + */ +#include "usd_writer_curves.h" +#include "usd_hierarchy_iterator.h" + +#include +#include +#include +#include +#include +#include + +#include "BKE_curves.hh" + +#include + +extern "C" { + +#include "BKE_material.h" + +#include "BLI_math_geom.h" + +#include "WM_api.h" +#include "WM_types.h" +} + +namespace blender::io::usd { + +USDCurvesWriter::USDCurvesWriter(const USDExporterContext &ctx) + : USDAbstractWriter(ctx), converted_curves(nullptr) +{ +} + +USDCurvesWriter::USDCurvesWriter(const USDExporterContext &ctx, Curves *converted_legacy_curves) + : USDAbstractWriter(ctx), converted_curves(converted_legacy_curves) +{ +} + +/* +void USDCurvesWriter::do_write(HierarchyContext &context) +{ + Curves *curve = static_cast(context.object->data); + bke::CurvesGeometry &geometry = bke::CurvesGeometry::wrap(curve->geometry); + + pxr::UsdTimeCode timecode = get_export_time_code(); + + const int num_curves = geometry.curve_num; + const Span positions = geometry.positions(); + + const bke::AttributeAccessor curve_attributes = geometry.attributes(); + auto radii = curve_attributes.lookup_or_default("radius", ATTR_DOMAIN_POINT, 0.0f); + + for (int i_curve = 0; i_curve < num_curves; i_curve++) { + + pxr::UsdGeomCurves curve; + int8_t curve_type = geometry.curve_types()[i_curve]; + + switch (curve_type) { + case CURVE_TYPE_CATMULL_ROM: + curve = pxr::UsdGeomBasisCurves::Define(usd_export_context_.stage, + usd_export_context_.usd_path); + pxr::UsdGeomBasisCurves(curve).CreateBasisAttr( + pxr::VtValue(pxr::UsdGeomTokens->catmullRom)); + + pxr::UsdGeomBasisCurves(curve).CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->linear)); + break; + case CURVE_TYPE_POLY: + curve = pxr::UsdGeomBasisCurves::Define(usd_export_context_.stage, + usd_export_context_.usd_path); + pxr::UsdGeomBasisCurves(curve).CreateBasisAttr(pxr::VtValue(pxr::UsdGeomTokens->bezier)); + + + pxr::UsdGeomBasisCurves(curve).CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->linear)); + return; + case CURVE_TYPE_BEZIER: + curve = pxr::UsdGeomBasisCurves::Define(usd_export_context_.stage, + usd_export_context_.usd_path); + pxr::UsdGeomBasisCurves(curve).CreateBasisAttr(pxr::VtValue(pxr::UsdGeomTokens->bezier)); + + pxr::UsdGeomBasisCurves(curve).CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->linear)); + return; + case CURVE_TYPE_NURBS: + curve = pxr::UsdGeomNurbsCurves::Define(usd_export_context_.stage, + usd_export_context_.usd_path); + break; + default: + BLI_assert_unreachable(); + } + + bool cyclic = geometry.cyclic()[i_curve]; + if (cyclic) { + pxr::UsdGeomBasisCurves(curve).CreateWrapAttr(pxr::VtValue(pxr::UsdGeomTokens->periodic)); + } + else { + pxr::UsdGeomBasisCurves(curve).CreateWrapAttr(pxr::VtValue(pxr::UsdGeomTokens->nonperiodic)); + } + + pxr::VtArray verts; + pxr::VtIntArray curve_point_counts; + pxr::VtArray widths; + + long tot_points = 0; + for (const auto i_point : geometry.points_for_curve(i_curve)) { + verts.push_back( + pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); + widths.push_back(radii[i_point] * 2.0f); + tot_points++; + } + + curve_point_counts.push_back(tot_points); + + pxr::UsdAttribute attr_points = curve.CreatePointsAttr(pxr::VtValue(), true); + usd_value_writer_.SetAttribute(attr_points, pxr::VtValue(verts), timecode); + + pxr::UsdAttribute attr_vertex_counts = curve.CreateCurveVertexCountsAttr(pxr::VtValue(), + true); + usd_value_writer_.SetAttribute(attr_vertex_counts, pxr::VtValue(curve_point_counts), timecode); + + pxr::UsdAttribute attr_widths = curve.CreateWidthsAttr(pxr::VtValue(), true); + usd_value_writer_.SetAttribute(attr_widths, pxr::VtValue(widths), timecode); + } + + +} +*/ + +pxr::UsdGeomCurves USDCurvesWriter::DefineUsdGeomBasisCurves(pxr::VtValue curve_basis, + bool cyclic, + bool cubic) +{ + pxr::UsdGeomCurves curves = pxr::UsdGeomBasisCurves::Define(usd_export_context_.stage, + usd_export_context_.usd_path); + + // Not required to set the basis attribute for linear curves + // https://graphics.pixar.com/usd/dev/api/class_usd_geom_basis_curves.html#details + if (cubic) { + pxr::UsdGeomBasisCurves(curves).CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->cubic)); + pxr::UsdGeomBasisCurves(curves).CreateBasisAttr(curve_basis); + } + else { + pxr::UsdGeomBasisCurves(curves).CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->linear)); + } + + if (cyclic) { + pxr::UsdGeomBasisCurves(curves).CreateWrapAttr(pxr::VtValue(pxr::UsdGeomTokens->periodic)); + } + else { + pxr::UsdGeomBasisCurves(curves).CreateWrapAttr(pxr::VtValue(pxr::UsdGeomTokens->nonperiodic)); + } + + return curves; +} + +void populate_curve_verts(const bke::CurvesGeometry &geometry, + pxr::VtArray &verts, + pxr::VtIntArray &curve_point_counts, + pxr::VtArray &widths) +{ + const int num_curves = geometry.curve_num; + const Span positions = geometry.positions(); + const bke::AttributeAccessor curve_attributes = geometry.attributes(); + + const VArray radii = curve_attributes.lookup_or_default( + "radius", ATTR_DOMAIN_POINT, 0.0f); + + for (int i_curve = 0; i_curve < num_curves; i_curve++) { + auto curve_points = geometry.points_for_curve(i_curve); + long tot_points = 0; + for (const auto i_point : curve_points) { + widths.push_back(radii[i_point] * 2.0f); + + verts.push_back( + pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); + tot_points++; + } + + curve_point_counts.push_back(tot_points); + } +} + +void populate_curve_verts_for_bezier(const bke::CurvesGeometry &geometry, + pxr::VtArray &verts, + pxr::VtIntArray &curve_point_counts, + pxr::VtArray &widths) +{ + const int num_curves = geometry.curve_num; + const Span positions = geometry.positions(); + + const bke::AttributeAccessor curve_attributes = geometry.attributes(); + const VArray radii = curve_attributes.lookup_or_default( + "radius", ATTR_DOMAIN_POINT, 0.0f); + + const Span handles_l = geometry.handle_positions_left(); + const Span handles_r = geometry.handle_positions_right(); + + for (int i_curve = 0; i_curve < num_curves; i_curve++) { + auto curve_points = geometry.points_for_curve(i_curve); + long tot_points = 0; + for (const auto i_point : curve_points) { + widths.push_back(radii[i_point] * 2.0f); + + verts.push_back( + pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); + tot_points++; + + if (i_point < curve_points.size() - 1) { + verts.push_back( + pxr::GfVec3f(handles_r[i_point][0], handles_r[i_point][1], handles_r[i_point][2])); + verts.push_back(pxr::GfVec3f( + handles_l[i_point + 1][0], handles_l[i_point + 1][1], handles_l[i_point + 1][2])); + tot_points += 2; + } + } + + curve_point_counts.push_back(tot_points); + } +} + +void USDCurvesWriter::do_write(HierarchyContext &context) +{ + + Curves *curve = converted_curves ? converted_curves : + static_cast(context.object->data); + + bke::CurvesGeometry &geometry = bke::CurvesGeometry::wrap(curve->geometry); + if (geometry.points_num() == 0) { + WM_reportf(RPT_WARNING, "Curve geometry has zero points. Will not export curve."); + return; + } + + const std::array curve_type_counts = geometry.curve_type_counts(); + const int number_of_curve_types = std::reduce( + curve_type_counts.begin(), curve_type_counts.end(), 0, [](int previousResult, int item) { + return item > 0 ? ++previousResult : previousResult; + }); + + if (number_of_curve_types > 1) { + WM_reportf(RPT_WARNING, "Cannot export mixed curve types."); + return; + } + + const VArray cyclic_values = geometry.cyclic(); + if (cyclic_values.size() == 0) { + WM_reportf(RPT_ERROR, "Cannot determine if curve is cyclic or not. Cannot export curve."); + return; + } + + const bool cyclic = cyclic_values[0]; + bool all_same_cyclic_type = true; + + for (int i = 0; i < cyclic_values.size(); i++) { + if (cyclic_values[i] != cyclic) { + all_same_cyclic_type = false; + break; + } + } + + if (!all_same_cyclic_type) { + WM_reportf(RPT_WARNING, "Cannot export mixed cyclic and non-cyclic curves."); + return; + } + + pxr::UsdTimeCode timecode = get_export_time_code(); + pxr::UsdGeomCurves usd_curves; + + pxr::VtArray verts; + pxr::VtIntArray curve_point_counts; + pxr::VtArray widths; + + int8_t curve_type = geometry.curve_types()[0]; + switch (curve_type) { + case CURVE_TYPE_CATMULL_ROM: + usd_curves = DefineUsdGeomBasisCurves( + pxr::VtValue(pxr::UsdGeomTokens->catmullRom), cyclic, true); + + populate_curve_verts(geometry, verts, curve_point_counts, widths); + break; + case CURVE_TYPE_BEZIER: + usd_curves = DefineUsdGeomBasisCurves( + pxr::VtValue(pxr::UsdGeomTokens->bezier), cyclic, true); + + populate_curve_verts_for_bezier(geometry, verts, curve_point_counts, widths); + break; + case CURVE_TYPE_POLY: + usd_curves = DefineUsdGeomBasisCurves(pxr::VtValue(), cyclic, false); + populate_curve_verts(geometry, verts, curve_point_counts, widths); + break; + case CURVE_TYPE_NURBS: + // curves = pxr::UsdGeomNurbsCurves::Define(usd_export_context_.stage, + // usd_export_context_.usd_path); + return; + default: + BLI_assert_unreachable(); + } + + pxr::UsdAttribute attr_points = usd_curves.CreatePointsAttr(pxr::VtValue(), true); + usd_value_writer_.SetAttribute(attr_points, pxr::VtValue(verts), timecode); + + pxr::UsdAttribute attr_vertex_counts = usd_curves.CreateCurveVertexCountsAttr(pxr::VtValue(), + true); + usd_value_writer_.SetAttribute(attr_vertex_counts, pxr::VtValue(curve_point_counts), timecode); + + pxr::UsdAttribute attr_widths = usd_curves.CreateWidthsAttr(pxr::VtValue(), true); + usd_value_writer_.SetAttribute(attr_widths, pxr::VtValue(widths), timecode); +} + +bool USDCurvesWriter::check_is_animated(const HierarchyContext &) const +{ + return true; +} + +void USDCurvesWriter::assign_materials(const HierarchyContext &context, + pxr::UsdGeomCurves usd_curve) +{ + if (context.object->totcol == 0) { + return; + } + + bool curve_material_bound = false; + for (short mat_num = 0; mat_num < context.object->totcol; mat_num++) { + Material *material = BKE_object_material_get(context.object, mat_num + 1); + if (material == nullptr) { + continue; + } + + pxr::UsdShadeMaterialBindingAPI api = pxr::UsdShadeMaterialBindingAPI(usd_curve.GetPrim()); + pxr::UsdShadeMaterial usd_material = ensure_usd_material(context, material); + api.Bind(usd_material); + + /* USD seems to support neither per-material nor per-face-group double-sidedness, so we just + * use the flag from the first non-empty material slot. */ + usd_curve.CreateDoubleSidedAttr( + pxr::VtValue((material->blend_flag & MA_BL_CULL_BACKFACE) == 0)); + + curve_material_bound = true; + break; + } + + if (!curve_material_bound) { + /* Blender defaults to double-sided, but USD to single-sided. */ + usd_curve.CreateDoubleSidedAttr(pxr::VtValue(true)); + } + + if (!curve_material_bound) { + /* Either all material slots were empty or there is only one material in use. As geometry + * subsets are only written when actually used to assign a material, and the mesh already has + * the material assigned, there is no need to continue. */ + return; + } +} + +} // namespace blender::io::usd diff --git a/source/blender/io/usd/intern/usd_writer_curves.h b/source/blender/io/usd/intern/usd_writer_curves.h new file mode 100644 index 00000000000..73c4ab029db --- /dev/null +++ b/source/blender/io/usd/intern/usd_writer_curves.h @@ -0,0 +1,47 @@ +/* + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * The Original Code is Copyright (C) 2019 Blender Foundation. + * All rights reserved. + */ +#ifndef __USD_WRITER_CURVES_H__ +#define __USD_WRITER_CURVES_H__ + +#include "usd_writer_abstract.h" +#include "DNA_curves_types.h" + +#include + +namespace blender::io::usd { + +/* Writer for writing Curve data as USD curves. */ +class USDCurvesWriter : public USDAbstractWriter { + public: + USDCurvesWriter(const USDExporterContext &ctx); + USDCurvesWriter(const USDExporterContext &ctx, Curves *converted_legacy_curves); + + protected: + virtual void do_write(HierarchyContext &context) override; + virtual bool check_is_animated(const HierarchyContext &context) const override; + void assign_materials(const HierarchyContext &context, pxr::UsdGeomCurves usd_curve); + + private: + Curves *converted_curves; + pxr::UsdGeomCurves DefineUsdGeomBasisCurves(pxr::VtValue curve_basis, bool cyclic, bool cubic); +}; + +} // namespace blender::io::usd + +#endif /* __USD_WRITER_CURVES_H__ */ -- 2.30.2 From 63ce89abbce22777e8117bc24e6c1dcc9c2f0e48 Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Thu, 2 Mar 2023 13:50:01 +0000 Subject: [PATCH 02/19] Updated with feedback from the patch review comments. Added NURBS export functionality. Added tests to cover curves export. --- .../io/usd/intern/usd_hierarchy_iterator.cc | 4 +- .../io/usd/intern/usd_writer_curves.cc | 374 ++++++++++-------- .../blender/io/usd/intern/usd_writer_curves.h | 28 +- 3 files changed, 220 insertions(+), 186 deletions(-) diff --git a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc index 60379a33f0e..b24c14e4558 100644 --- a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc +++ b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc @@ -111,14 +111,14 @@ AbstractHierarchyWriter *USDHierarchyIterator::create_data_writer(const Hierarch auto legacy_curve = static_cast(context->object->data); auto curves = bke::curve_legacy_to_curves(*legacy_curve); data_writer = new USDCurvesWriter(usd_export_context, curves); - } break; + break; + } case OB_CURVES: data_writer = new USDCurvesWriter(usd_export_context); break; case OB_VOLUME: data_writer = new USDVolumeWriter(usd_export_context); break; - case OB_EMPTY: case OB_SURF: case OB_FONT: diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index 9394e63c583..fb88e70b8c1 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -1,23 +1,7 @@ -/* - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation; either version 2 - * of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - * - * The Original Code is Copyright (C) 2019 Blender Foundation. - * All rights reserved. - */ -#include "usd_writer_curves.h" -#include "usd_hierarchy_iterator.h" +/* SPDX-License-Identifier: GPL-2.0-or-later + * Copyright 2022 Blender Foundation. All rights reserved. */ + +#include #include #include @@ -26,19 +10,16 @@ #include #include +#include "usd_hierarchy_iterator.h" +#include "usd_writer_curves.h" + #include "BKE_curves.hh" - -#include - -extern "C" { - #include "BKE_material.h" #include "BLI_math_geom.h" #include "WM_api.h" #include "WM_types.h" -} namespace blender::io::usd { @@ -52,94 +33,6 @@ USDCurvesWriter::USDCurvesWriter(const USDExporterContext &ctx, Curves *converte { } -/* -void USDCurvesWriter::do_write(HierarchyContext &context) -{ - Curves *curve = static_cast(context.object->data); - bke::CurvesGeometry &geometry = bke::CurvesGeometry::wrap(curve->geometry); - - pxr::UsdTimeCode timecode = get_export_time_code(); - - const int num_curves = geometry.curve_num; - const Span positions = geometry.positions(); - - const bke::AttributeAccessor curve_attributes = geometry.attributes(); - auto radii = curve_attributes.lookup_or_default("radius", ATTR_DOMAIN_POINT, 0.0f); - - for (int i_curve = 0; i_curve < num_curves; i_curve++) { - - pxr::UsdGeomCurves curve; - int8_t curve_type = geometry.curve_types()[i_curve]; - - switch (curve_type) { - case CURVE_TYPE_CATMULL_ROM: - curve = pxr::UsdGeomBasisCurves::Define(usd_export_context_.stage, - usd_export_context_.usd_path); - pxr::UsdGeomBasisCurves(curve).CreateBasisAttr( - pxr::VtValue(pxr::UsdGeomTokens->catmullRom)); - - pxr::UsdGeomBasisCurves(curve).CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->linear)); - break; - case CURVE_TYPE_POLY: - curve = pxr::UsdGeomBasisCurves::Define(usd_export_context_.stage, - usd_export_context_.usd_path); - pxr::UsdGeomBasisCurves(curve).CreateBasisAttr(pxr::VtValue(pxr::UsdGeomTokens->bezier)); - - - pxr::UsdGeomBasisCurves(curve).CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->linear)); - return; - case CURVE_TYPE_BEZIER: - curve = pxr::UsdGeomBasisCurves::Define(usd_export_context_.stage, - usd_export_context_.usd_path); - pxr::UsdGeomBasisCurves(curve).CreateBasisAttr(pxr::VtValue(pxr::UsdGeomTokens->bezier)); - - pxr::UsdGeomBasisCurves(curve).CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->linear)); - return; - case CURVE_TYPE_NURBS: - curve = pxr::UsdGeomNurbsCurves::Define(usd_export_context_.stage, - usd_export_context_.usd_path); - break; - default: - BLI_assert_unreachable(); - } - - bool cyclic = geometry.cyclic()[i_curve]; - if (cyclic) { - pxr::UsdGeomBasisCurves(curve).CreateWrapAttr(pxr::VtValue(pxr::UsdGeomTokens->periodic)); - } - else { - pxr::UsdGeomBasisCurves(curve).CreateWrapAttr(pxr::VtValue(pxr::UsdGeomTokens->nonperiodic)); - } - - pxr::VtArray verts; - pxr::VtIntArray curve_point_counts; - pxr::VtArray widths; - - long tot_points = 0; - for (const auto i_point : geometry.points_for_curve(i_curve)) { - verts.push_back( - pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); - widths.push_back(radii[i_point] * 2.0f); - tot_points++; - } - - curve_point_counts.push_back(tot_points); - - pxr::UsdAttribute attr_points = curve.CreatePointsAttr(pxr::VtValue(), true); - usd_value_writer_.SetAttribute(attr_points, pxr::VtValue(verts), timecode); - - pxr::UsdAttribute attr_vertex_counts = curve.CreateCurveVertexCountsAttr(pxr::VtValue(), - true); - usd_value_writer_.SetAttribute(attr_vertex_counts, pxr::VtValue(curve_point_counts), timecode); - - pxr::UsdAttribute attr_widths = curve.CreateWidthsAttr(pxr::VtValue(), true); - usd_value_writer_.SetAttribute(attr_widths, pxr::VtValue(widths), timecode); - } - - -} -*/ - pxr::UsdGeomCurves USDCurvesWriter::DefineUsdGeomBasisCurves(pxr::VtValue curve_basis, bool cyclic, bool cubic) @@ -147,74 +40,147 @@ pxr::UsdGeomCurves USDCurvesWriter::DefineUsdGeomBasisCurves(pxr::VtValue curve_ pxr::UsdGeomCurves curves = pxr::UsdGeomBasisCurves::Define(usd_export_context_.stage, usd_export_context_.usd_path); + pxr::UsdGeomBasisCurves basis_curves = pxr::UsdGeomBasisCurves(curves); // Not required to set the basis attribute for linear curves // https://graphics.pixar.com/usd/dev/api/class_usd_geom_basis_curves.html#details if (cubic) { - pxr::UsdGeomBasisCurves(curves).CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->cubic)); - pxr::UsdGeomBasisCurves(curves).CreateBasisAttr(curve_basis); + basis_curves.CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->cubic)); + basis_curves.CreateBasisAttr(curve_basis); } else { - pxr::UsdGeomBasisCurves(curves).CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->linear)); + basis_curves.CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->linear)); } if (cyclic) { - pxr::UsdGeomBasisCurves(curves).CreateWrapAttr(pxr::VtValue(pxr::UsdGeomTokens->periodic)); + basis_curves.CreateWrapAttr(pxr::VtValue(pxr::UsdGeomTokens->periodic)); } else { - pxr::UsdGeomBasisCurves(curves).CreateWrapAttr(pxr::VtValue(pxr::UsdGeomTokens->nonperiodic)); + basis_curves.CreateWrapAttr(pxr::VtValue(pxr::UsdGeomTokens->nonperiodic)); } return curves; } -void populate_curve_verts(const bke::CurvesGeometry &geometry, +void populate_curve_widths(const bke::CurvesGeometry &geometry, pxr::VtArray &widths) +{ + const bke::AttributeAccessor curve_attributes = geometry.attributes(); + const VArray radii = curve_attributes.lookup("radius", ATTR_DOMAIN_POINT); + + widths.resize(radii.size()); + + if (widths.size() == 0) { + return; + } + + for (const int i : radii.index_range()) { + widths[i] = radii[i] * 2.0f; + } +} + +void set_curve_width_interpolation(const pxr::VtArray &widths, + const pxr::VtArray &segments, + const pxr::VtIntArray &control_point_counts, + const bool cyclic, + pxr::TfToken &interpolation) +{ + size_t expectedVaryingSize; + if (cyclic) { + expectedVaryingSize = std::accumulate(segments.begin(), segments.end(), 0); + } + else { + expectedVaryingSize = std::accumulate(segments.begin(), segments.end(), 0) + + control_point_counts.size(); + } + + size_t accumulatedControlPointCount = std::accumulate( + control_point_counts.begin(), control_point_counts.end(), 0); + + if (widths.size() == 1) + interpolation = pxr::UsdGeomTokens->constant; + else if (widths.size() == accumulatedControlPointCount) + interpolation = pxr::UsdGeomTokens->vertex; + else if (widths.size() == control_point_counts.size()) + interpolation = pxr::UsdGeomTokens->uniform; + else if (widths.size() == expectedVaryingSize) + interpolation = pxr::UsdGeomTokens->varying; + else { + /*TF_WARN( + "MFnNurbsCurve has unsupported width size " + "for standard interpolation metadata: %s", + GetDagPath().fullPathName().asChar());*/ + } +} + +void populate_curve_props(const bke::CurvesGeometry &geometry, pxr::VtArray &verts, - pxr::VtIntArray &curve_point_counts, - pxr::VtArray &widths) + pxr::VtIntArray &control_point_counts, + pxr::VtArray &widths, + pxr::TfToken &interpolation, + const bool cyclic, + const bool cubic) { const int num_curves = geometry.curve_num; const Span positions = geometry.positions(); - const bke::AttributeAccessor curve_attributes = geometry.attributes(); - const VArray radii = curve_attributes.lookup_or_default( - "radius", ATTR_DOMAIN_POINT, 0.0f); + pxr::VtArray segments; + segments.resize(num_curves); for (int i_curve = 0; i_curve < num_curves; i_curve++) { auto curve_points = geometry.points_for_curve(i_curve); long tot_points = 0; for (const auto i_point : curve_points) { - widths.push_back(radii[i_point] * 2.0f); - verts.push_back( pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); tot_points++; } - curve_point_counts.push_back(tot_points); + control_point_counts[i_curve] = tot_points; + + if (cubic) { + if (cyclic) { + segments[i_curve] = tot_points; + } + else { + segments[i_curve] = (tot_points - 4) + 1; + } + } + else { + if (cyclic) { + segments[i_curve] = tot_points; + } + else { + segments[i_curve] = tot_points - 1; + } + } } + + populate_curve_widths(geometry, widths); + set_curve_width_interpolation(widths, segments, control_point_counts, cyclic, interpolation); } -void populate_curve_verts_for_bezier(const bke::CurvesGeometry &geometry, +void populate_curve_props_for_bezier(const bke::CurvesGeometry &geometry, pxr::VtArray &verts, - pxr::VtIntArray &curve_point_counts, - pxr::VtArray &widths) + pxr::VtIntArray &control_point_counts, + pxr::VtArray &widths, + pxr::TfToken &interpolation, + const bool cyclic) { + const int bezier_vstep = 3; const int num_curves = geometry.curve_num; - const Span positions = geometry.positions(); - const bke::AttributeAccessor curve_attributes = geometry.attributes(); - const VArray radii = curve_attributes.lookup_or_default( - "radius", ATTR_DOMAIN_POINT, 0.0f); + const Span positions = geometry.positions(); const Span handles_l = geometry.handle_positions_left(); const Span handles_r = geometry.handle_positions_right(); + pxr::VtArray segments; + segments.resize(num_curves); + for (int i_curve = 0; i_curve < num_curves; i_curve++) { auto curve_points = geometry.points_for_curve(i_curve); - long tot_points = 0; - for (const auto i_point : curve_points) { - widths.push_back(radii[i_point] * 2.0f); + long tot_points = 0; + for (const long i_point : curve_points) { verts.push_back( pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); tot_points++; @@ -226,10 +192,77 @@ void populate_curve_verts_for_bezier(const bke::CurvesGeometry &geometry, handles_l[i_point + 1][0], handles_l[i_point + 1][1], handles_l[i_point + 1][2])); tot_points += 2; } + else if (i_point == curve_points.size() - 1 && cyclic) { + verts.push_back( + pxr::GfVec3f(handles_r[i_point][0], handles_r[i_point][1], handles_r[i_point][2])); + + auto start_point = curve_points[0]; + verts.push_back(pxr::GfVec3f( + handles_l[start_point][0], handles_l[start_point][1], handles_l[start_point][2])); + + tot_points += 2; + } } - curve_point_counts.push_back(tot_points); + control_point_counts[i_curve] = tot_points; + + if (cyclic) { + segments[i_curve] = tot_points / bezier_vstep; + } + else { + segments[i_curve] = ((tot_points - 4) / bezier_vstep) + 1; + } } + + populate_curve_widths(geometry, widths); + set_curve_width_interpolation(widths, segments, control_point_counts, cyclic, interpolation); +} + +void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, + pxr::VtArray &verts, + pxr::VtIntArray &control_point_counts, + pxr::VtArray &widths, + pxr::VtArray &knots, + pxr::VtArray &orders, + pxr::TfToken &interpolation, + const bool cyclic) +{ + const int num_curves = geometry.curve_num; + orders.resize(num_curves); + + const Span positions = geometry.positions(); + + VArray geom_orders = geometry.nurbs_orders(); + VArray knots_modes = geometry.nurbs_knots_modes(); + + for (int i_curve = 0; i_curve < num_curves; i_curve++) { + auto curve_points = geometry.points_for_curve(i_curve); + long tot_points = 0; + for (const auto i_point : curve_points) { + verts.push_back( + pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); + tot_points++; + } + + control_point_counts[i_curve] = tot_points; + + const int8_t order = geom_orders[i_curve]; + orders[i_curve] = (int)geom_orders[i_curve]; + + const KnotsMode mode = KnotsMode(knots_modes[i_curve]); + + const int knots_num = bke::curves::nurbs::knots_num(curve_points.size(), order, cyclic); + Array temp_knots(knots_num); + bke::curves::nurbs::calculate_knots(curve_points.size(), mode, order, cyclic, temp_knots); + + knots.resize(knots_num); + for (int i_knot = 0; i_knot < knots_num; i_knot++) { + knots[i_knot] = (double)temp_knots[i_knot]; + } + } + + populate_curve_widths(geometry, widths); + interpolation = pxr::UsdGeomTokens->vertex; } void USDCurvesWriter::do_write(HierarchyContext &context) @@ -238,9 +271,8 @@ void USDCurvesWriter::do_write(HierarchyContext &context) Curves *curve = converted_curves ? converted_curves : static_cast(context.object->data); - bke::CurvesGeometry &geometry = bke::CurvesGeometry::wrap(curve->geometry); + const bke::CurvesGeometry &geometry = bke::CurvesGeometry::wrap(curve->geometry); if (geometry.points_num() == 0) { - WM_reportf(RPT_WARNING, "Curve geometry has zero points. Will not export curve."); return; } @@ -256,15 +288,10 @@ void USDCurvesWriter::do_write(HierarchyContext &context) } const VArray cyclic_values = geometry.cyclic(); - if (cyclic_values.size() == 0) { - WM_reportf(RPT_ERROR, "Cannot determine if curve is cyclic or not. Cannot export curve."); - return; - } - const bool cyclic = cyclic_values[0]; bool all_same_cyclic_type = true; - for (int i = 0; i < cyclic_values.size(); i++) { + for (const int i : cyclic_values.index_range()) { if (cyclic_values[i] != cyclic) { all_same_cyclic_type = false; break; @@ -280,31 +307,52 @@ void USDCurvesWriter::do_write(HierarchyContext &context) pxr::UsdGeomCurves usd_curves; pxr::VtArray verts; - pxr::VtIntArray curve_point_counts; + pxr::VtIntArray control_point_counts; + control_point_counts.resize(geometry.curves_num()); pxr::VtArray widths; + pxr::TfToken interpolation; - int8_t curve_type = geometry.curve_types()[0]; + const int8_t curve_type = geometry.curve_types()[0]; switch (curve_type) { + case CURVE_TYPE_POLY: + usd_curves = DefineUsdGeomBasisCurves(pxr::VtValue(), cyclic, false); + + populate_curve_props( + geometry, verts, control_point_counts, widths, interpolation, cyclic, false); + break; case CURVE_TYPE_CATMULL_ROM: usd_curves = DefineUsdGeomBasisCurves( pxr::VtValue(pxr::UsdGeomTokens->catmullRom), cyclic, true); - populate_curve_verts(geometry, verts, curve_point_counts, widths); + populate_curve_props( + geometry, verts, control_point_counts, widths, interpolation, cyclic, true); break; case CURVE_TYPE_BEZIER: usd_curves = DefineUsdGeomBasisCurves( pxr::VtValue(pxr::UsdGeomTokens->bezier), cyclic, true); - populate_curve_verts_for_bezier(geometry, verts, curve_point_counts, widths); + populate_curve_props_for_bezier( + geometry, verts, control_point_counts, widths, interpolation, cyclic); break; - case CURVE_TYPE_POLY: - usd_curves = DefineUsdGeomBasisCurves(pxr::VtValue(), cyclic, false); - populate_curve_verts(geometry, verts, curve_point_counts, widths); + case CURVE_TYPE_NURBS: { + pxr::VtArray knots; + pxr::VtArray orders; + orders.resize(geometry.curves_num()); + + usd_curves = pxr::UsdGeomNurbsCurves::Define(usd_export_context_.stage, + usd_export_context_.usd_path); + + populate_curve_props_for_nurbs( + geometry, verts, control_point_counts, widths, knots, orders, interpolation, cyclic); + + pxr::UsdAttribute attr_knots = + pxr::UsdGeomNurbsCurves(usd_curves).CreateKnotsAttr(pxr::VtValue(), true); + usd_value_writer_.SetAttribute(attr_knots, pxr::VtValue(knots), timecode); + pxr::UsdAttribute attr_order = + pxr::UsdGeomNurbsCurves(usd_curves).CreateOrderAttr(pxr::VtValue(), true); + usd_value_writer_.SetAttribute(attr_order, pxr::VtValue(orders), timecode); break; - case CURVE_TYPE_NURBS: - // curves = pxr::UsdGeomNurbsCurves::Define(usd_export_context_.stage, - // usd_export_context_.usd_path); - return; + } default: BLI_assert_unreachable(); } @@ -314,10 +362,14 @@ void USDCurvesWriter::do_write(HierarchyContext &context) pxr::UsdAttribute attr_vertex_counts = usd_curves.CreateCurveVertexCountsAttr(pxr::VtValue(), true); - usd_value_writer_.SetAttribute(attr_vertex_counts, pxr::VtValue(curve_point_counts), timecode); + usd_value_writer_.SetAttribute(attr_vertex_counts, pxr::VtValue(control_point_counts), timecode); - pxr::UsdAttribute attr_widths = usd_curves.CreateWidthsAttr(pxr::VtValue(), true); - usd_value_writer_.SetAttribute(attr_widths, pxr::VtValue(widths), timecode); + if (widths.size() > 0) { + pxr::UsdAttribute attr_widths = usd_curves.CreateWidthsAttr(pxr::VtValue(), true); + usd_value_writer_.SetAttribute(attr_widths, pxr::VtValue(widths), timecode); + + usd_curves.SetWidthsInterpolation(interpolation); + } } bool USDCurvesWriter::check_is_animated(const HierarchyContext &) const diff --git a/source/blender/io/usd/intern/usd_writer_curves.h b/source/blender/io/usd/intern/usd_writer_curves.h index 73c4ab029db..1835227a2d5 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.h +++ b/source/blender/io/usd/intern/usd_writer_curves.h @@ -1,32 +1,15 @@ -/* - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation; either version 2 - * of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - * - * The Original Code is Copyright (C) 2019 Blender Foundation. - * All rights reserved. - */ -#ifndef __USD_WRITER_CURVES_H__ -#define __USD_WRITER_CURVES_H__ +/* SPDX-License-Identifier: GPL-2.0-or-later + * Copyright 2022 Blender Foundation. All rights reserved. */ +#pragma once -#include "usd_writer_abstract.h" #include "DNA_curves_types.h" +#include "usd_writer_abstract.h" #include namespace blender::io::usd { -/* Writer for writing Curve data as USD curves. */ +/* Writer for writing Curves data as USD curves. */ class USDCurvesWriter : public USDAbstractWriter { public: USDCurvesWriter(const USDExporterContext &ctx); @@ -44,4 +27,3 @@ class USDCurvesWriter : public USDAbstractWriter { } // namespace blender::io::usd -#endif /* __USD_WRITER_CURVES_H__ */ -- 2.30.2 From 79bf67a8dca79f1d76f1537b032c418bfee338ac Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Thu, 2 Mar 2023 13:50:42 +0000 Subject: [PATCH 03/19] Added logic for USD representation of periodic NURBS curves at export. Fleshed out more unit tests. --- .../io/usd/intern/usd_writer_curves.cc | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index fb88e70b8c1..98f60ee7083 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -104,10 +104,7 @@ void set_curve_width_interpolation(const pxr::VtArray &widths, else if (widths.size() == expectedVaryingSize) interpolation = pxr::UsdGeomTokens->varying; else { - /*TF_WARN( - "MFnNurbsCurve has unsupported width size " - "for standard interpolation metadata: %s", - GetDagPath().fullPathName().asChar());*/ + WM_reportf(RPT_WARNING, "Curve width size not supported for standard USD interpolation."); } } @@ -259,6 +256,18 @@ void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, for (int i_knot = 0; i_knot < knots_num; i_knot++) { knots[i_knot] = (double)temp_knots[i_knot]; } + + /* Set end knots according to the USD spec for periodic curves + https://graphics.pixar.com/usd/dev/api/class_usd_geom_nurbs_curves.html#details */ + if (cyclic) { + knots[0] = knots[1] - (knots[knots.size() - 2] - knots[knots.size() - 3]); + knots[knots.size() - 1] = knots[knots.size() - 2] + (knots[2] - knots[1]); + } + else { + // Set end knots according to the USD spec for non-periodic curves + knots[0] = knots[1]; + knots[knots.size() - 1] = knots[knots.size() - 2]; + } } populate_curve_widths(geometry, widths); @@ -370,6 +379,8 @@ void USDCurvesWriter::do_write(HierarchyContext &context) usd_curves.SetWidthsInterpolation(interpolation); } + + assign_materials(context, usd_curves); } bool USDCurvesWriter::check_is_animated(const HierarchyContext &) const @@ -408,13 +419,6 @@ void USDCurvesWriter::assign_materials(const HierarchyContext &context, /* Blender defaults to double-sided, but USD to single-sided. */ usd_curve.CreateDoubleSidedAttr(pxr::VtValue(true)); } - - if (!curve_material_bound) { - /* Either all material slots were empty or there is only one material in use. As geometry - * subsets are only written when actually used to assign a material, and the mesh already has - * the material assigned, there is no need to continue. */ - return; - } } } // namespace blender::io::usd -- 2.30.2 From 1c7df5997f7913929c17ef735194300492bc97f4 Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Wed, 4 Jan 2023 15:23:46 +0000 Subject: [PATCH 04/19] Remove unnecessary width interpolation code. Updated changes to be consistent with style. --- .../io/usd/intern/usd_hierarchy_iterator.cc | 4 +- .../io/usd/intern/usd_writer_curves.cc | 68 +++++++++---------- 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc index b24c14e4558..2ca149de253 100644 --- a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc +++ b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc @@ -108,8 +108,8 @@ AbstractHierarchyWriter *USDHierarchyIterator::create_data_writer(const Hierarch data_writer = new USDMetaballWriter(usd_export_context); break; case OB_CURVES_LEGACY: { - auto legacy_curve = static_cast(context->object->data); - auto curves = bke::curve_legacy_to_curves(*legacy_curve); + const Curve *legacy_curve = static_cast(context->object->data); + Curves *curves = bke::curve_legacy_to_curves(*legacy_curve); data_writer = new USDCurvesWriter(usd_export_context, curves); break; } diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index 98f60ee7083..7f5cfc92a24 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -41,8 +41,8 @@ pxr::UsdGeomCurves USDCurvesWriter::DefineUsdGeomBasisCurves(pxr::VtValue curve_ usd_export_context_.usd_path); pxr::UsdGeomBasisCurves basis_curves = pxr::UsdGeomBasisCurves(curves); - // Not required to set the basis attribute for linear curves - // https://graphics.pixar.com/usd/dev/api/class_usd_geom_basis_curves.html#details + /* Not required to set the basis attribute for linear curves + * https://graphics.pixar.com/usd/dev/api/class_usd_geom_basis_curves.html#details */ if (cubic) { basis_curves.CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->cubic)); basis_curves.CreateBasisAttr(curve_basis); @@ -68,10 +68,6 @@ void populate_curve_widths(const bke::CurvesGeometry &geometry, pxr::VtArray &widths, const bool cyclic, pxr::TfToken &interpolation) { + if (widths.size() == 0) { + return; + } + size_t expectedVaryingSize; if (cyclic) { expectedVaryingSize = std::accumulate(segments.begin(), segments.end(), 0); @@ -95,16 +95,15 @@ void set_curve_width_interpolation(const pxr::VtArray &widths, size_t accumulatedControlPointCount = std::accumulate( control_point_counts.begin(), control_point_counts.end(), 0); - if (widths.size() == 1) - interpolation = pxr::UsdGeomTokens->constant; - else if (widths.size() == accumulatedControlPointCount) + /* For Blender curves, radii are always stored per point. For linear curves, this should match + * with USD's vertex interpolation. For cubic curves, this should match with USD's varying + * interpolation. */ + if (widths.size() == accumulatedControlPointCount) interpolation = pxr::UsdGeomTokens->vertex; - else if (widths.size() == control_point_counts.size()) - interpolation = pxr::UsdGeomTokens->uniform; - else if (widths.size() == expectedVaryingSize) + if (widths.size() == expectedVaryingSize) interpolation = pxr::UsdGeomTokens->varying; else { - WM_reportf(RPT_WARNING, "Curve width size not supported for standard USD interpolation."); + WM_reportf(RPT_WARNING, "Curve width size not supported for USD interpolation."); } } @@ -122,10 +121,10 @@ void populate_curve_props(const bke::CurvesGeometry &geometry, pxr::VtArray segments; segments.resize(num_curves); - for (int i_curve = 0; i_curve < num_curves; i_curve++) { - auto curve_points = geometry.points_for_curve(i_curve); + for (const int i_curve : geometry.curves_range()) { + const IndexRange curve_points = geometry.points_for_curve(i_curve); long tot_points = 0; - for (const auto i_point : curve_points) { + for (const long i_point : curve_points) { verts.push_back( pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); tot_points++; @@ -133,17 +132,12 @@ void populate_curve_props(const bke::CurvesGeometry &geometry, control_point_counts[i_curve] = tot_points; - if (cubic) { - if (cyclic) { - segments[i_curve] = tot_points; - } - else { - segments[i_curve] = (tot_points - 4) + 1; - } + if (cyclic) { + segments[i_curve] = tot_points; } else { - if (cyclic) { - segments[i_curve] = tot_points; + if (cubic) { + segments[i_curve] = (tot_points - 4) + 1; } else { segments[i_curve] = tot_points - 1; @@ -174,7 +168,7 @@ void populate_curve_props_for_bezier(const bke::CurvesGeometry &geometry, segments.resize(num_curves); for (int i_curve = 0; i_curve < num_curves; i_curve++) { - auto curve_points = geometry.points_for_curve(i_curve); + const IndexRange curve_points = geometry.points_for_curve(i_curve); long tot_points = 0; for (const long i_point : curve_points) { @@ -193,7 +187,9 @@ void populate_curve_props_for_bezier(const bke::CurvesGeometry &geometry, verts.push_back( pxr::GfVec3f(handles_r[i_point][0], handles_r[i_point][1], handles_r[i_point][2])); - auto start_point = curve_points[0]; + /* For USD representation of periodic bezier curve, one vertex must be repeated to close + * the curve. */ + const long start_point = curve_points[0]; verts.push_back(pxr::GfVec3f( handles_l[start_point][0], handles_l[start_point][1], handles_l[start_point][2])); @@ -232,10 +228,10 @@ void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, VArray geom_orders = geometry.nurbs_orders(); VArray knots_modes = geometry.nurbs_knots_modes(); - for (int i_curve = 0; i_curve < num_curves; i_curve++) { - auto curve_points = geometry.points_for_curve(i_curve); + for (const int i_curve : geometry.curves_range()) { + const IndexRange curve_points = geometry.points_for_curve(i_curve); long tot_points = 0; - for (const auto i_point : curve_points) { + for (const long i_point : curve_points) { verts.push_back( pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); tot_points++; @@ -244,7 +240,7 @@ void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, control_point_counts[i_curve] = tot_points; const int8_t order = geom_orders[i_curve]; - orders[i_curve] = (int)geom_orders[i_curve]; + orders[i_curve] = int(geom_orders[i_curve]); const KnotsMode mode = KnotsMode(knots_modes[i_curve]); @@ -254,17 +250,17 @@ void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, knots.resize(knots_num); for (int i_knot = 0; i_knot < knots_num; i_knot++) { - knots[i_knot] = (double)temp_knots[i_knot]; + knots[i_knot] = double(temp_knots[i_knot]); } /* Set end knots according to the USD spec for periodic curves - https://graphics.pixar.com/usd/dev/api/class_usd_geom_nurbs_curves.html#details */ + * https://graphics.pixar.com/usd/dev/api/class_usd_geom_nurbs_curves.html#details */ if (cyclic) { knots[0] = knots[1] - (knots[knots.size() - 2] - knots[knots.size() - 3]); knots[knots.size() - 1] = knots[knots.size() - 2] + (knots[2] - knots[1]); } else { - // Set end knots according to the USD spec for non-periodic curves + /* Set end knots according to the USD spec for non-periodic curves */ knots[0] = knots[1]; knots[knots.size() - 1] = knots[knots.size() - 2]; } @@ -287,8 +283,8 @@ void USDCurvesWriter::do_write(HierarchyContext &context) const std::array curve_type_counts = geometry.curve_type_counts(); const int number_of_curve_types = std::reduce( - curve_type_counts.begin(), curve_type_counts.end(), 0, [](int previousResult, int item) { - return item > 0 ? ++previousResult : previousResult; + curve_type_counts.begin(), curve_type_counts.end(), 0, [](int previous_result, int item) { + return item > 0 ? ++previous_result : previous_result; }); if (number_of_curve_types > 1) { -- 2.30.2 From 1e4bda4473b0e7e440f389e3084a6107545f4d53 Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Thu, 2 Mar 2023 13:51:13 +0000 Subject: [PATCH 05/19] Handle memory management of curves when converting from legacy curves to new curves type with `unique_ptr` and freeing the memory when `usd_writer_curves` is disposed of. Tidied up code style issues in the unit tests. --- .../io/usd/intern/usd_hierarchy_iterator.cc | 6 +- .../io/usd/intern/usd_writer_curves.cc | 111 ++++++++++-------- .../blender/io/usd/intern/usd_writer_curves.h | 8 +- 3 files changed, 68 insertions(+), 57 deletions(-) diff --git a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc index 2ca149de253..95b0aaa6196 100644 --- a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc +++ b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc @@ -13,6 +13,7 @@ #include "usd_writer_transform.h" #include "usd_writer_volume.h" +#include #include #include @@ -109,8 +110,9 @@ AbstractHierarchyWriter *USDHierarchyIterator::create_data_writer(const Hierarch break; case OB_CURVES_LEGACY: { const Curve *legacy_curve = static_cast(context->object->data); - Curves *curves = bke::curve_legacy_to_curves(*legacy_curve); - data_writer = new USDCurvesWriter(usd_export_context, curves); + std::unique_ptr curves = std::unique_ptr( + bke::curve_legacy_to_curves(*legacy_curve)); + data_writer = new USDCurvesWriter(usd_export_context, std::move(curves)); break; } case OB_CURVES: diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index 7f5cfc92a24..21c4b1b48ef 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -14,6 +14,7 @@ #include "usd_writer_curves.h" #include "BKE_curves.hh" +#include "BKE_lib_id.h" #include "BKE_material.h" #include "BLI_math_geom.h" @@ -24,15 +25,23 @@ namespace blender::io::usd { USDCurvesWriter::USDCurvesWriter(const USDExporterContext &ctx) - : USDAbstractWriter(ctx), converted_curves(nullptr) + : USDAbstractWriter(ctx), converted_curves_(nullptr) { } -USDCurvesWriter::USDCurvesWriter(const USDExporterContext &ctx, Curves *converted_legacy_curves) - : USDAbstractWriter(ctx), converted_curves(converted_legacy_curves) +USDCurvesWriter::USDCurvesWriter(const USDExporterContext &ctx, + std::unique_ptr converted_legacy_curves) + : USDAbstractWriter(ctx), converted_curves_(std::move(converted_legacy_curves)) { } +USDCurvesWriter::~USDCurvesWriter() +{ + if (converted_curves_) { + BKE_id_free(nullptr, converted_curves_.release()); + } +} + pxr::UsdGeomCurves USDCurvesWriter::DefineUsdGeomBasisCurves(pxr::VtValue curve_basis, bool cyclic, bool cubic) @@ -61,7 +70,7 @@ pxr::UsdGeomCurves USDCurvesWriter::DefineUsdGeomBasisCurves(pxr::VtValue curve_ return curves; } -void populate_curve_widths(const bke::CurvesGeometry &geometry, pxr::VtArray &widths) +static void populate_curve_widths(const bke::CurvesGeometry &geometry, pxr::VtArray &widths) { const bke::AttributeAccessor curve_attributes = geometry.attributes(); const VArray radii = curve_attributes.lookup("radius", ATTR_DOMAIN_POINT); @@ -73,47 +82,45 @@ void populate_curve_widths(const bke::CurvesGeometry &geometry, pxr::VtArray &widths, - const pxr::VtArray &segments, - const pxr::VtIntArray &control_point_counts, - const bool cyclic, - pxr::TfToken &interpolation) +static pxr::TfToken set_curve_width_interpolation(const pxr::VtArray &widths, + const pxr::VtArray &segments, + const pxr::VtIntArray &control_point_counts, + const bool cyclic) { - if (widths.size() == 0) { - return; + if (widths.empty()) { + return pxr::TfToken(); } - size_t expectedVaryingSize; - if (cyclic) { - expectedVaryingSize = std::accumulate(segments.begin(), segments.end(), 0); - } - else { - expectedVaryingSize = std::accumulate(segments.begin(), segments.end(), 0) + - control_point_counts.size(); - } - - size_t accumulatedControlPointCount = std::accumulate( + const size_t accumulatedControlPointCount = std::accumulate( control_point_counts.begin(), control_point_counts.end(), 0); /* For Blender curves, radii are always stored per point. For linear curves, this should match * with USD's vertex interpolation. For cubic curves, this should match with USD's varying * interpolation. */ - if (widths.size() == accumulatedControlPointCount) - interpolation = pxr::UsdGeomTokens->vertex; - if (widths.size() == expectedVaryingSize) - interpolation = pxr::UsdGeomTokens->varying; - else { - WM_reportf(RPT_WARNING, "Curve width size not supported for USD interpolation."); + if (widths.size() == accumulatedControlPointCount) { + return pxr::UsdGeomTokens->vertex; } + + size_t expectedVaryingSize = std::accumulate(segments.begin(), segments.end(), 0); + if (!cyclic) { + expectedVaryingSize += control_point_counts.size(); + } + + if (widths.size() == expectedVaryingSize) { + return pxr::UsdGeomTokens->varying; + } + + WM_reportf(RPT_WARNING, "Curve width size not supported for USD interpolation."); + return pxr::TfToken(); } -void populate_curve_props(const bke::CurvesGeometry &geometry, - pxr::VtArray &verts, - pxr::VtIntArray &control_point_counts, - pxr::VtArray &widths, - pxr::TfToken &interpolation, - const bool cyclic, - const bool cubic) +static void populate_curve_props(const bke::CurvesGeometry &geometry, + pxr::VtArray &verts, + pxr::VtIntArray &control_point_counts, + pxr::VtArray &widths, + pxr::TfToken &interpolation, + const bool cyclic, + const bool cubic) { const int num_curves = geometry.curve_num; const Span positions = geometry.positions(); @@ -146,15 +153,15 @@ void populate_curve_props(const bke::CurvesGeometry &geometry, } populate_curve_widths(geometry, widths); - set_curve_width_interpolation(widths, segments, control_point_counts, cyclic, interpolation); + interpolation = set_curve_width_interpolation(widths, segments, control_point_counts, cyclic); } -void populate_curve_props_for_bezier(const bke::CurvesGeometry &geometry, - pxr::VtArray &verts, - pxr::VtIntArray &control_point_counts, - pxr::VtArray &widths, - pxr::TfToken &interpolation, - const bool cyclic) +static void populate_curve_props_for_bezier(const bke::CurvesGeometry &geometry, + pxr::VtArray &verts, + pxr::VtIntArray &control_point_counts, + pxr::VtArray &widths, + pxr::TfToken &interpolation, + const bool cyclic) { const int bezier_vstep = 3; const int num_curves = geometry.curve_num; @@ -208,17 +215,17 @@ void populate_curve_props_for_bezier(const bke::CurvesGeometry &geometry, } populate_curve_widths(geometry, widths); - set_curve_width_interpolation(widths, segments, control_point_counts, cyclic, interpolation); + interpolation = set_curve_width_interpolation(widths, segments, control_point_counts, cyclic); } -void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, - pxr::VtArray &verts, - pxr::VtIntArray &control_point_counts, - pxr::VtArray &widths, - pxr::VtArray &knots, - pxr::VtArray &orders, - pxr::TfToken &interpolation, - const bool cyclic) +static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, + pxr::VtArray &verts, + pxr::VtIntArray &control_point_counts, + pxr::VtArray &widths, + pxr::VtArray &knots, + pxr::VtArray &orders, + pxr::TfToken &interpolation, + const bool cyclic) { const int num_curves = geometry.curve_num; orders.resize(num_curves); @@ -273,8 +280,8 @@ void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, void USDCurvesWriter::do_write(HierarchyContext &context) { - Curves *curve = converted_curves ? converted_curves : - static_cast(context.object->data); + const Curves *curve = converted_curves_ ? converted_curves_.get() : + static_cast(context.object->data); const bke::CurvesGeometry &geometry = bke::CurvesGeometry::wrap(curve->geometry); if (geometry.points_num() == 0) { diff --git a/source/blender/io/usd/intern/usd_writer_curves.h b/source/blender/io/usd/intern/usd_writer_curves.h index 1835227a2d5..71c9cee3118 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.h +++ b/source/blender/io/usd/intern/usd_writer_curves.h @@ -2,6 +2,8 @@ * Copyright 2022 Blender Foundation. All rights reserved. */ #pragma once +#include + #include "DNA_curves_types.h" #include "usd_writer_abstract.h" @@ -13,7 +15,8 @@ namespace blender::io::usd { class USDCurvesWriter : public USDAbstractWriter { public: USDCurvesWriter(const USDExporterContext &ctx); - USDCurvesWriter(const USDExporterContext &ctx, Curves *converted_legacy_curves); + USDCurvesWriter(const USDExporterContext &ctx, std::unique_ptr converted_legacy_curves); + ~USDCurvesWriter(); protected: virtual void do_write(HierarchyContext &context) override; @@ -21,9 +24,8 @@ class USDCurvesWriter : public USDAbstractWriter { void assign_materials(const HierarchyContext &context, pxr::UsdGeomCurves usd_curve); private: - Curves *converted_curves; + std::unique_ptr converted_curves_; pxr::UsdGeomCurves DefineUsdGeomBasisCurves(pxr::VtValue curve_basis, bool cyclic, bool cubic); }; } // namespace blender::io::usd - -- 2.30.2 From 0a415d5e644f3cfa7e90476d93310380afc739cd Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Wed, 25 Jan 2023 16:35:59 +0000 Subject: [PATCH 06/19] Refactor logic into more consistent abstraction levels. --- .../io/usd/intern/usd_writer_curves.cc | 244 +++++++++++------- .../blender/io/usd/intern/usd_writer_curves.h | 12 + 2 files changed, 162 insertions(+), 94 deletions(-) diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index 21c4b1b48ef..4de4b552cbd 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -82,7 +82,7 @@ static void populate_curve_widths(const bke::CurvesGeometry &geometry, pxr::VtAr } } -static pxr::TfToken set_curve_width_interpolation(const pxr::VtArray &widths, +static pxr::TfToken get_curve_width_interpolation(const pxr::VtArray &widths, const pxr::VtArray &segments, const pxr::VtIntArray &control_point_counts, const bool cyclic) @@ -110,10 +110,39 @@ static pxr::TfToken set_curve_width_interpolation(const pxr::VtArray &wid return pxr::UsdGeomTokens->varying; } - WM_reportf(RPT_WARNING, "Curve width size not supported for USD interpolation."); + WM_report(RPT_WARNING, "Curve width size not supported for USD interpolation."); return pxr::TfToken(); } +static void populate_curve_verts(const int i_curve, + const bool cyclic, + const bool cubic, + const bke::CurvesGeometry &geometry, + const Span positions, + pxr::VtArray &verts, + pxr::VtIntArray &control_point_counts, + pxr::VtArray &segments) +{ + const IndexRange curve_points = geometry.points_for_curve(i_curve); + for (const int i_point : curve_points) { + verts.push_back( + pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); + } + + const int tot_points = curve_points.size(); + control_point_counts[i_curve] = tot_points; + + if (cyclic) { + segments[i_curve] = tot_points; + } + else if (cubic) { + segments[i_curve] = (tot_points - 4) + 1; + } + else { + segments[i_curve] = tot_points - 1; + } +} + static void populate_curve_props(const bke::CurvesGeometry &geometry, pxr::VtArray &verts, pxr::VtIntArray &control_point_counts, @@ -125,35 +154,73 @@ static void populate_curve_props(const bke::CurvesGeometry &geometry, const int num_curves = geometry.curve_num; const Span positions = geometry.positions(); - pxr::VtArray segments; - segments.resize(num_curves); + pxr::VtArray segments(num_curves); for (const int i_curve : geometry.curves_range()) { - const IndexRange curve_points = geometry.points_for_curve(i_curve); - long tot_points = 0; - for (const long i_point : curve_points) { - verts.push_back( - pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); - tot_points++; - } - - control_point_counts[i_curve] = tot_points; - - if (cyclic) { - segments[i_curve] = tot_points; - } - else { - if (cubic) { - segments[i_curve] = (tot_points - 4) + 1; - } - else { - segments[i_curve] = tot_points - 1; - } - } + populate_curve_verts( + i_curve, cyclic, cubic, geometry, positions, verts, control_point_counts, segments); } populate_curve_widths(geometry, widths); - interpolation = set_curve_width_interpolation(widths, segments, control_point_counts, cyclic); + interpolation = get_curve_width_interpolation(widths, segments, control_point_counts, cyclic); +} + +static void populate_curve_verts_for_bezier(const int i_curve, + const bool cyclic, + const bke::CurvesGeometry &geometry, + const Span positions, + const Span handles_l, + const Span handles_r, + pxr::VtArray &verts, + pxr::VtIntArray &control_point_counts, + pxr::VtArray &segments) +{ + const int bezier_vstep = 3; + const IndexRange curve_points = geometry.points_for_curve(i_curve); + + const int start_verts_count = verts.size(); + + const int start_point_index = curve_points[0]; + const int last_point_index = curve_points[curve_points.size() - 1]; + for (int i_point = start_point_index; i_point < last_point_index; i_point++) { + + /* The order verts in the USD bezier curve representation is [control point 0, right handle 0, + * left handle 1, control point 1, right handle 1, left handle 2, control point 2, ...]. The + * last vert in the array doesn't need a right handle because the curve stops at that point. */ + verts.push_back( + pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); + + const blender::float3 right_handle = handles_r[i_point]; + verts.push_back(pxr::GfVec3f(right_handle[0], right_handle[1], right_handle[2])); + + const blender::float3 left_handle = handles_l[i_point + 1]; + verts.push_back(pxr::GfVec3f(left_handle[0], left_handle[1], left_handle[2])); + } + + verts.push_back(pxr::GfVec3f(positions[last_point_index][0], + positions[last_point_index][1], + positions[last_point_index][2])); + + /* For USD representation of periodic bezier curve, one of the curve's points must be repeated + * to close the curve. The repeated point is the first point. Since the curve is closed, we now + * need to include the right handle of the last point and the left handle of the first point. */ + if (cyclic) { + const blender::float3 right_handle = handles_r[last_point_index]; + verts.push_back(pxr::GfVec3f(right_handle[0], right_handle[1], right_handle[2])); + + const blender::float3 left_handle = handles_l[start_point_index]; + verts.push_back(pxr::GfVec3f(left_handle[0], left_handle[1], left_handle[2])); + } + + const int tot_points = verts.size() - start_verts_count; + control_point_counts[i_curve] = tot_points; + + if (cyclic) { + segments[i_curve] = tot_points / bezier_vstep; + } + else { + segments[i_curve] = ((tot_points - 4) / bezier_vstep) + 1; + } } static void populate_curve_props_for_bezier(const bke::CurvesGeometry &geometry, @@ -163,7 +230,7 @@ static void populate_curve_props_for_bezier(const bke::CurvesGeometry &geometry, pxr::TfToken &interpolation, const bool cyclic) { - const int bezier_vstep = 3; + const int num_curves = geometry.curve_num; const Span positions = geometry.positions(); @@ -171,51 +238,22 @@ static void populate_curve_props_for_bezier(const bke::CurvesGeometry &geometry, const Span handles_l = geometry.handle_positions_left(); const Span handles_r = geometry.handle_positions_right(); - pxr::VtArray segments; - segments.resize(num_curves); + pxr::VtArray segments(num_curves); for (int i_curve = 0; i_curve < num_curves; i_curve++) { - const IndexRange curve_points = geometry.points_for_curve(i_curve); - - long tot_points = 0; - for (const long i_point : curve_points) { - verts.push_back( - pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); - tot_points++; - - if (i_point < curve_points.size() - 1) { - verts.push_back( - pxr::GfVec3f(handles_r[i_point][0], handles_r[i_point][1], handles_r[i_point][2])); - verts.push_back(pxr::GfVec3f( - handles_l[i_point + 1][0], handles_l[i_point + 1][1], handles_l[i_point + 1][2])); - tot_points += 2; - } - else if (i_point == curve_points.size() - 1 && cyclic) { - verts.push_back( - pxr::GfVec3f(handles_r[i_point][0], handles_r[i_point][1], handles_r[i_point][2])); - - /* For USD representation of periodic bezier curve, one vertex must be repeated to close - * the curve. */ - const long start_point = curve_points[0]; - verts.push_back(pxr::GfVec3f( - handles_l[start_point][0], handles_l[start_point][1], handles_l[start_point][2])); - - tot_points += 2; - } - } - - control_point_counts[i_curve] = tot_points; - - if (cyclic) { - segments[i_curve] = tot_points / bezier_vstep; - } - else { - segments[i_curve] = ((tot_points - 4) / bezier_vstep) + 1; - } + populate_curve_verts_for_bezier(i_curve, + cyclic, + geometry, + positions, + handles_l, + handles_r, + verts, + control_point_counts, + segments); } populate_curve_widths(geometry, widths); - interpolation = set_curve_width_interpolation(widths, segments, control_point_counts, cyclic); + interpolation = get_curve_width_interpolation(widths, segments, control_point_counts, cyclic); } static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, @@ -237,13 +275,12 @@ static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, for (const int i_curve : geometry.curves_range()) { const IndexRange curve_points = geometry.points_for_curve(i_curve); - long tot_points = 0; - for (const long i_point : curve_points) { + for (const int i_point : curve_points) { verts.push_back( pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); - tot_points++; } + const int tot_points = curve_points.size(); control_point_counts[i_curve] = tot_points; const int8_t order = geom_orders[i_curve]; @@ -251,9 +288,9 @@ static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, const KnotsMode mode = KnotsMode(knots_modes[i_curve]); - const int knots_num = bke::curves::nurbs::knots_num(curve_points.size(), order, cyclic); + const int knots_num = bke::curves::nurbs::knots_num(tot_points, order, cyclic); Array temp_knots(knots_num); - bke::curves::nurbs::calculate_knots(curve_points.size(), mode, order, cyclic, temp_knots); + bke::curves::nurbs::calculate_knots(tot_points, mode, order, cyclic, temp_knots); knots.resize(knots_num); for (int i_knot = 0; i_knot < knots_num; i_knot++) { @@ -277,6 +314,41 @@ static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, interpolation = pxr::UsdGeomTokens->vertex; } +void USDCurvesWriter::set_writer_attributes_for_nurbs(const pxr::UsdGeomCurves usd_curves, + const pxr::VtArray knots, + const pxr::VtArray orders, + const pxr::UsdTimeCode timecode) +{ + pxr::UsdAttribute attr_knots = + pxr::UsdGeomNurbsCurves(usd_curves).CreateKnotsAttr(pxr::VtValue(), true); + usd_value_writer_.SetAttribute(attr_knots, pxr::VtValue(knots), timecode); + pxr::UsdAttribute attr_order = + pxr::UsdGeomNurbsCurves(usd_curves).CreateOrderAttr(pxr::VtValue(), true); + usd_value_writer_.SetAttribute(attr_order, pxr::VtValue(orders), timecode); +} + +void USDCurvesWriter::set_writer_attributes(pxr::UsdGeomCurves &usd_curves, + const pxr::VtArray verts, + const pxr::VtIntArray control_point_counts, + const pxr::VtArray widths, + const pxr::UsdTimeCode timecode, + const pxr::TfToken interpolation) +{ + pxr::UsdAttribute attr_points = usd_curves.CreatePointsAttr(pxr::VtValue(), true); + usd_value_writer_.SetAttribute(attr_points, pxr::VtValue(verts), timecode); + + pxr::UsdAttribute attr_vertex_counts = usd_curves.CreateCurveVertexCountsAttr(pxr::VtValue(), + true); + usd_value_writer_.SetAttribute(attr_vertex_counts, pxr::VtValue(control_point_counts), timecode); + + if (widths.size() > 0) { + pxr::UsdAttribute attr_widths = usd_curves.CreateWidthsAttr(pxr::VtValue(), true); + usd_value_writer_.SetAttribute(attr_widths, pxr::VtValue(widths), timecode); + + usd_curves.SetWidthsInterpolation(interpolation); + } +} + void USDCurvesWriter::do_write(HierarchyContext &context) { @@ -295,7 +367,7 @@ void USDCurvesWriter::do_write(HierarchyContext &context) }); if (number_of_curve_types > 1) { - WM_reportf(RPT_WARNING, "Cannot export mixed curve types."); + WM_report(RPT_WARNING, "Cannot export mixed curve types."); return; } @@ -311,11 +383,11 @@ void USDCurvesWriter::do_write(HierarchyContext &context) } if (!all_same_cyclic_type) { - WM_reportf(RPT_WARNING, "Cannot export mixed cyclic and non-cyclic curves."); + WM_report(RPT_WARNING, "Cannot export mixed cyclic and non-cyclic curves."); return; } - pxr::UsdTimeCode timecode = get_export_time_code(); + const pxr::UsdTimeCode timecode = get_export_time_code(); pxr::UsdGeomCurves usd_curves; pxr::VtArray verts; @@ -357,31 +429,15 @@ void USDCurvesWriter::do_write(HierarchyContext &context) populate_curve_props_for_nurbs( geometry, verts, control_point_counts, widths, knots, orders, interpolation, cyclic); - pxr::UsdAttribute attr_knots = - pxr::UsdGeomNurbsCurves(usd_curves).CreateKnotsAttr(pxr::VtValue(), true); - usd_value_writer_.SetAttribute(attr_knots, pxr::VtValue(knots), timecode); - pxr::UsdAttribute attr_order = - pxr::UsdGeomNurbsCurves(usd_curves).CreateOrderAttr(pxr::VtValue(), true); - usd_value_writer_.SetAttribute(attr_order, pxr::VtValue(orders), timecode); + set_writer_attributes_for_nurbs(usd_curves, knots, orders, timecode); + break; } default: BLI_assert_unreachable(); } - pxr::UsdAttribute attr_points = usd_curves.CreatePointsAttr(pxr::VtValue(), true); - usd_value_writer_.SetAttribute(attr_points, pxr::VtValue(verts), timecode); - - pxr::UsdAttribute attr_vertex_counts = usd_curves.CreateCurveVertexCountsAttr(pxr::VtValue(), - true); - usd_value_writer_.SetAttribute(attr_vertex_counts, pxr::VtValue(control_point_counts), timecode); - - if (widths.size() > 0) { - pxr::UsdAttribute attr_widths = usd_curves.CreateWidthsAttr(pxr::VtValue(), true); - usd_value_writer_.SetAttribute(attr_widths, pxr::VtValue(widths), timecode); - - usd_curves.SetWidthsInterpolation(interpolation); - } + set_writer_attributes(usd_curves, verts, control_point_counts, widths, timecode, interpolation); assign_materials(context, usd_curves); } diff --git a/source/blender/io/usd/intern/usd_writer_curves.h b/source/blender/io/usd/intern/usd_writer_curves.h index 71c9cee3118..0183cdaeba6 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.h +++ b/source/blender/io/usd/intern/usd_writer_curves.h @@ -26,6 +26,18 @@ class USDCurvesWriter : public USDAbstractWriter { private: std::unique_ptr converted_curves_; pxr::UsdGeomCurves DefineUsdGeomBasisCurves(pxr::VtValue curve_basis, bool cyclic, bool cubic); + + void set_writer_attributes(pxr::UsdGeomCurves &usd_curves, + const pxr::VtArray verts, + const pxr::VtIntArray control_point_counts, + const pxr::VtArray widths, + const pxr::UsdTimeCode timecode, + const pxr::TfToken interpolation); + + void set_writer_attributes_for_nurbs(const pxr::UsdGeomCurves usd_curves, + const pxr::VtArray knots, + const pxr::VtArray orders, + const pxr::UsdTimeCode timecode); }; } // namespace blender::io::usd -- 2.30.2 From 22e63a561c04ae434b1399086dd20af1c307fb05 Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Thu, 2 Mar 2023 14:08:21 +0000 Subject: [PATCH 07/19] Rebased changes onto `main` and resolved compilation errors due to changed api for `CurvesGeometry`. Moved tests into new `usd_curves_test` file. --- source/blender/io/usd/CMakeLists.txt | 1 + .../io/usd/intern/usd_hierarchy_iterator.cc | 1 - .../io/usd/intern/usd_writer_curves.cc | 134 ++++---- .../blender/io/usd/tests/usd_curves_test.cc | 294 ++++++++++++++++++ 4 files changed, 359 insertions(+), 71 deletions(-) create mode 100644 source/blender/io/usd/tests/usd_curves_test.cc diff --git a/source/blender/io/usd/CMakeLists.txt b/source/blender/io/usd/CMakeLists.txt index 7a076abcdd0..f5fbb844005 100644 --- a/source/blender/io/usd/CMakeLists.txt +++ b/source/blender/io/usd/CMakeLists.txt @@ -161,6 +161,7 @@ if(WITH_GTESTS) tests/usd_tests_common.cc tests/usd_tests_common.h tests/usd_usdz_export_test.cc + tests/usd_curves_test.cc intern/usd_writer_material.h ) if(USD_IMAGING_HEADERS) diff --git a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc index 95b0aaa6196..d4bb4f96dd6 100644 --- a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc +++ b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc @@ -130,7 +130,6 @@ AbstractHierarchyWriter *USDHierarchyIterator::create_data_writer(const Hierarch case OB_ARMATURE: case OB_GPENCIL_LEGACY: case OB_POINTCLOUD: - case OB_CURVES: return nullptr; case OB_TYPE_MAX: BLI_assert_msg(0, "OB_TYPE_MAX should not be used"); diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index 4de4b552cbd..cc7cc2f2d5c 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -114,8 +114,7 @@ static pxr::TfToken get_curve_width_interpolation(const pxr::VtArray &wid return pxr::TfToken(); } -static void populate_curve_verts(const int i_curve, - const bool cyclic, +static void populate_curve_verts(const bool cyclic, const bool cubic, const bke::CurvesGeometry &geometry, const Span positions, @@ -123,23 +122,26 @@ static void populate_curve_verts(const int i_curve, pxr::VtIntArray &control_point_counts, pxr::VtArray &segments) { - const IndexRange curve_points = geometry.points_for_curve(i_curve); - for (const int i_point : curve_points) { - verts.push_back( - pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); - } + const OffsetIndices points_by_curve = geometry.points_by_curve(); + for (const int i_curve : geometry.curves_range()) { + const IndexRange curve_points = points_by_curve[i_curve]; + for (const int i_point : curve_points) { + verts.push_back( + pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); + } - const int tot_points = curve_points.size(); - control_point_counts[i_curve] = tot_points; + const int tot_points = curve_points.size(); + control_point_counts[i_curve] = tot_points; - if (cyclic) { - segments[i_curve] = tot_points; - } - else if (cubic) { - segments[i_curve] = (tot_points - 4) + 1; - } - else { - segments[i_curve] = tot_points - 1; + if (cyclic) { + segments[i_curve] = tot_points; + } + else if (cubic) { + segments[i_curve] = (tot_points - 4) + 1; + } + else { + segments[i_curve] = tot_points - 1; + } } } @@ -156,17 +158,13 @@ static void populate_curve_props(const bke::CurvesGeometry &geometry, pxr::VtArray segments(num_curves); - for (const int i_curve : geometry.curves_range()) { - populate_curve_verts( - i_curve, cyclic, cubic, geometry, positions, verts, control_point_counts, segments); - } + populate_curve_verts(cyclic, cubic, geometry, positions, verts, control_point_counts, segments); populate_curve_widths(geometry, widths); interpolation = get_curve_width_interpolation(widths, segments, control_point_counts, cyclic); } -static void populate_curve_verts_for_bezier(const int i_curve, - const bool cyclic, +static void populate_curve_verts_for_bezier(const bool cyclic, const bke::CurvesGeometry &geometry, const Span positions, const Span handles_l, @@ -176,50 +174,54 @@ static void populate_curve_verts_for_bezier(const int i_curve, pxr::VtArray &segments) { const int bezier_vstep = 3; - const IndexRange curve_points = geometry.points_for_curve(i_curve); - + const OffsetIndices points_by_curve = geometry.points_by_curve(); const int start_verts_count = verts.size(); - const int start_point_index = curve_points[0]; - const int last_point_index = curve_points[curve_points.size() - 1]; - for (int i_point = start_point_index; i_point < last_point_index; i_point++) { + for (int i_curve = 0; i_curve < geometry.curve_num; i_curve++) { + const IndexRange curve_points = points_by_curve[i_curve]; + const int start_point_index = curve_points[0]; + const int last_point_index = curve_points[curve_points.size() - 1]; + for (int i_point = start_point_index; i_point < last_point_index; i_point++) { - /* The order verts in the USD bezier curve representation is [control point 0, right handle 0, - * left handle 1, control point 1, right handle 1, left handle 2, control point 2, ...]. The - * last vert in the array doesn't need a right handle because the curve stops at that point. */ - verts.push_back( - pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); + /* The order verts in the USD bezier curve representation is [control point 0, right handle + * 0, left handle 1, control point 1, right handle 1, left handle 2, control point 2, ...]. + * The last vert in the array doesn't need a right handle because the curve stops at that + * point. */ + verts.push_back( + pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); - const blender::float3 right_handle = handles_r[i_point]; - verts.push_back(pxr::GfVec3f(right_handle[0], right_handle[1], right_handle[2])); + const blender::float3 right_handle = handles_r[i_point]; + verts.push_back(pxr::GfVec3f(right_handle[0], right_handle[1], right_handle[2])); - const blender::float3 left_handle = handles_l[i_point + 1]; - verts.push_back(pxr::GfVec3f(left_handle[0], left_handle[1], left_handle[2])); - } + const blender::float3 left_handle = handles_l[i_point + 1]; + verts.push_back(pxr::GfVec3f(left_handle[0], left_handle[1], left_handle[2])); + } - verts.push_back(pxr::GfVec3f(positions[last_point_index][0], - positions[last_point_index][1], - positions[last_point_index][2])); + verts.push_back(pxr::GfVec3f(positions[last_point_index][0], + positions[last_point_index][1], + positions[last_point_index][2])); - /* For USD representation of periodic bezier curve, one of the curve's points must be repeated - * to close the curve. The repeated point is the first point. Since the curve is closed, we now - * need to include the right handle of the last point and the left handle of the first point. */ - if (cyclic) { - const blender::float3 right_handle = handles_r[last_point_index]; - verts.push_back(pxr::GfVec3f(right_handle[0], right_handle[1], right_handle[2])); + /* For USD representation of periodic bezier curve, one of the curve's points must be repeated + * to close the curve. The repeated point is the first point. Since the curve is closed, we now + * need to include the right handle of the last point and the left handle of the first point. + */ + if (cyclic) { + const blender::float3 right_handle = handles_r[last_point_index]; + verts.push_back(pxr::GfVec3f(right_handle[0], right_handle[1], right_handle[2])); - const blender::float3 left_handle = handles_l[start_point_index]; - verts.push_back(pxr::GfVec3f(left_handle[0], left_handle[1], left_handle[2])); - } + const blender::float3 left_handle = handles_l[start_point_index]; + verts.push_back(pxr::GfVec3f(left_handle[0], left_handle[1], left_handle[2])); + } - const int tot_points = verts.size() - start_verts_count; - control_point_counts[i_curve] = tot_points; + const int tot_points = verts.size() - start_verts_count; + control_point_counts[i_curve] = tot_points; - if (cyclic) { - segments[i_curve] = tot_points / bezier_vstep; - } - else { - segments[i_curve] = ((tot_points - 4) / bezier_vstep) + 1; + if (cyclic) { + segments[i_curve] = tot_points / bezier_vstep; + } + else { + segments[i_curve] = ((tot_points - 4) / bezier_vstep) + 1; + } } } @@ -240,17 +242,8 @@ static void populate_curve_props_for_bezier(const bke::CurvesGeometry &geometry, pxr::VtArray segments(num_curves); - for (int i_curve = 0; i_curve < num_curves; i_curve++) { - populate_curve_verts_for_bezier(i_curve, - cyclic, - geometry, - positions, - handles_l, - handles_r, - verts, - control_point_counts, - segments); - } + populate_curve_verts_for_bezier( + cyclic, geometry, positions, handles_l, handles_r, verts, control_point_counts, segments); populate_curve_widths(geometry, widths); interpolation = get_curve_width_interpolation(widths, segments, control_point_counts, cyclic); @@ -273,8 +266,9 @@ static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, VArray geom_orders = geometry.nurbs_orders(); VArray knots_modes = geometry.nurbs_knots_modes(); + const OffsetIndices points_by_curve = geometry.points_by_curve(); for (const int i_curve : geometry.curves_range()) { - const IndexRange curve_points = geometry.points_for_curve(i_curve); + const IndexRange curve_points = points_by_curve[i_curve]; for (const int i_point : curve_points) { verts.push_back( pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); @@ -355,7 +349,7 @@ void USDCurvesWriter::do_write(HierarchyContext &context) const Curves *curve = converted_curves_ ? converted_curves_.get() : static_cast(context.object->data); - const bke::CurvesGeometry &geometry = bke::CurvesGeometry::wrap(curve->geometry); + const bke::CurvesGeometry &geometry = curve->geometry.wrap(); if (geometry.points_num() == 0) { return; } diff --git a/source/blender/io/usd/tests/usd_curves_test.cc b/source/blender/io/usd/tests/usd_curves_test.cc new file mode 100644 index 00000000000..66e81fd48c5 --- /dev/null +++ b/source/blender/io/usd/tests/usd_curves_test.cc @@ -0,0 +1,294 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include "testing/testing.h" +#include "tests/blendfile_loading_base_test.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "DNA_material_types.h" +#include "DNA_node_types.h" + +#include "BKE_context.h" +#include "BKE_lib_id.h" +#include "BKE_main.h" +#include "BKE_mesh.h" +#include "BKE_node.h" +#include "BLI_fileops.h" +#include "BLI_math.h" +#include "BLI_math_vector_types.hh" +#include "BLI_path_util.h" +#include "BLO_readfile.h" + +#include "BKE_node_runtime.hh" + +#include "DEG_depsgraph.h" + +#include "WM_api.h" + +#include "usd.h" +#include "usd_tests_common.h" + +namespace blender::io::usd { + +const StringRefNull usd_curves_test_filename = "usd/usd_curves_test.blend"; +const StringRefNull output_filename = "usd/output.usda"; + +void check_catmullRom_curve(const pxr::UsdPrim prim, + const bool is_periodic, + const int vertex_count); +void check_bezier_curve(const pxr::UsdPrim bezier_prim, + const bool is_periodic, + const int vertex_count); +void check_nurbs_curve(const pxr::UsdPrim nurbs_prim, + const int vertex_count, + const int knots_count, + const int order, + const bool is_periodic); + +class UsdCurvesTest : public BlendfileLoadingBaseTest { + protected: + struct bContext *context = nullptr; + + public: + bool load_file_and_depsgraph(const StringRefNull &filepath, + const eEvaluationMode eval_mode = DAG_EVAL_VIEWPORT) + { + if (!blendfile_load(filepath.c_str())) { + return false; + } + depsgraph_create(eval_mode); + + context = CTX_create(); + CTX_data_main_set(context, bfile->main); + CTX_data_scene_set(context, bfile->curscene); + + return true; + } + + virtual void SetUp() override + { + BlendfileLoadingBaseTest::SetUp(); + std::string usd_plugin_path = register_usd_plugins_for_tests(); + if (usd_plugin_path.empty()) { + FAIL(); + } + } + + virtual void TearDown() override + { + BlendfileLoadingBaseTest::TearDown(); + CTX_free(context); + context = nullptr; + + if (BLI_exists(output_filename.c_str())) { + BLI_delete(output_filename.c_str(), false, false); + } + } +}; + +TEST_F(UsdCurvesTest, usd_export_curves) +{ + if (!load_file_and_depsgraph(usd_curves_test_filename)) { + ADD_FAILURE(); + return; + } + + /* File sanity check. */ + EXPECT_EQ(BLI_listbase_count(&bfile->main->objects), 6); + + USDExportParams params{}; + + const bool result = USD_export(context, output_filename.c_str(), ¶ms, false); + EXPECT_TRUE(result); + + pxr::UsdStageRefPtr stage = pxr::UsdStage::Open(output_filename); + ASSERT_NE(stage, nullptr); + + { + std::string prim_name = pxr::TfMakeValidIdentifier("BezierCurve"); + pxr::UsdPrim test_prim = stage->GetPrimAtPath(pxr::SdfPath("/BezierCurve/" + prim_name)); + EXPECT_TRUE(test_prim.IsValid()); + check_bezier_curve(test_prim, false, 7); + } + + { + std::string prim_name = pxr::TfMakeValidIdentifier("BezierCircle"); + pxr::UsdPrim test_prim = stage->GetPrimAtPath(pxr::SdfPath("/BezierCircle/" + prim_name)); + EXPECT_TRUE(test_prim.IsValid()); + check_bezier_curve(test_prim, true, 12); + } + + { + std::string prim_name = pxr::TfMakeValidIdentifier("NurbsCurve"); + pxr::UsdPrim test_prim = stage->GetPrimAtPath(pxr::SdfPath("/NurbsCurve/" + prim_name)); + EXPECT_TRUE(test_prim.IsValid()); + check_nurbs_curve(test_prim, 6, 10, 4, false); + } + + { + std::string prim_name = pxr::TfMakeValidIdentifier("NurbsCircle"); + pxr::UsdPrim test_prim = stage->GetPrimAtPath(pxr::SdfPath("/NurbsCircle/" + prim_name)); + EXPECT_TRUE(test_prim.IsValid()); + check_nurbs_curve(test_prim, 8, 13, 3, true); + } + + { + std::string prim_name = pxr::TfMakeValidIdentifier("Curves"); + pxr::UsdPrim test_prim = stage->GetPrimAtPath(pxr::SdfPath("/Cube/Curves/" + prim_name)); + EXPECT_TRUE(test_prim.IsValid()); + check_catmullRom_curve(test_prim, false, 8); + } +} + +/** + * Test that the provided prim is a valid catmullRom curve. We also check it matches the expected + * wrap type, and has the expected number of vertices. + */ +static void check_catmullRom_curve(const pxr::UsdPrim prim, + const bool is_periodic, + const int vertex_count) +{ + auto curve = pxr::UsdGeomBasisCurves(prim); + + pxr::VtValue basis; + pxr::UsdAttribute basis_attr = curve.GetBasisAttr(); + basis_attr.Get(&basis); + auto basis_token = basis.Get(); + + EXPECT_EQ(basis_token, pxr::UsdGeomTokens->catmullRom); + + pxr::VtValue type; + pxr::UsdAttribute type_attr = curve.GetTypeAttr(); + type_attr.Get(&type); + auto type_token = type.Get(); + + EXPECT_EQ(type_token, pxr::UsdGeomTokens->cubic); + + pxr::VtValue wrap; + pxr::UsdAttribute wrap_attr = curve.GetWrapAttr(); + wrap_attr.Get(&wrap); + auto wrap_token = wrap.Get(); + + if (is_periodic) { + EXPECT_EQ(wrap_token, pxr::UsdGeomTokens->periodic); + } + else { + EXPECT_EQ(wrap_token, pxr::UsdGeomTokens->nonperiodic); + } + + pxr::UsdAttribute vert_count_attr = curve.GetCurveVertexCountsAttr(); + pxr::VtArray vert_counts; + vert_count_attr.Get(&vert_counts); + + EXPECT_EQ(vert_counts.size(), 3); + EXPECT_EQ(vert_counts[0], vertex_count); + EXPECT_EQ(vert_counts[1], vertex_count); + EXPECT_EQ(vert_counts[2], vertex_count); +} + +/** + * Test that the provided prim is a valid bezier curve. We also check it matches the expected + * wrap type, and has the expected number of vertices. + */ +static void check_bezier_curve(const pxr::UsdPrim bezier_prim, + const bool is_periodic, + const int vertex_count) +{ + auto curve = pxr::UsdGeomBasisCurves(bezier_prim); + + pxr::VtValue basis; + pxr::UsdAttribute basis_attr = curve.GetBasisAttr(); + basis_attr.Get(&basis); + auto basis_token = basis.Get(); + + EXPECT_EQ(basis_token, pxr::UsdGeomTokens->bezier); + + pxr::VtValue type; + pxr::UsdAttribute type_attr = curve.GetTypeAttr(); + type_attr.Get(&type); + auto type_token = type.Get(); + + EXPECT_EQ(type_token, pxr::UsdGeomTokens->cubic); + + pxr::VtValue wrap; + pxr::UsdAttribute wrap_attr = curve.GetWrapAttr(); + wrap_attr.Get(&wrap); + auto wrap_token = wrap.Get(); + + if (is_periodic) { + EXPECT_EQ(wrap_token, pxr::UsdGeomTokens->periodic); + } + else { + EXPECT_EQ(wrap_token, pxr::UsdGeomTokens->nonperiodic); + } + + auto widths_interp_token = curve.GetWidthsInterpolation(); + EXPECT_EQ(widths_interp_token, pxr::UsdGeomTokens->varying); + + pxr::UsdAttribute vert_count_attr = curve.GetCurveVertexCountsAttr(); + pxr::VtArray vert_counts; + vert_count_attr.Get(&vert_counts); + + EXPECT_EQ(vert_counts.size(), 1); + EXPECT_EQ(vert_counts[0], vertex_count); +} + +/** + * Test that the provided prim is a valid NURBS curve. We also check it matches the expected + * wrap type, and has the expected number of vertices. For NURBS, we also validate that the knots + * layout matches the expected layout for periodic/non-periodic curves according to the USD spec. + */ +static void check_nurbs_curve(const pxr::UsdPrim nurbs_prim, + const int vertex_count, + const int knots_count, + const int order, + const bool is_periodic) +{ + auto curve = pxr::UsdGeomNurbsCurves(nurbs_prim); + + pxr::UsdAttribute order_attr = curve.GetOrderAttr(); + pxr::VtArray orders; + order_attr.Get(&orders); + + EXPECT_EQ(orders.size(), 1); + EXPECT_EQ(orders[0], order); + + pxr::UsdAttribute knots_attr = curve.GetKnotsAttr(); + pxr::VtArray knots; + knots_attr.Get(&knots); + + EXPECT_EQ(knots.size(), knots_count); + if (is_periodic) { + EXPECT_EQ(knots[0], knots[1] - (knots[knots.size() - 2] - knots[knots.size() - 3])); + EXPECT_EQ(knots[knots.size() - 1], knots[knots.size() - 2] + (knots[2] - knots[1])); + } + else { + EXPECT_EQ(knots[0], knots[1]); + EXPECT_EQ(knots[knots.size() - 1], knots[knots.size() - 2]); + } + + auto widths_interp_token = curve.GetWidthsInterpolation(); + EXPECT_EQ(widths_interp_token, pxr::UsdGeomTokens->vertex); + + pxr::UsdAttribute vert_count_attr = curve.GetCurveVertexCountsAttr(); + pxr::VtArray vert_counts; + vert_count_attr.Get(&vert_counts); + + EXPECT_EQ(vert_counts.size(), 1); + EXPECT_EQ(vert_counts[0], vertex_count); +} + +} // namespace blender::io::usd -- 2.30.2 From 1399031b0f02b5c301b4465d442812a6269aa591 Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Fri, 3 Mar 2023 09:59:05 +0000 Subject: [PATCH 08/19] Switch to `is_cyclic` and `is_cubic` for bool parameters --- .../io/usd/intern/usd_writer_curves.cc | 78 ++++++++++--------- 1 file changed, 41 insertions(+), 37 deletions(-) diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index cc7cc2f2d5c..e9d991a3225 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -43,8 +43,8 @@ USDCurvesWriter::~USDCurvesWriter() } pxr::UsdGeomCurves USDCurvesWriter::DefineUsdGeomBasisCurves(pxr::VtValue curve_basis, - bool cyclic, - bool cubic) + const bool is_cyclic, + const bool is_cubic) { pxr::UsdGeomCurves curves = pxr::UsdGeomBasisCurves::Define(usd_export_context_.stage, usd_export_context_.usd_path); @@ -52,7 +52,7 @@ pxr::UsdGeomCurves USDCurvesWriter::DefineUsdGeomBasisCurves(pxr::VtValue curve_ pxr::UsdGeomBasisCurves basis_curves = pxr::UsdGeomBasisCurves(curves); /* Not required to set the basis attribute for linear curves * https://graphics.pixar.com/usd/dev/api/class_usd_geom_basis_curves.html#details */ - if (cubic) { + if (is_cubic) { basis_curves.CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->cubic)); basis_curves.CreateBasisAttr(curve_basis); } @@ -60,7 +60,7 @@ pxr::UsdGeomCurves USDCurvesWriter::DefineUsdGeomBasisCurves(pxr::VtValue curve_ basis_curves.CreateTypeAttr(pxr::VtValue(pxr::UsdGeomTokens->linear)); } - if (cyclic) { + if (is_cyclic) { basis_curves.CreateWrapAttr(pxr::VtValue(pxr::UsdGeomTokens->periodic)); } else { @@ -85,7 +85,7 @@ static void populate_curve_widths(const bke::CurvesGeometry &geometry, pxr::VtAr static pxr::TfToken get_curve_width_interpolation(const pxr::VtArray &widths, const pxr::VtArray &segments, const pxr::VtIntArray &control_point_counts, - const bool cyclic) + const bool is_cyclic) { if (widths.empty()) { return pxr::TfToken(); @@ -102,7 +102,7 @@ static pxr::TfToken get_curve_width_interpolation(const pxr::VtArray &wid } size_t expectedVaryingSize = std::accumulate(segments.begin(), segments.end(), 0); - if (!cyclic) { + if (!is_cyclic) { expectedVaryingSize += control_point_counts.size(); } @@ -114,16 +114,17 @@ static pxr::TfToken get_curve_width_interpolation(const pxr::VtArray &wid return pxr::TfToken(); } -static void populate_curve_verts(const bool cyclic, - const bool cubic, - const bke::CurvesGeometry &geometry, +static void populate_curve_verts(const bke::CurvesGeometry &geometry, const Span positions, pxr::VtArray &verts, pxr::VtIntArray &control_point_counts, - pxr::VtArray &segments) + pxr::VtArray &segments, + const bool is_cyclic, + const bool is_cubic) { const OffsetIndices points_by_curve = geometry.points_by_curve(); for (const int i_curve : geometry.curves_range()) { + const IndexRange curve_points = points_by_curve[i_curve]; for (const int i_point : curve_points) { verts.push_back( @@ -133,10 +134,10 @@ static void populate_curve_verts(const bool cyclic, const int tot_points = curve_points.size(); control_point_counts[i_curve] = tot_points; - if (cyclic) { + if (is_cyclic) { segments[i_curve] = tot_points; } - else if (cubic) { + else if (is_cubic) { segments[i_curve] = (tot_points - 4) + 1; } else { @@ -150,37 +151,40 @@ static void populate_curve_props(const bke::CurvesGeometry &geometry, pxr::VtIntArray &control_point_counts, pxr::VtArray &widths, pxr::TfToken &interpolation, - const bool cyclic, - const bool cubic) + const bool is_cyclic, + const bool is_cubic) { const int num_curves = geometry.curve_num; const Span positions = geometry.positions(); pxr::VtArray segments(num_curves); - populate_curve_verts(cyclic, cubic, geometry, positions, verts, control_point_counts, segments); + populate_curve_verts( + geometry, positions, verts, control_point_counts, segments, is_cyclic, is_cubic); populate_curve_widths(geometry, widths); - interpolation = get_curve_width_interpolation(widths, segments, control_point_counts, cyclic); + interpolation = get_curve_width_interpolation(widths, segments, control_point_counts, is_cyclic); } -static void populate_curve_verts_for_bezier(const bool cyclic, - const bke::CurvesGeometry &geometry, +static void populate_curve_verts_for_bezier(const bke::CurvesGeometry &geometry, const Span positions, const Span handles_l, const Span handles_r, pxr::VtArray &verts, pxr::VtIntArray &control_point_counts, - pxr::VtArray &segments) + pxr::VtArray &segments, + const bool is_cyclic) { const int bezier_vstep = 3; const OffsetIndices points_by_curve = geometry.points_by_curve(); const int start_verts_count = verts.size(); for (int i_curve = 0; i_curve < geometry.curve_num; i_curve++) { + const IndexRange curve_points = points_by_curve[i_curve]; const int start_point_index = curve_points[0]; const int last_point_index = curve_points[curve_points.size() - 1]; + for (int i_point = start_point_index; i_point < last_point_index; i_point++) { /* The order verts in the USD bezier curve representation is [control point 0, right handle @@ -205,7 +209,7 @@ static void populate_curve_verts_for_bezier(const bool cyclic, * to close the curve. The repeated point is the first point. Since the curve is closed, we now * need to include the right handle of the last point and the left handle of the first point. */ - if (cyclic) { + if (is_cyclic) { const blender::float3 right_handle = handles_r[last_point_index]; verts.push_back(pxr::GfVec3f(right_handle[0], right_handle[1], right_handle[2])); @@ -216,7 +220,7 @@ static void populate_curve_verts_for_bezier(const bool cyclic, const int tot_points = verts.size() - start_verts_count; control_point_counts[i_curve] = tot_points; - if (cyclic) { + if (is_cyclic) { segments[i_curve] = tot_points / bezier_vstep; } else { @@ -230,7 +234,7 @@ static void populate_curve_props_for_bezier(const bke::CurvesGeometry &geometry, pxr::VtIntArray &control_point_counts, pxr::VtArray &widths, pxr::TfToken &interpolation, - const bool cyclic) + const bool is_cyclic) { const int num_curves = geometry.curve_num; @@ -243,10 +247,10 @@ static void populate_curve_props_for_bezier(const bke::CurvesGeometry &geometry, pxr::VtArray segments(num_curves); populate_curve_verts_for_bezier( - cyclic, geometry, positions, handles_l, handles_r, verts, control_point_counts, segments); + geometry, positions, handles_l, handles_r, verts, control_point_counts, segments, is_cyclic); populate_curve_widths(geometry, widths); - interpolation = get_curve_width_interpolation(widths, segments, control_point_counts, cyclic); + interpolation = get_curve_width_interpolation(widths, segments, control_point_counts, is_cyclic); } static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, @@ -256,7 +260,7 @@ static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, pxr::VtArray &knots, pxr::VtArray &orders, pxr::TfToken &interpolation, - const bool cyclic) + const bool is_cyclic) { const int num_curves = geometry.curve_num; orders.resize(num_curves); @@ -282,9 +286,9 @@ static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, const KnotsMode mode = KnotsMode(knots_modes[i_curve]); - const int knots_num = bke::curves::nurbs::knots_num(tot_points, order, cyclic); + const int knots_num = bke::curves::nurbs::knots_num(tot_points, order, is_cyclic); Array temp_knots(knots_num); - bke::curves::nurbs::calculate_knots(tot_points, mode, order, cyclic, temp_knots); + bke::curves::nurbs::calculate_knots(tot_points, mode, order, is_cyclic, temp_knots); knots.resize(knots_num); for (int i_knot = 0; i_knot < knots_num; i_knot++) { @@ -293,7 +297,7 @@ static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, /* Set end knots according to the USD spec for periodic curves * https://graphics.pixar.com/usd/dev/api/class_usd_geom_nurbs_curves.html#details */ - if (cyclic) { + if (is_cyclic) { knots[0] = knots[1] - (knots[knots.size() - 2] - knots[knots.size() - 3]); knots[knots.size() - 1] = knots[knots.size() - 2] + (knots[2] - knots[1]); } @@ -366,11 +370,11 @@ void USDCurvesWriter::do_write(HierarchyContext &context) } const VArray cyclic_values = geometry.cyclic(); - const bool cyclic = cyclic_values[0]; + const bool is_cyclic = cyclic_values[0]; bool all_same_cyclic_type = true; for (const int i : cyclic_values.index_range()) { - if (cyclic_values[i] != cyclic) { + if (cyclic_values[i] != is_cyclic) { all_same_cyclic_type = false; break; } @@ -393,24 +397,24 @@ void USDCurvesWriter::do_write(HierarchyContext &context) const int8_t curve_type = geometry.curve_types()[0]; switch (curve_type) { case CURVE_TYPE_POLY: - usd_curves = DefineUsdGeomBasisCurves(pxr::VtValue(), cyclic, false); + usd_curves = DefineUsdGeomBasisCurves(pxr::VtValue(), is_cyclic, false); populate_curve_props( - geometry, verts, control_point_counts, widths, interpolation, cyclic, false); + geometry, verts, control_point_counts, widths, interpolation, is_cyclic, false); break; case CURVE_TYPE_CATMULL_ROM: usd_curves = DefineUsdGeomBasisCurves( - pxr::VtValue(pxr::UsdGeomTokens->catmullRom), cyclic, true); + pxr::VtValue(pxr::UsdGeomTokens->catmullRom), is_cyclic, true); populate_curve_props( - geometry, verts, control_point_counts, widths, interpolation, cyclic, true); + geometry, verts, control_point_counts, widths, interpolation, is_cyclic, true); break; case CURVE_TYPE_BEZIER: usd_curves = DefineUsdGeomBasisCurves( - pxr::VtValue(pxr::UsdGeomTokens->bezier), cyclic, true); + pxr::VtValue(pxr::UsdGeomTokens->bezier), is_cyclic, true); populate_curve_props_for_bezier( - geometry, verts, control_point_counts, widths, interpolation, cyclic); + geometry, verts, control_point_counts, widths, interpolation, is_cyclic); break; case CURVE_TYPE_NURBS: { pxr::VtArray knots; @@ -421,7 +425,7 @@ void USDCurvesWriter::do_write(HierarchyContext &context) usd_export_context_.usd_path); populate_curve_props_for_nurbs( - geometry, verts, control_point_counts, widths, knots, orders, interpolation, cyclic); + geometry, verts, control_point_counts, widths, knots, orders, interpolation, is_cyclic); set_writer_attributes_for_nurbs(usd_curves, knots, orders, timecode); -- 2.30.2 From a1587d6b0156d6b27e835b69415ad15d9dbc794f Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Fri, 3 Mar 2023 13:23:04 +0000 Subject: [PATCH 09/19] Include first vert as repeated vert for bezier curve. Include comment to explain segment counts for linear and cubic curves. --- source/blender/io/usd/intern/usd_writer_curves.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index e9d991a3225..0c148866915 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -134,6 +134,12 @@ static void populate_curve_verts(const bke::CurvesGeometry &geometry, const int tot_points = curve_points.size(); control_point_counts[i_curve] = tot_points; + /* For periodic linear curve, segment count = curveVertexCount. + For periodic cubic curve, segment count = curveVertexCount / vstep. + For nonperiodic linear curve, segment count = curveVertexCount - 1. + For nonperiodic cubic curve, segment count = ((curveVertexCount - 4) / vstep) + 1. + This function handles linear and Catmull-Rom curves. For Catmull-Rom, vstep is 1. + https://graphics.pixar.com/usd/dev/api/class_usd_geom_basis_curves.html */ if (is_cyclic) { segments[i_curve] = tot_points; } @@ -215,6 +221,10 @@ static void populate_curve_verts_for_bezier(const bke::CurvesGeometry &geometry, const blender::float3 left_handle = handles_l[start_point_index]; verts.push_back(pxr::GfVec3f(left_handle[0], left_handle[1], left_handle[2])); + + verts.push_back(pxr::GfVec3f(positions[start_point_index][0], + positions[start_point_index][1], + positions[start_point_index][2])); } const int tot_points = verts.size() - start_verts_count; -- 2.30.2 From a86c4e406cc00fba2129a2d095130d06de9b3c41 Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Mon, 6 Mar 2023 10:13:29 +0000 Subject: [PATCH 10/19] Remove `check_is_animated` to perform default animation check on usd export --- source/blender/io/usd/intern/usd_writer_curves.cc | 5 ----- source/blender/io/usd/intern/usd_writer_curves.h | 1 - 2 files changed, 6 deletions(-) diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index 0c148866915..4b8bc74db68 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -450,11 +450,6 @@ void USDCurvesWriter::do_write(HierarchyContext &context) assign_materials(context, usd_curves); } -bool USDCurvesWriter::check_is_animated(const HierarchyContext &) const -{ - return true; -} - void USDCurvesWriter::assign_materials(const HierarchyContext &context, pxr::UsdGeomCurves usd_curve) { diff --git a/source/blender/io/usd/intern/usd_writer_curves.h b/source/blender/io/usd/intern/usd_writer_curves.h index 0183cdaeba6..2c27fef63e9 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.h +++ b/source/blender/io/usd/intern/usd_writer_curves.h @@ -20,7 +20,6 @@ class USDCurvesWriter : public USDAbstractWriter { protected: virtual void do_write(HierarchyContext &context) override; - virtual bool check_is_animated(const HierarchyContext &context) const override; void assign_materials(const HierarchyContext &context, pxr::UsdGeomCurves usd_curve); private: -- 2.30.2 From c72c159babfac8fae935500387ecf57d4292df99 Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Mon, 6 Mar 2023 15:45:24 +0000 Subject: [PATCH 11/19] Move legacy curve conversion into `do_write` function as the context object data can and will change frame by frame. --- .../io/usd/intern/usd_hierarchy_iterator.cc | 9 +---- .../io/usd/intern/usd_writer_curves.cc | 37 ++++++++++++------- .../blender/io/usd/intern/usd_writer_curves.h | 3 +- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc index d4bb4f96dd6..e9abdcfd22d 100644 --- a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc +++ b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc @@ -18,7 +18,6 @@ #include -#include "BKE_curve_legacy_convert.hh" #include "BKE_duplilist.h" #include "BLI_assert.h" @@ -108,13 +107,7 @@ AbstractHierarchyWriter *USDHierarchyIterator::create_data_writer(const Hierarch case OB_MBALL: data_writer = new USDMetaballWriter(usd_export_context); break; - case OB_CURVES_LEGACY: { - const Curve *legacy_curve = static_cast(context->object->data); - std::unique_ptr curves = std::unique_ptr( - bke::curve_legacy_to_curves(*legacy_curve)); - data_writer = new USDCurvesWriter(usd_export_context, std::move(curves)); - break; - } + case OB_CURVES_LEGACY: case OB_CURVES: data_writer = new USDCurvesWriter(usd_export_context); break; diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index 4b8bc74db68..226cb369520 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -13,6 +13,7 @@ #include "usd_hierarchy_iterator.h" #include "usd_writer_curves.h" +#include "BKE_curve_legacy_convert.hh" #include "BKE_curves.hh" #include "BKE_lib_id.h" #include "BKE_material.h" @@ -24,22 +25,12 @@ namespace blender::io::usd { -USDCurvesWriter::USDCurvesWriter(const USDExporterContext &ctx) - : USDAbstractWriter(ctx), converted_curves_(nullptr) -{ -} - -USDCurvesWriter::USDCurvesWriter(const USDExporterContext &ctx, - std::unique_ptr converted_legacy_curves) - : USDAbstractWriter(ctx), converted_curves_(std::move(converted_legacy_curves)) +USDCurvesWriter::USDCurvesWriter(const USDExporterContext &ctx) : USDAbstractWriter(ctx) { } USDCurvesWriter::~USDCurvesWriter() { - if (converted_curves_) { - BKE_id_free(nullptr, converted_curves_.release()); - } } pxr::UsdGeomCurves USDCurvesWriter::DefineUsdGeomBasisCurves(pxr::VtValue curve_basis, @@ -359,11 +350,25 @@ void USDCurvesWriter::set_writer_attributes(pxr::UsdGeomCurves &usd_curves, void USDCurvesWriter::do_write(HierarchyContext &context) { + Curves *curves; + std::unique_ptr converted_curves; - const Curves *curve = converted_curves_ ? converted_curves_.get() : - static_cast(context.object->data); + switch (context.object->type) { + case OB_CURVES_LEGACY: { + const Curve *legacy_curve = static_cast(context.object->data); + converted_curves = std::unique_ptr(bke::curve_legacy_to_curves(*legacy_curve)); + curves = converted_curves.get(); + break; + } + case OB_CURVES: + curves = static_cast(context.object->data); + break; + default: + BLI_assert_unreachable(); + return; + } - const bke::CurvesGeometry &geometry = curve->geometry.wrap(); + const bke::CurvesGeometry &geometry = curves->geometry.wrap(); if (geometry.points_num() == 0) { return; } @@ -448,6 +453,10 @@ void USDCurvesWriter::do_write(HierarchyContext &context) set_writer_attributes(usd_curves, verts, control_point_counts, widths, timecode, interpolation); assign_materials(context, usd_curves); + + if (converted_curves) { + BKE_id_free(nullptr, converted_curves.release()); + } } void USDCurvesWriter::assign_materials(const HierarchyContext &context, diff --git a/source/blender/io/usd/intern/usd_writer_curves.h b/source/blender/io/usd/intern/usd_writer_curves.h index 2c27fef63e9..1e9bd20cf41 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.h +++ b/source/blender/io/usd/intern/usd_writer_curves.h @@ -23,9 +23,8 @@ class USDCurvesWriter : public USDAbstractWriter { void assign_materials(const HierarchyContext &context, pxr::UsdGeomCurves usd_curve); private: - std::unique_ptr converted_curves_; pxr::UsdGeomCurves DefineUsdGeomBasisCurves(pxr::VtValue curve_basis, bool cyclic, bool cubic); - + void set_writer_attributes(pxr::UsdGeomCurves &usd_curves, const pxr::VtArray verts, const pxr::VtIntArray control_point_counts, -- 2.30.2 From 35df5aa325c2855a8c1cc527f48a796fc30cdbd7 Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Mon, 6 Mar 2023 15:46:07 +0000 Subject: [PATCH 12/19] Add descriptions to unit test checks and assertions --- .../blender/io/usd/tests/usd_curves_test.cc | 70 +++++++++++-------- 1 file changed, 42 insertions(+), 28 deletions(-) diff --git a/source/blender/io/usd/tests/usd_curves_test.cc b/source/blender/io/usd/tests/usd_curves_test.cc index 66e81fd48c5..e3e2be2009b 100644 --- a/source/blender/io/usd/tests/usd_curves_test.cc +++ b/source/blender/io/usd/tests/usd_curves_test.cc @@ -112,10 +112,10 @@ TEST_F(UsdCurvesTest, usd_export_curves) USDExportParams params{}; const bool result = USD_export(context, output_filename.c_str(), ¶ms, false); - EXPECT_TRUE(result); + EXPECT_TRUE(result) << "USD export should succed."; pxr::UsdStageRefPtr stage = pxr::UsdStage::Open(output_filename); - ASSERT_NE(stage, nullptr); + ASSERT_NE(stage, nullptr) << "Stage should not be null after opening usd file."; { std::string prim_name = pxr::TfMakeValidIdentifier("BezierCurve"); @@ -128,7 +128,7 @@ TEST_F(UsdCurvesTest, usd_export_curves) std::string prim_name = pxr::TfMakeValidIdentifier("BezierCircle"); pxr::UsdPrim test_prim = stage->GetPrimAtPath(pxr::SdfPath("/BezierCircle/" + prim_name)); EXPECT_TRUE(test_prim.IsValid()); - check_bezier_curve(test_prim, true, 12); + check_bezier_curve(test_prim, true, 13); } { @@ -168,14 +168,16 @@ static void check_catmullRom_curve(const pxr::UsdPrim prim, basis_attr.Get(&basis); auto basis_token = basis.Get(); - EXPECT_EQ(basis_token, pxr::UsdGeomTokens->catmullRom); + EXPECT_EQ(basis_token, pxr::UsdGeomTokens->catmullRom) + << "Basis token should be catmullRom for catmullRom curve"; pxr::VtValue type; pxr::UsdAttribute type_attr = curve.GetTypeAttr(); type_attr.Get(&type); auto type_token = type.Get(); - EXPECT_EQ(type_token, pxr::UsdGeomTokens->cubic); + EXPECT_EQ(type_token, pxr::UsdGeomTokens->cubic) + << "Type token should be cubic for catmullRom curve"; pxr::VtValue wrap; pxr::UsdAttribute wrap_attr = curve.GetWrapAttr(); @@ -183,20 +185,22 @@ static void check_catmullRom_curve(const pxr::UsdPrim prim, auto wrap_token = wrap.Get(); if (is_periodic) { - EXPECT_EQ(wrap_token, pxr::UsdGeomTokens->periodic); + EXPECT_EQ(wrap_token, pxr::UsdGeomTokens->periodic) + << "Wrap token should be periodic for periodic curve"; } else { - EXPECT_EQ(wrap_token, pxr::UsdGeomTokens->nonperiodic); + EXPECT_EQ(wrap_token, pxr::UsdGeomTokens->nonperiodic) + << "Wrap token should be nonperiodic for nonperiodic curve"; } pxr::UsdAttribute vert_count_attr = curve.GetCurveVertexCountsAttr(); pxr::VtArray vert_counts; vert_count_attr.Get(&vert_counts); - EXPECT_EQ(vert_counts.size(), 3); - EXPECT_EQ(vert_counts[0], vertex_count); - EXPECT_EQ(vert_counts[1], vertex_count); - EXPECT_EQ(vert_counts[2], vertex_count); + EXPECT_EQ(vert_counts.size(), 3) << "Prim should contain verts for three curves"; + EXPECT_EQ(vert_counts[0], vertex_count) << "Curve 0 should have " << vertex_count << " verts."; + EXPECT_EQ(vert_counts[1], vertex_count) << "Curve 1 should have " << vertex_count << " verts."; + EXPECT_EQ(vert_counts[2], vertex_count) << "Curve 2 should have " << vertex_count << " verts."; } /** @@ -214,14 +218,16 @@ static void check_bezier_curve(const pxr::UsdPrim bezier_prim, basis_attr.Get(&basis); auto basis_token = basis.Get(); - EXPECT_EQ(basis_token, pxr::UsdGeomTokens->bezier); + EXPECT_EQ(basis_token, pxr::UsdGeomTokens->bezier) + << "Basis token should be bezier for bezier curve"; pxr::VtValue type; pxr::UsdAttribute type_attr = curve.GetTypeAttr(); type_attr.Get(&type); auto type_token = type.Get(); - EXPECT_EQ(type_token, pxr::UsdGeomTokens->cubic); + EXPECT_EQ(type_token, pxr::UsdGeomTokens->cubic) + << "Type token should be cubic for bezier curve"; pxr::VtValue wrap; pxr::UsdAttribute wrap_attr = curve.GetWrapAttr(); @@ -229,21 +235,24 @@ static void check_bezier_curve(const pxr::UsdPrim bezier_prim, auto wrap_token = wrap.Get(); if (is_periodic) { - EXPECT_EQ(wrap_token, pxr::UsdGeomTokens->periodic); + EXPECT_EQ(wrap_token, pxr::UsdGeomTokens->periodic) + << "Wrap token should be periodic for periodic curve"; } else { - EXPECT_EQ(wrap_token, pxr::UsdGeomTokens->nonperiodic); + EXPECT_EQ(wrap_token, pxr::UsdGeomTokens->nonperiodic) + << "Wrap token should be nonperiodic for nonperiodic curve"; } auto widths_interp_token = curve.GetWidthsInterpolation(); - EXPECT_EQ(widths_interp_token, pxr::UsdGeomTokens->varying); + EXPECT_EQ(widths_interp_token, pxr::UsdGeomTokens->varying) + << "Widths interpolation token should be varying for bezier curve"; pxr::UsdAttribute vert_count_attr = curve.GetCurveVertexCountsAttr(); pxr::VtArray vert_counts; vert_count_attr.Get(&vert_counts); - EXPECT_EQ(vert_counts.size(), 1); - EXPECT_EQ(vert_counts[0], vertex_count); + EXPECT_EQ(vert_counts.size(), 1) << "Prim should only contains verts for a single curve"; + EXPECT_EQ(vert_counts[0], vertex_count) << "Curve should have " << vertex_count << " verts."; } /** @@ -263,32 +272,37 @@ static void check_nurbs_curve(const pxr::UsdPrim nurbs_prim, pxr::VtArray orders; order_attr.Get(&orders); - EXPECT_EQ(orders.size(), 1); - EXPECT_EQ(orders[0], order); + EXPECT_EQ(orders.size(), 1) << "Prim should only contain orders for a single curve"; + EXPECT_EQ(orders[0], order) << "Curve should have order " << order; pxr::UsdAttribute knots_attr = curve.GetKnotsAttr(); pxr::VtArray knots; knots_attr.Get(&knots); - EXPECT_EQ(knots.size(), knots_count); + EXPECT_EQ(knots.size(), knots_count) << "Curve should have " << knots_count << " knots."; if (is_periodic) { - EXPECT_EQ(knots[0], knots[1] - (knots[knots.size() - 2] - knots[knots.size() - 3])); - EXPECT_EQ(knots[knots.size() - 1], knots[knots.size() - 2] + (knots[2] - knots[1])); + EXPECT_EQ(knots[0], knots[1] - (knots[knots.size() - 2] - knots[knots.size() - 3])) + << "NURBS curve should satisfy this knots rule for a periodic curve"; + EXPECT_EQ(knots[knots.size() - 1], knots[knots.size() - 2] + (knots[2] - knots[1])) + << "NURBS curve should satisfy this knots rule for a periodic curve"; } else { - EXPECT_EQ(knots[0], knots[1]); - EXPECT_EQ(knots[knots.size() - 1], knots[knots.size() - 2]); + EXPECT_EQ(knots[0], knots[1]) + << "NURBS curve should satisfy this knots rule for a nonperiodic curve"; + EXPECT_EQ(knots[knots.size() - 1], knots[knots.size() - 2]) + << "NURBS curve should satisfy this knots rule for a nonperiodic curve"; } auto widths_interp_token = curve.GetWidthsInterpolation(); - EXPECT_EQ(widths_interp_token, pxr::UsdGeomTokens->vertex); + EXPECT_EQ(widths_interp_token, pxr::UsdGeomTokens->vertex) + << "Widths interpolation token should be vertex for NURBS curve"; pxr::UsdAttribute vert_count_attr = curve.GetCurveVertexCountsAttr(); pxr::VtArray vert_counts; vert_count_attr.Get(&vert_counts); - EXPECT_EQ(vert_counts.size(), 1); - EXPECT_EQ(vert_counts[0], vertex_count); + EXPECT_EQ(vert_counts.size(), 1) << "Prim should only contains verts for a single curve"; + EXPECT_EQ(vert_counts[0], vertex_count) << "Curve should have " << vertex_count << " verts."; } } // namespace blender::io::usd -- 2.30.2 From 57510ffdfe65446cfbb4bcfb1e0b4932138a88cc Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Tue, 7 Mar 2023 09:34:10 +0000 Subject: [PATCH 13/19] Use custom `unique_ptr` deleter to handle early return cases --- source/blender/io/usd/intern/usd_writer_curves.cc | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index 226cb369520..3432d01ed7b 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -351,12 +351,13 @@ void USDCurvesWriter::set_writer_attributes(pxr::UsdGeomCurves &usd_curves, void USDCurvesWriter::do_write(HierarchyContext &context) { Curves *curves; - std::unique_ptr converted_curves; + std::unique_ptr> converted_curves; switch (context.object->type) { case OB_CURVES_LEGACY: { const Curve *legacy_curve = static_cast(context.object->data); - converted_curves = std::unique_ptr(bke::curve_legacy_to_curves(*legacy_curve)); + converted_curves = std::unique_ptr>( + bke::curve_legacy_to_curves(*legacy_curve), [](Curves *c) { BKE_id_free(nullptr, c); }); curves = converted_curves.get(); break; } @@ -453,10 +454,6 @@ void USDCurvesWriter::do_write(HierarchyContext &context) set_writer_attributes(usd_curves, verts, control_point_counts, widths, timecode, interpolation); assign_materials(context, usd_curves); - - if (converted_curves) { - BKE_id_free(nullptr, converted_curves.release()); - } } void USDCurvesWriter::assign_materials(const HierarchyContext &context, -- 2.30.2 From a1cf11eafbc6123cfef1deeeccc968532545a926 Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Tue, 7 Mar 2023 10:26:01 +0000 Subject: [PATCH 14/19] More clear warning message for mixed curve types or mixed cyclic curves. --- source/blender/io/usd/intern/usd_writer_curves.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index 3432d01ed7b..9b2c1686914 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -381,7 +381,7 @@ void USDCurvesWriter::do_write(HierarchyContext &context) }); if (number_of_curve_types > 1) { - WM_report(RPT_WARNING, "Cannot export mixed curve types."); + WM_report(RPT_WARNING, "Cannot export mixed curve types in the same Curve object."); return; } @@ -397,7 +397,7 @@ void USDCurvesWriter::do_write(HierarchyContext &context) } if (!all_same_cyclic_type) { - WM_report(RPT_WARNING, "Cannot export mixed cyclic and non-cyclic curves."); + WM_report(RPT_WARNING, "Cannot export mixed cyclic and non-cyclic curves in the same Curve object."); return; } -- 2.30.2 From 465ccb38420f791279aa839152a43bc42bfb26a2 Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Wed, 8 Mar 2023 14:16:43 +0000 Subject: [PATCH 15/19] USD doesn't support animating the type of curves. Due to this limitation we will only export curves if they match the curve type exported on the first frame. --- .../io/usd/intern/usd_writer_curves.cc | 22 ++++++++++++++++--- .../blender/io/usd/intern/usd_writer_curves.h | 2 +- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index 9b2c1686914..aa2e3f5d99b 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-or-later - * Copyright 2022 Blender Foundation. All rights reserved. */ + * Copyright 2023 Blender Foundation. All rights reserved. */ #include @@ -174,7 +174,6 @@ static void populate_curve_verts_for_bezier(const bke::CurvesGeometry &geometry, { const int bezier_vstep = 3; const OffsetIndices points_by_curve = geometry.points_by_curve(); - const int start_verts_count = verts.size(); for (int i_curve = 0; i_curve < geometry.curve_num; i_curve++) { @@ -182,6 +181,8 @@ static void populate_curve_verts_for_bezier(const bke::CurvesGeometry &geometry, const int start_point_index = curve_points[0]; const int last_point_index = curve_points[curve_points.size() - 1]; + const int start_verts_count = verts.size(); + for (int i_point = start_point_index; i_point < last_point_index; i_point++) { /* The order verts in the USD bezier curve representation is [control point 0, right handle @@ -397,7 +398,8 @@ void USDCurvesWriter::do_write(HierarchyContext &context) } if (!all_same_cyclic_type) { - WM_report(RPT_WARNING, "Cannot export mixed cyclic and non-cyclic curves in the same Curve object."); + WM_report(RPT_WARNING, + "Cannot export mixed cyclic and non-cyclic curves in the same Curve object."); return; } @@ -411,6 +413,20 @@ void USDCurvesWriter::do_write(HierarchyContext &context) pxr::TfToken interpolation; const int8_t curve_type = geometry.curve_types()[0]; + + if (first_frame_curve_type == -1) { + first_frame_curve_type = curve_type; + } + else if (first_frame_curve_type != curve_type) { + WM_reportf(RPT_WARNING, + "USD does not support animating curve types. The curve type changes from %i to " + "%i on frame %f", + first_frame_curve_type, + curve_type, + timecode); + return; + } + switch (curve_type) { case CURVE_TYPE_POLY: usd_curves = DefineUsdGeomBasisCurves(pxr::VtValue(), is_cyclic, false); diff --git a/source/blender/io/usd/intern/usd_writer_curves.h b/source/blender/io/usd/intern/usd_writer_curves.h index 1e9bd20cf41..3b41a01671f 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.h +++ b/source/blender/io/usd/intern/usd_writer_curves.h @@ -15,7 +15,6 @@ namespace blender::io::usd { class USDCurvesWriter : public USDAbstractWriter { public: USDCurvesWriter(const USDExporterContext &ctx); - USDCurvesWriter(const USDExporterContext &ctx, std::unique_ptr converted_legacy_curves); ~USDCurvesWriter(); protected: @@ -23,6 +22,7 @@ class USDCurvesWriter : public USDAbstractWriter { void assign_materials(const HierarchyContext &context, pxr::UsdGeomCurves usd_curve); private: + int8_t first_frame_curve_type = -1; pxr::UsdGeomCurves DefineUsdGeomBasisCurves(pxr::VtValue curve_basis, bool cyclic, bool cubic); void set_writer_attributes(pxr::UsdGeomCurves &usd_curves, -- 2.30.2 From 88b89df20759d27886342223d55866b0d90cd640 Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Mon, 20 Mar 2023 11:42:42 +0000 Subject: [PATCH 16/19] Fix comment formatting. Use canonical name for `points`. Update report warning to use enum names. --- source/blender/io/usd/CMakeLists.txt | 1 + .../io/usd/intern/usd_writer_curves.cc | 60 +++++++++++-------- .../blender/io/usd/tests/usd_curves_test.cc | 22 +++---- 3 files changed, 48 insertions(+), 35 deletions(-) diff --git a/source/blender/io/usd/CMakeLists.txt b/source/blender/io/usd/CMakeLists.txt index f5fbb844005..29ca242a3f7 100644 --- a/source/blender/io/usd/CMakeLists.txt +++ b/source/blender/io/usd/CMakeLists.txt @@ -48,6 +48,7 @@ set(INC ../../blenkernel ../../blenlib ../../blenloader + ../../blentranslation ../../bmesh ../../depsgraph ../../editors/include diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index aa2e3f5d99b..91bdf7910ff 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -19,6 +19,10 @@ #include "BKE_material.h" #include "BLI_math_geom.h" +#include "BLT_translation.h" + +#include "RNA_access.h" +#include "RNA_enum_types.h" #include "WM_api.h" #include "WM_types.h" @@ -116,21 +120,21 @@ static void populate_curve_verts(const bke::CurvesGeometry &geometry, const OffsetIndices points_by_curve = geometry.points_by_curve(); for (const int i_curve : geometry.curves_range()) { - const IndexRange curve_points = points_by_curve[i_curve]; - for (const int i_point : curve_points) { + const IndexRange points = points_by_curve[i_curve]; + for (const int i_point : points) { verts.push_back( pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); } - const int tot_points = curve_points.size(); + const int tot_points = points.size(); control_point_counts[i_curve] = tot_points; /* For periodic linear curve, segment count = curveVertexCount. - For periodic cubic curve, segment count = curveVertexCount / vstep. - For nonperiodic linear curve, segment count = curveVertexCount - 1. - For nonperiodic cubic curve, segment count = ((curveVertexCount - 4) / vstep) + 1. - This function handles linear and Catmull-Rom curves. For Catmull-Rom, vstep is 1. - https://graphics.pixar.com/usd/dev/api/class_usd_geom_basis_curves.html */ + * For periodic cubic curve, segment count = curveVertexCount / vstep. + * For nonperiodic linear curve, segment count = curveVertexCount - 1. + * For nonperiodic cubic curve, segment count = ((curveVertexCount - 4) / vstep) + 1. + * This function handles linear and Catmull-Rom curves. For Catmull-Rom, vstep is 1. + * https://graphics.pixar.com/usd/dev/api/class_usd_geom_basis_curves.html */ if (is_cyclic) { segments[i_curve] = tot_points; } @@ -175,11 +179,11 @@ static void populate_curve_verts_for_bezier(const bke::CurvesGeometry &geometry, const int bezier_vstep = 3; const OffsetIndices points_by_curve = geometry.points_by_curve(); - for (int i_curve = 0; i_curve < geometry.curve_num; i_curve++) { + for (const int i_curve : geometry.curves_range()) { - const IndexRange curve_points = points_by_curve[i_curve]; - const int start_point_index = curve_points[0]; - const int last_point_index = curve_points[curve_points.size() - 1]; + const IndexRange points = points_by_curve[i_curve]; + const int start_point_index = points[0]; + const int last_point_index = points[points.size() - 1]; const int start_verts_count = verts.size(); @@ -203,9 +207,10 @@ static void populate_curve_verts_for_bezier(const bke::CurvesGeometry &geometry, positions[last_point_index][1], positions[last_point_index][2])); - /* For USD representation of periodic bezier curve, one of the curve's points must be repeated - * to close the curve. The repeated point is the first point. Since the curve is closed, we now - * need to include the right handle of the last point and the left handle of the first point. + /* For USD representation of periodic bezier curve, one of the curve's points must be + * repeated to close the curve. The repeated point is the first point. Since the curve is + * closed, we now need to include the right handle of the last point and the left handle of + * the first point. */ if (is_cyclic) { const blender::float3 right_handle = handles_r[last_point_index]; @@ -274,13 +279,13 @@ static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, const OffsetIndices points_by_curve = geometry.points_by_curve(); for (const int i_curve : geometry.curves_range()) { - const IndexRange curve_points = points_by_curve[i_curve]; - for (const int i_point : curve_points) { + const IndexRange points = points_by_curve[i_curve]; + for (const int i_point : points) { verts.push_back( pxr::GfVec3f(positions[i_point][0], positions[i_point][1], positions[i_point][2])); } - const int tot_points = curve_points.size(); + const int tot_points = points.size(); control_point_counts[i_curve] = tot_points; const int8_t order = geom_orders[i_curve]; @@ -382,7 +387,7 @@ void USDCurvesWriter::do_write(HierarchyContext &context) }); if (number_of_curve_types > 1) { - WM_report(RPT_WARNING, "Cannot export mixed curve types in the same Curve object."); + WM_report(RPT_WARNING, "Cannot export mixed curve types in the same Curves object."); return; } @@ -399,7 +404,7 @@ void USDCurvesWriter::do_write(HierarchyContext &context) if (!all_same_cyclic_type) { WM_report(RPT_WARNING, - "Cannot export mixed cyclic and non-cyclic curves in the same Curve object."); + "Cannot export mixed cyclic and non-cyclic curves in the same Curves object."); return; } @@ -418,11 +423,18 @@ void USDCurvesWriter::do_write(HierarchyContext &context) first_frame_curve_type = curve_type; } else if (first_frame_curve_type != curve_type) { + const char *first_frame_curve_type_name = nullptr; + RNA_enum_name_from_value( + rna_enum_curves_types, (int)first_frame_curve_type, &first_frame_curve_type_name); + + const char *current_curve_type_name = nullptr; + RNA_enum_name_from_value(rna_enum_curves_types, (int)curve_type, ¤t_curve_type_name); + WM_reportf(RPT_WARNING, - "USD does not support animating curve types. The curve type changes from %i to " - "%i on frame %f", - first_frame_curve_type, - curve_type, + "USD does not support animating curve types. The curve type changes from %s to " + "%s on frame %f", + IFACE_(first_frame_curve_type_name), + IFACE_(current_curve_type_name), timecode); return; } diff --git a/source/blender/io/usd/tests/usd_curves_test.cc b/source/blender/io/usd/tests/usd_curves_test.cc index e3e2be2009b..c0d88e0238d 100644 --- a/source/blender/io/usd/tests/usd_curves_test.cc +++ b/source/blender/io/usd/tests/usd_curves_test.cc @@ -46,17 +46,17 @@ namespace blender::io::usd { const StringRefNull usd_curves_test_filename = "usd/usd_curves_test.blend"; const StringRefNull output_filename = "usd/output.usda"; -void check_catmullRom_curve(const pxr::UsdPrim prim, - const bool is_periodic, - const int vertex_count); -void check_bezier_curve(const pxr::UsdPrim bezier_prim, - const bool is_periodic, - const int vertex_count); -void check_nurbs_curve(const pxr::UsdPrim nurbs_prim, - const int vertex_count, - const int knots_count, - const int order, - const bool is_periodic); +static void check_catmullRom_curve(const pxr::UsdPrim prim, + const bool is_periodic, + const int vertex_count); +static void check_bezier_curve(const pxr::UsdPrim bezier_prim, + const bool is_periodic, + const int vertex_count); +static void check_nurbs_curve(const pxr::UsdPrim nurbs_prim, + const int vertex_count, + const int knots_count, + const int order, + const bool is_periodic); class UsdCurvesTest : public BlendfileLoadingBaseTest { protected: -- 2.30.2 From a7a1a3d3cae36787473b3608626952799dab1fbd Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Tue, 16 May 2023 10:49:24 +0100 Subject: [PATCH 17/19] Update NURBS export logic so that knots are concatenated for all batched curves. Added a test to cover batched NURBS export. --- .../io/usd/intern/usd_writer_curves.cc | 36 ++++----- .../blender/io/usd/tests/usd_curves_test.cc | 76 ++++++++++++++----- 2 files changed, 78 insertions(+), 34 deletions(-) diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index 91bdf7910ff..addfa7fff40 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -29,13 +29,9 @@ namespace blender::io::usd { -USDCurvesWriter::USDCurvesWriter(const USDExporterContext &ctx) : USDAbstractWriter(ctx) -{ -} +USDCurvesWriter::USDCurvesWriter(const USDExporterContext &ctx) : USDAbstractWriter(ctx) {} -USDCurvesWriter::~USDCurvesWriter() -{ -} +USDCurvesWriter::~USDCurvesWriter() {} pxr::UsdGeomCurves USDCurvesWriter::DefineUsdGeomBasisCurves(pxr::VtValue curve_basis, const bool is_cyclic, @@ -68,12 +64,13 @@ pxr::UsdGeomCurves USDCurvesWriter::DefineUsdGeomBasisCurves(pxr::VtValue curve_ static void populate_curve_widths(const bke::CurvesGeometry &geometry, pxr::VtArray &widths) { const bke::AttributeAccessor curve_attributes = geometry.attributes(); - const VArray radii = curve_attributes.lookup("radius", ATTR_DOMAIN_POINT); + const bke::AttributeReader radii = curve_attributes.lookup("radius", + ATTR_DOMAIN_POINT); - widths.resize(radii.size()); + widths.resize(radii.varray.size()); - for (const int i : radii.index_range()) { - widths[i] = radii[i] * 2.0f; + for (const int i : radii.varray.index_range()) { + widths[i] = radii.varray[i] * 2.0f; } } @@ -269,6 +266,8 @@ static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, pxr::TfToken &interpolation, const bool is_cyclic) { + /* Order and range, when representing a batched NurbsCurve should be authored one value per + * curve.*/ const int num_curves = geometry.curve_num; orders.resize(num_curves); @@ -297,20 +296,23 @@ static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &geometry, Array temp_knots(knots_num); bke::curves::nurbs::calculate_knots(tot_points, mode, order, is_cyclic, temp_knots); - knots.resize(knots_num); + /* Knots should be the concatentation of all batched curves. + * https://graphics.pixar.com/usd/dev/api/class_usd_geom_nurbs_curves.html#details */ for (int i_knot = 0; i_knot < knots_num; i_knot++) { - knots[i_knot] = double(temp_knots[i_knot]); + knots.push_back(double(temp_knots[i_knot])); } - /* Set end knots according to the USD spec for periodic curves + /* For USD it is required to set specific end knots for periodic/non-periodic curves * https://graphics.pixar.com/usd/dev/api/class_usd_geom_nurbs_curves.html#details */ + int zeroth_knot_index = knots.size() - knots_num; if (is_cyclic) { - knots[0] = knots[1] - (knots[knots.size() - 2] - knots[knots.size() - 3]); - knots[knots.size() - 1] = knots[knots.size() - 2] + (knots[2] - knots[1]); + knots[zeroth_knot_index] = knots[zeroth_knot_index + 1] - + (knots[knots.size() - 2] - knots[knots.size() - 3]); + knots[knots.size() - 1] = knots[knots.size() - 2] + + (knots[zeroth_knot_index + 2] - knots[zeroth_knot_index + 1]); } else { - /* Set end knots according to the USD spec for non-periodic curves */ - knots[0] = knots[1]; + knots[zeroth_knot_index] = knots[zeroth_knot_index + 1]; knots[knots.size() - 1] = knots[knots.size() - 2]; } } diff --git a/source/blender/io/usd/tests/usd_curves_test.cc b/source/blender/io/usd/tests/usd_curves_test.cc index c0d88e0238d..5a19e172c2e 100644 --- a/source/blender/io/usd/tests/usd_curves_test.cc +++ b/source/blender/io/usd/tests/usd_curves_test.cc @@ -55,8 +55,11 @@ static void check_bezier_curve(const pxr::UsdPrim bezier_prim, static void check_nurbs_curve(const pxr::UsdPrim nurbs_prim, const int vertex_count, const int knots_count, - const int order, - const bool is_periodic); + const int order); +static void check_nurbs_circle(const pxr::UsdPrim nurbs_prim, + const int vertex_count, + const int knots_count, + const int order); class UsdCurvesTest : public BlendfileLoadingBaseTest { protected: @@ -135,14 +138,14 @@ TEST_F(UsdCurvesTest, usd_export_curves) std::string prim_name = pxr::TfMakeValidIdentifier("NurbsCurve"); pxr::UsdPrim test_prim = stage->GetPrimAtPath(pxr::SdfPath("/NurbsCurve/" + prim_name)); EXPECT_TRUE(test_prim.IsValid()); - check_nurbs_curve(test_prim, 6, 10, 4, false); + check_nurbs_curve(test_prim, 6, 20, 4); } { std::string prim_name = pxr::TfMakeValidIdentifier("NurbsCircle"); pxr::UsdPrim test_prim = stage->GetPrimAtPath(pxr::SdfPath("/NurbsCircle/" + prim_name)); EXPECT_TRUE(test_prim.IsValid()); - check_nurbs_curve(test_prim, 8, 13, 3, true); + check_nurbs_circle(test_prim, 8, 13, 3); } { @@ -263,8 +266,7 @@ static void check_bezier_curve(const pxr::UsdPrim bezier_prim, static void check_nurbs_curve(const pxr::UsdPrim nurbs_prim, const int vertex_count, const int knots_count, - const int order, - const bool is_periodic) + const int order) { auto curve = pxr::UsdGeomNurbsCurves(nurbs_prim); @@ -272,22 +274,19 @@ static void check_nurbs_curve(const pxr::UsdPrim nurbs_prim, pxr::VtArray orders; order_attr.Get(&orders); - EXPECT_EQ(orders.size(), 1) << "Prim should only contain orders for a single curve"; - EXPECT_EQ(orders[0], order) << "Curve should have order " << order; + EXPECT_EQ(orders.size(), 2) << "Prim should contain orders for two curves"; + EXPECT_EQ(orders[0], order) << "Curves should have order " << order; + EXPECT_EQ(orders[1], order) << "Curves should have order " << order; pxr::UsdAttribute knots_attr = curve.GetKnotsAttr(); pxr::VtArray knots; knots_attr.Get(&knots); EXPECT_EQ(knots.size(), knots_count) << "Curve should have " << knots_count << " knots."; - if (is_periodic) { - EXPECT_EQ(knots[0], knots[1] - (knots[knots.size() - 2] - knots[knots.size() - 3])) - << "NURBS curve should satisfy this knots rule for a periodic curve"; - EXPECT_EQ(knots[knots.size() - 1], knots[knots.size() - 2] + (knots[2] - knots[1])) - << "NURBS curve should satisfy this knots rule for a periodic curve"; - } - else { - EXPECT_EQ(knots[0], knots[1]) + for (int i = 0; i < 2; i++) { + int zeroth_knot_index = i * (knots_count / 2); + + EXPECT_EQ(knots[zeroth_knot_index], knots[zeroth_knot_index + 1]) << "NURBS curve should satisfy this knots rule for a nonperiodic curve"; EXPECT_EQ(knots[knots.size() - 1], knots[knots.size() - 2]) << "NURBS curve should satisfy this knots rule for a nonperiodic curve"; @@ -301,7 +300,50 @@ static void check_nurbs_curve(const pxr::UsdPrim nurbs_prim, pxr::VtArray vert_counts; vert_count_attr.Get(&vert_counts); - EXPECT_EQ(vert_counts.size(), 1) << "Prim should only contains verts for a single curve"; + EXPECT_EQ(vert_counts.size(), 2) << "Prim should contain verts for two curves"; + EXPECT_EQ(vert_counts[0], vertex_count) << "Curve should have " << vertex_count << " verts."; + EXPECT_EQ(vert_counts[1], vertex_count) << "Curve should have " << vertex_count << " verts."; +} + +/** + * Test that the provided prim is a valid NURBS curve. We also check it matches the expected + * wrap type, and has the expected number of vertices. For NURBS, we also validate that the knots + * layout matches the expected layout for periodic/non-periodic curves according to the USD spec. + */ +static void check_nurbs_circle(const pxr::UsdPrim nurbs_prim, + const int vertex_count, + const int knots_count, + const int order) +{ + auto curve = pxr::UsdGeomNurbsCurves(nurbs_prim); + + pxr::UsdAttribute order_attr = curve.GetOrderAttr(); + pxr::VtArray orders; + order_attr.Get(&orders); + + EXPECT_EQ(orders.size(), 1) << "Prim should contain orders for one curves"; + EXPECT_EQ(orders[0], order) << "Curve should have order " << order; + + pxr::UsdAttribute knots_attr = curve.GetKnotsAttr(); + pxr::VtArray knots; + knots_attr.Get(&knots); + + EXPECT_EQ(knots.size(), knots_count) << "Curve should have " << knots_count << " knots."; + + EXPECT_EQ(knots[0], knots[1] - (knots[knots.size() - 2] - knots[knots.size() - 3])) + << "NURBS curve should satisfy this knots rule for a periodic curve"; + EXPECT_EQ(knots[knots.size() - 1], knots[knots.size() - 2] + (knots[2] - knots[1])) + << "NURBS curve should satisfy this knots rule for a periodic curve"; + + auto widths_interp_token = curve.GetWidthsInterpolation(); + EXPECT_EQ(widths_interp_token, pxr::UsdGeomTokens->vertex) + << "Widths interpolation token should be vertex for NURBS curve"; + + pxr::UsdAttribute vert_count_attr = curve.GetCurveVertexCountsAttr(); + pxr::VtArray vert_counts; + vert_count_attr.Get(&vert_counts); + + EXPECT_EQ(vert_counts.size(), 1) << "Prim should contain verts for one curve"; EXPECT_EQ(vert_counts[0], vertex_count) << "Curve should have " << vertex_count << " verts."; } -- 2.30.2 From bbbd19ba968c3a7ef1a8e40cc0a7b7ceed193285 Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Tue, 16 May 2023 10:52:37 +0100 Subject: [PATCH 18/19] Small tidy up changes --- source/blender/io/usd/intern/usd_writer_curves.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index addfa7fff40..cef3378da0c 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -83,13 +83,13 @@ static pxr::TfToken get_curve_width_interpolation(const pxr::VtArray &wid return pxr::TfToken(); } - const size_t accumulatedControlPointCount = std::accumulate( + const size_t accumulated_control_point_count = std::accumulate( control_point_counts.begin(), control_point_counts.end(), 0); /* For Blender curves, radii are always stored per point. For linear curves, this should match * with USD's vertex interpolation. For cubic curves, this should match with USD's varying * interpolation. */ - if (widths.size() == accumulatedControlPointCount) { + if (widths.size() == accumulated_control_point_count) { return pxr::UsdGeomTokens->vertex; } @@ -102,7 +102,7 @@ static pxr::TfToken get_curve_width_interpolation(const pxr::VtArray &wid return pxr::UsdGeomTokens->varying; } - WM_report(RPT_WARNING, "Curve width size not supported for USD interpolation."); + WM_report(RPT_WARNING, "Curve width size not supported for USD interpolation"); return pxr::TfToken(); } @@ -389,7 +389,7 @@ void USDCurvesWriter::do_write(HierarchyContext &context) }); if (number_of_curve_types > 1) { - WM_report(RPT_WARNING, "Cannot export mixed curve types in the same Curves object."); + WM_report(RPT_WARNING, "Cannot export mixed curve types in the same Curves object"); return; } @@ -406,7 +406,7 @@ void USDCurvesWriter::do_write(HierarchyContext &context) if (!all_same_cyclic_type) { WM_report(RPT_WARNING, - "Cannot export mixed cyclic and non-cyclic curves in the same Curves object."); + "Cannot export mixed cyclic and non-cyclic curves in the same Curves object"); return; } -- 2.30.2 From 93fa6259b2cac6aa9bfb5b25c9f0dd7e02f67a5e Mon Sep 17 00:00:00 2001 From: "DESKTOP-ON14TH5\\Sonny Campbell" Date: Tue, 16 May 2023 14:13:56 +0100 Subject: [PATCH 19/19] Update to use function-style casts as in the style guide --- source/blender/io/usd/intern/usd_writer_curves.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/blender/io/usd/intern/usd_writer_curves.cc b/source/blender/io/usd/intern/usd_writer_curves.cc index cef3378da0c..20558b66226 100644 --- a/source/blender/io/usd/intern/usd_writer_curves.cc +++ b/source/blender/io/usd/intern/usd_writer_curves.cc @@ -427,10 +427,10 @@ void USDCurvesWriter::do_write(HierarchyContext &context) else if (first_frame_curve_type != curve_type) { const char *first_frame_curve_type_name = nullptr; RNA_enum_name_from_value( - rna_enum_curves_types, (int)first_frame_curve_type, &first_frame_curve_type_name); + rna_enum_curves_types, int(first_frame_curve_type), &first_frame_curve_type_name); const char *current_curve_type_name = nullptr; - RNA_enum_name_from_value(rna_enum_curves_types, (int)curve_type, ¤t_curve_type_name); + RNA_enum_name_from_value(rna_enum_curves_types, int(curve_type), ¤t_curve_type_name); WM_reportf(RPT_WARNING, "USD does not support animating curve types. The curve type changes from %s to " -- 2.30.2