Skip to content

fix: Fix problems related to the root node#231

Merged
mwcampbell merged 3 commits intomainfrom
macos-root-rect
Mar 29, 2023
Merged

fix: Fix problems related to the root node#231
mwcampbell merged 3 commits intomainfrom
macos-root-rect

Conversation

@mwcampbell
Copy link
Copy Markdown
Contributor

AccessKit allows the root node to omit the bounding rectangle. On Windows and Linux, we can use the window's own rectangle as a fallback. This PR implements the equivalent behavior on macOS.

@mwcampbell mwcampbell changed the title fix: Use the view's frame rectangle as a fallback for the root node fix: Fix problems related to the root node Mar 25, 2023
@mwcampbell
Copy link
Copy Markdown
Contributor Author

The scope of this PR expanded somewhat, based on feedback from a user who was trying to examine AccessKit nodes with Accessibility Inspector.

@mwcampbell
Copy link
Copy Markdown
Contributor Author

@spencerudnick Can you please review this?

@rain-sk
Copy link
Copy Markdown

rain-sk commented Mar 26, 2023

I didn't try building and running anything yet, but in principle these changes look correct. It's nice to get hit-testing working!

Let me know if you would like me to test things locally.

@mwcampbell
Copy link
Copy Markdown
Contributor Author

Merging this now based on the feedback I already got from the user who raised the problems that this PR fixes.

@mwcampbell mwcampbell merged commit 7228494 into main Mar 29, 2023
@mwcampbell mwcampbell deleted the macos-root-rect branch March 29, 2023 22:18
@github-actions github-actions bot mentioned this pull request Mar 29, 2023
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

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

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

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

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants