Fix inconsistent asset paths in mj_saveLastXML: canonicalize includes…

[Adityakk9031]

Fixes: #2941

Problem

Users observed inconsistent behavior of mj_saveLastXML with respect to asset file paths:

• loading an included model directly produced relative paths in saved XML,
• loading a top-level scene via a relative path resulted in doubled prefixes like xmls/xmls/... and broken loading,
• loading the top-level scene via an absolute path produced absolute paths in the saved XML.

Root cause

The problem was caused by two interacting issues:

1. The parser (IncludeXML in xml.cpp) mixed different filename forms when checking and recording included files: it checked the included set against the raw filename but then inserted a dir-prefixed filename (i.e. dir + filename) into included. This inconsistent canonicalization caused the mjSpec to contain a mix of relative and dir-prefixed paths, which the writer later serialized incorrectly.
2. The writer emits asset filenames using spec->modelfiledir as the base. When saving to a new target file, the writer had no information about the output directory and thus could not deterministically produce paths relative to the output file — causing inconsistent absolute/relative outputs.

What this PR does

1. Parser canonicalization (xml.cc):

   • Canonicalize include filenames early (fullname = dir + filename) and use fullname consistently for included checks and insertions and for subsequent processing.
   • This prevents having mixed forms in mjSpec and avoids doubled prefixes when the writer later emits file paths.

2. Output-dir awareness at write time (xml_api.cc):

   • Temporarily override spec->modelfiledir with the directory of the output file when calling the writer via GetGlobalModel().ToXML(...). This is implemented in a backward-compatible way (temporary override + restore).
   • This lets the existing writer produce asset paths relative to the output file location, removing ambiguity and making saved XML deterministic for the common mj_saveLastXML case.

Why this fixes #2941

• Parser canonicalization eliminates mixed path forms inside the mjSpec, preventing double-prefix issues like xmls/xmls/....
• Temporarily setting spec->modelfiledir to the output directory ensures the writer computes relative/absolute paths consistently w.r.t. the output location, so the saved XML can be loaded reliably.

Testing

Run the three cases reported in #2941 and verify saved XML loads in each case:

1. Load the child XML directly:
   • model = mujoco.MjModel.from_xml_path("xmls/panda_2f85/panda_2f85.xml")
   • mujoco.mj_saveLastXML("test.xml", model)
   • mj_loadXML("test.xml") should succeed.
2. Load scene.xml via relative path:
   • model = mujoco.MjModel.from_xml_path("xmls/scene.xml")
   • mujoco.mj_saveLastXML("test.xml", model)
   • mj_loadXML("test.xml") should succeed (no xmls/xmls/... errors).
3. Load scene.xml via absolute path:
   • model = mujoco.MjModel.from_xml_path(os.path.abspath("xmls/scene.xml"))
   • mujoco.mj_saveLastXML("test.xml", model)
   • mj_loadXML("test.xml") should succeed.

Additional tests:

• VFS/custom resource provider cases.
• mj_saveXML and mj_saveXMLString callers (consider parity later).

Files changed

• src/xml/xml.cc (parser include canonicalization)
• src/xml/xml_api.cc (temporary modelfiledir override for writer calls, helper DirnameOfPath)

[Adityakk9031]

@yuvaltassa please check this pr

[havess]

Hi! Thanks for taking a look at this, left a few comments. I'd like to see some unit tests that validate the behaviour if you don't mind.

[Adityakk9031]

i will fix it asap

[younghyopark]

Hi @Adityakk9031, thanks so much for opening this PR addressing the issue I posted.

Just wanted to check whether you’re still planning to finish it up. If you’re busy, I’d be happy to help or take over the remaining work to get it across the finish line -- whatever works best for you!

[Adityakk9031]

i wiil fix it today

[Adityakk9031]

@havess Check now

[havess]

Once again I don't believe this will work by changing the spec modelfiledir this way. I could be wrong, but please include a unit test as requested in my first review.

[Adityakk9031]

ok