From 1d099bd464c1564c45ae0644b2633b41fb2f88f4 Mon Sep 17 00:00:00 2001 From: sjoshid Date: Sun, 12 Apr 2020 13:40:27 -0400 Subject: [PATCH 01/10] 1) More aggressive Timer event traversal. 2) Aggressive traversal is achieved by storing a map of TimerToken -> WidgetId in each window. 3) On Timer event, we check the map from step 2 and recurse only if child MIGHT contain the WidgetId. --- druid/src/contexts.rs | 11 ++++++++++- druid/src/core.rs | 27 ++++++++++++++++++++++++--- druid/src/window.rs | 15 ++++++++++++--- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index bbdebfd161..bc77550690 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -26,6 +26,7 @@ use crate::{ Affine, Command, Cursor, Insets, Rect, Size, Target, Text, TimerToken, WidgetId, WindowHandle, WindowId, }; +use std::collections::HashMap; /// A mutable context provided to event handling methods of widgets. /// @@ -46,6 +47,8 @@ pub struct EventCtx<'a> { pub(crate) had_active: bool, pub(crate) is_handled: bool, pub(crate) is_root: bool, + /// Map of TimerTokens and WidgetIds that requested them. + pub(crate) timers: &'a HashMap, } /// A mutable context provided to the [`lifecycle`] method on widgets. @@ -349,7 +352,13 @@ impl<'a> EventCtx<'a> { /// request with the event. pub fn request_timer(&mut self, deadline: Instant) -> TimerToken { self.base_state.request_timer = true; - self.window.request_timer(deadline) + let timer_token = self.window.request_timer(deadline); + self.base_state.add_timer(timer_token); + timer_token + } + + pub fn remove_timer(&mut self, timer_token: &TimerToken) { + self.base_state.remove_timer(timer_token); } /// The layout size. diff --git a/druid/src/core.rs b/druid/src/core.rs index c23402dec6..645731acfe 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -14,7 +14,7 @@ //! The fundamental druid types. -use std::collections::VecDeque; +use std::collections::{HashMap, VecDeque}; use log; @@ -23,7 +23,7 @@ use crate::kurbo::{Affine, Insets, Rect, Shape, Size}; use crate::piet::RenderContext; use crate::{ BoxConstraints, Command, Data, Env, Event, EventCtx, LayoutCtx, LifeCycle, LifeCycleCtx, - PaintCtx, Target, UpdateCtx, Widget, WidgetId, + PaintCtx, Target, TimerToken, UpdateCtx, Widget, WidgetId, }; /// Our queue type @@ -99,6 +99,7 @@ pub(crate) struct BaseState { pub(crate) request_focus: Option, pub(crate) children: Bloom, pub(crate) children_changed: bool, + pub(crate) timers: HashMap, } /// Methods by which a widget can attempt to change focus state. @@ -376,6 +377,7 @@ impl> WidgetPod { is_handled: false, is_root: false, focus_widget: ctx.focus_widget, + timers: &ctx.timers, }; let rect = child_ctx.base_state.layout_rect; // Note: could also represent this as `Option`. @@ -438,7 +440,14 @@ impl> WidgetPod { Event::Zoom(*zoom) } Event::Timer(id) => { - recurse = child_ctx.base_state.request_timer; + if let Some(widget_id) = child_ctx.timers.get(id) { + if *widget_id != child_ctx.base_state.id { + recurse = child_ctx.base_state.children.may_contain(widget_id); + } + } else { + log::error!("Timer Token must be in timers map."); + recurse = false; + } Event::Timer(*id) } Event::Command(cmd) => Event::Command(cmd.clone()), @@ -467,6 +476,8 @@ impl> WidgetPod { }; ctx.base_state.merge_up(&child_ctx.base_state); + // Clear current widget's timers after merging with parent. + child_ctx.base_state.timers.clear(); ctx.is_handled |= child_ctx.is_handled; } @@ -630,9 +641,18 @@ impl BaseState { focus_chain: Vec::new(), children: Bloom::new(), children_changed: false, + timers: HashMap::new(), } } + pub(crate) fn add_timer(&mut self, timer_token: TimerToken) { + self.timers.insert(timer_token, self.id); + } + + pub(crate) fn remove_timer(&mut self, timer_token: &TimerToken) { + self.timers.remove(timer_token); + } + /// Update to incorporate state changes from a child. fn merge_up(&mut self, child_state: &BaseState) { self.needs_inval |= child_state.needs_inval; @@ -642,6 +662,7 @@ impl BaseState { self.has_active |= child_state.has_active; self.children_changed |= child_state.children_changed; self.request_focus = self.request_focus.or(child_state.request_focus); + self.timers.extend(&child_state.timers); } #[inline] diff --git a/druid/src/window.rs b/druid/src/window.rs index 8be999066b..6cc63bf46d 100644 --- a/druid/src/window.rs +++ b/druid/src/window.rs @@ -14,18 +14,19 @@ //! Management of multiple windows. +use std::collections::HashMap; use std::mem; use std::time::Instant; +use crate::core::{BaseState, CommandQueue, FocusChange}; use crate::kurbo::{Insets, Point, Rect, Size}; use crate::piet::{Piet, RenderContext}; use crate::shell::{Counter, Cursor, WindowHandle}; - -use crate::core::{BaseState, CommandQueue, FocusChange}; use crate::win_handler::RUN_COMMANDS_TOKEN; use crate::{ BoxConstraints, Command, Data, Env, Event, EventCtx, LayoutCtx, LifeCycle, LifeCycleCtx, - LocalizedString, MenuDesc, PaintCtx, UpdateCtx, Widget, WidgetId, WidgetPod, WindowDesc, + LocalizedString, MenuDesc, PaintCtx, TimerToken, UpdateCtx, Widget, WidgetId, WidgetPod, + WindowDesc, }; /// A unique identifier for a window. @@ -43,6 +44,7 @@ pub struct Window { pub(crate) last_anim: Option, pub(crate) focus: Option, pub(crate) handle: WindowHandle, + pub(crate) timers: HashMap, // delegate? } @@ -58,6 +60,7 @@ impl Window { last_anim: None, focus: None, handle, + timers: HashMap::new(), } } } @@ -145,6 +148,7 @@ impl Window { window: &self.handle, window_id: self.id, focus_widget: self.focus, + timers: &self.timers, }; self.root.event(&mut ctx, &event, data, env); @@ -169,6 +173,11 @@ impl Window { self.lifecycle(queue, &LifeCycle::RouteWidgetAdded, data, env); } + //If at least one widget requested timer, collect those timers from widgets and add to window's timers map. + if base_state.request_timer { + self.timers.extend(base_state.timers); + } + is_handled } From 3f47c3fbed4ee586c8a19a0b1f30193789c6d8d5 Mon Sep 17 00:00:00 2001 From: sjoshid Date: Sun, 12 Apr 2020 14:08:06 -0400 Subject: [PATCH 02/10] 1) Comment --- druid/src/core.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/druid/src/core.rs b/druid/src/core.rs index 645731acfe..f73274586e 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -99,6 +99,7 @@ pub(crate) struct BaseState { pub(crate) request_focus: Option, pub(crate) children: Bloom, pub(crate) children_changed: bool, + /// Associate timers with widgets that requested them. pub(crate) timers: HashMap, } From 1bdc803944e5e66d62c7552566547e566b575cd6 Mon Sep 17 00:00:00 2001 From: sjoshid Date: Sun, 12 Apr 2020 14:30:55 -0400 Subject: [PATCH 03/10] Fix clippy warnings. --- druid/src/contexts.rs | 2 +- druid/src/core.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index bc77550690..74f947f921 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -357,7 +357,7 @@ impl<'a> EventCtx<'a> { timer_token } - pub fn remove_timer(&mut self, timer_token: &TimerToken) { + pub fn remove_timer(&mut self, timer_token: TimerToken) { self.base_state.remove_timer(timer_token); } diff --git a/druid/src/core.rs b/druid/src/core.rs index f73274586e..d478ec7c45 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -650,8 +650,8 @@ impl BaseState { self.timers.insert(timer_token, self.id); } - pub(crate) fn remove_timer(&mut self, timer_token: &TimerToken) { - self.timers.remove(timer_token); + pub(crate) fn remove_timer(&mut self, timer_token: TimerToken) { + self.timers.remove(&timer_token); } /// Update to incorporate state changes from a child. From dd7892e426d2d1954f921b0a122e7e742d7a1742 Mon Sep 17 00:00:00 2001 From: Sujit Joshi Date: Mon, 13 Apr 2020 08:56:33 -0400 Subject: [PATCH 04/10] 1) After merging up, Im clearing the map of BaseState. So remove is not needed when event is handled. --- druid/src/contexts.rs | 4 ---- druid/src/core.rs | 4 ---- 2 files changed, 8 deletions(-) diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index 74f947f921..a2641614f3 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -357,10 +357,6 @@ impl<'a> EventCtx<'a> { timer_token } - pub fn remove_timer(&mut self, timer_token: TimerToken) { - self.base_state.remove_timer(timer_token); - } - /// The layout size. /// /// This is the layout size as ultimately determined by the parent diff --git a/druid/src/core.rs b/druid/src/core.rs index d478ec7c45..0cc49bab8f 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -650,10 +650,6 @@ impl BaseState { self.timers.insert(timer_token, self.id); } - pub(crate) fn remove_timer(&mut self, timer_token: TimerToken) { - self.timers.remove(&timer_token); - } - /// Update to incorporate state changes from a child. fn merge_up(&mut self, child_state: &BaseState) { self.needs_inval |= child_state.needs_inval; From 6349aa4480496d56327ce2eda8954eb75b3058c4 Mon Sep 17 00:00:00 2001 From: sjoshid Date: Sun, 19 Apr 2020 11:18:43 -0400 Subject: [PATCH 05/10] 1) Removing window's timer token before adding base state's token to it. --- druid/src/window.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/druid/src/window.rs b/druid/src/window.rs index dbab091959..9c3f5f6029 100644 --- a/druid/src/window.rs +++ b/druid/src/window.rs @@ -218,6 +218,12 @@ impl Window { self.post_event_processing(queue, data, env, false); + //In some platforms, timer tokens are reused. So it is necessary to remove token from + //window's timer before adding base state's timers to it. + if let Event::Timer(token) = event { + self.timers.remove(&token); + } + //If at least one widget requested timer, collect those timers from widgets and add to window's timers map. if base_state.request_timer { self.timers.extend(base_state.timers); From 3d4870a29ce13e97ae703b4e94c0d29502bc46de Mon Sep 17 00:00:00 2001 From: Sujit Joshi Date: Sun, 26 Apr 2020 14:49:53 -0400 Subject: [PATCH 06/10] Changes as per https://github.com/xi-editor/druid/pull/831#issuecomment-616188133 --- druid/src/contexts.rs | 2 -- druid/src/core.rs | 20 ++++++++++---------- druid/src/event.rs | 1 + druid/src/window.rs | 9 ++++++++- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index 3490ff5917..f904851d2f 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -44,8 +44,6 @@ pub struct EventCtx<'a> { pub(crate) focus_widget: Option, pub(crate) is_handled: bool, pub(crate) is_root: bool, - /// Map of TimerTokens and WidgetIds that requested them. - pub(crate) timers: &'a HashMap, } /// A mutable context provided to the [`lifecycle`] method on widgets. diff --git a/druid/src/core.rs b/druid/src/core.rs index fe5230dcf3..a5945e3ae0 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -479,7 +479,6 @@ impl> WidgetPod { is_handled: false, is_root: false, focus_widget: ctx.focus_widget, - timers: &ctx.timers, }; let rect = child_ctx.base_state.layout_rect.unwrap_or_default(); @@ -519,6 +518,15 @@ impl> WidgetPod { } } } + InternalEvent::RouteTimer(token, widget_id) => { + let widget_id = *widget_id; + if widget_id != child_ctx.base_state.id { + recurse = child_ctx.base_state.children.may_contain(&widget_id); + Event::Internal(InternalEvent::RouteTimer(*token, widget_id)) + } else { + Event::Timer(*token) + } + } }, Event::WindowConnected => Event::WindowConnected, Event::WindowSize(size) => { @@ -595,15 +603,7 @@ impl> WidgetPod { Event::Zoom(*zoom) } Event::Timer(id) => { - if let Some(widget_id) = child_ctx.timers.get(id) { - if *widget_id != child_ctx.base_state.id { - recurse = child_ctx.base_state.children.may_contain(widget_id); - } - } else { - log::error!("Timer Token must be in timers map."); - recurse = false; - } - Event::Timer(*id) + panic!("We cannot be here"); } Event::Command(cmd) => Event::Command(cmd.clone()), }; diff --git a/druid/src/event.rs b/druid/src/event.rs index 6bb306f9c8..7c4c8d80ac 100644 --- a/druid/src/event.rs +++ b/druid/src/event.rs @@ -147,6 +147,7 @@ pub enum InternalEvent { MouseLeave, /// A command still in the process of being dispatched. TargetedCommand(Target, Command), + RouteTimer(TimerToken, WidgetId), } /// Application life cycle events. diff --git a/druid/src/window.rs b/druid/src/window.rs index 8c867e9246..3cd9edcdb8 100644 --- a/druid/src/window.rs +++ b/druid/src/window.rs @@ -173,6 +173,14 @@ impl Window { let scale = 96.0 / dpi; self.size = Size::new(size.width * scale, size.height * scale); Event::WindowSize(self.size) + }, + Event::Timer(token) => { + if let Some(widget_id) = self.timers.get(&token) { + Event::Internal(InternalEvent::RouteTimer(token, *widget_id)) + } else { + log::error!("No widget found for timer {:?}", token); + return false; + } } other => other, }; @@ -198,7 +206,6 @@ impl Window { window: &self.handle, window_id: self.id, focus_widget: self.focus, - timers: &self.timers, }; self.root.event(&mut ctx, &event, data, env); From 2e6b4935f87a693ce13b610474b8ebd40ea56327 Mon Sep 17 00:00:00 2001 From: Sujit Joshi Date: Sun, 26 Apr 2020 15:14:07 -0400 Subject: [PATCH 07/10] Changes as per https://github.com/xi-editor/druid/pull/831#issuecomment-616188133 --- druid/src/core.rs | 4 ++-- druid/src/window.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/druid/src/core.rs b/druid/src/core.rs index a5945e3ae0..7f86cb21e6 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -602,8 +602,8 @@ impl> WidgetPod { recurse = had_active || child_ctx.base_state.is_hot; Event::Zoom(*zoom) } - Event::Timer(id) => { - panic!("We cannot be here"); + Event::Timer(token) => { + Event::Timer(*token) } Event::Command(cmd) => Event::Command(cmd.clone()), }; diff --git a/druid/src/window.rs b/druid/src/window.rs index 3cd9edcdb8..8041287ef4 100644 --- a/druid/src/window.rs +++ b/druid/src/window.rs @@ -228,7 +228,7 @@ impl Window { //In some platforms, timer tokens are reused. So it is necessary to remove token from //window's timer before adding base state's timers to it. - if let Event::Timer(token) = event { + if let Event::Internal(InternalEvent::RouteTimer(token, _)) = event { self.timers.remove(&token); } From 9a919132206a3b0c390bfd9b0ba98c545cfe1148 Mon Sep 17 00:00:00 2001 From: Sujit Joshi Date: Sun, 26 Apr 2020 15:42:25 -0400 Subject: [PATCH 08/10] Removing unused import. --- druid/src/contexts.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index f904851d2f..dc3f9df4dd 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -24,7 +24,6 @@ use crate::{ Affine, Command, Cursor, Insets, Point, Rect, Size, Target, Text, TimerToken, Vec2, WidgetId, WindowHandle, WindowId, }; -use std::collections::HashMap; /// A mutable context provided to event handling methods of widgets. /// From 0b777632940f4cd7e351d369c41fee6c85b15849 Mon Sep 17 00:00:00 2001 From: sjoshid Date: Sun, 26 Apr 2020 15:58:12 -0400 Subject: [PATCH 09/10] 1) Fmt changes --- druid/src/core.rs | 8 +++----- druid/src/window.rs | 6 +++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/druid/src/core.rs b/druid/src/core.rs index 7f86cb21e6..b6719cff7c 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -21,8 +21,8 @@ use crate::kurbo::{Affine, Insets, Point, Rect, Shape, Size, Vec2}; use crate::piet::RenderContext; use crate::{ BoxConstraints, Command, Data, Env, Event, EventCtx, InternalEvent, InternalLifeCycle, - LayoutCtx, LifeCycle, LifeCycleCtx, PaintCtx, Region, Target, TimerToken, UpdateCtx, Widget, WidgetId, - WindowId, + LayoutCtx, LifeCycle, LifeCycleCtx, PaintCtx, Region, Target, TimerToken, UpdateCtx, Widget, + WidgetId, WindowId, }; /// Our queue type @@ -602,9 +602,7 @@ impl> WidgetPod { recurse = had_active || child_ctx.base_state.is_hot; Event::Zoom(*zoom) } - Event::Timer(token) => { - Event::Timer(*token) - } + Event::Timer(token) => Event::Timer(*token), Event::Command(cmd) => Event::Command(cmd.clone()), }; if recurse { diff --git a/druid/src/window.rs b/druid/src/window.rs index 8041287ef4..c863458bb3 100644 --- a/druid/src/window.rs +++ b/druid/src/window.rs @@ -29,8 +29,8 @@ use crate::widget::LabelText; use crate::win_handler::RUN_COMMANDS_TOKEN; use crate::{ BoxConstraints, Command, Data, Env, Event, EventCtx, InternalEvent, InternalLifeCycle, - LayoutCtx, LifeCycle, LifeCycleCtx, MenuDesc, PaintCtx, TimerToken, UpdateCtx, Widget, WidgetId, WidgetPod, - WindowDesc, + LayoutCtx, LifeCycle, LifeCycleCtx, MenuDesc, PaintCtx, TimerToken, UpdateCtx, Widget, + WidgetId, WidgetPod, WindowDesc, }; /// A unique identifier for a window. @@ -173,7 +173,7 @@ impl Window { let scale = 96.0 / dpi; self.size = Size::new(size.width * scale, size.height * scale); Event::WindowSize(self.size) - }, + } Event::Timer(token) => { if let Some(widget_id) = self.timers.get(&token) { Event::Internal(InternalEvent::RouteTimer(token, *widget_id)) From 38210fa63c925d56885b4d0941cda0b977b0caa1 Mon Sep 17 00:00:00 2001 From: sjoshid Date: Mon, 27 Apr 2020 09:33:24 -0400 Subject: [PATCH 10/10] 1) Code review changes. --- druid/src/core.rs | 5 ++++- druid/src/event.rs | 1 + druid/src/window.rs | 6 +++--- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/druid/src/core.rs b/druid/src/core.rs index b6719cff7c..1c3cca22f1 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -602,7 +602,10 @@ impl> WidgetPod { recurse = had_active || child_ctx.base_state.is_hot; Event::Zoom(*zoom) } - Event::Timer(token) => Event::Timer(*token), + Event::Timer(token) => { + recurse = false; + Event::Timer(*token) + } Event::Command(cmd) => Event::Command(cmd.clone()), }; if recurse { diff --git a/druid/src/event.rs b/druid/src/event.rs index 7c4c8d80ac..cb2ebd885d 100644 --- a/druid/src/event.rs +++ b/druid/src/event.rs @@ -147,6 +147,7 @@ pub enum InternalEvent { MouseLeave, /// A command still in the process of being dispatched. TargetedCommand(Target, Command), + /// Used for routing timer events. RouteTimer(TimerToken, WidgetId), } diff --git a/druid/src/window.rs b/druid/src/window.rs index c863458bb3..2437530a89 100644 --- a/druid/src/window.rs +++ b/druid/src/window.rs @@ -226,13 +226,13 @@ impl Window { self.post_event_processing(queue, data, env, false); - //In some platforms, timer tokens are reused. So it is necessary to remove token from - //window's timer before adding base state's timers to it. + //In some platforms, timer tokens are reused. So it is necessary to remove the token from + //the window's timer map before adding new tokens to it. if let Event::Internal(InternalEvent::RouteTimer(token, _)) = event { self.timers.remove(&token); } - //If at least one widget requested timer, collect those timers from widgets and add to window's timers map. + //If at least one widget requested a timer, add all the requested timers to window's timers map. if base_state.request_timer { self.timers.extend(base_state.timers); }