Support 3D Tile I3dm legacy tile format

[timoore]

Initial commit for discussion. Some implementation remains, such as implementation of the ENU rotation option.

I3dm strains the GltfConverters interface because it can require a "sub read" of external content: the instanced glb file can be specified by a URI. I created a "ConverterSubprocessor" subclass (pretty bad name) to pass, the async system asset Accessor, base URI, etc. GltfConverters::convert returns a complete model, not a future, so I do a wait() on the accessor future that returns the .glb file. I don't know if that's bad form or if there's a better alternative.

This addresses #458.

[csciguy8]

Haven't dug in real deep yet, but noticed there are no new unit tests .

Might be nice to add a few, especially if you know of typical use cases, or cases that should fail for some reason.

[kring]

GltfConverters::convert returns a complete model, not a future, so I do a wait() on the accessor future that returns the .glb file. I don't know if that's bad form or if there's a better alternative.

Yeah adding a wait is pretty bad. In Unreal, for example, that'll block up Unreal's task graph on a network request, which in extreme cases could completely pause rendering.

Based on a quick look, I don't think it will be too big a change to make GltfConverters::convert return a Future. If it is, though, another possibility is to convert the i3dm to glTF synchronously and include references to the external resources in that glTF (using a private extension if needed). Then do the async portion later. If you happen to be able to hook into the existing GltfReader::resolveExternalData, the extra async step will just happen automatically. If not, you'll need to add another processing step near where resolveExternalData is located in order to do this extra async work.

[csciguy8]

GltfConverters::convert returns a complete model, not a future, so I do a wait() on the accessor future that returns the .glb file. I don't know if that's bad form or if there's a better alternative.

Agree with @kring , hidden waits aren't great when trying to improve performance.

Will also chime in that hidden IAssetAccessor::gets can become problematic too, especially when trying to manage simultaneous connections to a server, or decouple their execution from other futures that are just CPU processing work. This is the bulk of the work in this Staged Loading PR, and it's not a trivial refactor.

[timoore]

GltfConverters::convert returns a complete model, not a future, so I do a wait() on the accessor future that returns the .glb file. I don't know if that's bad form or if there's a better alternative.

Agree with @kring , hidden waits aren't great when trying to improve performance.

I'm rewriting the converter code to return a future.

Will also chime in that hidden IAssetAccessor::gets can become problematic too, especially when trying to manage simultaneous connections to a server, or decouple their execution from other futures that are just CPU processing work. This is the bulk of the work in this Staged Loading PR, and it's not a trivial refactor.

What makes an IAssetAccessor::get call hidden? Or, put another way, what's the alternative when another get() needs to be made in the while processing the content of a previous read? I guess I should look at #779 to see what you're doing there.

[csciguy8]

What makes an IAssetAccessor::get call hidden? Or, put another way, what's the alternative when another get() needs to be made in the while processing the content of a previous read? I guess I should look at #779 to see what you're doing there.

That's a fair question. In #779, a good example of the approach is here in ImplicitOctreeLoader

• The caller would typically call ::loadTileContent, which returns a future, and somewhere in the chain is a IAssetAccessor::get
• This PR reworks the approach to have the caller use ::getLoadWork, which returns the needed request and the CPU processing work separately
• This pain is in the rework of the code in the .cpp, and not break anything

Admittedly, this is "a" solution to separating asset request work from other work. This PR hasn't been reviewed or merged, so there may be a better solution still.

[j9liu]

@timoore I did a first pass through everything, and so far it's looking good! I definitely want to look more closely at the tile loading changes / actual matrix math, when I have some more uninterrupted time.

I tried to cut down on bikeshedding, but most of my comments ended up being about clarity. 😅 It can get confusing when both I3DM instances and ext_mesh_gpu_instancing are operating in the same file. So I apologize in advance for the quantity and nitpickiness, feel free to push back if something is off

[j9liu]

@timoore haven't forgotten about this PR, I'll plan to take another look tomorrow 🙏

[j9liu]

@timoore Thanks for addressing my previous feedback, and for your patience with the second review.

I haven't verified the math yet in Unreal, but compared with the notes in #458 , the computation looks good. And I assume that Kevin already did some testing. 😄

[j9liu]

Thanks for addressing all the feedback @timoore ! Sorry I didn't see this earlier, you're welcome to @ me after you push changes to get my attention 😄

Merging now!