-
Notifications
You must be signed in to change notification settings - Fork 210
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
Expose 3D Tiles feature attributes #71
Comments
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
As |
This code from CesiumJS may be useful: https://github.com/CesiumGS/cesium/blob/master/Source/Scene/parseBatchTable.js |
Current progress from @kring: 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 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 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, On the other hand, if we put a Metadata instance in 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 |
I can definitely imagine that some specialization will be needed for
I probably don't know what you mean by offset, but it sounds like |
The array type has offset buffer to determine the length of the array for each instance. For example: The offset buffer means that the first element's array will point to from 0th to 3rd values in the value buffer (4 elements) 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 |
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:
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 |
Another thought is that, let's template
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 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 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
|
CesiumJS has the same dilemma for GPU styling and may require lossy fallbacks/warnings CesiumGS/cesium#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 |
@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.
The text was updated successfully, but these errors were encountered: