Fix T89260: Eevee crashes with custom node sockets.
ClosedPublic
Actions

Authored by Jeroen Bakker (jbakker) on Nov 10 2021, 12:38 PM.

Details

Reviewers

Campbell Barton (campbellbarton)
Jacques Lucke (JacquesLucke)

Maniphest Tasks

T89260: Eevee instant crash with a custom node from template example

Commits

rB419b4e0b00c6: Fix T89260: Eevee crashes with custom node sockets.
rBa5c59fb90ef9: Fix T89260: Eevee crashes with custom node sockets.

Summary

Cause of this issue is that Custom Node Sockets info type was
initialized as SOCK_FLOAT when registering. Areas within the core that
would ignore custom socket types by checking its type would use the
socket as being a float type.

When custom node sockets have a property called default_value blender
tries to store it as an internal default value what failed in debug
builds.

This patch will set the socket type to SOCK_CUSTOM when registering a
custom socket type and won't trigger the storage of internal default values.
Default values are already stored as property.

Diff Detail

Repository

rB Blender

Branch

T89260 (branched from master)

Build Status

Buildable 18657
Build 18657: arc lint + arc unit

Event Timeline

Jeroen Bakker (jbakker) requested review of this revision.Nov 10 2021, 12:38 PM

Jeroen Bakker (jbakker) created this revision.

Harbormaster completed remote builds in B18575: Diff 44572.Nov 10 2021, 12:38 PM

Jeroen Bakker (jbakker) added reviewers: Campbell Barton (campbellbarton), Jacques Lucke (JacquesLucke).Nov 10 2021, 12:39 PM

Jeroen Bakker (jbakker) added projects: Python API, Nodes & Physics, BF Blender (3.0).

Jacques Lucke (JacquesLucke) added inline comments.Nov 10 2021, 1:33 PM

source/blender/blenkernel/intern/node.cc
499	What do you think about handling `SOCK_CUSTOM` in `write_node_socket_default_value` instead of here?

Jeroen Bakker (jbakker) added inline comments.Nov 10 2021, 2:08 PM

source/blender/blenkernel/intern/node.cc
499	Depends on the context of that function. I read the current idea to be to call the write_node_socket_default_value to store the internal default_value. The idea came from the assert statement there. But I am fine change the SOCK_CUSTOM logic there (break with comment).

Other than the requested change, this looks good to me, don't have to look at it again.

source/blender/blenkernel/intern/node.cc
499	Think `break` with comment would be better.

Jeroen Bakker (jbakker) added inline comments.Nov 10 2021, 6:41 PM

source/blender/blenkernel/intern/node.cc
499	Ok will do

LGTM, some request for comments inline.

source/blender/blenkernel/intern/node.cc
497–498	Is this always true? Or only for `SOCK_CUSTOM` defined dynamically from Python. From what I could see C/C++ defined `SOCK_CUSTOM` don't use the ID properties, although I may have missed this. Comment could also note that it's called "default_value" in `sock->prop`.
499	There is an identical function below (`write_node_socket_interface`), would it make sense to add this check here too? If not, there should be come explanation for why it's not needed.