Skip to content

Commit c694ed6

Browse files
authored
[SG2] Add tests for node models (#7796)
* Remove registry singleton access when model is available * Update SGGraphModelMock * Add node model test to demo SGGraphModelMock * Remove direct access of UI data in StaticPortsInspector * Fix singleton Registry references, update SGGraphModelMock * Model tests: CreateGraphDataNode, TryGetNodeHandler, existsInGraphData * Fix TryGetNodeHandler return, test get registryKey TryGetNodeHandler could return true despite assigning its out param to null, violating its contract. * Add preliminary tests for ChangeNodeFunction, reader -> handler * Naming, update selected function tests * Post-rebase fix * Add asset stub for SGGraphModelMock, write option list tests * Add remaining test cases incl. versioning * Add GetNodeHandler, use when success is implied * Add stub ShaderGraphStencilMock, split mock classes into own files
1 parent 7795b2d commit c694ed6

18 files changed

+568
-68
lines changed

com.unity.sg2/Editor/GraphUI/DataModel/SGGraphModel.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class SGGraphModel : GraphModel
2727
private bool isSubGraph = false;
2828

2929
internal GraphHandler GraphHandler => graphHandlerBox.Graph;
30-
internal ShaderGraphRegistry RegistryInstance => ShaderGraphRegistry.Instance;
30+
internal virtual ShaderGraphRegistry RegistryInstance => ShaderGraphRegistry.Instance;
3131
internal List<JsonData<Target>> Targets => targetSettingsBox.Targets; // TODO: Store the active editing target in the box?
3232
internal Target ActiveTarget => Targets.FirstOrDefault();
3333
internal SGMainPreviewModel MainPreviewData => mainPreviewModel;
@@ -57,7 +57,7 @@ class SGGraphModel : GraphModel
5757

5858
internal void Init(GraphHandler graph, bool isSubGraph, Target target)
5959
{
60-
graphHandlerBox.Init(graph);
60+
graphHandlerBox.Init(graph, RegistryInstance.Registry);
6161
this.isSubGraph = isSubGraph;
6262
if (!isSubGraph && target != null)
6363
{
@@ -78,7 +78,7 @@ internal void Init(GraphHandler graph, bool isSubGraph, Target target)
7878

7979
public override void OnEnable()
8080
{
81-
graphHandlerBox.OnEnable(false);
81+
graphHandlerBox.OnEnable(RegistryInstance.Registry, false);
8282

8383
targetSettingsBox.OnEnable();
8484
foreach (var target in Targets)

com.unity.sg2/Editor/GraphUI/DataModel/SGNodeModel.cs

Lines changed: 50 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,11 @@ public RegistryKey registryKey
5757
if (!existsInGraphData)
5858
return m_PreviewRegistryKey;
5959

60-
Assert.IsTrue(TryGetNodeHandler(out var reader));
60+
Debug.Assert(TryGetNodeHandler(out var handler));
61+
6162
// Store the registry key to use for node duplication
62-
duplicationRegistryKey = reader.GetRegistryKey();
63-
return reader.GetRegistryKey();
63+
duplicationRegistryKey = handler.GetRegistryKey();
64+
return handler.GetRegistryKey();
6465
}
6566
}
6667

@@ -80,22 +81,17 @@ public RegistryKey registryKey
8081
public bool existsInGraphData =>
8182
m_GraphDataName != null && TryGetNodeHandler(out _);
8283

83-
protected GraphHandler graphHandler =>
84+
GraphHandler graphHandler =>
8485
((SGGraphModel)GraphModel).GraphHandler;
8586

8687
ShaderGraphRegistry registry =>
87-
((ShaderGraphStencil)GraphModel.Stencil).GetRegistry();
88+
((SGGraphModel)GraphModel).RegistryInstance;
8889

8990
public bool TryGetNodeHandler(out NodeHandler reader)
9091
{
9192
try
9293
{
93-
if (graphDataName == null)
94-
{
95-
reader = registry.GetDefaultTopology(m_PreviewRegistryKey);
96-
return true;
97-
}
98-
reader = graphHandler.GetNode(graphDataName);
94+
reader = GetNodeHandler();
9995
return reader != null;
10096
}
10197
catch (Exception exception)
@@ -106,6 +102,15 @@ public bool TryGetNodeHandler(out NodeHandler reader)
106102
}
107103
}
108104

105+
public NodeHandler GetNodeHandler()
106+
{
107+
// Use the default topology handler for preview nodes.
108+
var isPreview = graphDataName == null;
109+
return isPreview ?
110+
registry.GetDefaultTopology(m_PreviewRegistryKey) :
111+
graphHandler.GetNode(graphDataName);
112+
}
113+
109114
public virtual bool HasPreview { get; private set; }
110115

111116
// By default every node's preview is visible
@@ -171,7 +176,12 @@ internal int latestAvailableVersion
171176

172177
public void UpgradeToLatestVersion()
173178
{
174-
var nodeHandler = graphHandler.GetNode(graphDataName);
179+
if (!existsInGraphData)
180+
{
181+
return;
182+
}
183+
184+
var nodeHandler = GetNodeHandler();
175185

176186
if (latestAvailableVersion < currentVersion)
177187
{
@@ -214,9 +224,15 @@ public void SetSearcherPreviewRegistryKey(RegistryKey key)
214224

215225
public void ChangeNodeFunction(string newFunctionName)
216226
{
217-
NodeHandler nodeHandler = graphHandler.GetNode(graphDataName);
218-
string fieldName = NodeDescriptorNodeBuilder.SELECTED_FUNCTION_FIELD_NAME;
219-
FieldHandler selectedFunctionField = nodeHandler.GetField<string>(fieldName);
227+
if (!existsInGraphData)
228+
{
229+
return;
230+
}
231+
232+
var nodeHandler = GetNodeHandler();
233+
var fieldName = NodeDescriptorNodeBuilder.SELECTED_FUNCTION_FIELD_NAME;
234+
var selectedFunctionField = nodeHandler.GetField<string>(fieldName);
235+
220236
if (selectedFunctionField == null)
221237
{
222238
Debug.LogError("Unable to update selected function. Node has no selected function field.");
@@ -245,7 +261,13 @@ public void ChangeNodeFunction(string newFunctionName)
245261
/// <param name="optionIndex">Index of the Option in the port's parameter descriptor to use.</param>
246262
public void SetPortOption(string portName, int optionIndex)
247263
{
248-
if (!TryGetNodeHandler(out var handler)) return;
264+
// If not backed by real data (i.e., we are a searcher preview), changing options doesn't make sense.
265+
if (!existsInGraphData)
266+
{
267+
return;
268+
}
269+
270+
var nodeHandler = GetNodeHandler();
249271
var parameterInfo = GetViewModel().GetParameterInfo(portName);
250272
var (_, optionValue) = parameterInfo.Options[optionIndex];
251273

@@ -255,7 +277,7 @@ public void SetPortOption(string portName, int optionIndex)
255277
return;
256278
}
257279

258-
var port = handler.GetPort(portName);
280+
var port = nodeHandler.GetPort(portName);
259281
var existing = GetCurrentPortOption(portName);
260282
if (existing != -1)
261283
{
@@ -277,10 +299,12 @@ public void SetPortOption(string portName, int optionIndex)
277299
/// <returns>Index into the Options list for the given port, or -1 if there are no options or no option is selected.</returns>
278300
public int GetCurrentPortOption(string portName)
279301
{
302+
if (!TryGetNodeHandler(out var handler)) return -1;
303+
if (string.IsNullOrEmpty(m_GraphDataName)) return 0; // default to first option
304+
280305
var paramInfo = GetViewModel().GetParameterInfo(portName);
281-
if (!existsInGraphData) return 0; // default to first option
306+
if (paramInfo.Options == null || paramInfo.Options.Count < 1) return -1;
282307

283-
if (!TryGetNodeHandler(out var handler)) return -1;
284308
var port = handler.GetPort(portName);
285309

286310
var connection = graphHandler.graphDelta.GetDefaultConnectionToPort(port.ID);
@@ -302,11 +326,6 @@ public void OnPreviewTextureUpdated(Texture newTexture)
302326
PreviewShaderIsCompiling = false;
303327
}
304328

305-
public void OnPreviewShaderCompiling()
306-
{
307-
PreviewShaderIsCompiling = true;
308-
}
309-
310329
SGNodeViewModel CreateNodeViewModel(NodeUIDescriptor nodeUIInfo, NodeHandler node)
311330
{
312331
var portViewModels = new List<SGPortViewModel>();
@@ -421,21 +440,18 @@ bool CreatePortViewModel(PortHandler portInfo, ParameterUIDescriptor parameter,
421440

422441
protected override void OnDefineNode()
423442
{
424-
if (!TryGetNodeHandler(out var nodeReader))
443+
if (!TryGetNodeHandler(out var nodeHandler))
425444
{
426445
Debug.LogErrorFormat("Node \"{0}\" is missing from graph data", graphDataName);
427446
return;
428447
}
429448

430-
NodeUIDescriptor nodeUIDescriptor = new();
431-
if(GraphModel.Stencil is ShaderGraphStencil shaderGraphStencil)
432-
nodeUIDescriptor = shaderGraphStencil.GetUIHints(registryKey, nodeReader);
433-
434-
bool nodeHasPreview = nodeUIDescriptor.HasPreview && existsInGraphData;
435-
m_NodeViewModel = CreateNodeViewModel(nodeUIDescriptor, nodeReader);
449+
var nodeUIDescriptor = registry.GetNodeUIDescriptor(registryKey, nodeHandler);
450+
var nodeHasPreview = nodeUIDescriptor.HasPreview && existsInGraphData;
451+
m_NodeViewModel = CreateNodeViewModel(nodeUIDescriptor, nodeHandler);
436452

437453
// TODO: Convert this to a NodePortsPart maybe?
438-
foreach (var portReader in nodeReader.GetPorts().Where(e => !e.LocalID.Contains("out_")))
454+
foreach (var portReader in nodeHandler.GetPorts().Where(e => !e.LocalID.Contains("out_")))
439455
{
440456
if (!portReader.IsHorizontal)
441457
continue;
@@ -449,7 +465,7 @@ protected override void OnDefineNode()
449465

450466
// var type = ShaderGraphTypes.GetTypeHandleFromKey(portReader.GetRegistryKey());
451467
var type = ShaderGraphExampleTypes.GetGraphType(portReader);
452-
var nodeId = nodeReader.ID;
468+
var nodeId = nodeHandler.ID;
453469
void initCallback(Constant e)
454470
{
455471
var constant = e as BaseShaderGraphConstant;
@@ -476,7 +492,7 @@ void initCallback(Constant e)
476492
var newPortModel = this.AddDataInputPort(portReader.LocalID, type, orientation: orientation, initializationCallback: initCallback);
477493
// If we were deserialized, the InitCallback doesn't get triggered.
478494
if (newPortModel != null)
479-
((BaseShaderGraphConstant)newPortModel.EmbeddedValue).Initialize(((SGGraphModel)GraphModel), nodeReader.ID.LocalPath, portReader.LocalID);
495+
((BaseShaderGraphConstant)newPortModel.EmbeddedValue).Initialize(((SGGraphModel)GraphModel), nodeHandler.ID.LocalPath, portReader.LocalID);
480496
}
481497
else
482498
this.AddDataOutputPort(portReader.LocalID, type, orientation: orientation);

com.unity.sg2/Editor/GraphUI/GraphElements/ModelUI/Inspector/StaticPortsInspector.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ protected override IEnumerable<BaseModelPropertyField> GetFields()
2525

2626
var graphModel = (SGGraphModel)nodeModel.GraphModel;
2727
var stencil = (ShaderGraphStencil)graphModel.Stencil;
28-
var nodeUIDescriptor = stencil.GetUIHints(nodeModel.registryKey, nodeReader);
28+
var nodeViewModel = nodeModel.GetViewModel();
2929

3030
foreach (var port in nodeReader.GetPorts())
3131
{
@@ -34,9 +34,9 @@ protected override IEnumerable<BaseModelPropertyField> GetFields()
3434
if (!isStatic) continue;
3535

3636
var portName = port.ID.LocalPath;
37-
var parameterUIDescriptor = nodeUIDescriptor.GetParameterInfo(portName);
37+
var portViewModel = nodeViewModel.GetParameterInfo(portName);
3838

39-
if (!parameterUIDescriptor.InspectorOnly) continue;
39+
if (!portViewModel.InspectorOnly) continue;
4040

4141
var constant = stencil.CreateConstantValue(ShaderGraphExampleTypes.GetGraphType(port));
4242
if (constant is BaseShaderGraphConstant cldsConstant)

com.unity.sg2/Editor/GraphUI/Importers/ShaderGraphAssetUtils.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -193,10 +193,9 @@ internal class SerializableGraphHandler : ISerializationCallbackReceiver
193193
GraphHandler m_graph;
194194

195195
// Provide a previously initialized graphHandler-- round-trip it for ownership.
196-
public void Init(GraphHandler value)
196+
public void Init(GraphHandler value, Registry reg)
197197
{
198198
json = value.ToSerializedFormat();
199-
var reg = ShaderGraphRegistry.Instance.Registry; // TODO: Singleton?
200199
m_graph = GraphHandler.FromSerializedFormat(json, reg);
201200
m_graph.ReconcretizeAll();
202201
}
@@ -214,9 +213,8 @@ public void OnBeforeSerialize()
214213

215214
public void OnAfterDeserialize() { }
216215

217-
public void OnEnable(bool reconcretize = true)
216+
public void OnEnable(Registry reg, bool reconcretize = true)
218217
{
219-
var reg = ShaderGraphRegistry.Instance.Registry;
220218
m_graph = GraphHandler.FromSerializedFormat(json, reg);
221219
if (reconcretize)
222220
{

com.unity.sg2/Editor/GraphUI/ShaderGraphStencil.cs

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -77,16 +77,6 @@ public override ILibraryFilterProvider GetLibraryFilterProvider()
7777
return new ShaderGraphSearcherFilterProvider();
7878
}
7979

80-
internal ShaderGraphRegistry GetRegistry()
81-
{
82-
return ShaderGraphRegistry.Instance;
83-
}
84-
85-
internal NodeUIDescriptor GetUIHints(RegistryKey nodeKey, NodeHandler node = null)
86-
{
87-
return ShaderGraphRegistry.Instance.GetNodeUIDescriptor(nodeKey, node);
88-
}
89-
9080
protected override void CreateGraphProcessors()
9181
{
9282
GetGraphProcessorContainer().AddGraphProcessor(new ShaderGraphProcessor());

com.unity.sg2/Editor/GraphUI/Utilities/GraphModelExtensions.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ public static SGNodeModel CreateGraphDataNode(
3030
}
3131
else
3232
{
33-
var registry = ((ShaderGraphStencil)graphModel.Stencil).GetRegistry().Registry;
3433
// Use this node's generated guid to bind it to an underlying element in the graph data.
3534
var graphDataName = nodeModel.Guid.ToString();
3635
((SGGraphModel)graphModel).GraphHandler.AddNode(registryKey, graphDataName);

0 commit comments

Comments
 (0)