Curves: Add box selection #104411

Merged
Falk David merged 5 commits from filedescriptor/blender:curves-mouse-selection into main 2023-02-09 15:53:44 +01:00
4 changed files with 409 additions and 250 deletions

File diff suppressed because it is too large Load Diff

View File

@ -7,6 +7,7 @@
#include "BLI_array_utils.hh"
#include "BLI_index_mask_ops.hh"
#include "BLI_rand.hh"
#include "BLI_rect.h"
#include "BKE_attribute.hh"
#include "BKE_crazyspace.hh"
@ -285,6 +286,46 @@ void select_random(bke::CurvesGeometry &curves,
selection.finish();
}
static void apply_selection_operation_at_index(GMutableSpan selection,
filedescriptor marked this conversation as resolved
Review

Theoretically this function could return whether the value was changed. Maybe a premature optimization though..

Theoretically this function could return whether the value was changed. Maybe a premature optimization though..
const int index,
filedescriptor marked this conversation as resolved
Review

const int index, const eSelectOp sel_op just for consistency.

`const int index, const eSelectOp sel_op` just for consistency.
const eSelectOp sel_op)
{
if (selection.type().is<bool>()) {
MutableSpan<bool> selection_typed = selection.typed<bool>();
switch (sel_op) {
case SEL_OP_ADD:
case SEL_OP_SET:
selection_typed[index] = true;
break;
case SEL_OP_SUB:
selection_typed[index] = false;
break;
case SEL_OP_XOR:
selection_typed[index] ^= selection_typed[index];
break;
default:
break;
}
}
else if (selection.type().is<float>()) {
MutableSpan<float> selection_typed = selection.typed<float>();
switch (sel_op) {
case SEL_OP_ADD:
case SEL_OP_SET:
selection_typed[index] = 1.0f;
break;
case SEL_OP_SUB:
selection_typed[index] = 0.0f;
break;
case SEL_OP_XOR:
selection_typed[index] = 1.0f - selection_typed[index];
break;
default:
break;
}
}
}
/**
* Helper struct for `find_closest_point_to_screen_co`.
*/
@ -392,33 +433,69 @@ bool select_pick(const ViewContext &vc,
elem_index = std::distance(curves.offsets().begin(), it) - 1;
}
selection.span.type().to_static_type_tag<bool, float>([&](auto type_tag) {
using T = typename decltype(type_tag)::type;
if constexpr (std::is_void_v<T>) {
BLI_assert_unreachable();
}
else {
MutableSpan<T> selection_typed = selection.span.typed<T>();
switch (params.sel_op) {
case SEL_OP_ADD:
case SEL_OP_SET:
selection_typed[elem_index] = T(1);
break;
case SEL_OP_SUB:
selection_typed[elem_index] = T(0);
break;
case SEL_OP_XOR:
selection_typed[elem_index] = T(1 - selection_typed[elem_index]);
break;
default:
break;
}
}
});
apply_selection_operation_at_index(selection.span, elem_index, params.sel_op);
selection.finish();
}
return changed || found;
}
bool select_box(const ViewContext &vc,
bke::CurvesGeometry &curves,
const eAttrDomain selection_domain,
const rcti &rect,
const eSelectOp sel_op)
{
rctf rectf;
BLI_rctf_rcti_copy(&rectf, &rect);
bke::GSpanAttributeWriter selection = ensure_selection_attribute(
curves, selection_domain, CD_PROP_BOOL);
bool changed = false;
filedescriptor marked this conversation as resolved
Review

Not sure if it would make a difference here, but it's usually preferred to move as much code as possible outside of the to_static_type lambda to avoid generating unnecessary code. That also makes the purpose of the to_static_type clearer and makes it easier to replace it with a generic utility with a runtime type in the future.

In this case, since this isn't really a performance bottleneck, it would probably be best for apply_selection_operation to take a GMutableSpan and do the type switch itself.

Not sure if it would make a difference here, but it's usually preferred to move as much code as possible outside of the `to_static_type` lambda to avoid generating unnecessary code. That also makes the purpose of the `to_static_type` clearer and makes it easier to replace it with a generic utility with a runtime type in the future. In this case, since this isn't really a performance bottleneck, it would probably be best for `apply_selection_operation` to take a `GMutableSpan` and do the type switch itself.
if (sel_op == SEL_OP_SET) {
fill_selection_false(selection.span);
changed = true;
}
float4x4 projection;
ED_view3d_ob_project_mat_get(vc.rv3d, vc.obact, projection.ptr());
const bke::crazyspace::GeometryDeformation deformation =
filedescriptor marked this conversation as resolved
Review

It would be nice to extract this switch and share it with the select pick operator too, is that possible?

It would be nice to extract this switch and share it with the select pick operator too, is that possible?
bke::crazyspace::get_evaluated_curves_deformation(*vc.depsgraph, *vc.obact);
const OffsetIndices points_by_curve = curves.points_by_curve();
if (selection_domain == ATTR_DOMAIN_POINT) {
threading::parallel_for(curves.points_range(), 1024, [&](const IndexRange point_range) {
for (const int point_i : point_range) {
float2 pos_proj;
ED_view3d_project_float_v2_m4(
vc.region, deformation.positions[point_i], pos_proj, projection.ptr());
if (BLI_rctf_isect_pt_v(&rectf, pos_proj)) {
apply_selection_operation_at_index(selection.span, point_i, sel_op);
changed = true;
}
}
});
}
else if (selection_domain == ATTR_DOMAIN_CURVE) {
threading::parallel_for(curves.curves_range(), 512, [&](const IndexRange curves_range) {
for (const int curve_i : curves_range) {
for (const int point_i : points_by_curve[curve_i]) {
float2 pos_proj;
ED_view3d_project_float_v2_m4(
vc.region, deformation.positions[point_i], pos_proj, projection.ptr());
if (BLI_rctf_isect_pt_v(&rectf, pos_proj)) {
apply_selection_operation_at_index(selection.span, curve_i, sel_op);
changed = true;
break;
}
}
}
});
}
selection.finish();
return changed;
}
} // namespace blender::ed::curves

View File

@ -11,6 +11,7 @@ struct Curves;
struct UndoType;
struct SelectPick_Params;
struct ViewContext;
struct rcti;
#ifdef __cplusplus
extern "C" {
@ -46,6 +47,8 @@ float (*ED_curves_point_normals_array_create(const struct Curves *curves_id))[3]
# include "BKE_curves.hh"
# include "ED_select_utils.h"
namespace blender::ed::curves {
bool object_has_editable_curves(const Main &bmain, const Object &object);
@ -146,6 +149,14 @@ bool select_pick(const ViewContext &vc,
const SelectPick_Params &params,
const int2 mval);
/**
* Select points or curves in a (screenspace) rectangle.
*/
bool select_box(const ViewContext &vc,
bke::CurvesGeometry &curves,
const eAttrDomain selection_domain,
const rcti& rect,
filedescriptor marked this conversation as resolved
Review

const rcti &rect,

I guess it passes, the "small" test, but maybe better to just default to const reference here. Also struct is unnecessary in C++

` const rcti &rect, ` I guess it passes, the "small" test, but maybe better to just default to const reference here. Also `struct` is unnecessary in C++
const eSelectOp sel_op);
/** \} */
} // namespace blender::ed::curves

View File

@ -3893,6 +3893,7 @@ static bool do_pose_box_select(bContext *C, ViewContext *vc, rcti *rect, const e
static int view3d_box_select_exec(bContext *C, wmOperator *op)
{
using namespace blender;
Depsgraph *depsgraph = CTX_data_ensure_evaluated_depsgraph(C);
ViewContext vc;
rcti rect;
@ -3955,6 +3956,19 @@ static int view3d_box_select_exec(bContext *C, wmOperator *op)
WM_event_add_notifier(C, NC_GEOM | ND_SELECT, vc.obedit->data);
}
break;
case OB_CURVES: {
Curves &curves_id = *static_cast<Curves *>(vc.obact->data);
bke::CurvesGeometry &curves = curves_id.geometry.wrap();
changed = ed::curves::select_box(
vc, curves, eAttrDomain(curves_id.selection_domain), rect, sel_op);
if (changed) {
/* Use #ID_RECALC_GEOMETRY instead of #ID_RECALC_SELECT because it is handled as a
* generic attribute for now. */
DEG_id_tag_update(static_cast<ID *>(vc.obedit->data), ID_RECALC_GEOMETRY);
WM_event_add_notifier(C, NC_GEOM | ND_DATA, vc.obedit->data);
}
break;
}
default:
BLI_assert_msg(0, "box select on incorrect object type");
break;