Skip to content

Commit 2b10ce7

Browse files
xStromxStrom
authored
Refactor X11 Application to make use of the new structure. (#894)
1 parent d1742e5 commit 2b10ce7

File tree

5 files changed

+486
-287
lines changed

5 files changed

+486
-287
lines changed

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ While some features like the clipboard, menus or file dialogs are not yet availa
8989
- Enabled Clippy checks for all targets. ([#850] by [@xStrom])
9090
- Added rendering tests. ([#784] by [@fishrockz])
9191
- Revamped CI testing to optimize coverage and speed. ([#857] by [@xStrom])
92+
- X11: Refactored `Application` to use the new structure. ([#894] by [@xStrom])
93+
- X11: Refactored `Window` to support some reentrancy and invalidation. ([#894] by [@xStrom])
9294

9395
### Outside News
9496

@@ -136,6 +138,7 @@ While some features like the clipboard, menus or file dialogs are not yet availa
136138
[#869]: https://github.com/xi-editor/druid/pull/869
137139
[#878]: https://github.com/xi-editor/druid/pull/878
138140
[#889]: https://github.com/xi-editor/druid/pull/899
141+
[#894]: https://github.com/xi-editor/druid/pull/894
139142

140143
## [0.5.0] - 2020-04-01
141144

‎druid-shell/src/platform/x11/application.rs‎

Lines changed: 91 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -16,174 +16,139 @@
1616
1717
use std::cell::RefCell;
1818
use std::collections::HashMap;
19-
use std::sync::Arc;
20-
21-
use lazy_static::lazy_static;
19+
use std::rc::Rc;
2220

2321
use crate::application::AppHandler;
24-
use crate::kurbo::{Point, Rect};
25-
use crate::{KeyCode, KeyModifiers, MouseButton, MouseButtons, MouseEvent};
2622

2723
use super::clipboard::Clipboard;
2824
use super::error::Error;
29-
use super::window::XWindow;
30-
31-
struct XcbConnection {
32-
connection: Arc<xcb::Connection>,
33-
screen_num: i32,
34-
}
25+
use super::window::Window;
3526

36-
lazy_static! {
37-
static ref XCB_CONNECTION: XcbConnection = XcbConnection::new();
27+
#[derive(Clone)]
28+
pub(crate) struct Application {
29+
connection: Rc<xcb::Connection>,
30+
screen_num: i32, // Needs a container when no longer const
31+
state: Rc<RefCell<State>>,
3832
}
3933

40-
thread_local! {
41-
static WINDOW_MAP: RefCell<HashMap<u32, XWindow>> = RefCell::new(HashMap::new());
34+
struct State {
35+
windows: HashMap<u32, Rc<Window>>,
4236
}
4337

44-
#[derive(Clone)]
45-
pub(crate) struct Application;
46-
4738
impl Application {
4839
pub fn new() -> Result<Application, Error> {
49-
Ok(Application)
40+
let (conn, screen_num) = match xcb::Connection::connect_with_xlib_display() {
41+
Ok(conn) => conn,
42+
Err(err) => return Err(Error::ConnectionError(err.to_string())),
43+
};
44+
let state = Rc::new(RefCell::new(State {
45+
windows: HashMap::new(),
46+
}));
47+
Ok(Application {
48+
connection: Rc::new(conn),
49+
screen_num,
50+
state,
51+
})
5052
}
5153

52-
pub(crate) fn add_xwindow(id: u32, xwindow: XWindow) {
53-
WINDOW_MAP.with(|map| map.borrow_mut().insert(id, xwindow));
54+
pub(crate) fn add_window(&self, id: u32, window: Rc<Window>) {
55+
if let Ok(mut state) = self.state.try_borrow_mut() {
56+
state.windows.insert(id, window);
57+
} else {
58+
log::warn!("Application::add_window - state already borrowed");
59+
}
5460
}
5561

56-
pub(crate) fn get_connection() -> Arc<xcb::Connection> {
57-
XCB_CONNECTION.connection_cloned()
62+
fn remove_window(&self, id: u32) {
63+
if let Ok(mut state) = self.state.try_borrow_mut() {
64+
state.windows.remove(&id);
65+
} else {
66+
log::warn!("Application::remove_window - state already borrowed");
67+
}
68+
}
69+
70+
fn window(&self, id: u32) -> Option<Rc<Window>> {
71+
if let Ok(state) = self.state.try_borrow() {
72+
state.windows.get(&id).cloned()
73+
} else {
74+
log::warn!("Application::window - state already borrowed");
75+
None
76+
}
5877
}
5978

60-
pub(crate) fn get_screen_num() -> i32 {
61-
XCB_CONNECTION.screen_num()
79+
#[inline]
80+
pub(crate) fn connection(&self) -> &Rc<xcb::Connection> {
81+
&self.connection
82+
}
83+
84+
#[inline]
85+
pub(crate) fn screen_num(&self) -> i32 {
86+
self.screen_num
6287
}
6388

6489
// TODO(x11/events): handle mouse scroll events
6590
pub fn run(self, _handler: Option<Box<dyn AppHandler>>) {
66-
let conn = XCB_CONNECTION.connection_cloned();
6791
loop {
68-
if let Some(ev) = conn.wait_for_event() {
92+
if let Some(ev) = self.connection.wait_for_event() {
6993
let ev_type = ev.response_type() & !0x80;
70-
// NOTE: I don't think we should be doing this here, but I'm trying to keep
71-
// the code mostly unchanged. My personal feeling is that the best approach
72-
// is to dispatch events to the window as early as possible, that is to say
73-
// I would send the *raw* events to the window and then let the window figure
74-
// out how to handle them. - @cmyr
75-
//
76-
// Can't get which window to send the raw event to without first parsing the event
77-
// and getting the window ID out of it :) - @crsaracco
7894
match ev_type {
7995
xcb::EXPOSE => {
8096
let expose: &xcb::ExposeEvent = unsafe { xcb::cast_event(&ev) };
8197
let window_id = expose.window();
82-
// TODO(x11/dpi_scaling): when dpi scaling is
83-
// implemented, it needs to be used here too
84-
let rect = Rect::from_origin_size(
85-
(expose.x() as f64, expose.y() as f64),
86-
(expose.width() as f64, expose.height() as f64),
87-
);
88-
WINDOW_MAP.with(|map| {
89-
let mut windows = map.borrow_mut();
90-
if let Some(w) = windows.get_mut(&window_id) {
91-
w.render(rect);
92-
}
93-
})
98+
if let Some(w) = self.window(window_id) {
99+
w.handle_expose(expose);
100+
} else {
101+
log::warn!("EXPOSE - failed to get window");
102+
}
94103
}
95104
xcb::KEY_PRESS => {
96105
let key_press: &xcb::KeyPressEvent = unsafe { xcb::cast_event(&ev) };
97-
let key: u32 = key_press.detail() as u32;
98-
let key_code: KeyCode = key.into();
99-
100106
let window_id = key_press.event();
101-
println!("window_id {}", window_id);
102-
WINDOW_MAP.with(|map| {
103-
let mut windows = map.borrow_mut();
104-
if let Some(w) = windows.get_mut(&window_id) {
105-
w.key_down(key_code);
106-
}
107-
})
107+
if let Some(w) = self.window(window_id) {
108+
w.handle_key_press(key_press);
109+
} else {
110+
log::warn!("KEY_PRESS - failed to get window");
111+
}
108112
}
109113
xcb::BUTTON_PRESS => {
110114
let button_press: &xcb::ButtonPressEvent = unsafe { xcb::cast_event(&ev) };
111115
let window_id = button_press.event();
112-
let mouse_event = MouseEvent {
113-
pos: Point::new(
114-
button_press.event_x() as f64,
115-
button_press.event_y() as f64,
116-
),
117-
// TODO: Fill with held down buttons
118-
buttons: MouseButtons::new().with(MouseButton::Left),
119-
mods: KeyModifiers {
120-
shift: false,
121-
alt: false,
122-
ctrl: false,
123-
meta: false,
124-
},
125-
count: 0,
126-
button: MouseButton::Left,
127-
};
128-
WINDOW_MAP.with(|map| {
129-
let mut windows = map.borrow_mut();
130-
if let Some(w) = windows.get_mut(&window_id) {
131-
w.mouse_down(&mouse_event);
132-
}
133-
})
116+
if let Some(w) = self.window(window_id) {
117+
w.handle_button_press(button_press);
118+
} else {
119+
log::warn!("BUTTON_PRESS - failed to get window");
120+
}
134121
}
135122
xcb::BUTTON_RELEASE => {
136123
let button_release: &xcb::ButtonReleaseEvent =
137124
unsafe { xcb::cast_event(&ev) };
138125
let window_id = button_release.event();
139-
let mouse_event = MouseEvent {
140-
pos: Point::new(
141-
button_release.event_x() as f64,
142-
button_release.event_y() as f64,
143-
),
144-
// TODO: Fill with held down buttons
145-
buttons: MouseButtons::new(),
146-
mods: KeyModifiers {
147-
shift: false,
148-
alt: false,
149-
ctrl: false,
150-
meta: false,
151-
},
152-
count: 0,
153-
button: MouseButton::Left,
154-
};
155-
WINDOW_MAP.with(|map| {
156-
let mut windows = map.borrow_mut();
157-
if let Some(w) = windows.get_mut(&window_id) {
158-
w.mouse_up(&mouse_event);
159-
}
160-
})
126+
if let Some(w) = self.window(window_id) {
127+
w.handle_button_release(button_release);
128+
} else {
129+
log::warn!("BUTTON_RELEASE - failed to get window");
130+
}
161131
}
162132
xcb::MOTION_NOTIFY => {
163-
let mouse_move: &xcb::MotionNotifyEvent = unsafe { xcb::cast_event(&ev) };
164-
let window_id = mouse_move.event();
165-
let mouse_event = MouseEvent {
166-
pos: Point::new(
167-
mouse_move.event_x() as f64,
168-
mouse_move.event_y() as f64,
169-
),
170-
// TODO: Fill with held down buttons
171-
buttons: MouseButtons::new(),
172-
mods: KeyModifiers {
173-
shift: false,
174-
alt: false,
175-
ctrl: false,
176-
meta: false,
177-
},
178-
count: 0,
179-
button: MouseButton::None,
180-
};
181-
WINDOW_MAP.with(|map| {
182-
let mut windows = map.borrow_mut();
183-
if let Some(w) = windows.get_mut(&window_id) {
184-
w.mouse_move(&mouse_event);
185-
}
186-
})
133+
let motion_notify: &xcb::MotionNotifyEvent =
134+
unsafe { xcb::cast_event(&ev) };
135+
let window_id = motion_notify.event();
136+
if let Some(w) = self.window(window_id) {
137+
w.handle_motion_notify(motion_notify);
138+
} else {
139+
log::warn!("MOTION_NOTIFY - failed to get window");
140+
}
141+
}
142+
xcb::DESTROY_NOTIFY => {
143+
let destroy_notify: &xcb::DestroyNotifyEvent =
144+
unsafe { xcb::cast_event(&ev) };
145+
let window_id = destroy_notify.window();
146+
if let Some(w) = self.window(window_id) {
147+
w.handle_destroy_notify(destroy_notify);
148+
} else {
149+
log::warn!("DESTROY_NOTIFY - failed to get window");
150+
}
151+
self.remove_window(window_id);
187152
}
188153
_ => {}
189154
}
@@ -192,7 +157,7 @@ impl Application {
192157
}
193158

194159
pub fn quit(&self) {
195-
// No-op.
160+
// TODO(x11/quit): implement Application::quit
196161
}
197162

198163
pub fn clipboard(&self) -> Clipboard {
@@ -207,22 +172,3 @@ impl Application {
207172
"en-US".into()
208173
}
209174
}
210-
211-
impl XcbConnection {
212-
fn new() -> Self {
213-
let (conn, screen_num) = xcb::Connection::connect_with_xlib_display().unwrap();
214-
215-
Self {
216-
connection: Arc::new(conn),
217-
screen_num,
218-
}
219-
}
220-
221-
fn connection_cloned(&self) -> Arc<xcb::Connection> {
222-
self.connection.clone()
223-
}
224-
225-
fn screen_num(&self) -> i32 {
226-
self.screen_num
227-
}
228-
}

‎druid-shell/src/platform/x11/error.rs‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,22 @@ use std::fmt;
1818

1919
#[derive(Debug, Clone)]
2020
pub enum Error {
21-
// TODO(x11/errors): enumerate `Error`s for X11
22-
NoError,
21+
// Generic error
22+
Generic(String),
23+
// TODO: Replace String with xcb::ConnError once that gets Clone support
24+
ConnectionError(String),
25+
// Runtime borrow failure
26+
BorrowError(String),
2327
}
2428

2529
impl fmt::Display for Error {
2630
fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
27-
// TODO(x11/errors): implement Error::fmt
28-
log::warn!("Error::fmt is currently unimplemented for X11 platforms.");
29-
write!(f, "X11 Error")
31+
match self {
32+
Error::Generic(msg) => write!(f, "Error: {}", msg),
33+
Error::ConnectionError(err) => write!(f, "Connection error: {}", err),
34+
Error::BorrowError(msg) => write!(f, "Borrow error: {}", msg),
35+
}
3036
}
3137
}
38+
39+
impl std::error::Error for Error {}

0 commit comments

Comments
 (0)