Skip to content

Ignore nested events in GTK shell #837

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.

Sign up for GitHub

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

Merged
luleyleo merged 3 commits into from
Apr 14, 2020
Merged

Ignore nested events in GTK shell #837

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e513f2
Ignore nested events in GTK shell
luleyleo Apr 13, 2020
546c78c
Log when skipping GTK event
luleyleo Apr 14, 2020
b9e8809
Fix key_up and use full path for log functions
luleyleo Apr 14, 2020
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
161 changes: 91 additions & 70 deletions druid-shell/src/platform/gtk/window.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ use crate::keyboard;
use crate::mouse::{Cursor, MouseButton, MouseEvent};
use crate::window::{IdleToken, Text, TimerToken, WinHandler};
use crate::Error;
use log::{info, warn};

/// Taken from https://gtk-rs.org/docs-src/tutorial/closures
/// It is used to reduce the boilerplate of setting up gtk callbacks
Expand Down Expand Up @@ -270,6 +271,8 @@ impl WindowBuilder {
if anim {
widget.queue_draw();
}
} else {
warn!("Drawing was skipped because the handler was already borrowed");
}

}
Expand All @@ -279,31 +282,37 @@ impl WindowBuilder {

drawing_area.connect_button_press_event(clone!(handle => move |_widget, button| {
if let Some(state) = handle.state.upgrade() {

state.handler.borrow_mut().mouse_down(
&MouseEvent {
pos: Point::from(button.get_position()),
count: get_mouse_click_count(button.get_event_type()),
mods: get_modifiers(button.get_state()),
button: get_mouse_button(button.get_button()),
},
);
if let Ok(mut handler) = state.handler.try_borrow_mut() {
handler.mouse_down(
&MouseEvent {
pos: Point::from(button.get_position()),
count: get_mouse_click_count(button.get_event_type()),
mods: get_modifiers(button.get_state()),
button: get_mouse_button(button.get_button()),
},
);
} else {
info!("GTK event was dropped because the handler was already borrowed");
}
}

Inhibit(true)
}));

drawing_area.connect_button_release_event(clone!(handle => move |_widget, button| {
if let Some(state) = handle.state.upgrade() {

state.handler.borrow_mut().mouse_up(
&MouseEvent {
pos: Point::from(button.get_position()),
mods: get_modifiers(button.get_state()),
count: 0,
button: get_mouse_button(button.get_button()),
},
);
if let Ok(mut handler) = state.handler.try_borrow_mut() {
handler.mouse_up(
&MouseEvent {
pos: Point::from(button.get_position()),
mods: get_modifiers(button.get_state()),
count: 0,
button: get_mouse_button(button.get_button()),
},
);
} else {
info!("GTK event was dropped because the handler was already borrowed");
}
}

Inhibit(true)
Expand All @@ -320,10 +329,11 @@ impl WindowBuilder {
button: get_mouse_button_from_modifiers(motion.get_state()),
};

state
.handler
.borrow_mut()
.mouse_move(&mouse_event);
if let Ok(mut handler) = state.handler.try_borrow_mut() {
handler.mouse_move(&mouse_event);
} else {
info!("GTK event was dropped because the handler was already borrowed");
}
}

Inhibit(true)
Expand All @@ -340,61 +350,65 @@ impl WindowBuilder {
button: get_mouse_button_from_modifiers(crossing.get_state()),
};

state
.handler
.borrow_mut()
.mouse_move(&mouse_event);
if let Ok(mut handler) = state.handler.try_borrow_mut() {
handler.mouse_move(&mouse_event);
} else {
info!("GTK event was dropped because the handler was already borrowed");
}
}

Inhibit(true)
}));

drawing_area.connect_scroll_event(clone!(handle => move |_widget, scroll| {
if let Some(state) = handle.state.upgrade() {

let modifiers = get_modifiers(scroll.get_state());

// The magic "120"s are from Microsoft's documentation for WM_MOUSEWHEEL.
// They claim that one "tick" on a scroll wheel should be 120 units.
let mut handler = state.handler.borrow_mut();
match scroll.get_direction() {
ScrollDirection::Up => {
if modifiers.shift {
if let Ok(mut handler) = state.handler.try_borrow_mut() {

let modifiers = get_modifiers(scroll.get_state());

// The magic "120"s are from Microsoft's documentation for WM_MOUSEWHEEL.
// They claim that one "tick" on a scroll wheel should be 120 units.
match scroll.get_direction() {
ScrollDirection::Up => {
if modifiers.shift {
handler.wheel(Vec2::from((-120.0, 0.0)), modifiers);
} else {
handler.wheel(Vec2::from((0.0, -120.0)), modifiers);
}
}
ScrollDirection::Down => {
if modifiers.shift {
handler.wheel(Vec2::from((120.0, 0.0)), modifiers);
} else {
handler.wheel(Vec2::from((0.0, 120.0)), modifiers);
}
}
ScrollDirection::Left => {
handler.wheel(Vec2::from((-120.0, 0.0)), modifiers);
} else {
handler.wheel(Vec2::from((0.0, -120.0)), modifiers);
}
}
ScrollDirection::Down => {
if modifiers.shift {
ScrollDirection::Right => {
handler.wheel(Vec2::from((120.0, 0.0)), modifiers);
} else {
handler.wheel(Vec2::from((0.0, 120.0)), modifiers);
}
}
ScrollDirection::Left => {
handler.wheel(Vec2::from((-120.0, 0.0)), modifiers);
}
ScrollDirection::Right => {
handler.wheel(Vec2::from((120.0, 0.0)), modifiers);
}
ScrollDirection::Smooth => {
//TODO: Look at how gtk's scroll containers implements it
let (mut delta_x, mut delta_y) = scroll.get_delta();
delta_x *= 120.;
delta_y *= 120.;
if modifiers.shift {
delta_x += delta_y;
delta_y = 0.;
ScrollDirection::Smooth => {
//TODO: Look at how gtk's scroll containers implements it
let (mut delta_x, mut delta_y) = scroll.get_delta();
delta_x *= 120.;
delta_y *= 120.;
if modifiers.shift {
delta_x += delta_y;
delta_y = 0.;
}
handler.wheel(Vec2::from((delta_x, delta_y)), modifiers)
}
e => {
eprintln!(
"Warning: the Druid widget got some whacky scroll direction {:?}",
e
);
}
handler.wheel(Vec2::from((delta_x, delta_y)), modifiers)
}
e => {
eprintln!(
"Warning: the Druid widget got some whacky scroll direction {:?}",
e
);
}
} else {
info!("GTK event was dropped because the handler was already borrowed");
}
}

Expand All @@ -409,8 +423,11 @@ impl WindowBuilder {

*current_keyval = Some(key.get_keyval());

let key_event = make_key_event(key, repeat);
state.handler.borrow_mut().key_down(key_event);
if let Ok(mut handler) = state.handler.try_borrow_mut() {
handler.key_down(make_key_event(key, repeat));
} else {
info!("GTK event was dropped because the handler was already borrowed");
}
}

Inhibit(true)
Expand All @@ -421,8 +438,11 @@ impl WindowBuilder {

*(state.current_keyval.borrow_mut()) = None;

let key_event = make_key_event(key, false);
state.handler.borrow_mut().key_up(key_event);
if let Ok(mut handler) = state.handler.try_borrow_mut() {
handler.key_down(make_key_event(key, false));
} else {
info!("GTK event was dropped because the handler was already borrowed");
}
}

Inhibit(true)
Expand Down Expand Up @@ -479,7 +499,7 @@ impl WindowHandle {
/// Bring this window to the front of the window stack and give it focus.
pub fn bring_to_front_and_focus(&self) {
//FIXME: implementation goes here
log::warn!("bring_to_front_and_focus not yet implemented for gtk");
warn!("bring_to_front_and_focus not yet implemented for gtk");
Copy link
Member

@xStrom xStrom Apr 14, 2020

Choose a reason for hiding this comment

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

I think @cmyr wants us to not use imported functions directly. That is log::warn! is better than warn!. It's from the Rust style guide.

Copy link
Collaborator Author

@luleyleo luleyleo Apr 14, 2020

Choose a reason for hiding this comment

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

I prefer that too, was just done differently in the Windows version I used as reference.
Wonder if there are lints for such things 🤔

Copy link
Member

@xStrom xStrom Apr 14, 2020

Choose a reason for hiding this comment

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

Yeah the windows platform code has some of the oldest code in druid and so follows style the least.

I've been also thinking about looking into custom clippy/rustfmt options to enforce some of our style rules.

Copy link
Member

@cmyr cmyr Apr 14, 2020

Choose a reason for hiding this comment

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

yea, this is a nit but I like macros and functions to be module-qualified. Would gladly accept a PR that standardizes on this :)

}

// Request invalidation of the entire window contents.
Expand All @@ -501,7 +521,7 @@ impl WindowHandle {
let interval = match u32::try_from(interval) {
Ok(iv) => iv,
Err(_) => {
log::warn!("timer duration exceeds gtk max of 2^32 millis");
warn!("timer duration exceeds gtk max of 2^32 millis");
u32::max_value()
}
};
Expand Down Expand Up @@ -616,6 +636,7 @@ impl WindowHandle {
window.add_accel_group(&accel_group);

let menu = menu.into_gtk_menu(&self, &accel_group);
menu.set_property_attach_widget(Some(window));
menu.show_all();
menu.popup_easy(3, gtk::get_current_event_time());
}
Expand Down