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.

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

Improved zoom and navigation performance for large graphs with many nodes #15749

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 21 additions & 0 deletions src/DynamoCoreWpf/UI/Converters.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,27 @@

namespace Dynamo.Controls
{
public class BooleanStyleConverter : IValueConverter
{
public Style TrueStyle { get; set; }
public Style FalseStyle { get; set; }

public object Convert(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
{
if (value is bool v && v)
{
return TrueStyle;
}

return FalseStyle;
}

public object ConvertBack(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
{
return false;
Copy link
Member

Choose a reason for hiding this comment

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

why should this always return false?

}
}

public class ToolTipFirstLineOnly : IValueConverter
{
public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
Expand Down
191 changes: 135 additions & 56 deletions src/DynamoCoreWpf/UI/Themes/Modern/DynamoModern.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
xmlns:fa="clr-namespace:FontAwesome5;assembly=FontAwesome5.Net"
xmlns:nodes="clr-namespace:Dynamo.Nodes;assembly=DynamoCoreWpf"
xmlns:p="clr-namespace:Dynamo.Wpf.Properties;assembly=DynamoCoreWpf"
xmlns:ui="clr-namespace:Dynamo.UI;assembly=DynamoCoreWpf">
xmlns:ui="clr-namespace:Dynamo.UI;assembly=DynamoCoreWpf"
xmlns:conv="clr-namespace:Dynamo.Controls;assembly=DynamoCoreWpf">

<ResourceDictionary.MergedDictionaries>
<ui:SharedResourceDictionary Source="{x:Static ui:SharedDictionaryManager.DynamoConvertersDictionaryUri}" />
Expand Down Expand Up @@ -867,9 +868,13 @@
</Style>

<!-- Zoom fade text -->
<Style x:Key="SZoomFadeText" TargetType="{x:Type TextBlock}">
<Style
x:Key="SZoomFadeBase_Animation"
TargetType="{x:Type FrameworkElement}">
Copy link
Member

Choose a reason for hiding this comment

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

why the change of target type? Is the comment still accurate?

<Style.Triggers>
<DataTrigger Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}" Value="true">
<DataTrigger
Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}"
Value="true">
<DataTrigger.EnterActions>
<BeginStoryboard>
<Storyboard>
Expand All @@ -892,36 +897,36 @@
</Style.Triggers>
</Style>

<!-- Zoom fade label -->
<Style x:Key="SZoomFadeLabel" TargetType="{x:Type Label}">
<Style
x:Key="SZoomFadeBase_Basic"
TargetType="{x:Type FrameworkElement}">
<Setter
Property="Opacity"
Value="0" />
<Style.Triggers>
<DataTrigger Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}" Value="true">
<DataTrigger.EnterActions>
<BeginStoryboard>
<Storyboard>
<DoubleAnimation Storyboard.TargetProperty="Opacity"
To="1.0"
Duration="0:0:0.2" />
</Storyboard>
</BeginStoryboard>
</DataTrigger.EnterActions>
<DataTrigger.ExitActions>
<BeginStoryboard>
<Storyboard>
<DoubleAnimation Storyboard.TargetProperty="Opacity"
To="0.0"
Duration="0:0:0.2" />
</Storyboard>
</BeginStoryboard>
</DataTrigger.ExitActions>
<DataTrigger
Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}"
Value="true">
<Setter
Property="Opacity"
Value="1" />
</DataTrigger>
</Style.Triggers>
</Style>

<conv:BooleanStyleConverter
x:Key="SZoomFadeControl"
TrueStyle="{StaticResource SZoomFadeBase_Basic}"
FalseStyle="{StaticResource SZoomFadeBase_Animation}" />

<!-- Zoom fade preview -->
<Style x:Key="SZoomFadePreview" TargetType="{x:Type Border}">
<Style
x:Key="SZoomFadePreview_Animation"
TargetType="{x:Type FrameworkElement}">
<Style.Triggers>
<DataTrigger Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}" Value="true">
<DataTrigger
Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}"
Value="true">
<DataTrigger.EnterActions>
<BeginStoryboard>
<Storyboard>
Expand All @@ -944,10 +949,36 @@
</Style.Triggers>
</Style>

<Style
x:Key="SZoomFadePreview_Basic"
TargetType="{x:Type FrameworkElement}">
<Setter
Property="Opacity"
Value="0.7" />
<Style.Triggers>
<DataTrigger
Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}"
Value="true">
<Setter
Property="Opacity"
Value="0.4" />
</DataTrigger>
</Style.Triggers>
</Style>

<conv:BooleanStyleConverter
x:Key="SZoomFadePreview"
TrueStyle="{StaticResource SZoomFadePreview_Basic}"
FalseStyle="{StaticResource SZoomFadePreview_Animation}" />

<!-- Zoom fade-in preview -->
<Style x:Key="SZoomFadeInPreview" TargetType="{x:Type Border}">
<Style
x:Key="SZoomFadeInPreview_Animation"
TargetType="{x:Type FrameworkElement}">
<Style.Triggers>
<DataTrigger Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}" Value="true">
<DataTrigger
Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}"
Value="true">
<DataTrigger.EnterActions>
<BeginStoryboard>
<Storyboard>
Expand All @@ -970,10 +1001,36 @@
</Style.Triggers>
</Style>

<!-- Zoom fade-in preview -->
<Style x:Key="SZoomFadeOutPreview" TargetType="{x:Type Border}">
<Style
x:Key="SZoomFadeInPreview_Basic"
TargetType="{x:Type FrameworkElement}">
<Setter
Property="Opacity"
Value="0.5" />
<Style.Triggers>
<DataTrigger Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}" Value="true">
<DataTrigger
Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}"
Value="true">
<Setter
Property="Opacity"
Value="0.0" />
</DataTrigger>
</Style.Triggers>
</Style>

<conv:BooleanStyleConverter
x:Key="SZoomFadeInPreview"
TrueStyle="{StaticResource SZoomFadeInPreview_Basic}"
FalseStyle="{StaticResource SZoomFadeInPreview_Animation}" />

<!-- Zoom fade-out preview -->
<Style
x:Key="SZoomFadeOutPreview_Animation"
TargetType="{x:Type FrameworkElement}">
<Style.Triggers>
<DataTrigger
Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}"
Value="true">
<DataTrigger.EnterActions>
<BeginStoryboard>
<Storyboard>
Expand All @@ -996,36 +1053,36 @@
</Style.Triggers>
</Style>

<!-- Zoom fade-out framework element -->
<Style x:Key="SZoomFadeOutFrameworkElement" TargetType="{x:Type FrameworkElement}">
<Style
x:Key="SZoomFadeOutPreview_Basic"
TargetType="{x:Type FrameworkElement}">
<Setter
Property="Opacity"
Value="0.0" />
<Style.Triggers>
<DataTrigger Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}" Value="true">
<DataTrigger.EnterActions>
<BeginStoryboard>
<Storyboard>
<DoubleAnimation Storyboard.TargetProperty="Opacity"
To="1.0"
Duration="0:0:0.2" />
</Storyboard>
</BeginStoryboard>
</DataTrigger.EnterActions>
<DataTrigger.ExitActions>
<BeginStoryboard>
<Storyboard>
<DoubleAnimation Storyboard.TargetProperty="Opacity"
To="0.0"
Duration="0:0:0.2" />
</Storyboard>
</BeginStoryboard>
</DataTrigger.ExitActions>
<DataTrigger
Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}"
Value="true">
<Setter
Property="Opacity"
Value="0.5" />
</DataTrigger>
</Style.Triggers>
</Style>

<conv:BooleanStyleConverter
x:Key="SZoomFadeOutPreview"
TrueStyle="{StaticResource SZoomFadeOutPreview_Basic}"
FalseStyle="{StaticResource SZoomFadeOutPreview_Animation}" />

<!-- Zoom fade-in framework element -->
<Style x:Key="SZoomFadeInFrameworkElement" TargetType="{x:Type FrameworkElement}">
<Style
x:Key="SZoomFadeInControl_Animation"
TargetType="{x:Type FrameworkElement}">
<Style.Triggers>
<DataTrigger Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}" Value="true">
<DataTrigger
Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}"
Value="true">
<DataTrigger.EnterActions>
<BeginStoryboard>
<Storyboard>
Expand All @@ -1048,6 +1105,28 @@
</Style.Triggers>
</Style>

<Style
x:Key="SZoomFadeInControl_Basic"
TargetType="{x:Type FrameworkElement}">
<Setter
Property="Opacity"
Value="1" />
<Style.Triggers>
<DataTrigger
Binding="{Binding Path=DataContext.Zoom, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource ZoomToBooleanConverter}}"
Value="true">
<Setter
Property="Opacity"
Value="0.0" />
</DataTrigger>
</Style.Triggers>
</Style>

<conv:BooleanStyleConverter
x:Key="SZoomFadeInControl"
TrueStyle="{StaticResource SZoomFadeInControl_Basic}"
FalseStyle="{StaticResource SZoomFadeInControl_Animation}" />

<Style x:Key="TextButtonStyle" TargetType="Button">
<Setter Property="HorizontalAlignment" Value="Right" />
<Setter Property="Cursor" Value="Hand" />
Expand Down Expand Up @@ -1800,7 +1879,7 @@
FontSize="{TemplateBinding FontSize}"
FontWeight="Bold"
Foreground="{TemplateBinding Foreground}"
Style="{StaticResource SZoomFadeText}"
Style="{Binding Path=DataContext.StopAnimations, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource SZoomFadeControl}}"
Copy link
Member

Choose a reason for hiding this comment

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

I think this has the potential to be very slow given there are a bunch of elements that are making these find ancestor calls - maybe we can do something like - pass this down to the nodeView data context and have all elements reference that data context instead of searching all the way back up to the workspace view?

Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to to profile this somehow for a very large graph, thousands of nodes? I want to make sure we don't fix one issue only to introduce another.

I see that this PR adds a lot of additional find Ancestor calls.

Copy link
Member

Choose a reason for hiding this comment

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

maybe a simpler approach would be to just make this property static... I think you can then bind directly to it without the ancestor lookup.

Text="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=Content}" />
</Border>
</Grid>
Expand Down Expand Up @@ -1845,7 +1924,7 @@
FontSize="28px"
FontWeight="Bold"
Foreground="#999999"
Style="{StaticResource SZoomFadeText}"
Style="{Binding Path=DataContext.StopAnimations, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controls:WorkspaceView}}, Converter={StaticResource SZoomFadeControl}}"
Text="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=Content}" />
</Border>
</Grid>
Expand Down
7 changes: 4 additions & 3 deletions src/DynamoCoreWpf/UI/Themes/Modern/Ports.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
xmlns:p="clr-namespace:Dynamo.Wpf.Properties;assembly=DynamoCoreWpf"
xmlns:ui="clr-namespace:Dynamo.UI;assembly=DynamoCoreWpf"
xmlns:viewModels="clr-namespace:Dynamo.ViewModels;assembly=DynamoCoreWpf"
xmlns:views="clr-namespace:Dynamo.UI.Views;assembly=DynamoCoreWpf">
xmlns:views="clr-namespace:Dynamo.UI.Views;assembly=DynamoCoreWpf"
xmlns:controlsWpf="clr-namespace:Dynamo.Views;assembly=DynamoCoreWpf">

<!-- Templates

Expand Down Expand Up @@ -125,7 +126,7 @@
FontWeight="Medium"
Foreground="{StaticResource PrimaryCharcoal200Brush}"
IsHitTestVisible="False"
Style="{StaticResource SZoomFadeLabel}" />
Style="{Binding Path=DataContext.StopAnimations, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controlsWpf:WorkspaceView}}, Converter={StaticResource SZoomFadeControl}}" />
<Grid.Style>
<Style TargetType="Grid">
<Setter Property="Height" Value="{Binding Path=Height}" />
Expand Down Expand Up @@ -326,7 +327,7 @@
Foreground="Transparent"
IsHitTestVisible="False"
Opacity="0.4"
Style="{StaticResource SZoomFadeText}"
Style="{Binding Path=DataContext.StopAnimations, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type controlsWpf:WorkspaceView}}, Converter={StaticResource SZoomFadeControl}}"
Text="{Binding Path=PortName}" />
<dynui:UseLevelSpinner x:Name="useLevelControl"
Width="50"
Expand Down
10 changes: 10 additions & 0 deletions src/DynamoCoreWpf/ViewModels/Core/WorkspaceViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,10 @@ public double Zoom
}
}

[JsonIgnore]
public bool StopAnimations { get => stopAnimations; set { stopAnimations = value; RaisePropertyChanged(nameof(StopAnimations)); } }
Copy link
Member

Choose a reason for hiding this comment

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

please add /// comments

private bool stopAnimations = false;

[JsonIgnore]
public bool CanZoomIn
{
Expand Down Expand Up @@ -886,6 +890,8 @@ private void unsubscribeNodeEvents(NodeViewModel nodeViewModel)
nodeViewModel.NodeLogic.Modified -= OnNodeModified;
}

private const int MaxNodesBeforeAnimationStops = 150;

void Model_NodeRemoved(NodeModel node)
{
NodeViewModel nodeViewModel;
Expand All @@ -901,6 +907,8 @@ void Model_NodeRemoved(NodeModel node)
nodeViewModel.Dispose();

PostNodeChangeActions();

StopAnimations = Nodes.Count > MaxNodesBeforeAnimationStops;
}

void Model_NodeAdded(NodeModel node)
Expand All @@ -916,6 +924,8 @@ void Model_NodeAdded(NodeModel node)
Errors.Add(nodeViewModel.ErrorBubble);

PostNodeChangeActions();

StopAnimations = Nodes.Count > MaxNodesBeforeAnimationStops;
}

void PostNodeChangeActions()
Expand Down
Loading
Loading