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

Clean up FabricMaterial #540

Merged
merged 2 commits into from
Nov 9, 2023
Merged

Clean up FabricMaterial #540

merged 2 commits into from
Nov 9, 2023

Conversation

lilleyse
Copy link
Contributor

@lilleyse lilleyse commented Nov 9, 2023

I did some cleanup in FabricMaterial while adding support for feature ids.

  • The base color texture is now set in setMaterial rather than a separate function. This keeps the material setting code more cohesive. Imagery layers are still set separately because their loading happens in a different phase of the pipeline.
  • Reverted some of the changes in Add base color texture node to tileset materials #406. E.g. if the user has multiple cesium_base_color_texture_float4 nodes, we no longer create multiple base color texture prims to feed into each one. Instead we create a single prim and add multiple connections. This is cleaner and should be more performant. This applies to imagery layers too.
  • General cleanup that made it easier to implement feature ids, and might make it easier to fix Update tiles automatically when material changes #504

I tested this PR with a variety of scenes from previous PRs: test-data-sets.zip

Comment on lines -58 to 60
bool hasBaseColorTextureGltf(const FabricMesh& fabricMesh) {
return fabricMesh.material != nullptr && fabricMesh.material->getMaterialDefinition().hasBaseColorTexture() &&
fabricMesh.materialInfo.baseColorTexture.has_value();
bool hasBaseColorTexture(const FabricMesh& fabricMesh) {
return fabricMesh.material != nullptr && fabricMesh.material->getMaterialDefinition().hasBaseColorTexture();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deleted part used to be important, but is no longer needed after #495.

@lilleyse lilleyse merged commit 7abe3d8 into main Nov 9, 2023
3 checks passed
@lilleyse lilleyse deleted the cleanup-fabric-material branch November 9, 2023 19:08
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.

Update tiles automatically when material changes
2 participants