From 6f9725a1daa14ff10afa5447f2190ad283f5036c Mon Sep 17 00:00:00 2001 From: Kevin Zakka Date: Tue, 26 May 2026 15:05:54 -0700 Subject: [PATCH] Make mju_getXMLDependencies read XMLs through the resource provider. The function called tinyxml2's LoadFile directly, which only works on the OS file system. Reading through mju_openResource lets it work against any registered backend (VFS, HTTP, github:, ...). --- src/xml/xml_util.cc | 60 +++++++++++++++++++++--------------- test/xml/xml_utils_test.cc | 62 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 24 deletions(-) diff --git a/src/xml/xml_util.cc b/src/xml/xml_util.cc index b224a372..e5446da3 100644 --- a/src/xml/xml_util.cc +++ b/src/xml/xml_util.cc @@ -222,34 +222,46 @@ void mjCopyError(char* dst, const char* src, int maxlen) { } void mju_getXMLDependencies(const char* filename, mjStringVec* dependencies) { - // load XML file or parse string - tinyxml2::XMLDocument doc; - doc.LoadFile(filename); - - // error checking - if (doc.Error()) { - mju_error("Problem reading XML file '%s': %s", filename, doc.ErrorStr()); + // Open through the resource provider so the function works against any + // registered backend (OS file system, VFS, HTTP, "github:", ...) rather + // than only the OS file system. + mjResource* resource = mju_openResource("", filename, nullptr, nullptr, 0); + if (resource == nullptr) { + mju_error("Could not open '%s'", filename); } - // get top-level element - tinyxml2::XMLElement *root = doc.RootElement(); - if (!root) { - mju_error("XML root element not found"); - } - std::unordered_set files = {filename}; - - std::optional model_dir = std::nullopt; - mjResource *resource = mju_openResource("", filename, nullptr, - nullptr, 0); - if (resource != nullptr) { - const char* dir; - int ndir; - mju_getResourceDir(resource, &dir, &ndir); - model_dir = FilePath(std::string(dir, ndir)); + // Read the XML bytes from the resource. + const void* buffer = nullptr; + int size = mju_readResource(resource, &buffer); + if (size < 0 || !size) { mju_closeResource(resource); + mju_error("Could not read '%s'", filename); } - // Get file references from include and model tags. - AccumulateFiles(files, root, model_dir.value()); + + // Capture the model directory while the resource is still open. + const char* dir = nullptr; + int ndir = 0; + mju_getResourceDir(resource, &dir, &ndir); + FilePath model_dir(std::string(dir, ndir)); + + // Parse from buffer and close (the parsed DOM is independent of the + // resource buffer once Parse returns). + tinyxml2::XMLDocument doc; + tinyxml2::XMLError err = + doc.Parse(static_cast(buffer), static_cast(size)); + mju_closeResource(resource); + + if (err != tinyxml2::XML_SUCCESS) { + mju_error("Problem reading XML file '%s': %s", filename, + doc.ErrorStr() ? doc.ErrorStr() : ""); + } + tinyxml2::XMLElement* root = doc.RootElement(); + if (!root) { + mju_error("XML root element not found in '%s'", filename); + } + + std::unordered_set files = {filename}; + AccumulateFiles(files, root, model_dir); *dependencies = {files.begin(), files.end()}; } diff --git a/test/xml/xml_utils_test.cc b/test/xml/xml_utils_test.cc index c820753d..5d49053c 100644 --- a/test/xml/xml_utils_test.cc +++ b/test/xml/xml_utils_test.cc @@ -13,11 +13,14 @@ // limitations under the License. +#include #include #include #include +#include #include +#include #include #include "test/fixture.h" @@ -42,5 +45,64 @@ TEST_F(MujocoTest, GetXMLDependenciesTest) { kModelPaths.end()}; EXPECT_EQ(dependency_set, expected_dependency_set); } + +// Custom resource provider that serves XML strings from an in-memory map, +// used to exercise the non-filesystem code path. +namespace memxml { +static const std::map* g_files = nullptr; + +int Open(mjResource* resource) { + return g_files && g_files->count(resource->name) ? 1 : 0; +} +int Read(mjResource* resource, const void** buffer) { + auto it = g_files->find(resource->name); + if (it == g_files->end()) return -1; + *buffer = it->second.data(); + return static_cast(it->second.size()); +} +void Close(mjResource* resource) {} +} // namespace memxml + +// Verifies mju_getXMLDependencies works against a non-filesystem resource +// provider (the case the WASM/HTTP build relies on). +TEST_F(MujocoTest, GetXMLDependenciesViaResourceProvider) { + const std::map files = { + {"memxml:/scene.xml", + "\n" + " \n" + " \n" + " \n" + " \n" + " \n" + ""}, + {"memxml:/child.xml", + "\n" + " \n" + " \n" + " \n" + ""}, + }; + memxml::g_files = &files; + + mjpResourceProvider provider = { + .prefix = "memxml", + .open = memxml::Open, + .read = memxml::Read, + .close = memxml::Close, + }; + ASSERT_GT(mjp_registerResourceProvider(&provider), 0); + + mjStringVec dependencies; + mju_getXMLDependencies("memxml:/scene.xml", &dependencies); + std::set dep_set(dependencies.begin(), dependencies.end()); + + EXPECT_THAT(dep_set, testing::UnorderedElementsAre( + "memxml:/scene.xml", + "memxml:/child.xml", + "memxml:/meshes/m.obj", + "memxml:/t.png")); + + memxml::g_files = nullptr; +} } // namespace } // namespace mujoco