blender/blender
154
492
Fork 729
Code Issues 6.7k Pull Requests 904 Projects 18 Wiki Activity

Fix #126074: Sculping on a non-basis shape key modifies mesh #126107

Merged
Sean Kim merged 2 commits from Sean-Kim/blender:fix-126074 into main 2024-08-09 22:34:49 +02:00
Conversation 1 Commits 2 Files Changed 1 +7 -1
1 changed files with 7 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
source/blender/editors/sculpt_paint/sculpt.cc
View File

@ -7315,7 +7315,13 @@ void write_translations(const Sculpt &sd,
apply_crazyspace_to_translations(ss.deform_imats, verts, translations);
}
apply_translations(translations, verts, positions_orig);
const Mesh &mesh = *static_cast<Mesh *>(object.data);
Sean-Kim marked this conversation as resolved Outdated
Iliya Katushenock commented 2024-08-08 22:10:47 +02:00
Outdated
Review
Copy Link

Can be const/
Same for key

Can be const/ Same for key
👍 1
const KeyBlock *active_key = BKE_keyblock_from_object(&object);
const bool relative_shapekey_active = active_key != nullptr && active_key != mesh.key->refkey;
if (!relative_shapekey_active) {
apply_translations(translations, verts, positions_orig);
Sean-Kim marked this conversation as resolved
Hans Goudey commented 2024-08-09 17:33:34 +02:00
Review
Copy Link

This comment says exactly what the code says (due to your helpful variable name); I don't think it's helpful. Generally this sort of comment makes it feel like the code is brittle, and code should document itself, making this sort of "what is happening" comment generally unnecessary.

This comment says exactly what the code says (due to your helpful variable name); I don't think it's helpful. Generally this sort of comment makes it feel like the code is brittle, and code should document itself, making this sort of "what is happening" comment generally unnecessary.
Sean Kim commented 2024-08-09 20:39:27 +02:00
Review
Copy Link

Yeah, that's fair. I was trying to think of more descriptive / "why" things to put here since this did cause a bug, but I agree that as is this is just duplicated info.

Yeah, that's fair. I was trying to think of more descriptive / "why" things to put here since this did cause a bug, but I agree that as is this is just duplicated info.
}
apply_translations_to_shape_keys(object, verts, translations, positions_orig);
}
Hans Goudey commented 2024-08-09 17:32:17 +02:00
Review
Copy Link

The apply_translations_to_shape_keys abstraction breaks down a bit now. I don't have a strong opinion for whether to do this now or later, but I'd probably remove the function and put it in here.

The `apply_translations_to_shape_keys` abstraction breaks down a bit now. I don't have a strong opinion for whether to do this now or later, but I'd probably remove the function and put it in here.
Sean Kim commented 2024-08-09 20:40:11 +02:00
Review
Copy Link

I'll follow up on this afterwards; there's another place that apply_translations_to_shape_keys is used in the undo code, so I don't want to clutter this fix with that cleanup.

I'll follow up on this afterwards; there's another place that `apply_translations_to_shape_keys` is used in the undo code, so I don't want to clutter this fix with that cleanup.
Hans Goudey commented 2024-08-09 20:41:06 +02:00
Review
Copy Link

Sounds good, thanks!

Sounds good, thanks!