Skip to content

Fix mesh deduplication for meshes with different materials #842

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

Merged
merged 1 commit into from
Apr 15, 2025

Conversation

hybridherbst
Copy link
Collaborator

Fixes #836.

Instead of overriding the node mesh id for shared meshes, we override now the UnityMeshData and LoadedMesh to fix the issue.

@hybridherbst hybridherbst requested a review from Copilot April 15, 2025 13:25
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request addresses issue #836 by fixing the mesh deduplication problem for meshes with different materials. The key changes include:

  • Invoking CreateMeshMaterials asynchronously for shared meshes in ConstructMesh.
  • Refactoring the call to CreateMaterials for each primitive.
  • Updating the assignment of LoadedMesh in UnityMeshData to cover all cache entries referencing the same data.
Comments suppressed due to low confidence (1)

Runtime/Scripts/SceneImporter/ImporterMeshes.cs:725

  • [nitpick] Verify that using reference equality for UnityMeshData in the loop reliably identifies shared meshes. If duplicate instances could exist, consider an alternative approach to ensure all related MeshCache entries are updated correctly.
// Assign the loaded mesh to all MeshCache entries that reference the same UnityMeshData

… we override now the UnityMeshData and LoadedMesh to fix the issue
@hybridherbst hybridherbst force-pushed the fix/mesh-deduplication-material-losing branch from 7cc3021 to f800bef Compare April 15, 2025 13:29
@hybridherbst hybridherbst merged commit f800bef into dev Apr 15, 2025
1 check passed
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.

2 participants