DYN-8303: Enable deferred node load #16361
DYN-8303: Enable deferred node load #16361zeusongit wants to merge 46 commits intoDynamoDS:masterfrom
Conversation
# Conflicts: # src/DynamoCoreWpf/Views/Core/NodeView.xaml # src/DynamoCoreWpf/Views/Core/NodeView.xaml.cs
…eNodeBackground
Introduced a new middleware to handle user authentication for protected routes. This ensures that only authenticated users can access certain endpoints, improving application security.
Replaces the static node loading indicator with an animated progress spinner and detailed loading status in NotificationsControl.xaml. Makes LoadedNodesCount and NodesLoading properties public in WorkspaceViewModel for better data binding. Caches node images after conversion in NodeView.xaml.cs to improve performance.
DYN-8303: Static Field Optimizations
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8303
There was a problem hiding this comment.
Pull Request Overview
This PR enables deferred loading of node visuals for large graphs, simplifies node styling, and integrates cached node images as embedded resources.
- Implements deferred node rendering above a node-count threshold with a progress counter
- Refactors zoom fade styles into shared bindings and cleans up animated styles
- Adds embedded resource support for node images and wiring in the workspace view model
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| NodeView.xaml.cs | Introduces deferred UI loading, bitmap-based port markers, new style bindings, and resource-based node images |
| WorkspaceViewModel.cs | Tracks loaded node count, exposes NodesLoading, and logs load times |
| PublicAPI.Unshipped.txt | Exposes new LoadedNodesCount, NodesLoading, and GetNodeImage APIs |
| DynamoCoreWpf.csproj | Embeds NodeCacheImages.resx as a resource |
| NotificationsControl.xaml | Displays node-loading progress spinner and counter |
Comments suppressed due to low confidence (3)
src/DynamoCoreWpf/ViewModels/Core/WorkspaceViewModel.cs:198
- [nitpick] Private fields are typically prefixed with
_(e.g._nodeLoadStopwatch) to distinguish them from properties. Consider renaming for consistency.
private Stopwatch nodeLoadStopwatch = new Stopwatch();
src/DynamoCoreWpf/ViewModels/Core/WorkspaceViewModel.cs:605
- The new
NodesLoadingproperty and related deferred-load behavior appear untested. Consider adding unit or integration tests to verify the loading indicator and count logic.
public bool NodesLoading
src/DynamoCoreWpf/Views/Core/NodeView.xaml.cs:1453
- Ensure that the embedded resource name matches the manifest path of
NodeCacheImages.resx. A mismatch here can cause a null stream and silent failures.
var stream = assembly.GetManifestResourceStream("Dynamo.Wpf.NodeCacheImages.resources");
| }; | ||
|
|
||
| Canvas.SetZIndex(renameIndicator, 5); | ||
| SetValue(Panel.ZIndexProperty, 5); |
There was a problem hiding this comment.
Calling SetValue here applies ZIndex to the NodeView itself rather than to renameIndicator. You should use renameIndicator.SetValue(Panel.ZIndexProperty, 5); to target the correct element.
| SetValue(Panel.ZIndexProperty, 5); | |
| renameIndicator.SetValue(Panel.ZIndexProperty, 5); |
| var writeableBitmap = new WriteableBitmap(bitmap); | ||
|
|
||
| // Define rectangle position and size | ||
| int width = 5, height = 29; |
There was a problem hiding this comment.
[nitpick] These magic numbers (5, 29) are not self-documenting. Consider extracting them into named constants or adding a comment explaining their origin.
| private static readonly Dictionary<string, BitmapImage> _cachedImages = | ||
| new Dictionary<string, BitmapImage>(StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| //Freeze the static resource to reduce memory overhead... Not sure we need this. |
There was a problem hiding this comment.
[nitpick] This TODO-style comment is ambiguous. Either clarify whether freezing is still required or remove it to keep the codebase clean.
| //Freeze the static resource to reduce memory overhead... Not sure we need this. | |
| // Freeze the static resources to reduce memory overhead and improve performance. |
| return result; | ||
|
|
||
| // Define the specific invalid characters to remove | ||
| char[] invalidChars = { '/', '\\', ':', '*', '?', '"', '<', '>', '|' }; |
There was a problem hiding this comment.
[nitpick] Consider using Path.GetInvalidFileNameChars() instead of hard-coding invalid filename characters to keep this list in sync with the underlying OS.
| <StackPanel | ||
| Orientation="Horizontal" Margin="5,0" | ||
| VerticalAlignment="Center"> | ||
| <Image Name="downloadingIcon" |
There was a problem hiding this comment.
[nitpick] Use x:Name instead of Name in XAML to ensure uniqueness across templates and avoid potential conflicts.
| <StackPanel | ||
| Orientation="Horizontal" Margin="5,0" | ||
| VerticalAlignment="Center"> | ||
| <Image Name="downloadingIcon" |
There was a problem hiding this comment.
The progress spinner lacks an automation name or tooltip for screen readers. Consider adding AutomationProperties.Name="Loading nodes" or similar.
| } | ||
|
|
||
| // Write transparent pixels to the left 5px of the image | ||
| writeableBitmap.WritePixels( |
There was a problem hiding this comment.
[nitpick] Calling WritePixels in a loop for each port marker may cause UI thread stalls on large graphs. Evaluate batching or offloading to a background thread to improve responsiveness.
Replaces the hardcoded list of invalid filename characters with System.IO.Path.GetInvalidFileNameChars() when sanitizing node names. Also removes obsolete API entries related to node image retrieval from the public API file.
|
@cursor review |
|
Skipping Bugbot: Unable to authenticate your request. Please make sure Bugbot is properly installed and configured for this repository. |
Purpose
This PR:
Profile results (time denotes when the workspace becomes available)
512
Manekin
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Adds deferred node load for graphs above 150 node count.
Reviewers
(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)
(FILL ME IN, optional) Any additional notes to reviewers or testers.
FYIs
@achintyabhat