atteneder / glTFast

Efficient glTF 3D import / export package for Unity
Other
1.22k stars 246 forks source link

Expose custom vertex attributes #154

Open FreakTheMighty opened 3 years ago

FreakTheMighty commented 3 years ago

The gltf spec supports custom vertex attributes prefixed with an underscore. We'd like to access those attributes and are looking to make a contribution to enable this. Couple questions:

  1. Is this a feature you'd consider adding to this library?
  2. Do you have any thoughts on how to implement this?

It kinda looks like this method would do what we want, but even with this public, we'd still need access to the Root object which I don't think we currently have. https://github.com/atteneder/glTFast/blob/196b5bce6819a6cbfba7d77a0dcbfec776547032/Runtime/Scripts/GltFast.cs#L1950

We added this same feature to MRTK's gltf loader. Here's the pull request for reference https://github.com/microsoft/MixedRealityToolkit-Unity/pull/7846

CC: @bhtruong93

bhtruong93 commented 3 years ago

We might also want to take a look at https://github.com/microsoft/MixedRealityToolkit-Unity/pull/7535 for turning attributes into a dictionary to hold custom data Maybe we can do something similar and get the attributes like in here?

atteneder commented 3 years ago

Hi @FreakTheMighty ,

Thanks for your interest and sorry for the late reply.

I'm very much open to changes like that as long as they don't compromise the original goal of providing a performant loader for more regular cases. We should figure out how we can achieve both.

If your vertex attribute is somewhat generic/not too specific (so that others might use it as well), we could think about adding it (maybe behind a scripting define)

One challenge is that this requires adding the custom attribute to the Schema classes (at least with the current JSON solution JsonUtility), so that it is properly de-serialized from JSON. Maybe we can use partial class implementation to solve this?

Could you share a little bit more detail about the expected structure and use case of your custom attribute.

Thanks!

FreakTheMighty commented 3 years ago

No worries! @bhtruong93 has already opened a pull request https://github.com/atteneder/glTFast/pull/157

I don’t think our attribute will be broadly useful, is “_BATCHID”, from the cesium 3D tiles spec. I’m with you on the performance goals, that’s why we’re switching to this library. We can talk more specifically about the pull request, but my sense is that this wouldn’t impact performance unless there are additional attributes to parse. If there are, I suspect the impact is relatively small and proportional to the amount of additional data.

atteneder commented 3 years ago

Thanks for the additional info! Not sure how popular this 3D tiles spec is, but it might just be one of those edge cases.

FreakTheMighty commented 3 years ago

I think it is very much an edge case. I can’t imagine it’s more than a handful of people who would want that specific vertex attribute. With that said, the ability to access custom attributes generically probably has more broad uses.

brnkhy commented 6 months ago

@atteneder are there any updates on this? I noticed there has been some big changes in the codebase since 2021, is this somehow possible in current version? I also saw a metadata test branch being mentioned somewhere but branch isn't available anymore. Any chance that was relevant to this (or was it just for "extra"?).

atteneder commented 6 months ago

No direct update on the topic.

However, since version 6 one can at the very least parse those properties (using the Newtonsoft JSON parser). The next step would be to provide an API that lets you assign accessors' data to VertexAttributes (or do the actual conversion yourself).