Skip to content

DYN-9823: This Visual is not connected to a PresentationSource. #16958

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

Merged
johnpierson merged 2 commits into from
Mar 20, 2026
Merged

DYN-9823: This Visual is not connected to a PresentationSource. #16958

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6decc0d
DYN-9823: This Visual is not connected to a PresentationSource.
chubakueno Mar 12, 2026
76f6aa6
add static qualifier
chubakueno Mar 12, 2026
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
38 changes: 28 additions & 10 deletions src/DynamoCoreWpf/Views/Core/NodeView.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1593,33 +1593,51 @@ private void CachedValueChanged()
}));
}

private Point PointToLocal(double x, double y, UIElement target)
private static bool TryPointToLocal(double x, double y, UIElement target, out Point result)
{
Point positionFromScreen = target.PointToScreen(new Point(x, y));
PresentationSource source = PresentationSource.FromVisual(target);
Point targetPoints = source.CompositionTarget.TransformFromDevice.Transform(positionFromScreen);
return targetPoints;
result = default;
try
{
Copy link
Copy Markdown
Contributor

@BogdanZavu BogdanZavu Mar 12, 2026

Choose a reason for hiding this comment

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

Does it make sense to do some extra checks here to avoid an exception ?
for source, source.RootVisual, source.CompositionTarget

Point positionFromScreen = target.PointToScreen(new Point(x, y));
PresentationSource source = PresentationSource.FromVisual(target);
result = source.CompositionTarget.TransformFromDevice.Transform(positionFromScreen);
return true;
}
catch
{
// DYN-9823: PointToScreen can throw when the visual is not connected to a PresentationSource
// (e.g. during layout, tab switch, or node deletion). Popup placement is non-critical,
// so just return false for any exception and let the caller handle it.
return false;
Copy link
Copy Markdown
Contributor

@BogdanZavu BogdanZavu Mar 12, 2026

Choose a reason for hiding this comment

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

So the side effect will be that we position the window in a weird position ?
Since we still continue with the window setup.
But I guess this happens so rare that we can accept this.

Copy link
Copy Markdown
Contributor Author

@chubakueno chubakueno Mar 12, 2026

Choose a reason for hiding this comment

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

Yeah the autocomplete popup could end up in a weird position if the user stops moving the node around just in time where the exception happens. It should then continue following the node if the node is moved again. So its like a single frame thing in very weird cases.

}
Comment on lines +1599 to +1612
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

TryPointToLocal currently uses a blanket catch and relies on exceptions (including NullReference from PresentationSource.FromVisual(target)/CompositionTarget) for normal control flow. This can mask unexpected failures and also adds overhead during rapid UI updates. Consider adding explicit target/source/source.CompositionTarget null checks and catching only the expected WPF exception(s) from PointToScreen (e.g., InvalidOperationException when the visual is not connected).

Suggested change
try
{
Point positionFromScreen = target.PointToScreen(new Point(x, y));
PresentationSource source = PresentationSource.FromVisual(target);
result = source.CompositionTarget.TransformFromDevice.Transform(positionFromScreen);
return true;
}
catch
{
// DYN-9823: PointToScreen can throw when the visual is not connected to a PresentationSource
// (e.g. during layout, tab switch, or node deletion). Popup placement is non-critical,
// so just return false for any exception and let the caller handle it.
return false;
}
if (target == null)
{
return false;
}
Point positionFromScreen;
try
{
positionFromScreen = target.PointToScreen(new Point(x, y));
}
catch (InvalidOperationException)
{
// DYN-9823: PointToScreen can throw when the visual is not connected to a PresentationSource
// (e.g. during layout, tab switch, or node deletion). Popup placement is non-critical,
// so just return false for this expected exception and let the caller handle it.
return false;
}
PresentationSource source = PresentationSource.FromVisual(target);
if (source?.CompositionTarget == null)
{
return false;
}
result = source.CompositionTarget.TransformFromDevice.Transform(positionFromScreen);
return true;

Copilot uses AI. Check for mistakes.
}

private void ViewModel_RequestAutoCompletePopupPlacementTarget(Window window, PortModel portModel, double spacing)
{
if (portModel.PortType == PortType.Input)
{
var portView = inputPortControl.ItemContainerGenerator.ContainerFromIndex(portModel.Index) as FrameworkElement;
window.Top = PointToLocal(0, 0, portView).Y;
window.Left = PointToLocal(0, 0, this).X - window.Width - spacing;
if (TryPointToLocal(0, 0, portView, out var portTop) && TryPointToLocal(0, 0, this, out var nodeLeft))
{
window.Top = portTop.Y;
window.Left = nodeLeft.X - window.Width - spacing;
}
}
else
{
var portView = outputPortControl.ItemContainerGenerator.ContainerFromIndex(portModel.Index) as FrameworkElement;
window.Top = PointToLocal(0, 0, portView).Y;
window.Left = PointToLocal(ActualWidth, 0, this).X + spacing;
if (TryPointToLocal(0, 0, portView, out var portTop) && TryPointToLocal(ActualWidth, 0, this, out var nodeRight))
{
window.Top = portTop.Y;
window.Left = nodeRight.X + spacing;
}
Comment on lines 1615 to +1633
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

If either TryPointToLocal call fails here, the window position is left unchanged (or stays at the default for a newly created window), but the autocomplete window may still be shown by the caller. That can result in the popup appearing in an arbitrary/stale location. Consider a fallback behavior when placement can't be computed (e.g., hide/close the window for that tick, or place it relative to the owner/workspace until a valid target is available).

Copilot uses AI. Check for mistakes.
}
}

private void ViewModel_RequestClusterAutoCompletePopupPlacementTarget(Window window, double spacing)
{
Point targetPoints = PointToLocal(0, ActualHeight, this);
if (!TryPointToLocal(0, ActualHeight, this, out var targetPoints))
return;
window.Left = Math.Clamp(targetPoints.X,
SystemParameters.VirtualScreenLeft,
SystemParameters.VirtualScreenLeft + SystemParameters.VirtualScreenWidth - window.Width);
Expand Down
Loading