Initial code review

[baothientran]

Overall about the code base:

• There are two main classes in the project:

  • CDB class is responsible for traversing the CDB directory structure and retrieves the correct dataset with the mesh (e.g Elevation, GSModel, GTModel, etc) for each tile.
  • Converter class which is in the CDBTo3DTiles.cpp file is used to convert CDB dataset to tileset.json. It handles gltf, b3dm, i3dm, and cmpt conversion as well (those conversion functions are implemented as separate free functions in Gltf.cpp and TileFormatIO.cpp)
  • The main conversion function is in the Converter::convert() member function of the Converter class

• Overall about each CDB dataset:

  • Tiling scheme:

    • Dataset at the negative level only has one child. The lowest level is -10 and go up to -1 level
    • Dataset at the positive level are organized as quadtree structure with the min level being 0 and the max level being 23.
    • CDBTile and CDBTileset are used to represent such structure. At negative level, each CDBTile will only has one children, and at positive level, it will have 4 children (include nullptr)
    • CDBTile has UREF and RREF. UREF is similar to the y component of the quadtree, and RREF is similar to the x component. They are indexed from the bottom left instead of the top left. In other word, (0, 0) quadtree coordinate will be at the bottom left. You can see the UREF value in the U directory and RREF value is embedded as R + number in the dataset filename.
    • This is an example of negative level tileset: This is the path of one tile in the road network dataset: CDBPath/Tiles/N32/W118/201_RoadNetwork/LC/U0/N32W118_D201_S002_T004_LC05_U0_R0.shx:
      • N32 and W118 are the coordinates of a Geo Cell. The GeoCell is located at latitude 32 and longitude -118
      • All negative level tiles will be placed in the LC directory. U0 indicates UREF of each tile is 0 (or y component = 0). RREF is embedded in the tile under the U symbol. So LC05_U0_R0 means the tile is at level -5, UREF = 0 and RREF = 0.
      • For D201_S002_T004 in the tile filename, D201 is the codename for RoadNetwork (All the dataset begin with D symbol). S002 is the component selector 1 and T004 is the component selector 4. They have a different meaning depend on the dataset in the CDB.
    • Positive level tile are very similar. Except they are placed in the L + number (e.g L00, L01, etc) directory
    • This is the diagram for the tiling schema: https://github.com/opengeospatial/OGC-ISG-Sprint-Sep-2020/blob/master/Sprint%20Report/images/Cesium-San-Diego-Tiling-Scheme.png

  • Elevation and Imagery:

    • Elevation dataset traverse code: https://github.com/CesiumGS/cdb-to-3dtiles/blob/4102f433cde99ef21d7a1832a39dcb5ccf332267/CDBTo3DTiles/src/CDB.cpp#L48
    • Elevation conversion code: https://github.com/CesiumGS/cdb-to-3dtiles/blob/4102f433cde99ef21d7a1832a39dcb5ccf332267/CDBTo3DTiles/src/CDBTo3DTiles.cpp#L530
    • Elevation are represented as CDBElevation class. This class will convert the heightmap to a uniform grid mesh. I also provide the CDBElevation::createSimplifiedMesh() to decimate the mesh before writing to b3dm
    • Elevation and imagery are converted together. Imagery is just a texture in the elevation gltf.
    • There are edge cases when converting elevation and imagery together:
      • Higher level elevation tiles do not necessarily cover the whole area that is occupied by the lower level. If we use REPLACE refinement, it will create holes when lower level is replaced by higher level tiles. To remedy this, I have to cut the uniform mesh into 4 pieces and plug the hole where it is necessary. The cutting is handled by function CDBElevation::createSubRegion(). It is wasteful but that seems to be the only option I have.
      • Elevation and imagery do not have the same LOD. In case imagery has more LOD than elevation. Elevation will have to be duplicated until there are no more imagery.
      • If Elevation has more LOD than imagery, the parent's imagery is used for the higher LOD elevation. However, the UVs of the elevation have to be reindex to be relative to the parent tile, so that imagery can cover the tile correctly.
      • The code for solving those edge cases can be found here: https://github.com/CesiumGS/cdb-to-3dtiles/blob/4102f433cde99ef21d7a1832a39dcb5ccf332267/CDBTo3DTiles/src/CDBTo3DTiles.cpp#L173

  • Vector dataset (RoadNetwork, HydrographyNetwork, RailRoadNetwork, PowerlineNetwork)

  • All vector data are stored in the ESRI ShapeFile. They are in the Tiles directory with sub directory name being the same as those above. You can view the content of each ShapeFile with command ogrinfo -al path-to-dbf-or-shapefile with ogrinfo being installed with gdal

  • Vector data has the concept of instance and class metadata. They are stored in the ShapeFile format. However, the difference is that instance metadata will have vector geometry with it (For example, point, line, and polygon) and other metadata for each instance. Class metadata will only contain metadata of a class that will be shared by instances with the same class name.

  • Example of instance metadata:

    Notice the CNAM attribute. It indicates the class name of the instance and used to connect with class metadata. LINESTRING attribute above indicates the line geometry of the vector

  • Example of class metadata:

    Notice the CNAM of the class metadata is the same as CNAM of the instance metadata. That means if instance has the same CNAM with a class metadata will share the same class metadata.

    Other attributes have different meanings in CDB, but I just simply copy them to the feature table without any additional processing

  • Component selector 2 in the tile filename is used to distinguish which file contains instance metadata. The list can be found in the enum class CDBVectorCS2 https://github.com/CesiumGS/cdb-to-3dtiles/blob/4102f433cde99ef21d7a1832a39dcb5ccf332267/CDBTo3DTiles/src/CDBAttributes.h#L12

    I only read in instance metadata file and deduce the filename for the class metadata. Also since 3DTiles doesn't have the concept of class metadata, class metadata is merged or copied to instance metadata before converting 3DTiles

  • Currently all the vector data above are represented by CDBGeometryVectors class. It stores the geometry (points, lines, and polygon) in the mesh and instances metadata in the CDBInstancesAttributes

  • CDBInstancesAttributes and CDBClassAttributes classes represents collection of class and instance metadata in a tile. CDBInstancesAttributes are similar to struct of array rather than array of struct. This organization is similar to feature table spec https://github.com/CesiumGS/3d-tiles/blob/master/specification/TileFormats/FeatureTable/README.md. CDBClassAttributes is organized as struct of array as well. However, it maps CNAM to the array of each class attributes to allow for easy query class attribute.

  • GTModel dataset

  • GTModel is instance 3D model like trees. The mesh and texture are stored in CDBPath/GTModel. The position and metadata of each instance is stored in GTFeature directory

  • GTFeature data is stored in the ShapeFile similar to vector data above. The position of the instance is stored as point geometry. The orientation of the model is stored in AO1 attribute. This is an example:

  • To find the model name, we need class metadata:

    Notice 3 fields above FACC, FSC, and MODL.

    We use each character in FACC field to identify the directory in 500_GTModelGeometry directory. For example, FACC=AL015 means the model is in 500_GTModelGeometry/A_{custom name}/L_{custom name}/015_{custom name}

    FSC and MODL are parts of model filename. So FSC = 0 and MODL = coronado_bridge means the model filename is in 500_GTModelGeometry/A_{custom name}/L_{custom name}/015_{custom name}/D500_S001_T001_AL015_000_coronado_bridge

  • All GTModels are cached in the CDBGTModelCache class. It is used to prevent from parsing OpenFLight twice.

  • Similar to vector dataset above, all instance metadata are stored in CDBInstancesAttributes. You may notice there is class CDBModelsAttributes. It is the wrapper of CDBInstancesAttributes and very similar to CDBGeometryVectors. However, unlike CDBGeometryVectors, it doesn't convert point geometry into Cartesian 3 like CDBGeometryVectors, but leave it as Cartographic as it is used to clamp the model on the terrain later

  • GSModel dataset

  • GSModel meshes are stored in GSModelGeometry. Its textures can be stored in GTModel directory or GSModelTexture directory. For the first case, no special process is needed. However, when texture is stored in GSModelTexture, the texture filename is concealed in the zip file. It is necessary to unzip it to find the texture. Fortunately, OpenSceneGraph allows us to create a custom texture finder to locate unknown texture file. The code to handle it is located here: https://github.com/CesiumGS/cdb-to-3dtiles/blob/4102f433cde99ef21d7a1832a39dcb5ccf332267/CDBTo3DTiles/src/CDBModels.cpp#L691

    It will use OSG existing finder. If the finder doesn't find it. We will look into zip file to see if the texture is there. If the texture is not found, we can return empty string to flag not found. Otherwise, read the image from zip archive

  • Similar to GTModel, position, orientation, and scales are stored in GSFeature directory. The model file is identified by FACC, FSC, and MODL attributes. For example, below is the class metadata of one tile:

    Using the 3 attributes above, we can find the model file in the zip file in the GSModelGeometry directory with the same LOD coordinate as GSFeature tile: 300_GSModelGeometry/N32W118_D300_S001_T001_L00_U0_R0_{FACC}_{FSC}_{MODL}.flt

[baothientran]

@likangning93 I think all the reviewed comments are addressed in the new commits. Please let me know if I miss anything

[lilleyse]

Some initial comments after reading the README and inspecting the San Diego output.

• [x] Will there be Windows build instructions?
• [x] Are there any system requires besides those mentioned? E.g. OpenGL for OpenFlight. Have we tried running the built executable on a completely fresh docker container to see what .so it would fail to find?
• [x] README
  • [x] Include a link to download the San Diego CDB and provide a simple CLI example
  • [x] Include statistics about tiling speed and before/after file size
  • [x] Remove mkdir Build from the build instructions. It might be unnecessary.
• [ ] CLI flags
  • [ ] --elevation-decimate-error. What units are these? Meters? CDB data is always meters right? EDIT: see below - is it possible to "un-normalize" the option so that the units are meters?
  • [ ] --elevation-threshold-indices not sure if this option should be included. --elevation-decimate-error is a more concrete option to most users.
  • [x] --elevation-LOD - rename to --elevation-lod
• [x] Should the converter delete the output directory before tiling?
• [ ] The current directory structure might be confusing since each geocell and each layer within the geocell is disconnected from other tilesets. There should be a top-level tileset.json (e.g. SanDiego/tileset.json) and a tileset.json for each geocell (e.g. Tiles/N32/W118/tileset.json. The top-level tileset.json should point to all the geocell tilesets, and each geocell tileset should point to the layer tilesets.
• [x] Are sandcastle examples still in progress?
• [ ] Samplers have a wrapR property, but this property doesn't exist for glTF samplers. Looks like something in tinygltf...
• [x] Generally 9986 (NEAREST_MIPMAP_LINEAR) produces better results than 9985 (LINEAR_MIPMAP_NEAREST) for the minFilter
• [x] Remove min/max for _BATCHID accessors to save space in the glTF
• [ ] Typically leaf tiles should have a geometric error of 0.0
• [ ] Future work: write jpeg instead of png for models that don't have an alpha channel
• [ ] Future work: CLI option to control what layers are included in the final tileset, say if someone wants to use the converter to just convert terrain.

[baothientran]

@lilleyse --elevation-decimate-error is normalized to 0..1. So 0.01 means the simplifier will maintain the target error below 1% of the mesh extent. I've update the explanation in the CLI

[lilleyse]

@baothientran can you open issues for anything not finished in https://github.com/CesiumGS/cdb-to-3dtiles/pull/14#issuecomment-724351326?

[lilleyse]

Merging into master!