From 8b78405bfd29572eca569649881cd7fe0bff0928 Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Thu, 13 Aug 2020 11:50:10 -0700 Subject: [PATCH 1/6] First cut at request_update Fixes #1073. This version is largely for discussion, as it hasn't been tested at all. --- druid/src/contexts.rs | 19 ++++++++++++++++++- druid/src/core.rs | 29 +++++++++++++++++++---------- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index 3d4e156db1..33939ac381 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -25,7 +25,7 @@ use crate::piet::Piet; use crate::piet::RenderContext; use crate::{ commands, Affine, Command, ContextMenu, Cursor, Insets, MenuDesc, Point, Rect, SingleUse, Size, - Target, Text, TimerToken, Vec2, WidgetId, WindowDesc, WindowHandle, WindowId, + Target, Text, TimerToken, Vec2, WidgetId, WidgetPod, WindowDesc, WindowHandle, WindowId, }; /// A macro for implementing methods on multiple contexts. @@ -495,6 +495,23 @@ impl EventCtx<'_, '_> { ); } } + + /// Request an update cycle. + /// + /// After this, `update` will be called on the widget in the next update cycle, even + /// if there's not a data change. + pub fn request_update(&mut self) { + self.widget_state.request_update = true; + } + + /// Request an update cycle for a child widget. + /// + /// After this, `update` will be called on the child widget in the next update cycle, + /// even if there's not a data change. + pub fn request_update_child(&mut self, child: &mut WidgetPod) { + self.widget_state.request_update = true; + child.state.request_update = true; + } } impl LifeCycleCtx<'_, '_> { diff --git a/druid/src/core.rs b/druid/src/core.rs index e9a024bc4f..79ee0df96b 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -48,7 +48,7 @@ pub(crate) type CommandQueue = VecDeque<(Target, Command)>; /// /// [`update`]: trait.Widget.html#tymethod.update pub struct WidgetPod { - state: WidgetState, + pub(crate) state: WidgetState, old_data: Option, env: Option, inner: W, @@ -108,6 +108,9 @@ pub(crate) struct WidgetState { /// Any descendant has requested an animation frame. pub(crate) request_anim: bool, + /// Any descendant has requested update. + pub(crate) request_update: bool, + pub(crate) focus_chain: Vec, pub(crate) request_focus: Option, pub(crate) children: Bloom, @@ -844,15 +847,17 @@ impl> WidgetPod { /// /// [`update`]: trait.Widget.html#tymethod.update pub fn update(&mut self, ctx: &mut UpdateCtx, data: &T, env: &Env) { - match (self.old_data.as_ref(), self.env.as_ref()) { - (Some(d), Some(e)) if d.same(data) && e.same(env) => return, - (None, _) => { - log::warn!("old_data missing in {:?}, skipping update", self.id()); - self.old_data = Some(data.clone()); - self.env = Some(env.clone()); - return; + if !ctx.widget_state.request_update { + match (self.old_data.as_ref(), self.env.as_ref()) { + (Some(d), Some(e)) if d.same(data) && e.same(env) => return, + (None, _) => { + log::warn!("old_data missing in {:?}, skipping update", self.id()); + self.old_data = Some(data.clone()); + self.env = Some(env.clone()); + return; + } + _ => (), } - _ => (), } let mut child_ctx = UpdateCtx { @@ -865,7 +870,8 @@ impl> WidgetPod { self.old_data = Some(data.clone()); self.env = Some(env.clone()); - ctx.widget_state.merge_up(&mut self.state) + ctx.widget_state.merge_up(&mut self.state); + ctx.widget_state.request_update = false; } } @@ -877,6 +883,7 @@ impl + 'static> WidgetPod { pub fn boxed(self) -> WidgetPod>> { WidgetPod::new(Box::new(self.inner)) } + } impl WidgetState { @@ -893,6 +900,7 @@ impl WidgetState { has_active: false, has_focus: false, request_anim: false, + request_update: false, request_focus: None, focus_chain: Vec::new(), children: Bloom::new(), @@ -925,6 +933,7 @@ impl WidgetState { self.has_active |= child_state.has_active; self.has_focus |= child_state.has_focus; self.children_changed |= child_state.children_changed; + self.request_update |= child_state.request_update; self.request_focus = child_state.request_focus.take().or(self.request_focus); self.timers.extend_drain(&mut child_state.timers); } From 2ca7dcaaac2108b55ad3fe824782921c070c257b Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Thu, 13 Aug 2020 15:09:51 -0700 Subject: [PATCH 2/6] Clear up parent/child context confusion --- druid/src/core.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/druid/src/core.rs b/druid/src/core.rs index 79ee0df96b..839d85f7ab 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -847,7 +847,7 @@ impl> WidgetPod { /// /// [`update`]: trait.Widget.html#tymethod.update pub fn update(&mut self, ctx: &mut UpdateCtx, data: &T, env: &Env) { - if !ctx.widget_state.request_update { + if !self.state.request_update { match (self.old_data.as_ref(), self.env.as_ref()) { (Some(d), Some(e)) if d.same(data) && e.same(env) => return, (None, _) => { @@ -870,8 +870,8 @@ impl> WidgetPod { self.old_data = Some(data.clone()); self.env = Some(env.clone()); + self.state.request_update = false; ctx.widget_state.merge_up(&mut self.state); - ctx.widget_state.request_update = false; } } From 6c88295513bc1f7bc02ad8abdf613cd9d869e06b Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Mon, 17 Aug 2020 09:21:51 -0700 Subject: [PATCH 3/6] Clean up patch Remove the `request_update_child` variant, as it likely won't be used. Add changelog. --- CHANGELOG.md | 2 ++ druid/src/contexts.rs | 11 +---------- druid/src/core.rs | 3 +-- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bda1fd677..5711e63c2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ You can find its changes [documented below](#060---2020-06-01). - Re-export `druid_shell::Scalable` under `druid` namespace. ([#1075] by [@ForLoveOfCats]) - `TextBox` now supports ctrl and shift hotkeys. ([#1076] by [@vkahl]) - Added selection text color to textbox. ([#1093] by [@sysint64]) +- `request_update` ([#1145] by [@raphlinus]) ### Changed @@ -384,6 +385,7 @@ Last release without a changelog :( [#1103]: https://github.com/linebender/druid/pull/1103 [#1119]: https://github.com/linebender/druid/pull/1119 [#1120]: https://github.com/linebender/druid/pull/1120 +[#1145]: https://github.com/linebender/druid/pull/1145 [Unreleased]: https://github.com/linebender/druid/compare/v0.6.0...master [0.6.0]: https://github.com/linebender/druid/compare/v0.5.0...v0.6.0 diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index 33939ac381..d5c74c356b 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -25,7 +25,7 @@ use crate::piet::Piet; use crate::piet::RenderContext; use crate::{ commands, Affine, Command, ContextMenu, Cursor, Insets, MenuDesc, Point, Rect, SingleUse, Size, - Target, Text, TimerToken, Vec2, WidgetId, WidgetPod, WindowDesc, WindowHandle, WindowId, + Target, Text, TimerToken, Vec2, WidgetId, WindowDesc, WindowHandle, WindowId, }; /// A macro for implementing methods on multiple contexts. @@ -503,15 +503,6 @@ impl EventCtx<'_, '_> { pub fn request_update(&mut self) { self.widget_state.request_update = true; } - - /// Request an update cycle for a child widget. - /// - /// After this, `update` will be called on the child widget in the next update cycle, - /// even if there's not a data change. - pub fn request_update_child(&mut self, child: &mut WidgetPod) { - self.widget_state.request_update = true; - child.state.request_update = true; - } } impl LifeCycleCtx<'_, '_> { diff --git a/druid/src/core.rs b/druid/src/core.rs index 839d85f7ab..783b48a6e0 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -48,7 +48,7 @@ pub(crate) type CommandQueue = VecDeque<(Target, Command)>; /// /// [`update`]: trait.Widget.html#tymethod.update pub struct WidgetPod { - pub(crate) state: WidgetState, + state: WidgetState, old_data: Option, env: Option, inner: W, @@ -883,7 +883,6 @@ impl + 'static> WidgetPod { pub fn boxed(self) -> WidgetPod>> { WidgetPod::new(Box::new(self.inner)) } - } impl WidgetState { From 800fe1c2f204b242b68126481ffb08725d93b401 Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Thu, 20 Aug 2020 08:08:32 -0700 Subject: [PATCH 4/6] Fix commit number I forgot a commit number was already assigned, as this was in draft. Also remove generic parameters, as per review. --- CHANGELOG.md | 4 ++-- druid/src/contexts.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc6c2d32c9..d0b0ef51c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ You can find its changes [documented below](#060---2020-06-01). - `BoxConstraints::UNBOUNDED` constant. ([#1126] by [@danieldulaney]) - Close requests from the shell can now be intercepted ([#1118] by [@jneem]) - The Lens derive now supports an `ignore` attribute. ([#1133] by [@jneem]) -- `request_update` in `EventCtx`. ([#1145] by [@raphlinus]) +- `request_update` in `EventCtx`. ([#1128] by [@raphlinus]) ### Changed @@ -390,8 +390,8 @@ Last release without a changelog :( [#1119]: https://github.com/linebender/druid/pull/1119 [#1120]: https://github.com/linebender/druid/pull/1120 [#1126]: https://github.com/linebender/druid/pull/1120 +[#1128]: https://github.com/linebender/druid/pull/1128 [#1133]: https://github.com/linebender/druid/pull/1133 -[#1145]: https://github.com/linebender/druid/pull/1145 [Unreleased]: https://github.com/linebender/druid/compare/v0.6.0...master [0.6.0]: https://github.com/linebender/druid/compare/v0.5.0...v0.6.0 diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index d5c74c356b..4a0dddf593 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -500,7 +500,7 @@ impl EventCtx<'_, '_> { /// /// After this, `update` will be called on the widget in the next update cycle, even /// if there's not a data change. - pub fn request_update(&mut self) { + pub fn request_update(&mut self) { self.widget_state.request_update = true; } } From 49fdad4a99ababeb9cf09f3c66fbf545fe09e9e6 Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Thu, 20 Aug 2020 08:49:48 -0700 Subject: [PATCH 5/6] Add test --- druid/src/tests/mod.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/druid/src/tests/mod.rs b/druid/src/tests/mod.rs index 1c173acd89..268e3f9dea 100644 --- a/druid/src/tests/mod.rs +++ b/druid/src/tests/mod.rs @@ -463,3 +463,29 @@ fn register_after_adding_child() { assert_eq!(harness.get_state(id_5).children.entry_count(), 5); }) } + +#[test] +/// Test that request_update actually causes the request. +fn request_update() { + const REQUEST_UPDATE: Selector = Selector::new("druid-tests.request_update"); + let updated: Rc> = Default::default(); + let updated_clone = updated.clone(); + + let widget = ModularWidget::new(()) + .event_fn(|_, ctx, event, _data, _env| { + if let Event::Command(cmd) = event { + if cmd.is(REQUEST_UPDATE) { + ctx.request_update(); + } + } + }) + .update_fn(move |_, _ctx, _old_data, _data, _env| { + updated_clone.set(true); + }); + Harness::create_simple((), widget, |harness| { + harness.send_initial_events(); + assert!(!updated.get()); + harness.submit_command(REQUEST_UPDATE, None); + assert!(updated.get()); + }) +} From bb47a0e0e9cea01360fe3e1ceee1faae6ac0abcf Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Thu, 20 Aug 2020 09:13:55 -0700 Subject: [PATCH 6/6] Doc update, per review --- druid/src/contexts.rs | 5 +++++ druid/src/tests/mod.rs | 6 ++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index 4a0dddf593..dbf05233ea 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -500,6 +500,11 @@ impl EventCtx<'_, '_> { /// /// After this, `update` will be called on the widget in the next update cycle, even /// if there's not a data change. + /// + /// The use case for this method is when a container widget synthesizes data for its + /// children. This is appropriate in specialized cases, but before reaching for this + /// method, consider whether it might be better to refactor to be more idiomatic, in + /// particular to make that data available in the app state. pub fn request_update(&mut self) { self.widget_state.request_update = true; } diff --git a/druid/src/tests/mod.rs b/druid/src/tests/mod.rs index 268e3f9dea..f20e1eae98 100644 --- a/druid/src/tests/mod.rs +++ b/druid/src/tests/mod.rs @@ -473,10 +473,8 @@ fn request_update() { let widget = ModularWidget::new(()) .event_fn(|_, ctx, event, _data, _env| { - if let Event::Command(cmd) = event { - if cmd.is(REQUEST_UPDATE) { - ctx.request_update(); - } + if matches!(event, Event::Command(cmd) if cmd.is(REQUEST_UPDATE)) { + ctx.request_update(); } }) .update_fn(move |_, _ctx, _old_data, _data, _env| {