GPv3: Keyframe on-click selection #110492

Amélie Fondevilla · 2023-07-26T12:57:21+02:00

2023-07-26 12:57:21 +02:00

Implementation of the click-selection operator of keyframes in the dopesheet, along with its alternatives : deselect all when no frames is hit, extend selection with shift, colum selection with alt, select all keyframes on current channel with ctrl+alt.

Includes the new following API functions :

select_frame_at : selects a frame in a layer at a specific time (if such frame exists),
select_all_frames : selects all frames of a layer,
select_layer_channel : selects a layer, and sets it as active (if layer is not null, otherwise the active layer is set null),
layer_has_frame_selected : checks if any of the frames in the layer is selected.

Implementation of the click-selection operator of keyframes in the dopesheet, along with its alternatives : deselect all when no frames is hit, extend selection with shift, colum selection with alt, select all keyframes on current channel with ctrl+alt. Includes the new following API functions : * `select_frame_at` : selects a frame in a layer at a specific time (if such frame exists), * `select_all_frames` : selects all frames of a layer, * `select_layer_channel` : selects a layer, and sets it as active (if layer is not null, otherwise the active layer is set null), * `layer_has_frame_selected` : checks if any of the frames in the layer is selected.

Amélie Fondevilla added this to the Grease Pencil project 2023-07-26 12:57:42 +02:00

Amélie Fondevilla referenced this pull request

2023-07-26 12:58:34 +02:00

GPv3: Integration in the Dopesheets #110056

Amélie Fondevilla changed title from ~~WIP: GPv3: Keyframe on-click selection.~~ to GPv3: Keyframe on-click selection.

2023-07-26 14:46:22 +02:00

Amélie Fondevilla requested review from Falk David 2023-07-26 14:47:28 +02:00

Falk David changed title from ~~GPv3: Keyframe on-click selection.~~ to GPv3: Keyframe on-click selection

2023-07-26 14:47:47 +02:00

Falk David requested changes 2023-07-26 14:53:24 +02:00

Falk David left a comment

Few initial comments.

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

						
				@ -25,0 +64,4 @@

				{

				  for (auto item : layer->frames().items()) {

				    const GreasePencilFrame &frame = item.value;

				    if ((frame.flag & GP_FRAME_SELECTED) != 0) {

Falk David commented

2023-07-26 14:50:28 +02:00

Similar to how we have functions on the GreasePencilFrame like is_implicit_hold() I think it makes sense to add a is_selected() one.

Similar to how we have functions on the `GreasePencilFrame` like `is_implicit_hold()` I think it makes sense to add a `is_selected()` one.

amelief marked this conversation as resolved

source/blender/editors/include/ED_grease_pencil.h Outdated

						
				@ -58,0 +74,4 @@

				/**

				 * Checks if any frame of the \a layer is selected.

				 */

				bool layer_has_frame_selected(const bke::greasepencil::Layer *layer);

Falk David commented

2023-07-26 14:51:22 +02:00

Maybe layer_has_any_frame_selected(...)?

Maybe `layer_has_any_frame_selected(...)`?

amelief marked this conversation as resolved

source/blender/editors/space_action/action_select.cc Outdated

						
				@ -1843,3 +1877,1 @@

				        if (ale != nullptr && ale->data != nullptr && ale->type == ANIMTYPE_GPLAYER) {

				          bGPdata *gpd = (bGPdata *)ale->id;

				          bGPDlayer *gpl = static_cast<bGPDlayer *>(ale->data);

				        if (U.experimental.use_grease_pencil_version3) {

Falk David commented

2023-07-26 14:53:03 +02:00

I wonder if the check for the experimental option is still needed here.
Especially with the ale->type == ANIMTYPE_GREASE_PENCIL_LAYER check afterwards.

I wonder if the check for the experimental option is still needed here. Especially with the `ale->type == ANIMTYPE_GREASE_PENCIL_LAYER` check afterwards.

amelief marked this conversation as resolved

Amélie Fondevilla force-pushed gpv3-keyframes-click-select from 63e95c5665 to f7960114bd

2023-07-26 14:54:38 +02:00

Compare

Amélie Fondevilla added 4 commits 2023-07-26 15:05:58 +02:00

2221593bff Add frame is selected function

0b81bf4bf3 Refactor code

6962524e40 Rename function layer has any frame selected

buildbot/vexp-code-patch-coordinator Build done. Details

287d77c36e Remove unnecessary check for experimental mode

Falk David commented

2023-07-26 15:15:10 +02:00

@blender-bot build

Falk David approved these changes 2023-07-26 16:20:48 +02:00

Falk David left a comment

Two small comments, but otherwise LGTM!

source/blender/editors/include/ED_grease_pencil.h Outdated

						
				@ -57,1 +59,4 @@

				void select_layer_channel(GreasePencil *grease_pencil, bke::greasepencil::Layer *layer);

				void set_active_layer(GreasePencil *grease_pencil, bke::greasepencil::Layer *layer);

Falk David commented

2023-07-26 16:16:56 +02:00

This can be reomved.

amelief marked this conversation as resolved

source/blender/editors/include/ED_grease_pencil.h Outdated

						
				@ -58,0 +72,4 @@

				void select_all_frames(bke::greasepencil::Layer *layer, const short select_mode);

				/**

				 * Checks if any frame of the \a layer is selected.