PythonExpressionEngine parsing bug

[masterkeech]

Version: Gaffer 0.56.0.0b1-linux
Third-party tools: None
Third-party modules: None

Description

When updating a string vector data in a context variable node that is referenced upstream image node inside an image loop, the hash is not evaluated causing the image not to update.

This doesn't seem to work in the latest gaffer 0.56.0.0b1 or subsequent previous versions 0.55.., 0.54..

Steps to reproduce

1. Open [gaffer-image-loop-context-variable-bug.txt] in Gaffer (https://github.com/GafferHQ/gaffer/files/4191892/gaffer-image-loop-context-variable-bug.txt)
2. View the ContextVariables node
3. update one of the four names in 'ugh:what2', notice the image doesn't refresh with the new name
4. copy and pasta all the nodes to see the names update

[johnhaddon]

This appears to be a deficiency in our python expression parsing (we need to parse them to find out what context variables they are sensitive to). If you adjust the expression on your text node to use a temporary variable, the parser is able to figure out what is happening, and then things update as expected :

index = context.get('loop:index', 0)
tmp = context.get('ugh:what2', ['Hello World'])
parent["Text"]["text"] = tmp[index]

[masterkeech]

Hi John,

That snippet does indeed fix that example script, so thank you for that.

But I may have found a second bug that is very related as that fix did not solve my original issue. I have managed to make another simple example that sets the context based on an expression to a Dot node's plugDirtiedSignal and updates the context variable if any of the dots custom label changes.

Steps to Reproduce

1. load the gaffer script gaffer-pinned-image-loop-box-bug.txt
2. run the following code snippet to set up callbacks on the dots for plugs dirtied

import GafferImage

def update_yall(plug):
	if plug.getName() != "label":
		return
	print 'update yall: {} label: {}'.format(plug.parent().getName(), plug.getValue())
	box = plug.parent()['out'].outputs()[0].parent()
	labels = IECore.StringVectorData()
	for i in range(4):
		name = "in{}".format(i)
		if i == 0:
			name = name[:-1]
		dot_node = box[name].getInput().parent()
		label = dot_node["label"].getValue()
		labels.append(label)
	box['ContextVariables1']['variables']['member2']['value'].setValue(labels)

connections = []
for plug in root['Box'].children():
	if isinstance(plug, GafferImage.ImagePlug) and plug.direction() == Gaffer.Plug.Direction.In:
		connection = plug.getInput().parent().plugDirtiedSignal().connect(update_yall)
		connections.append(connection)

3. edit a Dot's custom name
4. view the Box to see the new custom name, yay
5. Pin the Box
6. edit a Dot's custom name
7. view the Box to see no change, sad face

[johnhaddon]

Hey Greg, thanks for the simple repros. I'm afraid this second problem lies in your code rather than in Gaffer though. It isn't stated strongly enough in the docs for plugDirtiedSignal(), but it is there :

slots connected to this signal must not rewire the graph - they should be passive observers only.

plugDirtiedSignal() is what we emit to tell observers (the Viewer etc) that they might want to update. If you think about it, it doesn't really make sense to modify the graph from a signal that exists to tell people that the graph has been modified. We should probably deal with attempts to do this much more robustly, at least emitting a warning saying what is up.

I don't know if it will work for your real-world use case, but if you modify your script to connect to plugSetSignal() instead of plugDirtiedSignal() everything appears to work fine here. As documented, we do allow modification of the graph from within plugSetSignal() and plugInputChangedSignal().

Hope that makes sense...feel free to ping me on chat if there are details of the real-world setup that you can't share here...

[masterkeech]

Yes that makes complete sense and thank you for the very thorough explanation. I will update my code accordingly.

Cheers!

[johnhaddon]

Cool. Using plugSetSignal() does of course rely on users not driving the plugs with an expression or any other computed value though, so you might want to bear that in mind. Ultimately you might want to consider using an expression or custom node to merge your individual strings into the string array "live".