From 5040c39d1a2a638b2b366a55f5403cd7067814fa Mon Sep 17 00:00:00 2001 From: Michael B Johnson Date: Tue, 14 Feb 2023 12:11:53 +0100 Subject: [PATCH] Fix T103354: Author extents on UsdGeomMesh A properly authored USD file will have the extent attribute authored on all prims conforming to UsdGeomBoundable. This cached extent information is useful because it allows the 3D range of prims to be quickly understood without reading potentially large arrays of data. Note that because the shape of prims may change over time, extent attributes are always evaluated for a given timecode. This patch introduces support for authoring extents on meshes and volumes during export to USD. Because extents are common to multiple kinds of geometries, the main support for authoring extents has been placed in USDAbstractWriter, whose new author_extent method can operate on any prim conforming to pxr::UsdGeomBoundable. The USD library already provides us the code necessary to compute the bounds for a given prim, in pxr::UsdGeomBBoxCache::ComputeLocalBound. Note that not all prims that are imageable are boundable, such as transforms and cameras. For more details on extents, see https://graphics.pixar.com/usd/release/api/class_usd_geom_boundable.html#details. Note that when new types of geometries are introduced, such as curves in https://developer.blender.org/D16545, we will need to update the USD writer for that geometry such that it calls this->author_extent. Update on Feb 2: This patch has been updated to include a unit test to ensure authored extents are valid. This test requires new test assets that will need to be submitted via svn. The test assets are attached in the d16837_usd_test_assets.zip file. To use, unzip and merge the contents of this zip into the lib/tests/usd folder. This unit test also addresses #104269 by validating compliance of exported USD via UsdUtils.ComplianceChecker. Pull Request #104676 --- .../io/usd/intern/usd_writer_abstract.cc | 24 +++ .../io/usd/intern/usd_writer_abstract.h | 19 +++ .../blender/io/usd/intern/usd_writer_hair.cc | 2 + .../blender/io/usd/intern/usd_writer_mesh.cc | 10 ++ tests/python/CMakeLists.txt | 6 + tests/python/bl_usd_export_test.py | 144 ++++++++++++++++++ 6 files changed, 205 insertions(+) create mode 100644 tests/python/bl_usd_export_test.py diff --git a/source/blender/io/usd/intern/usd_writer_abstract.cc b/source/blender/io/usd/intern/usd_writer_abstract.cc index f4ae9d20e52..79cb521aacd 100644 --- a/source/blender/io/usd/intern/usd_writer_abstract.cc +++ b/source/blender/io/usd/intern/usd_writer_abstract.cc @@ -5,12 +5,15 @@ #include "usd_writer_material.h" #include +#include #include "BKE_customdata.h" #include "BLI_assert.h" #include "DNA_mesh_types.h" +#include "WM_api.h" + /* TfToken objects are not cheap to construct, so we do it once. */ namespace usdtokens { /* Materials */ @@ -149,4 +152,25 @@ bool USDAbstractWriter::mark_as_instance(const HierarchyContext &context, const return true; } +void USDAbstractWriter::author_extent(const pxr::UsdTimeCode timecode, pxr::UsdGeomBoundable &prim) +{ + /* Do not use any existing `extentsHint` that may be authored, instead recompute the extent when + * authoring it. */ + const bool useExtentsHint = false; + const pxr::TfTokenVector includedPurposes{pxr::UsdGeomTokens->default_}; + pxr::UsdGeomBBoxCache bboxCache(timecode, includedPurposes, useExtentsHint); + pxr::GfBBox3d bounds = bboxCache.ComputeLocalBound(prim.GetPrim()); + if (pxr::GfBBox3d() == bounds) { + /* This will occur, for example, if a mesh does not have any vertices. */ + WM_reportf(RPT_WARNING, + "USD Export: no bounds could be computed for %s", + prim.GetPrim().GetName().GetText()); + return; + } + + pxr::VtArray extent{(pxr::GfVec3f)bounds.GetRange().GetMin(), + (pxr::GfVec3f)bounds.GetRange().GetMax()}; + prim.CreateExtentAttr().Set(extent); +} + } // namespace blender::io::usd diff --git a/source/blender/io/usd/intern/usd_writer_abstract.h b/source/blender/io/usd/intern/usd_writer_abstract.h index 8adc054c2c2..e6ac6a8ba21 100644 --- a/source/blender/io/usd/intern/usd_writer_abstract.h +++ b/source/blender/io/usd/intern/usd_writer_abstract.h @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -67,6 +68,24 @@ class USDAbstractWriter : public AbstractHierarchyWriter { * Reference the original data instead of writing a copy. */ virtual bool mark_as_instance(const HierarchyContext &context, const pxr::UsdPrim &prim); + + /** + * Compute the bounds for a boundable prim, and author the result as the `extent` attribute. + * + * Although this method works for any boundable prim, it is preferred to use Blender's own + * cached bounds when possible. + * + * This method does not author the `extentsHint` attribute, which is also important to provide. + * Whereas the `extent` attribute can only be authored on prims inheriting from + * `UsdGeomBoundable`, an `extentsHint` can be provided on any prim, including scopes. This + * `extentsHint` should be authored on every prim in a hierarchy being exported. + * + * Note that this hint is only useful when importing or inspecting layers, and should not be + * taken into account when computing extents during export. + * + * TODO: also provide method for authoring extentsHint on every prim in a hierarchy. + */ + virtual void author_extent(const pxr::UsdTimeCode timecode, pxr::UsdGeomBoundable &prim); }; } // namespace blender::io::usd diff --git a/source/blender/io/usd/intern/usd_writer_hair.cc b/source/blender/io/usd/intern/usd_writer_hair.cc index 8ec1447b505..31c41909480 100644 --- a/source/blender/io/usd/intern/usd_writer_hair.cc +++ b/source/blender/io/usd/intern/usd_writer_hair.cc @@ -62,6 +62,8 @@ void USDHairWriter::do_write(HierarchyContext &context) colors.push_back(pxr::GfVec3f(cache[0]->col)); curves.CreateDisplayColorAttr(pxr::VtValue(colors)); } + + this->author_extent(timecode, curves); } bool USDHairWriter::check_is_animated(const HierarchyContext & /*context*/) const diff --git a/source/blender/io/usd/intern/usd_writer_mesh.cc b/source/blender/io/usd/intern/usd_writer_mesh.cc index 62656c902d0..8363f9f8b35 100644 --- a/source/blender/io/usd/intern/usd_writer_mesh.cc +++ b/source/blender/io/usd/intern/usd_writer_mesh.cc @@ -3,6 +3,7 @@ #include "usd_writer_mesh.h" #include "usd_hierarchy_iterator.h" +#include #include #include #include @@ -247,6 +248,15 @@ void USDGenericMeshWriter::write_mesh(HierarchyContext &context, Mesh *mesh) if (usd_export_context_.export_params.export_materials) { assign_materials(context, usd_mesh, usd_mesh_data.face_groups); } + + /* Blender grows its bounds cache to cover animated meshes, so only author once. */ + float bound_min[3]; + float bound_max[3]; + INIT_MINMAX(bound_min, bound_max); + BKE_mesh_minmax(mesh, bound_min, bound_max); + pxr::VtArray extent{pxr::GfVec3f{bound_min[0], bound_min[1], bound_min[2]}, + pxr::GfVec3f{bound_max[0], bound_max[1], bound_max[2]}}; + usd_mesh.CreateExtentAttr().Set(extent); } static void get_vertices(const Mesh *mesh, USDMeshData &usd_mesh_data) diff --git a/tests/python/CMakeLists.txt b/tests/python/CMakeLists.txt index 2dc3e560e35..20a7e2e000a 100644 --- a/tests/python/CMakeLists.txt +++ b/tests/python/CMakeLists.txt @@ -910,6 +910,12 @@ if(WITH_ALEMBIC) endif() if(WITH_USD) + add_blender_test( + usd_export_test + --python ${CMAKE_CURRENT_LIST_DIR}/bl_usd_export_test.py + -- + --testdir "${TEST_SRC_DIR}/usd" + ) add_blender_test( usd_import_test --python ${CMAKE_CURRENT_LIST_DIR}/bl_usd_import_test.py diff --git a/tests/python/bl_usd_export_test.py b/tests/python/bl_usd_export_test.py new file mode 100644 index 00000000000..80edb89a9b3 --- /dev/null +++ b/tests/python/bl_usd_export_test.py @@ -0,0 +1,144 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +import enum +import pathlib +import pprint +import sys +import tempfile +import unittest +from pxr import Usd +from pxr import UsdUtils +from pxr import UsdGeom +from pxr import Gf + +import bpy + +args = None + +class AbstractUSDTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls._tempdir = tempfile.TemporaryDirectory() + cls.testdir = args.testdir + cls.tempdir = pathlib.Path(cls._tempdir.name) + + return cls + + def setUp(self): + self.assertTrue( + self.testdir.exists(), "Test dir {0} should exist".format(self.testdir) + ) + + def tearDown(self): + self._tempdir.cleanup() + + +class USDExportTest(AbstractUSDTest): + def test_export_usdchecker(self): + """Test exporting a scene and verifying it passes the usdchecker test suite""" + bpy.ops.wm.open_mainfile( + filepath=str(self.testdir / "usd_materials_export.blend") + ) + export_path = self.tempdir / "usdchecker.usda" + res = bpy.ops.wm.usd_export( + filepath=str(export_path), + export_materials=True, + evaluation_mode="RENDER", + ) + self.assertEqual({'FINISHED'}, res, f"Unable to export to {export_path}") + + checker = UsdUtils.ComplianceChecker( + arkit=False, + skipARKitRootLayerCheck=False, + rootPackageOnly=False, + skipVariants=False, + verbose=False, + ) + checker.CheckCompliance(str(export_path)) + + failed_checks = {} + + # The ComplianceChecker does not know how to resolve tags, so + # it will flag "textures/test_grid_.png" as a missing reference. + # That reference is in fact OK, so we skip the rule for this test. + to_skip = ("MissingReferenceChecker",) + for rule in checker._rules: + name = rule.__class__.__name__ + if name in to_skip: + continue + + issues = rule.GetFailedChecks() + rule.GetWarnings() + rule.GetErrors() + if not issues: + continue + + failed_checks[name] = issues + + self.assertFalse(failed_checks, pprint.pformat(failed_checks)) + + def compareVec3d(self, first, second): + places = 5 + self.assertAlmostEqual(first[0], second[0], places) + self.assertAlmostEqual(first[1], second[1], places) + self.assertAlmostEqual(first[2], second[2], places) + + def test_export_extents(self): + """Test that exported scenes contain have a properly authored extent attribute on each boundable prim""" + bpy.ops.wm.open_mainfile(filepath=str(self.testdir / "usd_extent_test.blend")) + export_path = self.tempdir / "usd_extent_test.usda" + res = bpy.ops.wm.usd_export( + filepath=str(export_path), + export_materials=True, + evaluation_mode="RENDER", + ) + self.assertEqual({'FINISHED'}, res, f"Unable to export to {export_path}") + + # if prims are missing, the exporter must have skipped some objects + stats = UsdUtils.ComputeUsdStageStats(str(export_path)) + self.assertEqual(stats["totalPrimCount"], 15, "Unexpected number of prims") + + # validate the overall world bounds of the scene + stage = Usd.Stage.Open(str(export_path)) + scenePrim = stage.GetPrimAtPath("/scene") + bboxcache = UsdGeom.BBoxCache(Usd.TimeCode.Default(), [UsdGeom.Tokens.default_]) + bounds = bboxcache.ComputeWorldBound(scenePrim) + bound_min = bounds.GetRange().GetMin() + bound_max = bounds.GetRange().GetMax() + self.compareVec3d(bound_min, Gf.Vec3d(-5.752975881, -1, -2.798513651)) + self.compareVec3d(bound_max, Gf.Vec3d(1, 2.9515805244, 2.7985136508)) + + # validate the locally authored extents + prim = stage.GetPrimAtPath("/scene/BigCube/BigCubeMesh") + extent = UsdGeom.Boundable(prim).GetExtentAttr().Get() + self.compareVec3d(Gf.Vec3d(extent[0]), Gf.Vec3d(-1, -1, -2.7985137)) + self.compareVec3d(Gf.Vec3d(extent[1]), Gf.Vec3d(1, 1, 2.7985137)) + prim = stage.GetPrimAtPath("/scene/LittleCube/LittleCubeMesh") + extent = UsdGeom.Boundable(prim).GetExtentAttr().Get() + self.compareVec3d(Gf.Vec3d(extent[0]), Gf.Vec3d(-1, -1, -1)) + self.compareVec3d(Gf.Vec3d(extent[1]), Gf.Vec3d(1, 1, 1)) + prim = stage.GetPrimAtPath("/scene/Volume/Volume") + extent = UsdGeom.Boundable(prim).GetExtentAttr().Get() + self.compareVec3d( + Gf.Vec3d(extent[0]), Gf.Vec3d(-0.7313742, -0.68043584, -0.5801515) + ) + self.compareVec3d( + Gf.Vec3d(extent[1]), Gf.Vec3d(0.7515701, 0.5500924, 0.9027928) + ) + + +def main(): + global args + import argparse + + if "--" in sys.argv: + argv = [sys.argv[0]] + sys.argv[sys.argv.index("--") + 1:] + else: + argv = sys.argv + + parser = argparse.ArgumentParser() + parser.add_argument("--testdir", required=True, type=pathlib.Path) + args, remaining = parser.parse_known_args(argv) + + unittest.main(argv=remaining) + + +if __name__ == "__main__": + main()