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
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 34 additions & 7 deletions crates/rnote-engine/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ use futures::channel::{mpsc, oneshot};
use p2d::bounding_volume::{Aabb, BoundingVolume};
use rnote_compose::eventresult::EventPropagation;
use rnote_compose::ext::AabbExt;
use rnote_compose::penevent::{PenEvent, ShortcutKey};
use rnote_compose::penevent::{KeyboardKey, PenEvent, ShortcutKey};
use rnote_compose::{Color, SplitOrder};
use serde::{Deserialize, Serialize};
use snapshot::Snapshotable;
use std::collections::HashSet;
use std::path::PathBuf;
use std::sync::{Arc, RwLock};
use std::time::Instant;
Expand Down Expand Up @@ -785,13 +786,39 @@ impl Engine {
| self.update_rendering_current_viewport()
}

pub fn trash_selection(&mut self) -> WidgetFlags {
pub fn cancel_selection_temporary_pen(&self) -> bool {
let selection_keys = self.store.selection_keys_as_rendered();
self.store.set_trashed_keys(&selection_keys, true);
self.current_pen_update_state()
| self.doc_resize_autoexpand()
| self.record(Instant::now())
| self.update_rendering_current_viewport()
!selection_keys.is_empty()
&& self
.penholder
.pen_mode_state()
.take_style_override()
.is_some()
}

pub fn trash_selection(&mut self) -> WidgetFlags {
// check if we have a selector as a temporary tool and need to change the pen
let cancel_selection = self.cancel_selection_temporary_pen();
if cancel_selection {
// we trigger a delete press event as this reset the pen back to its
// original mode and deletes the content
let (_, widget_flags) = self.handle_pen_event(
rnote_compose::PenEvent::KeyPressed {
keyboard_key: KeyboardKey::Delete,
modifier_keys: HashSet::new(),
},
None,
Instant::now(),
);
widget_flags
} else {
let selection_keys = self.store.selection_keys_as_rendered();
self.store.set_trashed_keys(&selection_keys, true);
self.current_pen_update_state()
| self.doc_resize_autoexpand()
| self.record(Instant::now())
| self.update_rendering_current_viewport()
}
}

pub fn nothing_selected(&self) -> bool {
Expand Down
33 changes: 33 additions & 0 deletions crates/rnote-engine/src/pens/brush.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use rnote_compose::builders::{
use rnote_compose::eventresult::{EventPropagation, EventResult};
use rnote_compose::penevent::{PenEvent, PenProgress};
use rnote_compose::penpath::{Element, Segment};
use rnote_compose::shapes::Shapeable;
use std::time::Instant;

#[derive(Debug)]
Expand All @@ -41,6 +42,15 @@ impl Default for Brush {
}
}

impl Brush {
/// Threshold for the ratio of stroke bounds volume over
/// stroke width**2 over which we consider that strokes
/// or shapes that are left by pressing the pen down when
/// cancelling a selection should be kept. Smaller ratio
/// are deleted
const VOLUME_RATIO_THRESHOLD: f64 = 5.0;
}

impl PenBehaviour for Brush {
fn init(&mut self, _engine_view: &EngineView) -> WidgetFlags {
WidgetFlags::default()
Expand All @@ -63,6 +73,7 @@ impl PenBehaviour for Brush {
event: PenEvent,
now: Instant,
engine_view: &mut EngineViewMut,
_temporary_tool: bool,
) -> (EventResult<PenProgress>, WidgetFlags) {
let mut widget_flags = WidgetFlags::default();

Expand Down Expand Up @@ -235,6 +246,28 @@ impl PenBehaviour for Brush {
);
}

// remove strokes that follow a selection cancellation if they are large
// hence we can write after selecting strokes but we won't leave tiny spots
// behind
let volume = engine_view
.store
.get_stroke_ref(*current_stroke_key)
.unwrap()
.bounds()
.volume();
if engine_view.store.get_cancelled_state()
&& volume
< Self::VOLUME_RATIO_THRESHOLD
* engine_view
.config
.pens_config
.brush_config
.get_stroke_width()
.powi(2)
{
engine_view.store.remove_stroke(*current_stroke_key);
}

// Finish up the last stroke
engine_view
.store
Expand Down
8 changes: 6 additions & 2 deletions crates/rnote-engine/src/pens/eraser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,17 @@ impl PenBehaviour for Eraser {
event: PenEvent,
_now: Instant,
engine_view: &mut EngineViewMut,
_temporary_tool: bool,
) -> (EventResult<PenProgress>, WidgetFlags) {
let mut widget_flags = WidgetFlags::default();

let event_result = match (&mut self.state, event) {
(EraserState::Up | EraserState::Proximity { .. }, PenEvent::Down { element, .. }) => {
widget_flags |= erase(element, engine_view);
self.state = EraserState::Down(element);
if !engine_view.store.get_cancelled_state() {
widget_flags |= erase(element, engine_view);
self.state = EraserState::Down(element);
// this means we need one more up/down event here to activate the eraser after a selection cancellation
}
EventResult {
handled: true,
propagate: EventPropagation::Stop,
Expand Down
17 changes: 11 additions & 6 deletions crates/rnote-engine/src/pens/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,19 @@ impl PenBehaviour for Pen {
event: PenEvent,
now: Instant,
engine_view: &mut EngineViewMut,
temporary_tool: bool,
) -> (EventResult<PenProgress>, WidgetFlags) {
match self {
Pen::Brush(brush) => brush.handle_event(event, now, engine_view),
Pen::Shaper(shaper) => shaper.handle_event(event, now, engine_view),
Pen::Typewriter(typewriter) => typewriter.handle_event(event, now, engine_view),
Pen::Eraser(eraser) => eraser.handle_event(event, now, engine_view),
Pen::Selector(selector) => selector.handle_event(event, now, engine_view),
Pen::Tools(tools) => tools.handle_event(event, now, engine_view),
Pen::Brush(brush) => brush.handle_event(event, now, engine_view, temporary_tool),
Pen::Shaper(shaper) => shaper.handle_event(event, now, engine_view, temporary_tool),
Pen::Typewriter(typewriter) => {
typewriter.handle_event(event, now, engine_view, temporary_tool)
}
Pen::Eraser(eraser) => eraser.handle_event(event, now, engine_view, temporary_tool),
Pen::Selector(selector) => {
selector.handle_event(event, now, engine_view, temporary_tool)
}
Pen::Tools(tools) => tools.handle_event(event, now, engine_view, temporary_tool),
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/rnote-engine/src/pens/penbehaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub trait PenBehaviour: DrawableOnDoc {
event: PenEvent,
now: Instant,
engine_view: &mut EngineViewMut,
temporary_tool: bool,
) -> (EventResult<PenProgress>, WidgetFlags);

/// Handle a requested animation frame.
Expand Down
37 changes: 32 additions & 5 deletions crates/rnote-engine/src/pens/penholder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ pub struct PenHolder {
toggle_pen_style: Option<PenStyle>,
#[serde(skip)]
prev_shortcut_key: Option<ShortcutKey>,

/// indicates if we toggled a shortcut key
/// in temporary mode without changing
/// `PenMode` in the current sequence.
/// Used to tweak the behavior of tools
/// on exit so that we don't exit the tool
/// with the shortcut key pressed
#[serde(skip)]
temporary_style: bool,
}

impl Default for PenHolder {
Expand All @@ -57,6 +66,7 @@ impl Default for PenHolder {
progress: PenProgress::Idle,
toggle_pen_style: None,
prev_shortcut_key: None,
temporary_style: false,
}
}
}
Expand Down Expand Up @@ -110,6 +120,7 @@ impl PenHolder {
// When the style is changed externally, the toggle mode / internal states are reset
self.toggle_pen_style = None;
self.prev_shortcut_key = None;
self.temporary_style = false;

widget_flags
}
Expand Down Expand Up @@ -147,6 +158,7 @@ impl PenHolder {

if self.pen_mode_state.pen_mode() != new_pen_mode {
self.pen_mode_state.set_pen_mode(new_pen_mode);
self.temporary_style = false;
widget_flags |= self.reinstall_pen_current_style(engine_view);
widget_flags.refresh_ui = true;
}
Expand All @@ -163,7 +175,7 @@ impl PenHolder {
// first cancel the current pen
let (_, mut widget_flags) =
self.current_pen
.handle_event(PenEvent::Cancel, Instant::now(), engine_view);
.handle_event(PenEvent::Cancel, Instant::now(), engine_view, false);

// then reinstall a new pen instance
let mut new_pen = new_pen(self.current_pen_style_w_override(&engine_view.as_im()));
Expand Down Expand Up @@ -193,17 +205,25 @@ impl PenHolder {
widget_flags |= self.change_pen_mode(pen_mode, engine_view);
}

if matches!(event, PenEvent::Cancel | PenEvent::Up { .. }) {
self.temporary_style = false;
}

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

if !event_result.handled {
let (propagate, wf) = self.handle_pen_event_global(event, now, engine_view);
let (propagate, wf) = self.handle_pen_event_global(event.clone(), now, engine_view);
event_result.propagate |= propagate;
widget_flags |= wf;
}
// reset the cancelled state as we just handled a pen tool (and not a selection tool) event
if matches!(event, PenEvent::Up { .. }) {
engine_view.store.set_cancelled_state(false);
}

// Always redraw after handling a pen event.
//
Expand Down Expand Up @@ -239,6 +259,13 @@ impl PenHolder {
match action {
ShortcutAction::ChangePenStyle { style, mode } => match mode {
ShortcutMode::Temporary => {
if self
.pen_mode_state
.current_style_w_override(&engine_view.config.pens_config)
== style
{
self.temporary_style = true;
};
widget_flags |= self.change_style_override(Some(style), engine_view);
}
ShortcutMode::Permanent => {
Expand Down
8 changes: 8 additions & 0 deletions crates/rnote-engine/src/pens/pensconfig/brushconfig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,12 @@ impl BrushConfig {
}
}
}

pub(crate) fn get_stroke_width(&self) -> f64 {
match &self.style {
BrushStyle::Marker => self.marker_options.stroke_width,
BrushStyle::Solid => self.solid_options.stroke_width,
BrushStyle::Textured => self.textured_options.stroke_width,
}
}
}
5 changes: 4 additions & 1 deletion crates/rnote-engine/src/pens/selector/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,15 @@ impl PenBehaviour for Selector {
event: PenEvent,
now: Instant,
engine_view: &mut EngineViewMut,
temporary_tool: bool,
) -> (EventResult<PenProgress>, WidgetFlags) {
match event {
PenEvent::Down {
element,
modifier_keys,
} => self.handle_pen_event_down(element, modifier_keys, now, engine_view),
} => {
self.handle_pen_event_down(element, modifier_keys, now, engine_view, temporary_tool)
}
PenEvent::Up {
element,
modifier_keys,
Expand Down
19 changes: 17 additions & 2 deletions crates/rnote-engine/src/pens/selector/penevents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ impl Selector {
modifier_keys: HashSet<ModifierKey>,
_now: Instant,
engine_view: &mut EngineViewMut,
temporary_tool: bool,
) -> (EventResult<PenProgress>, WidgetFlags) {
let mut widget_flags = WidgetFlags::default();
self.pos = Some(element.pos);
Expand Down Expand Up @@ -198,10 +199,24 @@ impl Selector {
};
} else {
// when clicking outside the selection bounds, reset
tracing::debug!("cancelling the selection");
engine_view.store.set_selected_keys(selection, false);
self.state = SelectorState::Idle;

progress = PenProgress::Finished;
if temporary_tool {
// we had a selection active then for the next sequence
// 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)

progress = PenProgress::InProgress;
} else {
self.state = SelectorState::Idle;
progress = PenProgress::Finished;

// This event is in the same sequence than the one that
// cancelled the selection. We thus set the variable to true here
engine_view.store.set_cancelled_state(true);
}
}
}
ModifyState::Translate {
Expand Down
Loading