blender
/
blender
130
398
Fork 573
Code Issues 6.2k Pull Requests 746 Projects 19 Wiki Activity

Fix #105109: Pick selection with multi object edit #105184

Merged
Falk David merged 4 commits from filedescriptor/blender:curves-fix-pick-selection-multi-object-editing into blender-v3.5-release 2023-03-02 15:43:26 +01:00
Conversation 0 Commits 4 Files Changed 3 +74 -34
3 changed files with 74 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
source/blender/editors/curves/intern/curves_selection.cc
View File

@ -361,14 +361,6 @@ static void apply_selection_operation_at_index(GMutableSpan selection,
}
}
/**
* Helper struct for `select_pick`.
*/
struct FindClosestData {
int index = -1;
float distance = FLT_MAX;
};
static bool find_closest_point_to_screen_co(const Depsgraph &depsgraph,
const ARegion *region,
const RegionView3D *rv3d,
@ -388,7 +380,7 @@ static bool find_closest_point_to_screen_co(const Depsgraph &depsgraph,
closest_data = threading::parallel_reduce(
curves.points_range(),
1024,
FindClosestData(),
closest_data,
[&](const IndexRange point_range, const FindClosestData &init) {
FindClosestData best_match = init;
for (const int point_i : point_range) {
@ -419,7 +411,7 @@ static bool find_closest_point_to_screen_co(const Depsgraph &depsgraph,
}
return b;
});
if (closest_data.index > 0) {
if (closest_data.index >= 0) {
return true;
}
@ -447,7 +439,7 @@ static bool find_closest_curve_to_screen_co(const Depsgraph &depsgraph,
closest_data = threading::parallel_reduce(
curves.curves_range(),
256,
FindClosestData(),
closest_data,
[&](const IndexRange curves_range, const FindClosestData &init) {
FindClosestData best_match = init;
for (const int curve_i : curves_range) {
@ -515,18 +507,21 @@ static bool find_closest_curve_to_screen_co(const Depsgraph &depsgraph,
}
bool select_pick(const ViewContext &vc,
const Object &object,
bke::CurvesGeometry &curves,
const eAttrDomain selection_domain,
const SelectPick_Params &params,
const int2 coord)
const int2 coord,
FindClosestData initial)
{
FindClosestData closest;
FindClosestData closest = initial;
bool found = false;
if (selection_domain == ATTR_DOMAIN_POINT) {
found = find_closest_point_to_screen_co(*vc.depsgraph,
vc.region,
vc.rv3d,
*vc.obact,
object,
curves,
float2(coord),
ED_view3d_select_dist_px(),
@ -536,22 +531,22 @@ bool select_pick(const ViewContext &vc,
found = find_closest_curve_to_screen_co(*vc.depsgraph,
vc.region,
vc.rv3d,
*vc.obact,
object,
curves,
float2(coord),
ED_view3d_select_dist_px(),
closest);
}
bool changed = false;
bool deselected = false;
if (params.sel_op == SEL_OP_SET) {
if (found || params.deselect_all) {
bke::GSpanAttributeWriter selection = ensure_selection_attribute(
curves, selection_domain, CD_PROP_BOOL);
bke::GSpanAttributeWriter selection = ensure_selection_attribute(
curves, selection_domain, CD_PROP_BOOL);
if (found || ((params.deselect_all && has_anything_selected(selection.span)))) {
fill_selection_false(selection.span);
selection.finish();
changed = true;
deselected = true;
}
selection.finish();
}
if (found) {
@ -561,7 +556,7 @@ bool select_pick(const ViewContext &vc,
selection.finish();
}
return changed || found;
return deselected || found;
}
bool select_box(const ViewContext &vc,

12
source/blender/editors/include/ED_curves.h
View File

@ -148,14 +148,24 @@ void select_random(bke::CurvesGeometry &curves,
uint32_t random_seed,
float probability);
/**
* Helper struct for `select_pick`.
*/
struct FindClosestData {
int index = -1;
float distance = FLT_MAX;
};
/**
* Select point or curve at a (screen-space) point.
*/
bool select_pick(const ViewContext &vc,
const Object &object,
bke::CurvesGeometry &curves,
eAttrDomain selection_domain,
const SelectPick_Params &params,
int2 coord);
int2 coord,
FindClosestData initial = {});
/**
* Select points or curves in a (screen-space) rectangle.

57
source/blender/editors/space_view3d/view3d_select.cc
View File

@ -2985,9 +2985,53 @@ static bool ed_wpaint_vertex_select_pick(bContext *C,
return changed || found;
}
static int view3d_select_exec(bContext *C, wmOperator *op)
/**
* Cursor selection for the Curves object.
*
* \returns true if the selection changed.
*/
static bool ed_curves_select_pick(bContext &C, const int mval[2], const SelectPick_Params &params)
filedescriptor marked this conversation as resolved
Hans Goudey commented 2023-02-27 16:16:45 +01:00
Review
Copy Link

Pass C and params as references

Pass `C` and `params` as references
{
using namespace blender;
Depsgraph *depsgraph = CTX_data_ensure_evaluated_depsgraph(&C);
ViewContext vc;
/* Setup view context for argument to callbacks. */
ED_view3d_viewcontext_init(&C, &vc, depsgraph);
uint bases_len;
filedescriptor marked this conversation as resolved
Hans Goudey commented 2023-02-27 16:17:06 +01:00
Review
Copy Link

Declare changed right above closest_data, closer to where it's set and used

Declare `changed` right above `closest_data`, closer to where it's set and used
Base **bases = BKE_view_layer_array_from_bases_in_edit_mode_unique_data(
vc.scene, vc.view_layer, vc.v3d, &bases_len);
bool changed = false;
ed::curves::FindClosestData closest_data;
for (uint base_index = 0; base_index < bases_len; base_index++) {
Base *base = bases[base_index];
Object *curves_ob = base->object;
Curves &curves_id = *static_cast<Curves *>(curves_ob->data);
bke::CurvesGeometry &curves = curves_id.geometry.wrap();
if (ed::curves::select_pick(vc,
*curves_ob,
curves,
eAttrDomain(curves_id.selection_domain),
params,
mval,
closest_data)) {
changed = true;
filedescriptor marked this conversation as resolved
Hans Goudey commented 2023-02-27 16:18:21 +01:00
Review
Copy Link

Seems like multiple curve objects could get a dependency graph tag, when this operator is only supposed to affect one of them. Maybe it needs to keep track of which object has the closets element and only tag the update at the end.

Seems like multiple curve objects could get a dependency graph tag, when this operator is only supposed to affect one of them. Maybe it needs to keep track of which object has the closets element and only tag the update at the end.
Falk David commented 2023-02-27 16:25:54 +01:00
Review
Copy Link

That could work except that click select can also cause a deselection of elements in the other objects. It would have to be more sophisticated

That could work except that click select can also cause a deselection of elements in the other objects. It would have to be more sophisticated
Hans Goudey commented 2023-02-27 17:28:58 +01:00
Review
Copy Link

I think it's worth solving that here. Otherwise that leaves some unknown restructuring of the logic for later, and these reevaluations often aren't trivial.

The "found" and "changed" return states could be split up maybe?

I think it's worth solving that here. Otherwise that leaves some unknown restructuring of the logic for later, and these reevaluations often aren't trivial. The "found" and "changed" return states could be split up maybe?
/* Use #ID_RECALC_GEOMETRY instead of #ID_RECALC_SELECT because it is handled as a
* generic attribute for now. */
DEG_id_tag_update(&curves_id.id, ID_RECALC_GEOMETRY);
WM_event_add_notifier(&C, NC_GEOM | ND_DATA, &curves_id);
}
}
MEM_freeN(bases);
return changed;
}
static int view3d_select_exec(bContext *C, wmOperator *op)
{
Scene *scene = CTX_data_scene(C);
Object *obedit = CTX_data_edit_object(C);
Object *obact = CTX_data_active_object(C);
@ -3077,16 +3121,7 @@ static int view3d_select_exec(bContext *C, wmOperator *op)
changed = ED_curve_editfont_select_pick(C, mval, &params);
}
else if (obedit->type == OB_CURVES) {
Curves &curves_id = *static_cast<Curves *>(obact->data);
bke::CurvesGeometry &curves = curves_id.geometry.wrap();
changed = ed::curves::select_pick(
vc, curves, eAttrDomain(curves_id.selection_domain), params, mval);
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(&curves_id.id, ID_RECALC_GEOMETRY);
WM_event_add_notifier(C, NC_GEOM | ND_DATA, &curves_id);
}
changed = ed_curves_select_pick(*C, mval, params);
}
}
else if (obact && obact->mode & OB_MODE_PARTICLE_EDIT) {