Add support for Google Photorealistic 3d Tiles
As of https://github.com/iTwin/itwinjs-core/pull/7604 we support Google Maps 2D Tiles. We should look to add support for Google Photorealistic 3D Tiles.
Areas of concern we need to investigate is how to work with terrain and draped imagery / reality data / etc
@aruniverse can we break this down into a series of task to review the areas of concern and how we might address them. Given that we will want to add support for GP3DT as we move towards providing a consistent view / geospatial context across our applications.
I wanted to see if we support GP3DT dataset in iTwin.js so similar to what can be done in Cesium:
Cesium.Cesium3DTileset.fromUrl("https://tile.googleapis.com/v1/3dtiles/root.json?key=XXX");
I attached GP3DT in iTwin.js:
viewport.displayStyle.attachRealityModel( { tilesetUrl: "https://tile.googleapis.com/v1/3dtiles/root.json?key=XXX", name: "googleMap3dTiles" });
The root will load fine but it will fail to load sub-tiles.
Example:
'https://tile.googleapis.com/v1/3dtiles//v1/3dtiles/datasets/CgA/files/UlRPVEYuYnVsa21ldGFkYXRhLnBsYW5ldG9pZD1lYXJ0aCxidWxrX21ldGFkYXRhX2Vwb2NoPTk5NyxwYXRoPSxjYWNoZV92ZXJzaW9uPTYsYWxpZ25tZW50X3ZlcnNpb249Uk9DS1RSRUVfOTk3X0dPT0dMRV9EQVRVTV8yMDI1MDIwNlQxNDUzWl9nZW5lcmF0ZWRfYXRfMjAyNTA0MThUMTIwOFo.json?session=CJ2fmueU_ILr_wEQpIjvwAY
Problems:
- The API key was not propagated.
- Sub tile URL was not properly constructed. ...googleapis.com/v1/3dtiles//v1/3dtiles/datasets/CgA/files...
So on top of handling the Google curated API authentication, terrain and drape imagery... we also need to look into supporting this specific dataset in our Cesium3dTiles implementation.
cc: @calebmshafer
Problems:
1. The API key was not propagated.
Seems like similar problem that @mdastous-bentley solved for Cesium ION URLs.
2. Sub tile URL was not properly constructed. ...googleapis.com/v1/3dtiles//v1/3dtiles/datasets/CgA/files...
Seems like a URL parsing bug, can we see an example of a child tile URL supplied by the root tileset?
I just spoke with @matmarchand on a call.
See: RealityDataSourceTilesetUrlImpl.ts Major problems:
- Session IDs are not propagated from any .json beyond the root level .json. In the case of Google 3D tiles, this needs to happen; we encounter sub-jsons at lower levels with meaningful params that need to get picked up.
- Same with API keys propagating.
- The relative URIs are not correct for sub-levels.
@matmarchand please feel free to add anything I missed.
Pushed my investigation branch https://github.com/iTwin/itwinjs-core/tree/MathieuM/GP3DT
@eringram and I discussed some of the problems today on a call. Recap:
- There potentially is some broader work to be done in RealityModelTileTree.ts / RealityDataSourceTilesetUrlImpl.ts to support handling relative URLs in relation to tileset json files and where they reside.
- However, in this Google Photorealistic 3D Tiles case, it looks like we actually need to better handle a leading
/in a tile's content URI. Such a leading/should mean "prepend the site's base url without any trailing subdirectories." In the case of these Google tiles, the subdirectories are included in the content URI. I coded up a quick version of handling this properly, and it solves that problem. - We then move on to the problem of getting 400 errors (bad request). We theorize this is because nested .json files can have new session parameters --- those parameters likely need to apply to all of their children. The way the code works, it only propagates parameters from the root URL, not any subsequent URLs.
Number 2 is solved on a local branch but could use some cleanup.
Number 3 is the trickiest thing to solve. @andremig-bentley @markschlosseratbentley and @eringram are going to meet tomorrow to discuss in more detail.
@markschlosseratbentley Thanks for the update.
My understanding is that both 1) and 3) are the same in the sense that they need to be "in relation to tileset json files and where they reside." which includes their query params that could be inherited from one of their parent.
For example:
-
domain.com/root.json?key=x" |_ reference: "ref.json?session=123" ==> we need propagate 'key" from parent 1) : "domain.com/ref.json?session=123&key=x"
-
domain.com/ref.json?session=123&key=x" |_ reference: "nestedRef.gdb" => we need to propagate 'key' and 'session' from parent 2) : "domain.com/nestedRef.gdb?session=123&key=x"
-
domain.com/nestedRef.gdb?session=123&key=x
Thanks @matmarchand for the clear examples.
@markschlosseratbentley I made a quick test on my own branch where I store all query params from tile URLs for ending in "json" in RealityDataSourceTilesetUrlImpl._searchParams, which lets the Google Photorealistic 3D Tiles load correctly in the test case you provided, because all the tiles after the root receive the key and session params.
However, this is not a complete solution because you could have the following case:
"domain.com/root.json?key=x" |_ reference: "ref_1.json?session=123" ==> we need propagate 'key" from parent 1) : "domain.com/ref_1.json?session=123&key=x" |____ |_ reference: "nestedRef.gdb" => we need to propagate 'key' and 'session' from parent 2) : "domain.com/nestedRef.gdb?session=123&key=x" |_ reference: "ref_2.json?session=456" ==> we need propagate 'key" from parent 1) : "domain.com/ref_2.json?session=456&key=x" |____ |_ reference: "nestedRef.gdb" => we need to propagate 'key' and 'session' from parent 2) : "domain.com/nestedRef.gdb?session=456&key=x"
My code would either store both session values 123 and 456, or one would overwrite the other, as both will go in RealityDataSourceTilesetUrlImpl._searchParams (speculating, I haven't written a test for this). This is a first step, and I think we need to figure out how to correctly pass the params down to only tiles that need them
@eringram Thanks for the commit to a test branch - it looks promising. I went ahead and merged your changes into a working branch titled google-3d-tiles where we can all collaborate. I suggest we checkout + work on that branch going forward.
This now also includes a commit by me where I solve the baseURL issue with preceding slashes in a generic way.
@eringram @andremig-bentley @markschlosseratbentley met today about this topic.
We have a temporary solution put together that loads Google Photorealistic 3D Tiles, but there is more work to make this "real." See the end of this comment for a screenshot of NYC / central park in iTwin.js display-test-app.
Status:
- The way we fixed propagation of search params is a temporary solution that does not truly take into account the tile tree structure. We need to do this better.
- Loading the Google Photorealistic 3D tileset in iTwin.js resulted in a huge usage of memory and loading of way too much detail (inappropriate for the given zoom level). We diagnosed this and found that it is caused by the
geometricErrorof the tileset not being considered by iTwin.js. If we hack iTwin.js to consider this value, it loads much smoother and with less memory overhead. We need to make this not a hack.
@eringram will:
- [x] compare the way these load in iTwin.js vs. Cesium.js.
@andremig-bentley will collaborate with @eringram to:
- [x] write some unit tests for the query parameters to verify our current temporary solution does not work in other cases.
- [ ] look at a way to get the query parameters loading in the most conformant way rather than our current temporary solution.
@markschlosseratbentley will:
- [x] look at how to make iTwin.js correctly use the
geometricErrorspecified in the Google Photorealistic 3D Tiles. This seems like it will resolve major loading and memory issues we observed.
* Loading the Google Photorealistic 3D tileset in iTwin.js resulted in a huge usage of memory and loading of way too much detail (inappropriate for the given zoom level). We diagnosed this and found that it is caused by the `geometricError` of the tileset not being considered by iTwin.js. If we hack iTwin.js to consider this value, it loads _much_ _smoother_ and with less memory overhead. We need to make this not a hack.
I added usesGeometricError for this purpose as part of my PR to support the Google Photorealistic 3d Tiles PoC. Is that what you're using? Did you try it and find it unsatisfactory? By default it uses the same default maximumScreenSpaceError as CesiumJS, which was appropriate for the G3DTs.
I added usesGeometricError for this purpose. .... Is that what you're using? Did you try it and find it unsatisfactory?
Yes, we are using that. It solves many issues. The issue is on our end (the caller side) - the way we are setting it needs a bit more work - our enabling of that flag is hacked in place deeper in the code that it should be, and needs to be done correctly at the API level.
I just pushed some more changes to the google-3d-tiles branch.
- display-test-app UI is now handled better for G3DT (WIP). See screenshot at bottom of this comment. From DTA settings dialog, we can disable Background Map, then enable Google Photorealistic 3D Tiles. BGMap / G3DT cannot be enabled at the same time (need to disable one before enabling the other).
- I made a new
RealityDataSourceimplementation for G3DT tilesets in fileRealityDataSourceG3DTImpl.ts. This seems necessary so we can specifyusesGeometricErroron this implementation and only this implementation. Furthermore, we can fix up the handling of the G3DT tileset in this class without potentially causing regressions inRealityDataSourceTilesetUrlImpl.tswhere processing of many other tilesets happens. Perhaps once we get past the initial development stage we can converge these instances somehow...
Can see entire diff here (click then scroll down): https://github.com/iTwin/itwinjs-core/compare/master...google-3d-tiles?diff=unified&w
By the way, iTwin.js navigation tools seem to work fine with this approach. You can even zoom way out and see the globe and spin it around.
Draft PR: https://github.com/iTwin/itwinjs-core/pull/8104 fyi @pmconne, as discussed in the status call earlier today.
Here's an example visual comparison of how these look in iTwin.js and CesiumJS to make sure we're loading similarly:
CesiumJS Sandcastle with tile bounding volumes on
iTwin.js DTA with reality tiles bounding boxes on
@markschlosseratbentley this made me realize we also need to add the data attribution when G3DT are on in iTwin.js. Worth looking at Michel's Google Maps PR that includes it for the map layer
TODO
GP3DT attribution is the most complicated remaining part.
- [ ] Finish the draft PR here: https://github.com/iTwin/itwinjs-core/pull/8104
- [x] Decide whether to keep the GP3DT reality data source implementation separate or combine it with the TilesetUrl implementation to have a single implementation. See more comments below.
- [x] We need to add copyright attribution as required by Google. See more comments below.
- [ ] Test via Image Tests -- especially if we merge the GP3DT implementation into the TilesetUrl implementation.
GP3DT vs. TilesetUrl implementation
Is the separate reality data source implementation for GP3DT okay for now? We can keep the new API alpha for easy later removal. Or do we need to, right now, merge the GP3DT implementation into the TilesetUrl implementation to have one unified reality data source implementation?
GP3DT attribution
@eringram pointed out @mdastous-bentley's PR here added attribution for Google Maps in the past. Reading through the comments, it looks like a routine titled addLogoCards was utilized. According to this comment in RealityModelTileTree.ts, addLogoCards is deprecated in 5.0 and we should use addAttributions instead. Looking through the code this morning, this may be difficult. Here are details from Google about where to find the copyright attribution you need to display. From that link: "You must aggregate, sort, and display in a line, all attributions for displayed tiles; usually along the bottom of the rendering. For example, you can find the data attributions in a glTF tile by looking under asset, copyright." Doing this kind of complicated operation makes me lean toward keeping the GP3DT reality data source separate from the TilesetUrl one for now. Google 2D Maps in iTwin.js achieves this through GoogleMapsImageryProvider.ts, doing a separate viewport info request for all tiles and adding them all up. We have no imagery provider, just the reality data source tile tree parsing stuff. Do we even have information about visible tiles?
@markschlosseratbentley is working on adding code to RealityTileLoader.ts which finds and stores attribution information for each GP3DT tile.
@eringram is working on adding code to display the attribution information for each visible tile using addAttributions() in RealityModelTileTree.ts.
These two bits of work can converge into a final solution hopefully.
I am working on handling auth in a proper manner.
@eringram and I made progress yesterday with providing attribution of the GP3DT tiles.
Here's an example visual comparison of how these look in iTwin.js and CesiumJS to make sure we're loading similarly:
@eringram Nice to see that. Can we have a comparison with a more tilted view; toward the horizon? I see that the top most tiles have lower resolution in CesiumJS and not in iTwin.js.
In our draft PR, the attribution UI works the following way.
The white Google logo shows up in the lower left corner if GP3DT tiles exist in the view:
Once you click on the iTwin.js icon (where we have other attribution stuff), a dialog pops up showing a list of data attribution sources:
This seems roughly similar to the way Cesium.js is doing it in a sandcastle which enables GP3DT. @eringram has a link to that sandcastle. There, a "Data attribution" link shows up in the lower left hand corner which you can click which brings up a similar popup with a list like the one shown above.
In some Cesium.js tutorial screenshots, though, there is a list of data sources showing up in the view itself in the lower left hand corner (no clicking necessary):
Is this the way it used to work in Cesium.js, or...?
Questions for this implementation in iTwin.js:
- Do we need to make it clearer how to view the attributions? (Add a link in the lower titled "Data attributions" as can be seen in the Cesium sandcastle).
- OR, instead of a pop-up UI, do we need to display all of these attributions on-screen in the lower left corner?
@ggetz Do you have any feedback?
cc @eringram
Hi @markschlosseratbentley,
Google's attribution requirements and what we should be doing to best comply with them are described in detail in this issue. The TLDR is:
- We currently do not display all Google P3DT credits on screen, except for the Google logo itself.
- We do turn them on for screenshots or when capturing video, per our interpretation of the usage requirements. (We have a flag in CesiumJS which can toggle this behavior on and off.)
- We are considering always showing the credits on screen for Google P3DT to best help users remain in compliance with Google's terms of service.
CC @LisaBosCesium for awareness.
Re: How to view attributions in general–
- Generally, how we manage data attributions is modeled on recommendations from other geospatial runtimes, such as Esri ArcGIS.
- We manage attribution location though a flag on each credit: A credit can be shown in "the lightbox" (our name for that dialogue window), or on screen in the lower left corner.
- The "Data attributions" link toggles the lightbox. Having this be a separate link from the logos is an established practice, but not a struct requirement AFAIK. But because attributions are often determined by data loaded at runtime, they may not map to any displayed logos; In other words, there may be a runtime attribution from user data that should show up in the lightbox but comes from neither Cesium ion or another logo'd source like Google.
@ggetz Thanks for the helpful breakdown! @markschlosseratbentley I guess I missed the CesiumJS flag to show them onscreen when we were investigating this. I can look into how viable it is for us to also have a flag like that.
@matmarchand
Can we have a comparison with a more tilted view; toward the horizon? I see that the top most tiles have lower resolution in CesiumJS and not in iTwin.js.
Is this the type of view you meant? Also I just realized those first screenshots had CesiumJS in perspective projection and iTwin.js in orthographic, so that could contribute to the difference-- with both in orthographic, the tiles look even more similar.
CesiumJS perspective projection
CesiumJS orthographic projection
iTwin.js orthographic projection (tile bounds not working properly with perspective projection/camera on)
PR up for review here: https://github.com/iTwin/itwinjs-core/pull/8104