CesiumGS / 3d-tiles-tools

Apache License 2.0
313 stars 47 forks source link

Fix merge path handling #143

Closed javagl closed 4 months ago

javagl commented 4 months ago

There have been some quirks in the handling of paths for the merge and mergeJson commands.

Some details are described in the conversation around a recent PR. An attempt to summarize some of the main points:

This PR changes these aforementioned tilesetSourceIdentifiers to actually be externalTilesetDirectories:

@jo-chemla Maybe you want to give this a try. I tried to cover some of the relevant parts in the specs, differentiating the cases of [merge, mergeJson] β¨― [directories, files]. But it could be worthwhile to consider adding "more tricky cases" there.


Details:

Most of the 3D Tiles Tools commands can transparently operate on 3tz or 3dtiles files, or on the file system. The latter generally allowed inputs and outputs to either be given as an explicit file name like "./example/tileset.json", or as a directory name like "./example", and then make the assumption that this directory contains a file called tileset.json. This is a leftover from https://github.com/CesiumGS/3d-tiles/issues/184 . It becomes more tricky due to the fact that in 3tz and 3dtiles files, there is a requirement for the top-level tileset JSON to be called tileset.json. And conversely, when someone gives something like ./output as the target, it is by no means clear whether this should be a JSON file without an extension, or whether the output should be ./output/tileset.json 😬 I tried to sort some of that out, using 20 lines of comments for a 5 line function ...

alouis-jpg commented 4 months ago

Thank you for this update! It will save us some time.

Stepping in on behalf of @jo-chemla here, I ran all the tests listed in his PR and everything works as expected. Great work!

jo-chemla commented 4 months ago

Thanks @alouis-jpg , indeed all 5 cases do work the same way the initial suggested PR did - mix of relative/absolute paths for input and output tileset-name-i.json listed in the table here extracted below:

inputs output Works? Children uri
absolute C:\tilesets\folderN\tilesetN.json absolute C:\tilesets\merged\tilesetMerged.json βœ… ../folderN/tilesetN.json
relative .\tilesets\folderN\tilesetN.json relative .\tilesets\merged\tilesetMerged.json βœ… ../folderN/tilesetN.json
relative .\tilesets\folderN\tilesetN.json absolute C:\tilesets\merged\tilesetMerged.json βœ… ../folderN/tilesetN.json
absolute C:\tilesets\folderN\tilesetN.json relative .\tilesets\merged\tilesetMerged.json βœ… ../folderN/tilesetN.json
absolute C:\tilesets\folderN\tilesetN.json absolute on other drive Z:\out\merged\tilesetMerged.json βœ… C:/tilesets/folderN/tilesetN.json