Cycles Stand Alone Node Connect Issue #49985

New Issue

David · 2016-11-10T13:29:18+01:00

David commented

2016-11-10 13:29:18 +01:00

I believe this is a bug in the Cycles Standalone xml reader code.

When trying to connect sockets, it uses the following code:
*similar for both inputs and outputs...input shown for example

					foreach(ShaderInput *in, tonode->inputs)
						if(string_iequals(xml_socket_name(in->name().c_str()), to_socket_name.c_str()))
							input = in;

So it's looking for the socket by the UI name. (works most of the time)
as you see ShaderInput/ShaderOutput gets "name" from this code:

	ustring name() { return socket_type.ui_name; }

however, if the socket name doesn't match the UI name = fail to connect. (example below)

Now the function: void xml_read_node(XMLReader& reader, Node *node, pugi::xml_node xml_node)

	pugi::xml_attribute attr = xml_node.attribute(socket.name.c_str());

works cause socket is type of "SocketType" not ShaderInput/ShaderOutput, so it doesn't get the "UI" name.
*value is correctly assigned from xml

Where it fails:
I found the issue using the Disney branch, so i'll use that as the example.
the node "disney_bsdf" has a socket "base_color" and UI name as "Base Color"

It has no problem assigning the base_color (xml_read_node correctly uses socket name)
But if you try to connect another node to it the xml reader doesn't find it since it's looking for a "BaseColor" socket.
*it gets the UI name "Base Color", strips the space using xml_socket_name

so the simple fix in cycles_xml.cpp, compare using socket_type.name

  			foreach(ShaderOutput *out, fromnode->outputs)
  				if(string_iequals(xml_socket_name(out->socket_type.name.c_str()), from_socket_name.c_str()))
  					output = out;

...

  			foreach(ShaderInput *in, tonode->inputs)
  				if(string_iequals(xml_socket_name(in->socket_type.name.c_str()), to_socket_name.c_str()))
  					input = in;

I believe this is a bug in the Cycles Standalone xml reader code. When trying to connect sockets, it uses the following code: **similar for both inputs and outputs...input shown for example* ``` foreach(ShaderInput *in, tonode->inputs) if(string_iequals(xml_socket_name(in->name().c_str()), to_socket_name.c_str())) input = in; ``` So it's looking for the socket by the UI name. *(works most of the time)* as you see ShaderInput/ShaderOutput gets "name" from this code: ``` ustring name() { return socket_type.ui_name; } ``` however, if the socket name doesn't match the UI name = fail to connect. **(example below)** Now the function: void xml_read_node(XMLReader& reader, Node *node, pugi::xml_node xml_node) ``` pugi::xml_attribute attr = xml_node.attribute(socket.name.c_str()); ``` works cause socket is type of "SocketType" not ShaderInput/ShaderOutput, so it doesn't get the "UI" name. **value is correctly assigned from xml* **Where it fails:** I found the issue using the Disney branch, so i'll use that as the example. the node "disney_bsdf" has a socket "base_color" and UI name as "Base Color" It has no problem assigning the base_color (xml_read_node correctly uses socket name) But if you try to connect another node to it the xml reader doesn't find it since it's looking for a "BaseColor" socket. *it gets the UI name "Base Color", strips the space using xml_socket_name so the simple fix in cycles_xml.cpp, compare using socket_type.name > foreach(ShaderOutput *out, fromnode->outputs) > if(string_iequals(xml_socket_name(out->socket_type.name.c_str()), from_socket_name.c_str())) > output = out; ... > foreach(ShaderInput *in, tonode->inputs) > if(string_iequals(xml_socket_name(in->socket_type.name.c_str()), to_socket_name.c_str())) > input = in; >

David commented

2016-11-10 13:29:18 +01:00

Changed status to: 'Open'

David commented

2016-11-10 13:29:18 +01:00

Added subscriber: @bocs

David commented

2016-11-10 17:16:20 +01:00

2nd thought, why even use "xml_socket_name" to strip spaces since they can't have a space unlike UI name?

fixed code could be shorted more like this:

					foreach(ShaderInput *in, tonode->inputs)
						if(string_iequals(in->socket_type.name.c_str(), to_socket_name.c_str()))
							input = in;

also why is LIGHT_DISTANT missing from light.cpp
*asking here also cause I consider it a node connect issue

I always add this to the code to : NODE_DEFINE(Light)

	type_enum.insert("distant", LIGHT_DISTANT);

2nd thought, why even use "xml_socket_name" to strip spaces since they can't have a space unlike UI name? fixed code could be shorted more like this: ``` foreach(ShaderInput *in, tonode->inputs) if(string_iequals(in->socket_type.name.c_str(), to_socket_name.c_str())) input = in; ``` also why is LIGHT_DISTANT missing from light.cpp *asking here also cause I consider it a node connect issue I always add this to the code to : NODE_DEFINE(Light) ``` type_enum.insert("distant", LIGHT_DISTANT); ```

Bastien Montagne commented

2016-11-11 12:09:29 +01:00

Added subscribers: @ThomasDinges, @mont29

Thomas Dinges was assigned by Bastien Montagne

2016-11-11 12:09:29 +01:00

Bastien Montagne commented

2016-11-11 12:09:29 +01:00

@ThomasDinges think you handle Cycles standalone, right?

blender-admin commented

2016-11-12 15:26:24 +01:00

This issue was referenced by blender/cycles@72fca011cb

This issue was referenced by blender/cycles@72fca011cbbe690c3c62e00b9f178cad2b1a3e53

blender-admin commented

2016-11-12 15:26:24 +01:00

This issue was referenced by 188ecee642

This issue was referenced by 188ecee6424f0eae1cf94ca9256795d068aa4200

blender-admin commented

2016-11-12 15:26:51 +01:00

This issue was referenced by blender/cycles@a5f1927f94

This issue was referenced by blender/cycles@a5f1927f94220654ab5f2d2237d2f10db1e7a5a0

blender-admin commented

2016-11-12 15:26:51 +01:00

This issue was referenced by 478e59a04e

This issue was referenced by 478e59a04ec8eec36217fbc0302e0fbe5ed519a9

Brecht Van Lommel commented

2016-11-12 17:01:08 +01:00

Added subscriber: @brecht

Thomas Dinges was unassigned by Brecht Van Lommel

2016-11-12 17:01:08 +01:00

Brecht Van Lommel self-assigned this 2016-11-12 17:01:08 +01:00

Brecht Van Lommel commented

2016-11-12 17:01:08 +01:00

This new XML reading code is more my area. There is indeed some outdated code hanging around, we should be using the name rather than the UI name. I'll also fix the missing distant light.

Brecht Van Lommel commented

2016-11-12 17:33:56 +01:00

Changed status from 'Open' to: 'Resolved'

Brecht Van Lommel closed this issue

2016-11-12 17:33:56 +01:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Cycles Stand Alone Node Connect Issue #49985