Unable to make a new selection with pen button while having another active selection.

[RayJW]

Describe the bug
When using the buttons on the pen to select something, that button does not work again until the selection has been cancelled again. That means when e.g. moving things around after selecting one part and moving it, you first have to cancel the selection e.g. switching tools before the selection tool works again.

To Reproduce
Steps to reproduce the behavior:

1. Use pen hotkey to select content
2. Do whatever with the selection, e.g. scale, move that keeps the selection, so not deleting
3. Try to do another selection with the pen button
4. Instead of the selection tool, it will just start to draw

Expected behavior
I think generally, when pressing a "hotkey" again after releasing it, the intended behaviour would be for the action to get triggered again. An example workflow would be selecting something while pressing the pen button with the lasso tool, moving it where I want the selection to be, and then selecting something else that I want to move to the previous selection's place without first writing something new. I think this wouldn't break any workflows, since you could still just start writing if that's what you want to do.

Console Output

2023-11-16T15:15:10.571Z DEBUG rnote > ... env_logger initialized

(rnote:2): Gtk-CRITICAL **: 16:15:10.820: gtk_widget_class_set_accessible_role: assertion '!gtk_accessible_role_is_abstract (accessible_role)' failed

Screenshots
Video demo

Desktop (please complete the following information):

• OS: Fedora 39
• App Version: Rnote 0.9.2
• Installation Source: Flatpak
• Desktop Environment: 45.1

[Doublonmousse]

I thought I would be able to add a fix to this issue within #1139 but I'm yet to find how to do that.
When a selection is active, the next pen down event will go through this part of the code

rnote/crates/rnote-engine/src/pens/selector/penevents.rs

Lines 183 to 184 in 7f3ff8a

	// when clicking outside the selection bounds, reset
	engine_view.store.set_selected_keys(selection, false);

inside of handle_pen_event_down
Now the thing is, how to distinguish here an event with (new event with the selector button activated) and without the hotkey activated ? Looking at the inputs of that function I don't think there's anything here to do that.

So we have to look one level higher, here

rnote/crates/rnote-engine/src/pens/penholder.rs

Lines 244 to 248 in 7f3ff8a

	// Handle the event with the current pen
	let (mut event_result, wf) = self
	.current_pen
	.handle_event(event.clone(), now, engine_view);
	widget_flags |= wf | self.handle_pen_progress(event_result.progress, engine_view);

The first call is the one where we don't have the info and the second one but self which is the PenHolder which might have more info (? Maybe inside prev_shortcut_key ?). But I'm yet to be sure of this...

Either we can find some way to extract from the penholder that the mode is active and pass it inside the function (and need to change PenBehaviour in consequence) or detect this one step higher and do a second round of handle_event calls (change the mode back to a fresh eraser and relaunch it on a PenDown event)

Edit : I can't find a thing that would differentiate the two situations currently, the current state of the shortcut isn't saved, there's only changes of state on button press/release

[Doublonmousse]

I found a way to fix this issue.

If the event is a ButtonPress, the event history is of size 1. In the case of such an event, I get the shortcut key, and if there is any, check whether the associated shortcut would or not activate the selector (for now it still need to be refined a little, so that the toggle and disabled case are taken into account).

Then I add to the PenDown event an additional field that is a bool so that it can be passed down to the selector function and discriminate

• if we are outside the previous selection with the shortcut key pressed so that it should reactivate the selection (so for example set to temporary or permanent for the selector tool): we reset the selection and immediately start a new one
• if not, we reset it

This won't affect the selection cancellation PR as well.

Now I'm not sure whether or not this is the right pattern (this additional field is only really useful for the first press of the pen with a ButtonPress action so maybe we need to change a little bit the enum with PenDown, Proximity and Up ?).

Though I think this makes a little more sense as we may have other situation where we need to take action inside of the same tool based on the current pen shortcut status (hence where the code path for handling the shortcut key can't help us).

(sadly, I don't think we can query the current pen button state for doing fancier things, like a refined hold mode: if the button is released midway and the pen stays on the screen, we don't get a ButtonRelease event or a way to track this ...)

[RayJW]

Sounds amazing, thank you for the work @Doublonmousse. If you need any help with testing whatsoever, let me know! :)

As for your questions, I sadly can't really comment much on that. However, I do think that this sounds like a solid solution.

[Doublonmousse]

Hello, actually what I wrote wasn't really correct (this is an artefact of the windows version of gtk where button presses only register on a down event, not before), though I was able to adapt this fix to work either way (and it's a little cleaner as well).

I've bundled what I hope is a fix in #1405, and run a nightly flatpak here https://github.com/Doublonmousse/rnote/actions/runs/14316005769/job/40122142440 so that you can test on your side whether this works as expected

[RayJW]

Hello, actually what I wrote wasn't really correct (this is an artefact of the windows version of gtk where button presses only register on a down event, not before), though I was able to adapt this fix to work either way (and it's a little cleaner as well).

I've bundled what I hope is a fix in #1405, and run a nightly flatpak here Doublonmousse/rnote/actions/runs/14316005769/job/40122142440 so that you can test on your side whether this works as expected

I just did some testing with your build and I can happily confirm that the behaviour is now much more intuitive IMHO. I can now seamlessly select and manipulate multiple things without switching back and forth between tools!

Thanks a lot for working on that :)