Support Bevy v0.12.0

[divark]

Summary

This PR makes the necessary changes in order for this library to be bevy v0.12.0 compliant. This closes #488.

Testing

1. Clone the bevy_ecs_tilemap repository as-is.
2. Run all of the examples as described in the README.md.
3. Clone my version of bevy_ecs_tilemap
4. Run cargo check to verify there are no compilation issues.
5. Run all of the examples as described in the README.md.
6. Observe there are (hopefully) no differences between the original.

[rparrett]

Nice work! This looks good to me, just a few minor notes.

@Trouv Could you let us know how this branch goes with bevy_ecs_ldtk?

Also, just a note for reviewers: there is some expected weirdness going on in the examples using text2d on high dpi displays due to bevyengine/bevy#10407.

I also tested examples with the atlas feature and spot checked a few with webgl2.

[EliottGaboreau]

Great work ! I tried testing for wasm target but stumbled on an issue caused by a dependency so I'm guessing we can't test for this target right now although I don't know how necessary it would be for merging this PR

[EliottGaboreau]

Went through all the example (without the atlas feature) and I didn't find any issue !

[rparrett]

I'd like to hear back from bevy_ecs_ldtk, and then poke StarArawn to take a look and hopefully merge.

[rparrett]

Sorry, noticed a couple other minor things.

[rparrett]

Oh?? So I can use a "non-release" branch in my rust project? I'll take a look at that now! thanks for letting me know.

Yep! Something like

bevy_ecs_tilemap = { git = "https://github.com/divark/bevy_ecs_tilemap", branch = "0.12-fixes" }

[OwlyCode]

Thank you for your work on that topic!

I migrated my project using your branch and I get this error:

2023-11-15T18:15:44.813550Z ERROR wgpu::backend::direct: Handling wgpu errors as fatal by default
thread 'Compute Task Pool (0)' panicked at 'wgpu error: Validation Error

Caused by:
    In Device::create_texture
      note: label = `texture_array`
    Dimension Z is zero
    
    ...
    
Encountered a panic in system `bevy_ecs_tilemap::render::prepare_textures`!

I'm not sure how I can confirm this is on my side or on the lib.

[OwlyCode]

I found the issue with my code, I didn't notice I was creating a tilemap without a texture. Which seems to work with the current version of this library.

Should this case also be handled by this PR?

[Twyker]

looking forward to this beautiful merge 🙏

[csandven]

Seems to work very well with my project as well, so another thumbs up ! Looking forward to the merge.

[Trouv]

Hello again - one of my users @musjj caught a bug regarding render targets that wasn't happening in 0.11. I looked into it a little bit and it seems that, when adding a camera with a custom RenderTarget after a tilemap is rendered, we get errors like this:

thread '<unnamed>' panicked at $HOME/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.17.2/src/backend/direct.rs:3056:5:
wgpu error: Validation Error

Caused by:
    In a RenderPass
      note: encoder = `<CommandBuffer-(0, 4, Vulkan)>`
    In a set_bind_group command
      note: bind group = `tilemap_view_bind_group`
    Dynamic binding offset index 0 with offset 2304 would overrun the buffer bound to bind group 0 -> binding 0. Buffer size is 1536 bytes, the binding binds bytes 0..608, meaning the maximum the binding can be offset is 928 bytes

Or on my system (similar but slightly different):

wgpu error: Validation Error

Caused by:
    In a RenderPass
      note: encoder = `<CommandBuffer-(0, 13, Vulkan)>`
    In a set_bind_group command
      note: bind group = `<BindGroup-(11, 6, Vulkan)>`
    Dynamic binding offset index 0 with offset 608 would overrun the buffer bound to bind group 0 -> binding 0. Buffer size is 608 bytes, the binding binds bytes 0..608, meaning the maximum the binding can be offset is 0 bytes

W/ the help of @musjj's own minimal example using bevy_ecs_ldtk, I've updated the basic example of bevy_ecs_tilemap to encounter this bug in the following branch: https://github.com/Trouv/bevy_ecs_tilemap/blob/bug/0.12-render-target/examples/basic.rs#L173

In that system, waiting until frame 10 to spawn the camera causes the crash, but removing the frame_count restriction and having it spawn the camera on frame 0 fixes the crash.

[divark]

@Trouv Thanks for the heads up and the augmented basic example! With closer inspection, it looks like I missed a spot when performing the RenderSet changes. The give away was from that first error listed where it referenced the "tilemap_view_bind_group", which originates from the function queue_material_tilemap_meshes that tries to create those bind groups. That function was not registered in the new RenderSet PrepareBindGroups, which is necessary for Bind Group creation.

Luckily, this is a one-line change, and it seems that example now works on my end. I'll push this new commit now. Let me know if this change resolves the issue, as well as whether all the other examples still work as intended!

EDIT: Also, @OwlyCode, mind checking if this fixes your issue as well?
EDIT 2: Looks like this change now alters the 3d_iso example to not render properly. Before this commit, it looked like this:

and now it looks like this:

I'm not sure how to fix this at the moment. Any help would be greatly appreciated.

[Trouv]

I can confirm this does seem to fix the crash, but I am also running into new rendering issues now. In particular sprites seem to be rendering behind tilemaps, and the ordering of the tilemaps themselves seems to be inconsistent
On 50718d6:

On dbc6b17:

It seems to be some regression with z-ordering somehow.

[divark]

Okay, so I have a workaround that I just pushed. My attempt involves splitting the instance of creating bind groups into its own function and "soft reverting" the earlier fix such that

• The original function is back to being registered in the Queue RenderSet.
• The new function with all of the created bind groups still remain in the PrepareBindGroup RenderSet.

So, let's see if this fixes all of the issues so far! However, I feel like these changes can be greatly refactored to look nicer. All of these changes were done in a very experimental way without a lot of understanding for the concepts involved. Because of that, I'm going to re-request the reviews and ask for another round of testing just in case I missed anything.

It feels like we're so close to it being done, thanks everyone for the help so far!

[Data5tream]

@divark I was having the same issue and just tried your new commit. It fixes the z-index issue, but I sometimes get some weird rendering (happens only sometimes when I start my app, also happend with the previous version) (this seems to be caused by an issue unrelated to bevy ECS tilemap, I updated the comment to include only the useful comparison):

Old (dbc6b17)	New (658b6c1)

[divark]

@divark I was having the same issue and just tried your new commit. It seems to fix the z-index issue, but I sometimes get some weird rendering (happens only sometimes when I start my app, also happend with the previous version):

When you say previous version, do you mean the last commit, or v0.11.1, the last version of this library?

[Trouv]

For what it's worth, things seem to be working correctly for bevy_ecs_ldtk again - including the late RenderTargets.

[musjj]

Just tested it and everything now works perfectly for me with bevy_ecs_ldtk, thanks for the fix!

[Data5tream]

When you say previous version, do you mean the last commit, or v0.11.1, the last version of this library?

I meant the previous version of your branch. I've updated the comment with the commit hashes from my cargo.lock and removed some stuff that's not related to neither your work nor the library itself (crappy laptop doing funky stuff).

[SIGSTACKFAULT]

can we get this shipped so we can use it in the jam please 🥺

[MickHarrigan]

can we get this shipped so we can use it in the jam please 🥺

Quick update for any that are looking to use this in the jam.

To use this with bevy_ecs_ldtk add

[patch.crates-io]
bevy_ecs_tilemap = { git = "https://github.com/divark/bevy_ecs_tilemap", branch = "0.12-fixes" }

to your Cargo.toml, as well as

bevy_ecs_ldtk = { git = "https://github.com/Trouv/bevy_ecs_ldtk.git", branch = "feat/bevy-0.12" }

These will make it that it uses the branches that are capable of using bevy 0.12 now, before they have been put on crates.io or the main branches of the respective repos. Hope this helped!

Also found here.

[mbrc12]

Maybe I am wrong, but there appears to be some issues with the Tiled helper. Consider this minimal example https://github.com/mbrc12/minimal-example-for-tiled-issue . The file src/tiled.rs is copied from examples/helpers/tiled.rs.

Specifically, the issue appears to be that the tiled crate processes the image path as described here: https://docs.rs/tiled/latest/tiled/struct.Image.html, which is different from what is assumed by the current helper code. A potential fix is in tiled_fix.rs which works for my case, but may fail elsewhere.

[divark]

@mbrc12

Specifically, the issue appears to be that the tiled crate processes the image path as described here: https://docs.rs/tiled/latest/tiled/struct.Image.html, which is different from what is assumed by the current helper code. A potential fix is in tiled_fix.rs which works for my case, but may fail elsewhere.

The image path from the Tiled library processes an uncanonicalized path, meaning the paths are relative, not absolute, which could cause problems as noted here: #484. I believe the Tiled helper is designed for absolute/canonical paths in mind, hence the extra setup to get the path from the asset loader and such.

If you think this warrants further investigation, I'd recommend making a new issue pointing it out unless this is a regression in comparison to v0.11.1 of this library.

[mbrc12]

Thanks for pointing it out, I have added to the issue you referenced.

[OwlyCode]

Hello @divark sorry I'm super late with this answer. I checked with the latest commits of the branch and it still crashes if the TilemapBundle is created without a texture.