Skip to content
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

Type information for metadata #12269

Open
javagl opened this issue Oct 30, 2024 · 2 comments
Open

Type information for metadata #12269

javagl opened this issue Oct 30, 2024 · 2 comments

Comments

@javagl
Copy link
Contributor

javagl commented Oct 30, 2024

tl;dr: There are some functions for accessing metadata in CesiumJS. These functions are generally declared to return * (aka any) in the JSDoc. This is converted to any for the TypeScript bindings. We could consider to narrow down the types that are returned by these functions.


Context

CesiumJS supports the 3D Metadata Specification in two flavors: Once as part of the 3D Tiles 1.1 support, and once as part of the Cesium EXT_structural_metadata glTF extension. On the level of the API, these are treated equally (and intentionally so).

There are examples like the MetadataGranularities sample where the metadata is accessed casually in the example sandcastle. But this is using a private API. The metadata is not exposed publicly right now. The representations of the metadata - in classes like TilesetMetadata, TileMetadata ... - follows (implicitly - not explicitly!) the interface that is defined as MetadataEntity. This might become part of a public API at some point. The main function in this interface for accessing metadata is the getProperty function, which returns the value of a metadata property. Beyond that, the only function to obtain metadata values right now is via Cesium3DTileFeature.getProperty/Inherited.

The types of these functions are currently documented as

@returns {*} The value of the property

This is translated to any on the TypeScript layer.

People might wish for more specific type information. This could be accomplished by adding "typedefs" on the JSDoc layer.

Possible approach

Translating the existing type system from the 3D Metadata Specification into TypeScript typings could be pretty straightforward. But there are caveats.

For example: In some cases, types like the VEC3/FLOAT32 from the metadata specification are represented as a number[] array, and in other cases, they are represented as a Cartesian3. Both can make sense, depending on the context. But for the case of users accessing that metadata, the context is simply not known. Converting these types back and forth is usually easy, with the pack/unpack functions. But this may be a bit inconvenient - and in order to know which function to call, the type has to be known, which is kind of a chicken-egg-problem.

Another caveat is that in many cases, the types that are (and can be) represented as metadata already are limited. For example, in EXT_structural_metadata, a 'property texture' cannot contain STRING valued metadata. Whether or not this is made explicit on the level of the type system at some point has to be decided.

Draft of a possible approach

The following is a draft of a (largely) 1:1 translation of the 3D Metadata Specification type system into TypeScript. This is shown as a standalone file here, for quicker feedback and iterations:

// Component types
type INT8 = number;
type UINT8 = number;
type INT16 = number;
type UINT16 = number;
type INT32 = number;
type UINT32 = number;
type INT64 = bigint;
type UINT64 = bigint;
type FLOAT32 = number;
type FLOAT64 = number;

// Non-numeric component types
type STRING = string;
type BOOLEAN = boolean;
type ENUM = string; // TODO: Also number in some cases...

// Numeric types
type UnsignedInteger = UINT8 | UINT16 | UINT32 | UINT64;
type SignedInteger = INT8 | INT16 | INT32 | INT64;
type Integer = UnsignedInteger | SignedInteger;
type FloatingPoint = FLOAT32 | FLOAT64;
type Numeric = Integer | FloatingPoint;

// Vector types
type VEC2 = [Numeric, Numeric];
type VEC3 = [Numeric, Numeric, Numeric];
type VEC4 = [Numeric, Numeric, Numeric, Numeric];
type Vector = VEC2 | VEC3 | VEC4;

// Matrix types
type MAT2 = [Numeric, Numeric, Numeric, Numeric];
type MAT3 = [
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric
];
type MAT4 = [
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric,
  Numeric
];
type Matrix = MAT2 | MAT3 | MAT4;

// Helpers
type NonNumeric = STRING | BOOLEAN | ENUM;
type MetadataValue = Numeric | NonNumeric | Vector | Matrix;

//----------------------------------------------------------------------------
// Usage/test:

function getMetadataValue(): MetadataValue {
  //const value : STRING = "example";
  //const value : BOOLEAN = true;
  //const value : Numeric = 123;
  //const value : Vector = [0, 1, 2];
  const value: Matrix = [0, 1, 2, 3];
  return value;
}

function callIt() {
  let value: MetadataValue | undefined = undefined;
  value = getMetadataValue();
}

//----------------------------------------------------------------------------
// Bridge to CesiumJS

// Built-in in CesiumJS (dummy types here)
type Cartesian2 = { name: "Cartesian2" };
type Cartesian3 = { name: "Cartesian3" };
type Cartesian4 = { name: "Cartesian4" };
type Matrix2 = { name: "Matrix2" };
type Matrix3 = { name: "Matrix3" };
type Matrix4 = { name: "Matrix4" };

// Helpers
type CesiumVector = Cartesian2 | Cartesian3 | Cartesian4;
type CesiumMatrix = Matrix2 | Matrix3 | Matrix4;
type CesiumMetadataValue = Numeric | NonNumeric | CesiumVector | CesiumMatrix;

function getMetadataValueCesium(): CesiumMetadataValue {
  //const value : STRING = "example";
  //const value : BOOLEAN = true;
  //const value : Numeric = 123;
  //const value : CesiumVector = "Cartesian2";
  const value: CesiumMatrix = { name: "Matrix2" };
  return value;
}

function callItForCesium() {
  let value: CesiumMetadataValue | undefined = undefined;
  value = getMetadataValueCesium();
}

(EDIT: Array types are not covered here - in many cases, this could be solved by appending a [] at the right place. Whether or not this is made explicit is also to be decided)

Disclaimer:

One could make a case that this type information is of very limited use for clients. They can call a function (as drafted in the snippet above). And then they still don't know whether that value is a single boolean, or a bigint[], or a Matrix4. Actually using that type information will require it to be narrowed down - probably, by inspecting the ClassProperty of the metadata (if it is accessible...), or by some sort of runtime type checks.

@javagl
Copy link
Contributor Author

javagl commented Nov 17, 2024

I did some experiments for this. The current state/draft is only in d7aeb52 (not in a shape for a PR, I guess).

The type system, visibility concepts, and translation from JSDoc to TypeScript appear to simply be not powerful enough to reasonably support this.

The typings that are drafted in the TypeScript snippet above can be translated into JDoc @typedefs. Each of them will have to be in its own /** block */ for whatever reason. But more importantly, ...

So the resulting Cesium.d.ts will contain the whole stuff like

export type INT8 = number;
export type UINT8 = number;
...
export type VEC2 = Numeric[];
...
export type MetadataValue = Numeric | NonNumeric | Vector | Matrix;

and it does not appear to be possible to only export the MetadataValue type, in its full expansion, while leaving the other types @private.

These types (like UINT8 = number) should (of course? or probably?) not be visible for clients. They are only intended to cleanly model the actual types.

Workarounds like indicating that in the name, like _INTERNAL_UINT8 or whatnot, would not make sense here.


There's still the option of exporting only the actual type, without intermediate @typedefs. Then there are still two further options:

The expanded metadata type in terms of "raw values" (arrays) is this:

type _MetadataValue =
  | number
  | bigint
  | string
  | boolean
  | [number | bigint, number | bigint]
  | [number | bigint, number | bigint, number | bigint]
  | [number | bigint, number | bigint, number | bigint, number | bigint]
  | [
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint
    ]
  | [
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint,
      number | bigint
    ];

EDIT: Given that there are no tuples in JSDoc, this would boil down to

type _MetadataValueJs =
  | number
  | bigint
  | string
  | boolean
  | (number | bigint)[];

The expanded metadata type with "object types" (like Cartesian2 instead of [Numeric, Numeric] is this:

type _CesiumMetadataValue =
  | number
  | bigint
  | string
  | boolean
  | Cartesian2
  | Cartesian3
  | Cartesian4
  | Matrix2 
  | Matrix3
  | Matrix4;

The latter looks OK-ish. But ....

  • we still have to think about cases where there may be arrays of these. This could be modeled with something like
    type CesiumMetadataValueElement = ...; // See above
    type CesiumMetadataValue = CesiumMetadataValueElement | CesiumMetadataValueElement[];
    
  • It is not yet clear which part of the API will require which type (i.e. the "raw" or the "object" form)

It might be that we'll need both, unless we can ensure that the public part of the API really, always, only needs the "object-based" form. Whether or not this is the case for the existing getProperty remains to be investigated.

(In the worst case, there is a single function that could return both, depending on where the data is coming from - hopefully, this is not the case 🤞 )

@javagl
Copy link
Contributor Author

javagl commented Nov 19, 2024

Given that it is not possible to nicely construct the actual type in a clean bottom-up fashion, it will boil down to a fixed string in the
@typedef {(lots of types)} MetadataValue

This is drafted in #12315

(I'd lean towards closing this issue, but there might be a connection to #10455 : If the current ts-jsdoc based approach is revised, then it might be possible to model the type more cleanly...)

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

No branches or pull requests

2 participants