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-7838] Color range Node: Show Input ports with default value #15715

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+96 −5
Open

[DYN-7838] Color range Node: Show Input ports with default value #15715

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
23520a9
Update ColorRange.cs
ivaylo-matov Dec 9, 2024
d6e0e0f
Update NodeViewTests.cs
ivaylo-matov Dec 9, 2024
0748265
Update ColorRange.cs
ivaylo-matov Dec 9, 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
71 changes: 69 additions & 2 deletions src/Libraries/CoreNodeModels/ColorRange.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ namespace CoreNodeModels
[AlsoKnownAs("DSCoreNodesUI.ColorRange")]
public class ColorRange : NodeModel
{
private List<Color> _defaultColors;
QilongTang marked this conversation as resolved.
Show resolved Hide resolved
private List<Color> DefaultColors => _defaultColors ??= DefaultColorRanges.Analysis.ToList();
Copy link
Member

@mjkkirschner mjkkirschner Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

it does not look like you use any dynamic resizing list functionality here, is there a reason you use this type here and in the create helpers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CreateParametersForColors() was already accepting List and didn't want to chage existing code unless I really have to. Hepe the latest commit is okay.


private AssociativeNode _defaultColorsNode;
private AssociativeNode DefaultColorsNode => _defaultColorsNode ??= CreateDefaultColorsNode(DefaultColors);

private AssociativeNode _defaultIndicesNode;
private AssociativeNode DefaultIndicesNode => _defaultIndicesNode ??= CreateDefaultIndicesNode(DefaultColors);

public event Action RequestChangeColorRange;
protected virtual void OnRequestChangeColorRange()
{
Expand All @@ -42,6 +51,18 @@ protected virtual void OnRequestChangeColorRange()
[JsonConstructor]
private ColorRange(IEnumerable<PortModel> inPorts, IEnumerable<PortModel> outPorts) : base(inPorts, outPorts)
{
if (inPorts.Count() == 3 && outPorts.Count() == 1)
Copy link
Member

@mjkkirschner mjkkirschner Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@QilongTang I think we should come up with something better than this pattern - for example default value attributes or something where the author only needs to specify the default value once and not need to consider it in their json constructor unless they want to.

{
inPorts.ElementAt(0).DefaultValue = DefaultColorsNode;
inPorts.ElementAt(1).DefaultValue = DefaultIndicesNode;
}
else
{
// If information from json does not look correct, clear the default ports and add ones with default value
InPorts.Clear();
InitializePorts();
}

this.PropertyChanged += ColorRange_PropertyChanged;
foreach (var port in InPorts)
{
Expand All @@ -51,6 +72,8 @@ private ColorRange(IEnumerable<PortModel> inPorts, IEnumerable<PortModel> outPor

public ColorRange()
{
// Initialize default values of the ports
InitializePorts();
RegisterAllPorts();

this.PropertyChanged += ColorRange_PropertyChanged;
Expand All @@ -60,6 +83,14 @@ public ColorRange()
}
}

private void InitializePorts()
{
InPorts.Add(new PortModel(PortType.Input, this, new PortData("colors", Resources.ColorRangePortDataColorsToolTip, DefaultColorsNode)));
InPorts.Add(new PortModel(PortType.Input, this, new PortData("indices", Resources.ColorRangePortDataIndicesToolTip, DefaultIndicesNode)));
InPorts.Add(new PortModel(PortType.Input, this, new PortData("value", Resources.ColorRangePortDataValueToolTip)));
OutPorts.Add(new PortModel(PortType.Output, this, new PortData("color", Resources.ColorRangePortDataResultToolTip)));
}

void Connectors_CollectionChanged(object sender, System.Collections.Specialized.NotifyCollectionChangedEventArgs e)
{
OnRequestChangeColorRange();
Expand Down Expand Up @@ -119,7 +150,7 @@ public ColorRange1D ComputeColorRange(EngineController engine)
List<double> parameters;

// If there are colors supplied
if (InPorts[0].IsConnected)
if (InPorts[0].Connectors.Any())
Copy link
Member

Choose a reason for hiding this comment

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

?

{
var colorsNode = InPorts[0].Connectors[0].Start.Owner;
var colorsIndex = InPorts[0].Connectors[0].Start.Index;
Expand All @@ -131,10 +162,14 @@ public ColorRange1D ComputeColorRange(EngineController engine)
{
colors = new List<Color>();
colors.AddRange(DefaultColorRanges.Analysis);

// Create an AssociativeNode for the default colors
InPorts[0].DefaultValue = DefaultColorsNode;
InPorts[0].UsingDefaultValue = true;
}

// If there are indices supplied
if (InPorts[1].IsConnected)
if (InPorts[1].Connectors.Any())
{
var valuesNode = InPorts[1].Connectors[0].Start.Owner;
var valuesIndex = InPorts[1].Connectors[0].Start.Index;
Expand All @@ -145,11 +180,43 @@ public ColorRange1D ComputeColorRange(EngineController engine)
else
{
parameters = CreateParametersForColors(colors);

// Create an AssociativeNode for the default indices
InPorts[1].DefaultValue = DefaultIndicesNode;
InPorts[1].UsingDefaultValue = true;
}

return ColorRange1D.ByColorsAndParameters(colors, parameters);
}

private AssociativeNode CreateDefaultColorsNode(List<Color> defaultColors)
{
return AstFactory.BuildExprList(
defaultColors.Select(color =>
AstFactory.BuildFunctionCall(
new Func<int, int, int, int, Color>(DSCore.Color.ByARGB),
new List<AssociativeNode>
{
AstFactory.BuildIntNode(color.Red),
AstFactory.BuildIntNode(color.Green),
AstFactory.BuildIntNode(color.Blue)
}
)
).ToList()
);
}

private AssociativeNode CreateDefaultIndicesNode(List<Color> defaultColors)
{
var parameters = CreateParametersForColors(defaultColors);

return AstFactory.BuildExprList(
parameters.Select(AstFactory.BuildDoubleNode)
.Cast<AssociativeNode>()
.ToList()
);
}

private static List<double> CreateParametersForColors(List<Color> colors)
{
var parameters = new List<double>();
Expand Down
28 changes: 26 additions & 2 deletions test/DynamoCoreWpfTests/NodeViewTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,8 +584,8 @@ public void TestPortColors_NodeModel()
Assert.AreEqual(3, portVMs.Count);
Assert.AreEqual(1, outPorts.Count);

Assert.AreEqual(InPortViewModel.PortValueMarkerRed.Color, (portVMs[0] as InPortViewModel).PortValueMarkerColor.Color);
Assert.AreEqual(InPortViewModel.PortValueMarkerRed.Color, (portVMs[1] as InPortViewModel).PortValueMarkerColor.Color);
Assert.AreEqual(InPortViewModel.PortValueMarkerBlue.Color, (portVMs[0] as InPortViewModel).PortValueMarkerColor.Color);
Assert.AreEqual(InPortViewModel.PortValueMarkerBlue.Color, (portVMs[1] as InPortViewModel).PortValueMarkerColor.Color);
Assert.AreEqual(InPortViewModel.PortValueMarkerBlue.Color, (portVMs[2] as InPortViewModel).PortValueMarkerColor.Color);

Assert.False((outPorts[0] as OutPortViewModel).PortDefaultValueMarkerVisible);
Expand Down Expand Up @@ -670,5 +670,29 @@ public void TestSelectNeighborPins()

Assert.AreEqual(5, countAfter);
}
[Test]
public void ColorRange_InputPortsShowBlueIndicatorForDefaultValues()
{
Open(@"UI\color_range_ports.dyn");

var colorRangeVM = NodeViewWithGuid("423d7eaf-9308-4129-b11f-14c186fa4279");

var inPorts = colorRangeVM.ViewModel.InPorts;

// Assert that the first two input ports show blue markers on graph loaded.
Assert.AreEqual(InPortViewModel.PortValueMarkerBlue.Color, (inPorts[0] as InPortViewModel).PortValueMarkerColor.Color);
Assert.AreEqual(InPortViewModel.PortValueMarkerBlue.Color, (inPorts[1] as InPortViewModel).PortValueMarkerColor.Color);
Assert.IsTrue((inPorts[0] as InPortViewModel).PortDefaultValueMarkerVisible);
Assert.IsTrue((inPorts[1] as InPortViewModel).PortDefaultValueMarkerVisible);

Run();
DispatcherUtil.DoEvents();

// Assert that input ports retain default value indicators after graph execution.
Assert.AreEqual(InPortViewModel.PortValueMarkerBlue.Color, (inPorts[0] as InPortViewModel).PortValueMarkerColor.Color);
Assert.AreEqual(InPortViewModel.PortValueMarkerBlue.Color, (inPorts[1] as InPortViewModel).PortValueMarkerColor.Color);
Assert.IsTrue((inPorts[0] as InPortViewModel).PortDefaultValueMarkerVisible);
Assert.IsTrue((inPorts[1] as InPortViewModel).PortDefaultValueMarkerVisible);
}
}
}
Loading