From 9d242fb6b8e4ec4afd0cc3dd1ed5b2b49709e522 Mon Sep 17 00:00:00 2001 From: Ashley Wulber Date: Wed, 14 Jun 2023 20:37:03 -0400 Subject: [PATCH] fix / refactor iced-sctk redraw & frame event handling --- examples/sctk_todos/Cargo.toml | 2 +- examples/sctk_todos/src/main.rs | 3 +- examples/todos/Cargo.toml | 2 +- sctk/src/application.rs | 182 ++++++++++++++------------ sctk/src/event_loop/mod.rs | 22 +++- sctk/src/event_loop/state.rs | 1 + sctk/src/handlers/compositor.rs | 2 +- sctk/src/handlers/shell/layer.rs | 3 +- sctk/src/handlers/shell/xdg_popup.rs | 3 +- sctk/src/handlers/shell/xdg_window.rs | 2 +- sctk/src/sctk_event.rs | 5 +- 11 files changed, 123 insertions(+), 104 deletions(-) diff --git a/examples/sctk_todos/Cargo.toml b/examples/sctk_todos/Cargo.toml index df43833aaa..c1603d1f23 100644 --- a/examples/sctk_todos/Cargo.toml +++ b/examples/sctk_todos/Cargo.toml @@ -6,7 +6,7 @@ edition = "2021" publish = false [dependencies] -iced = { path = "../..", default-features=false, features = ["async-std", "wayland", "debug", "a11y"] } +iced = { path = "../..", default-features=false, features = ["async-std", "wayland", "debug"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" once_cell = "1.15" diff --git a/examples/sctk_todos/src/main.rs b/examples/sctk_todos/src/main.rs index cc4a54ff2d..0d2a4be9f6 100644 --- a/examples/sctk_todos/src/main.rs +++ b/examples/sctk_todos/src/main.rs @@ -210,8 +210,7 @@ impl Application for Todos { widget::focus_next() } } - Message::CloseRequested(s) => { - dbg!(s); + Message::CloseRequested(_) => { std::process::exit(0); } Message::TextInputDragged(text_input_state) => { diff --git a/examples/todos/Cargo.toml b/examples/todos/Cargo.toml index 3741b53206..7ad4d558bd 100644 --- a/examples/todos/Cargo.toml +++ b/examples/todos/Cargo.toml @@ -6,7 +6,7 @@ edition = "2021" publish = false [dependencies] -iced = { path = "../..", features = ["async-std", "debug", "a11y"] } +iced = { path = "../..", features = ["async-std", "debug"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" once_cell = "1.15" diff --git a/sctk/src/application.rs b/sctk/src/application.rs index f0710aeb45..aa01fb24f5 100644 --- a/sctk/src/application.rs +++ b/sctk/src/application.rs @@ -304,15 +304,15 @@ where let poll = instance.as_mut().poll(&mut context); - *control_flow = match poll { + match poll { task::Poll::Pending => { if let Ok(Some(flow)) = control_receiver.try_next() { - flow - } else { - ControlFlow::Wait + *control_flow = flow } } - task::Poll::Ready(_) => ControlFlow::ExitWithCode(1), + task::Poll::Ready(_) => { + *control_flow = ControlFlow::ExitWithCode(1) + } }; }); @@ -700,14 +700,6 @@ where } SctkEvent::RemovedOutput( ..) => { } - SctkEvent::Frame(surface) => { - if let Some(id) = surface_ids.get(&surface.id()) { - if let Some(state) = states.get_mut(&id.inner()) { - // TODO set this to the callback? - state.set_frame(Some(surface)); - } - } - }, SctkEvent::ScaleFactorChanged { .. } => {} SctkEvent::DataSource(DataSourceEvent::DndFinished) | SctkEvent::DataSource(DataSourceEvent::DndCancelled)=> { surface_ids.retain(|id, surface_id| { @@ -839,6 +831,13 @@ where interfaces.insert(native_id, user_interface); } IcedSctkEvent::MainEventsCleared => { + if !redraw_pending + && sctk_events.is_empty() + && messages.is_empty() + { + continue; + } + let mut i = 0; while i < sctk_events.len() { let remove = matches!( @@ -897,6 +896,7 @@ where break 'main; } } else { + let mut needs_update = false; for (object_id, surface_id) in &surface_ids { if matches!(surface_id, SurfaceIdWrapper::Dnd(_)) { continue; @@ -1014,10 +1014,15 @@ where user_interface::State::Outdated ) { - state.set_needs_redraw(true); + state.set_needs_redraw(state.frame.is_some()); + needs_update = !messages.is_empty() + || matches!( + interface_state, + user_interface::State::Outdated + ); } } - if states.iter().any(|(_, s)| s.needs_redraw) { + if needs_update { let mut pure_states: HashMap<_, _> = ManuallyDrop::into_inner(interfaces) .drain() @@ -1076,86 +1081,84 @@ where &mut auto_size_surfaces, &mut ev_proxy, )); + } + for (object_id, surface_id) in &surface_ids { + let state = match states.get_mut(&surface_id.inner()) { + Some(s) => { + if !s.needs_redraw() { + continue; + } else { + s.set_needs_redraw(false); - for (object_id, surface_id) in &surface_ids { - let state = match states - .get_mut(&surface_id.inner()) - { - Some(s) => { - if !s.needs_redraw() || s.frame().is_none() - { - continue; - } else { - s - } + s } - None => continue, + } + None => continue, + }; + let Some(user_interface) = interfaces + .get_mut(&surface_id.inner()) else { + continue; }; - let Some(user_interface) = interfaces - .get_mut(&surface_id.inner()) else { - continue; - }; - let redraw_event = CoreEvent::Window( - surface_id.inner(), - crate::core::window::Event::RedrawRequested( - Instant::now(), - ), - ); - - let (interface_state, _) = user_interface.update( - &[redraw_event.clone()], - state.cursor(), - &mut renderer, - &mut simple_clipboard, - &mut messages, - ); - debug.draw_started(); - let new_mouse_interaction = user_interface.draw( - &mut renderer, - state.theme(), - &Style { - text_color: state.text_color(), - }, - state.cursor(), - ); - debug.draw_finished(); + let redraw_event = CoreEvent::Window( + surface_id.inner(), + crate::core::window::Event::RedrawRequested( + Instant::now(), + ), + ); - if new_mouse_interaction != mouse_interaction { - mouse_interaction = new_mouse_interaction; - ev_proxy.send_event(Event::SetCursor( - mouse_interaction, - )); - } + let (interface_state, _) = user_interface.update( + &[redraw_event.clone()], + state.cursor(), + &mut renderer, + &mut simple_clipboard, + &mut messages, + ); - runtime.broadcast(redraw_event, Status::Ignored); + debug.draw_started(); + let new_mouse_interaction = user_interface.draw( + &mut renderer, + state.theme(), + &Style { + text_color: state.text_color(), + }, + state.cursor(), + ); + debug.draw_finished(); - ev_proxy.send_event(Event::SctkEvent( - IcedSctkEvent::RedrawRequested( - object_id.clone(), - ), + if new_mouse_interaction != mouse_interaction { + mouse_interaction = new_mouse_interaction; + ev_proxy.send_event(Event::SetCursor( + mouse_interaction, )); - - let _ = - control_sender - .start_send(match interface_state { - user_interface::State::Updated { - redraw_request: Some(redraw_request), - } => match redraw_request { - crate::core::window::RedrawRequest::NextFrame => { - ControlFlow::Poll - } - crate::core::window::RedrawRequest::At(at) => { - ControlFlow::WaitUntil(at) - } - }, - _ => ControlFlow::Wait, - }); - state.set_needs_redraw(false); - redraw_pending = false; } + + runtime.broadcast(redraw_event, Status::Ignored); + + ev_proxy.send_event(Event::SctkEvent( + IcedSctkEvent::RedrawRequested(object_id.clone()), + )); + + let _ = + control_sender + .start_send(match interface_state { + user_interface::State::Updated { + redraw_request: Some(redraw_request), + } => { + match redraw_request { + crate::core::window::RedrawRequest::NextFrame => { + ControlFlow::Poll + } + crate::core::window::RedrawRequest::At(at) => { + ControlFlow::WaitUntil(at) + } + }}, + _ => ControlFlow::Wait, + }); } + redraw_pending = false; } + sctk_events.clear(); // clear the destroyed surfaces after they have been handled destroyed_surface_ids.clear(); @@ -1369,6 +1372,14 @@ where IcedSctkEvent::A11ySurfaceCreated(surface_id, adapter) => { adapters.insert(surface_id.inner(), adapter); } + IcedSctkEvent::Frame(surface) => { + if let Some(id) = surface_ids.get(&surface.id()) { + if let Some(state) = states.get_mut(&id.inner()) { + // TODO set this to the callback? + state.set_frame(Some(surface)); + } + } + } } } @@ -1960,8 +1971,7 @@ fn event_is_for_surface( SctkEvent::WindowEvent { id, .. } => &id.id() == object_id, SctkEvent::LayerSurfaceEvent { id, .. } => &id.id() == object_id, SctkEvent::PopupEvent { id, .. } => &id.id() == object_id, - SctkEvent::Frame(_) - | SctkEvent::NewOutput { .. } + SctkEvent::NewOutput { .. } | SctkEvent::UpdateOutput { .. } | SctkEvent::RemovedOutput(_) => false, SctkEvent::ScaleFactorChanged { id, .. } => &id.id() == object_id, diff --git a/sctk/src/event_loop/mod.rs b/sctk/src/event_loop/mod.rs index 7aaffb3095..1e5741ca73 100644 --- a/sctk/src/event_loop/mod.rs +++ b/sctk/src/event_loop/mod.rs @@ -196,6 +196,7 @@ where dnd_source: None, _kbd_focus: None, sctk_events: Vec::new(), + frame_events: Vec::new(), pending_user_events: Vec::new(), token_ctr: 0, selection_source: None, @@ -300,6 +301,7 @@ where let mut sctk_event_sink_back_buffer = Vec::new(); let mut compositor_event_back_buffer = Vec::new(); + let mut frame_event_back_buffer = Vec::new(); // NOTE We break on errors from dispatches, since if we've got protocol error // libwayland-client/wayland-rs will inform us anyway, but crashing downstream is not @@ -470,6 +472,20 @@ where } } + std::mem::swap( + &mut frame_event_back_buffer, + &mut self.state.frame_events, + ); + + for event in frame_event_back_buffer.drain(..) { + sticky_exit_callback( + IcedSctkEvent::Frame(event), + &self.state, + &mut control_flow, + &mut callback, + ); + } + // The purpose of the back buffer and that swap is to not hold borrow_mut when // we're doing callback to the user, since we can double borrow if the user decides // to create a window in one of those callbacks. @@ -503,12 +519,6 @@ where // Handle pending sctk events. for event in sctk_event_sink_back_buffer.drain(..) { match event { - SctkEvent::Frame(id) => sticky_exit_callback( - IcedSctkEvent::SctkEvent(SctkEvent::Frame(id)), - &self.state, - &mut control_flow, - &mut callback, - ), SctkEvent::PopupEvent { variant: PopupEventVariant::Done, toplevel_id, diff --git a/sctk/src/event_loop/state.rs b/sctk/src/event_loop/state.rs index 23b1e3a1f1..cc78101286 100644 --- a/sctk/src/event_loop/state.rs +++ b/sctk/src/event_loop/state.rs @@ -314,6 +314,7 @@ pub struct SctkState { /// A sink for window and device events that is being filled during dispatching /// event loop and forwarded downstream afterwards. pub(crate) sctk_events: Vec, + pub(crate) frame_events: Vec, /// pending user events pub pending_user_events: Vec>, diff --git a/sctk/src/handlers/compositor.rs b/sctk/src/handlers/compositor.rs index 055f310107..a328f40df5 100644 --- a/sctk/src/handlers/compositor.rs +++ b/sctk/src/handlers/compositor.rs @@ -27,7 +27,7 @@ impl CompositorHandler for SctkState { surface: &wl_surface::WlSurface, _time: u32, ) { - self.sctk_events.push(SctkEvent::Frame(surface.clone())); + self.frame_events.push(surface.clone()); } } diff --git a/sctk/src/handlers/shell/layer.rs b/sctk/src/handlers/shell/layer.rs index 6b844a1663..0c8ff2ec77 100644 --- a/sctk/src/handlers/shell/layer.rs +++ b/sctk/src/handlers/shell/layer.rs @@ -73,8 +73,7 @@ impl LayerShellHandler for SctkState { ), id: layer.surface.wl_surface().clone(), }); - self.sctk_events - .push(SctkEvent::Frame(layer.surface.wl_surface().clone())); + self.frame_events.push(layer.surface.wl_surface().clone()); } } diff --git a/sctk/src/handlers/shell/xdg_popup.rs b/sctk/src/handlers/shell/xdg_popup.rs index 84e81ca6d1..8be7166323 100644 --- a/sctk/src/handlers/shell/xdg_popup.rs +++ b/sctk/src/handlers/shell/xdg_popup.rs @@ -36,8 +36,7 @@ impl PopupHandler for SctkState { SctkSurface::Popup(s) => s.clone(), }, }); - self.sctk_events - .push(SctkEvent::Frame(popup.wl_surface().clone())); + self.frame_events.push(popup.wl_surface().clone()); } fn done( diff --git a/sctk/src/handlers/shell/xdg_window.rs b/sctk/src/handlers/shell/xdg_window.rs index 5292b33fbd..431770a8c0 100644 --- a/sctk/src/handlers/shell/xdg_window.rs +++ b/sctk/src/handlers/shell/xdg_window.rs @@ -92,7 +92,7 @@ impl WindowHandler for SctkState { ), id, }); - self.sctk_events.push(SctkEvent::Frame(wl_surface.clone())); + self.frame_events.push(wl_surface.clone()); } } diff --git a/sctk/src/sctk_event.rs b/sctk/src/sctk_event.rs index 9453a995ca..ffc25ba385 100755 --- a/sctk/src/sctk_event.rs +++ b/sctk/src/sctk_event.rs @@ -115,6 +115,9 @@ pub enum IcedSctkEvent { /// Dnd source created with an icon surface. DndSurfaceCreated(WlSurface, DndIcon, SurfaceId), + + /// Frame callback event + Frame(WlSurface), } #[derive(Debug, Clone)] @@ -174,7 +177,6 @@ pub enum SctkEvent { // // compositor events // - Frame(WlSurface), ScaleFactorChanged { factor: f64, id: WlOutput, @@ -747,7 +749,6 @@ impl SctkEvent { .into_iter() .collect() } - SctkEvent::Frame(_) => Default::default(), SctkEvent::ScaleFactorChanged { factor: _, id: _,