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

Tweaks to asset sharing to support Unreal implementation #959

Merged
merged 8 commits into from
Oct 4, 2024

Conversation

kring
Copy link
Member

@kring kring commented Oct 3, 2024

This is a PR into #926. It goes with CesiumGS/cesium-unreal#1527.

@lilleyse
Copy link
Contributor

lilleyse commented Oct 3, 2024

Couple questions since I have a similar enum/struct thing in another project and would like to use this eventually

  • Should it be called VertexAttributeSemantics in case we add InstanceAttributeSemantics in the future?
  • Is there the possibility of allowing custom semantics like _FEATURE_ID_n?

@kring
Copy link
Member Author

kring commented Oct 3, 2024

Should it be called VertexAttributeSemantics in case we add InstanceAttributeSemantics in the future?

Good idea, I'll rename it.

Is there the possibility of allowing custom semantics like _FEATURE_ID_n?

Do you have an idea what that would look like? It's not obvious to me what value we could provide for custom semantics in a class like this, but I could be missing something.

@azrogers azrogers merged commit 80502ad into shared-assets Oct 4, 2024
24 checks passed
@azrogers azrogers deleted the shared-assets-kring branch October 4, 2024 14:39
@lilleyse
Copy link
Contributor

lilleyse commented Oct 4, 2024

Do you have an idea what that would look like? It's not obvious to me what value we could provide for custom semantics in a class like this, but I could be missing something.

I think it would look the same as TEXCOORD_n and others. But I could also be missing context from CesiumGS/cesium-unreal#1527

@kring
Copy link
Member Author

kring commented Oct 8, 2024

Oh, ok, I don't have any problem with adding extra commonly-used attributes from extensions to this class. I thought you were suggesting we make it externally-extensible somehow.

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

Successfully merging this pull request may close these issues.

4 participants