Cycles: Merge Anisotropic BSDF node into Glossy BSDF node #104445

Lukas Stockner · 2023-02-08T00:33:11+01:00

Lukas Stockner commented

2023-02-08 00:33:11 +01:00

Used to be https://archive.blender.org/developer/D17123.

Internally these are already using the same code path anyways, there's no point in maintaining two distinct nodes.

The obvious approach would be to add Anisotropy controls to the Glossy BSDF node and remove the Anisotropic BSDF node. However, that would break forward compability, since older Blender versions don't know how to handle the Anisotropy input on the Glossy BSDF node.

Therefore, this commit technically removes the Glossy BSDF node, uses versioning to replace them with an Anisotropic BSDF node, and renames that node to "Glossy BSDF".

That way, when you open a new file in an older version, all the nodes show up as Anisotropic BSDF nodes and render correctly.

This is a bit ugly internally since we need to preserve the old idname which now no longer matches the UI name, but that's not too bad.

Also removes the "Sharp" distribution option and replaces it with GGX, sets Roughness to zero and disconnects any input to the Roughness socket.

Used to be https://archive.blender.org/developer/D17123. Internally these are already using the same code path anyways, there's no point in maintaining two distinct nodes. The obvious approach would be to add Anisotropy controls to the Glossy BSDF node and remove the Anisotropic BSDF node. However, that would break forward compability, since older Blender versions don't know how to handle the Anisotropy input on the Glossy BSDF node. Therefore, this commit technically removes the Glossy BSDF node, uses versioning to replace them with an Anisotropic BSDF node, and renames that node to "Glossy BSDF". That way, when you open a new file in an older version, all the nodes show up as Anisotropic BSDF nodes and render correctly. This is a bit ugly internally since we need to preserve the old `idname` which now no longer matches the UI name, but that's not too bad. Also removes the "Sharp" distribution option and replaces it with GGX, sets Roughness to zero and disconnects any input to the Roughness socket.

❤️ 1

Lukas Stockner requested review from Brecht Van Lommel 2023-02-08 00:33:30 +01:00

Brecht Van Lommel added this to the Render & Cycles project 2023-02-08 03:11:05 +01:00

Lukas Stockner changed title from ~~Merge Anisotropic BSDF node into Glossy BSDF node~~ to Cycles: Merge Anisotropic BSDF node into Glossy BSDF node

2023-02-08 13:16:32 +01:00

Brecht Van Lommel commented

2023-02-08 22:41:03 +01:00

@blender-bot build

Blender Bot commented

2023-02-08 22:41:05 +01:00

Only blender organization members with write access can start builds. See documentation for details.

Only blender organization members with write access can start builds. See [documentation](https://projects.blender.org/infrastructure/blender-bot) for details.

Lukas Stockner force-pushed remove-anisotropic-bsdf-node from fe049753ac to 4426121a07

2023-02-11 21:28:53 +01:00

Compare

Lukas Stockner requested review from Weizhen Huang 2023-02-11 21:32:49 +01:00

Brecht Van Lommel commented

2023-02-17 15:07:14 +01:00

The one place where this is still breaking compatibility is the Python API. We could provide most of the compatibility like this:

/* For backwards compatibility. */
void nodeRegisterTypeAlias(bNodeType *nt, const char *alias_idname)
{
  BLI_ghash_insert(nodetypes_hash, alias_idname, nt);
}

  nodeRegisterTypeAlias(&ntype, "ShaderNodeBsdfGlossy");

Not great but still worth doing I think.

The one place where this is still breaking compatibility is the Python API. We could provide most of the compatibility like this: ```C /* For backwards compatibility. */ void nodeRegisterTypeAlias(bNodeType *nt, const char *alias_idname) { BLI_ghash_insert(nodetypes_hash, alias_idname, nt); } ``` ```C nodeRegisterTypeAlias(&ntype, "ShaderNodeBsdfGlossy"); ``` Not great but still worth doing I think.

Brecht Van Lommel requested changes 2023-02-17 15:07:55 +01:00

Brecht Van Lommel left a comment

LGTM except for the note about API compatibility.

Lukas Stockner commented

2023-05-08 00:06:34 +02:00

The one place where this is still breaking compatibility is the Python API. We could provide most of the compatibility like this:
/* For backwards compatibility. */
void nodeRegisterTypeAlias(bNodeType *nt, const char *alias_idname)
{
  BLI_ghash_insert(nodetypes_hash, alias_idname, nt);
}
  nodeRegisterTypeAlias(&ntype, "ShaderNodeBsdfGlossy");
Not great but still worth doing I think.

From what I can tell the node type list is used by NODE_TYPES_BEGIN, which would then iterate over the same node type twice, which would cause some double-free etc. issues.
Also, the new node would end up with the alias in idname, which would probably also cause issues?
It might be easier to apply the alias at the RNA level.

> The one place where this is still breaking compatibility is the Python API. We could provide most of the compatibility like this: > ```C > /* For backwards compatibility. */ > void nodeRegisterTypeAlias(bNodeType *nt, const char *alias_idname) > { > BLI_ghash_insert(nodetypes_hash, alias_idname, nt); > } > ``` > > ```C > nodeRegisterTypeAlias(&ntype, "ShaderNodeBsdfGlossy"); > ``` > > Not great but still worth doing I think. From what I can tell the node type list is used by `NODE_TYPES_BEGIN`, which would then iterate over the same node type twice, which would cause some double-free etc. issues. Also, the new node would end up with the alias in `idname`, which would probably also cause issues? It might be easier to apply the alias at the RNA level.

Lukas Stockner force-pushed remove-anisotropic-bsdf-node from 4426121a07 to 34792de366

2023-05-08 01:10:34 +02:00

Compare

Brecht Van Lommel commented

2023-05-10 21:09:26 +02:00

Let's not worry about API compatibility then, and just land this in 4.0 where we can make such breaking changes. We will likely rename a bunch of nodes and sockets anyway.

We just need to remember to add it to the release notes.

Let's not worry about API compatibility then, and just land this in 4.0 where we can make such breaking changes. We will likely rename a bunch of nodes and sockets anyway. We just need to remember to add it to the release notes.

Brecht Van Lommel approved these changes 2023-05-10 21:09:36 +02:00

Lukas Stockner added the

Module

Render & Cycles

label 2023-05-16 02:16:58 +02:00

Lukas Stockner added this to the 4.0 milestone 2023-05-16 02:17:01 +02:00

Lukas Stockner force-pushed remove-anisotropic-bsdf-node from 34792de366 to 7ed90ebd90

2023-05-18 23:09:54 +02:00

Compare

Lukas Stockner merged commit 8cde7d8f8a into main

2023-05-18 23:12:28 +02:00

Lukas Stockner deleted branch remove-anisotropic-bsdf-node

2023-05-18 23:12:29 +02:00

Lukas Stockner referenced this issue from a commit

2023-05-18 23:12:36 +02:00

Cycles: Merge Anisotropic BSDF node into Glossy BSDF node

Falk David referenced this issue from a commit

2023-05-23 14:35:34 +02:00

Cycles: Merge Anisotropic BSDF node into Glossy BSDF node

Brecht Van Lommel removed this from the Render & Cycles project 2023-05-24 19:05:20 +02:00

Howard Trickey referenced this issue from a commit

2023-05-29 02:51:38 +02:00

Merge in orign/main:

Bastien Montagne commented

2023-06-07 17:16:24 +02:00

Commit 8cde7d8f8a adds versioning code to versioning_300.cc, when it only exists in 4.0 and not 3.6. Think it should rather be moved to versioning_400.cc?

    /* Convert anisotropic BSDF node to glossy BSDF. */
    FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
      versioning_replace_legacy_glossy_node(ntree);
      versioning_remove_microfacet_sharp_distribution(ntree);
    }
    FOREACH_NODETREE_END;

Commit 8cde7d8f8a5a07 adds versioning code to `versioning_300.cc`, when it only exists in 4.0 and not 3.6. Think it should rather be moved to `versioning_400.cc`? ``` /* Convert anisotropic BSDF node to glossy BSDF. */ FOREACH_NODETREE_BEGIN (bmain, ntree, id) { versioning_replace_legacy_glossy_node(ntree); versioning_remove_microfacet_sharp_distribution(ntree); } FOREACH_NODETREE_END; ```

Brecht Van Lommel referenced this issue from a commit

2023-06-07 18:04:56 +02:00

Fix anisotropic BSDF versioning code being in the wrong place

Brecht Van Lommel commented

2023-06-07 18:05:53 +02:00

I fixed it now, since these were failing after this was made to only run for 3.x files.

👍 2

Brecht Van Lommel referenced this issue from a commit

2023-07-06 16:00:09 +02:00

Merge anisotropic bsdf node into glossy bsdf node

Brecht Van Lommel referenced this pull request

2023-08-14 16:38:17 +02:00