Skip to content

Support Google Photorealistic 3D Tiles in iTwin.js #8104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 129 commits into
base: master
Choose a base branch
from

Conversation

markschlosseratbentley
Copy link
Contributor

@markschlosseratbentley markschlosseratbentley commented May 15, 2025

Related to #8036.

iTwin.js can already attach tilesets from a variety of sources. Google Photorealistic 3d Tiles are in the common 3D Tiles format so in theory should work in iTwin.js. However, a few hurdles were found which this PR works around, and provides a public API to make them easier to attach.

Google3dTilesProvider

This PR adds a Google3dTilesProvider class that implements the RealityDataSourceProvider interface. You can register the provider it with IModelApp.realityDataSourceProviders.register(), and then attach a reality model via DisplayStyleState.attachRealityModel() with your Google3dTilesProvider name in the rdSourceKey. The following example shows this:

// Specify an API key or getAuthToken function in the provider options
const provider = new Google3dTilesProvider({ apiKey: process.env.IMJS_GOOGLE_3D_TILES_KEY! });
// The provider must be initialized before attaching, to load imagery for its decorator
await provider.initialize();

// This function just provides the Google 3D Tiles URL, or you can get it another way via a service, etc.
const url = getGoogle3dTilesURL();

IModelApp.realityDataSourceProviders.register("GP3DT", provider);
view.displayStyle.attachRealityModel({
  tilesetUrl: url,
  name: "googleMap3dTiles",
  rdSourceKey: {
    // This must be the same name you registered your provider under
    provider: "GP3DT",
    format: "ThreeDTile",
    id: url,
  },
});

RealityDataSourceGoogle3dTilesImpl

It also adds a new RealityDataSourceGoogle3dTilesImpl implementation of RealityDataSource which is used by the Google3dTilesProvider to request and process Google Photorealistic 3D Tiles. This RealityDataSourceGoogle3dTilesImpl implementation does the following:

  • implements usesGeometricError as true which tells iTwin.js to use the geometric errors found inside the Google 3D tileset, which is crucial for good performance.
  • sets maximumScreenSpaceError to 16
  • fixes some base URL handling for some situations found in Google 3D tilesets that RealityDataSourceTilesetUrlImpl did not handle properly.
  • fixes query param propagation for some situations found in Google 3D tilesets so the query params get passed down to children tiles from child json files. This could be fixed more thoroughly (to account for other possible arrangements of tile trees).
  • handles authentication for Google 3D tilesets using IModelApp.realityDataFormatRegistry.

GoogleMapsDecorator

Other changes:

  • Moves GoogleMapsDecorator.ts from map-layers-formats to the core-frontend package, to use the decorator to display the logo in the bottom left of the viewport when the Google Photorealistic 3d Tiles are visible
  • Gets the optional copyright property when reading glTF files, which is used to display attributions in accordance with Google's policy
  • These attributions are always displayed in a pop-up when the iTwin.js logo is clicked. They are optionally displayed on-screen in the viewport via the showCreditsOnScreen flag when you create a Google3dTilesProvider (the flag is true by default)

Screenshots

image

image

image

image

Testing

  • Added unit tests for RealityDataSourceGoogle3dTilesImpl
  • Added unit tests for Google3dTilesProvider, for its addAttributions and decorate methods
  • Added a unit test to RealityDataSource.test.ts for creating a RealityDataSource from a Google3dTilesProvider

You can also test this implementation using display-test-app (DTA) after setting the IMJS_GOOGLE_3D_TILES_KEY environment variable to a proper value.

Enable the Google Photorealistic 3D Tiles in DTA by toggling off Background Map and toggling on Google Photorealistic 3D Tiles in the view settings dialog:

image

TODO

  • This PR, in the code, uses the acronym G3DT for Google Photorealistic 3d Tiles. Should we use GP3DT instead? I have seen both forms used. We have now decided to use GP3DT and this change has been pushed.
  • Should the base URL handling improvements also end up on RealityDataSourceTilesetUrlImpl? It should not. We should keep this separate for now, IMO. We need special handling for the GP3DT case including special auth handling that the TilesetUrl implementation does not handle.
  • Properly handle attribution data for the GP3DT tiles that are currently visible.
  • Add Google logo in view itself, associated with the attribution data for GP3DT tiles.
  • Consider proper code location of Google logo decorator.
  • Consider how the glTF tracking code is done --- copyright is currently in the Tile class - is this okay?
  • Clean up the glTF copyright tracking code -- this is not guarded behind GP3DT, and that is probably fine. Remove the dead code which tries to guard it behind GP3DT.
  • Instead of adding gp3dtKey onto TileAdmin, look into implementing something similar to MapLayerFormatRegistry.ts.
  • Refactor this above to add RealityDataSourceGP3DTProvider and allow users to pass in apiKey and getAuthToken function
  • Add onscreen data attributions, with flag inGoogleMapsDecorator
  • Unit tests
  • ImageTests - Ran ImageTests to look for side effects, found no relevant changes

Future (later PRs):

  • How can we do the query param propagation in a way that actually considers the entire tree structure, rather than just checking for json files? (see code for how it works, and comments). We probably should consider this for a future PR to consolidate things further. Not necessary for getting this up and running quickly in iTwin.js.

@matmarchand
Copy link
Contributor

Thanks for your effort on this.
I understand the careful approach of having a separate source implementation but I feel like RealityDataSourceTilesetUrlImpl and RealityDataSourceG3DTImpl should merge since they are based on the same standard. In that sense, maybe we should avoid exposing new APIs and hide that complexity internally by enabling Google behavior based on the url?

Unrelated to this PR but I'm surprised that GeometricError is not enabled by default. Would we get a performance improvement for reality mesh as well?

@pmconne
Copy link
Member

pmconne commented May 15, 2025

Unrelated to this PR but I'm surprised that GeometricError is not enabled by default. Would we get a performance improvement for reality mesh as well?

Yes, at the expense of drawing much lower-resolution tiles for the vast majority of reality models used with iTwins.
Apparently most reality models produced by Bentley have been created with a target error of 1 versus Cesium's 16. So they render with appropriate LOD in iTwin.js and need to override the default error to render properly in CesiumJS.
I think some changes were made in that area recently - can you check with that team?

@markschlosseratbentley
Copy link
Contributor Author

markschlosseratbentley commented May 16, 2025

Thanks for your effort on this. I understand the careful approach of having a separate source implementation but I feel like RealityDataSourceTilesetUrlImpl and RealityDataSourceG3DTImpl should merge since they are based on the same standard. In that sense, maybe we should avoid exposing new APIs and hide that complexity internally by enabling Google behavior based on the url?

Doing everything in RealityDataSourceTilesetUrlImpl was the original approach, but when we found we needed to define different behavior for usesGeometricError for Google Photorealistic 3D Tiles, we made a separate source implementation.

We could just put all of this code in RealityDataSourceTilesetUrlImpl and conditionally trigger it based on the URL, but what if the GP3DT URL changes in the future?

I also hesitate to throw all of this into RealityDataSourceTilesetUrlImpl because this comment indicates that RealityDataProvider.TilesetUrl is considered legacy.

I generally agree that the proper handling of all of this stuff should eventually also go into other source implementations that parse 3d Tiles format tiles, probably getting combined into one implementation.

Should that work be in a followup PR? (This current fix is not 100% proper, it just gets GP3DT up and running). Edit: we could tag this implementation alpha for easy followup changes.

Edit: there is also a concern that altering the core logic of RealityDataSourceTilesetUrlImpl could break other cases (but we can extensively test to be sure we're okay). But a separate implementation feels safer to put out there quickly.

@pmconne
Copy link
Member

pmconne commented May 19, 2025

const url = `https://tile.googleapis.com/v1/3dtiles/root.json?key=YOUR_GOOGLE_3D_TILES_KEY_GOES_HERE`;
viewState.displayStyle.attachRealityModel({
  tilesetUrl: url,

The url gets stored in the display style JSON. People will not want to store their Google 3D Tiles API key in there. The primary job of the various map and reality data "provider" objects is to handle authentication and format URLs. Shouldn't your provider implement those similarly to RealityDataSourceCesiumIonAssetImpl?

@markschlosseratbentley
Copy link
Contributor Author

const url = `https://tile.googleapis.com/v1/3dtiles/root.json?key=YOUR_GOOGLE_3D_TILES_KEY_GOES_HERE`;
viewState.displayStyle.attachRealityModel({
  tilesetUrl: url,

The url gets stored in the display style JSON. People will not want to store their Google 3D Tiles API key in there. The primary job of the various map and reality data "provider" objects is to handle authentication and format URLs. Shouldn't your provider implement those similarly to RealityDataSourceCesiumIonAssetImpl?

Agree.

@eringram
Copy link
Member

@markschlosseratbentley I added attributions ordered by number of occurrences. If there are copyright properties in the tiles but the tree provider is not RealityDataProvider.G3DT, it still displays them but without the Google header and icon. If there are no copyrights there will be no new logo card added to this list.

image

I also started working on adding the logo as a decorator on top of the view itself. GoogleMapsImageryProvider uses GoogleMapsDecorator to achieve this which I think we should reuse, but it's in the map-layer-formats package which we don't want to import into core-frontend, so maybe that class can be moved (it is internal). I'll try that tomorrow morning

Copy link
Member

@pmconne pmconne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good PR, some nits.

@eringram
Copy link
Member

@pmconne By the way, we noticed a side effect here from your recent maximumScreenSpaceError-related PR. Previously, we set usesGeometricError = true for the Google 3D Tiles, but did not set the maximumScreenSpaceError. It used to default to 16 I think due to this line in TileDrawArgs. After your PR it defaults to 1, so we have to set it explicitly.

This might not be an issue as it didn't show up in the reality data cases in ImageTests, and if you set usesGeometricError = true maybe you also should set max SSE, but it is a change in the default behavior for this specific case so we wanted to let you know.

Max SSE change was added in 1e74630, so I was able to confirm it still defaulted to 16 in 840bdfa.

@pmconne
Copy link
Member

pmconne commented Jun 17, 2025

@pmconne By the way, we noticed a side effect here from your recent maximumScreenSpaceError-related PR. Previously, we set usesGeometricError = true for the Google 3D Tiles, but did not set the maximumScreenSpaceError. It used to default to 16 I think due to this line in TileDrawArgs. After your PR it defaults to 1, so we have to set it explicitly.

Please fix it to default to 16 again.

@pmconne
Copy link
Member

pmconne commented Jun 17, 2025

@aruniverse please revisit your comments.

Copy link
Member

@aruniverse aruniverse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work

apiKey: string;
getAuthToken?: never;
/** If true, the data attributions/copyrights from the Google 3D Tiles will be displayed on screen. The Google Maps logo will always be displayed. Defaults to `true`. */
showCreditsOnScreen?: boolean
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isnt allowing users to opt out of this a liability for us?
when would this be ok?

Copy link
Contributor Author

@markschlosseratbentley markschlosseratbentley Jun 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We followed the pattern discussed in this comment: #8036 (comment)
Tagging @ggetz as well.

Copy link
Contributor Author

@markschlosseratbentley markschlosseratbentley Jun 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ggetz Any feedback / thoughts if a user should be able to disable this feature? (showCreditsOnScreen)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants