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

Update for bevy master #24

Merged
merged 18 commits into from
Feb 19, 2021
Merged

Update for bevy master #24

merged 18 commits into from
Feb 19, 2021

Conversation

rparrett
Copy link
Contributor

@rparrett rparrett commented Jan 23, 2021

Fixes #23.

But in a pretty half-assed way. I do not know how to implement the render/render resource context stuff, so they are just stubs.

Providing this for anyone else who's tracking bevy master and just wants to get this working again.

CI obviously failing because this won't work with bevy 0.4.

@rparrett rparrett marked this pull request as draft January 24, 2021 19:02
@rparrett
Copy link
Contributor Author

rparrett commented Feb 2, 2021

Up to date with current bevy master, but the new wgpu stuff was rough on me and I've done the bare minimum only to get my 2d game working.

@rparrett
Copy link
Contributor Author

rparrett commented Feb 4, 2021

Oops! Thanks for that.

I'm super happy to keep working on this. But if someone with any experience at all with opengl wants to take the reins, that's great too.

Some of the examples are looking to be in pretty bad shape now that I've actually even run them.

The biggest known problem I have is here: d30ac8e#diff-a731f7f5f708022a34572ab832954c802f7fd77de1e8c38418ed7049717912f7R150

Which (I think) is resulting in a glGenerateMipmap / GL_INVALID_OPERATION in my project (but no visible problems weirdly), but must be causing all sorts of other issues, right?

@rparrett
Copy link
Contributor Author

rparrett commented Feb 4, 2021

Here's a survey of the examples:

Example Status (This PR) Status (0.4) Status (Bevy Master/Native)
font_atlas_debug panic same bit broken but no panic
z_sort_debug broken working broken

Everything else seems to be working.

font_atlas_debug (same panic in master branch)

wasm.js:338 panicked at 'called `Option::unwrap()` on a `None` value', src/webgl2_render_pass.rs:227:89

z_sort_debug - cubes appear completely black

@rparrett
Copy link
Contributor Author

rparrett commented Feb 4, 2021

Oops. I wanted PerspectiveCameraBundle, not a 3d orthographic camera. Will fix that up.

@rparrett
Copy link
Contributor Author

rparrett commented Feb 4, 2021

I am not sure why I've been manually fixing examples instead of just copying them and using fix_bevy_example.sh.

edit: oh, that strategy doesn't quite work for shader/* and contributors

@rparrett
Copy link
Contributor Author

rparrett commented Feb 5, 2021

Issue with the contributors example was fixed in bevy

@mrk-its
Copy link
Owner

mrk-its commented Feb 5, 2021

Thanks, you've done a lot of work! I'll take a closer look tomorrow.

@mrk-its
Copy link
Owner

mrk-its commented Feb 6, 2021

load_gltf

appears to work, but GL_INVALID_OPERATION : glGenerateMipmap: in console

Do you still have this warning? I cannot reproduce it

@rparrett
Copy link
Contributor Author

rparrett commented Feb 6, 2021

I am (chrome 88, macos).

image

But my assumption that this is caused by my having basically removed support for anything but the default textureformat is a totally blind one.

I'm pretty sure that I don't remember seeing this with bevy 0.4 / bevy_webgl2 but I could be wrong.

@mrk-its
Copy link
Owner

mrk-its commented Feb 6, 2021

The only place mipmaps are generated is

gl_call!(gl.generate_mipmap(Gl::TEXTURE_2D));
- It seems we can simply remove it because mipmaps are not used at all (because of hardcoded TEXTURE_MIN_FILTER = Gl::NEAREST). All texture filtering support has yet to be implemented - so let's remove this mipmap generation now (It would be great to run all texture-related examples after that, but I think everything should work exactly the same way)

Fixes GL_INVALID_OPERATION errors
@rparrett
Copy link
Contributor Author

rparrett commented Feb 6, 2021

Cool. That seems to resolve the scary error message. I double checked load_gltf, sprite, sprite_sheet, texture and they continue to work.

Any leads on what might be going on with z_sort_debug?

@mrk-its
Copy link
Owner

mrk-its commented Feb 6, 2021

Cool. That seems to resolve the scary error message. I double checked load_gltf, sprite, sprite_sheet, texture and they continue to work.

Any leads on what might be going on with z_sort_debug?

Not yet, will take a look

@mrk-its
Copy link
Owner

mrk-its commented Feb 7, 2021

It seems z_order_debug in bevy's master is also broken, I've filled an issue: bevyengine/bevy#1411

@@ -145,11 +147,8 @@ impl<'a> RenderPass for WebGL2RenderPass<'a> {
}

fn draw_indexed(&mut self, indices: Range<u32>, _base_vertex: i32, instances: Range<u32>) {
let (index_type, type_size) = match self.pipeline_descriptor.as_ref().unwrap().index_format
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 remove WebGL2RenderPass.pipeline_descriptor, as it was used only for providing access to index_format.
We can store gl index_format / index_type_size in WebGL2Pipeline object (in set_index_buffer method) or directly in WebGL2RenderPass object

Copy link
Contributor Author

@rparrett rparrett Feb 7, 2021

Choose a reason for hiding this comment

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

I had attempted moving the IndexFormat to WebGL2Pipeline at some point, but the code ended up in a state where I had duplicated this chunk of code from setup_vao

        let resources = &self.render_context.render_resource_context.resources;
        let mut pipelines = resources.pipelines.write();
        let pipeline_handle = self.pipeline.as_ref().unwrap();
        let pipeline = pipelines.get_mut(&pipeline_handle).unwrap();

And that bothered me. I was pretty sure I could clean that up so I wasn't (acquiring a lock unnecessarily?) but never got that far because I had somehow broken everything in a way that nothing even showed up on the screen.

I'll give it another go though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, this is the deeply mysterious panic I was getting when I attempted this last time:

wasm.js:752 panicked at 'Parking not supported on this platform', /Users/robparrett/.cargo/registry/src/github.com-1ecc6299db9ec823/parking_lot_core-0.8.2/src/thread_parker/wasm.rs:26:9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I think I worked around that. Commit is incoming. But maybe you can suggest a cleaner way to do it.

Copy link
Owner

Choose a reason for hiding this comment

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

I think it is fine now, it may be refactored later if we find a better way.

@JeanMertz
Copy link

Nice work @rparrett. It looks like this PR is ready to review/merge, correct? cc @mrk-its.

@rparrett
Copy link
Contributor Author

It's probably not a goal to get this merged until we're much closer to bevy 0.5 being released.

@JeanMertz
Copy link

Got it. Makes sense. I'll just point to this branch meanwhile 👍

@mrk-its
Copy link
Owner

mrk-its commented Feb 18, 2021

@rparrett @JeanMertz I think it is good idea to merge it (if everything still works with current bevy master), and we can keep syncing with bevy in separate PR's. Some time ago I've created separate branch https://github.com/mrk-its/bevy_webgl2/tree/v0.4 for 0.4.x version of bevy_webgl2, so we can sync with bevy's master on master branch here.

Let's simply check few examples with current bevy - if it works I'll merge it.

@rparrett rparrett marked this pull request as ready for review February 18, 2021 22:25
@rparrett
Copy link
Contributor Author

rparrett commented Feb 18, 2021

Looks like all bevy_webgl2 examples that are expected to work (everything but font_atlas_debug) are working on my end.

@mrk-its mrk-its merged commit b5e842e into mrk-its:master Feb 19, 2021
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.

Build fails with recent bevy master
3 participants