-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Merged by Bors] - Integrate AccessKit #6874
Conversation
… Also, other cleanup.
|
||
fn handle_focus( | ||
last_focus: Res<LastFocus>, | ||
adapters: NonSend<Adapters>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be NonSend
? This is going to run up against #6552 eventually. If we can make it Send
, it'd be for the best long term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are the errors I get when trying to drop the NonSend
requirement. Not sure if there's anything I can do about them, but please let me know if there's some other way of storing this:
error[E0277]: std::cell::Cell<bool>
cannot be shared between threads safely
--> crates\bevy_winit\src\accessibility.rs:28:10
|
28 | #[derive(Resource, Default, Deref, DerefMut)]
| ^^^^^^^^ std::cell::Cell<bool>
cannot be shared between threads safely
|
= help: within accesskit_windows::subclass::SubclassImpl
, the trait Sync
is not implemented for std::cell::Cell<bool>
= note: required because it appears within the type accesskit_windows::subclass::SubclassImpl
= note: required for Unique<accesskit_windows::subclass::SubclassImpl>
to implement Sync
= note: required because it appears within the type Box<accesskit_windows::subclass::SubclassImpl>
= note: required because it appears within the type accesskit_windows::subclass::SubclassingAdapter
= note: required because it appears within the type accesskit_winit::platform_impl::platform::Adapter
= note: required because it appears within the type Adapter
= note: required because it appears within the type (bevy_window::WindowId, Adapter)
= note: required for RawTable<(bevy_window::WindowId, Adapter)>
to implement Sync
= note: required because it appears within the type bevy_utils::hashbrown::HashMap<bevy_window::WindowId, Adapter>
note: required because it appears within the type Adapters
--> crates\bevy_winit\src\accessibility.rs:29:12
|
29 | pub struct Adapters(pub HashMap<WindowId, Adapter>);
| ^^^^^^^^
= help: see issue #48214
= note: this error originates in the derive macro Resource
(in Nightly builds, run with -Z macro-backtrace for more info)
error[E0277]: std::cell::Cell<Option<Box<(dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)>>>
cannot be shared between threads safely
--> crates\bevy_winit\src\accessibility.rs:28:10
|
28 | #[derive(Resource, Default, Deref, DerefMut)]
| ^^^^^^^^ std::cell::Cell<Option<Box<(dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)>>>
cannot be shared between threads safely
|
= help: within accesskit_windows::subclass::SubclassImpl
, the trait Sync
is not implemented for std::cell::Cell<Option<Box<(dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)>>>
= note: required because it appears within the type once_cell::unsync::Lazy<accesskit_windows::adapter::Adapter, Box<(dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)>>
= note: required because it appears within the type accesskit_windows::subclass::SubclassImpl
= note: required for Unique<accesskit_windows::subclass::SubclassImpl>
to implement Sync
= note: required because it appears within the type Box<accesskit_windows::subclass::SubclassImpl>
= note: required because it appears within the type accesskit_windows::subclass::SubclassingAdapter
= note: required because it appears within the type accesskit_winit::platform_impl::platform::Adapter
= note: required because it appears within the type Adapter
= note: required because it appears within the type (bevy_window::WindowId, Adapter)
= note: required for RawTable<(bevy_window::WindowId, Adapter)>
to implement Sync
= note: required because it appears within the type bevy_utils::hashbrown::HashMap<bevy_window::WindowId, Adapter>
note: required because it appears within the type Adapters
--> crates\bevy_winit\src\accessibility.rs:29:12
|
29 | pub struct Adapters(pub HashMap<WindowId, Adapter>);
| ^^^^^^^^
= help: see issue #48214
= note: this error originates in the derive macro Resource
(in Nightly builds, run with -Z macro-backtrace for more info)
error[E0277]: UnsafeCell<Option<accesskit_windows::adapter::Adapter>>
cannot be shared between threads safely
--> crates\bevy_winit\src\accessibility.rs:28:10
|
28 | #[derive(Resource, Default, Deref, DerefMut)]
| ^^^^^^^^ UnsafeCell<Option<accesskit_windows::adapter::Adapter>>
cannot be shared between threads safely
|
= help: within accesskit_windows::subclass::SubclassImpl
, the trait Sync
is not implemented for UnsafeCell<Option<accesskit_windows::adapter::Adapter>>
= note: required because it appears within the type once_cell::unsync::OnceCell<accesskit_windows::adapter::Adapter>
= note: required because it appears within the type once_cell::unsync::Lazy<accesskit_windows::adapter::Adapter, Box<(dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)>>
= note: required because it appears within the type accesskit_windows::subclass::SubclassImpl
= note: required for Unique<accesskit_windows::subclass::SubclassImpl>
to implement Sync
= note: required because it appears within the type Box<accesskit_windows::subclass::SubclassImpl>
= note: required because it appears within the type accesskit_windows::subclass::SubclassingAdapter
= note: required because it appears within the type accesskit_winit::platform_impl::platform::Adapter
= note: required because it appears within the type Adapter
= note: required because it appears within the type (bevy_window::WindowId, Adapter)
= note: required for RawTable<(bevy_window::WindowId, Adapter)>
to implement Sync
= note: required because it appears within the type bevy_utils::hashbrown::HashMap<bevy_window::WindowId, Adapter>
note: required because it appears within the type Adapters
--> crates\bevy_winit\src\accessibility.rs:29:12
|
29 | pub struct Adapters(pub HashMap<WindowId, Adapter>);
| ^^^^^^^^
= help: see issue #48214
= note: this error originates in the derive macro Resource
(in Nightly builds, run with -Z macro-backtrace for more info)
error[E0277]: (dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)
cannot be sent between threads safely
--> crates\bevy_winit\src\accessibility.rs:28:10
|
28 | #[derive(Resource, Default, Deref, DerefMut)]
| ^^^^^^^^ (dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)
cannot be sent between threads safely
|
= help: the trait Send
is not implemented for (dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)
= note: required for Unique<(dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)>
to implement Send
= note: required because it appears within the type Box<(dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)>
= note: required because it appears within the type Option<Box<(dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)>>
= note: required for std::cell::Cell<Option<Box<(dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)>>>
to implement Send
= note: required because it appears within the type once_cell::unsync::Lazy<accesskit_windows::adapter::Adapter, Box<(dyn FnOnce() -> accesskit_windows::adapter::Adapter + 'static)>>
= note: required because it appears within the type accesskit_windows::subclass::SubclassImpl
= note: required for Unique<accesskit_windows::subclass::SubclassImpl>
to implement Send
= note: required because it appears within the type Box<accesskit_windows::subclass::SubclassImpl>
= note: required because it appears within the type accesskit_windows::subclass::SubclassingAdapter
= note: required because it appears within the type accesskit_winit::platform_impl::platform::Adapter
= note: required because it appears within the type Adapter
= note: required because it appears within the type (bevy_window::WindowId, Adapter)
= note: required for RawTable<(bevy_window::WindowId, Adapter)>
to implement Send
= note: required because it appears within the type bevy_utils::hashbrown::HashMap<bevy_window::WindowId, Adapter>
note: required because it appears within the type Adapters
--> crates\bevy_winit\src\accessibility.rs:29:12
|
29 | pub struct Adapters(pub HashMap<WindowId, Adapter>);
| ^^^^^^^^
= help: see issue #48214
= note: this error originates in the derive macro Resource
(in Nightly builds, run with -Z macro-backtrace for more info)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a known limitation that the AccessKit adapters don't implement Send
, and one that I don't plan on changing in the near future. For me, the question is whether arbitrary Bevy systems, like the ones in the Bevy UI accessibility plugin, really need to access the adapter. Maybe the solution is to add one or more additional commands, like the one that the Bevy UI accessibility plugin already uses to add or update AccessKit nodes.
@ndarilek this is awesome!
I do think that we shouldn't use |
I've introduced a bevy_a11y crate for the AccessKit dependency and non-Winit code, decoupled bevy_ui from bevy_winit, and tried to clean up focus-handling so AccessKit always gets a focus as needed. I've also given the The integration now aggressively pushes all changes to AccessKit each frame. I removed change-detection just to get something working, but this is preliminary to making AccessKit lazily poll for changes so it's only temporary. I'd like to get action handling work next, so access technologies can tell bevy_ui to focus/click widgets. I think I'm miscalculating widget bounding boxes, and would appreciate any help fixing that. I believe Narrator has a tool to draw visual rectangles around where it thinks widgets are, but since I can't see those, I'm not sure if there's a way for me to use that tool independently. Help very welcome. Also, there's probably additional plumbing needed to ensure bevy_a11y is more completely integrated into CI/releases. Help with that is welcome too, particularly if we can address or silence the CI warnings. |
I'm removing the draft status on this, even though it isn't fully ready, because I'd like a bit more attention on it. I don't want to go too far down the rabbit hole of modifying bevy_ui because it needs a lot of work in general, but one limitation of my current approach is that all It may be that this PR has to ship with a reasonably sound AccessKit integration that others can integrate with, but a broken bevy_ui accessibility implementation, and future UI work will have to mold itself to the AccessKit integration and vice versa. But with AccessKit support in the newly-released egui, I'd really like to ship something that bevy_egui and other UI experiments can hook into. Just wanted to prepare folks for the possibility that bevy_ui may be broken for accessibility until it becomes a focus area. |
bors r+ |
# Objective UIs created for Bevy cannot currently be made accessible. This PR aims to address that. ## Solution Integrate AccessKit as a dependency, adding accessibility support to existing bevy_ui widgets. ## Changelog ### Added * Integrate with and expose [AccessKit](https://accesskit.dev) for platform accessibility. * Add `Label` for marking text specifically as a label for UI controls.
Build failed: |
bors r+ |
# Objective UIs created for Bevy cannot currently be made accessible. This PR aims to address that. ## Solution Integrate AccessKit as a dependency, adding accessibility support to existing bevy_ui widgets. ## Changelog ### Added * Integrate with and expose [AccessKit](https://accesskit.dev) for platform accessibility. * Add `Label` for marking text specifically as a label for UI controls.
Excellent, looking forward to building on this!
Agreed. I'd like to suggest that we adopt a subset of AccessKit's schema into Bevy's UI component structs, using it as the basis for our widget source of truth where possible. Essentially, the closer bevy_ui can come to creating NodeBuilders from its components, the fewer explicit accessibility examples we'll need, I feel like the accessibility schema covers much of what you'd need for a UI definition. At the very least, it'd be a solid start. Thanks again for merging this. |
## How This Works For the Bevy 0.10 release blog post (and for the first time ever), I'm publicly opening the doors to other people writing blog post sections. Specifically, if you worked on a feature in a substantial way and are interested in presenting it, you can now ask to claim a section by leaving a comment in this PR. If you claim a section, submit a pull request to the `release-0.10.0` branch in this repo. For the next week, we will be filling in sections (the release target is Saturday March 4th). Please don't claim a section if you don't plan on completing it within that timeline. Also don't claim a section if you weren't an active participant in the design and implementation of the change (unless you are a Maintainer or SME). I will claim any unclaimed sections. Try to match the style of previous release blog posts as much as possible. 1. Show, don't tell. Don't bombard people with information. Avoid large walls of text _and_ large walls of code. Prefer the pattern "byte sized description of one thing" -> "example code/picture/video contextualizing that one thing" -> repeat. Take readers on a journey step by simple step. 2. Don't use up reader's "mental bandwidth" without good reason. We can't afford page-long descriptions of minor bug fixes. If it isn't a "headliner change", keep the description short and sweet. If a change is self describing, let it do that (ex: We now support this new mesh shape primitive ... this is what it looks like). If it is a "headliner change", still try to keep it reasonable. We always have a lot to cover. 3. In slight competition with point (2), don't omit interesting technical information when it is truly fun and engaging. A good chunk of our users are highly technical and enjoy learning how the sausage is made. Try to strike a balance between "terse and simple" and "nerdy details". 4. When relevant, briefly describe the problem being solved first, then describe the solution we chose. This contextualizes the change and gives the feature value and purpose. 5. When possible, provide visuals. They create interest / keep people hooked / break up the monotony. 6. Record images and videos at the default bevy resolution (1280x720) 7. Provide an accurate listing of authors that meaningfully contributed to the feature. Try to sort in order of "contribution scale". This is hard to define, but try to be fair. When in doubt, ask other contributors, SMEs, and/or maintainers. 8. Provide numbers and graphs where possible. If something is faster, use numbers to back it up. We don't (yet) have automated graph generation in blog post style, so send data / info to me (@cart) if you want a graph made. ## Headliners Headliners are our "big ticket high importance / high profile" changes. They are listed briefly at the beginning of the blog post, their entries are roughly sorted "to the top", and they are given priority when it comes to "space in the blog post". If you think we missed something (or didn't prioritize something appropriately), let us know. * ECS Schedule v3 (previously known as "stageless") * Partial Android Support * Depth and Normal Prepass * Environment Map Lighting * Cascaded Shadow Maps * Distance and Atmospheric Fog * Smooth Skeletal Animation Transitions * Enable Parallel Pipelined Rendering * Windows as Entities * Renderer Optimizations * ECS Optimizations ## Sections These are the sections we will cover in the blog post. If a section has been claimed, it will have `(claimed by X)` in the title. If it is unclaimed it will have `(unclaimed)` in the title. Let us know if we missed a section. We don't cover every feature, but we should cover pretty much everything that would be interesting to users. Note that what is interesting or challenging to implement is not necessarily something that is relevant to our blog post readers. And sometimes the reverse is true! If you believe a section should be split up or reorganized, just bring it up here and we can discuss it. ### ~~Schedule V3 (claimed by @alice-i-cecile)~~ * [Migrate engine to Schedule v3][7267] * [Add `bevy_ecs::schedule_v3` module][6587] * [Stageless: fix unapplied systems][7446] * [Stageless: move final apply outside of spawned executor][7445] * Sets * Base Sets * [Base Sets][7466] * Reporting * [Report sets][7756] * [beter cycle reporting][7463] * Run Conditions * [Add condition negation][7559] * [And/Or][7605] * [Add more common run conditions][7579] * States * [States derive macro][7535] * System Piping Flexibility * [Support piping exclusive systems][7023] * [Allow piping run conditions][7547] ### ~~Depth and Normal Prepass (claimed by @IceSentry)~~ * [Add depth and normal prepass][6284] * [Move prepass functions to prepass_utils][7354] ### ~~Distance and Atmospheric Fog (claimed by @coreh)~~ * [Add Distance and Atmospheric Fog support][6412] ### ~~Cascaded Shadow Maps (claimed by @cart)~~ * [Cascaded shadow maps.][7064] * [Better cascades config defaults + builder, tweak example configs][7456] ### ~~Environment Map Lighting (claimed by @cart)~~ * [EnvironmentMapLight, BRDF Improvements][7051] * [Webgl2 support][7737] ### ~~Tonemapping options (claimed by @cart)~~ * [Initial tonemapping options][7594] ### ~~Android support + unification (claimed by @mockersf)~~ * [IOS, Android... same thing][7493] ### ~~Windows as Entities (claimed by @Aceeri)~~ * [Windows as Entities][5589] * [break feedback loop when moving cursor][7298] * [Fix `Window` feedback loop between the OS and Bevy][7517] ### ~~Enable Parallel Pipelined Rendering (claimed by @james7132)~~ * [Pipelined Rendering][6503] * [Stageless: add a method to scope to always run a task on the scope thread][7415] * [Separate Extract from Sub App Schedule][7046] ### ~~Smooth Skeletal Animation Transitions (claimed by @james7132)~~ * [Smooth Transition between Animations][6922] ### ~~Spatial Audio (claimed by @harudagondi)~~ * [Spatial Audio][6028] ### ~~Shader Processor Features (claimed by @cart)~~ * [Shader defs can now have a value][5900] * [Shaders can now have #else ifdef chains][7431] * [Define shader defs in shader][7518] ### ~~Shader Flexibility Improvements (claimed by @cart)~~ * [add ambient lighting hook][5428] * [Refactor Globals and View structs into separate shaders][7512] ### ~~Renderer Optimizations (claimed by @james7132)~~ * [bevy_pbr: Avoid copying structs and using registers in shaders][7069] * [Flatten render commands][6885] * [Replace UUID based IDs with a atomic-counted ones][6988] * [improve compile time by type-erasing wgpu structs][5950] * [Shrink DrawFunctionId][6944] * [Shrink ComputedVisibility][6305] * [Reduce branching in TrackedRenderPass][7053] * [Make PipelineCache internally mutable.][7205] * [Improve `Color::hex` performance][6940] * [Support recording multiple CommandBuffers in RenderContext][7248] * [Parallelized transform propagation][4775] * [Introduce detailed_trace macro, use in TrackedRenderPass][7639] * [Optimize color computation in prepare_uinodes][7311] * [Directly extract joints into SkinnedMeshJoints][6833] * [Parallelize forward kinematics animation systems][6785] * [Move system_commands spans into apply_buffers][6900] * [Reduce the use of atomics in the render phase][7084] ### ~~ECS Optimizations (claimed by @james7132 )~~ * [Remove redundant table and sparse set component IDs from Archetype][4927] * [Immutable sparse sets for metadata storage][4928] * [Replace BlobVec's swap_scratch with a swap_nonoverlapping][4853] * [Use T::Storage::STORAGE_TYPE to optimize out unused branches][6800] * [Remove unnecessary branching from bundle insertion][6902] * [Split Component Ticks][6547] * [use bevy_utils::HashMap for better performance. TypeId is predefined …][7642] * [Extend EntityLocation with TableId and TableRow][6681] * [Basic adaptive batching for parallel quer- [Speed up `CommandQueue` by storing commands more densely][6391]y iteration][4777] ### ~~Reflect Improvements (claimed by @cart)~~ * [bevy_reflect: Add `ReflectFromReflect` (v2)][6245] * [Add reflection support for VecDeque][6831] * [reflect: add `insert` and `remove` methods to `List`][7063] * [Add `remove` method to `Map` reflection trait.][6564] * [bevy_reflect: Fix binary deserialization not working for unit structs][6722] * [Add `TypeRegistrationDeserializer` and remove `BorrowedStr`][7094] * [bevy_reflect: Add simple enum support to reflection paths][6560] * [Enable deriving Reflect on structs with generic types][7364] * [bevy_reflect: Support tuple reflection paths][7324] * [bevy_reflect: Pre-parsed paths][7321] * [bevy_ecs: ReflectComponentFns without World][7206] ### ~~AsBindGroup Improvements (claimed by @cart)~~ * [Support storage buffers in derive `AsBindGroup`][6129] * [Support raw buffers in AsBindGroup][7701] ### ~~Cylinder Shape (claimed by @cart)~~ * [Add cylinder shape][6809] ### ~~Subdividable Plane Shape (claimed by @cart)~~ * [added subdivisions to shape::Plane][7546] ### ~~StandardMaterial Blend Modes (claimed by @coreh)~~ * [Standard Material Blend Modes][6644] ### ~~Configurable Visibility Component (claimed by @cart)~~ * [enum `Visibility` component][6320] ### Task Improvements (claimed by @cart) * [Fix panicking on another scope][6524] * [Add thread create/destroy callbacks to TaskPool][6561] * [Thread executor for running tasks on specific threads.][7087] * [await tasks to cancel][6696] * [Stageless: move MainThreadExecutor to schedule_v3][7444] * [Stageless: close the finish channel so executor doesn't deadlock][7448] ### ~~Upgrade to wgpu 0.15 (claimed by @cart)~~ * [Wgpu 0.15][7356] ### ~~Expose Bindless / Non-uniform Indexing Support (claimed by @cart)~~ * [Request WGPU Capabilities for Non-uniform Indexing][6995] ### ~~Cubic Spline (claimed by @aevyrie)~~ * [Bezier][7653] ### ~~Revamp Bloom (claimed by @JMS55)~~ * [Revamp bloom](bevyengine/bevy#6677) ### ~~Use Prepass Shaders for Shadows (claimed by @superdump)~~ * [use prepass shaders for shadows](bevyengine/bevy#7784) ### ~~AccessKit (claimed by @alice-i-cecile)~~ * [accesskit](bevyengine/bevy#6874) ### ~~Camera Output Modes (claimed by @cart)~~ * [camera output modes](bevyengine/bevy#7671) ### ~~SystemParam Improvements (claimed by @JoJoJet)~~ * [Make the `SystemParam` derive macro more flexible][6694] * [Add a `SystemParam` primitive for deferred mutations; allow `#[derive]`ing more types of SystemParam][6817] ### ~~Gamepad Improvements (claimed by @cart)~~ * [Gamepad events refactor][6965] * [add `Axis::devices` to get all the input devices][5400] ### ~~Input Methods (claimed by @cart)~~ * [add Input Method Editor support][7325] ### ~~Color Improvements (claimed by @cart)~~ * [Add LCH(ab) color space to `bevy_render::color::Color`][7483] * [Add a more familiar hex color entry][7060] ### ~~Split Up CorePlugin (claimed by @cart)~~ * [Break `CorePlugin` into `TaskPoolPlugin`, `TypeRegistrationPlugin`, `FrameCountPlugin`.][7083] ### ~~ExtractComponent Derive (claimed by @cart)~~ * [Extract component derive][7399] ### ~~Added OpenGL and DX11 Backends By Default (claimed by @cart)~~ * [add OpenGL and DX11 backends][7481] ### ~~UnsafeWorldCell (claimed by @BoxyUwU)~~ * [Move all logic to `UnsafeWorldCell`][7381] * [Rename `UnsafeWorldCellEntityRef` to `UnsafeEntityCell`][7568] ### ~~Entity Commands (claimed by @cart)~~ * [Add a trait for commands that run for a given `Entity`][7015] * [Add an extension trait to `EntityCommands` to update hierarchy while preserving `GlobalTransform`][7024] * [Add ReplaceChildren and ClearChildren EntityCommands][6035] ### ~~Iterate EntityRef (claimed by @james7132)~~ * [Allow iterating over with EntityRef over the entire World][6843] ### ~~Ref Queries (@JoJoJet)~~ * [Added Ref to allow immutable access with change detection][7097] ### ~~Taffy Upgrade (claimed by @cart)~~ * [Upgrade to Taffy 0.2][6743] ### ~~Relative Cursor Position (claimed by @cart)~~ * [Relative cursor position][7199] ### ~~Const UI Config (claimed by @cart)~~ * [Add const to methods and const defaults to bevy_ui][5542] ### ~~Examples (claimed by @cart)~~ * [Add pixelated Bevy to assets and an example][6408] * [Organized scene_viewer into plugins for reuse and organization][6936] ### ~~CI Improvements (claimed by @cart)~~ * [add rust-version for MSRV and CI job to check][6852] * [msrv: only send a message on failure during the actual msrv part][7532] * [Make CI friendlier][7398] * [Fix CI welcome message][7428] * [add an action to ask for a migration guide when one is missing][7507] ### ~~SMEs (@cart)~~ This was already covered in another blog post. Just briefly call out what they are and that this is the first release that used them. Link to the other blog post. * [Subject Matter Experts and new Bevy Org docs][7185] [4775]: bevyengine/bevy#4775 [4777]: bevyengine/bevy#4777 [4853]: bevyengine/bevy#4853 [4927]: bevyengine/bevy#4927 [4928]: bevyengine/bevy#4928 [5400]: bevyengine/bevy#5400 [5428]: bevyengine/bevy#5428 [5542]: bevyengine/bevy#5542 [5589]: bevyengine/bevy#5589 [5900]: bevyengine/bevy#5900 [5950]: bevyengine/bevy#5950 [6028]: bevyengine/bevy#6028 [6035]: bevyengine/bevy#6035 [6129]: bevyengine/bevy#6129 [6179]: bevyengine/bevy#6179 [6245]: bevyengine/bevy#6245 [6284]: bevyengine/bevy#6284 [6305]: bevyengine/bevy#6305 [6320]: bevyengine/bevy#6320 [6391]: bevyengine/bevy#6391 [6408]: bevyengine/bevy#6408 [6412]: bevyengine/bevy#6412 [6503]: bevyengine/bevy#6503 [6524]: bevyengine/bevy#6524 [6547]: bevyengine/bevy#6547 [6557]: bevyengine/bevy#6557 [6560]: bevyengine/bevy#6560 [6561]: bevyengine/bevy#6561 [6564]: bevyengine/bevy#6564 [6587]: bevyengine/bevy#6587 [6644]: bevyengine/bevy#6644 [6649]: bevyengine/bevy#6649 [6681]: bevyengine/bevy#6681 [6694]: bevyengine/bevy#6694 [6696]: bevyengine/bevy#6696 [6722]: bevyengine/bevy#6722 [6743]: bevyengine/bevy#6743 [6785]: bevyengine/bevy#6785 [6800]: bevyengine/bevy#6800 [6802]: bevyengine/bevy#6802 [6809]: bevyengine/bevy#6809 [6817]: bevyengine/bevy#6817 [6831]: bevyengine/bevy#6831 [6833]: bevyengine/bevy#6833 [6843]: bevyengine/bevy#6843 [6852]: bevyengine/bevy#6852 [6885]: bevyengine/bevy#6885 [6900]: bevyengine/bevy#6900 [6902]: bevyengine/bevy#6902 [6922]: bevyengine/bevy#6922 [6926]: bevyengine/bevy#6926 [6936]: bevyengine/bevy#6936 [6940]: bevyengine/bevy#6940 [6944]: bevyengine/bevy#6944 [6965]: bevyengine/bevy#6965 [6988]: bevyengine/bevy#6988 [6995]: bevyengine/bevy#6995 [7015]: bevyengine/bevy#7015 [7023]: bevyengine/bevy#7023 [7024]: bevyengine/bevy#7024 [7046]: bevyengine/bevy#7046 [7051]: bevyengine/bevy#7051 [7053]: bevyengine/bevy#7053 [7060]: bevyengine/bevy#7060 [7063]: bevyengine/bevy#7063 [7064]: bevyengine/bevy#7064 [7069]: bevyengine/bevy#7069 [7083]: bevyengine/bevy#7083 [7084]: bevyengine/bevy#7084 [7087]: bevyengine/bevy#7087 [7094]: bevyengine/bevy#7094 [7097]: bevyengine/bevy#7097 [7185]: bevyengine/bevy#7185 [7199]: bevyengine/bevy#7199 [7205]: bevyengine/bevy#7205 [7206]: bevyengine/bevy#7206 [7248]: bevyengine/bevy#7248 [7267]: bevyengine/bevy#7267 [7298]: bevyengine/bevy#7298 [7311]: bevyengine/bevy#7311 [7321]: bevyengine/bevy#7321 [7324]: bevyengine/bevy#7324 [7325]: bevyengine/bevy#7325 [7354]: bevyengine/bevy#7354 [7356]: bevyengine/bevy#7356 [7364]: bevyengine/bevy#7364 [7381]: bevyengine/bevy#7381 [7398]: bevyengine/bevy#7398 [7399]: bevyengine/bevy#7399 [7415]: bevyengine/bevy#7415 [7428]: bevyengine/bevy#7428 [7431]: bevyengine/bevy#7431 [7444]: bevyengine/bevy#7444 [7445]: bevyengine/bevy#7445 [7446]: bevyengine/bevy#7446 [7448]: bevyengine/bevy#7448 [7456]: bevyengine/bevy#7456 [7463]: bevyengine/bevy#7463 [7466]: bevyengine/bevy#7466 [7481]: bevyengine/bevy#7481 [7483]: bevyengine/bevy#7483 [7493]: bevyengine/bevy#7493 [7507]: bevyengine/bevy#7507 [7510]: bevyengine/bevy#7510 [7512]: bevyengine/bevy#7512 [7517]: bevyengine/bevy#7517 [7518]: bevyengine/bevy#7518 [7532]: bevyengine/bevy#7532 [7535]: bevyengine/bevy#7535 [7546]: bevyengine/bevy#7546 [7547]: bevyengine/bevy#7547 [7559]: bevyengine/bevy#7559 [7568]: bevyengine/bevy#7568 [7579]: bevyengine/bevy#7579 [7594]: bevyengine/bevy#7594 [7605]: bevyengine/bevy#7605 [7639]: bevyengine/bevy#7639 [7642]: bevyengine/bevy#7642 [7653]: bevyengine/bevy#7653 [7701]: bevyengine/bevy#7701 [7737]: bevyengine/bevy#7737 [7756]: bevyengine/bevy#7756 Co-authored-by: François <[email protected]> Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: Mike <[email protected]> Co-authored-by: Boxy <[email protected]> Co-authored-by: IceSentry <[email protected]> Co-authored-by: JoJoJet <[email protected]> Co-authored-by: Aevyrie <[email protected]> Co-authored-by: James Liu <[email protected]> Co-authored-by: Marco Buono <[email protected]> Co-authored-by: Aceeri <[email protected]>
Before you folks go doing something you might later regret, it would be great if there was some objective criteria of how many dependencies are too many. Crates that depend on other smaller crates (mainly smol-rs ones in case of zbus unless tokio is enabled) instead of re-implementing everything internally, also bring some advantages, such as faster parallel build, significantly reduced hot build time, lower binary size in general (depends on how (un)lucky you are), and most importantly IMO, just being a good Open Source citizen. So just looking at the number of deps alone is not a good idea. A better criteria would be the total binary size of all the deps combined.
I appreciate that you don't want to complain but instead of putting pressure, maybe another option would be to work with zbus maintainers (currently it's just me, working on it in my spare time) to address the issues at hand?
If dbus-rs was a great API, I wouldn't have gone down the rabbit hole of writing zbus. If that works better for you, by all means go for it! Just keep in mind that portability wouldn't be as easy then.
Let me tell you a bit of a story. After I gave presentation on zbus (before the first release) in late 2019, a lot of people got this idea that D-Bus can't be that hard somehow (even though I said the opposite) and many D-Bus crates popped up. Many of them didn't go anywhere at all but some of them got to some point where they were just good enough for the very specific needs of the author. However, slowly they all just died out. Never got to the point of being generally useful. So if that's the route you want to take, that's entirely up to you of course but keep in mind, it's going to be a lot of work. You can reduce the work immensely if you use zvariant but I believe if the output of
That's w/o optimizations, isn't it? With some optimizations, it can be reduced by half.
Have you found a more reliable method that |
@zeenix You're right, that's a better way for me to approach this. And I appreciate all the work you've done on zbus in your spare time. I want to take a closer look at the state of dbus-rs though. AccessKit doesn't have to statically link the platforms' IPC mechanisms on Windows or macOS. So I think the only way to achieve a comparable (lack of) bloat for the free desktops is to use the D-Bus C library, as we use the equivalent C or Objective-C APIs on Windows or macOS. The portability complications that come with using a C library are definitely something to consider, though. What would the Bevy project prefer? |
Fair enough but if avoiding bloat through dynamically linking to libs is what you're aiming for, you may want to reconsider using Cargo at all tbh. |
Avoiding bloat as an app developer with cargo can be difficult, but avoiding bloat as a lib developer is entirely doable. The difference Matt is alluding to however is that on OSX, Windows, iOS and I believe also Android, the accessibility mechanisms are IPC built into the OS fundamentally. Linux is the weird outlier since it doesn't even necessarily have a desktop environment, "native" widgets are only a thing by convention, and accessibility isn't really a super high priority beyond specific circles. I personally think bevy should opt to not care and turn it on by default for Linux, but I am blind so I guess I'm biased, and starting optional at least gets it out the door. I think the more important point is that no one is going to turn accessibility on unless someone figures out how to get as far as the devs and also the app is still maintained the first time someone with insert-disability needs to use it. Since this is off at the "we didn't compile it in" level, there's nothing to be done for any app that doesn't fulfill the full set of conditions. A megabyte or so increased binary size isn't great but to me it's a lot better than subsets of users who could otherwise have used a thing without intervention needing to advocate on a per-app basis. |
@cart (and the rest of the Bevy core team): How much do we need to reduce the binary footprint of the AccessKit Unix adapter before it can be enabled by default, or ideally not feature-gated at all? If the answer is that the Unix adapter shouldn't be much larger than the Windows or macOS adapter, then I think the only way we can achieve that is by using C libraries (e.g. libdbus or GLib) where we currently use pure Rust (zbus). Which is the lesser evil, increased binary size or more C dependencies? And for the added C dependencies to meaningfully reduce binary size, they have to be installed system-wide. |
For me, "added C dependencies" is much much worse. My personal feeling is that "as reduced as reasonably possible" is the bar here: the goal is to reduce footprint where there's low hanging fruit. |
Just to make one thing very clear as I think some people have missed aspects of the discussion above due to GitHub pull request UI being really good at hiding discussions: My end criteria for this effort is for accessible UI to be enabled by default on every platform. The only question at this point is how. The current "feature flag on linux" is a short term middle ground solution while we sort out the best way to handle this situation.
I've already outlined my stance on this above and touched on the same pros and cons you discussed here. I can't really be more specific than "60 new deps is almost certainly way too much" and "17 is closer to being an acceptable number" without really diving into the details. I agree that dep count is a complicated topic with many dimensions to it. I'll call out that with the alternative being discussed (binding to a c lib), binary size and build time would both go way down. So I don't see "increased parallelism due to high dep count" as a particularly good selling point in this context when the alternative is "4 new tiny deps". I'll also note that we do value being a Good Open Source Citizen in Bevy land. We aren't dependency averse to the extent of some other projects in the space (such as Macroquad ... but I'll note that they have signficantly better build times and binary sizes than we do because of their rigorous dep-aversion / lack-of-ecosystem-participation). We take targeted ecosystem dependencies in the interest of sharing effort / being good ecosystem players. But we strike a balance with keeping our developer UX in check. See my stance linked above on this. If we take a laissez-faire attitude to deps, this will bite us in a multitude of ways. To summarize again: binary size (in many cases), compile times (in many cases), increased chance of "illegal" semver breakage, increased chance of mismatched dep versions, increased threat surface, decreased user perception of "slimness" ... for better or worse people will look at dep counts when forming their opinions, and increased licensing compliance burden / aggregate license file size for deployed apps.
I'm very open to this! Some thoughts:
Linking to c libs isn't inherently bad for portability, it just can be bad. We already link to a huge number of them via deps like wgpu, winit, cpal, gilrs, etc. I think this really depends on the specifics of library availablility on systems. I don't have an answer to this problem space. I'll be honest, if its not clear yet: I'm pretty biased toward pursuing the c lib linking route here (within the context of Bevy specifically). Alternatively, if anyone wants to do a breakdown of present or future zlib impls that demonstrate that the "dep cost" (in the dimensions I listed above) is competitive with whatever c-lib-linking-impl we would use, I'm on board. Make your case with numbers and I'll listen. I'm also curious to see how hard it would be to make accesskit support either zbus or dbus-rs / some other clib option (via cargo config). That would make comparisons easier and it would make it easier to migrate whenever we're happy with tradeoffs. Before making any final decision we need to compare the actual "dep-related concern" numbers and gather data on system compatibility.
This is certainly a hard call, but from my perspective if we enabled the current impl by default right now we would:
I don't think that would be the correct "optimal solve" for this situation. I'm confident that if we continue to hold our requirement bar reasonably high, that we can meet those requirements in relatively short order. I really don't want to be the "bad guy" here, but I consider myself responsible to all parties in the Bevy community, not just the ones in this thread currently. I'm doing that in the best way I know how. Please let me know if you think I'm not doing that job effectively. |
I can't stop people in general making the mistake of shallow assessments but if we want to make a fair assessment, we need to pay attention to specific details. E.g we needed a broadcast channel API in zbus. I could have just implemented it in zbus itself and that would have looked like a win from your perspective as it would have meant one less dependency. Instead I decided to revive an unfinished smol-rs crate so others can use it too. Another example of the same is ordered-stream crate. We didn't really need to split it into another crate. Many of ours deps are part of the same project, which are split into multiple smaller crates so people can only link the API they need. This means (among other things) there is no increased license compliance burden as all the crates under the same project are under the same license and no increased risk of MSRV bumps etc. BTW, I noticed that you also seem to like splitting your project into multiple smaller crates rather than keeping a monolith? People can raise the same point against bevy use (directly or indirectly) if they decide based primarily on number of dependencies added.
I wasn't saying that in the context of zbus vs dbus-rs, that would be apple and oranges in this case. There are other reasons (that I mentioned) to prefer zbus over dbus-rs. My point was that if zbus had fewer deps by 1. implementing things internally and 2. depending on monolithic crates, we'd be seen as heroes to people who just look at the number of deps but it would have meant (among other things), decreased build parallelism.
I did not say that it's inherently bad. All I can tell you is that I've had multiple people come to me about being unable to cross build their zbus-using code and then it turns out it was because they forgot to remove Don't know how much you know about D-Bus at all but the C library in question here is notoriously bad and undermaintained. I don't know any C app/lib that actually uses it. They all use gio (which is what inspired zbus coincidentally). Anyway. I think I've put enough of my energy here. It's your project so leave it in your capable hands to make the decision. I provided all the information I could provide so my job here is done. I will now unsubscribe from this but if you have any zbus (or D-Bus) questions, you're more than welcome to contact me either through our our matrix channel or the issue tracker but please don't |
Given the state of the libdbus C library, which was also made clear to me in a chat in the GNOME Accessibility matrix room last weekend, I'm ruling out that option for AccessKit. According to my measurements, the executable footprint of zbus increased sharply with zbus 2, when the implementation was converted from blocking to async. Of course, this change also added several new dependencies. So I'm thinking about forking zbus 1 to start a new project with the explicit goal of keeping executable size and dependency count low. Of course, this is going to lead to some duplicated effort to reimplement bug fixes and possibly some features that were already implemented in zbus 2.x and 3.x. But it may be the quickest way out of the current situation. |
This would backfire and have the opposite result for many applications which would still need zbus for other purposes. |
There's one thing Zeeshan Ali said about the libdbus C library (used by the dbus crate) that's not entirely accurate. It is in fact used by some applications. In particular, the Orca screen reader uses it (via the libatspi library), and all GTK 3 applications use it when accessibility is enabled (again via libatspi, and atk-bridge). While the GNOME accessibility stack doesn't exactly have a reputation for robustness, I don't know if any actual GNOME accessibility bugs or general instability are due to problems with libdbus. So I'm reconsidering the option of using libdbus via the dbus crate. |
Another complication with the option to use libdbus via the dbus crate: The D-Bus reference implementation, including libdbus, is dual-licensed under GPL 2 and Academic Free License (AFL) 2.1. My understanding is that AFL 2.1 is a permissive license similar to the Apache license, but still, it's another license. And this info isn't currently reflected in the metadata of the dbus or libdbus-sys crate. |
# Objective Noticed cargo-deny was failing on bevyengine#6874 because of toml_edit. ## Solution Update toml_edit to 0.19.
# Objective UIs created for Bevy cannot currently be made accessible. This PR aims to address that. ## Solution Integrate AccessKit as a dependency, adding accessibility support to existing bevy_ui widgets. ## Changelog ### Added * Integrate with and expose [AccessKit](https://accesskit.dev) for platform accessibility. * Add `Label` for marking text specifically as a label for UI controls.
Objective
UIs created for Bevy cannot currently be made accessible. This PR aims to address that.
Solution
Integrate AccessKit as a dependency, adding accessibility support to existing bevy_ui widgets.
Changelog
Added
Label
for marking text specifically as a label for UI controls.