Skip to content

fix: Fix problems related to the root node #231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mwcampbell merged 3 commits into from
Mar 29, 2023
Merged

fix: Fix problems related to the root node #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ae3467b
fix: Use the view's frame rectangle as a fallback for the root node
mwcampbell Mar 25, 2023
b294b70
Also fix accessibilityParent for the root node
mwcampbell Mar 25, 2023
c42a04a
Finally fix hit testing
mwcampbell Mar 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions platforms/macos/src/adapter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,7 @@ impl Adapter {
let state = tree.state();
let root = state.root();
let point = from_ns_point(&view, &root, point);
if let Some(node) = root.node_at_point(point, &filter) {
Copy link
Copy Markdown

@artursapek artursapek Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @mwcampbell - I've arrived at this change from bevy, which relies on accesskit. this specific code change adds significant latency to bevy registering mouse click events on my Mac, running 13.5 (22G74).

bevy 0.11 upgraded accesskit in https://github.com/bevyengine/bevy/pull/8655/files which included this change. there is now a noticeable delay in mouse click events registering on my machine.

I've recorded a video to show what I mean, you can hear when my mouse clicks and when the event registers in bevy. on the left is the updated version of your code, and on the right is the previous version.

https://imgur.com/a/py58Zk4

reverting this specific change in platforms/macos/src/adapter.rs fixes the latency.

I'm not familiar with this crate or code, but I'm guessing that get_or_create_platform_node(node.id()) with root is slow? can you help me understand why this was necessary? can you think of a reason why it might be adding latency?

in the meantime I'm going to open an issue in the Bevy repo at their request.

thanks in advance for any help

Copy link
Copy Markdown

@artursapek artursapek Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logged the issue: bevyengine/bevy#9391

Copy link
Copy Markdown
Contributor Author

@mwcampbell mwcampbell Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may need to post your video somewhere else, because on imgur, I don't hear anything.

Are you using any macOS accessibility features such as VoiceOver?

Copy link
Copy Markdown
Contributor Author

@mwcampbell mwcampbell Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disregard my comment about Imgur; I found the volume control. But that video won't help me anyway. You said I could hear the difference, but apparently I need to be able to see as well in order to understand it. Anyway, I'll continue the discussion on the Bevy issue.

Copy link
Copy Markdown

@artursapek artursapek Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh are you blind? I'm sorry I didn't know that

basically it shows debug prints in the console happening much later than the audible mouse click

return Id::autorelease_return(self.context.get_or_create_platform_node(node.id()))
as *mut _;
}
null_mut()
let node = root.node_at_point(point, &filter).unwrap_or(root);
Id::autorelease_return(self.context.get_or_create_platform_node(node.id())) as *mut _
}
}
8 changes: 8 additions & 0 deletions platforms/macos/src/appkit/view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,13 @@ extern_methods!(

#[sel(backingScaleFactor)]
pub(crate) fn backing_scale_factor(&self) -> CGFloat;

// NSView actually implements the full NSAccessibility protocol,
// but since we don't have complete metadata for that, it's easier
// to just expose the needed methods here.
#[sel(accessibilityFrame)]
pub(crate) fn accessibility_frame(&self) -> NSRect;
#[sel(accessibilityParent)]
pub(crate) fn accessibility_parent(&self) -> *mut NSObject;
}
);
14 changes: 11 additions & 3 deletions platforms/macos/src/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ declare_class!(
context
.view
.load()
.map_or_else(null_mut, |view| Id::autorelease_return(view) as *mut _)
.map_or_else(null_mut, |view| view.accessibility_parent())
}
})
.unwrap_or_else(null_mut)
Expand All @@ -403,8 +403,16 @@ declare_class!(
}
};

node.bounding_box()
.map_or(NSRect::ZERO, |rect| to_ns_rect(&view, rect))
node.bounding_box().map_or_else(
|| {
if node.is_root() {
view.accessibility_frame()
} else {
NSRect::ZERO
}
},
|rect| to_ns_rect(&view, rect),
)
})
.unwrap_or(NSRect::ZERO)
}
Expand Down