blender/blender
170
530
Fork 806
Code Issues 6.5k Pull Requests 963 Projects 21 Wiki Activity

Fix versioning old 2.6 node groups, causing dangling link pointers #111704

Merged
Lukas Tönne merged 11 commits from LukasTonne/blender:fix-versioning-26-node-groups into main 2023-09-05 12:37:13 +02:00
Conversation 2 Commits 11 Files Changed 4 +221 -177
4 changed files with 221 additions and 177 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
source/blender/blenkernel/intern/modifier.cc
View File

@ -458,7 +458,7 @@ void BKE_modifier_set_error(const Object *ob, ModifierData *md, const char *_for
}
#endif
CLOG_ERROR(&LOG, "Object: \"%s\", Modifier: \"%s\", %s", ob->id.name + 2, md->name, md->error);
CLOG_WARN(&LOG, "Object: \"%s\", Modifier: \"%s\", %s", ob->id.name + 2, md->name, md->error);
}
void BKE_modifier_set_warning(const Object *ob, ModifierData *md, const char *_format, ...)

6
source/blender/blenkernel/intern/node.cc
View File

@ -1358,14 +1358,16 @@ void ntreeSetTypes(const bContext *C, bNodeTree *ntree)
blender::bke::ntree_set_typeinfo(ntree, ntreeTypeFind(ntree->idname));
for (bNode *node : ntree->all_nodes()) {
blender::bke::node_set_typeinfo(C, ntree, node, nodeTypeFind(node->idname));
/* Set socket typeinfo first because node initialization may rely on socket typeinfo for
* generating declarations. */
LISTBASE_FOREACH (bNodeSocket *, sock, &node->inputs) {
blender::bke::node_socket_set_typeinfo(ntree, sock, nodeSocketTypeFind(sock->idname));
}
LISTBASE_FOREACH (bNodeSocket *, sock, &node->outputs) {
blender::bke::node_socket_set_typeinfo(ntree, sock, nodeSocketTypeFind(sock->idname));
}
blender::bke::node_set_typeinfo(C, ntree, node, nodeTypeFind(node->idname));
}
}

50
source/blender/blenloader/intern/versioning_250.cc
View File

@ -1871,7 +1871,6 @@ void blo_do_versions_250(FileData *fd, Library * /*lib*/, Main *bmain)
*/
bNodeLink *link = static_cast<bNodeLink *>(MEM_callocN(sizeof(bNodeLink), "link"));
BLI_addtail(&ntree->links, link);
nodeUniqueID(ntree, node);
link->fromnode = nullptr;
link->fromsock = gsock;
link->tonode = node;
@ -1896,7 +1895,6 @@ void blo_do_versions_250(FileData *fd, Library * /*lib*/, Main *bmain)
*/
bNodeLink *link = static_cast<bNodeLink *>(MEM_callocN(sizeof(bNodeLink), "link"));
BLI_addtail(&ntree->links, link);
nodeUniqueID(ntree, node);
link->fromnode = node;
link->fromsock = sock;
link->tonode = nullptr;
@ -2123,56 +2121,8 @@ void blo_do_versions_250(FileData *fd, Library * /*lib*/, Main *bmain)
}
}
/* updates group node socket identifier so that
* external links to/from the group node are preserved.
*/
static void lib_node_do_versions_group_indices(bNode *gnode)
{
bNodeTree *ngroup = (bNodeTree *)gnode->id;
LISTBASE_FOREACH (bNodeSocket *, sock, &gnode->outputs) {
int old_index = sock->to_index;
LISTBASE_FOREACH (bNodeLink *, link, &ngroup->links) {
if (link->tonode == nullptr && link->fromsock->own_index == old_index) {
STRNCPY(sock->identifier, link->fromsock->identifier);
/* deprecated */
sock->own_index = link->fromsock->own_index;
sock->to_index = 0;
}
}
}
LISTBASE_FOREACH (bNodeSocket *, sock, &gnode->inputs) {
int old_index = sock->to_index;
LISTBASE_FOREACH (bNodeLink *, link, &ngroup->links) {
if (link->fromnode == nullptr && link->tosock->own_index == old_index) {
STRNCPY(sock->identifier, link->tosock->identifier);
/* deprecated */
sock->own_index = link->tosock->own_index;
sock->to_index = 0;
}
}
}
}
void do_versions_after_linking_250(Main *bmain)
{
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 256, 2)) {
FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
/* updates external links for all group nodes in a tree */
LISTBASE_FOREACH (bNode *, node, &ntree->nodes) {
if (node->type == NODE_GROUP) {
bNodeTree *ngroup = (bNodeTree *)node->id;
if (ngroup) {
lib_node_do_versions_group_indices(node);
}
}
}
}
FOREACH_NODETREE_END;
}
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 258, 0)) {
/* Some very old (original comments claim pre-2.57) versioning that was wrongly done in
* lib-linking code... Putting it here just to be sure (this is also checked at runtime anyway

340
source/blender/blenloader/intern/versioning_260.cc
View File

@ -6,6 +6,7 @@
* \ingroup blenloader
*/
#include "BKE_idprop.h"
#include "BLI_utildefines.h"
/* allow readfile to use deprecated functionality */
@ -52,6 +53,7 @@
#include "BKE_mesh_legacy_convert.hh"
#include "BKE_modifier.h"
#include "BKE_node_runtime.hh"
#include "BKE_node_tree_update.h"
#include "BKE_particle.h"
#include "BKE_pointcache.h"
#include "BKE_scene.h"
@ -1034,6 +1036,97 @@ static bool seq_set_wipe_angle_cb(Sequence *seq, void * /*user_data*/)
return true;
}
/* Create a socket stub without typeinfo. */
static bNodeSocket *version_make_socket_stub(const char *idname,
eNodeSocketDatatype type,
eNodeSocketInOut in_out,
const char *identifier,
const char *name,
const void *default_value,
const IDProperty *prop)
{
bNodeSocket *socket = MEM_cnew<bNodeSocket>(__func__);
socket->runtime = MEM_new<blender::bke::bNodeSocketRuntime>(__func__);
STRNCPY(socket->idname, idname);
socket->type = int(type);
socket->in_out = int(in_out);
socket->limit = (in_out == SOCK_IN ? 1 : 0xFFF);
BLI_strncpy(socket->identifier, identifier, sizeof(socket->identifier));
BLI_strncpy(socket->name, name, sizeof(socket->name));
socket->storage = nullptr;
socket->flag |= SOCK_COLLAPSED;
/* Note: technically socket values can store ref-counted ID pointers, but at this stage the
* refcount can be ignored. It gets recomputed after lib-linking for all ID pointers. Socket
* values don't have allocated data, so a simple dupalloc works here. */
socket->default_value = default_value ? MEM_dupallocN(default_value) : nullptr;
socket->prop = prop ? IDP_CopyProperty(prop) : nullptr;
return socket;
}
/* Same as nodeAddStaticNode but does not rely on node typeinfo. */
static bNode *version_add_group_in_out_node(bNodeTree *ntree, const int type)
{
ListBase *ntree_socket_list = nullptr;
ListBase *node_socket_list = nullptr;
eNodeSocketInOut socket_in_out = SOCK_IN;
bNode *node = MEM_cnew<bNode>("new node");
switch (type) {
case NODE_GROUP_INPUT:
STRNCPY(node->idname, "NodeGroupInput");
ntree_socket_list = &ntree->inputs_legacy;
/* Group input has only outputs. */
node_socket_list = &node->outputs;
socket_in_out = SOCK_OUT;
break;
case NODE_GROUP_OUTPUT:
STRNCPY(node->idname, "NodeGroupOutput");
ntree_socket_list = &ntree->outputs_legacy;
/* Group output has only inputs. */
node_socket_list = &node->inputs;
socket_in_out = SOCK_IN;
break;
default:
BLI_assert_unreachable();
return nullptr;
}
node->runtime = MEM_new<blender::bke::bNodeRuntime>(__func__);
BLI_addtail(&ntree->nodes, node);
nodeUniqueID(ntree, node);
/* Manual initialization of the node,
* node->typeinfo is only set after versioning. */
node->type = type;
{
if (ntree->typeinfo && ntree->typeinfo->node_add_init) {
ntree->typeinfo->node_add_init(ntree, node);
}
/* Add sockets without relying on declarations or typeinfo.
* These are stubs for links, full typeinfo is defined later. */
LISTBASE_FOREACH (bNodeSocket *, tree_socket, ntree_socket_list) {
bNodeSocket *node_socket = version_make_socket_stub(tree_socket->idname,
eNodeSocketDatatype(tree_socket->type),
socket_in_out,
tree_socket->identifier,
tree_socket->name,
tree_socket->default_value,
tree_socket->prop);
BLI_addtail(node_socket_list, node_socket);
BKE_ntree_update_tag_socket_type(ntree, node_socket);
BKE_ntree_update_tag_socket_new(ntree, node_socket);
}
}
BKE_ntree_update_tag_node_new(ntree, node);
return node;
}
/* NOLINTNEXTLINE: readability-function-size */
void blo_do_versions_260(FileData *fd, Library * /*lib*/, Main *bmain)
{
@ -2170,6 +2263,128 @@ void blo_do_versions_260(FileData *fd, Library * /*lib*/, Main *bmain)
}
FOREACH_NODETREE_END;
}
/* Convert the previously used ntree->inputs/ntree->outputs lists to interface nodes.
* Pre 2.56.2 node trees automatically have all unlinked sockets exposed already,
* see do_versions_after_linking_250.
Jacques Lucke commented 2023-08-31 12:05:17 +02:00
Review
Copy Link

This comment hints at a potential problem, because now do_versions_after_linking_250 also runs after this versioning code now. I don't even know what node systems we had pre 2.5 tbh.

This comment hints at a potential problem, because now `do_versions_after_linking_250` also runs after this versioning code now. I don't even know what node systems we had pre 2.5 tbh.
Lukas Tönne commented 2023-09-01 12:51:16 +02:00
Review
Copy Link

I've started adding a simple compatibility test, loading files from 2.5, 2.6, 3.6, etc (trying to cover major transitions in node code). Loading a simple 2.5 file already fails an assert in unrelated versioning code, because a node gets added to the runtime nodes_by_id set twice.

source/blender/blenloader/intern/versioning_250.cc Line 1899 in cf12d11de6
nodeUniqueID(ntree, node);

I hope there aren't too many other issues and i can actually test the after_linking parts. This should have been done years ago, but better late than never ...

I've started adding a simple compatibility test, loading files from 2.5, 2.6, 3.6, etc (trying to cover major transitions in node code). Loading a simple 2.5 file already fails an assert in unrelated versioning code, because a node gets added to the runtime `nodes_by_id` set twice. https://projects.blender.org/blender/blender/src/commit/cf12d11de6b3dfd9df70f5621659d0ca4b7d1047/source/blender/blenloader/intern/versioning_250.cc#L1899 I hope there aren't too many other issues and i can actually test the `after_linking` parts. This should have been done years ago, but better late than never ...
Lukas Tönne commented 2023-09-01 13:06:15 +02:00
Review
Copy Link

The offending nodeUniqueName was added by ab4926bcff but it re-generates the identifier for every socket in a node and adds the node to nodes_by_id as many times.

The offending `nodeUniqueName` was added by ab4926bcffee but it re-generates the identifier for every socket in a node and adds the node to `nodes_by_id` as many times.
Lukas Tönne commented 2023-09-01 13:52:48 +02:00
Review
Copy Link

I find so many broken things with 2.5 nodes versioning, might be preferable to complete this PR for 2.6 compatibility and continue fixing 2.5 nodes separately.

I find so many broken things with 2.5 nodes versioning, might be preferable to complete this PR for 2.6 compatibility and continue fixing 2.5 nodes separately.
Lukas Tönne commented 2023-09-01 14:27:51 +02:00
Review
Copy Link

Getting back to the original question: "Does this after_linking code still work after regular versioning to 4.0?"

Yes! And in this case running in after_linking is necessary, because the socket identifier update looks at the node tree pointers of group nodes (i.e. other ID data blocks). It reads from bNodeSocket::own_index (deprecated by 2.5.2) and bNodeSocket::identifier (stable, existing socket identifiers never change). So this should be pretty safe to keep in after_linking.

Getting back to the original question: "Does this `after_linking` code still work after regular versioning to 4.0?" Yes! And in this case running in `after_linking` is necessary, because the socket identifier update looks at the node tree pointers of group nodes (i.e. other ID data blocks). It reads from `bNodeSocket::own_index` (deprecated by 2.5.2) and `bNodeSocket::identifier` (stable, existing socket identifiers never change). So this should be pretty safe to keep in `after_linking`.
Jacques Lucke commented 2023-09-01 19:16:37 +02:00
Review
Copy Link

The question was mostly whether it's a problem that now the order of the versioning is reversed, since the after-linking code runs after the code that is moved in this patch (but it used to run before).

The question was mostly whether it's a problem that now the order of the versioning is reversed, since the after-linking code runs after the code that is moved in this patch (but it used to run before).
Lukas Tönne commented 2023-09-03 15:12:25 +02:00
Review
Copy Link

In #111800 i'm adding a compatibility regression test for node groups. The 3.6 case already required some minor fixes, i want to add a 2.5 test if feasible.

Took me a long time to unravel the timeline of node changes, here's what i reconstructed so far:

TL;DR: We need to update after_linking code to find group socket matches and preserve external links.

  • Before 2.56.2 all the unconnected internal sockets were exposed. To find the internal socket for a group node input or output one would search for the internal socket whose own_index matches the external to_index. groupnode25.jpg

  • Starting with 2.56.2 and before 2.66.3 this behavior changed and now node groups would only show an explicit set of inputs and outputs (stored in bNodeTree inputs/outputs). This required versioning to

    1. Create a tree socket for every unconnected internal socket to preserve existing node setups.
    2. Add special links with tonode==nullptr or fromnode==nullptr which indicate a connection to the node tree interface.
      code link

    All of this happens inside the same bNodeTree as part of regular versioning. The after_linking versioning is needed for updating the own_index of group nodes (code link):
    own_index is still used to identify sockets at this point (replaced with identifiers later). to_index gets deprecated, the socket in the node tree and its counterpart on a group node instance should simply have the same own_index. In order to version this, however, each group node needs to inspect its linked node group, which is not available during regular versioning.

    Note: Socket identifier strings are already generated by 2.56.2 versioning, but not sure if they would have been used for anything. This might be some apocryphal versioning code added later?

  • In 2.66.2+ there was another set of changes:

    1. own_index mapping is replaced with unique socket identifier strings code link
    2. The "special" interface links with node nullptrs are replaced by group input/output nodes and regular links (no more node nullptrs allowed) code link

    The 2.66.3 versioning code was also running in the after_linking stage, but as demonstrated here this is only for the convenience of bNodeSocketType callbacks and not really necessary.

What could happen if we execute the 2.56.2 after_linking code after all the 4.0 versioning? Inside the node group all the unconnected sockets have been exposed first as ntree->inputs/ntree-outputs with nullptr links, those have then been replaced with group input/output nodes and regular links, and then the ntree->inputs/ntree-outputs have been replaced by tree_interface.ui_items. Even after all this the deprecated own_index of the sockets in the tree that should be exposed to mimick 2.5 behavior still exist, which would make it possible to find internal sockets and update the external socket identifiers.

However, the versioning code also expects there to be special links with node nullptrs now - so the 2.56.2 after_linking versioning code already includes changes from 2.66.3 in order to function correctly. The special links have been replaced by versioning code, so this part is now ineffective and never runs.

The consequence is that socket identifiers for node groups are not fully updated and some links get broken. This is because the identifiers of existing sockets are first generated by 2.66.2 versioning code (code link) but they are only based on names and unique suffixes. Sockets without a matching group tree socket are replaced later and any links discarded. groupnode25.jpg

We need to update do_versions_after_linking_250 to take this into account and search for matching internal socket like so:

  • For any socket inside the node tree whose own_index matches the group socket's to_index:
    • Check if it has a link to a group input/output node
    • Copy the matching group interface socket identifier

There is a lingering question of what might happen if some intermediate versioning depends on correct group socket identifiers? But i think this is as far as we can reasonably support such ancient files.

In #111800 i'm adding a compatibility regression test for node groups. The 3.6 case already required some minor fixes, i want to add a 2.5 test if feasible. Took me a long time to unravel the timeline of node changes, here's what i reconstructed so far: **TL;DR: We need to update after_linking code to find group socket matches and preserve external links.** - Before 2.56.2 all the unconnected internal sockets were exposed. To find the internal socket for a group node input or output one would search for the internal socket whose `own_index` matches the external `to_index`. ![groupnode25.jpg](/attachments/945dc5df-d121-4b34-8e83-f0cac4cf45de) - Starting with 2.56.2 and before 2.66.3 this behavior changed and now node groups would only show an explicit set of inputs and outputs (stored in `bNodeTree` `inputs`/`outputs`). This required versioning to 1. Create a tree socket for every unconnected internal socket to preserve existing node setups. 2. Add special links with `tonode==nullptr` or `fromnode==nullptr` which indicate a connection to the node tree interface. [code link](https://projects.blender.org/blender/blender/src/branch/main/source/blender/blenloader/intern/versioning_250.cc#L1837-L1913) All of this happens inside the same `bNodeTree` as part of regular versioning. The `after_linking` versioning is needed for updating the `own_index` of group nodes ([code link](https://projects.blender.org/blender/blender/src/branch/main/source/blender/blenloader/intern/versioning_250.cc#L2131-L2156)): `own_index` is still used to identify sockets at this point (replaced with identifiers later). `to_index` gets deprecated, the socket in the node tree and its counterpart on a group node instance should simply have the same `own_index`. In order to version this, however, each group node needs to inspect its linked node group, which is not available during regular versioning. _Note: Socket identifier strings are already generated by 2.56.2 versioning, but not sure if they would have been used for anything. This might be some apocryphal versioning code added later?_ - In 2.66.2+ there was another set of changes: 1. `own_index` mapping is replaced with unique socket identifier strings [code link](https://projects.blender.org/blender/blender/src/branch/main/source/blender/blenloader/intern/versioning_260.cc#L950-L990) 2. The "special" interface links with node nullptrs are replaced by group input/output nodes and regular links (no more node `nullptrs` allowed) [code link](https://projects.blender.org/blender/blender/src/branch/main/source/blender/blenloader/intern/versioning_260.cc#L2697-L2798) The 2.66.3 versioning code was also running in the `after_linking` stage, but as demonstrated here this is only for the convenience of `bNodeSocketType` callbacks and not really necessary. What could happen if we execute the 2.56.2 `after_linking` code after all the 4.0 versioning? Inside the node group all the unconnected sockets have been exposed first as `ntree->inputs`/`ntree-outputs` with nullptr links, those have then been replaced with group input/output nodes and regular links, and then the `ntree->inputs`/`ntree-outputs` have been replaced by `tree_interface.ui_items`. Even after all this the deprecated `own_index` of the sockets in the tree that should be exposed to mimick 2.5 behavior still exist, which would make it possible to find internal sockets and update the external socket identifiers. However, the versioning code also expects there to be special links with node nullptrs now - so the 2.56.2 `after_linking` versioning code already includes changes from 2.66.3 in order to function correctly. The special links have been replaced by versioning code, so this part is now ineffective and never runs. The consequence is that socket identifiers for node groups are not fully updated and some links get broken. This is because the identifiers of existing sockets are first generated by 2.66.2 versioning code ([code link](https://projects.blender.org/blender/blender/src/branch/main/source/blender/blenloader/intern/versioning_260.cc#L950-L990)) but they are only based on names and unique suffixes. Sockets without a matching group tree socket are replaced later and any links discarded. ![groupnode25.jpg](/attachments/3007ee7e-2e24-4c28-9a84-e1480c083997) We need to update `do_versions_after_linking_250` to take this into account and search for matching internal socket like so: - For any socket inside the node tree whose `own_index` matches the group socket's `to_index`: - Check if it has a link to a group input/output node - Copy the matching group interface socket identifier There is a lingering question of what might happen if some intermediate versioning depends on correct group socket identifiers? But i think this is as far as we can reasonably support such ancient files.
groupnode25_versioned.jpg
60 KiB
Jacques Lucke commented 2023-09-04 09:57:15 +02:00
Review
Copy Link

Thanks for looking into this. I think it's ok when we are not able to load these ancient files perfectly. Would be nice, but it is what it is. If people really rely on those files, they may have to load and save the files in a more recent version. It's good to know why things are not working anyway. Also, it's great that you are adding some tests for this!
Do you happen to know when the node systems that we use nowadays have been introduced?

Thanks for looking into this. I think it's ok when we are not able to load these ancient files perfectly. Would be nice, but it is what it is. If people really rely on those files, they may have to load and save the files in a more recent version. It's good to know why things are not working anyway. Also, it's great that you are adding some tests for this! Do you happen to know when the node systems that we use nowadays have been introduced?
Lukas Tönne commented 2023-09-04 10:33:05 +02:00
Review
Copy Link

Do you happen to know when the node systems that we use nowadays have been introduced?

Depends on what you mean by "systems that we use nowadays". Oldest commit to the nodes folder is from 2007. From then on the nodes system has been a Ship of Theseus: 2.5 saw the change to explicit node group sockets, then group input/output nodes and python nodes in 2.6, and other parts of the system have been modified and swapped out over the years.

> Do you happen to know when the node systems that we use nowadays have been introduced? Depends on what you mean by "systems that we use nowadays". Oldest commit to the `nodes` folder is from 2007. From then on the nodes system has been a Ship of Theseus: 2.5 saw the change to explicit node group sockets, then group input/output nodes and python nodes in 2.6, and other parts of the system have been modified and swapped out over the years.
Lukas Tönne commented 2023-09-04 15:31:38 +02:00
Review
Copy Link

I can't figure this out (in reasonable time and with acceptable sanity loss). I "fixed" the 2.5 after_linking code, but it still does not work because socket identifiers in the bNodeTree are changing so much that links cannot be restored from the 2.5 file.

  1. Socket identifiers first initialized by do_versions_nodetree_customnodes in versioning_260 to a FooBar.001 format
  2. Identifiers change somewhere between 2.7 and 4.0 into FooBar_001 format
  3. Then they change back to FooBar.001 format in the after_linking code
  4. Socket links break because the tree interface has identifiers in FooBar_001 format (not sure where from)
I can't figure this out (in reasonable time and with acceptable sanity loss). I "fixed" the 2.5 after_linking code, but it still does not work because socket identifiers in the bNodeTree are changing so much that links cannot be restored from the 2.5 file. 1. Socket identifiers first initialized by `do_versions_nodetree_customnodes` in versioning_260 to a `FooBar.001` format 2. Identifiers change _somewhere between 2.7 and 4.0_ into `FooBar_001` format 3. Then they change back to `FooBar.001` format in the after_linking code 4. Socket links break because the tree interface has identifiers in `FooBar_001` format (not sure where from)
*
* This assumes valid typeinfo pointers, as set in lib_link_ntree.
*
* NOTE: theoretically only needed in node groups (main->nodetree),
* but due to a temporary bug such links could have been added in all trees,
* so have to clean up all of them ...
*
* NOTE: this always runs, without it links with nullptr fromnode and tonode remain
* which causes problems.
*/
FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
bNode *input_node = nullptr, *output_node = nullptr;
int num_inputs = 0, num_outputs = 0;
bNodeLink *link, *next_link;
/* Only create new interface nodes for actual older files.
* New file versions already have input/output nodes with duplicate links,
* in that case just remove the invalid links.
*/
const bool create_io_nodes = MAIN_VERSION_FILE_OLDER(bmain, 266, 2);
float input_locx = 1000000.0f, input_locy = 0.0f;
float output_locx = -1000000.0f, output_locy = 0.0f;
/* Rough guess, not nice but we don't have access to UI constants here. */
const float offsetx = 42 + 3 * 20 + 20;
// const float offsety = 0.0f;
if (create_io_nodes) {
if (ntree->inputs_legacy.first) {
input_node = version_add_group_in_out_node(ntree, NODE_GROUP_INPUT);
}
if (ntree->outputs_legacy.first) {
output_node = version_add_group_in_out_node(ntree, NODE_GROUP_OUTPUT);
}
}
/* Redirect links from/to the node tree interface to input/output node.
* If the fromnode/tonode pointers are nullptr, this means a link from/to
* the ntree interface sockets, which need to be redirected to new interface nodes.
*/
for (link = static_cast<bNodeLink *>(ntree->links.first); link != nullptr; link = next_link)
{
bool free_link = false;
next_link = link->next;
if (link->fromnode == nullptr) {
if (input_node) {
link->fromnode = input_node;
link->fromsock = node_group_input_find_socket(input_node, link->fromsock->identifier);
BLI_assert(link->fromsock != nullptr);
num_inputs++;
if (link->tonode) {
if (input_locx > link->tonode->locx - offsetx) {
input_locx = link->tonode->locx - offsetx;
}
input_locy += link->tonode->locy;
}
}
else {
free_link = true;
}
}
if (link->tonode == nullptr) {
if (output_node) {
link->tonode = output_node;
link->tosock = node_group_output_find_socket(output_node, link->tosock->identifier);
BLI_assert(link->tosock != nullptr);
num_outputs++;
if (link->fromnode) {
if (output_locx < link->fromnode->locx + offsetx) {
output_locx = link->fromnode->locx + offsetx;
}
output_locy += link->fromnode->locy;
}
}
else {
free_link = true;
}
}
if (free_link) {
nodeRemLink(ntree, link);
}
}
if (num_inputs > 0) {
input_locy /= num_inputs;
input_node->locx = input_locx;
input_node->locy = input_locy;
}
if (num_outputs > 0) {
output_locy /= num_outputs;
output_node->locx = output_locx;
output_node->locy = output_locy;
}
}
FOREACH_NODETREE_END;
}
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 280, 60)) {
Jacques Lucke commented 2023-08-31 12:09:29 +02:00
Review
Copy Link

This versioning looks like it's at the wrong place (see version number)

This versioning looks like it's at the wrong place (see version number)
Lukas Tönne commented 2023-08-31 12:30:23 +02:00
Review
Copy Link

Correct. I considered moving it to the 280 file, and the first thing i saw at the bottom of the that file was a bit of versioning code for ... 3.0 :P

https://projects.blender.org/blender/blender/src/branch/main/source/blender/blenloader/intern/versioning_280.cc#L6300-L6324

I find it extremely difficult to reason about why something works or doesn't in nodes versioning, so i just left it next to the other bit for now ...

Correct. I considered moving it to the 280 file, and the first thing i saw at the bottom of the _that_ file was a bit of versioning code for ... 3.0 :P https://projects.blender.org/blender/blender/src/branch/main/source/blender/blenloader/intern/versioning_280.cc#L6300-L6324 I find it extremely difficult to reason about why something works or doesn't in nodes versioning, so i just left it next to the other bit for now ...
Jacques Lucke commented 2023-08-31 12:43:02 +02:00
Review
Copy Link

I see. It's not super horrible if it works. The code between here and where this versioning code would usually live is unlikely to change in major ways.

I see. It's not super horrible if it works. The code between here and where this versioning code would usually live is unlikely to change in major ways.
/* From this point we no longer write incomplete links for forward
* compatibility with 2.66, we have to clean them up for all previous
* versions. */
FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
bNodeLink *link, *next_link;
for (link = static_cast<bNodeLink *>(ntree->links.first); link != nullptr; link = next_link)
{
next_link = link->next;
if (link->fromnode == nullptr || link->tonode == nullptr) {
nodeRemLink(ntree, link);
}
}
}
FOREACH_NODETREE_END;
}
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 266, 4)) {
@ -2692,127 +2907,4 @@ void blo_do_versions_260(FileData *fd, Library * /*lib*/, Main *bmain)
}
}
void do_versions_after_linking_260(Main *bmain)
{
/* Convert the previously used ntree->inputs/ntree->outputs lists to interface nodes.
* Pre 2.56.2 node trees automatically have all unlinked sockets exposed already,
* see do_versions_after_linking_250.
*
* This assumes valid typeinfo pointers, as set in lib_link_ntree.
*
* NOTE: theoretically only needed in node groups (main->nodetree),
* but due to a temporary bug such links could have been added in all trees,
* so have to clean up all of them ...
*
* NOTE: this always runs, without it links with nullptr fromnode and tonode remain
* which causes problems.
*/
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 266, 3)) {
FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
bNode *input_node = nullptr, *output_node = nullptr;
int num_inputs = 0, num_outputs = 0;
bNodeLink *link, *next_link;
/* Only create new interface nodes for actual older files.
* New file versions already have input/output nodes with duplicate links,
* in that case just remove the invalid links.
*/
const bool create_io_nodes = MAIN_VERSION_FILE_OLDER(bmain, 266, 2);
float input_locx = 1000000.0f, input_locy = 0.0f;
float output_locx = -1000000.0f, output_locy = 0.0f;
/* Rough guess, not nice but we don't have access to UI constants here. */
const float offsetx = 42 + 3 * 20 + 20;
// const float offsety = 0.0f;
if (create_io_nodes) {
if (ntree->inputs_legacy.first) {
input_node = nodeAddStaticNode(nullptr, ntree, NODE_GROUP_INPUT);
}
if (ntree->outputs_legacy.first) {
output_node = nodeAddStaticNode(nullptr, ntree, NODE_GROUP_OUTPUT);
}
}
/* Redirect links from/to the node tree interface to input/output node.
* If the fromnode/tonode pointers are nullptr, this means a link from/to
* the ntree interface sockets, which need to be redirected to new interface nodes.
*/
for (link = static_cast<bNodeLink *>(ntree->links.first); link != nullptr; link = next_link)
{
bool free_link = false;
next_link = link->next;
if (link->fromnode == nullptr) {
if (input_node) {
link->fromnode = input_node;
link->fromsock = node_group_input_find_socket(input_node, link->fromsock->identifier);
num_inputs++;
if (link->tonode) {
if (input_locx > link->tonode->locx - offsetx) {
input_locx = link->tonode->locx - offsetx;
}
input_locy += link->tonode->locy;
}
}
else {
free_link = true;
}
}
if (link->tonode == nullptr) {
if (output_node) {
link->tonode = output_node;
link->tosock = node_group_output_find_socket(output_node, link->tosock->identifier);
num_outputs++;
if (link->fromnode) {
if (output_locx < link->fromnode->locx + offsetx) {
output_locx = link->fromnode->locx + offsetx;
}
output_locy += link->fromnode->locy;
}
}
else {
free_link = true;
}
}
if (free_link) {
nodeRemLink(ntree, link);
}
}
if (num_inputs > 0) {
input_locy /= num_inputs;
input_node->locx = input_locx;
input_node->locy = input_locy;
}
if (num_outputs > 0) {
output_locy /= num_outputs;
output_node->locx = output_locx;
output_node->locy = output_locy;
}
}
FOREACH_NODETREE_END;
}
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 280, 60)) {
/* From this point we no longer write incomplete links for forward
* compatibility with 2.66, we have to clean them up for all previous
* versions. */
FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
bNodeLink *link, *next_link;
for (link = static_cast<bNodeLink *>(ntree->links.first); link != nullptr; link = next_link)
{
next_link = link->next;
if (link->fromnode == nullptr || link->tonode == nullptr) {
nodeRemLink(ntree, link);
}
}
}
FOREACH_NODETREE_END;
}
}
void do_versions_after_linking_260(Main * /*bmain*/) {}