diff --git a/masonry/src/action.rs b/masonry/src/action.rs index 72bd0a7bc..29cce6754 100644 --- a/masonry/src/action.rs +++ b/masonry/src/action.rs @@ -5,7 +5,8 @@ use std::any::Any; use crate::event::PointerButton; -// TODO - Refactor - See issue https://github.com/linebender/xilem/issues/335 +// TODO - Replace actions with an associated type on the Widget trait +// See https://github.com/linebender/xilem/issues/664 // TODO - TextCursor changed, ImeChanged, EnterKey, MouseEnter #[non_exhaustive] diff --git a/masonry/src/box_constraints.rs b/masonry/src/box_constraints.rs index fd96d0fe1..5858730b0 100644 --- a/masonry/src/box_constraints.rs +++ b/masonry/src/box_constraints.rs @@ -184,7 +184,17 @@ impl BoxConstraints { /// /// Use this function when maintaining an aspect ratio is more important than minimizing the /// distance between input and output size width and height. + /// + /// ## Panics + /// + /// Panics if `aspect_ratio` or `width` are NaN, infinite or negative. + #[track_caller] pub fn constrain_aspect_ratio(&self, aspect_ratio: f64, width: f64) -> Size { + assert!(aspect_ratio.is_finite()); + assert!(width.is_finite()); + assert!(aspect_ratio >= 0.0); + assert!(width >= 0.0); + // Minimizing/maximizing based on aspect ratio seems complicated, but in reality everything // is linear, so the amount of work to do is low. let ideal_size = Size { @@ -201,8 +211,6 @@ impl BoxConstraints { return ideal_size; } - // Then we check if any `Size`s with our desired aspect ratio are inside the constraints. - // TODO this currently outputs garbage when things are < 0 - See https://github.com/linebender/xilem/issues/377 let min_w_min_h = self.min.height / self.min.width; let max_w_min_h = self.min.height / self.max.width; let min_w_max_h = self.max.height / self.min.width; diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index da78ce259..da8346d61 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -3,8 +3,6 @@ //! The context types that are passed into various widget methods. -use std::time::Duration; - use accesskit::TreeUpdate; use dpi::LogicalPosition; use parley::{FontContext, LayoutContext}; @@ -573,7 +571,7 @@ impl_context_method!(MutateCtx<'_>, EventCtx<'_>, UpdateCtx<'_>, { pub fn children_changed(&mut self) { trace!("children_changed"); self.widget_state.children_changed = true; - self.widget_state.update_focus_chain = true; + self.widget_state.needs_update_focus_chain = true; self.request_layout(); } @@ -709,14 +707,6 @@ impl_context_method!( .push_back(RenderRootSignal::ShowWindowMenu(position)); } - /// Request a timer event. - /// - /// The return value is a token, which can be used to associate the - /// request with the event. - pub fn request_timer(&mut self, _deadline: Duration) -> TimerToken { - todo!("request_timer"); - } - /// Mark child widget as stashed. /// /// If `stashed` is true, the child will not be painted or listed in the accessibility tree. @@ -737,9 +727,6 @@ impl_context_method!( } ); -// FIXME - Remove -pub struct TimerToken; - impl EventCtx<'_> { // TODO - clearly document all semantics of pointer capture when they've been decided on // TODO - Figure out cases where widget should be notified of pointer capture diff --git a/masonry/src/debug_logger.rs b/masonry/src/debug_logger.rs index 212b568b5..344d43210 100644 --- a/masonry/src/debug_logger.rs +++ b/masonry/src/debug_logger.rs @@ -204,7 +204,7 @@ impl DebugLogger { StateTree::new("has_focus", w_state.has_focus), StateTree::new("request_anim", w_state.request_anim), StateTree::new("children_changed", w_state.children_changed), - StateTree::new("update_focus_chain", w_state.update_focus_chain), + StateTree::new("update_focus_chain", w_state.needs_update_focus_chain), ] .into(); state diff --git a/masonry/src/doc/01_creating_app.md b/masonry/src/doc/01_creating_app.md index 3db10c7c1..c648c0da0 100644 --- a/masonry/src/doc/01_creating_app.md +++ b/masonry/src/doc/01_creating_app.md @@ -114,8 +114,8 @@ Because our widget tree only has one button and one textbox, there is no possibl When handling `ButtonPressed`: - `ctx.get_root()` returns a `WidgetMut>`. -- `root.child_mut()` returns a `WidgetMut>` for the `Portal`. -- `portal.child_mut()` returns a `WidgetMut` for the `Flex`. +- `root.child_mut()` returns a `WidgetMut>`. +- `portal.child_mut()` returns a `WidgetMut`. A [`WidgetMut`] is a smart reference type which lets us modify the widget tree. It's set up to automatically propagate update flags and update internal state when dropped. @@ -211,7 +211,7 @@ fn main() { next_task: String::new(), }; -use masonry::dpi::LogicalSize; + use masonry::dpi::LogicalSize; use winit::window::Window; let window_attributes = Window::default_attributes() diff --git a/masonry/src/doc/03_implementing_container_widget.md b/masonry/src/doc/03_implementing_container_widget.md index c4d58c68f..8bfed8d41 100644 --- a/masonry/src/doc/03_implementing_container_widget.md +++ b/masonry/src/doc/03_implementing_container_widget.md @@ -254,11 +254,11 @@ Doesn't `VerticalStack::paint` need to call `paint` on its children, for instanc It doesn't. -In Masonry, most passes are automatically propagated to children, without container widgets having to implement code iterating over their children. +In Masonry, most passes are automatically propagated to children, and so container widgets do not need to *and cannot* call the pass methods on their children. So for instance, if `VerticalStack::children_ids()` returns a list of three children, the paint pass will automatically call `paint` on all three children after `VerticalStack::paint()`. -So various methods in container widgets should only implement the logic that is specific to the container itself. +Pass methods in container widgets should only implement the logic that is specific to the container itself. For instance, a container widget with a background color should implement `paint` to draw the background. [`Widget`]: crate::Widget diff --git a/masonry/src/doc/05_pass_system.md b/masonry/src/doc/05_pass_system.md index 47bf1e47a..1fecad7db 100644 --- a/masonry/src/doc/05_pass_system.md +++ b/masonry/src/doc/05_pass_system.md @@ -12,15 +12,15 @@ -Masonry has a set of **passes**, which are computations run over a subset of the widget tree during every frame. +Masonry's internal architecture is based on a set of **passes**, which are computations run over a subset of the widget tree during every frame. Passes can be split into roughly three categories: -- **Event passes:** triggered by user interaction. -- **Rewrite passes:** run after every event pass, may run multiple times until all invalidation flags are cleared. -- **Render passes:** run just before rendering a new frame. +- **Event passes** are triggered by user interaction. +- **Rewrite passes** run after every event pass, may run multiple times until all invalidation flags are cleared. +- **Render passes** run just before rendering a new frame. -Note: unless otherwise specified, all passes run over widgets in depth-first preorder. +Note that unless otherwise specified, all passes run over widgets in depth-first preorder, where child order is determined by their position in the `children_ids()` array. ## Event passes @@ -68,7 +68,7 @@ To address these invalidations, Masonry runs a set of **rewrite passes** over th The layout and compose passes have methods with matching names in the Widget trait. The update_xxx passes call the widgets' update method. -By default, each of these passes completes immediately, unless pass-dependent invalidation flags are set. +By default, each of these passes completes without doing any work, unless pass-dependent invalidation flags are set. Each pass can generally request work for later passes; for instance, the mutate pass can invalidate the layout of a widget, in which case the layout pass will run on that widget and its children and parents. Passes may also request work for *previous* passes, in which case all rewrite passes are run again in sequence. @@ -106,6 +106,13 @@ It is called when new widgets are added to the tree, or existing widgets are rem It will call the `register_children()` widget method on container widgets whose children changed, then the `update()` method with the `WidgetAdded` event on new widgets. + + + + + + + ### Layout pass The layout pass runs bidirectionally, passing constraints from the top down and getting back sizes and other layout info from the bottom up. @@ -126,8 +133,6 @@ Because the compose pass is more limited than layout, it's easier to recompute i For instance, if a widget in a list changes size, its siblings and parents must be re-laid out to account for the change; whereas changing a given widget's transform only affects its children. -Masonry automatically calls the `compose` methods of all widgets in the tree, in depth-first preorder, where child order is determined by their position in the `children_ids()` array. - ## Render passes @@ -144,17 +149,16 @@ These nodes together form the accessibility tree. Methods for these passes should be written under the assumption that they can be skipped or called multiple times for arbitrary reasons. Therefore, their ability to affect the widget tree is limited. -Masonry automatically calls these methods for all widgets in the tree in depth-first preorder. ## External mutation Code with mutable access to the `RenderRoot`, like the Xilem app runner, can get mutable access to the root widget and all its children through the `edit_root_widget()` method, which takes a callback and passes it a `WidgetMut` to the root widget. -This is in effect a MUTATE pass which only processes one callback. +This is in effect a "mutate" pass which only processes one callback. External mutation is how Xilem applies any changes to the widget tree produced by its reactive step. -Calling the `edit_root_widget()` method, or any similar direct-mutation method, triggers the entire set of rewrite passes. +`RenderRoot::edit_root_widget()` triggers the entire set of rewrite passes before returning. ## Pass context types @@ -164,7 +168,7 @@ Some notes about pass context types: - Render passes should be pure and can be skipped occasionally, therefore their context types ([`PaintCtx`] and [`AccessCtx`]) can't set invalidation flags or send signals. - The `layout` and `compose` passes lay out all widgets, which are transiently invalid during the passes, therefore [`LayoutCtx`]and [`ComposeCtx`] cannot access the size and position of the `self` widget. They can access the layout of children if they have already been laid out. -- For the same reason, `LayoutCtx`and `ComposeCtx` cannot create a `WidgetRef` reference to a child. +- For the same reason, [`LayoutCtx`]and [`ComposeCtx`] cannot create a `WidgetRef` reference to a child. - [`MutateCtx`], [`EventCtx`] and [`UpdateCtx`] can let you add and remove children. - [`RegisterCtx`] can't do anything except register children. - [`QueryCtx`] provides read-only information about the widget. diff --git a/masonry/src/event.rs b/masonry/src/event.rs index 29294f62c..7eec05e3a 100644 --- a/masonry/src/event.rs +++ b/masonry/src/event.rs @@ -181,7 +181,6 @@ impl From for PointerButtons { // TODO - How can RenderRoot express "I started a drag-and-drop op"? // TODO - Touchpad, Touch, AxisMotion // TODO - How to handle CursorEntered? -// Note to self: Events like "pointerenter", "pointerleave" are handled differently at the Widget level. But that's weird because WidgetPod can distribute them. Need to think about this again. #[derive(Debug, Clone)] pub enum PointerEvent { PointerDown(PointerButton, PointerState), @@ -313,6 +312,7 @@ pub enum Update { } impl PointerEvent { + /// Create a [`PointerEvent::PointerLeave`] event with dummy values. pub fn new_pointer_leave() -> Self { // TODO - The fact we're creating so many dummy values might be // a sign we should refactor that struct @@ -328,6 +328,7 @@ impl PointerEvent { PointerEvent::PointerLeave(pointer_state) } + /// Returns the [`PointerState`] of the event. pub fn pointer_state(&self) -> &PointerState { match self { PointerEvent::PointerDown(_, state) @@ -343,6 +344,7 @@ impl PointerEvent { } } + /// Returns the position of the pointer event, except for [`PointerEvent::PointerLeave`] and [`PointerEvent::HoverFileCancel`]. pub fn position(&self) -> Option> { match self { PointerEvent::PointerLeave(_) | PointerEvent::HoverFileCancel(_) => None, @@ -350,6 +352,9 @@ impl PointerEvent { } } + /// Short name, for debug logging. + /// + /// Returns the enum variant name. pub fn short_name(&self) -> &'static str { match self { PointerEvent::PointerDown(_, _) => "PointerDown", @@ -365,6 +370,10 @@ impl PointerEvent { } } + /// Returns true if the event is likely to occur every frame. + /// + /// Developers should avoid logging during high-density events to avoid + /// cluttering the console. pub fn is_high_density(&self) -> bool { match self { PointerEvent::PointerDown(_, _) => false, @@ -382,20 +391,25 @@ impl PointerEvent { } impl TextEvent { + /// Short name, for debug logging. pub fn short_name(&self) -> &'static str { match self { - TextEvent::KeyboardKey(KeyEvent { repeat: true, .. }, _) => "KeyboardKey (repeat)", + TextEvent::KeyboardKey(KeyEvent { repeat: true, .. }, _) => "KeyboardKey(repeat)", TextEvent::KeyboardKey(_, _) => "KeyboardKey", TextEvent::Ime(Ime::Disabled) => "Ime::Disabled", TextEvent::Ime(Ime::Enabled) => "Ime::Enabled", TextEvent::Ime(Ime::Commit(_)) => "Ime::Commit", TextEvent::Ime(Ime::Preedit(s, _)) if s.is_empty() => "Ime::Preedit(\"\")", - TextEvent::Ime(Ime::Preedit(_, _)) => "Ime::Preedit", + TextEvent::Ime(Ime::Preedit(_, _)) => "Ime::Preedit(\"...\")", TextEvent::ModifierChange(_) => "ModifierChange", TextEvent::FocusChange(_) => "FocusChange", } } + /// Returns true if the event is likely to occur every frame. + /// + /// Developers should avoid logging during high-density events to avoid + /// cluttering the console. pub fn is_high_density(&self) -> bool { match self { TextEvent::KeyboardKey(_, _) => false, @@ -408,6 +422,9 @@ impl TextEvent { } impl AccessEvent { + /// Short name, for debug logging. + /// + /// Returns the enum variant name. pub fn short_name(&self) -> &'static str { match self.action { accesskit::Action::Click => "Click", @@ -442,15 +459,6 @@ impl AccessEvent { impl PointerState { pub fn empty() -> Self { - #[cfg(FALSE)] - #[allow(unsafe_code)] - // SAFETY: Uuuuh, unclear. Winit says the dummy id should only be used in - // tests and should never be passed to winit. In principle, we're never - // passing this id to winit, but it's still visible to custom widgets which - // might do so if they tried really hard. - // It would be a lot better if winit could just make this constructor safe. - let device_id = unsafe { DeviceId::dummy() }; - PointerState { physical_position: PhysicalPosition::new(0.0, 0.0), position: LogicalPosition::new(0.0, 0.0), @@ -466,7 +474,7 @@ impl PointerState { impl Update { /// Short name, for debug logging. /// - /// Essentially returns the enum variant name. + /// Returns the enum variant name. pub fn short_name(&self) -> &str { match self { Update::WidgetAdded => "WidgetAdded", diff --git a/masonry/src/passes/accessibility.rs b/masonry/src/passes/accessibility.rs index 03b61ff25..763714d95 100644 --- a/masonry/src/passes/accessibility.rs +++ b/masonry/src/passes/accessibility.rs @@ -102,7 +102,7 @@ fn build_access_node(widget: &mut dyn Widget, ctx: &mut AccessCtx) -> Node { .collect::>(), ); - // Note - These WidgetState flags can be modified by other passes. + // Note - The values returned by these methods can be modified by other passes. // When that happens, the other pass should set flags to request an accessibility pass. if ctx.is_disabled() { node.set_disabled(); diff --git a/masonry/src/passes/compose.rs b/masonry/src/passes/compose.rs index 71eb76242..b882c8582 100644 --- a/masonry/src/passes/compose.rs +++ b/masonry/src/passes/compose.rs @@ -74,7 +74,7 @@ fn compose_widget( pub(crate) fn run_compose_pass(root: &mut RenderRoot) { let _span = info_span!("compose").entered(); - // If widgets are moved, pointer-related info may be stale. + // If widgets have moved, pointer-related info may be stale. // For instance, the "hovered" widget may have moved and no longer be under the pointer. if root.root_state().needs_compose { root.global_state.needs_pointer_pass = true; diff --git a/masonry/src/passes/event.rs b/masonry/src/passes/event.rs index 3a9e3c23a..d9c0c68bd 100644 --- a/masonry/src/passes/event.rs +++ b/masonry/src/passes/event.rs @@ -11,7 +11,7 @@ use crate::render_root::RenderRoot; use crate::{AccessEvent, EventCtx, Handled, PointerEvent, TextEvent, Widget, WidgetId}; // --- MARK: HELPERS --- -fn get_target_widget( +fn get_pointer_target( root: &RenderRoot, pointer_pos: Option>, ) -> Option { @@ -89,8 +89,8 @@ pub(crate) fn run_on_pointer_event_pass(root: &mut RenderRoot, event: &PointerEv if event.is_high_density() { // We still want to record that this pass occurred in the debug file log. - // However, we choose not record any other tracing for this event, - // as that would have a lot of noise. + // However, we choose not to record any other tracing for this event, + // as that would create a lot of noise. trace!("Running ON_POINTER_EVENT pass with {}", event.short_name()); } else { debug!("Running ON_POINTER_EVENT pass with {}", event.short_name()); @@ -101,7 +101,7 @@ pub(crate) fn run_on_pointer_event_pass(root: &mut RenderRoot, event: &PointerEv root.last_mouse_pos = event.position(); } - let target_widget_id = get_target_widget(root, event.position()); + let target_widget_id = get_pointer_target(root, event.position()); let handled = run_event_pass( root, @@ -135,11 +135,6 @@ pub(crate) fn run_on_pointer_event_pass(root: &mut RenderRoot, event: &PointerEv handled } -// TODO https://github.com/linebender/xilem/issues/376 - Some implicit invariants: -// - If a Widget gets a keyboard event or an ImeStateChange, then -// focus is on it, its child or its parent. -// - If a Widget has focus, then none of its parents is hidden - // --- MARK: TEXT EVENT --- pub(crate) fn run_on_text_event_pass(root: &mut RenderRoot, event: &TextEvent) -> Handled { if matches!(event, TextEvent::FocusChange(false)) { diff --git a/masonry/src/passes/layout.rs b/masonry/src/passes/layout.rs index 00f78fb7a..218d031c4 100644 --- a/masonry/src/passes/layout.rs +++ b/masonry/src/passes/layout.rs @@ -161,22 +161,6 @@ pub(crate) fn run_layout_on( child_state.id, ); } - - // TODO - This check might be redundant with the code updating local_paint_rect - let child_rect = child_state.paint_rect(); - if !state.item.local_paint_rect.contains_rect(child_rect) - && state.item.clip_path.is_none() - { - debug_panic!( - "Error in '{}' {}: paint_rect {:?} doesn't contain paint_rect {:?} of child widget '{}' {}", - name, - pod.id(), - state.item.local_paint_rect, - child_rect, - child_state.widget_name, - child_state.id, - ); - } } let new_children_ids = widget.item.children_ids(); @@ -198,16 +182,6 @@ pub(crate) fn run_layout_on( } } - // TODO - Figure out how to deal with the overflow problem, eg: - // What happens if a widget returns a size larger than the allowed constraints? - // Some possibilities are: - // - Always clip: might be expensive - // - Display it anyway: might lead to graphical bugs - // - Panic: too harsh? - // Also, we need to avoid spurious crashes when we initialize the app and the - // size is (0,0) - // See https://github.com/linebender/xilem/issues/377 - let state_mut = parent_ctx.widget_state_children.get_child_mut(id).unwrap(); parent_ctx.widget_state.merge_up(state_mut.item); state_mut.item.size = new_size; diff --git a/masonry/src/passes/mutate.rs b/masonry/src/passes/mutate.rs index 18bc8896a..eb7285da5 100644 --- a/masonry/src/passes/mutate.rs +++ b/masonry/src/passes/mutate.rs @@ -16,8 +16,8 @@ pub(crate) fn mutate_widget( let (widget_mut, state_mut) = root.widget_arena.get_pair_mut(id); let _span = info_span!("mutate_widget", name = widget_mut.item.short_type_name()).entered(); - // NOTE - parent_widget_state can be None here, because the loop below will merge the - // state up to the root. + // NOTE - we can set parent_widget_state to None here, because the loop below will merge the + // states up to the root. let root_widget = WidgetMut { ctx: MutateCtx { global_state: &mut root.global_state, diff --git a/masonry/src/passes/paint.rs b/masonry/src/passes/paint.rs index fb059b44c..2d1125f69 100644 --- a/masonry/src/passes/paint.rs +++ b/masonry/src/passes/paint.rs @@ -28,7 +28,8 @@ fn paint_widget( let id = state.item.id; - // TODO - Handle invalidation regions + // TODO - Handle damage regions + // https://github.com/linebender/xilem/issues/789 let mut ctx = PaintCtx { global_state, widget_state: state.item, @@ -71,7 +72,7 @@ fn paint_widget( state.children, |widget, mut state| { // TODO - We skip painting stashed items. - // This may have knock-on effects we'd need to document. + // This may lead to zombie flags in rare cases, we need to fix this. if state.item.is_stashed { return; } diff --git a/masonry/src/passes/update.rs b/masonry/src/passes/update.rs index e22f7ae42..79d6111c5 100644 --- a/masonry/src/passes/update.rs +++ b/masonry/src/passes/update.rs @@ -74,7 +74,7 @@ fn run_single_update_pass( } } -// --- MARK: UPDATE TREE --- +// --- MARK: TREE --- fn update_widget_tree( global_state: &mut RenderRootState, mut widget: ArenaMut<'_, Box>, @@ -188,7 +188,7 @@ pub(crate) fn run_update_widget_tree_pass(root: &mut RenderRoot) { // ---------------- -// --- MARK: UPDATE DISABLED --- +// --- MARK: DISABLED --- fn update_disabled_for_widget( global_state: &mut RenderRootState, mut widget: ArenaMut<'_, Box>, @@ -214,7 +214,7 @@ fn update_disabled_for_widget( .item .update(&mut ctx, &Update::DisabledChanged(disabled)); state.item.is_disabled = disabled; - state.item.update_focus_chain = true; + state.item.needs_update_focus_chain = true; state.item.request_accessibility = true; state.item.needs_accessibility = true; } @@ -252,7 +252,7 @@ pub(crate) fn run_update_disabled_pass(root: &mut RenderRoot) { // The stereotypical use case would be the contents of hidden tabs in a "tab group" widget. // Scrolled-out widgets are *not* stashed. -// --- MARK: UPDATE STASHED --- +// --- MARK: STASHED --- fn update_stashed_for_widget( global_state: &mut RenderRootState, mut widget: ArenaMut<'_, Box>, @@ -278,7 +278,7 @@ fn update_stashed_for_widget( .item .update(&mut ctx, &Update::StashedChanged(stashed)); state.item.is_stashed = stashed; - state.item.update_focus_chain = true; + state.item.needs_update_focus_chain = true; // Note: We don't need request_repaint because stashing doesn't actually change // how widgets are painted, only how the Scenes they create are composed. state.item.needs_paint = true; @@ -315,7 +315,7 @@ pub(crate) fn run_update_stashed_pass(root: &mut RenderRoot) { // ---------------- -// --- MARK: UPDATE FOCUS CHAIN --- +// --- MARK: FOCUS CHAIN --- // TODO https://github.com/linebender/xilem/issues/376 - Some implicit invariants: // - A widget only receives BuildFocusChain if none of its parents are hidden. @@ -332,7 +332,7 @@ fn update_focus_chain_for_widget( let _span = enter_span(global_state, widget.reborrow(), state.reborrow()); let id = state.item.id; - if !state.item.update_focus_chain { + if !state.item.needs_update_focus_chain { return; } @@ -344,7 +344,7 @@ fn update_focus_chain_for_widget( if state.item.accepts_focus { state.item.focus_chain.push(id); } - state.item.update_focus_chain = false; + state.item.needs_update_focus_chain = false; let parent_state = &mut *state.item; recurse_on_children( @@ -391,7 +391,7 @@ pub(crate) fn run_update_focus_chain_pass(root: &mut RenderRoot) { // ---------------- -// --- MARK: UPDATE FOCUS --- +// --- MARK: FOCUS --- pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) { let _span = info_span!("update_focus").entered(); // If the next-focused widget is disabled, stashed or removed, we set @@ -448,6 +448,7 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) { // We don't just compare `prev_focused` and `next_focused` they could be the same widget // but one of their ancestors could have been reparented. + // (assuming we ever implement reparenting) if prev_focused_path != next_focused_path { let mut focused_set = HashSet::new(); for widget_id in &next_focused_path { @@ -534,7 +535,7 @@ pub(crate) fn run_update_focus_pass(root: &mut RenderRoot) { // ---------------- -// --- MARK: UPDATE SCROLL --- +// --- MARK: SCROLL --- // This pass will update scroll positions in cases where a widget has requested to be // scrolled into view (usually a textbox getting text events). // Each parent that implements scrolling will update its scroll position to ensure the @@ -547,6 +548,7 @@ pub(crate) fn run_update_scroll_pass(root: &mut RenderRoot) { for (target, rect) in scroll_request_targets { let mut target_rect = rect; + // TODO - Run top-down instead of bottom-up. run_targeted_update_pass(root, Some(target), |widget, ctx| { let event = Update::RequestPanToChild(rect); widget.update(ctx, &event); @@ -563,7 +565,7 @@ pub(crate) fn run_update_scroll_pass(root: &mut RenderRoot) { // ---------------- -// --- MARK: UPDATE POINTER --- +// --- MARK: POINTER --- pub(crate) fn run_update_pointer_pass(root: &mut RenderRoot) { if !root.global_state.needs_pointer_pass { return; @@ -604,6 +606,7 @@ pub(crate) fn run_update_pointer_pass(root: &mut RenderRoot) { // We don't just compare `prev_focused` and `next_focused` they could be the same widget // but one of their ancestors could have been reparented. + // (assuming we ever implement reparenting) if prev_hovered_path != next_hovered_path { let mut hovered_set = HashSet::new(); for widget_id in &next_hovered_path { @@ -655,17 +658,17 @@ pub(crate) fn run_update_pointer_pass(root: &mut RenderRoot) { } } - // -- UPDATE CURSOR -- + // -- UPDATE CURSOR ICON -- - // If the pointer is captured, its cursor always reflects the + // If the pointer is captured, its icon always reflects the // capture target, even when not hovered. - let cursor_source = root + let icon_source = root .global_state .pointer_capture_target .or(next_hovered_widget); - let new_cursor = if let (Some(cursor_source), Some(pos)) = (cursor_source, pointer_pos) { - let (widget, state) = root.widget_arena.get_pair(cursor_source); + let new_icon = if let (Some(icon_source), Some(pos)) = (icon_source, pointer_pos) { + let (widget, state) = root.widget_arena.get_pair(icon_source); let ctx = QueryCtx { global_state: &root.global_state, @@ -683,11 +686,11 @@ pub(crate) fn run_update_pointer_pass(root: &mut RenderRoot) { CursorIcon::Default }; - if root.global_state.cursor_icon != new_cursor { + if root.global_state.cursor_icon != new_icon { root.global_state - .emit_signal(RenderRootSignal::SetCursor(new_cursor)); + .emit_signal(RenderRootSignal::SetCursor(new_icon)); } - root.global_state.cursor_icon = new_cursor; + root.global_state.cursor_icon = new_icon; root.global_state.hovered_path = next_hovered_path; } diff --git a/masonry/src/testing/helper_widgets.rs b/masonry/src/testing/helper_widgets.rs index cf69eb506..b773f0a17 100644 --- a/masonry/src/testing/helper_widgets.rs +++ b/masonry/src/testing/helper_widgets.rs @@ -16,15 +16,14 @@ use accesskit::{Node, Role}; use smallvec::SmallVec; use tracing::trace_span; use vello::Scene; -use widget::widget::get_child_at_pos; -use widget::WidgetRef; use crate::event::{PointerEvent, TextEvent}; -use crate::widget::SizedBox; - -// TODO: Expect doesn't work here -#[allow(clippy::wildcard_imports, reason = "Deferred: Noisy")] -use crate::*; +use crate::widget::widget::get_child_at_pos; +use crate::widget::{SizedBox, WidgetRef}; +use crate::{ + AccessCtx, AccessEvent, AsAny, BoxConstraints, ComposeCtx, CursorIcon, EventCtx, LayoutCtx, + PaintCtx, Point, QueryCtx, RegisterCtx, Size, Update, UpdateCtx, Widget, WidgetId, WidgetPod, +}; pub type PointerEventFn = dyn FnMut(&mut S, &mut EventCtx, &PointerEvent); pub type TextEventFn = dyn FnMut(&mut S, &mut EventCtx, &TextEvent); @@ -284,13 +283,13 @@ impl ModularWidget { #[warn(clippy::missing_trait_methods)] impl Widget for ModularWidget { - fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &event::PointerEvent) { + fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &PointerEvent) { if let Some(f) = self.on_pointer_event.as_mut() { f(&mut self.state, ctx, event); } } - fn on_text_event(&mut self, ctx: &mut EventCtx, event: &event::TextEvent) { + fn on_text_event(&mut self, ctx: &mut EventCtx, event: &TextEvent) { if let Some(f) = self.on_text_event.as_mut() { f(&mut self.state, ctx, event); } @@ -441,9 +440,9 @@ impl Widget for ReplaceChild { self.child.on_event(ctx, event) } - fn on_pointer_event(&mut self, _ctx: &mut EventCtx, _event: &event::PointerEvent) {} + fn on_pointer_event(&mut self, _ctx: &mut EventCtx, _event: &PointerEvent) {} - fn on_text_event(&mut self, _ctx: &mut EventCtx, _event: &event::TextEvent) {} + fn on_text_event(&mut self, _ctx: &mut EventCtx, _event: &TextEvent) {} fn on_access_event(&mut self, _ctx: &mut EventCtx, _event: &AccessEvent) {} @@ -507,12 +506,12 @@ impl Recording { #[warn(clippy::missing_trait_methods)] impl Widget for Recorder { - fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &event::PointerEvent) { + fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &PointerEvent) { self.recording.push(Record::PE(event.clone())); self.child.on_pointer_event(ctx, event); } - fn on_text_event(&mut self, ctx: &mut EventCtx, event: &event::TextEvent) { + fn on_text_event(&mut self, ctx: &mut EventCtx, event: &TextEvent) { self.recording.push(Record::TE(event.clone())); self.child.on_text_event(ctx, event); } diff --git a/masonry/src/text/mod.rs b/masonry/src/text/mod.rs index 8ef98ed4c..9350858a5 100644 --- a/masonry/src/text/mod.rs +++ b/masonry/src/text/mod.rs @@ -4,7 +4,7 @@ //! Support for text display and rendering //! //! There are three kinds of text commonly needed: -//! 1) Entirely display text (e.g. a button) +//! 1) Non interactive text (e.g. a button's label) //! 2) Selectable text (e.g. a paragraph of content) //! 3) Editable text (e.g. a search bar) //! diff --git a/masonry/src/widget/align.rs b/masonry/src/widget/align.rs index 4c91fbfe7..fd354a1b5 100644 --- a/masonry/src/widget/align.rs +++ b/masonry/src/widget/align.rs @@ -163,7 +163,6 @@ fn log_size_warnings(size: Size) { } } -// --- MARK: TESTS --- // --- MARK: TESTS --- #[cfg(test)] mod tests { diff --git a/masonry/src/widget/button.rs b/masonry/src/widget/button.rs index 551727e09..8f88abd1f 100644 --- a/masonry/src/widget/button.rs +++ b/masonry/src/widget/button.rs @@ -18,7 +18,7 @@ use crate::{ PointerEvent, QueryCtx, Size, TextEvent, Update, UpdateCtx, Widget, WidgetId, }; -// the minimum padding added to a button. +// The minimum padding added to a button. // NOTE: these values are chosen to match the existing look of TextBox; these // should be reevaluated at some point. const LABEL_INSETS: Insets = Insets::uniform_xy(8., 2.); @@ -207,12 +207,6 @@ impl Widget for Button { fn make_trace_span(&self, ctx: &QueryCtx<'_>) -> Span { trace_span!("Button", id = ctx.widget_id().trace()) } - - // FIXME - #[cfg(FALSE)] - fn get_debug_text(&self) -> Option { - Some(self.label.as_ref().text().as_ref().to_string()) - } } // --- MARK: TESTS --- diff --git a/masonry/src/widget/flex.rs b/masonry/src/widget/flex.rs index 98e463505..3ed9aed0d 100644 --- a/masonry/src/widget/flex.rs +++ b/masonry/src/widget/flex.rs @@ -32,10 +32,9 @@ pub struct Flex { /// Optional parameters for an item in a [`Flex`] container (row or column). /// /// Generally, when you would like to add a flexible child to a container, -/// you can simply call [`with_flex_child`](Flex::with_flex_child), passing the -/// child and the desired flex factor as a `f64`, which has an impl of +/// you can simply call [`with_flex_child`](Flex::with_flex_child) or [`add_flex_child`](Flex::add_flex_child), +/// passing the child and the desired flex factor as a `f64`, which has an impl of /// `Into`. -// FIXME - "with_flex_child or [`add_flex_child`](FlexMut::add_flex_child)" #[derive(Default, Debug, Copy, Clone, PartialEq)] pub struct FlexParams { flex: Option, @@ -540,7 +539,6 @@ impl Flex { this.ctx.request_layout(); } - // FIXME - Remove Box pub fn child_mut<'t>( this: &'t mut WidgetMut<'_, Self>, idx: usize, diff --git a/masonry/src/widget/scroll_bar.rs b/masonry/src/widget/scroll_bar.rs index 6475051e4..0e7f96b4e 100644 --- a/masonry/src/widget/scroll_bar.rs +++ b/masonry/src/widget/scroll_bar.rs @@ -17,18 +17,13 @@ use crate::{ WidgetId, }; -// RULES -// - - -// TODO - Document names: -// - grabbybar -// - empty_space -// - _z -// - _length - -// TODO - Fade scrollbars? Find out how Linux/MacOS/Windows do it -// TODO - Rename cursor to oval/rect/bar/grabber/grabbybar -// TODO - Rename progress to ??? +// TODO +// - Fade scrollbars? Find out how Linux/MacOS/Windows do it +// - Rename cursor to oval/rect/bar/grabber/grabbybar +// - Rename progress to something more descriptive +// - Document names +// - Document invariants + pub struct ScrollBar { axis: Axis, pub(crate) cursor_progress: f64, @@ -287,41 +282,7 @@ mod tests { assert_render_snapshot!(harness, "scrollbar_horizontal_middle"); } - // TODO - portal larger than content - - #[cfg(FALSE)] - #[test] - fn edit_button() { - let image_1 = { - let button = Button::from_label( - Label::new("The quick brown fox jumps over the lazy dog") - .with_text_color(PRIMARY_LIGHT) - .with_text_size(20.0), - ); - - let mut harness = TestHarness::create_with_size(button, Size::new(50.0, 50.0)); - - harness.render() - }; - - let image_2 = { - let button = Button::new("Hello world"); + // TODO - Add "portal larger than content" test - let mut harness = TestHarness::create_with_size(button, Size::new(50.0, 50.0)); - - harness.edit_root_widget(|mut button, _| { - let mut button = button.downcast::