Skip to content

Commit

Permalink
Replace some !Send resources with thread_local! (#17730)
Browse files Browse the repository at this point in the history
# Objective

Work for issue #17682 

What's in this PR:
* Removal of some `!Send` resources that Bevy uses internally
* Replaces `!Send` resources with `thread_local!` static

What this PR does not cover:
* The ability to create `!Send` resources still exists
* Tests that test `!Send` resources are present (and should not be
removed until the ability to create `!Send` resources is removed)
* The example `log_layers_ecs` still uses a `!Send` resource. In this
example, removing the `!Send` resource results in the system that uses
it running on a thread other than the main thread, which doesn't work
with lazily initialized `thread_local!` static data. Removing this
`!Send` resource will need to be deferred until the System API is
extended to support configuring which thread the System runs on. Once an
issue for this work is created, it will be mentioned in #17667

Once the System API is extended to allow control of which thread the
System runs on, the rest of the `!Send` resources can be removed in a
different PR.
  • Loading branch information
joshua-holmes authored Mar 4, 2025
1 parent 17e300b commit 7b021b4
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 139 deletions.
170 changes: 85 additions & 85 deletions crates/bevy_gilrs/src/gilrs_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ use crate::{
};
use bevy_ecs::event::EventWriter;
use bevy_ecs::prelude::Commands;
#[cfg(target_arch = "wasm32")]
use bevy_ecs::system::NonSendMut;
use bevy_ecs::system::ResMut;
use bevy_input::gamepad::{
GamepadConnection, GamepadConnectionEvent, RawGamepadAxisChangedEvent,
Expand All @@ -15,101 +13,103 @@ use gilrs::{ev::filter::axis_dpad_to_button, EventType, Filter};

pub fn gilrs_event_startup_system(
mut commands: Commands,
#[cfg(target_arch = "wasm32")] mut gilrs: NonSendMut<Gilrs>,
#[cfg(not(target_arch = "wasm32"))] mut gilrs: ResMut<Gilrs>,
mut gilrs: ResMut<Gilrs>,
mut gamepads: ResMut<GilrsGamepads>,
mut events: EventWriter<GamepadConnectionEvent>,
) {
for (id, gamepad) in gilrs.0.get().gamepads() {
// Create entity and add to mapping
let entity = commands.spawn_empty().id();
gamepads.id_to_entity.insert(id, entity);
gamepads.entity_to_id.insert(entity, id);

events.write(GamepadConnectionEvent {
gamepad: entity,
connection: GamepadConnection::Connected {
name: gamepad.name().to_string(),
vendor_id: gamepad.vendor_id(),
product_id: gamepad.product_id(),
},
});
}
gilrs.with(|gilrs| {
for (id, gamepad) in gilrs.gamepads() {
// Create entity and add to mapping
let entity = commands.spawn_empty().id();
gamepads.id_to_entity.insert(id, entity);
gamepads.entity_to_id.insert(entity, id);
events.write(GamepadConnectionEvent {
gamepad: entity,
connection: GamepadConnection::Connected {
name: gamepad.name().to_string(),
vendor_id: gamepad.vendor_id(),
product_id: gamepad.product_id(),
},
});
}
});
}

pub fn gilrs_event_system(
mut commands: Commands,
#[cfg(target_arch = "wasm32")] mut gilrs: NonSendMut<Gilrs>,
#[cfg(not(target_arch = "wasm32"))] mut gilrs: ResMut<Gilrs>,
mut gilrs: ResMut<Gilrs>,
mut gamepads: ResMut<GilrsGamepads>,
mut events: EventWriter<RawGamepadEvent>,
mut connection_events: EventWriter<GamepadConnectionEvent>,
mut button_events: EventWriter<RawGamepadButtonChangedEvent>,
mut axis_event: EventWriter<RawGamepadAxisChangedEvent>,
) {
let gilrs = gilrs.0.get();
while let Some(gilrs_event) = gilrs.next_event().filter_ev(&axis_dpad_to_button, gilrs) {
gilrs.update(&gilrs_event);
match gilrs_event.event {
EventType::Connected => {
let pad = gilrs.gamepad(gilrs_event.id);
let entity = gamepads.get_entity(gilrs_event.id).unwrap_or_else(|| {
let entity = commands.spawn_empty().id();
gamepads.id_to_entity.insert(gilrs_event.id, entity);
gamepads.entity_to_id.insert(entity, gilrs_event.id);
entity
});

let event = GamepadConnectionEvent::new(
entity,
GamepadConnection::Connected {
name: pad.name().to_string(),
vendor_id: pad.vendor_id(),
product_id: pad.product_id(),
},
);
gilrs.with(|gilrs| {
while let Some(gilrs_event) = gilrs.next_event().filter_ev(&axis_dpad_to_button, gilrs) {
gilrs.update(&gilrs_event);
match gilrs_event.event {
EventType::Connected => {
let pad = gilrs.gamepad(gilrs_event.id);
let entity = gamepads.get_entity(gilrs_event.id).unwrap_or_else(|| {
let entity = commands.spawn_empty().id();
gamepads.id_to_entity.insert(gilrs_event.id, entity);
gamepads.entity_to_id.insert(entity, gilrs_event.id);
entity
});

events.write(event.clone().into());
connection_events.write(event);
}
EventType::Disconnected => {
let gamepad = gamepads
.id_to_entity
.get(&gilrs_event.id)
.copied()
.expect("mapping should exist from connection");
let event = GamepadConnectionEvent::new(gamepad, GamepadConnection::Disconnected);
events.write(event.clone().into());
connection_events.write(event);
}
EventType::ButtonChanged(gilrs_button, raw_value, _) => {
let Some(button) = convert_button(gilrs_button) else {
continue;
};
let gamepad = gamepads
.id_to_entity
.get(&gilrs_event.id)
.copied()
.expect("mapping should exist from connection");
events.write(RawGamepadButtonChangedEvent::new(gamepad, button, raw_value).into());
button_events.write(RawGamepadButtonChangedEvent::new(
gamepad, button, raw_value,
));
}
EventType::AxisChanged(gilrs_axis, raw_value, _) => {
let Some(axis) = convert_axis(gilrs_axis) else {
continue;
};
let gamepad = gamepads
.id_to_entity
.get(&gilrs_event.id)
.copied()
.expect("mapping should exist from connection");
events.write(RawGamepadAxisChangedEvent::new(gamepad, axis, raw_value).into());
axis_event.write(RawGamepadAxisChangedEvent::new(gamepad, axis, raw_value));
}
_ => (),
};
}
gilrs.inc();
let event = GamepadConnectionEvent::new(
entity,
GamepadConnection::Connected {
name: pad.name().to_string(),
vendor_id: pad.vendor_id(),
product_id: pad.product_id(),
},
);
events.write(event.clone().into());
connection_events.write(event);
}
EventType::Disconnected => {
let gamepad = gamepads
.id_to_entity
.get(&gilrs_event.id)
.copied()
.expect("mapping should exist from connection");
let event =
GamepadConnectionEvent::new(gamepad, GamepadConnection::Disconnected);
events.write(event.clone().into());
connection_events.write(event);
}
EventType::ButtonChanged(gilrs_button, raw_value, _) => {
let Some(button) = convert_button(gilrs_button) else {
continue;
};
let gamepad = gamepads
.id_to_entity
.get(&gilrs_event.id)
.copied()
.expect("mapping should exist from connection");
events.write(
RawGamepadButtonChangedEvent::new(gamepad, button, raw_value).into(),
);
button_events.write(RawGamepadButtonChangedEvent::new(
gamepad, button, raw_value,
));
}
EventType::AxisChanged(gilrs_axis, raw_value, _) => {
let Some(axis) = convert_axis(gilrs_axis) else {
continue;
};
let gamepad = gamepads
.id_to_entity
.get(&gilrs_event.id)
.copied()
.expect("mapping should exist from connection");
events.write(RawGamepadAxisChangedEvent::new(gamepad, axis, raw_value).into());
axis_event.write(RawGamepadAxisChangedEvent::new(gamepad, axis, raw_value));
}
_ => (),
};
}
gilrs.inc();
});
}
46 changes: 40 additions & 6 deletions crates/bevy_gilrs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,48 @@ mod converter;
mod gilrs_system;
mod rumble;

#[cfg(not(target_arch = "wasm32"))]
use bevy_utils::synccell::SyncCell;

#[cfg(target_arch = "wasm32")]
use core::cell::RefCell;

use bevy_app::{App, Plugin, PostUpdate, PreStartup, PreUpdate};
use bevy_ecs::entity::hash_map::EntityHashMap;
use bevy_ecs::prelude::*;
use bevy_input::InputSystem;
use bevy_platform_support::collections::HashMap;
use bevy_utils::synccell::SyncCell;
use gilrs::GilrsBuilder;
use gilrs_system::{gilrs_event_startup_system, gilrs_event_system};
use rumble::{play_gilrs_rumble, RunningRumbleEffects};
use tracing::error;

#[cfg_attr(not(target_arch = "wasm32"), derive(Resource))]
pub(crate) struct Gilrs(pub SyncCell<gilrs::Gilrs>);
#[cfg(target_arch = "wasm32")]
thread_local! {
/// Temporary storage of gilrs data to replace usage of `!Send` resources. This will be replaced with proper
/// storage of `!Send` data after issue #17667 is complete.
///
/// Using a `thread_local!` here relies on the fact that wasm32 can only be single threaded. Previously, we used a
/// `NonSendMut` parameter, which told Bevy that the system was `!Send`, but now with the removal of `!Send`
/// resource/system parameter usage, there is no internal guarantee that the system will run in only one thread, so
/// we need to rely on the platform to make such a guarantee.
static GILRS: RefCell<Option<gilrs::Gilrs>> = const { RefCell::new(None) };
}

#[derive(Resource)]
pub(crate) struct Gilrs {
#[cfg(not(target_arch = "wasm32"))]
cell: SyncCell<gilrs::Gilrs>,
}
impl Gilrs {
#[inline]
pub fn with(&mut self, f: impl FnOnce(&mut gilrs::Gilrs)) {
#[cfg(target_arch = "wasm32")]
GILRS.with(|g| f(g.borrow_mut().as_mut().expect("GILRS was not initialized")));
#[cfg(not(target_arch = "wasm32"))]
f(self.cell.get());
}
}

/// A [`resource`](Resource) with the mapping of connected [`gilrs::GamepadId`] and their [`Entity`].
#[derive(Debug, Default, Resource)]
Expand Down Expand Up @@ -65,10 +94,15 @@ impl Plugin for GilrsPlugin {
.build()
{
Ok(gilrs) => {
let g = Gilrs {
#[cfg(not(target_arch = "wasm32"))]
cell: SyncCell::new(gilrs),
};
#[cfg(target_arch = "wasm32")]
app.insert_non_send_resource(Gilrs(SyncCell::new(gilrs)));
#[cfg(not(target_arch = "wasm32"))]
app.insert_resource(Gilrs(SyncCell::new(gilrs)));
GILRS.with(|g| {
g.replace(Some(gilrs));
});
app.insert_resource(g);
app.init_resource::<GilrsGamepads>();
app.init_resource::<RunningRumbleEffects>()
.add_systems(PreStartup, gilrs_event_startup_system)
Expand Down
64 changes: 31 additions & 33 deletions crates/bevy_gilrs/src/rumble.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
//! Handle user specified rumble request events.
use crate::{Gilrs, GilrsGamepads};
use bevy_ecs::prelude::{EventReader, Res, ResMut, Resource};
#[cfg(target_arch = "wasm32")]
use bevy_ecs::system::NonSendMut;
use bevy_input::gamepad::{GamepadRumbleIntensity, GamepadRumbleRequest};
use bevy_platform_support::collections::HashMap;
use bevy_time::{Real, Time};
Expand Down Expand Up @@ -128,42 +126,42 @@ fn handle_rumble_request(
}
pub(crate) fn play_gilrs_rumble(
time: Res<Time<Real>>,
#[cfg(target_arch = "wasm32")] mut gilrs: NonSendMut<Gilrs>,
#[cfg(not(target_arch = "wasm32"))] mut gilrs: ResMut<Gilrs>,
mut gilrs: ResMut<Gilrs>,
gamepads: Res<GilrsGamepads>,
mut requests: EventReader<GamepadRumbleRequest>,
mut running_rumbles: ResMut<RunningRumbleEffects>,
) {
let gilrs = gilrs.0.get();
let current_time = time.elapsed();
// Remove outdated rumble effects.
for rumbles in running_rumbles.rumbles.values_mut() {
// `ff::Effect` uses RAII, dropping = deactivating
rumbles.retain(|RunningRumble { deadline, .. }| *deadline >= current_time);
}
running_rumbles
.rumbles
.retain(|_gamepad, rumbles| !rumbles.is_empty());

// Add new effects.
for rumble in requests.read().cloned() {
let gamepad = rumble.gamepad();
match handle_rumble_request(&mut running_rumbles, gilrs, &gamepads, rumble, current_time) {
Ok(()) => {}
Err(RumbleError::GilrsError(err)) => {
if let ff::Error::FfNotSupported(_) = err {
debug!("Tried to rumble {gamepad:?}, but it doesn't support force feedback");
} else {
warn!(
"Tried to handle rumble request for {gamepad:?} but an error occurred: {err}"
);
gilrs.with(|gilrs| {
let current_time = time.elapsed();
// Remove outdated rumble effects.
for rumbles in running_rumbles.rumbles.values_mut() {
// `ff::Effect` uses RAII, dropping = deactivating
rumbles.retain(|RunningRumble { deadline, .. }| *deadline >= current_time);
}
running_rumbles
.rumbles
.retain(|_gamepad, rumbles| !rumbles.is_empty());

// Add new effects.
for rumble in requests.read().cloned() {
let gamepad = rumble.gamepad();
match handle_rumble_request(&mut running_rumbles, gilrs, &gamepads, rumble, current_time) {
Ok(()) => {}
Err(RumbleError::GilrsError(err)) => {
if let ff::Error::FfNotSupported(_) = err {
debug!("Tried to rumble {gamepad:?}, but it doesn't support force feedback");
} else {
warn!(
"Tried to handle rumble request for {gamepad:?} but an error occurred: {err}"
);
}
}
}
Err(RumbleError::GamepadNotFound) => {
warn!("Tried to handle rumble request {gamepad:?} but it doesn't exist!");
}
};
}
Err(RumbleError::GamepadNotFound) => {
warn!("Tried to handle rumble request {gamepad:?} but it doesn't exist!");
}
};
}
});
}

#[cfg(test)]
Expand Down
14 changes: 5 additions & 9 deletions crates/bevy_winit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,15 @@ impl<T: Event> Plugin for WinitPlugin<T> {
event_loop_builder.with_android_app(bevy_window::ANDROID_APP.get().expect(msg).clone());
}

let event_loop = event_loop_builder
.build()
.expect("Failed to build event loop");

app.init_non_send_resource::<WinitWindows>()
.init_resource::<WinitMonitors>()
.init_resource::<WinitSettings>()
.add_event::<RawWinitWindowEvent>()
.set_runner(winit_runner::<T>)
.set_runner(|app| winit_runner(app, event_loop))
.add_systems(
Last,
(
Expand All @@ -139,14 +143,6 @@ impl<T: Event> Plugin for WinitPlugin<T> {

app.add_plugins(AccessKitPlugin);
app.add_plugins(cursor::CursorPlugin);

let event_loop = event_loop_builder
.build()
.expect("Failed to build event loop");

// `winit`'s windows are bound to the event loop that created them, so the event loop must
// be inserted as a resource here to pass it onto the runner.
app.insert_non_send_resource(event_loop);
}
}

Expand Down
Loading

0 comments on commit 7b021b4

Please sign in to comment.