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.

Sign up for GitHub

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

DYN-6449 Graph Loading Performance Improving #14929

Merged
merged 3 commits into from
Feb 14, 2024
+12 −6
Merged

DYN-6449 Graph Loading Performance Improving #14929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4276e2c
DYN-6449 Graph Loading Performance Improving
RobertGlobant20 Feb 8, 2024
2091c35
DYN-6449 Graph Loading Performance Improving Code Review
RobertGlobant20 Feb 12, 2024
a548050
DYN-6449 Graph Loading Performance Improving Code Review
RobertGlobant20 Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...namoCoreWpf/NodeViewCustomization/NodeViewCustomizations/FunctionNodeViewCustomization.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System;
using System.Windows.Controls;
using System.Windows.Threading;
using Dynamo.Controls;
using Dynamo.Graph.Nodes.CustomNodes;
using Dynamo.Graph.Workspaces;
Expand Down Expand Up @@ -59,8 +61,6 @@ public void CustomizeView(Function function, NodeView nodeView)

publishCustomNodeItem.Command = nodeView.ViewModel.DynamoViewModel.PublishSelectedNodesCommand;
publishCustomNodeItem.CommandParameter = functionNodeModel;

nodeView.UpdateLayout();
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this moved to other places based on your PR description?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the PR description

}

private void EditCustomNodeProperties()
Expand Down Expand Up @@ -136,4 +136,4 @@ public void Dispose()

}
}
}
}
8 changes: 7 additions & 1 deletion src/DynamoCoreWpf/Views/Core/NodeView.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Diagnostics;
Expand Down Expand Up @@ -551,6 +551,9 @@ private void OnNodeViewMouseLeave(object sender, MouseEventArgs e)
{
ViewModel.ZIndex = oldZIndex;

//Watch nodes doesn't have Preview so we should avoid to use any method/property in PreviewControl class due that Preview is created automatically
if (ViewModel.NodeModel != null && ViewModel.NodeModel is CoreNodeModels.Watch) return;

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this only apply to watch node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, When running dotTrace I noticed that we were creating the PreviewControl for Watch node when this kind of nodes doesn't have Preview.
Probably we will need to get a list of nodes that doesn't support preview or do you know if there is already a list?

// If mouse in over node/preview control or preview control is pined, we can not hide preview control.
Copy link
Contributor

Choose a reason for hiding this comment

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

another Idea would be to stop loading controls that are not immediately visible to the user (like context menu stuff, tooltips etc) These could be loaded on demand when the user interacts with the UI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pinzart90 I think for this case we will need a specific Jira task since I noticed that just the PreviewControl is being used in several places and due to the current implementation of this property, when you use it automatically creates the instance.
For example in this piece of code when using PreviewControl.StaysOpen, the instance is created automatically:
image

This is the declaration of the Property and the references in code
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, a lot of code assumes the existence of the PreviewControl. We should create a task for it if we can say for sure it would benefit performance.
It should either be properly refactored to be optional or create it hidden for certain node types (so that it does not contribute to the layout tree calculations) Not sure about the perf results of either.

if (IsMouseOver || PreviewControl.IsMouseOver || PreviewControl.StaysOpen || IsMouseInsidePreview(e) ||
(Mouse.Captured is DragCanvas && IsMouseInsideNodeOrPreview(e.GetPosition(this)))) return;
Expand Down Expand Up @@ -731,6 +734,9 @@ internal void TogglePreviewControlAllowance()
{
previewEnabled = !previewEnabled;

//Watch nodes doesn't have Preview so we should avoid to use any method/property in PreviewControl class due that Preview is created automatically
if (ViewModel.NodeModel != null && ViewModel.NodeModel is CoreNodeModels.Watch) return;

if (previewEnabled == false && !PreviewControl.StaysOpen)
{
if (PreviewControl.IsExpanded)
Expand Down
4 changes: 2 additions & 2 deletions src/Libraries/PythonNodeModelsWpf/PythonNode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Input;
using System.Windows.Threading;
using Dynamo.Configuration;
using Dynamo.Controls;
using Dynamo.Graph.Nodes;
Expand Down Expand Up @@ -99,8 +100,7 @@ public void CustomizeView(PythonNode nodeModel, NodeView nodeView)
PythonEngineManager.Instance.AvailableEngines.CollectionChanged += PythonEnginesChanged;

nodeView.MainContextMenu.Items.Add(learnMoreItem);

nodeView.UpdateLayout();


nodeView.MouseDown += View_MouseDown;
nodeModel.DeletionStarted += NodeModel_DeletionStarted;
Expand Down
Loading