fix(Navisworks): CNX-973 Fix: Material ID Logic and Render Proxy Cache #463
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Refactor material selection logic to include transparency values - Update material name generation to use absolute value of render color's ARGB - Check Item category for material name before falling back to Material category - Update object ID assignment in renderMaterialProxies dictionary
- Update NavisworksColorToColor method to include alpha value for transparency - Improve code readability by explicitly specifying parameter names in System.Drawing.Color.FromArgb method
…g and rendering - Add parameter `groupedNodes` to `UnpackRenderMaterial` method - Throw exception if `groupedNodes` is null - Initialize dictionaries `renderMaterialProxies` and `mergedIds` - Build the `mergedIds` map once using grouped nodes - Use merged ids in the iteration over navisworks objects for better performance
…sMaterialUnpacker.cs
This commit adds a new boolean property, SkipNodeMerging, to the NavisworksRootObjectBuilder class. This property is used as a temporary workaround to disable node merging for debugging purposes. By default, SkipNodeMerging is set to false.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #463 +/- ##
=====================================
Coverage 9.24% 9.24%
=====================================
Files 227 227
Lines 4260 4260
Branches 533 533
=====================================
Hits 394 394
Misses 3850 3850
Partials 16 16 ☔ View full report in Codecov by Sentry. |
The commit fixes an issue with the logger import in the NavisworksRootObjectBuilder.cs file. The incorrect namespace was causing a compilation error. This change ensures that the correct namespace is used for the logger, resolving the compilation error and allowing the code to compile successfully.
...avisworks/Speckle.Connectors.NavisworksShared/Operations/Send/NavisworksRootObjectBuilder.cs
Show resolved
Hide resolved
oguzhankoral
approved these changes
Dec 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This update addresses an issue in the
materialIDlogic, where therenderProxiesmaterial cache was based solely on colour. This caused transparent elements sharing the same colour to incorrectly reuse the same material, leading to rendering errors.Additionally, the material unpacking routine has been improved to cross-check grouped geometry nodes. Previously, unnamed sibling nodes with no unique properties could result in incorrect overrides or omissions of render proxies for child geometries. The update introduces a new parameter for grouped nodes, ensuring the parent group does not improperly affect the
displayValuerender proxies of its children.also fixes: CNX-893