CesiumGS / cdb-to-3dtiles

Convert CDB to 3D Tiles
Apache License 2.0
76 stars 28 forks source link

Add option to combine multiple tilesets #33

Closed baothientran closed 3 years ago

baothientran commented 3 years ago

Fixes #19

Add new option to combine multiple tilesets into one. For example: To combine Elevation with component selectors 1_1 and GSModels with component selectors 2_1 into one, we can do like this:

--combine=Elevation_1_1,GSModels_2_1

You can repeat that option multiple times to group different local tilesets into different global tilesets. For example: To group Elevation and GSModels into one tileset and group all GTModels in different tileset, we can do:

--combine=Elevation_1_1,GSModels_2_1 --combine=GTModels_1_1,GTModels_2_1

With or without the option, the converter still groups each dataset from different GeoCells into a different global tileset for each dataset:

For example, if CDB has two GeoCells with coordinates (N32, W118) and (N32, W119), and each GeoCell has elevation and GSModels dataset, the tiler will combine all the elevation of all GeoCell into one global tileset for Elevation. Same with GSModels. Below is the new organization for global tilesets:

Screenshot_20201123_131852

lilleyse commented 3 years ago

The 3D Tiles Validator is picking up a small error:

slilley@Lithium:~/Code/3d-tiles-validator/validator$ node bin/3d-tiles-validator.js -i ~/Downloads/San_Diego/Elevation_1_1GSModels_1_1GTModels_1_1GTModels_2_1.json 
Tileset must declare its geometricError as a top-level property.
baothientran commented 3 years ago

@lilleyse I've updated the fix in the new commit and update tests to check it as well

baothientran commented 3 years ago

@lilleyse I just realize I have an unused function combineTilesetJson(const std::vector<std::filesystem::path> &tilesetJsonPaths, const Core::BoundingRegion &regions, std::ofstream &fs);. This function was in TileFormatIO.h and TileFormatIO.cpp. This is now removed in the new commit

lilleyse commented 3 years ago

Overall, the way you've structured the tileset files makes a lot of sense. During testing I did have a few observations that I think could improve this PR.

baothientran commented 3 years ago

I've created issues for the remaining items:

lilleyse commented 3 years ago

Thanks for opening the issues @baothientran. I verified the fixes for everything that was checked off. I also ran the converter and loaded the tilesets in CesiumJS to confirm that nothing strange happened.

I briefly looked at the code, didn't have any comments.