blender
/
blender
136
407
Fork 590
Code Issues 6.3k Pull Requests 777 Projects 19 Wiki Activity

Cycles: Fix Principled BSDF versioning for keyframes and drivers #114300

Merged
Lukas Stockner merged 1 commits from LukasStockner/blender:fix-principled-animdata into blender-v4.0-release 2023-11-01 19:27:59 +01:00
Conversation 1 Commits 1 Files Changed 1 +66
1 changed files with 66 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
source/blender/blenloader/intern/versioning_400.cc
View File

@ -43,6 +43,8 @@
#include "BLI_string.h"
#include "BLI_string_ref.hh"
#include "BKE_anim_data.h"
#include "BKE_animsys.h"
#include "BKE_armature.h"
#include "BKE_attribute.h"
#include "BKE_curve.h"
@ -242,6 +244,60 @@ static void version_bonegroups_to_bonecollections(Main *bmain)
}
}
static void version_principled_bsdf_update_animdata(ID *owner_id, bNodeTree *ntree)
{
ID *id = &ntree->id;
AnimData *adt = BKE_animdata_from_id(id);
LISTBASE_FOREACH (bNode *, node, &ntree->nodes) {
if (node->type != SH_NODE_BSDF_PRINCIPLED) {
continue;
}
char node_name_escaped[MAX_NAME * 2];
BLI_str_escape(node_name_escaped, node->name, sizeof(node_name_escaped));
std::string prefix = "nodes[\"" + std::string(node_name_escaped) + "\"].inputs";
/* Remove animdata for inputs 18 (Transmission Roughness) and 3 (Subsurface Color). */
BKE_animdata_fix_paths_remove(id, (prefix + "[18]").c_str());
BKE_animdata_fix_paths_remove(id, (prefix + "[3]").c_str());
Brecht Van Lommel commented 2023-11-01 15:15:46 +01:00
Outdated
Review
Copy Link

This looks like it loops over all node trees in the file, not just this one?

This looks like it loops over all node trees in the file, not just this one?
Lukas Stockner commented 2023-11-01 18:56:34 +01:00
Outdated
Review
Copy Link

Yes, seems like it does. However, as far as I can tell, BKE_animdata_fix_paths_rename_all_ex does the same - the nodetree ID that is being passed into it is only used in one bone-specific check in drivers_path_rename_fix, which doesn't apply here. And that function is used for existing node versioning.

For the test file from the report, everything actually also works if we don't use BKE_animdata_fix_paths_rename_all_ex and BKE_animdata_main_cb for iteration, and instead just call BKE_animdata_fix_paths_remove/BKE_animdata_fix_paths_rename directly on the nodetree.
But I guess there is some reason to have the iteration in the original code? Not sure how all the ownership works here.

I'll update the code to skip the iteration for now, since it still seems to work.

Yes, seems like it does. However, as far as I can tell, `BKE_animdata_fix_paths_rename_all_ex` does the same - the nodetree ID that is being passed into it is only used in one bone-specific check in `drivers_path_rename_fix`, which doesn't apply here. And that function is used for existing node versioning. For the test file from the report, everything actually also works if we don't use `BKE_animdata_fix_paths_rename_all_ex` and `BKE_animdata_main_cb` for iteration, and instead just call `BKE_animdata_fix_paths_remove`/`BKE_animdata_fix_paths_rename` directly on the nodetree. But I guess there is some reason to have the iteration in the original code? Not sure how all the ownership works here. I'll update the code to skip the iteration for now, since it still seems to work.
/* Order is important here: If we e.g. want to change A->B and B->C, but perform A->B first,
* then later we don't know whether a B entry is an original B (and therefore should be
* changed to C) or used to be A and was already handled.
* In practise, going reverse mostly works, the two notable dependency chains are:
* - 8->13, then 2->8, then 9->2 (13 was changed before)
* - 1->9, then 6->1 (9 was changed before)
* - 4->10, then 21->4 (10 was changed before)
*
* 0 (Base Color) and 17 (Transmission) are fine as-is. */
std::pair<int, int> remap_table[] = {
{20, 27}, /* Emission Strength */
{19, 26}, /* Emission */
{16, 3}, /* IOR */
{15, 19}, /* Clearcoat Roughness */
{14, 18}, /* Clearcoat */
{13, 25}, /* Sheen Tint */
{12, 23}, /* Sheen */
{11, 15}, /* Anisotropic Rotation */
{10, 14}, /* Anisotropic */
{8, 13}, /* Specular Tint */
{2, 8}, /* Subsurface Radius */
{9, 2}, /* Roughness */
{7, 12}, /* Specular */
{1, 9}, /* Subsurface Scale */
{6, 1}, /* Metallic */
{5, 11}, /* Subsurface Anisotropy */
{4, 10}, /* Subsurface IOR */
{21, 4} /* Alpha */
};
for (const auto &entry : remap_table) {
BKE_animdata_fix_paths_rename(
id, adt, owner_id, prefix.c_str(), nullptr, nullptr, entry.first, entry.second, false);
}
}
}
void do_versions_after_linking_400(FileData *fd, Main *bmain)
{
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 400, 9)) {
@ -312,6 +368,16 @@ void do_versions_after_linking_400(FileData *fd, Main *bmain)
}
}
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 400, 24)) {
FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
if (ntree->type == NTREE_SHADER) {
/* Convert animdata on the Principled BSDF sockets. */
version_principled_bsdf_update_animdata(id, ntree);
}
}
FOREACH_NODETREE_END;
}
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 400, 34)) {
BKE_mesh_legacy_face_map_to_generic(bmain);
}