MAGN-7381 Distracting UI lag when selecting, unselecting any node when there is a node in the graph that produces a lot of geometry #4637
Conversation
This file contains hidden or 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
Method used to calculate smooth gradients between values.
We don't want this to default to showing [0]. It should default to whatever the render package's description is set to.
Graphics Perfomance changes
Update to Watch3Dview mesh
Updated helix binaries
Added comments
…e' into MAGN-7381-Graphics-Performance
Fix around performance when selecting the nodes
Added comments
…e' into MAGN-7381-Graphics-Performance
Contributor
There was a problem hiding this comment.
These are hidden, but not removed from the library so that existing workspaces continue to function, but the nodes are not available in the library.
Contributor
|
@ramramps I've added my comments. Can we also remove the WIP tag? Please let me know when you want further review. |
Contributor
|
@ramramps There is also one bug remaining with the selection of curve elements. Curve elements seem to be highlighting with selection during window drag, but not when you click on their parent node to select it. They also don't seem to be un-selecting after their initial placement, until the graph is executed. |
Fix for Watch3D node
renamed the events and members
Changed the property type
changed the geometry selection
Detach when a node is deleted
Remove Start and Stop methods from VisualizationManager
…---7381-Graphics-Perfomance Conflicts: src/DynamoCoreWpf/Views/Preview/Watch3DView.xaml.cs
Magn 7381 cont.
ikeough
pushed a commit
that referenced
this pull request
Jun 26, 2015
MAGN-7381 Distracting UI lag when selecting, unselecting any node when there is a node in the graph that produces a lot of geometry
This was referenced Jun 28, 2015
mjkkirschner
pushed a commit
that referenced
this pull request
Aug 18, 2020
* DYN-3026-CodeCoverage-AnalysisFolder I created several test methods for increasing the code coverage in the next classes (Analysis project). - Label - SurfaceData - AnalysisExtensions - Utils Also I removed two methods from the SurfaceData.cs file since both have cero references and both are private. * DYN-3026-CodeCoverage-AnalysisFolder Fix for make the Analysis assembly visible to the AnalysisTests project. * DYN-3026-CodeCoverage-AnalysisFolder-CodeReview Changes asked by Aparajit. * DYN-3026-CodeCoverage-AnalysisFolder-CodeReview2 I re-wrote the test for the Label class since Michael asked me to use a Label inside a Code Block node, then I created 3 dyn files with one CodeBlock creating a Label, one compiling and other two with errors (on purpose) when passing parameters to the Label.ByPointAndString method. Also by Michael response I undid the changes done by Aaron in the PR #8189. Finally I applied some changes that the developer Ian Keough did like 5 years ago (HelixRenderPackage.cs -> CleanTag) but were discarded in the PR below, with the code as it is now in the Dynamo repo when creating a Label inside a Code Block it shows a strange symbol in the Background Preview. The only way to show the label text correctly was using the syntax "label:text", then Ian's discarded change was re-applied by me and now we don't need to use the weird Label syntax. #4637 * DYN-3026-CodeCoverage-AnalysisFolder-CodeReview2 I applied some changes that the developer Ian Keough did like 5 years ago (HelixRenderPackage.cs -> CleanTag) but were discarded in the PR below, with the code as it is now in the Dynamo repo when creating a Label inside a Code Block it shows a strange symbol in the Background Preview. The only way to show the label text correctly was using the syntax "label:text", then Ian's discarded change was re-applied by me and now we don't need to use the weird Label syntax. #4637 * DYN-3026-CodeCoverage-AnalysisFolder-CodeReview2 I moved the LabelTests to the DynamoCoreWpfTests project inside the Libraries folder. I removed several dll in the AnalysisTests project that are producing erros in the mono.sln.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Purpose
This PR fixes addresses the problems with Graphics Perfomance. Today, the geometry is rendered everytime when the Selected property is changed.
This behavior is changed in this PR :
Created object for every node in the watch tree and attached to helix. This removes all the static declarations in the xaml.
Updated the Selected property on VisualizationManager. When a node is selected, there is an event on VM that gets triggered, which iterates on the watch tree and selects only that node.
Ian updated the shader and the "selected" behavior in the Helix.
This PR has changes from https://github.com/ramramps/Dynamo/pull/3
Declarations
Updated the VM tests to handle this change.
Testing strategy
Manual Testing required should be done with large graphs that involves points, lines and meshes.
Also, testing is required on top of Revit.
Important
This PR includes this work, which should be reviewed as part of this review. Due to interdependency of functionality in these two PRs it was required that they be merged before the final PR to master.
Reviewers