Closed kring closed 3 years ago
3D Tiles Next will use the the EXT_feature_metadata extension embedded in the tile glTF to encode feature metadata. So we'd like to use this extension to communicate feature data coming from cesium-native as well. That means converting b3dm batch tables to EXT_feature_metadata
on load. There are some challenges to that:
EXT_feature_metadata
only supports binary properties, not JSON ones. So batch table JSON properties need to be converted to binary.EXT_feature_metadata
requires that a given property have the same type across all features. The b3dm batch table is more flexible when encoded in JSON. So we'll have to convert all values to a common representation when converting the batch table. i.e. convert mixed-type properties to strings.EXT_feature_metadata
does not allow a property to have a "null" value for a particular property. So here, too, we'll need to convert nulls to zero-length strings.EXT_feature_metadata
does not support anything like the 3DTILES_batch_table_hierarchy
extension. So the hierarchy will need to be flattened into parallel arrays. This will likely introduce nulls, which will in turn require types to be widened to strings.As EXT_feature_metadata
evolves, or as further extensions with more capability are added, we can revisit these concessions. But this will likely be "good enough" for most tilesets.
This code from CesiumJS may be useful: https://github.com/CesiumGS/cesium/blob/master/Source/Scene/parseBatchTable.js
Current progress from @kring: https://github.com/CesiumGS/cesium-unreal-samples/tree/picking https://github.com/CesiumGS/cesium-unreal/tree/picking https://github.com/CesiumGS/cesium-native/tree/feature-metadata-extension
On the native side, the TODOs are:
On the unreal side, TODOs are:
@kring the common API to access both b3dm and EXT_feature_metadata is in the https://github.com/CesiumGS/cesium-native/tree/feature-metadata-api branch. This is a high level API to access both batch table and gltf EXT_feature_metadata without the need to transcoding from one format to the other.
The high level design is that we have an abstract Metadata
class and 5 PropertyView
classes. The Metadata is mostly just a simple map from a property name to a PropertyView
object. For PropertyView
class, we have PrimitivePropertyView
to represent scalar, StringPropertyView
to represent string, ArrayPropertyView
to represent array (this should work for fixed and dynamic array), JsonPropertyView
to represent json, and StructPropertyView
for struct (this is mostly a std::map<std::string, std::unique_ptr<PropertyView>>
). Those PropertyView classes represents the whole collection of instances similar to how batch table work. For example, in the batch table, if ID is a collection of Unsigned Int of instances, then we have a PrimitivePropertyView to represent the array of Unsigned Int similar to that.
So to bridge the difference between batch table and EXT_feature_metadata, we would need to implement those 5 properties for each format, and then on Unreal side, it will only deal with those high level API rather than using the batch table directly. Similar for EXT_feature_metadata. To transcoding from one to the other, one can create Transcoder class and only deal with those high level API rather than the format itself. Please let me know if we should go ahead with it
@kring I forget to mention, the whole code mostly in CesiumMetadata directory
Thanks @baothientran. I took a look at the feature-metadata-api
branch, and I guess the main thing I'm unclear on is how this will connect with what we already have. Will there be a new extension to hold a Metadata
instance? An additional Metadata
property on TileContentLoadResult
?
The problem is, I don't really like either of those options.
It seems like a bad idea to have cesium-native specific glTF extensions. Especially when we already have a new extension for metadata that we're in the process of defining, EXT_feature_metadata
. It doesn't capture everything in the b3dm batch table with perfect fidelity, of course. If that matters, we should fix the EXT_feature_metadata
extension. If it doesn't matter (because we don't think anyone is using the missing features for anything important, or there are acceptable alternative ways to do it), then that should be good enough for cesium-native too.
On the other hand, if we put a Metadata instance in TileContentLoadResult
, we're moving away from the ideal scenario where glTF (a well-designed and extensible standard!) is the native language of cesium-native and used to communicate everything about a model/tile to any rendering engines or other applications that use it.
So I think I would still rather see batch table transformed into EXT_feature_metadata. We should provide helpers, along the lines of AccessorView and AccessorWriter to make it easier to read and write feature data stored as EXT_feature_metadata. Ideally, we'd be able to use AccessorView and AccessorWriter directly. The writer would be used by Batched3DModelContent to do the conversion. I think that's putting our engineering effort where it belongs - our new EXT_feature_metadata extension - rather than in creating abstractions over legacy stuff like the b3dm batch table.
gsl::span is so painfully close to be an ideal interface for an array property type. What it can't represent is the array of string or maybe array of bool (I haven't tested it yet but my gut say it can't because boolean is bit pack together similar std::bitset
). For array of string, it typically uses one buffer to store value and two offset buffers to represent array length and string length (https://github.com/CesiumGS/3d-tiles/tree/3d-tiles-next/specification/Metadata/1.0.0#arrays-1). Unfortunately gsl::span doesn't have the concept of offset, so it's not possible without additional allocation. The only solution is to create a custom array view and specialize it for those cases, e.g ArrayView<std::string_view> or ArrayView<bool>
I can definitely imagine that some specialization will be needed for AccessorView<std::string_view>
and AccessorView<bool>
. Is that what you mean?
Unfortunately gsl::span doesn't have the concept of offset, so it's not possible without additional allocation.
I probably don't know what you mean by offset, but it sounds like subspan
?
The array type has offset buffer to determine the length of the array for each instance. For example: To represent: instance = [[1, 2, 3, 4], [3, 4], [5, 7, 8, 9]] The format will be: value buffer: [1, 2, 3, 4, 3, 4, 5, 7, 8, 9] offset buffer: [0, 4, 6, 10] (I simplify it here to indicate instance index, but it's actually byte offset)
The offset buffer means that the first element's array will point to from 0th to 3rd values in the value buffer (4 elements) the second element's array will point to from 4th to 5th value in the value buffer (2 elements) and so on
Beside array, string type has the same offset buffer as well.
If they are alone like that, gsl::span and std::string_view are enough to represent those data type, but when we have nested array of string, it's ideal to have gsl::span<std::string_view>
. However, the specs says that to represent array of string, we will need two offset buffers. Something like:
value buffer: [["asas", "ttasas"], ["aaaaa"], ["thiaaa", "sssss"]]
array offset: [point to array index]
string offset: [point to string index]
The implementation for the property view is in this link https://github.com/CesiumGS/cesium-native/blob/feature-metadata-extension/CesiumGltf/include/CesiumGltf/PropertyAccessorView.h
For the implementation, I decide not to template the whole class PropertyAccessorView. Instead, client will need to use a getter method to retrieve the value from the buffer. Something like this:
PropertyAccessorView property{}; // initialize property view here
if (property.getType() == PropertyType::Int32) {
for (size_t intanceIdx = 0; instanceIdx < property.numOfInstances(); ++instanceIdx) {
int32 val = property.get<int32_t>(instanceIdx);
// do something with the value
}
}
The only caveat is that if client casts a type that is different with what property.getType() reports, it is undefined behavior. But it may change to return nullptr instead. The only small problem with returning nullptr is we end up with a branching code every time we want to access an instance. Perhaps I should have two method getUnsafe<T>
and getSafe<T>
Another thought is that, let's template PropertyAccessorView
, e.g PropertyAccessorView<int32_t>
. Then we create a MetadataView
to capture the whole ModelExt_Feature_Metadata
. So the API usage will be something like this:
MetadataView metadata(ModelExt_Feature_Metadata);
if (metadata.getPropertyType("Height") == PropertyType::Int32) {
PropertyAccessorView<int32_t> heights = metadata.getProperty<int32_t>("Height");
for (size_t instanceIdx = 0; instanceIdx < heights.size(); ++instanceIdx) {
int32_t value = heights[instanceIdx];
}
}
The whole MetadataView is similar to a lightweight manager to manage properties in the model.
Compare to the first design, the second one may be safer to use because we can check if the property is valid before returning the result of the cast to client. That is we can put a check in the function metadata.getProperty<int32_t>("Height");
and return an optional if the property doesn't have a correct type. And the validation only happens at the beginning and not in the loop like in the previous design
Between two designs, please let me know if you have any opinions on any of them
@baothientran your second design definitely looks better to me. You can also provide a method to create a PropertyAccessorView and dispatch it to statically-typed callback, rather than requiring the user to manually switch on the type. Take a look at createAccessorView
in AccessorView.h
. So using it would look something like this:
struct PropertyVisitor {
void operator()(const PropertyAccessorView<int32_t>& view) {
for (size_t instanceIdx = 0; instanceIdx < view.size(); ++instanceIdx) {
int32_t value = view[instanceIdx];
}
}
void operator()(const PropertyAccessorView<std::string_view>& view) {
// ...
}
template <typename T>
void operator()(const PropertyAccessorView<T>& view) {
// catch all for other types of properties
}
};
ModelEXT_feature_metadata& metadata = ...;
// We can add properties directly to ModelEXT_feature_metadata by using
// the `toBeInherited` property in glTF.json, so maybe we don't need a separate
// MetadataView class at all?
metadata.createView("Height", PropertyVisitor{});
while I implement blueprint library for metadata, I just realize it doesn't support uint64
and double
. I'm not sure what to do with those cases. Possible solutions are
int64
and float
and have inaccuracy
word in the blueprint function name as a warning to the user.Add
, Subtract
, Multiply
, etc between those structs and native type as well. e.g some blueprint functions are like this Add(FCesiumDouble, FCesiumDouble)
or Add(FCesiumDouble, float)
, etc. Also provide Conversion
node to cast to them to native type. e.g: ConvertToFloat(FCesiumDouble) -> float
. Note that they are only for blueprints. The C++ interface is still normal. The only disadvantage of this is that it takes effort to maintainCesiumJS has the same dilemma for GPU styling and may require lossy fallbacks/warnings https://github.com/CesiumGS/cesium/issues/9572
ah I see it converts uint64 to float instead of int64. Good point! I guess we can live with the lossy keyword for blueprint for now
@kring This is probably a minor problem, but I think it's better to discuss here. Currently, to access metadata, we have *View
classes to inspect the data. On the Unreal side, I currently wrap those classes in the USTRUCT to be used with the blueprint. However, one problem is that because those classes don't own the metadata storage (e.g gltf buffer), it requires Gltf Model to be around until the tile is evicted from the cache. So it is probably a little wasteful since we can remove the mesh data right away once we are done creating unreal resources. In the case that we need to remove gltf model to free more memory, the metadata need to copy and own the metadata buffers. So between the two options a) keeping the gltf model around and use the view classes and b) has the metadata owns the its buffers, what's your opinion on them? Or maybe a compromise in the future is that we can have both just like we have std::string
and std::string_view
@baothientran we currently don't unload the glTF data even once we've created the Unreal resources, right? Doing so might be a useful optimization, but it definitely raises some problems, so we'd have to approach it cautiously. Unless I've missed something, I don't think we need to worry about it currently. Our solution to that might be impacted by how we make feature metadata available to Materials as well.
nope we still keep the gltf currently. The above problem is just a small assumption, but it doesn't affect much with the current implementation of metadata on unreal side
Because tiles are "sent" to the renderer as glTFs, we should consider encoding feature attributes the way they [are / will be] encoded in 3D Tiles Next.