fix tooltips not disappearing, clippy

wgui/src/components/button.rs

@@ -1,6 +1,6 @@
 use crate::{
 	animation::{Animation, AnimationEasing},
-	components::{self, tooltip::ComponentTooltip, Component, ComponentBase, ComponentTrait, RefreshData},
+	components::{self, Component, ComponentBase, ComponentTrait, RefreshData, tooltip::ComponentTooltip},
 	drawing::{self, Boundary, Color},
 	event::{CallbackDataCommon, EventListenerCollection, EventListenerID, EventListenerKind},
 	i18n::Translation,
@@ -10,15 +10,15 @@ use crate::{
 		util::centered_matrix,
 	},
 	widget::{
+		self, ConstructEssentials, EventResult, WidgetData,
 		label::{WidgetLabel, WidgetLabelParams},
 		rectangle::{WidgetRectangle, WidgetRectangleParams},
 		util::WLength,
-		ConstructEssentials, EventResult, WidgetData,
 	},
 };
 use glam::{Mat4, Vec3};
 use std::{cell::RefCell, rc::Rc};
-use taffy::{prelude::length, AlignItems, JustifyContent};
+use taffy::{AlignItems, JustifyContent, prelude::length};
 
 pub struct Params {
 	pub text: Option<Translation>, // if unset, label will not be populated
@@ -91,8 +91,19 @@ impl ComponentTrait for ComponentButton {
 		&mut self.base
 	}
 
-	fn refresh(&self, _data: &mut RefreshData) {
+	fn refresh(&self, data: &mut RefreshData) {
 		// nothing to do
+		let mut state = self.state.borrow_mut();
+
+		if state.active_tooltip.is_some() {
+			if let Some(node_id) = data.common.state.nodes.get(self.base.get_id()) {
+				if !widget::is_node_visible(&data.common.state.tree, *node_id) {
+					state.active_tooltip = None; // destroy the tooltip, this button is now hidden
+				}
+			} else {
+				debug_assert!(false);
+			}
+		}
 	}
 }
 
@@ -472,6 +483,6 @@ pub fn construct(ess: &mut ConstructEssentials, params: Params) -> anyhow::Resul
 
 	let button = Rc::new(ComponentButton { base, data, state });
 
-	ess.layout.defer_component_refresh(Component(button.clone()));
+	ess.layout.register_component_refresh(Component(button.clone()));
 	Ok((root, button))
 }

wgui/src/components/mod.rs

@@ -1,4 +1,5 @@
-use std::rc::Rc;
+use std::hash::Hash;
+use std::{hash::Hasher, rc::Rc};
 
 use crate::{
 	any::AnyTrait,
@@ -54,3 +55,21 @@ impl Component {
 		unsafe { Ok(Rc::from_raw(Rc::into_raw(self.0.clone()).cast())) }
 	}
 }
+
+// these hash/eq impls are required in case we want to do something like HashSet<Component> for convenience reasons.
+
+// hash by address
+impl Hash for Component {
+	fn hash<H: Hasher>(&self, state: &mut H) {
+		std::ptr::hash(&raw const self.0, state);
+	}
+}
+
+// match by address
+impl PartialEq for Component {
+	fn eq(&self, other: &Self) -> bool {
+		std::ptr::eq(&raw const self.0, &raw const other.0)
+	}
+}
+
+impl Eq for Component {}

wgui/src/layout.rs

@@ -1,6 +1,6 @@
 use std::{
 	cell::{RefCell, RefMut},
-	collections::{HashMap, VecDeque},
+	collections::{HashMap, HashSet, VecDeque},
 	io::Write,
 	rc::{Rc, Weak},
 };
@@ -139,7 +139,7 @@ pub struct Layout {
 
 	pub tasks: LayoutTasks,
 
-	components_to_refresh_once: Vec<Component>,
+	components_to_refresh_once: HashSet<Component>,
 	registered_components_to_refresh: HashMap<taffy::NodeId, Component>,
 
 	pub widgets_to_tick: Vec<WidgetID>,
@@ -320,10 +320,6 @@ impl Layout {
 				alterables,
 			};
 
-			/* todo
-			let widget_id = comp.0.base().get_id();
-			 */
-
 			comp.0.refresh(&mut RefreshData { common: &mut common });
 		}
 		self.components_to_refresh_once.clear();
@@ -340,12 +336,12 @@ impl Layout {
 		self.widgets_to_tick.clear();
 	}
 
-	// call ComponentTrait::refresh() once
+	// call ComponentTrait::refresh() *once* in the next tick
 	pub fn defer_component_refresh(&mut self, component: Component) {
-		self.components_to_refresh_once.push(component);
+		self.components_to_refresh_once.insert(component);
 	}
 
-	// call ComponentTrait::refresh() every time the layout is dirty
+	// call ComponentTrait::refresh() *every time time* the layout is dirty
 	pub fn register_component_refresh(&mut self, component: Component) {
 		let widget_id = component.0.base().get_id();
 		let Some(node_id) = self.state.nodes.get(widget_id) else {
@@ -571,7 +567,7 @@ impl Layout {
 			needs_redraw: true,
 			haptics_triggered: false,
 			animations: Animations::default(),
-			components_to_refresh_once: Vec::new(),
+			components_to_refresh_once: HashSet::new(),
 			registered_components_to_refresh: HashMap::new(),
 			widgets_to_tick: Vec::new(),
 			tasks: LayoutTasks::new(),
@@ -607,9 +603,9 @@ impl Layout {
 		self.refresh_recursively(self.tree_root_node, &mut to_refresh);
 
 		if !to_refresh.is_empty() {
-			log::debug!("refreshing {} registered widgets", to_refresh.len());
+			log::debug!("refreshing {} registered components", to_refresh.len());
 			for c in &to_refresh {
-				self.components_to_refresh_once.push(c.clone());
+				self.components_to_refresh_once.insert(c.clone());
 			}
 		}
 
@@ -725,13 +721,22 @@ impl Layout {
 			}
 		}
 
-		for (widget_id, style) in alterables.style_set_requests {
+		for (widget_id, new_style) in alterables.style_set_requests {
-			if let Some(node_id) = self.state.nodes.get(widget_id)
+			if let Some(node_id) = self.state.nodes.get(widget_id) {
-				&& let Err(e) = self.state.tree.set_style(*node_id, style)
+				let old_style = self.state.tree.style(*node_id).unwrap() /* always safe */;
+
+				// refresh the component in case if visibility/display mode has changed
+				if old_style.display != new_style.display
+					&& let Some(component) = self.registered_components_to_refresh.get(node_id)
 				{
+					self.components_to_refresh_once.insert(component.clone());
+				}
+
+				if let Err(e) = self.state.tree.set_style(*node_id, new_style) {
 					log::error!("failed to set style for taffy widget ID {node_id:?}: {e:?}");
 				}
 			}
+		}
 
 		Ok(())
 	}

wgui/src/widget/mod.rs

@@ -1,4 +1,5 @@
 use glam::Vec2;
+use taffy::{NodeId, TaffyTree};
 
 use super::drawing::RenderPrimitive;
 
@@ -392,25 +393,37 @@ impl WidgetState {
 
 		let step_pixels = 64.0;
 
-		if info.handle_size.x < 1.0 && wheel.pos.x != 0.0 {
+		let mut handle_scroll =
-			// Horizontal scrolling
+			|scrolling_target: &mut f32, wheel_delta: f32, handle_size: f32, content_length: f32, content_box_length: f32| {
-			let mult = (1.0 / (l.content_box_width() - info.content_size.x)) * step_pixels;
+				if handle_size >= 1.0 || wheel_delta == 0.0 {
-			let new_scroll = (self.data.scrolling_target.x + wheel.delta.x * mult).clamp(0.0, 1.0);
+					return;
-			if self.data.scrolling_target.x != new_scroll {
-				self.data.scrolling_target.x = new_scroll;
-				params.alterables.mark_tick(self.obj.get_id());
-			}
 				}
 
-		if info.handle_size.y < 1.0 && wheel.pos.y != 0.0 {
+				let mult = (1.0 / (content_box_length - content_length)) * step_pixels;
-			// Vertical scrolling
+				let new_scroll = (*scrolling_target + wheel_delta * mult).clamp(0.0, 1.0);
-			let mult = (1.0 / (l.content_box_height() - info.content_size.y)) * step_pixels;
+				if *scrolling_target == new_scroll {
-			let new_scroll = (self.data.scrolling_target.y + wheel.delta.y * mult).clamp(0.0, 1.0);
+					return;
-			if self.data.scrolling_target.y != new_scroll {
+				}
-				self.data.scrolling_target.y = new_scroll;
+
+				*scrolling_target = new_scroll;
 				params.alterables.mark_tick(self.obj.get_id());
-			}
+			};
-		}
+
+		handle_scroll(
+			&mut self.data.scrolling_target.x,
+			wheel.delta.x,
+			info.handle_size.x,
+			info.content_size.x,
+			l.content_box_width(),
+		);
+
+		handle_scroll(
+			&mut self.data.scrolling_target.y,
+			wheel.delta.y,
+			info.handle_size.y,
+			info.content_size.y,
+			l.content_box_height(),
+		);
 
 		true
 	}
@@ -507,3 +520,20 @@ pub struct ConstructEssentials<'a> {
 	pub layout: &'a mut Layout,
 	pub parent: WidgetID,
 }
+
+/// Determines whether a given node is visible within the layout tree.
+///
+/// Traversal is definitely a little bit more expensive than just checking the value, but
+/// Taffy doesn't calculate that for us, so here it is.
+pub fn is_node_visible(tree: &TaffyTree<WidgetID>, node_id: NodeId) -> bool {
+	let mut cur = Some(node_id);
+	while let Some(node_id) = cur {
+		if let Ok(style) = tree.style(node_id)
+			&& style.display == taffy::Display::None
+		{
+			return false;
+		}
+		cur = tree.parent(node_id);
+	}
+	true
+}

wlx-overlay-s/src/gui/panel/button.rs

@@ -59,7 +59,7 @@ pub(super) fn setup_custom_button<S: 'static>(
                 })
             }
             "::OverlayToggle" => {
-                let Some(arg): Option<Arc<str>> = args.next().map(|a| a.into()) else {
+                let Some(arg): Option<Arc<str>> = args.next().map(Into::into) else {
                     log::error!("{command} has missing arguments");
                     return;
                 };

wlx-overlay-s/src/main.rs

@@ -13,7 +13,8 @@
     clippy::struct_excessive_bools,
     clippy::needless_pass_by_value,
     clippy::needless_pass_by_ref_mut,
-    clippy::multiple_crate_versions
+    clippy::multiple_crate_versions,
+    clippy::cargo_common_metadata
 )]
 mod backend;
 mod config;

wlx-overlay-s/src/overlays/edit/mod.rs

@@ -19,22 +19,22 @@ use wgui::{
 use crate::{backend::task::TaskType, windowing::OverlaySelector};
 use crate::{
     backend::{input::HoverResult, task::TaskContainer},
-    gui::panel::{button::BUTTON_EVENTS, GuiPanel, NewGuiPanelParams, OnCustomAttribFunc},
+    gui::panel::{GuiPanel, NewGuiPanelParams, OnCustomAttribFunc, button::BUTTON_EVENTS},
     overlays::edit::{
         lock::InteractLockHandler, pos::PositioningHandler, tab::ButtonPaneTabSwitcher,
     },
     state::AppState,
     subsystem::hid::WheelDelta,
     windowing::{
+        OverlayID,
         backend::{DummyBackend, OverlayBackend, OverlayEventData, RenderResources, ShouldRender},
         window::OverlayWindowConfig,
-        OverlayID,
     },
 };
 
 mod lock;
 mod pos;
-pub(crate) mod tab;
+pub mod tab;
 
 pub(super) struct LongPressButtonState {
     pub(super) pressed: Instant,

wlx-overlay-s/src/overlays/watch.rs

@@ -19,16 +19,16 @@ use wlx_common::windowing::{OverlayWindowState, Positioning};
 use crate::{
     backend::task::{ManagerTask, TaskType},
     gui::{
-        panel::{button::BUTTON_EVENTS, GuiPanel, NewGuiPanelParams, OnCustomAttribFunc},
+        panel::{GuiPanel, NewGuiPanelParams, OnCustomAttribFunc, button::BUTTON_EVENTS},
         timer::GuiTimer,
     },
     overlays::edit::LongPressButtonState,
     state::AppState,
     windowing::{
+        OverlaySelector, Z_ORDER_WATCH,
         backend::{OverlayEventData, OverlayMeta},
         manager::MAX_OVERLAY_SETS,
         window::{OverlayWindowConfig, OverlayWindowData},
-        OverlaySelector, Z_ORDER_WATCH,
     },
 };
 
@@ -48,6 +48,7 @@ struct WatchState {
 }
 
 #[allow(clippy::significant_drop_tightening)]
+#[allow(clippy::too_many_lines)]
 pub fn create_watch(app: &mut AppState) -> anyhow::Result<OverlayWindowConfig> {
     let state = WatchState::default();
 
@@ -92,7 +93,7 @@ pub fn create_watch(app: &mut AppState) -> anyhow::Result<OverlayWindowConfig> {
                         };
 
                         app.tasks.enqueue(TaskType::Overlay(
-                            OverlaySelector::Id(overlay.id.clone()),
+                            OverlaySelector::Id(overlay.id),
                             Box::new(move |app, owc| {
                                 if owc.active_state.is_none() {
                                     owc.activate(app);
@@ -187,42 +188,36 @@ pub fn create_watch(app: &mut AppState) -> anyhow::Result<OverlayWindowConfig> {
                     } else {
                         taffy::Display::None
                     };
-                    panel.widget_set_display(rect_id, display, &mut common.alterables);
+                    panel.widget_set_display(rect_id, display, common.alterables);
                 }
                 let display = if num_sets < 7 {
                     taffy::Display::Flex
                 } else {
                     taffy::Display::None
                 };
-                panel.widget_set_display(
+                panel.widget_set_display(panel.state.edit_add_widget, display, common.alterables);
-                    panel.state.edit_add_widget,
-                    display,
-                    &mut common.alterables,
-                );
             }
             OverlayEventData::EditModeChanged(edit_mode) => {
-                for (w, e) in panel.state.edit_mode_widgets.iter() {
+                for (w, e) in &panel.state.edit_mode_widgets {
                     let display = if *e == edit_mode {
                         taffy::Display::Flex
                     } else {
                         taffy::Display::None
                     };
-                    panel.widget_set_display(*w, display, &mut common.alterables);
+                    panel.widget_set_display(*w, display, common.alterables);
                 }
                 let display = if edit_mode && panel.state.num_sets < 7 {
                     taffy::Display::Flex
                 } else {
                     taffy::Display::None
                 };
-                panel.widget_set_display(
+                panel.widget_set_display(panel.state.edit_add_widget, display, common.alterables);
-                    panel.state.edit_add_widget,
-                    display,
-                    &mut common.alterables,
-                );
             }
             OverlayEventData::OverlaysChanged(metas) => {
                 panel.state.overlay_metas = metas;
+                // FIXME: should we suppress this clippy warning in the crate itself? the resulting code isn't always more readable than before
                 for (idx, btn) in panel.state.overlay_buttons.iter().enumerate() {
+                    #[allow(clippy::option_if_let_else)]
                     let display = if let Some(meta) = panel.state.overlay_metas.get(idx) {
                         btn.set_text(&mut common, Translation::from_raw_text(&meta.name));
                         //TODO: add category icons
@@ -230,7 +225,7 @@ pub fn create_watch(app: &mut AppState) -> anyhow::Result<OverlayWindowConfig> {
                     } else {
                         taffy::Display::None
                     };
-                    panel.widget_set_display(btn.get_rect(), display, &mut common.alterables);
+                    panel.widget_set_display(btn.get_rect(), display, common.alterables);
                 }
             }
         }

wlx-overlay-s/src/windowing/backend.rs

@@ -7,8 +7,8 @@ use vulkano::{
     image::view::ImageView,
 };
 use wgui::gfx::{
-    cmd::{GfxCommandBuffer, WGfxClearMode},
     WGfx,
+    cmd::{GfxCommandBuffer, WGfxClearMode},
 };
 
 use crate::{
@@ -16,7 +16,7 @@ use crate::{
     graphics::ExtentExt,
     state::AppState,
     subsystem::hid::WheelDelta,
-    windowing::{window::OverlayCategory, OverlayID},
+    windowing::{OverlayID, window::OverlayCategory},
 };
 
 #[derive(Default, Clone, Copy)]
@@ -71,6 +71,7 @@ pub struct OverlayMeta {
     pub category: OverlayCategory,
 }
 
+#[allow(clippy::enum_variant_names)]
 pub enum OverlayEventData {
     ActiveSetChanged(Option<usize>),
     NumSetsChanged(usize),

wlx-overlay-s/src/windowing/manager.rs

@@ -12,11 +12,11 @@ use crate::{
     },
     state::AppState,
     windowing::{
+        OverlayID, OverlaySelector,
         backend::{OverlayEventData, OverlayMeta},
         set::OverlayWindowSet,
         snap_upright,
         window::{OverlayCategory, OverlayWindowData},
-        OverlayID, OverlaySelector,
     },
 };
 
@@ -246,12 +246,7 @@ impl<T> OverlayWindowManager<T> {
     ) -> Option<OverlayWindowData<T>> {
         let id = match selector {
             OverlaySelector::Id(id) => *id,
-            OverlaySelector::Name(name) => {
+            OverlaySelector::Name(name) => self.lookup(name)?,
-                let Some(id) = self.lookup(name) else {
-                    return None;
-                };
-                id
-            }
         };
 
         let ret_val = self.overlays.remove(id);
@@ -260,7 +255,7 @@ impl<T> OverlayWindowManager<T> {
             .is_some_and(|o| matches!(o.config.category, OverlayCategory::Internal));
 
         if !internal && let Err(e) = self.overlays_changed(app) {
-            log::error!("Error while removing overlay: {e:?}")
+            log::error!("Error while removing overlay: {e:?}");
         }
 
         ret_val
@@ -455,12 +450,12 @@ impl<T> OverlayWindowManager<T> {
 
     fn overlays_changed(&mut self, app: &mut AppState) -> anyhow::Result<()> {
         let mut meta = Vec::with_capacity(self.overlays.len());
-        for (id, data) in self.overlays.iter() {
+        for (id, data) in &self.overlays {
             if matches!(data.config.category, OverlayCategory::Internal) {
                 continue;
             }
             meta.push(OverlayMeta {
-                id: id.clone(),
+                id,
                 name: data.config.name.clone(),
                 category: data.config.category,
             });