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

Support for wasm32-unknown-unknown target (WebGL) #149

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

caiiiycuk
Copy link
Contributor

I tried to update wgpu to 0.11.0 (in order to support WebGL in future). I fixed some initialization code and now it compiles but render black screen, can you please point me what I did wrong.

Platform: linux
Renderer: vulkan

@caiiiycuk
Copy link
Contributor Author

Also _pad: vec2 in shader seems unused, I renamed it to apad because shader won’t compile

README.md Outdated Show resolved Hide resolved
@kvark
Copy link
Owner

kvark commented Nov 18, 2021

Please rebase!

@caiiiycuk caiiiycuk force-pushed the vange-rs-web branch 2 times, most recently from c36b8c4 to d646cca Compare November 19, 2021 10:41
@caiiiycuk
Copy link
Contributor Author

Everything looks working, but seems render is not fit for webgl, have this issue:

road.js:1416 panicked at 'wgpu error: Validation Error

Caused by:
    In Device::create_bind_group_layout
      note: label = `Shape`
    binding 0 entry is invalid
    Downlevel flags VERTEX_STORAGE are required but not supported on the device.
This is not an invalid use of WebGPU: the underlying API or device does not support enough features to be a fully compliant implementation. A subset of the features can still be used. If you are running this program on native and not in a browser and wish to work around this issue, call Adapter::downlevel_properties or Device::downlevel_properties to get a listing of the features the current platform supports.

@kvark
Copy link
Owner

kvark commented Nov 19, 2021

Please file this separately, I'll address it shortly.

@caiiiycuk
Copy link
Contributor Author

#150

bin/road/main.rs Outdated Show resolved Hide resolved
@caiiiycuk caiiiycuk force-pushed the vange-rs-web branch 4 times, most recently from 2b9a96b to 3a79885 Compare December 9, 2021 08:53
@caiiiycuk caiiiycuk force-pushed the vange-rs-web branch 8 times, most recently from c340ea2 to 853515e Compare December 9, 2021 17:26
@caiiiycuk caiiiycuk force-pushed the vange-rs-web branch 2 times, most recently from 6bc3562 to de2b7c8 Compare December 9, 2021 17:33
@caiiiycuk caiiiycuk marked this pull request as ready for review December 9, 2021 17:41
@caiiiycuk
Copy link
Contributor Author

Please comment what to fix, I will update README.md soon

@caiiiycuk caiiiycuk changed the title [DRAFT] Support for wasm32-unknown-unknown target Support for wasm32-unknown-unknown target (WebGL) Dec 9, 2021
@caiiiycuk caiiiycuk requested a review from kvark December 9, 2021 17:42
Copy link
Owner

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Amazing work!

@@ -1,9 +1,9 @@
name: Check
on:
push:
branches: [master]
branches: [master, vange-rs-web]
Copy link
Owner

Choose a reason for hiding this comment

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

since we are already in vange-rs repo, perhaps the branch could just be called web?

@@ -14,7 +14,9 @@ jobs:
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v2
- run: rustup target add wasm32-unknown-unknown
Copy link
Owner

Choose a reason for hiding this comment

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

let's add a note to tell what we are doing to whoever is reading the log

README.md Outdated

**Resources**

To build game **you must** put resources of original game in `res_linux/data` folder.
Copy link
Owner

Choose a reason for hiding this comment

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

why is this res_linux specifically and not some web/resources for example?

use wasm_bindgen::prelude::*;
use wasm_bindgen::JsCast;

extern crate console_error_panic_hook;
Copy link
Owner

Choose a reason for hiding this comment

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

you don't need to declare extern crates like this any more in Rust

create_file!("data/wrlds.dat", get_bundled_file!());

// the chain
create_file!("data/thechain/threall/world.ini", get_bundled_file!());
Copy link
Owner

Choose a reason for hiding this comment

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

We should probably do it in build.rs, for multiple reasons. First, we could drive this by data or environment instead of a cargo feature, which means there is less enabling/disabling of code paths going on.
And secondly, most importantly, we could just walk the directory tree and pick up all the files we need, instead of enumerating every single filename/path.

print("\tcreate_file(\"" + path + "\", include_bytes!(\"../" + path + "\"));");

print("\n\t// vangers resouces")
for root, subdirs, files in os.walk("res_linux"):
Copy link
Owner

Choose a reason for hiding this comment

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

ok, yeah, a build.rs script would be much better than a python script :)

let window = web_sys::window().expect("should have a window in this context");

{
let mappings: HashMap<String, Key> = [
Copy link
Owner

Choose a reason for hiding this comment

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

the key should be &'static str instead of String

</script>

<script>
const filesToFetch = [
Copy link
Owner

Choose a reason for hiding this comment

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

would be nice to have this queried instead of hard-coded, but we can do it as a follow-up

const generateConfig = (level) => `
(
data_path: "data",
game: (
Copy link
Owner

Choose a reason for hiding this comment

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

would it be feasible to read the config from the file? e.g. in build script.

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.

3 participants