Tileset and Tile Content Loader Refactor Overview

[baothientran]

Why we need to refactor our tile and tileset content loader?

• Currently the code that loads tile and tileset json does quite many things which may not be related to each other. For example:

  • LoadTilesetDotJson struct is used to load tileset.json and layer.json formats, which are two different formats for 3D Tiles and Quantized Mesh respectively
  • LoadSubtree struct is used to load a subtree of implicit quadtree and octree format. However, at most, only one of them is used for the entire life cycle of the tileset
  • ~Tile::requestContent() does upsampling for quantized mesh, but the code seems to never be used for normal tileset, implicit quadtree, and octree~ (Current implementation provides upsampling for all tilesets)
  • TileContext stores some unrelated info for an external tileset. It stores the info of a normal tileset.json, a layer.json, a quadtree, and an octree. But at most, only one of them is used
  • ImplicitTraversalUtilities::createImplicitChildrenIfNeeded() creates the children for a tile of quantized mesh, implicit quadtree, and octree. Tile::processLoadedContent() also create a tile's children in case it's a normal external tileset.json only. They have the same responsibility, but appears in different code path

• The state machine for keeping track of the tile loading process is leaked out to the Tileset class. This should be encapsulated within the tile itself, and never exposed to the client. Otherwise, we will never know who change the state of whom, and if not careful, will leak resources

What is the goal of the refactor?

• We should ensure that the tileset loader of one type will be able to load the tile content of that type only and nothing else. That means:
  • tileset.json, implicit quadtree, and octree will deal with loading only b3dm, cmpt, gltf, glb tile and no quantized mesh. Layer.json will deal with quantized mesh tile only.
  • tileset.json will load json as an external tileset. Implicit quadtree deals with quadtree subtree. And implicit octree deals with octree subtree. Layer.json doesn't have explicit external tileset and deal with rectangle availability metadata as an external subtree
  • ~Only layer.json will deal with upsampling~ (Current implementation provides upsampling for all tilesets)
  • Tile State machine should be put into one place and not leaked out
  • We should handle other similar types of tileset in the future as well like voxel, etc without modifying existing codes

Proposed architecture change

• Diagram:

• The plan is that we will have the TilesetContentLoader abstract interface with 3 methods:

  • requestTileContent(Tile &tile) is for ... loading the content of a tile:
    • For normal tileset, it will use the tile url directly. Implicit quadtree, octree, and layer.json will use the tile template url respectively based on the tile ID.
    • When a tile is an external tileset, different type will handle it differently. The normal tileset will construct a nested tileset loader depend on if the external is implicit or not, and thus create a hierarchy of external tileset. For implicit quadtree and octree, it will deal with subtree binary only (My idea is that if an implicit tile is requested, we may want to check if the subtree of the tile is loaded, if not, we should go ahead and request it along with the tile as well. Otherwise, we won't be able to traverse further). Layer.json only deal with rectangle availability to expand the tree further; however, because this info is only available when converting the quantized mesh to gltf, we can send this info to layer.json loader via the gltf model's extension (We can piggy pack the water mask and skirt edges to be an extensions as well for consistency, instead of a gltf extra currently)
    • When a tile has a renderable content, since normal tileset, implicit quadtree, and implicit octree support the same formats (b3dm, cmpt, and gltf), they will use the TileContentLoader helper to convert those formats into a gltf. Layer.json loader will only need QuantizedMesh to gltf in its implementation since it doesn't deal with the rest.
    • There is a problem with this though. Since we separate the 4 tileset types into 4 different loaders, some of the common code paths that are shared between them will be risking to be separated as well, which is also bad due to code duplication. For example, the code that generates raster overlay texture coordinates can be done for all 4 of them in the current implementation. It maybe mitigated by separating common code into helper, but the code for setting them up for 4 different loader will still be duplicated. This will happen for the implementation of other interfaces updateTileContent(Tile &) and unloadTileContent(Tile &). Another solution that I still keep thinking about is to use PostProcessor interface which I will explain later
    • updateTileContent(Tile &tile): When a tileset loader receives the content from the worker thread, it may choose not to propagate those contents to the tile immediately, but only when needed. That is the case for our current traversal. So this method is used to lazily propagate the content to the tile as we traverse down. Each type of the tilesets above can implement this function to create the children of a tile if needed
  • unloadTileContent(Tile &tile): Currently in our implementation in the Tile::unloadContent(), there is a code where it needs to wait for the children tiles finishing upsampling before allowing the tile content unloaded. But this code should be for Layer.json loader only. So there may be some divergences for different implementations

• As mentioned in the TilesetContentLoader::requestTileContent(Tile &tile) above, there maybe duplications in different implementations when processing tile content in the worker thread and in the main thread as well, for example, drape raster overlay on an arbitrary tile. That's where PostProcessor interface comes in. The current code flow of drapping raster overlay on tile is that:

  • In the Tile::requestContent(), it tries to map overlays to tile here. This happens before tile content is requested
  • It also generates UVs for each raster overlay here after finishing converting tile content to glTF or upsampling in the worker thread
  • Then in the main thread, it begins updating MapRasterOverlayToTile each frame which performs every frame.

• So PostProcessor interface is created to replicate this workflow above:

  • void onTileContentRequest(Tile &tile) is called right before a Tile content is requested in the main thread

  • void onTileContentDoneLoading(Tile &tile) is called right after the tile is moved to Done state. It's only called once during the entire cycle of content loading

  • void onTileContentUpdate(Tile &tile) is called every frame only after tile is moved to done and after the loader already updated the tile for the current frame

  • processTileContentInWorkerThread() is called after the tileset loader finishes processing the tile content in the worker thread. I can imagine generate UV texture coordinate for Raster Overlay will happen here after upsampling or converting tile content to glTF

  • A requirement for PostProcessor is that we should never allow it to modify the Tile's State. Only Tileset Loader itself is the only one does that.

  • We also need to automate the setup of PostProcessor, so that implementation of TilesetContentLoader won't need to setup the processor itself. It also allows consistent behavior as well

Another Benefits of the Changes

• Generating Mean Sea Level ellipsoid procedurally is possible by creating a new Tileset Loader and procedurally creating meshes at runtime
• Rendering 3DTiles-like formats like i3s or CDB is possible by creating a new tileset loader like above
• More ambitiously, we can create a layers of tileset where the first layer comes from Unreal engine meshes in the disk, and the second layer comes from CWT or any photogrammetry from the ground. That may allow user to authorize a new tileset in the editor as well

That's it for now. I'm still thinking about the design as we go on, so things will change though. I also know it requires massive change in the library itself, so it may not be planned soon or thing may not work as expected

[kring]

Thanks for writing this up Bao! I need to read it in detail, but I noticed one problem in quick read:

Tile::requestContent() does upsampling for quantized mesh, but the code seems to never be used for normal tileset, implicit quadtree, and octree

Upsampling is only needed when raster overlays are attached. If you attach a raster overlay to a regular tileset.json-driven tileset, you'll see that upsampling happens for it, too. It's not specific to quantized-mesh at all.

[kring]

We should handle other similar types of tileset in the future as well like S2 implicit tree

S2 implicit trees are already supported in the current code.

[baothientran]

Oh I didn't know upsampling happens for all tileset. It makes sense since raster overlay makes tile create children with 4 UpsamplesQuadtree id

[kring]

@baothientran I think this is really sensible, and a great improvement over what we're doing now. It also doesn't sound too drastic. A lot of code will change, but mostly to move around. So as far as refactorings go, it's not terribly scary. I'm curious if you have a rough idea how long you might need to implement this. And what help from others on the team (if any) you would need. @nithinp7 I'm also interested in your take if you have a little time to review this.

requestTileContent(Tile &tile) is for ... loading the content of a tile

What about calling it loadTileContent? It may or may not request the content, e.g. if the content is procedural. Plus symmetry with unloadTileContent.

A requirement for PostProcessor is that we should never allow it to modify the Tile's State. Only Tileset Loader itself is the only one does that.

I'm curious about your thinking behind having the loader manage the state transition. It seems like the loaders should be focused on loading, i.e. turning whatever the wire format is into tiles and glTFs. It would be nice if an MSL loader didn't need to concern itself with any details other than creating an MSL glTF when asked.

So in that scenario, I guess the Tile or Tileset would manage the state transitions, and just notify the loader along the way. What do you think?

One thing that strikes me is the similarity in the PostProcessor and TileContentLoader interfaces. The fundamental difference is that Tile Content Loader creates a glTF, while a Post Processor just operates on a previously created one. But that isn't really reflected in the interface. So maybe it's worth considering if the TileContentLoader could just be the first postprocessor, rather than something different? It'd need a different name at that point, though.

[baothientran]

What about calling it loadTileContent? It may or may not request the content, e.g. if the content is procedural. Plus symmetry with unloadTileContent.

👍

I'm curious about your thinking behind having the loader manage the state transition. It seems like the loaders should be focused on loading, i.e. turning whatever the wire format is into tiles and glTFs. It would be nice if an MSL loader didn't need to concern itself with any details other than creating an MSL glTF when asked.

My initial thought was that since the loader requests the tile content, it probably knows when the tile is done as well. But as you mention, it's will be very bad to let derived loaders manage those state changes due to inconsistency in behavior and code duplications in the derived class

So in that scenario, I guess the Tile or Tileset would manage the state transitions, and just notify the loader along the way. What do you think?

That certainly works. I think Tileset should be the one to drive the transition in this case. The reason is that TilesetContentLoader uses the Tile in its interface, so the Tile itself shouldn't use the loader internally to prevent cyclic dependency. That also makes the Tile lightweight as well

void Tileset::requestTile(Tile &tile) {
    tile.setState(LoadState::Loading);
    tilesetLoader.loadTileContent(tile).thenInMainThread([&tile](auto content){
          tile.setState(LoadState::ContentLoaded);
    });
}

void Tileset::updateTile(Tile &tile) {
    if (tile.getState() == LoadState::ContentLoaded) {
         tilesetLoader.processLoadedContent(tile);
         tile.setState(LoadState::Done);
    }
    else {
         // tilesetLoader may own raster overlay and the overlay tile needs to be updated
        // per frame
         tilesetLoader.updateTileContent(tile);
    }
}

Another idea I have is to use templated methods pattern. That is we can move those codes into the TilesetContentLoader base class, and derived class is required to implement private virtual functions:

• Future<TileContent> loadTileContent(Tile &)
• void processLoadedContent(TileContent &content, Tile &tile)
• void updateTile(Tile &tile) With this, only base class is the one to derive the state transition, and derived class only takes care of loading glTF or create tile's children in case the tile itself is external. We also leverage the base class to chain multiple TilesetContentLoader as Post processors like you suggest without the derived class taking care of it.

I don't have strong preferences between them though. What do you think?

One thing that strikes me is the similarity in the PostProcessor and TileContentLoader interfaces. The fundamental difference is that Tile Content Loader creates a glTF, while a Post Processor just operates on a previously created one.

I didn't notice this, but that's true. I agree that merging them together into one single interface like you suggest is a better option, and less interface for client to remember

[nithinp7]

I think this is a good idea, the refactor is well-motivated and the proposed architecture looks promising. A few notes / questions:

• unloadTileContent can probably be implemented in the TilesetContentLoader base class. Upsampling is possible for all content types, so the unloadContent implementations should not have to diverge.
• What is the DefaultTilesetLoader, is it for loading tileset.json?
• How does TileContext fit into this diagram? Will it also be virtualized into loader-specific contexts?
• I don't know if this affects anything, but currently there is a coupling between Tile and raster tile load states, since isRenderable requires all rasters to be done loading for the tile. I wonder if that will matter when trying to pull raster overlay handling into a decoupled PostProcessor?
• Just for my understanding regarding the PostProcessor interface, what are some potential uses of the PostProcessor other than raster overlays? What motivates the abstraction there instead of the current raster overlay-specific code?

[baothientran]

What is the DefaultTilesetLoader, is it for loading tileset.json?

Yup it's for normal tileset.json

How does TileContext fit into this diagram? Will it also be virtualized into loader-specific contexts?

it will be removed as it contains the info of tileset a tile belongs to, and is used to load the tile accordinly. So it's the TilesetContentLoader all along

I don't know if this affects anything, but currently there is a coupling between Tile and raster tile load states, since isRenderable requires all rasters to be done loading for the tile. I wonder if that will matter when trying to pull raster overlay handling into a decoupled PostProcessor?

Good question, I haven't thought much about it, but will give more thought on it

Just for my understanding regarding the PostProcessor interface, what are some potential uses of the PostProcessor other than raster overlays? What motivates the abstraction there instead of the current raster overlay-specific code?

I can imagine raster overlay is not the only one that can be used to modify the tileset content. Later on, we can add more modifier into the tileset (glTF batching, meshopt, etc), and it's the goal that we/clients won't have to modify the internal code to do it. So planning it early to have designated customization point seems to be reasonable to me

[lilleyse]

@baothientran out of curiosity are you planning on supporting multiple contents in the tile loading refactor?

[baothientran]

It's probably after the refactoring since I want to make sure existing features work first

[javagl]

It seems like some of the points from the "Discussion about the traversal process" are going to be addressed here. Particularly, the statement from the initial post here...

The state machine for keeping track of the tile loading process is leaked out to the Tileset class. This should be encapsulated within the tile itself, and never exposed to the client. Otherwise, we will never know who change the state of whom, and if not careful, will leak resources

seems to be a summary of some points from the other issue:

• Get rid of global state accesses and global state changes during traversal, i.e. the this->options and the three load queues....
• Make the state clearer. When a Tile is in a certain Tile::LoadState (or a RasterOverlayTile is in a certain RasterOverlayTile::LoadState...), or a tile is in a certain TileSelectionState, it must be clear
  • How did the tile get into this state?
  • What does that state say about the properties/fields of the tile?
  • What are the conditions for a tile going into another state?
  • What is the (in the best case: single) method that changes that state?
  • Which thread is causing the state change?

The last point is a crucial one. I might have missed it in the description above - I'm not really up to date with the codebase and plans - but: Will the refactor include a documentation about the threads on which state changes do take place or may take place? "Knowing" the code base or looking at a call stack and seeing that certain functions are called via thenInSomeThread is not sufficient here. I think that it is crucial to document the thread constraints for state transitions, on the method-level as well as in higher-level documentation like diagrams, to reduce the headaches of debugging race conditions...

[baothientran]

@javagl The only thread that does the state transition is the main thread and that main thread task has to be resided within TilesetContentManager. The only class that can does the transition is the TilesetContentManager which manages all aspect of tile content loading. Other classes including Tileset cannot do that transition.

In the TilesetContentManager, the state machine of a tile begins by calling loadTileContent(). Calling updateTileContent() will transition the tile to the next state, and calling unloadTileContent() will finish the state machine of a tile. That is the only three methods that do the transition for the tile.

I think that it is crucial to document the thread constraints for state transitions, on the method-level as well as in higher-level documentation like diagrams, to reduce the headaches of debugging race conditions...

I agree. I will try to document all of it as much as possible

[baothientran]

This is now merged to main. I'm closing now