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

WASM asset loading #559

Merged
merged 2 commits into from Sep 25, 2020
Merged

WASM asset loading #559

merged 2 commits into from Sep 25, 2020

Conversation

mrk-its
Copy link
Member

@mrk-its mrk-its commented Sep 23, 2020

This PR adds basic asset loading support for wasm32 target (see assets_wasm example).
Currently only AssetServer.load is implemented (AssetServer.load_sync makes no sense in web context, load_asset_folder is tricky, because it will need some html parsing, but it seems doable in the future).

@mrk-its mrk-its changed the title wasm assets WIP - wasm assets Sep 23, 2020
@ghost
Copy link

ghost commented Sep 23, 2020

In single_threaded_task_pool.rs, you can delete all Send bounds because spawned tasks are run on the same thread that has spawned them. This should fix the compilation errors on wasm.

diff --git a/crates/bevy_tasks/src/single_threaded_task_pool.rs b/crates/bevy_tasks/src/single_threaded_task_pool.rs
index 8899feb9..de641760 100644
--- a/crates/bevy_tasks/src/single_threaded_task_pool.rs
+++ b/crates/bevy_tasks/src/single_threaded_task_pool.rs
@@ -59,8 +59,8 @@ impl TaskPool {
     /// This is similar to `rayon::scope` and `crossbeam::scope`
     pub fn scope<'scope, F, T>(&self, f: F) -> Vec<T>
     where
-        F: FnOnce(&mut Scope<'scope, T>) + 'scope + Send,
-        T: Send + 'static,
+        F: FnOnce(&mut Scope<'scope, T>) + 'scope,
+        T: 'static,
     {
         let executor = &async_executor::LocalExecutor::new();
         let executor: &'scope async_executor::LocalExecutor<'scope> =
@@ -90,8 +90,8 @@ pub struct Scope<'scope, T> {
     results: Vec<Arc<Mutex<Option<T>>>>,
 }

-impl<'scope, T: Send + 'scope> Scope<'scope, T> {
-    pub fn spawn<Fut: Future<Output = T> + 'scope + Send>(&mut self, f: Fut) {
+impl<'scope, T: 'scope> Scope<'scope, T> {
+    pub fn spawn<Fut: Future<Output = T> + 'scope>(&mut self, f: Fut) {
         let result = Arc::new(Mutex::new(None));
         self.results.push(result.clone());
         let f = async move {

@Moxinilian Moxinilian added A-Assets Load files from disk to use for things like images, models, and sounds C-Enhancement A new feature O-Web Specific to web (WASM) builds labels Sep 24, 2020
@mrk-its mrk-its changed the title WIP - wasm assets wasm assets Sep 25, 2020
@mrk-its mrk-its changed the title wasm assets WASM asset loading Sep 25, 2020
@mrk-its
Copy link
Member Author

mrk-its commented Sep 25, 2020

Ok, removing WIP mark as I'm quite happy how it looks like now.
@cart could you take a look? I've changed a bit interface of AssetLoadRequestHandler - handle_request is async now (with help of async_trait macro). I've changed also the way request handlers are kept in AssetServer: RwLock<Vec<Arc<dyn AssetLoadRequestHandler>>> instead of Arc<RwLock<Vec<Box<dyn AssetLoadRequestHandler>>>> - this allows for easy passing cloned Arc<dyn AssetLoadRequestHandler> to spawned future.
BTW It seems RwLock above is also unnecessary (at least now) - or I don't understand something (but it compiles without it).

@smokku - I've added missing ThreadPool.spawn implementation to single_threaded_task_pool with some comment - could you take a look?

@smokku
Copy link
Member

smokku commented Sep 25, 2020

I've added missing ThreadPool.spawn implementation to single_threaded_task_pool with some comment - could you take a look?

The multi-threaded implementation returns Task<T> which is a future that can be awaited (returning the spawned future value).

Maybe we could use oneshot channel to pass a value from inside of spawn_local to a Received wrapped as a Task?

@mrk-its
Copy link
Member Author

mrk-its commented Sep 25, 2020

Maybe we could use oneshot channel to pass a value from inside of spawn_local to a Received wrapped as a Task?

Looks promising, but I'm not sure if it will work when sender is on other executor than receiver, will try. The other thing is it will require bigger refactoring of single_threaded_task_pool - currently we create async_executor::LocalExecutor for each scope)

And I would probably prefer to do that in separate PR

@cart
Copy link
Member

cart commented Sep 25, 2020

@cart could you take a look?

@mrk-its sure thing. looking now!

@cart
Copy link
Member

cart commented Sep 25, 2020

Also note that I'm sorting out the future of the asset system with @kabergstrom right now, so there will likely be some breaking changes in the near future.

// But for typical use cases it seems that current implementation should be sufficient:
// caller can spawn long-running future writing results to some channel / event queue
// and simply call detach on returned Task (like AssetServer does) - spawned future
// can write results to some channed / event queue.
Copy link
Member

Choose a reason for hiding this comment

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

nit: channel

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@mrk-its
Copy link
Member Author

mrk-its commented Sep 25, 2020

Also note that I'm sorting out the future of the asset system with @kabergstrom right now, so there will likely be some breaking changes in the near future.

Sure, I'll try to keep wasm part working and up to date.

@cart cart merged commit a3012d9 into bevyengine:master Sep 25, 2020
mrk-its added a commit to mrk-its/bevy that referenced this pull request Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Enhancement A new feature O-Web Specific to web (WASM) builds
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants