Skip to content

Improv : logic changes for selection cancellation #1405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Doublonmousse
Copy link
Collaborator

@Doublonmousse Doublonmousse commented Apr 6, 2025

Fixes #414
Fixes #888

  • This fixes the small dots that appears on selection cancellation with the pen (whilst allowing for strokes to still be written if they're larger than small dots)

A couple remarks :

  • shapes need an additional up/down cycle : we could change it to follow the same logic as the brush but this could also be done by not changing the logic of the shape part and only disallowing shapes to be too small (in that case we can create shapes that aren't even visible but are still part of the document)
  • the eraser and typewriter both need an additional pen up/down to activate
  • tools aren't affected (they won't change anything if they're activated on such a short pen down/up cycle)
  • This does not change behavior when a selection is deleted.

Rebase of the previous PR #1139 with the new clone! syntax

I've added a condition to only create shapes if they're large enough. As things like arrows would still appear on a single pen event, I'll leave the current behavior for shapes after a selection cancellation (that is we need an extra up/down cycle)

I've added the cancellation of the selection tool upon deleting a selection when the selection tool is temporary

Putting back in draft as this needs a little work before it can be merged. I'm wondering if the condition can be slightly modified to include time delay as well. It will cancel a dot after a selection but sometimes this dot can be large enough to stay (if the pen slides a bit instead of a clean hit)

Doublonmousse and others added 8 commits April 6, 2025 17:00
fixes bug where large vertical or horizontal elements would get cancelled
apply corrections

- move actions logic into `trash_selection`
- small rename of canceled* to have coherent naming between variables and functions getters/settesr
- move comment closer to the `canceled_state` variable (variable-level docstring)
- constant added as such

fmt and hashset
…hortcuts and pass to pens

This adds a `temporary_tool` bool to the `PenHolder` which signifies whether the following sequence occured (for now only with the selector)

1. activates the selector
2. Select something then lift the pen (triggering an up or cancel event)
3. go back with the selector activated via a temporary shortcut
4. Select something else

Then the temporary style will be `true` as going back will trigger again the `handle_pressed_shortcut_key` function on the pen down event.
@Doublonmousse
Copy link
Collaborator Author

I've added a commit to fix #888. This last commit needs some testing (and maybe some renaming) to be sure everything works as expected.

@Doublonmousse
Copy link
Collaborator Author

Rebased to main, seems to compile/work as before.
I think I still need to retest on tablet though

// we activated the temporary mode for the selector and
// clicked outside the selection. We expect to be able
// to do another selection
self.state = SelectorState::Selecting { path: Vec::new() };
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking maybe having a 'if shift' condition here would be good (so doing shift + new selection can add to what's already here).
Now that would mean we'd have to change the SelectorState strcture (so that we can select and have the previous selection still saved as an optional)

@Doublonmousse
Copy link
Collaborator Author

Retested and working almost as intended.

On smaller stroke width, the heuristic I'm using for cancelling small strokes doesn't work very well (volume comparison to a * stroke_width ^ 2). It works if the pen is orthogonal to the surface but will still leaves traces behind if you slip a little.

Maybe calculating the perimeter of the outline and comparing it to a fixed threshold * zoom will work better (so that we essentially check whether the stroke path is small or not compared to the current view).
I'll try but if that still fails I'll fallback to an additional up/down cycle

This is a little more robust that volume as tiny spots tend to be single elements or small lines. The perimeter also increase quite rapidly with non line elements.

The threshold is placed quite high (to limit unwanted spots)
@Doublonmousse
Copy link
Collaborator Author

I've changed the computation for the cancellation. It seems to work better though not totally foolproof. I'm still on the fence about switching back to an additional up/down cycle for the selection cancellation to pen stroke situation.
Other that than, does what I expect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to make a new selection with pen button while having another active selection. Define exit behavior of the tools
1 participant