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

Merge if node changes #11728

Merged
merged 9 commits into from
Jun 8, 2021
Merged

Merge if node changes #11728

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
218867e
Adding a new "IF" node that would simply return the truevalue or fals…
reddyashish Feb 16, 2021
297326b
Add list level and lacing functionality to the new "If" node. (#11598)
reddyashish Apr 13, 2021
cd80086
Add tests for the new IF node.
reddyashish May 28, 2021
b14d4af
Merge branch 'RefactoredIfNode' into MergeIfNodeChanges
reddyashish Jun 2, 2021
cd0018a
Update one of the If tests.
reddyashish Jun 3, 2021
c2c023c
Update the test.
reddyashish Jun 4, 2021
f94dd9e
Add the If node icon to the new node.
reddyashish Jun 7, 2021
0994097
Changes to the Icon image
reddyashish Jun 7, 2021
6490e1d
Update the IconUrl
reddyashish Jun 8, 2021
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
1 change: 1 addition & 0 deletions src/Engine/ProtoCore/DSASM/Defs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ public struct Constants
public const string kFunctionRangeExpression = "%generate_range";
public const string kDotMethodName = "%dot";
public const string kInlineConditionalMethodName = "%inlineconditional";
public const string kIfConditionalMethodName = "%conditionalIf";
public const string kGetTypeMethodName = "%get_type";
public const string kNodeAstFailed = "%nodeAstFailed";
public const string kWatchResultVar = "watch_result_var";
Expand Down
139 changes: 75 additions & 64 deletions src/Engine/ProtoCore/Lang/BuiltInFunctionEndPoint.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,72 +307,14 @@ public override StackValue Execute(ProtoCore.Runtime.Context c, List<StackValue>
break;
case ProtoCore.Lang.BuiltInMethods.MethodID.InlineConditional:
{
StackValue svCondition = formalParameters[0];
if (!svCondition.IsBoolean)
{
// Comment Jun: Perhaps we can allow coercion?
Type booleanType = TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.Bool, 0);
svCondition = TypeSystem.Coerce(svCondition, booleanType, runtimeCore);
if (svCondition.IsNull)
{
svCondition = StackValue.False;
}
}

StackValue svTrue = formalParameters[1];
StackValue svFalse = formalParameters[2];

// If run in delta execution environment, we don't
// create language blocks for true and false branch,
// so directly return the value.
if (runtimeCore.Options.GenerateSSA)
return svCondition.BooleanValue ? svTrue : svFalse;

Validity.Assert(svTrue.IsInteger);
Validity.Assert(svFalse.IsInteger);
int blockId = svCondition.BooleanValue ? (int)svTrue.IntegerValue : (int)svFalse.IntegerValue;
int oldRunningBlockId = runtimeCore.RunningBlock;
runtimeCore.RunningBlock = blockId;

int returnAddr = stackFrame.ReturnPC;

int ci = ProtoCore.DSASM.Constants.kInvalidIndex;
int fi = ProtoCore.DSASM.Constants.kInvalidIndex;
if (interpreter.runtime.rmem.Stack.Count >= ProtoCore.DSASM.StackFrame.StackFrameSize)
{
ci = stackFrame.ClassScope;
fi = stackFrame.FunctionScope;
}

// The class scope does not change for inline conditional calls
StackValue svThisPtr = stackFrame.ThisPtr;


int blockDecl = 0;
int blockCaller = oldRunningBlockId;
StackFrameType type = StackFrameType.LanguageBlock;
int depth = (int)interpreter.runtime.rmem.GetAtRelative(StackFrame.FrameIndexStackFrameDepth).IntegerValue;
int framePointer = rmem.FramePointer;

// Comment Jun: Calling convention data is stored on the TX register
StackValue svCallconvention = StackValue.BuildCallingConversion((int)ProtoCore.DSASM.CallingConvention.BounceType.Implicit);
interpreter.runtime.TX = svCallconvention;

List<StackValue> registers = interpreter.runtime.GetRegisters();

// Comment Jun: the caller type is the current type in the stackframe
StackFrameType callerType = stackFrame.StackFrameType;


blockCaller = runtimeCore.DebugProps.CurrentBlockId;
StackFrame bounceStackFrame = new StackFrame(svThisPtr, ci, fi, returnAddr, blockDecl, blockCaller, callerType, type, depth, framePointer, 0, registers, 0);

ret = interpreter.runtime.Bounce(blockId, 0, bounceStackFrame, 0, false, runtimeCore.CurrentExecutive.CurrentDSASMExec, runtimeCore.Breakpoints);

runtimeCore.RunningBlock = oldRunningBlockId;
ret = InlineConditionalMethod(formalParameters, stackFrame, runtimeCore, interpreter);
break;
}
case ProtoCore.Lang.BuiltInMethods.MethodID.ConditionalIf:
{
ret = InlineConditionalMethod(formalParameters, stackFrame, runtimeCore, interpreter);
break;
}

case ProtoCore.Lang.BuiltInMethods.MethodID.Dot:
ret = DotMethod(formalParameters[0], stackFrame, interpreter.runtime, c);
break;
Expand Down Expand Up @@ -544,6 +486,75 @@ public override StackValue Execute(ProtoCore.Runtime.Context c, List<StackValue>
return ret;
}

private StackValue InlineConditionalMethod(List<StackValue> formalParameters, ProtoCore.DSASM.StackFrame stackFrame, RuntimeCore runtimeCore, ProtoCore.DSASM.Interpreter interpreter)
{
RuntimeMemory rmem = runtimeCore.RuntimeMemory;
StackValue svCondition = formalParameters[0];
if (!svCondition.IsBoolean)
{
// Comment Jun: Perhaps we can allow coercion?
Type booleanType = TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.Bool, 0);
svCondition = TypeSystem.Coerce(svCondition, booleanType, runtimeCore);
if (svCondition.IsNull)
{
svCondition = StackValue.False;
}
}

StackValue svTrue = formalParameters[1];
StackValue svFalse = formalParameters[2];

// If run in delta execution environment, we don't
// create language blocks for true and false branch,
// so directly return the value.
if (runtimeCore.Options.GenerateSSA)
return svCondition.BooleanValue ? svTrue : svFalse;

Validity.Assert(svTrue.IsInteger);
Validity.Assert(svFalse.IsInteger);
int blockId = svCondition.BooleanValue ? (int)svTrue.IntegerValue : (int)svFalse.IntegerValue;
int oldRunningBlockId = runtimeCore.RunningBlock;
runtimeCore.RunningBlock = blockId;

int returnAddr = stackFrame.ReturnPC;

int ci = ProtoCore.DSASM.Constants.kInvalidIndex;
int fi = ProtoCore.DSASM.Constants.kInvalidIndex;
if (interpreter.runtime.rmem.Stack.Count >= ProtoCore.DSASM.StackFrame.StackFrameSize)
{
ci = stackFrame.ClassScope;
fi = stackFrame.FunctionScope;
}

// The class scope does not change for inline conditional calls
StackValue svThisPtr = stackFrame.ThisPtr;


int blockDecl = 0;
int blockCaller = oldRunningBlockId;
StackFrameType type = StackFrameType.LanguageBlock;
int depth = (int)interpreter.runtime.rmem.GetAtRelative(StackFrame.FrameIndexStackFrameDepth).IntegerValue;
int framePointer = rmem.FramePointer;

// Comment Jun: Calling convention data is stored on the TX register
StackValue svCallconvention = StackValue.BuildCallingConversion((int)ProtoCore.DSASM.CallingConvention.BounceType.Implicit);
interpreter.runtime.TX = svCallconvention;

List<StackValue> registers = interpreter.runtime.GetRegisters();

// Comment Jun: the caller type is the current type in the stackframe
StackFrameType callerType = stackFrame.StackFrameType;


blockCaller = runtimeCore.DebugProps.CurrentBlockId;
StackFrame bounceStackFrame = new StackFrame(svThisPtr, ci, fi, returnAddr, blockDecl, blockCaller, callerType, type, depth, framePointer, 0, registers, 0);

StackValue ret = interpreter.runtime.Bounce(blockId, 0, bounceStackFrame, 0, false, runtimeCore.CurrentExecutive.CurrentDSASMExec, runtimeCore.Breakpoints);

runtimeCore.RunningBlock = oldRunningBlockId;
return ret;
}

private StackValue DotMethod(StackValue lhs, StackFrame stackFrame, DSASM.Executive runtime, Context context)
{
var runtimeCore = runtime.RuntimeCore;
Expand Down
14 changes: 14 additions & 0 deletions src/Engine/ProtoCore/Lang/BuiltInMethods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public enum MethodID
Evaluate,
NodeAstFailed,
GC,
ConditionalIf,
}

private static string[] methodNames = new string[]
Expand Down Expand Up @@ -128,6 +129,7 @@ public enum MethodID
"Evaluate", // kEvaluateFunctionPointer
Constants.kNodeAstFailed, // kNodeAstFailed
"__GC", // kGC
Constants.kIfConditionalMethodName,
};

public static string GetMethodName(MethodID id)
Expand Down Expand Up @@ -751,6 +753,18 @@ public BuiltInMethods(Core core)
},
ID = BuiltInMethods.MethodID.InlineConditional
},

new BuiltInMethod
{
ReturnType = TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.Var, Constants.kArbitraryRank),
Parameters = new List<KeyValuePair<string, ProtoCore.Type>>
{
new KeyValuePair<string, ProtoCore.Type>("condition", TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.Bool, 0)),
new KeyValuePair<string, ProtoCore.Type>("dyn1", TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.Var, Constants.kArbitraryRank)),
new KeyValuePair<string, ProtoCore.Type>("dyn2", TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.Var, Constants.kArbitraryRank))
},
ID = BuiltInMethods.MethodID.ConditionalIf
},

new BuiltInMethod
{
Expand Down
4 changes: 2 additions & 2 deletions src/Libraries/CoreNodeModels/CoreNodeModelsImages.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@
KBgFo2CIAwYGADGe0Dftz7JCAAAAAElFTkSuQmCC
</value>
</data>
<data name="CoreNodeModels.Logic.If.Large" type="System.Drawing.Bitmap, System.Drawing" mimetype="application/x-microsoft.net.object.bytearray.base64">
<data name="CoreNodeModels.Logic.RefactoredIf.Large" type="System.Drawing.Bitmap, System.Drawing" mimetype="application/x-microsoft.net.object.bytearray.base64">
<value>
iVBORw0KGgoAAAANSUhEUgAAAGAAAABgCAYAAADimHc4AAAABGdBTUEAALGPC/xhBQAAABl0RVh0U29m
dHdhcmUAQWRvYmUgSW1hZ2VSZWFkeXHJZTwAAAMNSURBVHhe7ZfNbRsxFAZVgktICS7BOQbIwSWklHRg
Expand All @@ -768,7 +768,7 @@
L0B4qfoOUL6IiIiIiIiIiIiIiIiIiIiIiIiIyG73B7BuavcidkHUAAAAAElFTkSuQmCC
</value>
</data>
<data name="CoreNodeModels.Logic.If.Small" type="System.Drawing.Bitmap, System.Drawing" mimetype="application/x-microsoft.net.object.bytearray.base64">
<data name="CoreNodeModels.Logic.RefactoredIf.Small" type="System.Drawing.Bitmap, System.Drawing" mimetype="application/x-microsoft.net.object.bytearray.base64">
<value>
iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAYAAABzenr0AAAABGdBTUEAALGPC/xhBQAAABl0RVh0U29m
dHdhcmUAQWRvYmUgSW1hZ2VSZWFkeXHJZTwAAADcSURBVFhH7ZPRCQIxEERTwpViCfopWERKsARrsJEr
Expand Down
84 changes: 82 additions & 2 deletions src/Libraries/CoreNodeModels/Logic/If.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;
using System.Linq;
using Autodesk.DesignScript.Runtime;
using CoreNodeModels.Properties;
using Dynamo.Graph.Nodes;
using Newtonsoft.Json;
Expand All @@ -14,6 +16,8 @@ namespace CoreNodeModels.Logic
[OutPortTypes("Function")]
[IsDesignScriptCompatible]
[AlsoKnownAs("DSCoreNodesUI.Logic.If")]
[IsVisibleInDynamoLibrary(false)]
[Obsolete("This node will be removed in a future version of Dynamo. Please use the new 'If' node instead.")]
public class If : NodeModel
{
[JsonConstructor]
Expand All @@ -32,13 +36,15 @@ public If()

public override IEnumerable<AssociativeNode> BuildOutputAst(List<AssociativeNode> inputAstNodes)
{
Warning(Resources.IFNodeWarningMessage, true);

var lhs = GetAstIdentifierForOutputIndex(0);
AssociativeNode rhs;

if (IsPartiallyApplied)
{
var connectedInputs = Enumerable.Range(0, InPorts.Count)
.Where(index=>InPorts[index].IsConnected)
.Where(index => InPorts[index].IsConnected)
.Select(x => new IntNode(x) as AssociativeNode)
.ToList();
var functionNode = new IdentifierNode(Constants.kInlineConditionalMethodName);
Expand Down Expand Up @@ -72,4 +78,78 @@ public override IEnumerable<AssociativeNode> BuildOutputAst(List<AssociativeNode
};
}
}

[NodeName("If")]
[NodeCategory(BuiltinNodeCategories.LOGIC)]
[NodeDescription("ScopeIfDescription", typeof(Resources))]
[OutPortTypes("Function")]
[IsDesignScriptCompatible]
[AlsoKnownAs("DSCoreNodesUI.Logic.If")]
public class RefactoredIf : NodeModel
Copy link
Contributor Author

@reddyashish reddyashish Jun 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Since this is going to be a public class, is "RefactoredIf" name fine with everyone? Are we going to obsolete the old node in 3.0 and then rename this class name to "If"?

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 more descriptive name and summary? Is there any reason not to obsolete the old node right now?

Copy link
Contributor Author

@reddyashish reddyashish Jun 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry I meant, if we are going to remove this old node completely in 3.0. Currently the old one is obsoleted and the new node has a summary under BuildOutputAst(). Do you want me to add any other information?

{
[JsonConstructor]
private RefactoredIf(IEnumerable<PortModel> inPorts, IEnumerable<PortModel> outPorts) : base(inPorts, outPorts)
{
ArgumentLacing = LacingStrategy.Auto;
}

public RefactoredIf()
{
ArgumentLacing = LacingStrategy.Auto;
InPorts.Add(new PortModel(PortType.Input, this, new PortData("test", Resources.PortDataTestBlockToolTip)));
InPorts.Add(new PortModel(PortType.Input, this, new PortData("true", Resources.PortDataTrueBlockToolTip)));
InPorts.Add(new PortModel(PortType.Input, this, new PortData("false", Resources.PortDataFalseBlockToolTip)));

OutPorts.Add(new PortModel(PortType.Output, this, new PortData("result", Resources.PortDataResultToolTip)));

RegisterAllPorts();
}

/// <summary>
/// This node will translate the following DS code into an AST output for the If NodeModel node.
/// result = [trueValue, falseValue][boolCondition ? 0 : 1]
/// This node will just return the trueValue or the falseValue based on the input condition
/// and would be different to the conditional "If" node that was present before.
/// </summary>
/// <param name="inputAstNodes"> List of input AST nodes. </param>
/// <returns></returns>
public override IEnumerable<AssociativeNode> BuildOutputAst(List<AssociativeNode> inputAstNodes)
{
var lhs = GetAstIdentifierForOutputIndex(0);
AssociativeNode rhs;

if (IsPartiallyApplied)
{
var connectedInputs = Enumerable.Range(0, InPorts.Count)
.Where(index => InPorts[index].IsConnected)
.Select(x => new IntNode(x) as AssociativeNode)
.ToList();

var functionNode = new IdentifierNode(Constants.kIfConditionalMethodName);
var paramNumNode = new IntNode(3);
var positionNode = AstFactory.BuildExprList(connectedInputs);
var arguments = AstFactory.BuildExprList(inputAstNodes);
var inputParams = new List<AssociativeNode>
{
functionNode,
paramNumNode,
positionNode,
arguments,
AstFactory.BuildBooleanNode(true)
};

rhs = AstFactory.BuildFunctionCall("__CreateFunctionObject", inputParams);
}
else
{
UseLevelAndReplicationGuide(inputAstNodes);
rhs = AstFactory.BuildFunctionCall(Constants.kIfConditionalMethodName, inputAstNodes);
}

return new[]
{
AstFactory.BuildAssignment(lhs, rhs)
};
}
}
}
9 changes: 9 additions & 0 deletions src/Libraries/CoreNodeModels/Properties/Resources.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Libraries/CoreNodeModels/Properties/Resources.en-US.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,4 +614,7 @@ Default value: {0}</value>
<data name="FilePathOutputDescription" xml:space="preserve">
<value>File Path</value>
</data>
<data name="IFNodeWarningMessage" xml:space="preserve">
<value>This node has been updated and will be removed in a future version of Dynamo. Existing behavior is retained, but a new version now supports Empty Lists, Null values and inputs of varying length. Please replace this node if you wish to use this improved behavior.</value>
</data>
</root>
3 changes: 3 additions & 0 deletions src/Libraries/CoreNodeModels/Properties/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,4 +614,7 @@ Default value: {0}</value>
<data name="FilePathOutputDescription" xml:space="preserve">
<value>File Path</value>
</data>
<data name="IFNodeWarningMessage" xml:space="preserve">
<value>This node has been updated and will be removed in a future version of Dynamo. Existing behavior is retained, but a new version now supports Empty Lists, Null values and inputs of varying length. Please replace this node if you wish to use this improved behavior.</value>
</data>
</root>
0 ...geIcons/CoreNodeModels.Logic.If.Large.png → ...reNodeModels.Logic.RefactoredIf.Large.png
View file
Open in desktop
File renamed without changes
0 ...llIcons/CoreNodeModels.Logic.If.Small.png → ...reNodeModels.Logic.RefactoredIf.Small.png
View file
Open in desktop
File renamed without changes
Loading