From 739bb8d0672e879b75a442ca2bf147ab08cb5473 Mon Sep 17 00:00:00 2001 From: Jeremy Nimmer Date: Tue, 27 Feb 2024 07:31:41 -0800 Subject: [PATCH] [parsing] Add UsdParser smoke test stub This turns up a slew of previously-unseen errors in our OpenUSD build system, which we also fix here. --- multibody/parsing/BUILD.bazel | 57 +++++++++++++- multibody/parsing/detail_usd_parser.cc | 75 +++++++++++++++++++ multibody/parsing/detail_usd_parser.h | 36 +++++++++ multibody/parsing/detail_usd_parser_off.cc | 37 +++++++++ .../parsing/test/detail_usd_parser_test.cc | 60 +++++++++++++++ tools/workspace/openusd_internal/BUILD.bazel | 30 -------- tools/workspace/openusd_internal/defs.bzl | 13 +++- .../openusd_internal/package.BUILD.bazel | 12 +-- .../openusd_internal/patches/no_gnu_ext.patch | 20 +++++ .../workspace/openusd_internal/repository.bzl | 1 + 10 files changed, 299 insertions(+), 42 deletions(-) create mode 100644 multibody/parsing/detail_usd_parser.cc create mode 100644 multibody/parsing/detail_usd_parser.h create mode 100644 multibody/parsing/detail_usd_parser_off.cc create mode 100644 multibody/parsing/test/detail_usd_parser_test.cc create mode 100644 tools/workspace/openusd_internal/patches/no_gnu_ext.patch diff --git a/multibody/parsing/BUILD.bazel b/multibody/parsing/BUILD.bazel index 0532077a304c..f97a8d8d9f25 100644 --- a/multibody/parsing/BUILD.bazel +++ b/multibody/parsing/BUILD.bazel @@ -229,6 +229,40 @@ drake_cc_library( ], ) +drake_cc_library( + name = "detail_usd_parser", + srcs = select({ + "//tools:with_usd": [ + "detail_usd_parser.cc", + ], + "//conditions:default": [ + "detail_usd_parser_off.cc", + ], + }), + hdrs = ["detail_usd_parser.h"], + copts = [ + # TODO(jwnimmer-tri) OpenUSD has a gargantuan number of warnings. + # We should really try to patch it to fix most/all of them. + "-w", + ], + internal = True, + visibility = ["//visibility:private"], + interface_deps = [ + ":detail_misc", + ":detail_parsing_workspace", + "//multibody/plant", + ], + deps = [ + "//common:find_runfiles", + "//common:unused", + ] + select({ + "//tools:with_usd": [ + "@openusd_internal//:openusd", + ], + "//conditions:default": [], + }), +) + drake_cc_library( name = "detail_mujoco_parser", srcs = ["detail_mujoco_parser.cc"], @@ -708,6 +742,21 @@ drake_cc_googletest( ], ) +drake_cc_googletest( + name = "detail_usd_parser_test", + args = select({ + "//tools:with_usd": [], + "//conditions:default": [ + "--gtest_filter=-*", + ], + }), + deps = [ + ":detail_usd_parser", + "//common/test_utilities:diagnostic_policy_test_base", + "//common/test_utilities:expect_throws_message", + ], +) + install_files( name = "install", dest = "share/drake/multibody/parsing", @@ -718,4 +767,10 @@ install_files( visibility = ["//visibility:public"], ) -add_lint_tests(enable_clang_format_lint = False) +add_lint_tests( + cpplint_extra_srcs = [ + "detail_usd_parser.cc", + "detail_usd_parser_off.cc", + ], + enable_clang_format_lint = False, +) diff --git a/multibody/parsing/detail_usd_parser.cc b/multibody/parsing/detail_usd_parser.cc new file mode 100644 index 000000000000..49d6078eb065 --- /dev/null +++ b/multibody/parsing/detail_usd_parser.cc @@ -0,0 +1,75 @@ +#include "drake/multibody/parsing/detail_usd_parser.h" + +#include +#include +#include +#include + +#include "pxr/base/plug/registry.h" +#include "pxr/usd/usd/stage.h" + +#include "drake/common/find_runfiles.h" +#include "drake/common/unused.h" + +namespace drake { +namespace multibody { +namespace internal { + +namespace fs = std::filesystem; +namespace pxr = drake_vendor_pxr; + +namespace { +void InitializeOpenUsdLibrary() { + // Register all relevant plugins. + auto& registry = pxr::PlugRegistry::PlugRegistry::GetInstance(); + std::vector json_paths{{ + "openusd_internal/pxr/usd/ar/plugInfo.json", + "openusd_internal/pxr/usd/ndr/plugInfo.json", + "openusd_internal/pxr/usd/sdf/plugInfo.json", + "openusd_internal/pxr/usd/usdGeom/plugInfo.json", + "openusd_internal/pxr/usd/usd/plugInfo.json", + "openusd_internal/pxr/usd/usdShade/plugInfo.json", + }}; + for (const auto& json_path : json_paths) { + const RlocationOrError location = FindRunfile(json_path); + if (!location.error.empty()) { + throw std::runtime_error(location.error); + } + const fs::path info_dir = fs::path(location.abspath).parent_path(); + registry.RegisterPlugins(info_dir.string()); + } +} +} // namespace + +UsdParser::UsdParser() { + static const int ignored = []() { + InitializeOpenUsdLibrary(); + return 0; + }(); + unused(ignored); +} + +UsdParser::~UsdParser() = default; + +std::optional UsdParser::AddModel( + const DataSource& data_source, const std::string& model_name, + const std::optional& parent_model_name, + const ParsingWorkspace& workspace) { + unused(data_source, model_name, parent_model_name, workspace); + throw std::runtime_error("UsdParser::AddModel is not implemented"); +} + +std::vector UsdParser::AddAllModels( + const DataSource& data_source, + const std::optional& parent_model_name, + const ParsingWorkspace& workspace) { + // Create a stage. This is just a simple placeholder at the moment. + pxr::UsdStageRefPtr stage = pxr::UsdStage::CreateInMemory(); + + unused(data_source, parent_model_name, workspace); + throw std::runtime_error("UsdParser::AddAllModels is not implemented"); +} + +} // namespace internal +} // namespace multibody +} // namespace drake diff --git a/multibody/parsing/detail_usd_parser.h b/multibody/parsing/detail_usd_parser.h new file mode 100644 index 000000000000..6cf98438ebe0 --- /dev/null +++ b/multibody/parsing/detail_usd_parser.h @@ -0,0 +1,36 @@ +#pragma once + +#include +#include +#include + +#include "drake/multibody/parsing/detail_common.h" +#include "drake/multibody/parsing/detail_parsing_workspace.h" +#include "drake/multibody/tree/multibody_tree_indexes.h" + +namespace drake { +namespace multibody { +namespace internal { + +class UsdParser final : public ParserInterface { + public: + DRAKE_NO_COPY_NO_MOVE_NO_ASSIGN(UsdParser) + + UsdParser(); + + ~UsdParser() final; + + std::optional AddModel( + const DataSource& data_source, const std::string& model_name, + const std::optional& parent_model_name, + const ParsingWorkspace& workspace) final; + + std::vector AddAllModels( + const DataSource& data_source, + const std::optional& parent_model_name, + const ParsingWorkspace& workspace) final; +}; + +} // namespace internal +} // namespace multibody +} // namespace drake diff --git a/multibody/parsing/detail_usd_parser_off.cc b/multibody/parsing/detail_usd_parser_off.cc new file mode 100644 index 000000000000..c58953c4f7ed --- /dev/null +++ b/multibody/parsing/detail_usd_parser_off.cc @@ -0,0 +1,37 @@ +/* clang-format off to disable clang-format-includes */ +#include "drake/multibody/parsing/detail_usd_parser.h" +/* clang-format on */ + +#include + +#include "drake/common/unused.h" + +namespace drake { +namespace multibody { +namespace internal { + +UsdParser::UsdParser() = default; + +UsdParser::~UsdParser() = default; + +std::optional UsdParser::AddModel( + const DataSource& data_source, const std::string& model_name, + const std::optional& parent_model_name, + const ParsingWorkspace& workspace) { + unused(data_source, model_name, parent_model_name, workspace); + throw std::runtime_error( + "UsdParser is not available because WITH_USD is not ON"); +} + +std::vector UsdParser::AddAllModels( + const DataSource& data_source, + const std::optional& parent_model_name, + const ParsingWorkspace& workspace) { + unused(data_source, parent_model_name, workspace); + throw std::runtime_error( + "UsdParser is not available because WITH_USD is not ON"); +} + +} // namespace internal +} // namespace multibody +} // namespace drake diff --git a/multibody/parsing/test/detail_usd_parser_test.cc b/multibody/parsing/test/detail_usd_parser_test.cc new file mode 100644 index 000000000000..af3b3b564b0b --- /dev/null +++ b/multibody/parsing/test/detail_usd_parser_test.cc @@ -0,0 +1,60 @@ +#include "drake/multibody/parsing/detail_usd_parser.h" + +#include + +#include "drake/common/test_utilities/diagnostic_policy_test_base.h" +#include "drake/common/test_utilities/expect_throws_message.h" + +namespace drake { +namespace multibody { +namespace internal { +namespace { + +namespace fs = std::filesystem; + +using drake::internal::DiagnosticPolicy; +using geometry::SceneGraph; + +class UsdParserTest : public test::DiagnosticPolicyTestBase { + public: + UsdParserTest() { plant_.RegisterAsSourceForSceneGraph(&scene_graph_); } + + std::vector ParseFile(const fs::path& filename) { + const std::string source_filename = filename.string(); + const DataSource source{DataSource::kFilename, &source_filename}; + const std::optional parent_model_name; + internal::CollisionFilterGroupResolver resolver{&plant_}; + ParsingWorkspace w{options_, package_map_, diagnostic_policy_, + &plant_, &resolver, NoSelect}; + UsdParser dut; + auto result = dut.AddAllModels(source, parent_model_name, w); + resolver.Resolve(diagnostic_policy_); + return result; + } + + // USD cannot delegate to any other parsers. + static ParserInterface& NoSelect(const drake::internal::DiagnosticPolicy&, + const std::string&) { + DRAKE_UNREACHABLE(); + } + + protected: + ParsingOptions options_; + PackageMap package_map_; + MultibodyPlant plant_{0.01}; + SceneGraph scene_graph_; +}; + +// TODO(jwnimmer-tri) This is a very basic sanity test, just to get the ball +// rolling. It spews lots of error messages that probably indicate deeper +// problems. But for now, it passes! +TEST_F(UsdParserTest, Stub) { + const fs::path filename{"no_such_file.usda"}; + DRAKE_EXPECT_THROWS_MESSAGE(ParseFile(filename), + ".*UsdParser.*AddAllModels.*not implemented.*"); +} + +} // namespace +} // namespace internal +} // namespace multibody +} // namespace drake diff --git a/tools/workspace/openusd_internal/BUILD.bazel b/tools/workspace/openusd_internal/BUILD.bazel index acd7d29e1d70..67914ea7e0a0 100644 --- a/tools/workspace/openusd_internal/BUILD.bazel +++ b/tools/workspace/openusd_internal/BUILD.bazel @@ -1,33 +1,3 @@ -load("//tools/workspace:generate_file.bzl", "generate_file") load("//tools/lint:lint.bzl", "add_lint_tests") -# TODO(jwnimmer-tri) To prove that the @openusd_internal build system is -# working properly, here we perform a smoke test of `usdcat --help`. Once -# we have enough real unit tests in Drake that call into USD and prove out -# the build, we should remove this simple smoke test. - -alias( - name = "usdcat", - actual = "@openusd_internal//:usdcat", - tags = [ - # Only compile this binary when the usdcat_help test needs it. - "manual", - ], -) - -generate_file( - name = "empty.sh", - # When WITH_USD is off, this serves as a test stub dummy. - content = "echo 'WITH_USD is not ON'", -) - -sh_test( - name = "usdcat_help", - srcs = select({ - "//tools:with_usd": [":usdcat"], - "//conditions:default": ["empty.sh"], - }), - args = ["--help"], -) - add_lint_tests() diff --git a/tools/workspace/openusd_internal/defs.bzl b/tools/workspace/openusd_internal/defs.bzl index f46183a9bd13..5c711d712494 100644 --- a/tools/workspace/openusd_internal/defs.bzl +++ b/tools/workspace/openusd_internal/defs.bzl @@ -12,7 +12,7 @@ def pxr_library( Args: name: Matches the upstream name (the first argument in CMake). - subdir: The subdirectory under `OpenUSD/pxr` (e.g. "base/arch"). + subdir: The subdirectory under `OpenUSD` (e.g. "pxr/base/arch"). """ attrs = FILES[subdir] srcs = [ @@ -56,11 +56,22 @@ def pxr_library( "@boost_internal//:vmd", "@onetbb_internal//:tbb", ] + + # TODO(jwnimmer-tri) The plugInfo files will need to be pseudo-installed. + data = native.glob([subdir + "/plugInfo.json"], allow_empty = True) + + # OpenUSD uses `__attribute__((constructor))` in anger, so we must mark + # all of its code as "alwayslink" (aka "whole archive"). + alwayslink = True + cc_library( name = name, srcs = srcs, hdrs = hdrs, defines = defines, copts = ["-w"], + alwayslink = alwayslink, + linkstatic = True, + data = data, deps = deps, ) diff --git a/tools/workspace/openusd_internal/package.BUILD.bazel b/tools/workspace/openusd_internal/package.BUILD.bazel index bb4fe93a8b14..527a647b7fe9 100644 --- a/tools/workspace/openusd_internal/package.BUILD.bazel +++ b/tools/workspace/openusd_internal/package.BUILD.bazel @@ -73,15 +73,7 @@ cc_library( ":vt", ":work", ], + alwayslink = True, + linkstatic = True, visibility = ["//visibility:public"], ) - -cc_binary( - name = "usdcat", - srcs = ["pxr/usd/bin/usdcat/usdcat.cpp"], - copts = ["-w"], - deps = [":openusd"], - visibility = [ - "@drake//tools/workspace/openusd_internal:__pkg__", - ], -) diff --git a/tools/workspace/openusd_internal/patches/no_gnu_ext.patch b/tools/workspace/openusd_internal/patches/no_gnu_ext.patch new file mode 100644 index 000000000000..52f318b8464f --- /dev/null +++ b/tools/workspace/openusd_internal/patches/no_gnu_ext.patch @@ -0,0 +1,20 @@ +[openusd_internal] Opt-out of GNU libstdc++ extensions + +They are deprecated old smelly stuff that spews compiler warnings. +This opt-out is a Drake choice, do we don't plan to upstream this patch. + +--- pxr/base/arch/defines.h ++++ pxr/base/arch/defines.h +@@ -88,10 +88,8 @@ + // Features + // + +-// Only use the GNU STL extensions on Linux when using gcc. +-#if defined(ARCH_OS_LINUX) && defined(ARCH_COMPILER_GCC) +-#define ARCH_HAS_GNU_STL_EXTENSIONS +-#endif ++// N.B. Drake never uses the GNU STL extensions. ++// #define ARCH_HAS_GNU_STL_EXTENSIONS + + // The current version of Apple clang does not support the thread_local + // keyword. diff --git a/tools/workspace/openusd_internal/repository.bzl b/tools/workspace/openusd_internal/repository.bzl index 89d63e95b31b..7522db04e37c 100644 --- a/tools/workspace/openusd_internal/repository.bzl +++ b/tools/workspace/openusd_internal/repository.bzl @@ -10,6 +10,7 @@ def openusd_internal_repository( sha256 = "2add389b121568f3dfb9b7e4f4551a6c8445ae353f1725a0753c8ce5639c4f83", # noqa build_file = ":package.BUILD.bazel", patches = [ + ":patches/no_gnu_ext.patch", ":patches/onetbb.patch", ], mirrors = mirrors,