itowns
itowns copied to clipboard
Feat/google3d tiles
Description
- added C3DTilesGoogleSource type to C3DTilesSource.
- C3DTilesGoogleSource requires non-standard parsing metadata to provides valid urls => identified this code clearly
- Refactored b3dmParser to extract pure gltfparser logic
- added url parameters to Fetcher methods to allow passing dynamic parameters to url request, (required by C3DTilesGoogleSource authentification logic. https://developers.google.com/maps/documentation/tile/create-renderer?hl=en)
Motivation and Context
https://github.com/iTowns/itowns/issues/2099
I just pushed some modifications that should answer all of your comments:
-
GLTFParser
is now only a wrapper forgltfLoader
andlegacyGltfLoader
and everything 3D tiles related as been moved toB3DMParser
- Only export
GLTFParser
and notgltfLoader
andlegacyGltfLoader
anymore, which is a breaking change but avoids exporting two things having the same purpose. WDYT ? Should we keep it like that or also exportgltfLoader
andlegacyGltfLoader
to avoid a breaking change until the next major version? - Some 3D Tiles related parts could be refactored / simplified (especially the
B3dmParser
) but I didn't go into much trouble here since we are seriously considering to handle 3D tiles parsing with an external lib very soon. - I wanted to add more tests on the
GLTFParser
but I decided to wait until #2183 is merged since I needed to load local files.
@Desplandis let me know what you think :)
I went back on a previous change: I keep exporting gltfLoader
and legacyGLTFLoader
for now, for several reasons:
-
GLTFParser
only has aparse
method and not the other methods ofgltfLoader
andlegacyGLTFLoader
(e.g.load
,loadAsync
, etc.) that itowns users might use (including me :grin: ) - don't introduce an unnecessary breaking change now
- this is a bit out of scope of this PR
Therefore I think it's ready to merge @Desplandis
Note that I started a branch to migrate to 3d-tiles-renderer-js in which I started implementing an "itowns GltfLoader" that inherits from THREE.Loader
, shares the same interface than GLTFLoader but can load gltf 1.0 and 2.0 files. It makes more sense to do it in the 3d-tiles-renderer-js migration since we really need it in this context.
@jailln Since #2183 was merged before this PR, you have a conflict in the GLTFLoader
unit test but it should be easy to resolve!