From fc5125e27f8f4f655e1de5049d0d53536284d9a0 Mon Sep 17 00:00:00 2001 From: Arnold Loubriat Date: Thu, 23 May 2024 20:02:31 +0200 Subject: [PATCH] fix: Improve how coordinates are computed on Unix (#420) --- platforms/atspi-common/src/node.rs | 100 +++++++++++------------------ platforms/atspi-common/src/util.rs | 41 ++++++++---- 2 files changed, 68 insertions(+), 73 deletions(-) diff --git a/platforms/atspi-common/src/node.rs b/platforms/atspi-common/src/node.rs index 1df47662b..32eb8e402 100644 --- a/platforms/atspi-common/src/node.rs +++ b/platforms/atspi-common/src/node.rs @@ -416,22 +416,18 @@ impl<'a> NodeWrapper<'a> { (state.raw_bounds(), state.direct_transform()) } - fn extents(&self, window_bounds: &WindowBounds) -> AtspiRect { - if self.is_root() { - return window_bounds.outer.into(); - } - self.0.bounding_box().map_or_else( - || AtspiRect::INVALID, - |bounds| { - let window_top_left = window_bounds.inner.origin(); - let node_origin = bounds.origin(); - let new_origin = Point::new( - window_top_left.x + node_origin.x, - window_top_left.y + node_origin.y, + fn extents(&self, window_bounds: &WindowBounds, coord_type: CoordType) -> Option { + self.is_root() + .then(|| window_bounds.inner.with_origin(Point::ZERO)) + .or_else(|| self.0.bounding_box()) + .map(|bounds| { + let new_origin = window_bounds.accesskit_point_to_atspi_point( + bounds.origin(), + self.0.filtered_parent(&filter), + coord_type, ); - bounds.with_origin(new_origin).into() - }, - ) + bounds.with_origin(new_origin) + }) } fn current_value(&self) -> Option { @@ -524,10 +520,9 @@ impl<'a> NodeWrapper<'a> { old: &NodeWrapper<'_>, ) { if self.raw_bounds_and_transform() != old.raw_bounds_and_transform() { - adapter.emit_object_event( - self.id(), - ObjectEvent::BoundsChanged(self.extents(window_bounds)), - ); + if let Some(extents) = self.extents(window_bounds, CoordType::Window) { + adapter.emit_object_event(self.id(), ObjectEvent::BoundsChanged(extents.into())); + } } } @@ -819,24 +814,12 @@ impl PlatformNode { pub fn contains(&self, x: i32, y: i32, coord_type: CoordType) -> Result { self.resolve_with_context(|node, context| { let window_bounds = context.read_root_window_bounds(); - let bounds = match node.bounding_box() { - Some(node_bounds) => { - let top_left = window_bounds.top_left(coord_type, node.is_root()); - let new_origin = - Point::new(top_left.x + node_bounds.x0, top_left.y + node_bounds.y0); - node_bounds.with_origin(new_origin) - } - None if node.is_root() => { - let bounds = window_bounds.outer; - match coord_type { - CoordType::Screen => bounds, - CoordType::Window => bounds.with_origin(Point::ZERO), - _ => unimplemented!(), - } - } - _ => return Err(Error::UnsupportedInterface), - }; - Ok(bounds.contains(Point::new(x.into(), y.into()))) + let wrapper = NodeWrapper(&node); + if let Some(extents) = wrapper.extents(&window_bounds, coord_type) { + Ok(extents.contains(Point::new(x.into(), y.into()))) + } else { + Ok(false) + } }) } @@ -848,8 +831,11 @@ impl PlatformNode { ) -> Result> { self.resolve_with_context(|node, context| { let window_bounds = context.read_root_window_bounds(); - let top_left = window_bounds.top_left(coord_type, node.is_root()); - let point = Point::new(f64::from(x) - top_left.x, f64::from(y) - top_left.y); + let point = window_bounds.atspi_point_to_accesskit_point( + Point::new(x.into(), y.into()), + Some(node), + coord_type, + ); let point = node.transform().inverse() * point; Ok(node.node_at_point(point, &filter).map(|node| node.id())) }) @@ -858,23 +844,10 @@ impl PlatformNode { pub fn extents(&self, coord_type: CoordType) -> Result { self.resolve_with_context(|node, context| { let window_bounds = context.read_root_window_bounds(); - match node.bounding_box() { - Some(node_bounds) => { - let top_left = window_bounds.top_left(coord_type, node.is_root()); - let new_origin = - Point::new(top_left.x + node_bounds.x0, top_left.y + node_bounds.y0); - Ok(node_bounds.with_origin(new_origin).into()) - } - None if node.is_root() => { - let bounds = window_bounds.outer; - Ok(match coord_type { - CoordType::Screen => bounds.into(), - CoordType::Window => bounds.with_origin(Point::ZERO).into(), - _ => unimplemented!(), - }) - } - _ => Err(Error::UnsupportedInterface), - } + let wrapper = NodeWrapper(&node); + Ok(wrapper + .extents(&window_bounds, coord_type) + .map_or(AtspiRect::INVALID, AtspiRect::from)) }) } @@ -899,16 +872,19 @@ impl PlatformNode { } pub fn scroll_to_point(&self, coord_type: CoordType, x: i32, y: i32) -> Result { - self.do_action_internal(|tree_state, context| { + self.resolve_with_context(|node, context| { let window_bounds = context.read_root_window_bounds(); - let is_root = self.id == tree_state.root_id(); - let top_left = window_bounds.top_left(coord_type, is_root); - let point = Point::new(f64::from(x) - top_left.x, f64::from(y) - top_left.y); - ActionRequest { + let point = window_bounds.atspi_point_to_accesskit_point( + Point::new(x.into(), y.into()), + node.filtered_parent(&filter), + coord_type, + ); + context.do_action(ActionRequest { action: Action::ScrollToPoint, target: self.id, data: Some(ActionData::ScrollToPoint(point)), - } + }); + Ok(()) })?; Ok(true) } diff --git a/platforms/atspi-common/src/util.rs b/platforms/atspi-common/src/util.rs index bbfaefd4a..63b6265a9 100644 --- a/platforms/atspi-common/src/util.rs +++ b/platforms/atspi-common/src/util.rs @@ -4,6 +4,7 @@ // the LICENSE-MIT file), at your option. use accesskit::{Point, Rect}; +use accesskit_consumer::Node; use atspi_common::CoordType; #[derive(Clone, Copy, Default)] @@ -17,20 +18,38 @@ impl WindowBounds { Self { outer, inner } } - pub(crate) fn top_left(&self, coord_type: CoordType, is_root: bool) -> Point { + pub(crate) fn accesskit_point_to_atspi_point( + &self, + point: Point, + parent: Option, + coord_type: CoordType, + ) -> Point { + let origin = self.origin(parent, coord_type); + Point::new(origin.x + point.x, origin.y + point.y) + } + + pub(crate) fn atspi_point_to_accesskit_point( + &self, + point: Point, + parent: Option, + coord_type: CoordType, + ) -> Point { + let origin = self.origin(parent, coord_type); + Point::new(point.x - origin.x, point.y - origin.y) + } + + fn origin(&self, parent: Option, coord_type: CoordType) -> Point { match coord_type { - CoordType::Screen if is_root => self.outer.origin(), CoordType::Screen => self.inner.origin(), - CoordType::Window if is_root => Point::ZERO, - CoordType::Window => { - let outer_position = self.outer.origin(); - let inner_position = self.inner.origin(); - Point::new( - inner_position.x - outer_position.x, - inner_position.y - outer_position.y, - ) + CoordType::Window => Point::ZERO, + CoordType::Parent => { + if let Some(parent) = parent { + let parent_origin = parent.bounding_box().unwrap_or_default().origin(); + Point::new(-parent_origin.x, -parent_origin.y) + } else { + self.inner.origin() + } } - _ => unimplemented!(), } } }