Draw Face Sets does not work in Dyntopo and the sculpt API should be
responsible for that without needing to add checks all over the code,
but it was doing an undo push of type SCULPT_UNDO_FACE_SETS which is not
supported, causing the crash.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
Dyntopo and the sculpt API should be responsible for that without needing to add checks all over the code
Exactly my thoughts. So intuitively this belongs to sculpt_undo.c which takes care of all mode-specific logic and does proper thing.
So why is this solved on the "tool" level rather than on sculpt undo level? You can have a no-op placeholder there with a TODO.
| source/blender/editors/sculpt_paint/sculpt.c | ||
|---|---|---|
| 5326 | Usually comment answers "why", not :"what". Surely, in some cases of non-trivial optimized code having answer for "what" question adds a lot of value, and sometimes it also helps to split up code visually. Here it definitely deserved to be a "why" answer, since it's clear that you skip undo for dyntopo, but it is not clear why. But also read non-inlined comment. | |
Comment Actions
I clarified the comment about why the check is done there. I also tried to put the check inside the SCULPT_undo_push_node function and it seems to work fine, but this seems safer for adding the fix to the LTS. When vertex colors are added they are going to have the same problem, so probably the undo code needs to be refactored to check for unsupported types and multiple types in the same node
Comment Actions
If it's aimed for LTS, then it indeed sounds safer. And indeed undo needs some love.
Now, since you're mentioning LTS, @Jeroen Bakker (jbakker) coordinates that. Not sure if he wants to do extra checks now. At a very least he gets a gentle poke now :)