blender
/
blender
136
411
Fork 595
Code Issues 6.3k Pull Requests 770 Projects 19 Wiki Activity

Fix: Vertex paint filter operators broken undo #106136

Merged
Joseph Eagar merged 3 commits from JosephEagar/blender:blender-v3.5-vpaint_fixes into blender-v3.5-release 2023-03-25 22:10:20 +01:00
Conversation 2 Commits 3 Files Changed 2 +7 -3
2 changed files with 7 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 4347e72de7 - Show all commits

4
source/blender/blenkernel/intern/paint.cc
View File

@ -1848,7 +1848,9 @@ static void sculpt_update_object(
if (is_paint_tool) {
if (ss->vcol_domain == ATTR_DOMAIN_CORNER) {
/* Ensure pbvh nodes have loop indices, needed for undo. */
/* Ensure pbvh nodes have loop indices; the sculpt undo system
Hans Goudey commented 2023-03-25 20:52:03 +01:00
Outdated
Review
Copy Link

This sort of comment ends up being confusing a year later, it says "needed for undo" but that brings up more questions than it answers-- why would "undo" need this data? what is meant by "undo" here? etc.

I don't know the actual reason, but I'd expect something like "needed in order to interpolate from face corners to vertices for undo step storage"

And one more question, could the sculpt undo system call this function directly rather than doing it here?

This sort of comment ends up being confusing a year later, it says "needed for undo" but that brings up more questions than it answers-- why would "undo" need this data? what is meant by "undo" here? etc. I don't know the actual reason, but I'd expect something like "needed in order to interpolate from face corners to vertices for undo step storage" And one more question, could the sculpt undo system call this function directly rather than doing it here?
* needs them for color attributes.
*/
BKE_pbvh_ensure_node_loops(ss->pbvh);
}

6
source/blender/editors/sculpt_paint/paint_vertex_color_ops.cc
View File

@ -13,7 +13,6 @@
#include "BLI_array.hh"
#include "BLI_index_mask_ops.hh"
#include "BLI_index_range.hh"
#include "BLI_math_base.h"
Hans Goudey commented 2023-03-25 20:53:01 +01:00
Outdated
Review
Copy Link

Not sure where this BKE_mesh_mapping.h include is coming from, it doesn't look necessary here.

Not sure where this `BKE_mesh_mapping.h` include is coming from, it doesn't look necessary here.
Hans Goudey commented 2023-03-25 21:59:33 +01:00
Review
Copy Link

This header is still added in this patch.

This header is still added in this patch.
#include "BLI_math_color.h"
#include "BLI_vector.hh"
@ -308,11 +307,14 @@ static void transform_active_color(bContext *C, wmOperator *op, const TransformF
BKE_pbvh_search_gather(obact->sculpt->pbvh, nullptr, nullptr, &nodes, &nodes_num);
for (int i : IndexRange(nodes_num)) {
SCULPT_undo_push_node(obact, nodes[i], SCULPT_UNDO_COLOR);
Hans Goudey commented 2023-03-25 20:55:56 +01:00
Review
Copy Link

I'm not sure, but it seems more clear/common to add the update tags directly after the data is changed rather than before.

I'm not sure, but it seems more clear/common to add the update tags directly after the data is changed rather than before.
BKE_pbvh_node_mark_update_color(nodes[i]);
}
transform_active_color_data(*BKE_mesh_from_object(obact), transform_fn);
for (int i : IndexRange(nodes_num)) {
BKE_pbvh_node_mark_update_color(nodes[i]);
}
SCULPT_undo_push_end(obact);
WM_event_add_notifier(C, NC_OBJECT | ND_DRAW, obact);
}