Nodes: Add forward compatibility versioning for #111187 #114236

Hoshinova · 2023-10-28T23:52:05+02:00

Hoshinova commented

2023-10-28 23:52:05 +02:00

This PR adds the versioning code to convert the new combined Noise Texture node in Blender 4.1 back to the Noise and Musgrave Texture nodes in Blender 4.0.

Versioning is only done for the Musgrave exclusive noise modes, that is all modes except for fBM.

The output of the Noise and Musgrave Texture nodes in Blender 4.0 is usually identical to that in Blender 4.1, but with a few exceptions:

The combined Noise Texture node has a Distortion input and a Color output, which the Musgrave Texture node doesn't. These inputs/outputs are ignored and any links connected to them are deleted.
The Musgrave Texture Detail input computes one octave fewer than the Noise Texture Detail input. The versioning code corrects that by setting Detail_{Musgrave}=Detail_{Noise} + 1.0 however when the Detail input in Blender 4.1 is set to a value >14.0 it will be clamped to a maximum of 15.0. This happens internally and can't be circumvented using nodes.
The Musgrave Texture Dimension and Lacunarity inputs are internally clamped to a minimum of 10^{-5}.
The Noise Texture Roughness input is internally clamped to a maximum of 1.0.

This PR adds the versioning code to convert the new [combined Noise Texture node](https://projects.blender.org/blender/blender/pulls/111187) in Blender 4.1 back to the Noise and Musgrave Texture nodes in Blender 4.0. Versioning is only done for the Musgrave exclusive noise modes, that is all modes except for `fBM`. The output of the Noise and Musgrave Texture nodes in Blender 4.0 is usually identical to that in Blender 4.1, but with a few exceptions: 1) The combined Noise Texture node has a Distortion input and a Color output, which the Musgrave Texture node doesn't. These inputs/outputs are ignored and any links connected to them are deleted. 2) The Musgrave Texture Detail input computes one octave fewer than the Noise Texture Detail input. The versioning code corrects that by setting $Detail_{Musgrave}=Detail_{Noise} + 1.0$ however when the Detail input in Blender 4.1 is set to a value $>14.0$ it will be clamped to a maximum of $15.0$. This happens internally and can't be circumvented using nodes. 3) The Musgrave Texture Dimension and Lacunarity inputs are internally clamped to a minimum of $10^{-5}$. 4) The Noise Texture Roughness input is internally clamped to a maximum of $1.0$.

Hoshinova added 1 commit 2023-10-28 23:52:18 +02:00

7001a0791f -- Implement Noise Texture versioning for forward compatibility with 4.1.

Hoshinova added 1 commit 2023-10-29 00:00:52 +02:00

ce3048296d -- Delete links.

Hoshinova referenced this pull request

2023-10-29 00:08:08 +02:00

Nodes: Merge Musgrave Texture Node Into Noise Texture Node #111187

Hoshinova added 1 commit 2023-10-29 00:32:47 +02:00

ff53849b78 -- Change node locations.

Hoshinova requested review from Brecht Van Lommel 2023-10-29 00:37:28 +02:00

Brecht Van Lommel requested changes 2023-10-30 17:05:21 +01:00

source/blender/blenloader/intern/versioning_400.cc Outdated

						
				@ -1062,0 +1063,4 @@

				{

				  LISTBASE_FOREACH (bNode *, node, &ntree->nodes) {

				    if ((node->type != SH_NODE_TEX_NOISE) ||

				        ((static_cast<NodeTexNoise *>(node->storage))->type == SHD_NOISE_FBM))

Brecht Van Lommel commented

2023-10-30 16:32:58 +01:00

Check that node->storage is not null, to avoid crashing on 4.1 files saved before the node merging.

Check that `node->storage` is not null, to avoid crashing on 4.1 files saved before the node merging.

Hoshinova commented

2023-10-30 21:07:15 +01:00

The Noise Texture already had a node->storage before node merging. How could it be null?

The Noise Texture already had a `node->storage` before node merging. How could it be null?

Brecht Van Lommel commented

2023-10-30 22:26:38 +01:00

Ah ok. But then isn't it a problem that when reading .blend files saved with current main (without the merged nodes), type will be 0? Which means it will run the versioning code for such .blend files?

Ah ok. But then isn't it a problem that when reading .blend files saved with current main (without the merged nodes), `type` will be `0`? Which means it will run the versioning code for such .blend files?

Hoshinova commented

2023-10-30 22:27:58 +01:00

I increased the subversion to the one where I expect the merge will happen. So that case shouldn't happen.

Brecht Van Lommel commented

2023-10-30 22:35:41 +01:00

But we don't know when the merge will happen, we should not guess this.

Hoshinova commented

2023-10-30 22:39:54 +01:00

I synced #111187 to also run with subversion 4. Since were still at subversion 1 we should have it merged by then.

I synced https://projects.blender.org/blender/blender/pulls/111187 to also run with subversion 4. Since were still at subversion 1 we should have it merged by then.

Brecht Van Lommel commented

2023-10-31 19:00:34 +01:00

I added checks for the existence of the sockets to make this work also with different subversion. This was needed anyway in case future Blender versions remove or rename these sockets.

brecht marked this conversation as resolved

source/blender/blenloader/intern/versioning_400.cc Outdated

						
				@ -1062,0 +1068,4 @@

				      continue;

				    }

				    LISTBASE_FOREACH_BACKWARD_MUTABLE (bNodeLink *, link, &ntree->links) {

Brecht Van Lommel commented

2023-10-30 16:33:38 +01:00

Why not use socket = nodeFindSocket and socket->link here? I see that in other versioning code, and it seems both simpler and faster. If there is no reason, please change it.

Why not use `socket = nodeFindSocket` and `socket->link` here? I see that in other versioning code, and it seems both simpler and faster. If there is no reason, please change it.

Hoshinova commented

2023-10-30 21:14:04 +01:00

Apparently this shouldn't be done anymore because socket->link may not be up to date. See: #111187 (comment) and #111187 (comment)

Apparently this shouldn't be done anymore because `socket->link` may not be up to date. See: https://projects.blender.org/blender/blender/pulls/111187#issuecomment-1007971 and https://projects.blender.org/blender/blender/pulls/111187#issuecomment-1030921

Brecht Van Lommel commented

2023-10-30 22:38:20 +01:00

I rather doubt this is needed, but I guess it doesn't hurt.

Hoshinova marked this conversation as resolved

source/blender/blenloader/intern/versioning_400.cc Outdated

						
				@ -1062,0 +1080,4 @@

				    STRNCPY(node->idname, "ShaderNodeTexMusgrave");

				    node->type = SH_NODE_TEX_MUSGRAVE;

				    NodeTexMusgrave *data = MEM_cnew<NodeTexMusgrave>(__func__);

				    data->base = (static_cast<NodeTexNoise *>(node->storage))->base;

Brecht Van Lommel commented

2023-10-30 17:04:59 +01:00

Use const NodeTextNoise *noise_data = static_cast<const NodeTexNoise *>(node->storage); instead of casting multiple times.

Use `const NodeTextNoise *noise_data = static_cast<const NodeTexNoise *>(node->storage);` instead of casting multiple times.

Hoshinova marked this conversation as resolved

source/blender/blenloader/intern/versioning_400.cc

						
				@ -1062,0 +1105,4 @@

				    bNode *lacunarity_from_node = nullptr;

				    bNodeSocket *lacunarity_from_socket = nullptr;

				    LISTBASE_FOREACH (bNodeLink *, link, &ntree->links) {

Brecht Van Lommel commented

2023-10-30 16:58:52 +01:00

Similar question here, why not use detail_socket->link below?

Similar question here, why not use `detail_socket->link` below?

Hoshinova commented

2023-10-30 21:17:20 +01:00

Same reason as above.

Hoshinova marked this conversation as resolved

source/blender/blenloader/intern/versioning_400.cc Outdated

						
				@ -1062,0 +1199,4 @@

				      }

				    }

				    else {

				      *dimension = -(std::logf(*dimension) / std::logf(*lacunarity));

Brecht Van Lommel commented

2023-10-30 17:03:08 +01:00

Use safe_logf and safe_divide to avoid potential NaN/inf.

Use `safe_logf` and `safe_divide` to avoid potential NaN/inf.

Hoshinova commented

2023-10-30 21:43:28 +01:00

The libraries containing safe_logf and safe_divide aren't #included in versioning_400.cc so here's a faster solution.

The libraries containing `safe_logf` and `safe_divide` aren't `#included` in versioning_400.cc so here's a faster solution.

Brecht Van Lommel commented

2023-10-30 22:24:56 +01:00

These are inline functions from BLI_math_base_safe.h, it should be enough to include that header.

These are inline functions from `BLI_math_base_safe.h`, it should be enough to include that header.

Brecht Van Lommel commented

2023-10-31 19:01:11 +01:00

I change it to safe_ now, since the previous fix did not account for negative values which are also not allowed.

I change it to `safe_` now, since the previous fix did not account for negative values which are also not allowed.

Hoshinova marked this conversation as resolved

Brecht Van Lommel commented

2023-10-30 17:05:58 +01:00

Please be sure to check this on 4.1 files saved with and without the merged noise texture node.

Hoshinova added 1 commit 2023-10-30 21:38:04 +01:00

7221d9241c Merge branch 'blender-v4.0-release' into merge-noise-versioning

Hoshinova added 1 commit 2023-10-30 21:55:36 +01:00

75d816b5d9 -- Increase versioning subversion.

Hoshinova commented

2023-10-30 21:56:49 +01:00

Please be sure to check this on 4.1 files saved with and without the merged noise texture node.

I did and it works.

> Please be sure to check this on 4.1 files saved with and without the merged noise texture node. I did and it works.

Hoshinova added 1 commit 2023-10-30 22:27:04 +01:00

9489347775 Merge branch 'blender-v4.0-release' into merge-noise-versioning

Brecht Van Lommel added 1 commit 2023-10-31 18:59:45 +01:00

buildbot/vexp-code-patch-coordinator Build done. Details

ed299dc286 * Use safe_log that also handles negative values, not just zero

* Check for existence of sockets to avoid need to check the subversion,
  and to guard against crashes when future Blender versions may remove
  or rename sockets.
* Minor code organization and style changes

Brecht Van Lommel commented

2023-10-31 18:59:54 +01:00

@blender-bot build

Brecht Van Lommel approved these changes 2023-10-31 19:01:20 +01:00

Brecht Van Lommel added 1 commit 2023-10-31 19:24:50 +01:00

buildbot/vexp-code-patch-coordinator Build done. Details

504fc84bfb Fix build error with safe_logf, add test

Brecht Van Lommel commented

2023-10-31 19:25:07 +01:00

@blender-bot build

Hoshinova reviewed 2023-10-31 19:38:25 +01:00

source/blender/blenloader/intern/versioning_400.cc Outdated

						
				@ -1062,0 +1212,4 @@

				      }

				    }

				    else {

				      *dimension = -safe_divide(safe_logef(*dimension), safe_logef(*lacunarity));