DYN-5125: Fix for modified input nodes that are assigned non-primitiv…

…e values (#13136)

* force execute modified input node

* check for executing graph node to be non-null

* attempt new approach

* cleanup

* more cleanup

* simplify further

* final cleanup

* add test

* hide test node

* cleanup

* cleanup

* cleanup

src/Engine/ProtoAssociative/CodeGen.cs

@@ -5415,11 +5415,24 @@ private void EmitBinaryExpressionNode(AssociativeNode node, ref ProtoCore.Type i
 
                         if (bnode.IsInputExpression)
                         {
+
+                            // Emit the following instructions:
+                            // pop lx
+                            // <-- mark this pc as graphnode's start pc if pushed value is a primitive
+                            // push lx
+
                             StackValue regLX = StackValue.BuildRegister(Registers.LX);
                             EmitInstrConsole(kw.pop, kw.regLX);
                             EmitPopUpdateInstruction(regLX, bnode.OriginalAstID);
 
-                            graphNode.updateBlock.updateRegisterStartPC = pc;
+                            // In the case of the RHS being a primitive, we can skip executing instructions 
+                            // to push the updated value and pop it to the register as the register is directly updated.
+                            // In the case of a non-primitive, we let these instructions execute, so that the 
+                            // updated result from the VM can be pushed and popped to update the register.
+                            if (CoreUtils.IsPrimitiveASTNode(bnode.RightNode))
+                            {
+                                graphNode.updateBlock.updateRegisterStartPC = pc;
+                            }
 
                             EmitInstrConsole(kw.push, kw.regLX);
                             EmitPushUpdateInstruction(regLX, bnode.OriginalAstID);

src/Engine/ProtoScript/Runners/LiveRunner.cs

@@ -261,8 +261,8 @@ private void ApplyChangeSetForceExecute(ChangeSetData changeSet)
             if (changeSet.ForceExecuteASTList.Count > 0)
             {
                 // Mark all graphnodes dirty which are associated with the force exec ASTs
-                ProtoCore.AssociativeGraph.GraphNode firstDirtyNode = ProtoCore.AssociativeEngine.Utils.MarkGraphNodesDirtyAtGlobalScope
-(runtimeCore, changeSet.ForceExecuteASTList);
+                var firstDirtyNode = ProtoCore.AssociativeEngine.Utils.MarkGraphNodesDirtyAtGlobalScope(
+                    runtimeCore, changeSet.ForceExecuteASTList);
                 Validity.Assert(firstDirtyNode != null);
 
                 // If the only ASTs to execute are force exec, then set the entrypoint here.
@@ -755,18 +755,19 @@ private void UpdateCachedASTList(Subtree st, List<AssociativeNode> modifiedASTLi
                 }
                 else
                 {
+                    var unmodifiedASTs = GetUnmodifiedASTList(oldSubTree.AstNodes, st.AstNodes);
                     if (st.ForceExecution)
                     {
                         // Get the cached AST and append it to the changeSet
-                        csData.ForceExecuteASTList.AddRange(GetUnmodifiedASTList(oldSubTree.AstNodes, st.AstNodes));
+                        csData.ForceExecuteASTList.AddRange(unmodifiedASTs);
                     }
 
                     // Update the cached AST to reflect the change
 
                     List<AssociativeNode> newCachedASTList = new List<AssociativeNode>();
 
-                    // Get all the unomodified ASTs and append them to the cached ast list 
-                    newCachedASTList.AddRange(GetUnmodifiedASTList(oldSubTree.AstNodes, st.AstNodes));
+                    // Get all the unmodified ASTs and append them to the cached ast list 
+                    newCachedASTList.AddRange(unmodifiedASTs);
 
                     // Append all the modified ASTs to the cached ast list 
                     newCachedASTList.AddRange(modifiedASTList);
@@ -809,6 +810,7 @@ private IEnumerable<AssociativeNode> GetDeltaAstListModified(List<Subtree> modif
                 if (!modifiedSubTree.IsInput)
                 {
                     redefinitionAllowed = false;
+                    break;
                 }
             }
 

test/DynamoCoreTests/UpdateInputNodeModelTest.cs

@@ -0,0 +1,51 @@
+using NUnit.Framework;
+using System.Collections.Generic;
+using System.IO;
+
+namespace Dynamo.Tests
+{
+    [TestFixture]
+    internal class UpdateInputNodeModelTest : DynamoModelTestBase
+    {
+        protected override void GetLibrariesToPreload(List<string> libraries)
+        {
+            libraries.Add("DesignScriptBuiltin.dll");
+            libraries.Add("DSCoreNodes.dll");
+
+            base.GetLibrariesToPreload(libraries);
+        }
+
+        [Test]
+        public void TestUpdateInputNodeModel()
+        {
+            // DYN file contains nodemodel node that reads an image from hardcoded file name and returns a bitmap
+            string openPath = Path.Combine(TestDirectory, @"core\astbuilder\updateInputNodeModel.dyn");
+            RunModel(openPath);
+
+            // Assert on one of the color values belonging to the image read
+            var guid = "313bc594-8879-492b-aa99-8efc61c5707a";
+            AssertPreviewValue(guid, 91);
+
+            // Create backup of original file
+            var originalFile = Path.Combine(TestDirectory, @"core\astbuilder\hardcoded_image_file.jpg");
+            var backupFile = Path.Combine(TestDirectory, @"core\astbuilder\hardcoded_image_file - Copy.jpg");
+            File.Copy(originalFile, backupFile);
+
+            // Overwrite the file "harcoded_image_file.jpg" with the file "harcoded_image_file2.jpg"
+            var path = Path.Combine(TestDirectory, @"core\astbuilder\hardcoded_image_file2.jpg");
+            File.Copy(path, originalFile, true);
+
+            // Force re-execute the nodemodel node 
+            var guid2 = "a787e45f-f6ed-439f-9eb3-60008b4c8a72";
+            var nodeModel = CurrentDynamoModel.CurrentWorkspace.NodeFromWorkspace(guid2);
+            nodeModel.OnNodeModified(true);
+
+            File.Delete(originalFile);
+            // restore the file, "harcoded_image_file.jpg"
+            File.Move(backupFile, originalFile);
+
+            // Assert that the same color value has changed as the file is now pointing to a different image
+            AssertPreviewValue(guid, 41);
+        }
+    }
+}

test/TestUINodes/TestUINodes.cs

@@ -1,9 +1,13 @@
 using System;
 using System.Collections.Generic;
+using System.IO;
+using System.Reflection;
+using System.Drawing;
 using Autodesk.DesignScript.Runtime;
 using Dynamo.Graph.Nodes;
 using Newtonsoft.Json;
 using ProtoCore.AST.AssociativeAST;
+using DSCore.IO;
 
 namespace TestUINodes
 {
@@ -28,4 +32,52 @@ public override IEnumerable<AssociativeNode> BuildOutputAst(List<AssociativeNode
             throw new Exception("Dummy error message.");
         }
     }
+
+    [NodeName("Test Selection Node")]
+    [NodeCategory("TestUINodes")]
+    [NodeDescription("A test selection node.")]
+    [OutPortTypes("var")]
+    [IsDesignScriptCompatible]
+    [IsVisibleInDynamoLibrary(false)]
+    public class TestSelectionNode : NodeModel
+    {
+        public TestSelectionNode()
+        {
+            OutPorts.Add(new PortModel(PortType.Output, this, new PortData("File", "The selected file.")));
+
+            RegisterAllPorts();
+
+            ArgumentLacing = LacingStrategy.Disabled;
+
+            IsSetAsInput = true;
+        }
+
+        [JsonConstructor]
+        TestSelectionNode(IEnumerable<PortModel> inPorts, IEnumerable<PortModel> outPorts) : base(inPorts, outPorts)
+        {
+        }
+
+        public override IEnumerable<AssociativeNode> BuildOutputAst(List<AssociativeNode> inputAstNodes)
+        {
+            var executingDirectory = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
+            var directory = new DirectoryInfo(executingDirectory);
+            var testDirectory = Path.Combine(directory.Parent.Parent.Parent.Parent.FullName, "test/core/astbuilder");
+
+            string imagePath = Path.Combine(testDirectory, "hardcoded_image_file.jpg");
+            var func1 =
+                AstFactory.BuildFunctionCall(
+                    new Func<string, FileInfo>(FileSystem.FileFromPath),
+                    new List<AssociativeNode> { AstFactory.BuildStringNode(imagePath) });
+
+            var func2 = AstFactory.BuildFunctionCall(
+                new Func<FileInfo, Bitmap>(DSCore.IO.Image.ReadFromFile), new List<AssociativeNode> { func1 });
+
+            // returns an identifier that is assigned to a bitmap upon the call to Image.ReadFromFile.
+            return new[]
+            {
+                AstFactory.BuildAssignment(GetAstIdentifierForOutputIndex(0), func2),
+
+            };
+        }
+    }
 }

test/TestUINodes/TestUINodes.csproj

@@ -33,6 +33,7 @@
       <Private>False</Private>
       <Name>DynamoCore</Name>
     </ProjectReference>
+    <ProjectReference Include="..\..\src\Libraries\CoreNodes\CoreNodes.csproj" />
     <ProjectReference Include="..\..\src\Libraries\DynamoUnits\Units.csproj">
       <Project>{6E0A079E-85F1-45A1-AD5B-9855E4344809}</Project>
       <Name>Units</Name>