Skip to content
Closed
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
96 changes: 89 additions & 7 deletions src/DynamoManipulation/NodeManipulator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ public abstract class NodeManipulator : INodeManipulator
private string warning = string.Empty;
private Point originBeforeMove;// The manipulator position before the user moves the gizmo
private Point originAfterMove;// The manipulator position after the user moves the gizmo
private bool hasUpdatedInputNodesDuringDrag;
private readonly HashSet<NodeModel> inputNodesUpdatedDuringDrag = new HashSet<NodeModel>();
protected const double gizmoScale = 1.2;
protected readonly int ROUND_UP_PARAM = 3;
protected readonly double MIN_OFFSET_VAL = 0.001;
Expand Down Expand Up @@ -185,6 +187,22 @@ protected virtual void MouseDown(object sender, MouseButtonEventArgs mouseButton
{
if (!IsValidNode) return;

// Skip processing if user is panning or orbiting the camera.
// This prevents the manipulator from capturing mouse events intended for camera navigation,
// including cases where pan/orbit tools use the left mouse button.
if (BackgroundPreviewViewModel is DefaultWatch3DViewModel viewModel)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this condition to check if the backgroundpreview mode is on and not the graph view mode?

{
if (viewModel.IsPanning || viewModel.IsOrbiting)
return;
}

// Only process left mouse button presses for gizmo manipulation
// Right/middle button presses are for camera navigation (pan/orbit)
if (mouseButtonEventArgs.ChangedButton != MouseButton.Left)
{
return;
}

active = UpdatePosition();
if (Origin != null )
{
Expand All @@ -209,6 +227,8 @@ protected virtual void MouseDown(object sender, MouseButtonEventArgs mouseButton
if (item.HitTest(originPt, dirVec, out hitObject))
{
GizmoInAction = item;
hasUpdatedInputNodesDuringDrag = false;
inputNodesUpdatedDuringDrag.Clear();

var nodes = OnGizmoClick(item, hitObject).ToList();
if (nodes.Any())
Expand All @@ -228,19 +248,42 @@ protected virtual void MouseDown(object sender, MouseButtonEventArgs mouseButton
/// <param name="e"></param>
protected virtual void MouseUp(object sender, MouseButtonEventArgs e)
{
// Skip processing if user is panning or orbiting the camera.
// This prevents the manipulator from capturing mouse events intended for camera navigation,
// including cases where pan/orbit tools use the left mouse button.
if (BackgroundPreviewViewModel is DefaultWatch3DViewModel viewModel)
{
if (viewModel.IsPanning || viewModel.IsOrbiting)
return;
}

// Only process left mouse button releases for gizmo manipulation
// Right/middle button releases are for camera navigation (pan/orbit)
if (e.ChangedButton != MouseButton.Left)
{
return;
}

GizmoInAction = null;

if (originBeforeMove != null && originAfterMove != null)
{
var inputNodesToManipulate = InputNodesToUpdateAfterMove(Vector.ByTwoPoints(originBeforeMove, originAfterMove));
foreach (var (inputNode, amount) in inputNodesToManipulate)
if (hasUpdatedInputNodesDuringDrag)
{
if (inputNode == null) continue;
FinalizeInputNodesAfterDrag();
}
else
{
var inputNodesToManipulate = InputNodesToUpdateAfterMove(Vector.ByTwoPoints(originBeforeMove, originAfterMove));
Copy link
Contributor

Choose a reason for hiding this comment

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

When will control reach here inside this else?

foreach (var (inputNode, amount) in inputNodesToManipulate)
{
if (inputNode == null) continue;

if (Math.Abs(amount) < MIN_OFFSET_VAL) continue;
if (Math.Abs(amount) < MIN_OFFSET_VAL) continue;

dynamic uiNode = inputNode;
uiNode.Value = Math.Round(amount, ROUND_UP_PARAM);
dynamic uiNode = inputNode;
uiNode.Value = Math.Round(amount, ROUND_UP_PARAM);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe a good idea here to check if the final value we are setting for uiNode.Value is not the same or close as the current value?

       dynamic uiNode = inputNode;
       var currentValue = (double)uiNode.Value;
       var roundedValue = Math.Round(currentValue, ROUND_UP_PARAM);
       if (Math.Abs(currentValue - roundedValue) > MIN_OFFSET_VAL)
       {
            uiNode.Value = roundedValue;
       }

}
}
}

Expand All @@ -249,7 +292,15 @@ protected virtual void MouseUp(object sender, MouseButtonEventArgs e)
foreach (var gizmo in gizmos)
{
gizmo.UpdateGizmoGraphics();

// Clear hit state to prevent drift during subsequent camera operations
// This ensures stale axis/plane hit data doesn't affect pan/orbit movements
if (gizmo is TranslationGizmo translationGizmo)
{
translationGizmo.ClearHitState();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really necessary, even after preventing mouseup or mousedown events from doing anything after a camera change event?

}
}

}

/// <summary>
Expand Down Expand Up @@ -284,9 +335,11 @@ protected virtual void MouseMove(object sender, MouseEventArgs mouseEventArgs)
// Doing this triggers a graph update on scheduler thread
OnGizmoMoved(GizmoInAction, offset);

UpdateInputNodesDuringDrag(offset);
Copy link
Contributor

Choose a reason for hiding this comment

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

Calling this function inside MouseMove seems like the changes in #12614 to improve performance are now being undone since you're now updating the graph for every incremental mouse move event rather than waiting until the mouse button is released.


// redraw manipulator at new position synchronously
var packages = BuildRenderPackage();
BackgroundPreviewViewModel.AddGeometryForRenderPackages(packages);
BackgroundPreviewViewModel.AddGeometryForRenderPackages(packages, true);
}

/// <summary>
Expand Down Expand Up @@ -384,6 +437,35 @@ protected bool CanManipulateInputNode(int inputPortIndex, out NodeModel inputNod
return new List<(NodeModel, double)>();
}

private void UpdateInputNodesDuringDrag(Vector offset)
{
var inputNodesToManipulate = InputNodesToUpdateAfterMove(offset);
foreach (var (inputNode, amount) in inputNodesToManipulate)
{
if (inputNode == null) continue;
if (Math.Abs(amount) < MIN_OFFSET_VAL) continue;

dynamic uiNode = inputNode;
uiNode.Value = amount;
Comment on lines +448 to +449
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

The code uses 'dynamic' without type checking, which could fail at runtime if inputNode doesn't have a Value property. Consider adding type validation or using a more specific interface/base type that guarantees the Value property exists.

Copilot uses AI. Check for mistakes.
inputNodesUpdatedDuringDrag.Add(inputNode);
hasUpdatedInputNodesDuringDrag = true;
}
}

private void FinalizeInputNodesAfterDrag()
{
foreach (var inputNode in inputNodesUpdatedDuringDrag)
{
if (inputNode == null) continue;

dynamic uiNode = inputNode;
uiNode.Value = Math.Round((double)uiNode.Value, ROUND_UP_PARAM);
Comment on lines +461 to +462
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

The code uses 'dynamic' without type checking, which could fail at runtime if inputNode doesn't have a Value property of type double. Consider adding type validation or using a more specific interface/base type.

Copilot uses AI. Check for mistakes.
}

inputNodesUpdatedDuringDrag.Clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should also be called in the Dispose() method, since the HashSet contains NodeModel references which will not be cleared in case of early exit from MouseUp event.

hasUpdatedInputNodesDuringDrag = false;
}

/// <summary>
/// Executes CreateAndConnectNodeCommand to create DoubleSlider and
/// connect it's output port to the input port of the node.
Expand Down
12 changes: 11 additions & 1 deletion src/DynamoManipulation/TranslationGizmo.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Windows.Media;
Expand Down Expand Up @@ -275,6 +275,16 @@ private Line GetRayGeometry(Point source, Vector direction)
/// </summary>
public CoordinateSystem ReferenceCoordinateSystem { get; set; }

/// <summary>
/// Clears the cached hit axis and plane after manipulation is complete.
/// This prevents stale hit state from affecting subsequent camera operations.
/// </summary>
public void ClearHitState()
{
hitAxis = null;
hitPlane = null;
}

/// <summary>
/// Performs hit test on Gizmo to find out hit object. The returned
/// hitObject could be either axis vector or a plane.
Expand Down
Loading
Loading