GPv3: convert selection domain on curves when selection mode changes #109964

Sietse Brouwer · 2023-07-11T12:44:59+02:00

Sietse Brouwer commented

2023-07-11 12:44:59 +02:00

When the selection mode in the Grease Pencil tool settings changes,
the selection domain of all curves in the active Grease Pencil object
has to change.
This PR handles that conversion. Point selections are converted to
curve selection and vice versa.

Note: this PR only takes care of selection modes 'Point' and 'Stroke'.
'Segment' mode is handled in a separate PR (#109221).

When the selection mode in the Grease Pencil tool settings changes, the selection domain of all curves in the active Grease Pencil object has to change. This PR handles that conversion. Point selections are converted to curve selection and vice versa. Note: this PR only takes care of selection modes 'Point' and 'Stroke'. 'Segment' mode is handled in a separate PR (#109221).

Sietse Brouwer added 1 commit 2023-07-11 12:45:11 +02:00

5aa298c41e GPv3: convert selection domain on curves when selection mode changes

Sietse Brouwer requested review from Falk David 2023-07-11 12:45:24 +02:00

Sietse Brouwer added this to the Grease Pencil project 2023-07-11 12:45:29 +02:00

Falk David requested changes 2023-07-11 13:04:14 +02:00

Falk David left a comment

First pass.

source/blender/editors/grease_pencil/intern/grease_pencil_select.cc

						
				@ -323,0 +334,4 @@

				  for (const int index : drawings.index_range()) {

				    GreasePencilDrawingBase *drawing_base = drawings[index];

				    if (drawing_base->type == GP_DRAWING) {

Falk David commented

2023-07-11 12:57:55 +02:00

Since bases of type GP_DRAWING_REFERENCE are never editable, we can just skip them. So instead of this if, we can just do:

if (drawing_base->type != GP_DRAWING) {
   continue;
}

Since bases of type `GP_DRAWING_REFERENCE` are never editable, we can just skip them. So instead of this `if`, we can just do: ``` if (drawing_base->type != GP_DRAWING) { continue; } ```

SietseB marked this conversation as resolved

source/blender/editors/grease_pencil/intern/grease_pencil_select.cc Outdated

						
				@ -323,0 +344,4 @@

				      /* Skip curve when the selection domain already matches, or when there is no selection

				       * at all. */

				      bke::MutableAttributeAccessor attributes = curves.attributes_for_write();

Falk David commented

2023-07-11 13:02:00 +02:00

Since the following part is probably the same for curves, it might be worth it to put this logic into a common function. @HooglyBoogly What do you think?

Sietse Brouwer commented

2023-07-11 13:45:57 +02:00

I noticed an UX difference: in GP, when you have a few points on a curve selected and you switch to 'curve' mode, the entire curve is selected. In '3D' curves, the curve is only selected when you have all points selected.
So if we unify the functions, we have to decide if we honour that difference.

I noticed an UX difference: in GP, when you have _a few_ points on a curve selected and you switch to 'curve' mode, the entire curve is selected. In '3D' curves, the curve is only selected when you have _all points_ selected. So if we unify the functions, we have to decide if we honour that difference.

Falk David commented

2023-07-11 14:38:53 +02:00

Ah that is a good point. Keeping the current grease pencil behavior is the way to go, so I think this can stay as is.

source/blender/editors/grease_pencil/intern/grease_pencil_select.cc

						
				@ -323,0 +399,4 @@

				  ts->gpencil_selectmode_edit = mode_new;

				  /* Convert all drawings of the active GP to the new selection domain. */

				  select_convert_selection_domain(C);

Falk David commented

2023-07-11 13:03:40 +02:00

I don't think an extra function here is necessary. All the code can be in the _exec.

I don't think an extra function here is necessary. All the code can be in the `_exec`.

SietseB marked this conversation as resolved

Sietse Brouwer added 1 commit 2023-07-11 15:59:39 +02:00

7c8f6b850f Cleanup: merging two GP selection domain functions into one

Falk David requested changes 2023-07-11 17:36:02 +02:00

Falk David left a comment

Small comment.

source/blender/editors/grease_pencil/intern/grease_pencil_select.cc

						
				@ -323,0 +329,4 @@

				  ToolSettings *ts = CTX_data_tool_settings(C);

				  ts->gpencil_selectmode_edit = mode_new;

				  WM_main_add_notifier(NC_SPACE | ND_SPACE_VIEW3D, nullptr);