blender
/
blender
135
403
Fork 585
Code Issues 6.2k Pull Requests 760 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 +7 -7
1 changed files with 7 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 2b54cd81eb - Show all commits

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

@ -2990,19 +2990,19 @@ static bool ed_wpaint_vertex_select_pick(bContext *C,
*
* \returns true if the selection changed.
*/
static bool ed_curves_select_pick(bContext *C, const int mval[2], const SelectPick_Params *params)
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);
Depsgraph *depsgraph = CTX_data_ensure_evaluated_depsgraph(&C);
ViewContext vc;
/* Setup view context for argument to callbacks. */
ED_view3d_viewcontext_init(C, &vc, depsgraph);
ED_view3d_viewcontext_init(&C, &vc, depsgraph);
bool changed = false;
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];
@ -3014,14 +3014,14 @@ static bool ed_curves_select_pick(bContext *C, const int mval[2], const SelectPi
*curves_ob,
curves,
eAttrDomain(curves_id.selection_domain),
*params,
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);
WM_event_add_notifier(&C, NC_GEOM | ND_DATA, &curves_id);
}
}
@ -3121,7 +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) {
changed = ed_curves_select_pick(C, mval, &params);
changed = ed_curves_select_pick(*C, mval, params);
}
}
else if (obact && obact->mode & OB_MODE_PARTICLE_EDIT) {