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 41 additions and 15 deletions
Showing only changes of commit db8141df4e - Show all commits

View File

@ -1847,6 +1847,11 @@ 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. */

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

View File

@ -13,6 +13,7 @@
#include "BLI_array.hh"
#include "BLI_index_mask_ops.hh"
#include "BLI_index_range.hh"

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.
Review

This header is still added in this patch.

This header is still added in this patch.
#include "BLI_math_base.h"
#include "BLI_math_color.h"
#include "BLI_vector.hh"
@ -22,6 +23,7 @@
#include "BKE_deform.h"
#include "BKE_geometry_set.hh"
#include "BKE_mesh.h"
#include "BKE_mesh_mapping.h"
#include "DEG_depsgraph.h"
@ -34,11 +36,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 +249,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 +290,31 @@ 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)) {
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.
SCULPT_undo_push_node(obact, nodes[i], SCULPT_UNDO_COLOR);
BKE_pbvh_node_mark_update_color(nodes[i]);
}
transform_active_color_data(*BKE_mesh_from_object(obact), transform_fn);
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 +350,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 +396,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 +413,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 +436,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 +446,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 +483,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);
}