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
2 changed files with 44 additions and 15 deletions

View File

@ -1847,6 +1847,13 @@ static void sculpt_update_object(
}
if (is_paint_tool) {
if (ss->vcol_domain == ATTR_DOMAIN_CORNER) {
/* Ensure pbvh nodes have loop indices; the sculpt undo system

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);
}
/*
* We should rebuild the PBVH_pixels when painting canvas changes.
*

View File

@ -34,11 +34,13 @@
#include "ED_mesh.h"
#include "paint_intern.h" /* own include */
#include "sculpt_intern.hh"
using blender::Array;
using blender::ColorGeometry4f;
using blender::GMutableSpan;
using blender::IndexMask;
using blender::IndexRange;
using blender::Vector;
/* -------------------------------------------------------------------- */
@ -245,20 +247,20 @@ void PAINT_OT_vertex_color_smooth(wmOperatorType *ot)
* \{ */
template<typename TransformFn>
static bool transform_active_color(Mesh &mesh, const TransformFn &transform_fn)
static void transform_active_color_data(Mesh &mesh, const TransformFn &transform_fn)
{
using namespace blender;
const StringRef name = mesh.active_color_attribute;
bke::MutableAttributeAccessor attributes = mesh.attributes_for_write();
if (!attributes.contains(name)) {
BLI_assert_unreachable();
return false;
return;
}
bke::GAttributeWriter color_attribute = attributes.lookup_for_write(name);
if (!color_attribute) {
BLI_assert_unreachable();
return false;
return;
}
Vector<int64_t> indices;
@ -286,8 +288,34 @@ static bool transform_active_color(Mesh &mesh, const TransformFn &transform_fn)
color_attribute.finish();
DEG_id_tag_update(&mesh.id, 0);
}
return true;
template<typename TransformFn>
static void transform_active_color(bContext *C, wmOperator *op, const TransformFn &transform_fn)
{
Object *obact = CTX_data_active_object(C);
/* Ensure valid sculpt state. */
BKE_sculpt_update_object_for_edit(
CTX_data_ensure_evaluated_depsgraph(C), obact, true, false, true);
SCULPT_undo_push_begin(obact, op);
PBVHNode **nodes;
int nodes_num;
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);
}
Review

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.
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);
}
static int vertex_color_brightness_contrast_exec(bContext *C, wmOperator *op)
@ -323,14 +351,12 @@ static int vertex_color_brightness_contrast_exec(bContext *C, wmOperator *op)
return OPERATOR_CANCELLED;
}
transform_active_color(*me, [&](ColorGeometry4f &color) {
transform_active_color(C, op, [&](ColorGeometry4f &color) {
for (int i = 0; i < 3; i++) {
color[i] = gain * color[i] + offset;
}
});
WM_event_add_notifier(C, NC_OBJECT | ND_DRAW, obact);
return OPERATOR_FINISHED;
}
@ -371,7 +397,7 @@ static int vertex_color_hsv_exec(bContext *C, wmOperator *op)
return OPERATOR_CANCELLED;
}
transform_active_color(*me, [&](ColorGeometry4f &color) {
transform_active_color(C, op, [&](ColorGeometry4f &color) {
float hsv[3];
rgb_to_hsv_v(color, hsv);
@ -388,8 +414,6 @@ static int vertex_color_hsv_exec(bContext *C, wmOperator *op)
hsv_to_rgb_v(hsv, color);
});
WM_event_add_notifier(C, NC_OBJECT | ND_DRAW, obact);
return OPERATOR_FINISHED;
}
@ -413,7 +437,7 @@ void PAINT_OT_vertex_color_hsv(wmOperatorType *ot)
RNA_def_float(ot->srna, "v", 1.0f, 0.0f, 2.0f, "Value", "", 0.0f, 2.0f);
}
static int vertex_color_invert_exec(bContext *C, wmOperator * /*op*/)
static int vertex_color_invert_exec(bContext *C, wmOperator *op)
{
Object *obact = CTX_data_active_object(C);
@ -423,14 +447,12 @@ static int vertex_color_invert_exec(bContext *C, wmOperator * /*op*/)
return OPERATOR_CANCELLED;
}
transform_active_color(*me, [&](ColorGeometry4f &color) {
transform_active_color(C, op, [&](ColorGeometry4f &color) {
for (int i = 0; i < 3; i++) {
color[i] = 1.0f - color[i];
}
});
WM_event_add_notifier(C, NC_OBJECT | ND_DRAW, obact);
return OPERATOR_FINISHED;
}
@ -462,7 +484,7 @@ static int vertex_color_levels_exec(bContext *C, wmOperator *op)
return OPERATOR_CANCELLED;
}
transform_active_color(*me, [&](ColorGeometry4f &color) {
transform_active_color(C, op, [&](ColorGeometry4f &color) {
for (int i = 0; i < 3; i++) {
color[i] = gain * (color[i] + offset);
}