Fix T103354: Author extents on UsdGeomMesh #104676

Merged
Sybren A. Stüvel merged 6 commits from wave/blender_wave_Apple:contribs/T103354_author_extents into main 2023-02-14 12:12:05 +01:00
6 changed files with 205 additions and 0 deletions

View File

@ -5,12 +5,15 @@
#include "usd_writer_material.h"
#include <pxr/base/tf/stringUtils.h>
#include <pxr/usd/usdGeom/bboxCache.h>
#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,
wave marked this conversation as resolved

Since this situation doesn't seem to abort the export itself, I think RPT_WARNING would be more suitable here.

Since this situation doesn't seem to abort the export itself, I think `RPT_WARNING` would be more suitable here.
"USD Export: no bounds could be computed for %s",
prim.GetPrim().GetName().GetText());
return;
}
pxr::VtArray<pxr::GfVec3f> extent{(pxr::GfVec3f)bounds.GetRange().GetMin(),
(pxr::GfVec3f)bounds.GetRange().GetMax()};
prim.CreateExtentAttr().Set(extent);
}
} // namespace blender::io::usd

View File

@ -7,6 +7,7 @@
#include <pxr/usd/sdf/path.h>
#include <pxr/usd/usd/stage.h>
#include <pxr/usd/usdGeom/boundable.h>
#include <pxr/usd/usdShade/material.h>
#include <pxr/usd/usdUtils/sparseValueWriter.h>
@ -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

View File

@ -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

View File

@ -3,6 +3,7 @@
#include "usd_writer_mesh.h"
#include "usd_hierarchy_iterator.h"
#include <pxr/usd/usdGeom/bboxCache.h>
#include <pxr/usd/usdGeom/mesh.h>
#include <pxr/usd/usdGeom/primvarsAPI.h>
#include <pxr/usd/usdShade/material.h>
@ -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<pxr::GfVec3f> 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)

View File

@ -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

View File

@ -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
wave marked this conversation as resolved

I don't think this class is necessary. Comparison with {'FINISHED'} etc. is common enough in Blender unit tests that trying to abstract away from it will make it harder for people who know the Blender API to figure out what's going on.

I don't think this `class` is necessary. Comparison with `{'FINISHED'}` etc. is common enough in Blender unit tests that trying to abstract away from it will make it harder for people who know the Blender API to figure out what's going on.
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 <UDIM> tags, so
# it will flag "textures/test_grid_<UDIM>.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:
wave marked this conversation as resolved

collection is not the most descriptive name. failed_checks would be better.

`collection` is not the most descriptive name. `failed_checks` would be better.
name = rule.__class__.__name__
if name in to_skip:
wave marked this conversation as resolved

Why is this rule skipped? Might be worth a comment.

Why is this rule skipped? Might be worth a comment.
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()