From a902f569e76eaa16ecd20421ddd90de51d22bf84 Mon Sep 17 00:00:00 2001 From: Kaur Kuut Date: Thu, 9 Apr 2020 15:55:55 +0300 Subject: [PATCH 1/4] Make has_focus reliable. --- druid/src/contexts.rs | 14 ++------------ druid/src/core.rs | 20 ++++++++++++++++---- druid/src/window.rs | 39 +++++++++++++++++++++++++-------------- 3 files changed, 43 insertions(+), 30 deletions(-) diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index bbdebfd161..d514d76178 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -270,12 +270,7 @@ impl<'a> EventCtx<'a> { /// /// [`is_focused`]: struct.EventCtx.html#method.is_focused pub fn has_focus(&self) -> bool { - // The bloom filter we're checking can return false positives. - let is_child = self - .focus_widget - .map(|id| self.base_state.children.may_contain(&id)) - .unwrap_or(false); - is_child || self.focus_widget == Some(self.widget_id()) + self.base_state.has_focus } /// Request keyboard focus. @@ -613,12 +608,7 @@ impl<'a, 'b: 'a> PaintCtx<'a, 'b> { /// [`is_focused`]: #method.is_focused /// [`EventCtx::is_focused`]: struct.EventCtx.html#method.is_focused pub fn has_focus(&self) -> bool { - // The bloom filter we're checking can return false positives. - let is_child = self - .focus_widget - .map(|id| self.base_state.children.may_contain(&id)) - .unwrap_or(false); - is_child || self.focus_widget == Some(self.widget_id()) + self.base_state.has_focus } /// Returns the currently visible [`Region`]. diff --git a/druid/src/core.rs b/druid/src/core.rs index c23402dec6..884643b7d4 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -86,6 +86,10 @@ pub(crate) struct BaseState { /// Any descendant is active. has_active: bool, + /// In the focused path, starting from window and ending at the focused widget. + /// Descendants of the focused widget are not in the focused path. + pub(crate) has_focus: bool, + /// Any descendant has requested an animation frame. pub(crate) request_anim: bool, @@ -514,19 +518,25 @@ impl> WidgetPod { }; if let Some(change) = this_changed { + self.state.has_focus = change; let event = LifeCycle::FocusChanged(change); self.inner.lifecycle(ctx, &event, data, env); false } else { + self.state.has_focus = false; // Recurse when the target widgets could be our descendants. // The bloom filter we're checking can return false positives. - old.map(|id| self.state.children.may_contain(&id)) - .or_else(|| new.map(|id| self.state.children.may_contain(&id))) - .unwrap_or(false) + match (old, new) { + (Some(old), _) if self.state.children.may_contain(old) => true, + (_, Some(new)) if self.state.children.may_contain(new) => true, + _ => false, + } } } LifeCycle::FocusChanged(_) => { - self.state.request_focus = None; + // We are a descendant of a widget that has/had focus + self.state.has_focus = false; // Descendants don't inherit focus + self.state.request_focus = None; // Clear the handled request true } #[cfg(test)] @@ -624,6 +634,7 @@ impl BaseState { needs_layout: false, is_active: false, has_active: false, + has_focus: false, request_anim: false, request_timer: false, request_focus: None, @@ -640,6 +651,7 @@ impl BaseState { self.request_anim |= child_state.request_anim; self.request_timer |= child_state.request_timer; self.has_active |= child_state.has_active; + self.has_focus |= child_state.has_focus; self.children_changed |= child_state.children_changed; self.request_focus = self.request_focus.or(child_state.request_focus); } diff --git a/druid/src/window.rs b/druid/src/window.rs index 8be999066b..724a3ef470 100644 --- a/druid/src/window.rs +++ b/druid/src/window.rs @@ -328,22 +328,33 @@ impl Window { match focus { FocusChange::Resign => None, FocusChange::Focus(id) => Some(id), - FocusChange::Next => self - .focus - .and_then(|id| self.focus_chain().iter().position(|i| i == &id)) - .map(|idx| { - let next_idx = (idx + 1) % self.focus_chain().len(); - self.focus_chain()[next_idx] - }), - FocusChange::Previous => self - .focus - .and_then(|id| self.focus_chain().iter().position(|i| i == &id)) + FocusChange::Next => self.widget_from_focus_chain(true), + FocusChange::Previous => self.widget_from_focus_chain(false), + } + } + + fn widget_from_focus_chain(&self, forward: bool) -> Option { + self.focus.and_then(|focus| { + self.focus_chain() + .iter() + // Find where the focused widget is in the focus chain + .position(|id| id == &focus) .map(|idx| { + // Return the id that's next to it in the focus chain let len = self.focus_chain().len(); - let prev_idx = (idx + len - 1) % len; - self.focus_chain()[prev_idx] - }), - } + let new_idx = if forward { + (idx + 1) % len + } else { + (idx + len - 1) % len + }; + self.focus_chain()[new_idx] + }) + .or_else(|| { + // If the currently focused widget isn't in the focus chain, + // then we'll just return the first entry of the chain, if any. + self.focus_chain().first().copied() + }) + }) } } From 080c742069ef4d7f8d143881413ca4409524ae7a Mon Sep 17 00:00:00 2001 From: Kaur Kuut Date: Thu, 9 Apr 2020 17:47:40 +0300 Subject: [PATCH 2/4] Only send FocusChanged in case there's actual change. --- druid/src/core.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/druid/src/core.rs b/druid/src/core.rs index 884643b7d4..bc71980484 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -518,9 +518,12 @@ impl> WidgetPod { }; if let Some(change) = this_changed { - self.state.has_focus = change; - let event = LifeCycle::FocusChanged(change); - self.inner.lifecycle(ctx, &event, data, env); + // Only send FocusChanged in case there's actual change + if old != new { + self.state.has_focus = change; + let event = LifeCycle::FocusChanged(change); + self.inner.lifecycle(ctx, &event, data, env); + } false } else { self.state.has_focus = false; From fc9825ef0c0d25932d6764e6ede6a7399e8610de Mon Sep 17 00:00:00 2001 From: Kaur Kuut Date: Fri, 10 Apr 2020 13:20:47 +0300 Subject: [PATCH 3/4] Stop propagating FocusChanged to descendants of focused widget. --- druid/src/core.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/druid/src/core.rs b/druid/src/core.rs index bc71980484..86364fc02a 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -537,10 +537,9 @@ impl> WidgetPod { } } LifeCycle::FocusChanged(_) => { - // We are a descendant of a widget that has/had focus - self.state.has_focus = false; // Descendants don't inherit focus - self.state.request_focus = None; // Clear the handled request - true + // We are a descendant of a widget that has/had focus. + // Descendants don't inherit focus, so don't recurse. + false } #[cfg(test)] LifeCycle::DebugRequestState { widget, state_cell } => { From 310b69042e55fc0d8475c894451e754e4603f540 Mon Sep 17 00:00:00 2001 From: Kaur Kuut Date: Fri, 10 Apr 2020 21:52:30 +0300 Subject: [PATCH 4/4] Focus last entry in focus chain when going backwards from none. --- druid/src/window.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/druid/src/window.rs b/druid/src/window.rs index 724a3ef470..826e4f8abc 100644 --- a/druid/src/window.rs +++ b/druid/src/window.rs @@ -351,8 +351,12 @@ impl Window { }) .or_else(|| { // If the currently focused widget isn't in the focus chain, - // then we'll just return the first entry of the chain, if any. - self.focus_chain().first().copied() + // then we'll just return the first/last entry of the chain, if any. + if forward { + self.focus_chain().first().copied() + } else { + self.focus_chain().last().copied() + } }) }) }