Allow passing tileset options to reality data tilesets

[javagl]

Feature

The function to create tilesets from reality data IDs currently uses Cesium3DTileset.fromUrl to create the tileset, and passes in some default constructor options for the tileset. This means that it is not possible to pass in custom options into a reality data tileset.

We should consider to allow users to pass in additional options through the createTilesetForRealityDataId function.

The tileset ConstructorOptions are ... overwhelming, and one could make the strong case to try and not increase the API surface area here. Specifically, there may be options that ...

1. do not have to be constructor options. but can be set on the resulting tileset directly
2. or that do not make sense for a reality data tilesets

and it might be worthwhile to identify the options that should really be available via the createTilesetForRealityDataId function. But for now, this issue is rather about the broader question whether it should be possible to pass in additional options to this function at all.

[jjspace]

iirc my motivation to not include a pass through object for the tileset or url constructors was that the data loading functions use optional arguments. I thought it would feel too annoying/weird to have to set multiple arguments to undefined just to "get to" the constructor arguments. Maybe it's worth re-evaluating and converting the existing arguments into an options object themselves so the second argument can be the optional constructor options object.

[ggetz]

Would it make sense to keep the current function signature, but allow for an options object as the third(?) argument? That would mean if the third argument is defined, we check the type and parse the options accordingly.

For the options object itself, there's president to allow all the Cesium3DTileset constructor options, and to add a few context specific options. While I do agree that the sheer number of options there is overwhelming, I think for the sake of implementation simplicity, it's out of scope of this particular issue to figure out a more user-friendly design.

[javagl]

The current createTilesetForRealityDataId receives

• the iTwinId and realityDataId (both required)
• the type and rootDocument (both optional)

I don't know whether there are cases where someone wants to call
createTilesetForRealityDataId(iTwinId, realityDataId, undefined, rootDocument);
or
createTilesetForRealityDataId(iTwinId, realityDataId, type);
but it looks like this does not make sense anyhow: If any of the optional parameters is not present, it will query both of them from some metadata provider. This also means that a call like
createTilesetForRealityDataId(iTwinId, realityDataId, ITwinPlatform.RealityDataType.GeoJSON);
would succeed (despite GeoJSON not being a supported type), because the type is overridden due to the missing rootDocument.

---

The suggestions

Maybe it's worth re-evaluating and converting the existing arguments into an options object themselves so the second argument can be the optional constructor options object.

Would it make sense to keep the current function signature, but allow for an options object as the third(?) argument?

Seem to aim at the same thing. But I'm still not sure where the tileset constructor options could come in there.

What would be the type of the options for a call like
createTilesetForRealityDataId(iTwinId, realityDataId, options);

It could not be a Tileset.ConstructorOptions, because that doesn't have the type and rootDocument properties. So some special type would be necessary here. And given the complexity of Tileset.ConstructorOptions, I'm not sure whether that new type should simply extend the given type.

I think for the sake of implementation simplicity, it's out of scope of this particular issue to figure out a more user-friendly design.

Given the question about what the type of the options should be, I think that it should be possible to define some type like

class RealityTilesetConstructorOptions {
  // Exactly what we want and need:
  type: string;
  rootDocument: string;
  asynchronouslyLoadImagery: boolean
}

and then just pass this (as-a, or converted-to, a Tileset.ConstructorOptions) to the tileset constructor.

Of course, this has caveats. What if this object (coming from JavaScript, where everything goes) does contain a cullRequestsWhileMovingMultiplier:-1.0? Should that be "forwarded" blindly? Or to we want to use the createTilesetForRealityDataId function as a blood-brain-barrier to really and only pass along specific values?

However, one broader question (that I'd like to have more clarity on) is: Which of the Tileset.ConstructorOptions are actual constructor options, and which ones can also just be modified via 'setters' later? (I think that most of them can be set later, but I didn't examine them all)

[ggetz]

What would be the type of the options for a call like createTilesetForRealityDataId(iTwinId, realityDataId, options)?

For the technicalities of adding additional options to an existing type– In TS, that is typically accomplishing through extending or intersection. I don't think there is a representation of that available in jsDoc, but we could document something similar with something like the following:

/**
 * @typedef {object} ITwinData.createTilesetForRealityDataId.Options
 * Any property of {@link Cesium3DTileset.ConstructorOptions} may be specified here. In addition to those, the following properties are supported:
 * @property {string} [type] etc...
 * @property {string} [rootDocument] etc... 
 */

Since this is, at the end of the day, a helper function and we have precedent elsewhere in the library for passing along the Cesium3DTileset.ConstructorOptions options pretty much as-is, I think we should do something similar here. Plus,

1. it allows us a bit more flexibility in case a user needs to pass along whatever Cesium3DTileset.ConstructorOptions parameter they need to without us needing to update code.
2. From an implementation standpoint, it's a simple way of getting users what they need and keeping scope from creeping.

[jjspace]

Just to clarify what I was thinking with converting the existing arguments into an options object.

// Instead of the current signature like this:
ITwinData.createTilesetForRealityDataId = async function (
  iTwinId: string,
  realityDataId: string,
  type?: ITwinPlatform.RealityDataType,
  rootDocument?: string,
) { /* ... */ }

// Switch to something like this where the first argument is all the current arguments,
// the second argument is the full tileset options object and optional.
type ITwinOptions = {
  iTwinId: string,
  realityDataId: string,
  type?: ITwinPlatform.RealityDataType,
  rootDocument?: string,
}
ITwinData.createTilesetForRealityDataId = async function (
  options: ITwinOptions,
  tilesetOptions?: Cesium3DTileset.ConstructorOptions
) { /* ... */ }

// of course this could be extrapolated even further to just combine everything like this
type ITwinOptions = {
  iTwinId: string,
  realityDataId: string,
  type?: ITwinPlatform.RealityDataType,
  rootDocument?: string,
  tilesetOptions?: Cesium3DTileset.ConstructorOptions
}
ITwinData.createTilesetForRealityDataId = async function (
  options: ITwinOptions,
) { /* ... */ }

What I would not try and do is combine the current optional arguments with the tileset options object in the same argument like I think you were suggesting above. At least without encapsulating all the tileset options into their own property. If you wanna do this approach I think it could also work fairly nicely

type ITwinOptions = {
  type?: ITwinPlatform.RealityDataType,
  rootDocument?: string,
  tilesetOptions?: Cesium3DTileset.ConstructorOptions
}
ITwinData.createTilesetForRealityDataId = async function (
  iTwinId: string,
  realityDataId: string,
  options?: ITwinOptions,
) {/* ... */ }

(I wrote these in TS cause it was easier but they should all be reproducible in JSDoc too, lmk if you want help with that)

[jjspace]

The first half of this task, the decision on the new API structure, has been addressed in #12778. All that's left is to add the options properties to pass along the tileset options.