Skip to content

Commit 770f10b

Browse files
joshua-holmeschescockmockersf
authored
Remove remaining internal use of !Send resources (#18386)
# Objective Remaining work for and closes #17682. First half of work for that issue was completed in [PR 17730](#17730). However, the rest of the work ended up getting blocked because we needed a way of forcing systems to run on the main thread without the use of `!Send` resources. That was unblocked by [PR 18301](#18301). This work should finish unblocking the resources-as-components effort. # Testing Ran several examples using my Linux machine, just to make sure things are working as expected and no surprises pop up. --------- Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com> Co-authored-by: François Mockers <francois.mockers@vleue.com>
1 parent 7b1c9f1 commit 770f10b

File tree

6 files changed

+700
-632
lines changed

6 files changed

+700
-632
lines changed

crates/bevy_gilrs/src/lib.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,22 @@ impl Plugin for GilrsPlugin {
117117
}
118118
}
119119
}
120+
121+
#[cfg(test)]
122+
mod tests {
123+
use super::*;
124+
125+
// Regression test for https://github.com/bevyengine/bevy/issues/17697
126+
#[test]
127+
fn world_is_truly_send() {
128+
let mut app = App::new();
129+
app.add_plugins(GilrsPlugin);
130+
let world = core::mem::take(app.world_mut());
131+
132+
let handler = std::thread::spawn(move || {
133+
drop(world);
134+
});
135+
136+
handler.join().unwrap();
137+
}
138+
}

crates/bevy_winit/src/accessibility.rs

Lines changed: 52 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
33
use alloc::{collections::VecDeque, sync::Arc};
44
use bevy_input_focus::InputFocus;
5+
use core::cell::RefCell;
56
use std::sync::Mutex;
67
use winit::event_loop::ActiveEventLoop;
78

@@ -16,13 +17,26 @@ use bevy_a11y::{
1617
};
1718
use bevy_app::{App, Plugin, PostUpdate};
1819
use bevy_derive::{Deref, DerefMut};
19-
use bevy_ecs::{entity::EntityHashMap, prelude::*};
20+
use bevy_ecs::{entity::EntityHashMap, prelude::*, system::NonSendMarker};
2021
use bevy_window::{PrimaryWindow, Window, WindowClosed};
2122

23+
thread_local! {
24+
/// Temporary storage of access kit adapter data to replace usage of `!Send` resources. This will be replaced with proper
25+
/// storage of `!Send` data after issue #17667 is complete.
26+
pub static ACCESS_KIT_ADAPTERS: RefCell<AccessKitAdapters> = const { RefCell::new(AccessKitAdapters::new()) };
27+
}
28+
2229
/// Maps window entities to their `AccessKit` [`Adapter`]s.
2330
#[derive(Default, Deref, DerefMut)]
2431
pub struct AccessKitAdapters(pub EntityHashMap<Adapter>);
2532

33+
impl AccessKitAdapters {
34+
/// Creates a new empty `AccessKitAdapters`.
35+
pub const fn new() -> Self {
36+
Self(EntityHashMap::new())
37+
}
38+
}
39+
2640
/// Maps window entities to their respective [`ActionRequest`]s.
2741
#[derive(Resource, Default, Deref, DerefMut)]
2842
pub struct WinitActionRequestHandlers(pub EntityHashMap<Arc<Mutex<WinitActionRequestHandler>>>);
@@ -144,14 +158,16 @@ pub(crate) fn prepare_accessibility_for_window(
144158
}
145159

146160
fn window_closed(
147-
mut adapters: NonSendMut<AccessKitAdapters>,
148161
mut handlers: ResMut<WinitActionRequestHandlers>,
149162
mut events: EventReader<WindowClosed>,
163+
_non_send_marker: NonSendMarker,
150164
) {
151-
for WindowClosed { window, .. } in events.read() {
152-
adapters.remove(window);
153-
handlers.remove(window);
154-
}
165+
ACCESS_KIT_ADAPTERS.with_borrow_mut(|adapters| {
166+
for WindowClosed { window, .. } in events.read() {
167+
adapters.remove(window);
168+
handlers.remove(window);
169+
}
170+
});
155171
}
156172

157173
fn poll_receivers(
@@ -174,7 +190,6 @@ fn should_update_accessibility_nodes(
174190
}
175191

176192
fn update_accessibility_nodes(
177-
mut adapters: NonSendMut<AccessKitAdapters>,
178193
focus: Option<Res<InputFocus>>,
179194
primary_window: Query<(Entity, &Window), With<PrimaryWindow>>,
180195
nodes: Query<(
@@ -184,35 +199,38 @@ fn update_accessibility_nodes(
184199
Option<&ChildOf>,
185200
)>,
186201
node_entities: Query<Entity, With<AccessibilityNode>>,
202+
_non_send_marker: NonSendMarker,
187203
) {
188-
let Ok((primary_window_id, primary_window)) = primary_window.single() else {
189-
return;
190-
};
191-
let Some(adapter) = adapters.get_mut(&primary_window_id) else {
192-
return;
193-
};
194-
let Some(focus) = focus else {
195-
return;
196-
};
197-
if focus.is_changed() || !nodes.is_empty() {
198-
// Don't panic if the focused entity does not currently exist
199-
// It's probably waiting to be spawned
200-
if let Some(focused_entity) = focus.0 {
201-
if !node_entities.contains(focused_entity) {
202-
return;
204+
ACCESS_KIT_ADAPTERS.with_borrow_mut(|adapters| {
205+
let Ok((primary_window_id, primary_window)) = primary_window.single() else {
206+
return;
207+
};
208+
let Some(adapter) = adapters.get_mut(&primary_window_id) else {
209+
return;
210+
};
211+
let Some(focus) = focus else {
212+
return;
213+
};
214+
if focus.is_changed() || !nodes.is_empty() {
215+
// Don't panic if the focused entity does not currently exist
216+
// It's probably waiting to be spawned
217+
if let Some(focused_entity) = focus.0 {
218+
if !node_entities.contains(focused_entity) {
219+
return;
220+
}
203221
}
204-
}
205222

206-
adapter.update_if_active(|| {
207-
update_adapter(
208-
nodes,
209-
node_entities,
210-
primary_window,
211-
primary_window_id,
212-
focus,
213-
)
214-
});
215-
}
223+
adapter.update_if_active(|| {
224+
update_adapter(
225+
nodes,
226+
node_entities,
227+
primary_window,
228+
primary_window_id,
229+
focus,
230+
)
231+
});
232+
}
233+
});
216234
}
217235

218236
fn update_adapter(
@@ -290,8 +308,7 @@ pub struct AccessKitPlugin;
290308

291309
impl Plugin for AccessKitPlugin {
292310
fn build(&self, app: &mut App) {
293-
app.init_non_send_resource::<AccessKitAdapters>()
294-
.init_resource::<WinitActionRequestHandlers>()
311+
app.init_resource::<WinitActionRequestHandlers>()
295312
.add_event::<ActionRequestWrapper>()
296313
.add_systems(
297314
PostUpdate,

crates/bevy_winit/src/lib.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use bevy_derive::Deref;
1818
use bevy_reflect::prelude::ReflectDefault;
1919
use bevy_reflect::Reflect;
2020
use bevy_window::{RawHandleWrapperHolder, WindowEvent};
21+
use core::cell::RefCell;
2122
use core::marker::PhantomData;
2223
use winit::{event_loop::EventLoop, window::WindowId};
2324

@@ -37,7 +38,7 @@ pub use winit_config::*;
3738
pub use winit_windows::*;
3839

3940
use crate::{
40-
accessibility::{AccessKitAdapters, AccessKitPlugin, WinitActionRequestHandlers},
41+
accessibility::{AccessKitPlugin, WinitActionRequestHandlers},
4142
state::winit_runner,
4243
winit_monitors::WinitMonitors,
4344
};
@@ -53,6 +54,10 @@ mod winit_config;
5354
mod winit_monitors;
5455
mod winit_windows;
5556

57+
thread_local! {
58+
static WINIT_WINDOWS: RefCell<WinitWindows> = const { RefCell::new(WinitWindows::new()) };
59+
}
60+
5661
/// A [`Plugin`] that uses `winit` to create and manage windows, and receive window and input
5762
/// events.
5863
///
@@ -124,8 +129,7 @@ impl<T: Event> Plugin for WinitPlugin<T> {
124129
.build()
125130
.expect("Failed to build event loop");
126131

127-
app.init_non_send_resource::<WinitWindows>()
128-
.init_resource::<WinitMonitors>()
132+
app.init_resource::<WinitMonitors>()
129133
.init_resource::<WinitSettings>()
130134
.insert_resource(DisplayHandleWrapper(event_loop.owned_display_handle()))
131135
.add_event::<RawWinitWindowEvent>()
@@ -210,8 +214,6 @@ pub type CreateWindowParams<'w, 's, F = ()> = (
210214
F,
211215
>,
212216
EventWriter<'w, WindowCreated>,
213-
NonSendMut<'w, WinitWindows>,
214-
NonSendMut<'w, AccessKitAdapters>,
215217
ResMut<'w, WinitActionRequestHandlers>,
216218
Res<'w, AccessibilityRequested>,
217219
Res<'w, WinitMonitors>,

0 commit comments

Comments
 (0)