From 8135172f754d5635ced50529626493c90463d356 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Sat, 17 Aug 2024 11:44:31 +0200 Subject: [PATCH 01/21] Implement initial paint pass --- masonry/src/contexts.rs | 2 +- masonry/src/passes/mod.rs | 1 + masonry/src/passes/paint.rs | 168 +++++++++++++++++++++++ masonry/src/render_root.rs | 39 +----- masonry/src/widget/tests/safety_rails.rs | 38 ----- masonry/src/widget/widget_arena.rs | 20 +++ masonry/src/widget/widget_pod.rs | 75 +--------- masonry/src/widget/widget_state.rs | 3 + 8 files changed, 199 insertions(+), 147 deletions(-) create mode 100644 masonry/src/passes/paint.rs diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index 24deaee52..3acc23aa7 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -101,7 +101,6 @@ pub struct PaintCtx<'a> { /// The approximate depth in the tree at the time of painting. pub(crate) depth: u32, pub(crate) debug_paint: bool, - pub(crate) debug_widget: bool, } pub struct AccessCtx<'a> { @@ -405,6 +404,7 @@ impl_context_method!(MutateCtx<'_>, EventCtx<'_>, LifeCycleCtx<'_>, { /// Request a [`paint`](crate::Widget::paint) pass. pub fn request_paint(&mut self) { trace!("request_paint"); + self.widget_state.request_paint = true; self.widget_state.needs_paint = true; } diff --git a/masonry/src/passes/mod.rs b/masonry/src/passes/mod.rs index f8ffaa6f5..9e4efb5ca 100644 --- a/masonry/src/passes/mod.rs +++ b/masonry/src/passes/mod.rs @@ -7,6 +7,7 @@ use crate::WidgetId; pub mod compose; pub mod event; pub mod mutate; +pub mod paint; pub mod update; pub(crate) fn merge_state_up(arena: &mut WidgetArena, widget_id: WidgetId) { diff --git a/masonry/src/passes/paint.rs b/masonry/src/passes/paint.rs new file mode 100644 index 000000000..6f3abb565 --- /dev/null +++ b/masonry/src/passes/paint.rs @@ -0,0 +1,168 @@ +// Copyright 2024 the Xilem Authors +// SPDX-License-Identifier: Apache-2.0 + +use std::collections::HashMap; + +use tracing::{info_span, trace}; +use vello::kurbo::{Affine, Rect, Stroke}; +use vello::peniko::BlendMode; +use vello::Scene; + +use crate::render_root::{RenderRoot, RenderRootState}; +use crate::theme::get_debug_color; +use crate::tree_arena::{ArenaMut, ArenaMutChildren}; +use crate::{PaintCtx, Widget, WidgetId, WidgetState}; + +fn recurse_on_children( + id: WidgetId, + mut widget: ArenaMut<'_, Box>, + mut state: ArenaMutChildren<'_, WidgetState>, + mut callback: impl FnMut(ArenaMut<'_, Box>, ArenaMut<'_, WidgetState>), +) { + let parent_name = widget.item.short_type_name(); + let parent_id = id; + + for child_id in widget.item.children_ids() { + let widget = widget + .children + .get_child_mut(child_id.to_raw()) + .unwrap_or_else(|| { + panic!( + "Error in '{}' #{}: cannot find child #{} returned by children_ids()", + parent_name, + parent_id.to_raw(), + child_id.to_raw() + ) + }); + let state = state.get_child_mut(child_id.to_raw()).unwrap_or_else(|| { + panic!( + "Error in '{}' #{}: cannot find child #{} returned by children_ids()", + parent_name, + parent_id.to_raw(), + child_id.to_raw() + ) + }); + + callback(widget, state); + } +} + +fn paint_widget( + global_state: &mut RenderRootState, + complete_scene: &mut Scene, + scenes: &mut HashMap, + mut widget: ArenaMut<'_, Box>, + mut state: ArenaMut<'_, WidgetState>, + depth: u32, + debug_paint: bool, +) { + let _span = widget.item.make_trace_span().entered(); + let id = state.item.id; + + // TODO - Handle invalidation regions + let mut ctx = PaintCtx { + global_state, + widget_state: state.item, + widget_state_children: state.children.reborrow_mut(), + widget_children: widget.children.reborrow_mut(), + depth, + debug_paint, + }; + if ctx.widget_state.request_paint { + trace!( + "Painting widget '{}' #{}", + widget.item.short_type_name(), + id.to_raw(), + ); + + // TODO - Reserve (see comment below) + let mut scene = Scene::new(); + widget.item.paint(&mut ctx, &mut scene); + *scenes.entry(id).or_default() = scene; + } + + state.item.request_paint = false; + state.item.needs_paint = false; + + // TODO + let clip: Option = None; + let has_clip = clip.is_some(); + let transform = Affine::translate(dbg!(state.item.window_origin).to_vec2()); + let scene = scenes.get(&id).unwrap(); + + if let Some(clip) = clip { + complete_scene.push_layer(BlendMode::default(), 1., transform, &clip); + } + + complete_scene.append(scene, Some(transform)); + + let id = state.item.id; + let size = state.item.size; + let parent_state = state.item; + recurse_on_children( + id, + widget.reborrow_mut(), + state.children, + |widget, mut state| { + paint_widget( + global_state, + complete_scene, + scenes, + widget, + state.reborrow_mut(), + depth + 1, + debug_paint, + ); + parent_state.merge_up(state.item); + }, + ); + + if debug_paint { + const BORDER_WIDTH: f64 = 1.0; + let rect = size.to_rect().inset(BORDER_WIDTH / -2.0); + let color = get_debug_color(id.to_raw()); + complete_scene.stroke(&Stroke::new(BORDER_WIDTH), transform, color, None, &rect); + } + + if has_clip { + complete_scene.pop_layer(); + } +} + +// ---------------- + +pub fn root_paint(root: &mut RenderRoot) -> Scene { + let _span = info_span!("paint").entered(); + + let debug_paint = std::env::var("MASONRY_DEBUG_PAINT").is_ok_and(|it| !it.is_empty()); + + // TODO - Reserve + let mut complete_scene = Scene::new(); + + let (root_widget, root_state) = { + let widget_id = root.root.id(); + let widget = root + .widget_arena + .widgets + .find_mut(widget_id.to_raw()) + .expect("root_paint: root not in widget tree"); + let state = root + .widget_arena + .widget_states + .find_mut(widget_id.to_raw()) + .expect("root_paint: root state not in widget tree"); + (widget, state) + }; + + paint_widget( + &mut root.state, + &mut complete_scene, + &mut root.widget_arena.scenes, + root_widget, + root_state, + 0, + debug_paint, + ); + + complete_scene +} diff --git a/masonry/src/render_root.rs b/masonry/src/render_root.rs index 891295630..0b5c42745 100644 --- a/masonry/src/render_root.rs +++ b/masonry/src/render_root.rs @@ -1,7 +1,7 @@ // Copyright 2019 the Xilem Authors and the Druid Authors // SPDX-License-Identifier: Apache-2.0 -use std::collections::VecDeque; +use std::collections::{HashMap, VecDeque}; use accesskit::{ActionRequest, NodeBuilder, Tree, TreeUpdate}; use parley::fontique::{self, Collection, CollectionOptions}; @@ -23,6 +23,7 @@ use crate::event::{PointerEvent, TextEvent, WindowEvent}; use crate::passes::compose::root_compose; use crate::passes::event::{root_on_access_event, root_on_pointer_event, root_on_text_event}; use crate::passes::mutate::{mutate_widget, run_mutate_pass}; +use crate::passes::paint::root_paint; use crate::passes::update::run_update_pointer_pass; use crate::text::TextBrush; use crate::tree_arena::TreeArena; @@ -146,6 +147,7 @@ impl RenderRoot { widget_arena: WidgetArena { widgets: TreeArena::new(), widget_states: TreeArena::new(), + scenes: HashMap::new(), }, rebuild_access_tree: true, }; @@ -505,40 +507,7 @@ impl RenderRoot { // --- MARK: PAINT --- fn root_paint(&mut self) -> Scene { - // TODO - Handle Xilem's VIEW_CONTEXT_CHANGED - - let mut dummy_state = WidgetState::synthetic(self.root.id(), self.get_kurbo_size()); - let root_state_token = self.widget_arena.widget_states.root_token_mut(); - let root_widget_token = self.widget_arena.widgets.root_token_mut(); - let mut ctx = PaintCtx { - global_state: &mut self.state, - widget_state: &mut dummy_state, - widget_state_children: root_state_token, - widget_children: root_widget_token, - depth: 0, - debug_paint: false, - debug_widget: false, - }; - - let mut scene = Scene::new(); - { - let _span = info_span!("paint").entered(); - self.root.paint(&mut ctx, &mut scene); - } - - // FIXME - This is a workaround to Vello panicking when given an - // empty scene - // See https://github.com/linebender/vello/issues/291 - let empty_path = kurbo::Rect::ZERO; - scene.fill( - Fill::NonZero, - Affine::IDENTITY, - Color::TRANSPARENT, - None, - &empty_path, - ); - - scene + root_paint(self) } // --- MARK: ACCESSIBILITY --- diff --git a/masonry/src/widget/tests/safety_rails.rs b/masonry/src/widget/tests/safety_rails.rs index 6cca146f6..2327035e9 100644 --- a/masonry/src/widget/tests/safety_rails.rs +++ b/masonry/src/widget/tests/safety_rails.rs @@ -94,21 +94,6 @@ fn check_forget_to_call_place_child() { let _harness = TestHarness::create(widget); } -#[should_panic(expected = "not visited in method paint")] -#[test] -#[cfg_attr( - not(debug_assertions), - ignore = "This test doesn't work without debug assertions (i.e. in release mode). See https://github.com/linebender/xilem/issues/477" -)] -fn check_forget_to_recurse_paint() { - let widget = make_parent_widget(Flex::row()).paint_fn(|_child, _ctx, _scene| { - // We forget to call child.paint(); - }); - - let mut harness = TestHarness::create(widget); - harness.render(); -} - // --- // TODO - allow non-recurse in some cases @@ -338,29 +323,6 @@ fn check_layout_stashed() { harness.mouse_move(Point::ZERO); } -#[should_panic(expected = "trying to paint stashed widget")] -#[test] -fn check_paint_stashed() { - let widget = make_parent_widget(Flex::row()) - .lifecycle_fn(|child, ctx, event| { - child.lifecycle(ctx, event); - if matches!( - event, - LifeCycle::Internal(InternalLifeCycle::RouteWidgetAdded) - ) { - ctx.set_stashed(child, true); - } - }) - .layout_fn(|_child, _ctx, _bc| Size::ZERO) - .paint_fn(|child, ctx, scene| { - child.paint(ctx, scene); - }); - - let mut harness = TestHarness::create(widget); - harness.mouse_move(Point::ZERO); - harness.render(); -} - // --- // TODO - For now, paint_rect is automatically computed, so there's no way this test fails. diff --git a/masonry/src/widget/widget_arena.rs b/masonry/src/widget/widget_arena.rs index b26cd446b..ad7b2f7b4 100644 --- a/masonry/src/widget/widget_arena.rs +++ b/masonry/src/widget/widget_arena.rs @@ -1,13 +1,19 @@ // Copyright 2024 the Xilem Authors // SPDX-License-Identifier: Apache-2.0 +use std::collections::HashMap; + +use vello::Scene; + use crate::tree_arena::{ArenaMut, ArenaRef, TreeArena}; use crate::widget::WidgetRef; use crate::{Widget, WidgetId, WidgetState}; pub(crate) struct WidgetArena { pub(crate) widgets: TreeArena>, + // TODO - Rename to "states" pub(crate) widget_states: TreeArena, + pub(crate) scenes: HashMap, } #[allow(dead_code)] @@ -87,6 +93,20 @@ impl WidgetArena { .expect("get_state_mut: widget state not in widget tree") } + #[track_caller] + pub(crate) fn get_scene(&mut self, widget_id: WidgetId) -> &Scene { + self.scenes + .get(&widget_id) + .expect("get_scene: scene not in widget tree") + } + + #[track_caller] + pub(crate) fn get_scene_mut(&mut self, widget_id: WidgetId) -> &mut Scene { + self.scenes + .get_mut(&widget_id) + .expect("get_scene_mut: scene not in widget tree") + } + pub fn try_get_widget_ref(&self, id: WidgetId) -> Option> { let state_ref = self.widget_states.find(id.to_raw())?; let widget_ref = self diff --git a/masonry/src/widget/widget_pod.rs b/masonry/src/widget/widget_pod.rs index e022ad9e6..c99549615 100644 --- a/masonry/src/widget/widget_pod.rs +++ b/masonry/src/widget/widget_pod.rs @@ -649,79 +649,8 @@ impl WidgetPod { // --- MARK: PAINT --- - /// Paint the widget, translating it by the origin of its layout rectangle. - /// - /// This will recursively paint widgets, stopping if a widget's layout - /// rect is outside of the currently visible region. - pub fn paint(&mut self, parent_ctx: &mut PaintCtx, scene: &mut Scene) { - self.call_widget_method_with_checks( - "paint", - parent_ctx, - |ctx| { - ( - ctx.widget_state_children.reborrow(), - ctx.widget_children.reborrow(), - ) - }, - |self2, parent_ctx| self2.paint_inner(parent_ctx, scene), - ); - } - - fn paint_inner(&mut self, parent_ctx: &mut PaintCtx, scene: &mut Scene) -> bool { - let id = self.id().to_raw(); - let widget_mut = parent_ctx - .widget_children - .get_child_mut(id) - .expect("WidgetPod: inner widget not found in widget tree"); - let state_mut = parent_ctx - .widget_state_children - .get_child_mut(id) - .expect("WidgetPod: inner widget not found in widget tree"); - let widget = widget_mut.item; - let state = state_mut.item; - - if state.is_stashed { - debug_panic!( - "Error in '{}' #{}: trying to paint stashed widget.", - widget.short_type_name(), - self.id().to_raw(), - ); - return false; - } - - let call_widget = state.needs_paint; - if call_widget { - trace!( - "Painting widget '{}' #{}", - widget.short_type_name(), - self.id().to_raw() - ); - state.needs_paint = false; - - // TODO - Handle invalidation regions - let mut inner_ctx = PaintCtx { - global_state: parent_ctx.global_state, - widget_state: state, - widget_state_children: state_mut.children, - widget_children: widget_mut.children, - depth: parent_ctx.depth + 1, - debug_paint: parent_ctx.debug_paint, - debug_widget: parent_ctx.debug_widget, - }; - - self.fragment.reset(); - widget.paint(&mut inner_ctx, &mut self.fragment); - - if parent_ctx.debug_paint { - self.debug_paint_layout_bounds(state.size); - } - } - - let transform = Affine::translate(state.origin.to_vec2()); - scene.append(&self.fragment, Some(transform)); - - call_widget - } + // TODO - remove + pub fn paint(&mut self, _parent_ctx: &mut PaintCtx, _scene: &mut Scene) {} fn debug_paint_layout_bounds(&mut self, size: Size) { const BORDER_WIDTH: f64 = 1.0; diff --git a/masonry/src/widget/widget_state.rs b/masonry/src/widget/widget_state.rs index 3a1e614ae..dee5bf205 100644 --- a/masonry/src/widget/widget_state.rs +++ b/masonry/src/widget/widget_state.rs @@ -88,6 +88,7 @@ pub struct WidgetState { pub(crate) needs_compose: bool, pub(crate) needs_paint: bool, pub(crate) needs_accessibility_update: bool, + pub(crate) request_paint: bool, /// Any descendant has requested an animation frame. pub(crate) request_anim: bool, @@ -162,6 +163,7 @@ impl WidgetState { needs_compose: true, needs_paint: true, needs_accessibility_update: true, + request_paint: true, has_focus: false, request_anim: true, request_accessibility_update: true, @@ -191,6 +193,7 @@ impl WidgetState { needs_accessibility_update: false, needs_compose: false, request_compose: false, + request_paint: false, request_accessibility_update: false, request_anim: false, children_changed: false, From 697576521fe08dfe05a19ec030bb5f577a73ec71 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Sat, 17 Aug 2024 11:46:47 +0200 Subject: [PATCH 02/21] TMP --- masonry/src/passes/accessibility.rs | 1 + masonry/src/passes/mod.rs | 1 + 2 files changed, 2 insertions(+) create mode 100644 masonry/src/passes/accessibility.rs diff --git a/masonry/src/passes/accessibility.rs b/masonry/src/passes/accessibility.rs new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/masonry/src/passes/accessibility.rs @@ -0,0 +1 @@ + diff --git a/masonry/src/passes/mod.rs b/masonry/src/passes/mod.rs index 9e4efb5ca..58350213d 100644 --- a/masonry/src/passes/mod.rs +++ b/masonry/src/passes/mod.rs @@ -4,6 +4,7 @@ use crate::widget::WidgetArena; use crate::WidgetId; +pub mod accessibility; pub mod compose; pub mod event; pub mod mutate; From 2a433d1b0ba743312fe6a6d8d824d840e46fb15c Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Sat, 17 Aug 2024 12:29:14 +0200 Subject: [PATCH 03/21] Implement accessibility pass --- masonry/src/contexts.rs | 10 -- masonry/src/passes/accessibility.rs | 191 ++++++++++++++++++++++++++++ masonry/src/render_root.rs | 56 ++------ masonry/src/widget/widget_pod.rs | 129 +------------------ masonry/src/widget/widget_state.rs | 2 +- 5 files changed, 210 insertions(+), 178 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index 3acc23aa7..8698fec9e 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -948,16 +948,6 @@ impl AccessCtx<'_> { pub fn current_node(&mut self) -> &mut NodeBuilder { &mut self.current_node } - - /// Report whether accessibility was requested on this widget. - /// - /// This method is primarily intended for containers. The `accessibility` - /// method will be called on a widget when it or any of its descendants - /// have seen a request. However, in many cases a container need not push - /// a node for itself. - pub fn is_requested(&self) -> bool { - self.widget_state.needs_accessibility_update - } } // --- MARK: RAW WRAPPERS --- diff --git a/masonry/src/passes/accessibility.rs b/masonry/src/passes/accessibility.rs index 8b1378917..b974b87e6 100644 --- a/masonry/src/passes/accessibility.rs +++ b/masonry/src/passes/accessibility.rs @@ -1 +1,192 @@ +// Copyright 2024 the Xilem Authors +// SPDX-License-Identifier: Apache-2.0 +use crate::render_root::RenderRoot; +use accesskit::{NodeBuilder, NodeId, TreeUpdate}; +use tracing::debug; +use tracing::info_span; +use tracing::trace; +use vello::kurbo::Rect; + +use crate::render_root::RenderRootState; +use crate::tree_arena::{ArenaMut, ArenaMutChildren}; +use crate::{AccessCtx, Widget, WidgetId, WidgetState}; + +fn recurse_on_children( + id: WidgetId, + mut widget: ArenaMut<'_, Box>, + mut state: ArenaMutChildren<'_, WidgetState>, + mut callback: impl FnMut(ArenaMut<'_, Box>, ArenaMut<'_, WidgetState>), +) { + let parent_name = widget.item.short_type_name(); + let parent_id = id; + + for child_id in widget.item.children_ids() { + let widget = widget + .children + .get_child_mut(child_id.to_raw()) + .unwrap_or_else(|| { + panic!( + "Error in '{}' #{}: cannot find child #{} returned by children_ids()", + parent_name, + parent_id.to_raw(), + child_id.to_raw() + ) + }); + let state = state.get_child_mut(child_id.to_raw()).unwrap_or_else(|| { + panic!( + "Error in '{}' #{}: cannot find child #{} returned by children_ids()", + parent_name, + parent_id.to_raw(), + child_id.to_raw() + ) + }); + + callback(widget, state); + } +} + +fn build_accessibility_tree( + global_state: &mut RenderRootState, + tree_update: &mut TreeUpdate, + mut widget: ArenaMut<'_, Box>, + mut state: ArenaMut<'_, WidgetState>, + rebuild_all: bool, + scale_factor: f64, +) { + let _span = widget.item.make_trace_span().entered(); + let id = state.item.id; + + if !rebuild_all && !state.item.needs_accessibility_update { + return; + } + + if rebuild_all || state.item.request_accessibility_update { + trace!( + "Building accessibility node for widget '{}' #{}", + widget.item.short_type_name(), + id.to_raw(), + ); + let current_node = build_access_node(widget.item, state.item, scale_factor); + + let mut ctx = AccessCtx { + global_state, + widget_state: state.item, + widget_state_children: state.children.reborrow_mut(), + widget_children: widget.children.reborrow_mut(), + tree_update, + current_node, + rebuild_all, + scale_factor, + }; + widget.item.accessibility(&mut ctx); + + let id: NodeId = ctx.widget_state.id.into(); + trace!( + "Built node #{} with role={:?}, default_action={:?}", + id.0, + ctx.current_node.role(), + ctx.current_node.default_action_verb(), + ); + ctx.tree_update.nodes.push((id, ctx.current_node.build())); + } + + state.item.request_accessibility_update = false; + state.item.needs_accessibility_update = false; + + let id = state.item.id; + let parent_state = state.item; + recurse_on_children( + id, + widget.reborrow_mut(), + state.children, + |widget, mut state| { + build_accessibility_tree( + global_state, + tree_update, + widget, + state.reborrow_mut(), + rebuild_all, + scale_factor, + ); + parent_state.merge_up(state.item); + }, + ); +} + +fn build_access_node(widget: &dyn Widget, state: &WidgetState, scale_factor: f64) -> NodeBuilder { + let mut node = NodeBuilder::new(widget.accessibility_role()); + node.set_bounds(to_accesskit_rect(state.window_layout_rect(), scale_factor)); + + node.set_children( + widget + .children_ids() + .iter() + .copied() + .map(|id| id.into()) + .collect::>(), + ); + + if state.is_hot { + node.set_hovered(); + } + if state.is_disabled() { + node.set_disabled(); + } + if state.is_stashed { + node.set_hidden(); + } + + node +} + +fn to_accesskit_rect(r: Rect, scale_factor: f64) -> accesskit::Rect { + let s = scale_factor; + accesskit::Rect::new(s * r.x0, s * r.y0, s * r.x1, s * r.y1) +} + +// ---------------- + +pub(crate) fn root_accessibility( + root: &mut RenderRoot, + rebuild_all: bool, + scale_factor: f64, +) -> TreeUpdate { + let _span = info_span!("accessibility").entered(); + + let mut tree_update = TreeUpdate { + nodes: vec![], + tree: None, + focus: root.state.focused_widget.unwrap_or(root.root.id()).into(), + }; + + let (root_widget, root_state) = { + let widget_id = root.root.id(); + let widget = root + .widget_arena + .widgets + .find_mut(widget_id.to_raw()) + .expect("root_accessibility: root not in widget tree"); + let state = root + .widget_arena + .widget_states + .find_mut(widget_id.to_raw()) + .expect("root_accessibility: root state not in widget tree"); + (widget, state) + }; + + if rebuild_all { + debug!("Running ACCESSIBILITY pass with rebuild_all"); + } + + build_accessibility_tree( + &mut root.state, + &mut tree_update, + root_widget, + root_state, + rebuild_all, + scale_factor, + ); + + tree_update +} diff --git a/masonry/src/render_root.rs b/masonry/src/render_root.rs index 0b5c42745..dc7b50232 100644 --- a/masonry/src/render_root.rs +++ b/masonry/src/render_root.rs @@ -3,12 +3,11 @@ use std::collections::{HashMap, VecDeque}; -use accesskit::{ActionRequest, NodeBuilder, Tree, TreeUpdate}; +use accesskit::{ActionRequest, Tree, TreeUpdate}; use parley::fontique::{self, Collection, CollectionOptions}; use parley::{FontContext, LayoutContext}; -use tracing::{debug, info_span, warn}; -use vello::kurbo::{self, Affine, Point}; -use vello::peniko::{Color, Fill}; +use tracing::{info_span, warn}; +use vello::kurbo::{self, Point}; use vello::Scene; #[cfg(not(target_arch = "wasm32"))] @@ -16,10 +15,11 @@ use std::time::Instant; #[cfg(target_arch = "wasm32")] use web_time::Instant; -use crate::contexts::{LayoutCtx, LifeCycleCtx, PaintCtx}; +use crate::contexts::{LayoutCtx, LifeCycleCtx}; use crate::debug_logger::DebugLogger; use crate::dpi::{LogicalPosition, LogicalSize, PhysicalSize}; use crate::event::{PointerEvent, TextEvent, WindowEvent}; +use crate::passes::accessibility::root_accessibility; use crate::passes::compose::root_compose; use crate::passes::event::{root_on_access_event, root_on_pointer_event, root_on_text_event}; use crate::passes::mutate::{mutate_widget, run_mutate_pass}; @@ -30,8 +30,8 @@ use crate::tree_arena::TreeArena; use crate::widget::WidgetArena; use crate::widget::{WidgetMut, WidgetRef, WidgetState}; use crate::{ - AccessCtx, AccessEvent, Action, BoxConstraints, CursorIcon, Handled, InternalLifeCycle, - LifeCycle, Widget, WidgetId, WidgetPod, + AccessEvent, Action, BoxConstraints, CursorIcon, Handled, InternalLifeCycle, LifeCycle, Widget, + WidgetId, WidgetPod, }; // --- MARK: STRUCTS --- @@ -513,42 +513,14 @@ impl RenderRoot { // --- MARK: ACCESSIBILITY --- // TODO - Integrate in unit tests? fn root_accessibility(&mut self) -> TreeUpdate { - let mut tree_update = TreeUpdate { - nodes: vec![], - tree: None, - focus: self.state.focused_widget.unwrap_or(self.root.id()).into(), - }; - let mut dummy_state = WidgetState::synthetic(self.root.id(), self.get_kurbo_size()); - let root_state_token = self.widget_arena.widget_states.root_token_mut(); - let root_widget_token = self.widget_arena.widgets.root_token_mut(); - let mut ctx = AccessCtx { - global_state: &mut self.state, - widget_state: &mut dummy_state, - widget_state_children: root_state_token, - widget_children: root_widget_token, - tree_update: &mut tree_update, - current_node: NodeBuilder::default(), - rebuild_all: self.rebuild_access_tree, - scale_factor: self.scale_factor, - }; - - { - let _span = info_span!("accessibility").entered(); - if self.rebuild_access_tree { - debug!("Running ACCESSIBILITY pass with rebuild_all"); - } - self.root.accessibility(&mut ctx); - self.rebuild_access_tree = false; - } + let mut tree_update = root_accessibility(self, self.rebuild_access_tree, self.scale_factor); - if true { - tree_update.tree = Some(Tree { - root: self.root.id().into(), - app_name: None, - toolkit_name: Some("Masonry".to_string()), - toolkit_version: Some(env!("CARGO_PKG_VERSION").to_string()), - }); - } + tree_update.tree = Some(Tree { + root: self.root.id().into(), + app_name: None, + toolkit_name: Some("Masonry".to_string()), + toolkit_version: Some(env!("CARGO_PKG_VERSION").to_string()), + }); tree_update } diff --git a/masonry/src/widget/widget_pod.rs b/masonry/src/widget/widget_pod.rs index c99549615..2e890d617 100644 --- a/masonry/src/widget/widget_pod.rs +++ b/masonry/src/widget/widget_pod.rs @@ -1,14 +1,10 @@ // Copyright 2018 the Xilem Authors and the Druid Authors // SPDX-License-Identifier: Apache-2.0 -use accesskit::{NodeBuilder, NodeId}; use smallvec::SmallVec; use tracing::{info_span, trace, warn}; -use vello::kurbo::{Affine, Rect, Size}; -use vello::Scene; +use vello::kurbo::{Rect, Size}; -use crate::paint_scene_helpers::stroke; -use crate::theme::get_debug_color; use crate::tree_arena::ArenaRefChildren; use crate::widget::WidgetState; use crate::{ @@ -29,7 +25,6 @@ use crate::{ pub struct WidgetPod { id: WidgetId, inner: WidgetPodInner, - pub(crate) fragment: Scene, } // TODO - This is a simple state machine that lets users create WidgetPods @@ -59,7 +54,6 @@ impl WidgetPod { WidgetPod { id, inner: WidgetPodInner::Created(inner), - fragment: Scene::new(), } } @@ -650,126 +644,11 @@ impl WidgetPod { // --- MARK: PAINT --- // TODO - remove - pub fn paint(&mut self, _parent_ctx: &mut PaintCtx, _scene: &mut Scene) {} - - fn debug_paint_layout_bounds(&mut self, size: Size) { - const BORDER_WIDTH: f64 = 1.0; - let rect = size.to_rect().inset(BORDER_WIDTH / -2.0); - let id = self.id().to_raw(); - let color = get_debug_color(id); - let scene = &mut self.fragment; - stroke(scene, &rect, color, BORDER_WIDTH); - } + pub fn paint(&mut self, _parent_ctx: &mut PaintCtx, _scene: &mut vello::Scene) {} // --- MARK: ACCESSIBILITY --- - pub fn accessibility(&mut self, parent_ctx: &mut AccessCtx) { - self.call_widget_method_with_checks( - "accessibility", - parent_ctx, - |ctx| { - ( - ctx.widget_state_children.reborrow(), - ctx.widget_children.reborrow(), - ) - }, - |self2, parent_ctx| self2.accessibility_inner(parent_ctx), - ); - } - - fn accessibility_inner(&mut self, parent_ctx: &mut AccessCtx) -> bool { - // TODO - // if state.is_stashed {} - - let id = self.id().to_raw(); - let widget_mut = parent_ctx - .widget_children - .get_child_mut(id) - .expect("WidgetPod: inner widget not found in widget tree"); - let state_mut = parent_ctx - .widget_state_children - .get_child_mut(id) - .expect("WidgetPod: inner widget not found in widget tree"); - let widget = widget_mut.item; - let state = state_mut.item; - - // If this widget or a child has requested an accessibility update, - // or if AccessKit has requested a full rebuild, - // we call the accessibility method on this widget. - let call_widget = parent_ctx.rebuild_all || state.request_accessibility_update; - if call_widget { - trace!( - "Building accessibility node for widget '{}' #{}", - widget.short_type_name(), - id, - ); - - let current_node = self.build_access_node(widget, state, parent_ctx.scale_factor); - let mut inner_ctx = AccessCtx { - global_state: parent_ctx.global_state, - widget_state: state, - widget_state_children: state_mut.children, - widget_children: widget_mut.children, - tree_update: parent_ctx.tree_update, - current_node, - rebuild_all: parent_ctx.rebuild_all, - scale_factor: parent_ctx.scale_factor, - }; - widget.accessibility(&mut inner_ctx); - - let id: NodeId = inner_ctx.widget_state.id.into(); - trace!( - "Built node #{} with role={:?}, default_action={:?}", - id.0, - inner_ctx.current_node.role(), - inner_ctx.current_node.default_action_verb(), - ); - inner_ctx - .tree_update - .nodes - .push((id, inner_ctx.current_node.build())); - } - - state.request_accessibility_update = false; - state.needs_accessibility_update = false; - - call_widget - } - - fn build_access_node( - &mut self, - widget: &dyn Widget, - state: &WidgetState, - scale_factor: f64, - ) -> NodeBuilder { - let mut node = NodeBuilder::new(widget.accessibility_role()); - node.set_bounds(to_accesskit_rect(state.window_layout_rect(), scale_factor)); - - node.set_children( - widget - .children_ids() - .iter() - .copied() - .map(|id| id.into()) - .collect::>(), - ); - - if state.is_hot { - node.set_hovered(); - } - if state.is_disabled() { - node.set_disabled(); - } - if state.is_stashed { - node.set_hidden(); - } - - node - } -} - -fn to_accesskit_rect(r: Rect, scale_factor: f64) -> accesskit::Rect { - let s = scale_factor; - accesskit::Rect::new(s * r.x0, s * r.y0, s * r.x1, s * r.y1) + // TODO - Remove + pub fn accessibility(&mut self, _parent_ctx: &mut AccessCtx) {} } // TODO - negative rects? diff --git a/masonry/src/widget/widget_state.rs b/masonry/src/widget/widget_state.rs index dee5bf205..3222dd425 100644 --- a/masonry/src/widget/widget_state.rs +++ b/masonry/src/widget/widget_state.rs @@ -234,7 +234,7 @@ impl WidgetState { self.needs_compose |= child_state.needs_compose; self.needs_paint |= child_state.needs_paint; self.request_anim |= child_state.request_anim; - self.request_accessibility_update |= child_state.request_accessibility_update; + self.needs_accessibility_update |= child_state.needs_accessibility_update; self.children_disabled_changed |= child_state.children_disabled_changed; self.children_disabled_changed |= child_state.is_explicitly_disabled_new != child_state.is_explicitly_disabled; From 936584ef4ff2b5c147c0fdd67401ae8cc43b8121 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Sat, 17 Aug 2024 12:41:03 +0200 Subject: [PATCH 04/21] Request accessibility and paint passes in compose pass --- masonry/src/passes/compose.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/masonry/src/passes/compose.rs b/masonry/src/passes/compose.rs index 57dbb380d..0cd0f5b6e 100644 --- a/masonry/src/passes/compose.rs +++ b/masonry/src/passes/compose.rs @@ -55,6 +55,10 @@ fn compose_widget( let translation = parent_translation + state.item.translation + state.item.origin.to_vec2(); state.item.window_origin = translation.to_point(); + if !moved && !state.item.translation_changed && !state.item.needs_compose { + return; + } + let mut ctx = ComposeCtx { global_state, widget_state: state.item, @@ -65,6 +69,10 @@ fn compose_widget( widget.item.compose(&mut ctx); } + // We need to update the accessibility node's coordinates and repaint it at the new position. + state.item.needs_accessibility_update = true; + state.item.needs_paint = true; + state.item.needs_compose = false; state.item.request_compose = false; state.item.translation_changed = false; @@ -76,9 +84,6 @@ fn compose_widget( widget.reborrow_mut(), state.children, |widget, mut state| { - if !moved && !state.item.translation_changed && !state.item.needs_compose { - return; - } compose_widget( global_state, widget, From 9b057a69bad6fb212b4b305ca2050c32842dbf98 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Sat, 17 Aug 2024 12:47:50 +0200 Subject: [PATCH 05/21] Factor out recurse_on_children function --- masonry/src/passes/accessibility.rs | 39 +++-------------------------- masonry/src/passes/compose.rs | 39 +++-------------------------- masonry/src/passes/mod.rs | 37 ++++++++++++++++++++++++++- masonry/src/passes/paint.rs | 37 ++------------------------- 4 files changed, 44 insertions(+), 108 deletions(-) diff --git a/masonry/src/passes/accessibility.rs b/masonry/src/passes/accessibility.rs index b974b87e6..9fc565444 100644 --- a/masonry/src/passes/accessibility.rs +++ b/masonry/src/passes/accessibility.rs @@ -8,43 +8,10 @@ use tracing::info_span; use tracing::trace; use vello::kurbo::Rect; +use crate::passes::recurse_on_children; use crate::render_root::RenderRootState; -use crate::tree_arena::{ArenaMut, ArenaMutChildren}; -use crate::{AccessCtx, Widget, WidgetId, WidgetState}; - -fn recurse_on_children( - id: WidgetId, - mut widget: ArenaMut<'_, Box>, - mut state: ArenaMutChildren<'_, WidgetState>, - mut callback: impl FnMut(ArenaMut<'_, Box>, ArenaMut<'_, WidgetState>), -) { - let parent_name = widget.item.short_type_name(); - let parent_id = id; - - for child_id in widget.item.children_ids() { - let widget = widget - .children - .get_child_mut(child_id.to_raw()) - .unwrap_or_else(|| { - panic!( - "Error in '{}' #{}: cannot find child #{} returned by children_ids()", - parent_name, - parent_id.to_raw(), - child_id.to_raw() - ) - }); - let state = state.get_child_mut(child_id.to_raw()).unwrap_or_else(|| { - panic!( - "Error in '{}' #{}: cannot find child #{} returned by children_ids()", - parent_name, - parent_id.to_raw(), - child_id.to_raw() - ) - }); - - callback(widget, state); - } -} +use crate::tree_arena::ArenaMut; +use crate::{AccessCtx, Widget, WidgetState}; fn build_accessibility_tree( global_state: &mut RenderRootState, diff --git a/masonry/src/passes/compose.rs b/masonry/src/passes/compose.rs index 0cd0f5b6e..4a93892c8 100644 --- a/masonry/src/passes/compose.rs +++ b/masonry/src/passes/compose.rs @@ -4,43 +4,10 @@ use tracing::info_span; use vello::kurbo::Vec2; +use crate::passes::recurse_on_children; use crate::render_root::{RenderRoot, RenderRootState}; -use crate::tree_arena::{ArenaMut, ArenaMutChildren}; -use crate::{ComposeCtx, Widget, WidgetId, WidgetState}; - -fn recurse_on_children( - id: WidgetId, - mut widget: ArenaMut<'_, Box>, - mut state: ArenaMutChildren<'_, WidgetState>, - mut callback: impl FnMut(ArenaMut<'_, Box>, ArenaMut<'_, WidgetState>), -) { - let parent_name = widget.item.short_type_name(); - let parent_id = id; - - for child_id in widget.item.children_ids() { - let widget = widget - .children - .get_child_mut(child_id.to_raw()) - .unwrap_or_else(|| { - panic!( - "Error in '{}' #{}: cannot find child #{} returned by children_ids()", - parent_name, - parent_id.to_raw(), - child_id.to_raw() - ) - }); - let state = state.get_child_mut(child_id.to_raw()).unwrap_or_else(|| { - panic!( - "Error in '{}' #{}: cannot find child #{} returned by children_ids()", - parent_name, - parent_id.to_raw(), - child_id.to_raw() - ) - }); - - callback(widget, state); - } -} +use crate::tree_arena::ArenaMut; +use crate::{ComposeCtx, Widget, WidgetState}; fn compose_widget( global_state: &mut RenderRootState, diff --git a/masonry/src/passes/mod.rs b/masonry/src/passes/mod.rs index 58350213d..4bcad33ba 100644 --- a/masonry/src/passes/mod.rs +++ b/masonry/src/passes/mod.rs @@ -1,8 +1,9 @@ // Copyright 2024 the Xilem Authors // SPDX-License-Identifier: Apache-2.0 +use crate::tree_arena::{ArenaMut, ArenaMutChildren}; use crate::widget::WidgetArena; -use crate::WidgetId; +use crate::{Widget, WidgetId, WidgetState}; pub mod accessibility; pub mod compose; @@ -11,6 +12,40 @@ pub mod mutate; pub mod paint; pub mod update; +pub(crate) fn recurse_on_children( + id: WidgetId, + mut widget: ArenaMut<'_, Box>, + mut state: ArenaMutChildren<'_, WidgetState>, + mut callback: impl FnMut(ArenaMut<'_, Box>, ArenaMut<'_, WidgetState>), +) { + let parent_name = widget.item.short_type_name(); + let parent_id = id; + + for child_id in widget.item.children_ids() { + let widget = widget + .children + .get_child_mut(child_id.to_raw()) + .unwrap_or_else(|| { + panic!( + "Error in '{}' #{}: cannot find child #{} returned by children_ids()", + parent_name, + parent_id.to_raw(), + child_id.to_raw() + ) + }); + let state = state.get_child_mut(child_id.to_raw()).unwrap_or_else(|| { + panic!( + "Error in '{}' #{}: cannot find child #{} returned by children_ids()", + parent_name, + parent_id.to_raw(), + child_id.to_raw() + ) + }); + + callback(widget, state); + } +} + pub(crate) fn merge_state_up(arena: &mut WidgetArena, widget_id: WidgetId) { let parent_id = arena.parent_of(widget_id); diff --git a/masonry/src/passes/paint.rs b/masonry/src/passes/paint.rs index 6f3abb565..d2fa8ab20 100644 --- a/masonry/src/passes/paint.rs +++ b/masonry/src/passes/paint.rs @@ -8,45 +8,12 @@ use vello::kurbo::{Affine, Rect, Stroke}; use vello::peniko::BlendMode; use vello::Scene; +use crate::passes::recurse_on_children; use crate::render_root::{RenderRoot, RenderRootState}; use crate::theme::get_debug_color; -use crate::tree_arena::{ArenaMut, ArenaMutChildren}; +use crate::tree_arena::ArenaMut; use crate::{PaintCtx, Widget, WidgetId, WidgetState}; -fn recurse_on_children( - id: WidgetId, - mut widget: ArenaMut<'_, Box>, - mut state: ArenaMutChildren<'_, WidgetState>, - mut callback: impl FnMut(ArenaMut<'_, Box>, ArenaMut<'_, WidgetState>), -) { - let parent_name = widget.item.short_type_name(); - let parent_id = id; - - for child_id in widget.item.children_ids() { - let widget = widget - .children - .get_child_mut(child_id.to_raw()) - .unwrap_or_else(|| { - panic!( - "Error in '{}' #{}: cannot find child #{} returned by children_ids()", - parent_name, - parent_id.to_raw(), - child_id.to_raw() - ) - }); - let state = state.get_child_mut(child_id.to_raw()).unwrap_or_else(|| { - panic!( - "Error in '{}' #{}: cannot find child #{} returned by children_ids()", - parent_name, - parent_id.to_raw(), - child_id.to_raw() - ) - }); - - callback(widget, state); - } -} - fn paint_widget( global_state: &mut RenderRootState, complete_scene: &mut Scene, From 805fec03b8257147c9a68f267122bfd248c525b5 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Sat, 17 Aug 2024 14:09:42 +0200 Subject: [PATCH 06/21] Link to "Reserve scene" issue --- masonry/src/passes/paint.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/masonry/src/passes/paint.rs b/masonry/src/passes/paint.rs index d2fa8ab20..0413c0199 100644 --- a/masonry/src/passes/paint.rs +++ b/masonry/src/passes/paint.rs @@ -42,7 +42,8 @@ fn paint_widget( id.to_raw(), ); - // TODO - Reserve (see comment below) + // TODO - Reserve scene + // https://github.com/linebender/xilem/issues/524 let mut scene = Scene::new(); widget.item.paint(&mut ctx, &mut scene); *scenes.entry(id).or_default() = scene; @@ -103,7 +104,8 @@ pub fn root_paint(root: &mut RenderRoot) -> Scene { let debug_paint = std::env::var("MASONRY_DEBUG_PAINT").is_ok_and(|it| !it.is_empty()); - // TODO - Reserve + // TODO - Reserve scene + // https://github.com/linebender/xilem/issues/524 let mut complete_scene = Scene::new(); let (root_widget, root_state) = { From 94233fecce56650add336bc953639753f01651bf Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Sat, 17 Aug 2024 14:56:46 +0200 Subject: [PATCH 07/21] Add clip support to passes --- masonry/src/contexts.rs | 22 ++++++++++++++++++++++ masonry/src/passes/accessibility.rs | 5 +++++ masonry/src/passes/paint.rs | 5 +++-- masonry/src/widget/portal.rs | 24 ++++++------------------ masonry/src/widget/widget_ref.rs | 13 ++++++++++++- masonry/src/widget/widget_state.rs | 4 ++++ 6 files changed, 52 insertions(+), 21 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index 8698fec9e..65f929516 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -868,6 +868,28 @@ impl LayoutCtx<'_> { self.get_child_state_mut(child).needs_layout = false; } + /// Gives the widget a clip path. + /// + /// A widget's clip path will have two effects: + /// - It serves as a mask for painting operations of the widget's children (*not* the widget itself). + /// - Pointer events must be inside that path to reach the widget's children. + pub fn set_clip(&mut self, path: Rect) { + trace!("set_clip {:?}", path); + self.widget_state.clip = Some(path); + // TODO - May not be the right flag to set. + self.widget_state.needs_paint = true; + } + + /// Remove the widget's clip path. + /// + /// See [`LayoutCtx::set_clip`] for details. + pub fn clear_clip(&mut self) { + trace!("clear_clip"); + self.widget_state.clip = None; + // TODO - May not be the right flag to set. + self.widget_state.needs_paint = true; + } + /// Set the position of a child widget, in the parent's coordinate space. This /// will also implicitly change "hot" status and affect the parent's display rect. /// diff --git a/masonry/src/passes/accessibility.rs b/masonry/src/passes/accessibility.rs index 9fc565444..89b1fa3fe 100644 --- a/masonry/src/passes/accessibility.rs +++ b/masonry/src/passes/accessibility.rs @@ -61,6 +61,8 @@ fn build_accessibility_tree( state.item.request_accessibility_update = false; state.item.needs_accessibility_update = false; + // TODO - Skip stashed children + let id = state.item.id; let parent_state = state.item; recurse_on_children( @@ -103,6 +105,9 @@ fn build_access_node(widget: &dyn Widget, state: &WidgetState, scale_factor: f64 if state.is_stashed { node.set_hidden(); } + if state.clip.is_some() { + node.set_clips_children(); + } node } diff --git a/masonry/src/passes/paint.rs b/masonry/src/passes/paint.rs index 0413c0199..53fc0b147 100644 --- a/masonry/src/passes/paint.rs +++ b/masonry/src/passes/paint.rs @@ -52,8 +52,7 @@ fn paint_widget( state.item.request_paint = false; state.item.needs_paint = false; - // TODO - let clip: Option = None; + let clip = state.item.clip; let has_clip = clip.is_some(); let transform = Affine::translate(dbg!(state.item.window_origin).to_vec2()); let scene = scenes.get(&id).unwrap(); @@ -64,6 +63,8 @@ fn paint_widget( complete_scene.append(scene, Some(transform)); + // TODO - Skip stashed children + let id = state.item.id; let size = state.item.size; let parent_state = state.item; diff --git a/masonry/src/widget/portal.rs b/masonry/src/widget/portal.rs index 6bc56161d..30e540ea8 100644 --- a/masonry/src/widget/portal.rs +++ b/masonry/src/widget/portal.rs @@ -337,6 +337,8 @@ impl Widget for Portal { self.set_viewport_pos_raw(portal_size, content_size, self.viewport_pos); // TODO - recompute portal progress + ctx.set_clip(portal_size.to_rect()); + ctx.place_child(&mut self.child, Point::new(0.0, -self.viewport_pos.y)); self.scrollbar_horizontal_visible = @@ -378,24 +380,10 @@ impl Widget for Portal { portal_size } - fn paint(&mut self, ctx: &mut PaintCtx, scene: &mut Scene) { - // TODO - also clip the invalidated region - let clip_rect = ctx.size().to_rect(); - - scene.push_layer(BlendMode::default(), 1., Affine::IDENTITY, &clip_rect); - self.child.paint(ctx, scene); - scene.pop_layer(); - - if self.scrollbar_horizontal_visible { - self.scrollbar_horizontal.paint(ctx, scene); - } else { - ctx.skip_child(&mut self.scrollbar_horizontal); - } - if self.scrollbar_vertical_visible { - self.scrollbar_vertical.paint(ctx, scene); - } else { - ctx.skip_child(&mut self.scrollbar_vertical); - } + fn paint(&mut self, _ctx: &mut PaintCtx, _scene: &mut Scene) { + // TODO + // self.scrollbar_horizontal_visible + // self.scrollbar_vertical_visible } fn accessibility_role(&self) -> Role { diff --git a/masonry/src/widget/widget_ref.rs b/masonry/src/widget/widget_ref.rs index 28901f3f6..e0af2fdc6 100644 --- a/masonry/src/widget/widget_ref.rs +++ b/masonry/src/widget/widget_ref.rs @@ -179,16 +179,27 @@ impl<'w> WidgetRef<'w, dyn Widget> { return None; } + // TODO - Rewrite more elegantly loop { + if let Some(clip) = innermost_widget.state().clip { + let relative_pos = pos.to_vec2() - innermost_widget.state().window_origin.to_vec2(); + // If the widget has a clip, the point must be inside + // else we don't iterate over children. + if !clip.contains(relative_pos.to_point()) { + break; + } + } // TODO - Use Widget::get_child_at_pos method if let Some(child) = innermost_widget.children().into_iter().find(|child| { !child.widget.skip_pointer() && child.state().window_layout_rect().contains(pos) }) { innermost_widget = child; } else { - return Some(innermost_widget); + break; } } + + Some(innermost_widget) } /// Recursively check that the Widget tree upholds various invariants. diff --git a/masonry/src/widget/widget_state.rs b/masonry/src/widget/widget_state.rs index 3222dd425..134c0bd70 100644 --- a/masonry/src/widget/widget_state.rs +++ b/masonry/src/widget/widget_state.rs @@ -65,6 +65,9 @@ pub struct WidgetState { // TODO - Document pub(crate) is_portal: bool, + // TODO - Use general Shape + pub(crate) clip: Option, + pub(crate) request_compose: bool, // TODO - Handle matrix transforms pub(crate) translation: Vec2, @@ -151,6 +154,7 @@ impl WidgetState { paint_insets: Insets::ZERO, local_paint_rect: Rect::ZERO, is_portal: false, + clip: Default::default(), request_compose: true, translation: Vec2::ZERO, translation_changed: false, From fba1554b46bdd2267ab0c3e41ea7c8743e06eaa9 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Sat, 17 Aug 2024 15:06:17 +0200 Subject: [PATCH 08/21] Remove unused import --- masonry/src/widget/portal.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/masonry/src/widget/portal.rs b/masonry/src/widget/portal.rs index 30e540ea8..43d0bfd0a 100644 --- a/masonry/src/widget/portal.rs +++ b/masonry/src/widget/portal.rs @@ -8,8 +8,7 @@ use std::ops::Range; use accesskit::Role; use smallvec::{smallvec, SmallVec}; use tracing::{trace_span, Span}; -use vello::kurbo::{Affine, Point, Rect, Size, Vec2}; -use vello::peniko::BlendMode; +use vello::kurbo::{Point, Rect, Size, Vec2}; use vello::Scene; use crate::widget::{Axis, ScrollBar, WidgetMut}; From 7555bc4ca7f6e85dba73c787e5ef6e299d403a41 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Sat, 17 Aug 2024 15:08:51 +0200 Subject: [PATCH 09/21] Remove unused import --- masonry/src/passes/paint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/masonry/src/passes/paint.rs b/masonry/src/passes/paint.rs index 53fc0b147..9ff52dba2 100644 --- a/masonry/src/passes/paint.rs +++ b/masonry/src/passes/paint.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use tracing::{info_span, trace}; -use vello::kurbo::{Affine, Rect, Stroke}; +use vello::kurbo::{Affine, Stroke}; use vello::peniko::BlendMode; use vello::Scene; From 54ba10ccbd8e342fd8c8131e3224810797543681 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Mon, 19 Aug 2024 12:13:55 +0200 Subject: [PATCH 10/21] Remove leftover dbg macro --- masonry/src/passes/paint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/masonry/src/passes/paint.rs b/masonry/src/passes/paint.rs index 9ff52dba2..b927f8a46 100644 --- a/masonry/src/passes/paint.rs +++ b/masonry/src/passes/paint.rs @@ -54,7 +54,7 @@ fn paint_widget( let clip = state.item.clip; let has_clip = clip.is_some(); - let transform = Affine::translate(dbg!(state.item.window_origin).to_vec2()); + let transform = Affine::translate(state.item.window_origin.to_vec2()); let scene = scenes.get(&id).unwrap(); if let Some(clip) = clip { From 8d9266b5dc155914abb110965df62287c78b917b Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Mon, 19 Aug 2024 12:14:43 +0200 Subject: [PATCH 11/21] Clarify needs/request flags --- masonry/src/contexts.rs | 4 ++-- masonry/src/passes/accessibility.rs | 8 +++---- masonry/src/passes/compose.rs | 2 +- masonry/src/render_root.rs | 2 +- masonry/src/widget/widget_pod.rs | 4 ++-- masonry/src/widget/widget_state.rs | 37 +++++++++++++++++------------ 6 files changed, 32 insertions(+), 25 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index 65f929516..7d53387bb 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -436,8 +436,8 @@ impl_context_method!(MutateCtx<'_>, EventCtx<'_>, LifeCycleCtx<'_>, { pub fn request_accessibility_update(&mut self) { trace!("request_accessibility_update"); - self.widget_state.needs_accessibility_update = true; - self.widget_state.request_accessibility_update = true; + self.widget_state.needs_accessibility = true; + self.widget_state.request_accessibility = true; } /// Request an animation frame. diff --git a/masonry/src/passes/accessibility.rs b/masonry/src/passes/accessibility.rs index 89b1fa3fe..4ece0599c 100644 --- a/masonry/src/passes/accessibility.rs +++ b/masonry/src/passes/accessibility.rs @@ -24,11 +24,11 @@ fn build_accessibility_tree( let _span = widget.item.make_trace_span().entered(); let id = state.item.id; - if !rebuild_all && !state.item.needs_accessibility_update { + if !rebuild_all && !state.item.needs_accessibility { return; } - if rebuild_all || state.item.request_accessibility_update { + if rebuild_all || state.item.request_accessibility { trace!( "Building accessibility node for widget '{}' #{}", widget.item.short_type_name(), @@ -58,8 +58,8 @@ fn build_accessibility_tree( ctx.tree_update.nodes.push((id, ctx.current_node.build())); } - state.item.request_accessibility_update = false; - state.item.needs_accessibility_update = false; + state.item.request_accessibility = false; + state.item.needs_accessibility = false; // TODO - Skip stashed children diff --git a/masonry/src/passes/compose.rs b/masonry/src/passes/compose.rs index 4a93892c8..6f4ab93ca 100644 --- a/masonry/src/passes/compose.rs +++ b/masonry/src/passes/compose.rs @@ -37,7 +37,7 @@ fn compose_widget( } // We need to update the accessibility node's coordinates and repaint it at the new position. - state.item.needs_accessibility_update = true; + state.item.needs_accessibility = true; state.item.needs_paint = true; state.item.needs_compose = false; diff --git a/masonry/src/render_root.rs b/masonry/src/render_root.rs index dc7b50232..6ef799524 100644 --- a/masonry/src/render_root.rs +++ b/masonry/src/render_root.rs @@ -574,7 +574,7 @@ impl RenderRoot { // We request a redraw if either the render tree or the accessibility // tree needs to be rebuilt. Usually both happen at the same time. // A redraw will trigger a rebuild of the accessibility tree. - if self.root_state().needs_paint || self.root_state().needs_accessibility_update { + if self.root_state().needs_paint || self.root_state().needs_accessibility { self.state .signal_queue .push_back(RenderRootSignal::RequestRedraw); diff --git a/masonry/src/widget/widget_pod.rs b/masonry/src/widget/widget_pod.rs index 2e890d617..197ef0ade 100644 --- a/masonry/src/widget/widget_pod.rs +++ b/masonry/src/widget/widget_pod.rs @@ -545,8 +545,8 @@ impl WidgetPod { state.is_expecting_place_child_call = true; // TODO - Not everything that has been re-laid out needs to be repainted. state.needs_paint = true; - state.request_accessibility_update = true; - state.needs_accessibility_update = true; + state.request_accessibility = true; + state.needs_accessibility = true; bc.debug_check(widget.short_type_name()); trace!("Computing layout with constraints {:?}", bc); diff --git a/masonry/src/widget/widget_state.rs b/masonry/src/widget/widget_state.rs index 134c0bd70..b7cddc734 100644 --- a/masonry/src/widget/widget_state.rs +++ b/masonry/src/widget/widget_state.rs @@ -68,7 +68,6 @@ pub struct WidgetState { // TODO - Use general Shape pub(crate) clip: Option, - pub(crate) request_compose: bool, // TODO - Handle matrix transforms pub(crate) translation: Vec2, pub(crate) translation_changed: bool, @@ -88,17 +87,25 @@ pub struct WidgetState { pub(crate) is_explicitly_disabled_new: bool, pub(crate) needs_layout: bool, + + /// The compose method must be called on this widget + pub(crate) request_compose: bool, + /// The compose method must be called on this widget or a descendant pub(crate) needs_compose: bool, - pub(crate) needs_paint: bool, - pub(crate) needs_accessibility_update: bool, + + /// The paint method must be called on this widget pub(crate) request_paint: bool, + /// The paint method must be called on this widget or a descendant + pub(crate) needs_paint: bool, + + /// The accessibility method must be called on this widget + pub(crate) request_accessibility: bool, + /// The accessibility method must be called on this widget or a descendant + pub(crate) needs_accessibility: bool, /// Any descendant has requested an animation frame. pub(crate) request_anim: bool, - /// Any descendant has requested an accessibility update. - pub(crate) request_accessibility_update: bool, - pub(crate) update_focus_chain: bool, pub(crate) focus_chain: Vec, @@ -155,7 +162,6 @@ impl WidgetState { local_paint_rect: Rect::ZERO, is_portal: false, clip: Default::default(), - request_compose: true, translation: Vec2::ZERO, translation_changed: false, children_disabled_changed: false, @@ -164,13 +170,14 @@ impl WidgetState { baseline_offset: 0.0, is_hot: false, needs_layout: true, + request_compose: true, needs_compose: true, - needs_paint: true, - needs_accessibility_update: true, request_paint: true, + needs_paint: true, + request_accessibility: true, + needs_accessibility: true, has_focus: false, request_anim: true, - request_accessibility_update: true, focus_chain: Vec::new(), children: Bloom::new(), children_changed: true, @@ -193,12 +200,12 @@ impl WidgetState { WidgetState { size, needs_layout: false, - needs_paint: false, - needs_accessibility_update: false, - needs_compose: false, request_compose: false, + needs_compose: false, + needs_paint: false, request_paint: false, - request_accessibility_update: false, + request_accessibility: false, + needs_accessibility: false, request_anim: false, children_changed: false, update_focus_chain: false, @@ -238,7 +245,7 @@ impl WidgetState { self.needs_compose |= child_state.needs_compose; self.needs_paint |= child_state.needs_paint; self.request_anim |= child_state.request_anim; - self.needs_accessibility_update |= child_state.needs_accessibility_update; + self.needs_accessibility |= child_state.needs_accessibility; self.children_disabled_changed |= child_state.children_disabled_changed; self.children_disabled_changed |= child_state.is_explicitly_disabled_new != child_state.is_explicitly_disabled; From 4127c9815dfa9150bc526e5f58bacfd074097944 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Mon, 19 Aug 2024 12:29:54 +0200 Subject: [PATCH 12/21] Integrate stashed widgets into paint and access passes --- masonry/src/contexts.rs | 22 ++++++++++++-------- masonry/src/passes/accessibility.rs | 31 +++++++++++++++++++++++++---- masonry/src/passes/paint.rs | 7 +++++-- masonry/src/widget/portal.rs | 15 +++++++++----- 4 files changed, 56 insertions(+), 19 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index 7d53387bb..2bfae4825 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -170,6 +170,7 @@ impl_context_method!( EventCtx<'_>, LifeCycleCtx<'_>, LayoutCtx<'_>, + ComposeCtx<'_>, { /// Helper method to get a mutable reference to a child widget's `WidgetState` from its `WidgetPod`. /// @@ -490,14 +491,6 @@ impl_context_method!(MutateCtx<'_>, EventCtx<'_>, LifeCycleCtx<'_>, { self.widget_state.is_explicitly_disabled_new = disabled; } - /// Mark child widget as stashed. - /// - /// **Note:** Stashed widgets are a WIP feature - pub fn set_stashed(&mut self, child: &mut WidgetPod, stashed: bool) { - self.get_child_state_mut(child).is_stashed = stashed; - self.children_changed(); - } - #[allow(unused)] /// Indicate that text input state has changed. /// @@ -565,6 +558,19 @@ impl_context_method!( 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. + /// + /// **Note:** Stashed widgets are a WIP feature + pub fn set_stashed(&mut self, child: &mut WidgetPod, stashed: bool) { + self.get_child_state_mut(child).is_stashed = stashed; + // TODO - Maybe this should be + // self.children_changed(); + self.widget_state.children_changed = true; + self.widget_state.update_focus_chain = true; + } } ); diff --git a/masonry/src/passes/accessibility.rs b/masonry/src/passes/accessibility.rs index 4ece0599c..876aceb79 100644 --- a/masonry/src/passes/accessibility.rs +++ b/masonry/src/passes/accessibility.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::render_root::RenderRoot; +use crate::tree_arena::ArenaMutChildren; use accesskit::{NodeBuilder, NodeId, TreeUpdate}; use tracing::debug; use tracing::info_span; @@ -34,7 +35,12 @@ fn build_accessibility_tree( widget.item.short_type_name(), id.to_raw(), ); - let current_node = build_access_node(widget.item, state.item, scale_factor); + let current_node = build_access_node( + widget.item, + state.item, + state.children.reborrow_mut(), + scale_factor, + ); let mut ctx = AccessCtx { global_state, @@ -61,8 +67,6 @@ fn build_accessibility_tree( state.item.request_accessibility = false; state.item.needs_accessibility = false; - // TODO - Skip stashed children - let id = state.item.id; let parent_state = state.item; recurse_on_children( @@ -70,6 +74,11 @@ fn build_accessibility_tree( widget.reborrow_mut(), state.children, |widget, mut state| { + // TODO - We skip updating stashed items. + // This may have knock-on effects we'd need to document. + if state.item.is_stashed { + return; + } build_accessibility_tree( global_state, tree_update, @@ -83,15 +92,29 @@ fn build_accessibility_tree( ); } -fn build_access_node(widget: &dyn Widget, state: &WidgetState, scale_factor: f64) -> NodeBuilder { +fn build_access_node( + widget: &dyn Widget, + state: &WidgetState, + state_children: ArenaMutChildren<'_, WidgetState>, + scale_factor: f64, +) -> NodeBuilder { let mut node = NodeBuilder::new(widget.accessibility_role()); node.set_bounds(to_accesskit_rect(state.window_layout_rect(), scale_factor)); + // TODO - We skip listing stashed items. + // This may have knock-on effects we'd need to document. node.set_children( widget .children_ids() .iter() .copied() + .filter(|id| { + !state_children + .get_child(id.to_raw()) + .unwrap() + .item + .is_stashed + }) .map(|id| id.into()) .collect::>(), ); diff --git a/masonry/src/passes/paint.rs b/masonry/src/passes/paint.rs index b927f8a46..a942941f0 100644 --- a/masonry/src/passes/paint.rs +++ b/masonry/src/passes/paint.rs @@ -63,8 +63,6 @@ fn paint_widget( complete_scene.append(scene, Some(transform)); - // TODO - Skip stashed children - let id = state.item.id; let size = state.item.size; let parent_state = state.item; @@ -73,6 +71,11 @@ fn paint_widget( widget.reborrow_mut(), state.children, |widget, mut state| { + // TODO - We skip painting stashed items. + // This may have knock-on effects we'd need to document. + if state.item.is_stashed { + return; + } paint_widget( global_state, complete_scene, diff --git a/masonry/src/widget/portal.rs b/masonry/src/widget/portal.rs index 43d0bfd0a..0ee4f0ee5 100644 --- a/masonry/src/widget/portal.rs +++ b/masonry/src/widget/portal.rs @@ -376,14 +376,19 @@ impl Widget for Portal { ctx.skip_child(&mut self.scrollbar_vertical); } + ctx.set_stashed( + &mut self.scrollbar_vertical, + self.scrollbar_vertical_visible, + ); + ctx.set_stashed( + &mut self.scrollbar_horizontal, + self.scrollbar_horizontal_visible, + ); + portal_size } - fn paint(&mut self, _ctx: &mut PaintCtx, _scene: &mut Scene) { - // TODO - // self.scrollbar_horizontal_visible - // self.scrollbar_vertical_visible - } + fn paint(&mut self, _ctx: &mut PaintCtx, _scene: &mut Scene) {} fn accessibility_role(&self) -> Role { Role::GenericContainer From b33e09e9979b8083461de757d7778574618233a1 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Mon, 19 Aug 2024 14:30:25 +0200 Subject: [PATCH 13/21] Reset rebuild_access_tree flag --- masonry/src/render_root.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/masonry/src/render_root.rs b/masonry/src/render_root.rs index 6ef799524..1e6ef5a62 100644 --- a/masonry/src/render_root.rs +++ b/masonry/src/render_root.rs @@ -514,6 +514,7 @@ impl RenderRoot { // TODO - Integrate in unit tests? fn root_accessibility(&mut self) -> TreeUpdate { let mut tree_update = root_accessibility(self, self.rebuild_access_tree, self.scale_factor); + self.rebuild_access_tree = false; tree_update.tree = Some(Tree { root: self.root.id().into(), From 44f77ff1292906398c8fbc96bc4443d1f9707808 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Mon, 19 Aug 2024 14:31:47 +0200 Subject: [PATCH 14/21] Misc fixes --- masonry/src/contexts.rs | 11 +++++++---- masonry/src/widget/widget_pod.rs | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index 2bfae4825..0cb6bc344 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -565,11 +565,14 @@ impl_context_method!( /// /// **Note:** Stashed widgets are a WIP feature pub fn set_stashed(&mut self, child: &mut WidgetPod, stashed: bool) { + if self.get_child_state_mut(child).is_stashed != stashed { + // TODO - Maybe this should be + // self.children_changed(); + self.widget_state.children_changed = true; + self.widget_state.update_focus_chain = true; + } + self.get_child_state_mut(child).is_stashed = stashed; - // TODO - Maybe this should be - // self.children_changed(); - self.widget_state.children_changed = true; - self.widget_state.update_focus_chain = true; } } ); diff --git a/masonry/src/widget/widget_pod.rs b/masonry/src/widget/widget_pod.rs index 197ef0ade..aeff58e03 100644 --- a/masonry/src/widget/widget_pod.rs +++ b/masonry/src/widget/widget_pod.rs @@ -544,6 +544,7 @@ impl WidgetPod { state.needs_compose = true; state.is_expecting_place_child_call = true; // TODO - Not everything that has been re-laid out needs to be repainted. + state.request_paint = true; state.needs_paint = true; state.request_accessibility = true; state.needs_accessibility = true; From 4343e3ff641a0c96069787d29d99295f35e5a040 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Tue, 20 Aug 2024 11:34:51 +0200 Subject: [PATCH 15/21] Add request_layout flag --- masonry/src/contexts.rs | 1 + masonry/src/widget/widget_pod.rs | 35 ++++++++++++++++++++---------- masonry/src/widget/widget_state.rs | 4 ++++ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index 0cb6bc344..a48f8b406 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -421,6 +421,7 @@ impl_context_method!(MutateCtx<'_>, EventCtx<'_>, LifeCycleCtx<'_>, { /// [`layout`]: crate::Widget::layout pub fn request_layout(&mut self) { trace!("request_layout"); + self.widget_state.request_layout = true; self.widget_state.needs_layout = true; } diff --git a/masonry/src/widget/widget_pod.rs b/masonry/src/widget/widget_pod.rs index aeff58e03..48c5d3695 100644 --- a/masonry/src/widget/widget_pod.rs +++ b/masonry/src/widget/widget_pod.rs @@ -541,19 +541,12 @@ impl WidgetPod { return false; } - state.needs_compose = true; - state.is_expecting_place_child_call = true; - // TODO - Not everything that has been re-laid out needs to be repainted. - state.request_paint = true; - state.needs_paint = true; - state.request_accessibility = true; - state.needs_accessibility = true; - bc.debug_check(widget.short_type_name()); trace!("Computing layout with constraints {:?}", bc); state.local_paint_rect = Rect::ZERO; + state.request_layout = false; let new_size = { let mut inner_ctx = LayoutCtx { widget_state: state, @@ -566,12 +559,30 @@ impl WidgetPod { widget.layout(&mut inner_ctx, bc) }; - // We reset `needs_layout` after the layout call, in case `layout` calls `request_layout`. - // If this did happen, it would be a bug; however, it is allowed for a child of `widget` - // (accessed using `get_raw_mut`) to call `request_layout`, so long as its `layout` is called - // by the parent. We currently cannot differentiate these cases, so we allow both. + if state.request_layout { + debug_panic!( + "Error in '{}' #{}: layout pass has requested layout.", + widget.short_type_name(), + id, + ); + } + + // We reset `needs_layout` after the layout call, which means if + // the widget sets needs_layout to true, that will be overridden. + // This shouldn't happen in practice, except in one case: if we access + // a child using `get_raw_mut` before the child's layout is run. In that + // case the child's needs_layout is still true, and propagates up to + // this widget. The line below resets it to false. state.needs_layout = false; + state.needs_compose = true; + state.is_expecting_place_child_call = true; + // TODO - Not everything that has been re-laid out needs to be repainted. + state.request_paint = true; + state.needs_paint = true; + state.request_accessibility = true; + state.needs_accessibility = true; + state.local_paint_rect = state .local_paint_rect .union(new_size.to_rect() + state.paint_insets); diff --git a/masonry/src/widget/widget_state.rs b/masonry/src/widget/widget_state.rs index b7cddc734..74ffd25ff 100644 --- a/masonry/src/widget/widget_state.rs +++ b/masonry/src/widget/widget_state.rs @@ -86,6 +86,9 @@ pub struct WidgetState { // LifeCycle::DisabledChanged or InternalLifeCycle::RouteDisabledChanged pub(crate) is_explicitly_disabled_new: bool, + /// This widget explicitly requested layout + pub(crate) request_layout: bool, + /// This widget or a descendant explicitly requested layout pub(crate) needs_layout: bool, /// The compose method must be called on this widget @@ -169,6 +172,7 @@ impl WidgetState { is_explicitly_disabled: false, baseline_offset: 0.0, is_hot: false, + request_layout: true, needs_layout: true, request_compose: true, needs_compose: true, From 2129655543eec2456bb11bbc67687ec3628b0742 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Tue, 20 Aug 2024 11:58:08 +0200 Subject: [PATCH 16/21] Tweak clip path methods --- masonry/src/contexts.rs | 14 ++++++++------ masonry/src/passes/compose.rs | 1 + masonry/src/widget/portal.rs | 2 +- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index a48f8b406..9e6e35cc5 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -883,20 +883,22 @@ impl LayoutCtx<'_> { /// A widget's clip path will have two effects: /// - It serves as a mask for painting operations of the widget's children (*not* the widget itself). /// - Pointer events must be inside that path to reach the widget's children. - pub fn set_clip(&mut self, path: Rect) { - trace!("set_clip {:?}", path); + pub fn set_clip_path(&mut self, path: Rect) { + trace!("set_clip_path {:?}", path); self.widget_state.clip = Some(path); - // TODO - May not be the right flag to set. + self.widget_state.request_accessibility = true; + self.widget_state.needs_accessibility = true; self.widget_state.needs_paint = true; } /// Remove the widget's clip path. /// /// See [`LayoutCtx::set_clip`] for details. - pub fn clear_clip(&mut self) { - trace!("clear_clip"); + pub fn clear_clip_path(&mut self) { + trace!("clear_clip_path"); self.widget_state.clip = None; - // TODO - May not be the right flag to set. + self.widget_state.request_accessibility = true; + self.widget_state.needs_accessibility = true; self.widget_state.needs_paint = true; } diff --git a/masonry/src/passes/compose.rs b/masonry/src/passes/compose.rs index 6f4ab93ca..0dd603b2a 100644 --- a/masonry/src/passes/compose.rs +++ b/masonry/src/passes/compose.rs @@ -37,6 +37,7 @@ fn compose_widget( } // We need to update the accessibility node's coordinates and repaint it at the new position. + state.item.request_accessibility = true; state.item.needs_accessibility = true; state.item.needs_paint = true; diff --git a/masonry/src/widget/portal.rs b/masonry/src/widget/portal.rs index 0ee4f0ee5..564dc0a03 100644 --- a/masonry/src/widget/portal.rs +++ b/masonry/src/widget/portal.rs @@ -336,7 +336,7 @@ impl Widget for Portal { self.set_viewport_pos_raw(portal_size, content_size, self.viewport_pos); // TODO - recompute portal progress - ctx.set_clip(portal_size.to_rect()); + ctx.set_clip_path(portal_size.to_rect()); ctx.place_child(&mut self.child, Point::new(0.0, -self.viewport_pos.y)); From 2f0bf41fcac43939df55f9e6619c7d50cedb6a5a Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Tue, 20 Aug 2024 12:54:56 +0200 Subject: [PATCH 17/21] Rework Portal widget Fix stashed child handling. Handle scrolling in compose pass. Fix set_child_translation method. --- masonry/src/contexts.rs | 13 +++++++++---- masonry/src/widget/portal.rs | 34 +++++++++++++++++++--------------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index 9e6e35cc5..c3ab54555 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -937,10 +937,15 @@ impl ComposeCtx<'_> { /// Set a translation for the child widget. /// /// The translation is applied on top of the position from [`LayoutCtx::place_child`]. - pub fn set_child_translation(&mut self, translation: Vec2) { - if self.widget_state.translation != translation { - self.widget_state.translation = translation; - self.widget_state.translation_changed = true; + pub fn set_child_translation( + &mut self, + child: &mut WidgetPod, + translation: Vec2, + ) { + let child = self.get_child_state_mut(child); + if translation != child.translation { + child.translation = translation; + child.translation_changed = true; } } } diff --git a/masonry/src/widget/portal.rs b/masonry/src/widget/portal.rs index 564dc0a03..d06b6fa44 100644 --- a/masonry/src/widget/portal.rs +++ b/masonry/src/widget/portal.rs @@ -13,8 +13,8 @@ use vello::Scene; use crate::widget::{Axis, ScrollBar, WidgetMut}; use crate::{ - AccessCtx, AccessEvent, BoxConstraints, EventCtx, LayoutCtx, LifeCycle, LifeCycleCtx, PaintCtx, - PointerEvent, StatusChange, TextEvent, Widget, WidgetId, WidgetPod, + AccessCtx, AccessEvent, BoxConstraints, ComposeCtx, EventCtx, LayoutCtx, LifeCycle, + LifeCycleCtx, PaintCtx, PointerEvent, StatusChange, TextEvent, Widget, WidgetId, WidgetPod, }; // TODO - refactor - see https://github.com/linebender/xilem/issues/366 @@ -248,7 +248,7 @@ impl Widget for Portal { PointerEvent::MouseWheel(delta, _) => { let delta = Vec2::new(delta.x * -SCROLLING_SPEED, delta.y * -SCROLLING_SPEED); self.set_viewport_pos_raw(portal_size, content_size, self.viewport_pos + delta); - ctx.request_layout(); + ctx.request_compose(); // TODO - horizontal scrolling? let mut scrollbar = ctx.get_raw_mut(&mut self.scrollbar_vertical); @@ -338,13 +338,22 @@ impl Widget for Portal { ctx.set_clip_path(portal_size.to_rect()); - ctx.place_child(&mut self.child, Point::new(0.0, -self.viewport_pos.y)); + ctx.place_child(&mut self.child, Point::ZERO); self.scrollbar_horizontal_visible = !self.constrain_horizontal && portal_size.width < content_size.width; self.scrollbar_vertical_visible = !self.constrain_vertical && portal_size.height < content_size.height; + ctx.set_stashed( + &mut self.scrollbar_vertical, + !self.scrollbar_vertical_visible, + ); + ctx.set_stashed( + &mut self.scrollbar_horizontal, + !self.scrollbar_horizontal_visible, + ); + if self.scrollbar_horizontal_visible { let mut scrollbar = ctx.get_raw_mut(&mut self.scrollbar_horizontal); scrollbar.widget().portal_size = portal_size.width; @@ -358,7 +367,7 @@ impl Widget for Portal { Point::new(0.0, portal_size.height - scrollbar_size.height), ); } else { - ctx.skip_child(&mut self.scrollbar_horizontal); + ctx.skip_layout(&mut self.scrollbar_horizontal); } if self.scrollbar_vertical_visible { let mut scrollbar = ctx.get_raw_mut(&mut self.scrollbar_vertical); @@ -373,21 +382,16 @@ impl Widget for Portal { Point::new(portal_size.width - scrollbar_size.width, 0.0), ); } else { - ctx.skip_child(&mut self.scrollbar_vertical); + ctx.skip_layout(&mut self.scrollbar_vertical); } - ctx.set_stashed( - &mut self.scrollbar_vertical, - self.scrollbar_vertical_visible, - ); - ctx.set_stashed( - &mut self.scrollbar_horizontal, - self.scrollbar_horizontal_visible, - ); - portal_size } + fn compose(&mut self, ctx: &mut ComposeCtx) { + ctx.set_child_translation(&mut self.child, Vec2::new(0.0, -self.viewport_pos.y)); + } + fn paint(&mut self, _ctx: &mut PaintCtx, _scene: &mut Scene) {} fn accessibility_role(&self) -> Role { From 85daf068ffe274cb2569d41107eb5020c2df3ceb Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Tue, 20 Aug 2024 13:14:08 +0200 Subject: [PATCH 18/21] Add some doc comments --- masonry/src/contexts.rs | 8 ++++++-- masonry/src/widget/widget_pod.rs | 7 +++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index c3ab54555..0ac9d67f2 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -564,11 +564,11 @@ impl_context_method!( /// /// If `stashed` is true, the child will not be painted or listed in the accessibility tree. /// + /// This will *not* trigger a layout pass. + /// /// **Note:** Stashed widgets are a WIP feature pub fn set_stashed(&mut self, child: &mut WidgetPod, stashed: bool) { if self.get_child_state_mut(child).is_stashed != stashed { - // TODO - Maybe this should be - // self.children_changed(); self.widget_state.children_changed = true; self.widget_state.update_focus_chain = true; } @@ -886,6 +886,8 @@ impl LayoutCtx<'_> { pub fn set_clip_path(&mut self, path: Rect) { trace!("set_clip_path {:?}", path); self.widget_state.clip = Some(path); + // TODO - Updating the clip path may have + // other knock-on effects we'd need to document. self.widget_state.request_accessibility = true; self.widget_state.needs_accessibility = true; self.widget_state.needs_paint = true; @@ -897,6 +899,8 @@ impl LayoutCtx<'_> { pub fn clear_clip_path(&mut self) { trace!("clear_clip_path"); self.widget_state.clip = None; + // TODO - Updating the clip path may have + // other knock-on effects we'd need to document. self.widget_state.request_accessibility = true; self.widget_state.needs_accessibility = true; self.widget_state.needs_paint = true; diff --git a/masonry/src/widget/widget_pod.rs b/masonry/src/widget/widget_pod.rs index 48c5d3695..ec7788ff3 100644 --- a/masonry/src/widget/widget_pod.rs +++ b/masonry/src/widget/widget_pod.rs @@ -655,11 +655,14 @@ impl WidgetPod { // --- MARK: PAINT --- - // TODO - remove + // TODO - This should be removed in a follow-up PR immediately after + // this is merged. I'm leaving the method for now to avoid blowing up the diff. pub fn paint(&mut self, _parent_ctx: &mut PaintCtx, _scene: &mut vello::Scene) {} // --- MARK: ACCESSIBILITY --- - // TODO - Remove + + // TODO - This should be removed in a follow-up PR immediately after + // this is merged. I'm leaving the method for now to avoid blowing up the diff. pub fn accessibility(&mut self, _parent_ctx: &mut AccessCtx) {} } From d722aca5a93a78164ab657cfac3d35073f1daa46 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Tue, 20 Aug 2024 13:21:05 +0200 Subject: [PATCH 19/21] Make some root methods private --- masonry/src/passes/compose.rs | 2 +- masonry/src/passes/event.rs | 6 +++--- masonry/src/passes/paint.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/masonry/src/passes/compose.rs b/masonry/src/passes/compose.rs index 0dd603b2a..732a1e473 100644 --- a/masonry/src/passes/compose.rs +++ b/masonry/src/passes/compose.rs @@ -66,7 +66,7 @@ fn compose_widget( // ---------------- -pub fn root_compose(root: &mut RenderRoot, global_root_state: &mut WidgetState) { +pub(crate) fn root_compose(root: &mut RenderRoot, global_root_state: &mut WidgetState) { let _span = info_span!("compose").entered(); let (root_widget, root_state) = root.widget_arena.get_pair_mut(root.root.id()); diff --git a/masonry/src/passes/event.rs b/masonry/src/passes/event.rs index a9a34b670..872cc0583 100644 --- a/masonry/src/passes/event.rs +++ b/masonry/src/passes/event.rs @@ -83,7 +83,7 @@ fn run_event_pass( // TODO - Send synthetic MouseLeave events -pub fn root_on_pointer_event( +pub(crate) fn root_on_pointer_event( root: &mut RenderRoot, root_state: &mut WidgetState, event: &PointerEvent, @@ -119,7 +119,7 @@ pub fn root_on_pointer_event( handled } -pub fn root_on_text_event( +pub(crate) fn root_on_text_event( root: &mut RenderRoot, root_state: &mut WidgetState, event: &TextEvent, @@ -165,7 +165,7 @@ pub fn root_on_text_event( handled } -pub fn root_on_access_event( +pub(crate) fn root_on_access_event( root: &mut RenderRoot, root_state: &mut WidgetState, event: &AccessEvent, diff --git a/masonry/src/passes/paint.rs b/masonry/src/passes/paint.rs index a942941f0..fcf9aa207 100644 --- a/masonry/src/passes/paint.rs +++ b/masonry/src/passes/paint.rs @@ -103,7 +103,7 @@ fn paint_widget( // ---------------- -pub fn root_paint(root: &mut RenderRoot) -> Scene { +pub(crate) fn root_paint(root: &mut RenderRoot) -> Scene { let _span = info_span!("paint").entered(); let debug_paint = std::env::var("MASONRY_DEBUG_PAINT").is_ok_and(|it| !it.is_empty()); From 1d4f30df6d0973b3744fa49c9b9fa2a92a04c258 Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Wed, 21 Aug 2024 16:12:48 +0200 Subject: [PATCH 20/21] Apply suggestions from code review Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> --- masonry/src/contexts.rs | 2 +- masonry/src/passes/paint.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index 0ac9d67f2..eccc9814a 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -895,7 +895,7 @@ impl LayoutCtx<'_> { /// Remove the widget's clip path. /// - /// See [`LayoutCtx::set_clip`] for details. + /// See [`LayoutCtx::set_clip_path`] for details. pub fn clear_clip_path(&mut self) { trace!("clear_clip_path"); self.widget_state.clip = None; diff --git a/masonry/src/passes/paint.rs b/masonry/src/passes/paint.rs index fcf9aa207..d12594a20 100644 --- a/masonry/src/passes/paint.rs +++ b/masonry/src/passes/paint.rs @@ -44,9 +44,9 @@ fn paint_widget( // TODO - Reserve scene // https://github.com/linebender/xilem/issues/524 - let mut scene = Scene::new(); + let mut scene = scenes.entry(id).or_default(); + scene.reset(); widget.item.paint(&mut ctx, &mut scene); - *scenes.entry(id).or_default() = scene; } state.item.request_paint = false; @@ -58,7 +58,7 @@ fn paint_widget( let scene = scenes.get(&id).unwrap(); if let Some(clip) = clip { - complete_scene.push_layer(BlendMode::default(), 1., transform, &clip); + complete_scene.push_layer(Mix::Clip, 1., transform, &clip); } complete_scene.append(scene, Some(transform)); From 61cfb0754fc0d1cedfa1060e98e892ecbf04205e Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Wed, 21 Aug 2024 16:32:53 +0200 Subject: [PATCH 21/21] Misc fixes --- masonry/src/contexts.rs | 15 +-------------- masonry/src/passes/paint.rs | 21 +++++++++++++-------- masonry/src/render_root.rs | 3 ++- masonry/src/widget/widget_arena.rs | 19 ------------------- masonry/src/widget/widget_state.rs | 2 ++ 5 files changed, 18 insertions(+), 42 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index eccc9814a..01660bfb1 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -98,8 +98,6 @@ pub struct PaintCtx<'a> { pub(crate) widget_state: &'a WidgetState, pub(crate) widget_state_children: ArenaMutChildren<'a, WidgetState>, pub(crate) widget_children: ArenaMutChildren<'a, Box>, - /// The approximate depth in the tree at the time of painting. - pub(crate) depth: u32, pub(crate) debug_paint: bool, } @@ -473,6 +471,7 @@ impl_context_method!(MutateCtx<'_>, EventCtx<'_>, LifeCycleCtx<'_>, { .widget_children .remove_child(id) .expect("remove_child: child not found"); + self.global_state.scenes.remove(&child.id()); self.children_changed(); } @@ -966,18 +965,6 @@ impl_context_method!(LayoutCtx<'_>, PaintCtx<'_>, { }); impl PaintCtx<'_> { - /// The depth in the tree of the currently painting widget. - /// - /// This may be used in combination with [`paint_with_z_index`](Self::paint_with_z_index) in order - /// to correctly order painting operations. - /// - /// The `depth` here may not be exact; it is only guaranteed that a child will - /// have a greater depth than its parent. - #[inline] - pub fn depth(&self) -> u32 { - self.depth - } - // signal may be useful elsewhere, but is currently only used on PaintCtx /// Submit a [`RenderRootSignal`] /// diff --git a/masonry/src/passes/paint.rs b/masonry/src/passes/paint.rs index d12594a20..838eff7cd 100644 --- a/masonry/src/passes/paint.rs +++ b/masonry/src/passes/paint.rs @@ -5,7 +5,7 @@ use std::collections::HashMap; use tracing::{info_span, trace}; use vello::kurbo::{Affine, Stroke}; -use vello::peniko::BlendMode; +use vello::peniko::Mix; use vello::Scene; use crate::passes::recurse_on_children; @@ -20,7 +20,6 @@ fn paint_widget( scenes: &mut HashMap, mut widget: ArenaMut<'_, Box>, mut state: ArenaMut<'_, WidgetState>, - depth: u32, debug_paint: bool, ) { let _span = widget.item.make_trace_span().entered(); @@ -32,7 +31,6 @@ fn paint_widget( widget_state: state.item, widget_state_children: state.children.reborrow_mut(), widget_children: widget.children.reborrow_mut(), - depth, debug_paint, }; if ctx.widget_state.request_paint { @@ -44,9 +42,9 @@ fn paint_widget( // TODO - Reserve scene // https://github.com/linebender/xilem/issues/524 - let mut scene = scenes.entry(id).or_default(); + let scene = scenes.entry(id).or_default(); scene.reset(); - widget.item.paint(&mut ctx, &mut scene); + widget.item.paint(&mut ctx, scene); } state.item.request_paint = false; @@ -76,13 +74,16 @@ fn paint_widget( if state.item.is_stashed { return; } + // TODO: We could skip painting children outside the parent clip path. + // There's a few things to consider if we do: + // - Some widgets can paint outside of their layout box. + // - Once we implement compositor layers, we may want to paint outside of the clip path anyway in anticipation of user scrolling. paint_widget( global_state, complete_scene, scenes, widget, state.reborrow_mut(), - depth + 1, debug_paint, ); parent_state.merge_up(state.item); @@ -127,15 +128,19 @@ pub(crate) fn root_paint(root: &mut RenderRoot) -> Scene { (widget, state) }; + // TODO - This is a bit of a hack until we refactor widget tree mutation. + // This should be removed once remove_child is exclusive to MutqteCtx. + let mut scenes = std::mem::take(&mut root.state.scenes); + paint_widget( &mut root.state, &mut complete_scene, - &mut root.widget_arena.scenes, + &mut scenes, root_widget, root_state, - 0, debug_paint, ); + root.state.scenes = scenes; complete_scene } diff --git a/masonry/src/render_root.rs b/masonry/src/render_root.rs index 1e6ef5a62..61e25ad72 100644 --- a/masonry/src/render_root.rs +++ b/masonry/src/render_root.rs @@ -66,6 +66,7 @@ pub(crate) struct RenderRootState { pub(crate) font_context: FontContext, pub(crate) text_layout_context: LayoutContext, pub(crate) mutate_callbacks: Vec, + pub(crate) scenes: HashMap, } #[allow(clippy::type_complexity)] @@ -143,11 +144,11 @@ impl RenderRoot { }, text_layout_context: LayoutContext::new(), mutate_callbacks: Vec::new(), + scenes: HashMap::new(), }, widget_arena: WidgetArena { widgets: TreeArena::new(), widget_states: TreeArena::new(), - scenes: HashMap::new(), }, rebuild_access_tree: true, }; diff --git a/masonry/src/widget/widget_arena.rs b/masonry/src/widget/widget_arena.rs index ad7b2f7b4..4b2f858df 100644 --- a/masonry/src/widget/widget_arena.rs +++ b/masonry/src/widget/widget_arena.rs @@ -1,10 +1,6 @@ // Copyright 2024 the Xilem Authors // SPDX-License-Identifier: Apache-2.0 -use std::collections::HashMap; - -use vello::Scene; - use crate::tree_arena::{ArenaMut, ArenaRef, TreeArena}; use crate::widget::WidgetRef; use crate::{Widget, WidgetId, WidgetState}; @@ -13,7 +9,6 @@ pub(crate) struct WidgetArena { pub(crate) widgets: TreeArena>, // TODO - Rename to "states" pub(crate) widget_states: TreeArena, - pub(crate) scenes: HashMap, } #[allow(dead_code)] @@ -93,20 +88,6 @@ impl WidgetArena { .expect("get_state_mut: widget state not in widget tree") } - #[track_caller] - pub(crate) fn get_scene(&mut self, widget_id: WidgetId) -> &Scene { - self.scenes - .get(&widget_id) - .expect("get_scene: scene not in widget tree") - } - - #[track_caller] - pub(crate) fn get_scene_mut(&mut self, widget_id: WidgetId) -> &mut Scene { - self.scenes - .get_mut(&widget_id) - .expect("get_scene_mut: scene not in widget tree") - } - pub fn try_get_widget_ref(&self, id: WidgetId) -> Option> { let state_ref = self.widget_states.find(id.to_raw())?; let widget_ref = self diff --git a/masonry/src/widget/widget_state.rs b/masonry/src/widget/widget_state.rs index 74ffd25ff..2724c4d7e 100644 --- a/masonry/src/widget/widget_state.rs +++ b/masonry/src/widget/widget_state.rs @@ -66,6 +66,8 @@ pub struct WidgetState { pub(crate) is_portal: bool, // TODO - Use general Shape + // Currently Kurbo doesn't really provide a type that lets us + // efficiently hold an arbitrary shape. pub(crate) clip: Option, // TODO - Handle matrix transforms