Skip to content
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.

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

feat: Factor out core AT-SPI translation layer #352

Merged
merged 31 commits into from Feb 24, 2024
Merged

feat: Factor out core AT-SPI translation layer #352

merged 31 commits into from Feb 24, 2024

Conversation

mwcampbell
Copy link
Contributor

This will allow us to re-use the logic for translating between AccessKit and AT-SPI, in both the current Unix adapter and a new client-side library (for ATs like Orca) that is being developed as part of the new GNOME accessibility stack.

@mwcampbell
Copy link
Contributor Author

I don't expect to make any major changes to the accesskit_atspi_common API at this point, though I can't rule out needing to make a few more tweaks similar to my last few pushes.

@mwcampbell
Copy link
Contributor Author

Now I'm ready to show what I'm doing with this refactor. Note that the new GNOME accessibility stack is code-named Newton.

See this repository with a newton_atspi_compat crate and a Python binding on top:

https://gitlab.gnome.org/mwcampbell/newton_atspi_compat

and this proof-of-concept Orca modification that uses the Python module:

https://gitlab.gnome.org/mwcampbell/orca/tree/newton

It's still kind of a mess. I might be doing too much directly in the Python binding. And I do have to write a lot of conditionals in Orca, mostly because Orca is hard-coded to call functions on Atspi.Accessible, so I can't just take advantage of duck typing. And I need to document the steps to set up a complete working system, including my fork of Mutter (the compositor). But the Newton protocols are now working end to end, without passing through the AT-SPI D-Bus interface as I did in an earlier demo. And, back to this PR, my hope is that as we continue to develop the AccessKit AT-SPI backend, most of that work can be reused for the Newton project.

mwcampbell and others added 3 commits February 23, 2024 15:22
Co-authored-by: Arnold Loubriat <datatriny@gmail.com>
@mwcampbell
Copy link
Contributor Author

As soon as I add support for the action and component interfaces to the new "simplified" API I just pushed, I will be done adding things to this PR.

@mwcampbell
Copy link
Contributor Author

I don't plan to add or change any more in this PR. Once it passes review, I'm ready to merge it.

I wonder, though, if we should classify this as a refactor instead of a fix, even though the only way to do that is to use refactor!, which will trigger a breaking version increment on accesskit_unix. Really though, the main user of accesskit_unix so far is accesskit_winit, and the automatic dependency bump shouldn't cause a breaking version increment on accesskit_winit.

@DataTriny
Copy link
Member

I guess the correct way would have been to make all the changes to accesskit_unix in another PR because its public API will not change after this. We'll probably want to pack other things in the upcoming release anyway.

@mwcampbell
Copy link
Contributor Author

Yes, but then accesskit_unix would have been broken on the main branch in the meantime, leading to failing CI. I'm not sure that release-please really gives us a good option here.

@mwcampbell
Copy link
Contributor Author

I do have a breaking change in mind for accesskit_unix. To prepare the API to support both Newton and legacy AT-SPI in a single adapter, I believe we'll have to modify Adapter::new to take a RawWindowHandle, since the Newton backend requires a Wayland display and surface.

@DataTriny
Copy link
Member

I don't see why this would have broken accesskit_unix? We'd just have a lot of duplicate code for a brief period of time.

@mwcampbell
Copy link
Contributor Author

Oh, you're right, of course. Then I can still pull the accesskit_unix changes out into a separate PR.

@DataTriny
Copy link
Member

Doing it in two steps would also make it a bit simpler for me to rebase my unix-text branch.

@mwcampbell
Copy link
Contributor Author

Then I'll back out the accesskit_unix changes in this PR, and open the second one when this PR is merged. Before I do that, do you have any feedback about the design as a whole (that would require me to change both sides at once)?

@DataTriny
Copy link
Member

I think you already addressed my concerns. The last remaining one would be from an outsider point of view: we know the dependency count of accesskit_unix is an issue for some downstream projects, yet here you are adding another one. I haven't looked into this, but I wouldn't be surprized if build times would slightly decrease because of that though.

@mwcampbell
Copy link
Contributor Author

It bothers me that Bevy isn't enabling the Unix adapter by default. But I can't let perceptions related to dependency count and binary size dictate all design decisions, especially when it comes to adding just one small crate. Anyway, once Newton is stabilized, the accesskit_unix adapter could let users disable AT-SPI and only enable Newton via features, thus eliminating the dependency on zbus.

@mwcampbell mwcampbell changed the title fix: Factor out core AT-SPI translation layer feat: Factor out core AT-SPI translation layer Feb 24, 2024
@mwcampbell
Copy link
Contributor Author

OK, the changes to accesskit_unix are now in the unix-use-atspi-common branch, which I'll squash and rebase once this is merged.

@DataTriny
Copy link
Member

Oh, one final comment but it's not a blocker: I wonder if the platforms directory really is the best place for this crate. I now have the same feeling for accesskit_winit actually.

@mwcampbell
Copy link
Contributor Author

I agree it's not ideal, but nothing better immediately comes to mind. We can always move it later without breaking anything user-visible.

Are we ready to merge this first PR?

Copy link
Member

@DataTriny DataTriny left a comment

Choose a reason for hiding this comment

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

Congratulations on this first one!

@DataTriny DataTriny merged commit 8c0ab58 into main Feb 24, 2024
10 checks passed
@DataTriny DataTriny deleted the atspi-common branch February 24, 2024 18:23
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.

None yet

2 participants