source/blender/io/collada/Materials.cpp
328	what if from_socket is NULL ?
329	same as above
331	Suggest to encapsulate this change into an overwritten method: void MaterialNode::add_link(bNode from_node, const char from_label, bNode to_node, const char to_label)

This revision now requires changes to proceed.May 12 2021, 11:39 AM

Scurest (scurest) added inline comments.May 12 2021, 3:54 PM

source/blender/io/collada/Materials.cpp
329	The code right above this does nodeFindSocket(..., "Emission") without checking for NULL, so I assumed I didn't need to. What do you want to do if it's NULL?

Respond to comments

You are right. It is always different if you look at your own code or at the code made by others :)

I suggest to create a warning message when a socket can not be found.
I am actually not sure if this situation can ever happen, but being cautious here may result in less Blender crashes.

maybe something like:

bNodeSocket *from_socket = nodeFindSocket(from_node, SOCK_OUT, from_label);
if (from_socket==null) {
    fprintf(stderr, "Collada: Source node %s has no socket %s.\n", from_node->name, from_label);
    return;
}

bNodeSocket *to_socket = nodeFindSocket(to_node, SOCK_IN, to_label);
if (to_socket==null) {
    fprintf(stderr, "Collada: Target node %s has no socket %s.\n", to_node->name, to_label);
    return;
}

nodeAddLink(ntree, from_node, from_socket, to_node, to_socket);

Gaia Clary (gaiaclary) added inline comments.May 12 2021, 6:15 PM

source/blender/io/collada/Materials.h
51	I recall that this causes issues on some operating systems but since i am only on windows i do not know for sure. I believe that the following declaration would be OK (like all the other declarations in that header file): void add_link(bNode from_node, const char from_label, bNode to_node, const char to_label); }

I updated the patch to skip creating the link if either socket is null, so there should be no risk of crashing.

Since nothing in this file currently checks if sockets are null or not, I think warning or increasing robustness to null should be done in a different patch. This patch just brings emission-from-texture up to parity with emission-from-constant.

Fix copy paste error

Scurest (scurest) added inline comments.May 12 2021, 6:32 PM

source/blender/io/collada/Materials.h
51	My bad, that's a copy-paste error. Thanks.

Gaia Clary (gaiaclary) accepted this revision.May 12 2021, 9:16 PM

This revision is now accepted and ready to land.May 12 2021, 9:16 PM

@Scurest (scurest) hi again.

Would you mind to create a task and add a .dae file that can be used to verify that the patch works as intended?
Also are you able to land the patch or do you need me to do this for you?

thanks

@Scurest (scurest) i have already created the task T88254 Please can you add a dae file for testing?

How would I go about landing it? I've never sent a patch to Blender before.

Closed by commit rB2953732fc5e1: Collada import: connect Emission texture to Emission socket (authored by Scurest (scurest), committed by Gaia Clary (gaiaclary)). · Explain WhyMay 13 2021, 11:54 PM

This revision was automatically updated to reflect the committed changes.

Gaia Clary (gaiaclary) added a commit: rB2953732fc5e1: Collada import: connect Emission texture to Emission socket.