Bevy 0.15

[rparrett]

This seems to be in a pretty good place now. Some things left to do:

• Make CI green if it isn't
• Test examples on native (macos)
• Test examples with the atlas feature
• Test examples with webgl2 and webgpu
• Make sure that "map removal" is working properly
• Would be nice to test for any major perf regression
• Would be nice to double check that we aren't leaking entities anywhere, especially when adding/removing tiles/maps.

Many thanks to @bas-ie, @evilenzo, @adrien-bon, @tychedelia.

Let me know if I missed anything important.

[rparrett]

Looks like there's a major performance regression. It seems that something is going wrong with change detection for tilemaps and chunks are getting rebuilt every frame. Specifically, Changed<GlobalTransform>.

Seeing a ~20x regression in bench.rs.

edit: Okay, tilemaps are "changed" every frame without this PR too, so that's not it. But concerning anyway. Might need to do some tracing.

edit: Seems like there may be some subtle main/render entity mismatch happening that's preventing chunking from working properly? Tracing didn't get any more specific than "prepare::prepare is taking a lot of." Also saw a suspiciously large-looking vertex buffer.

edit: drilled down slightly further w/ tracy. Most of the time is spent in the first extracted_tiles loop in prepare::prepare

edit: seems like ExtractedTiles was previously being cleared every frame, but isn't now... this makes complete sense. The render world is now retained.

[rparrett]

Okay, fixed the performance regression. Frame times on bench.rs is now in the same ballpark as before (better, even!). The extraction code could really use a cleanup, but that's for another time.

This PR: ~9ms
Main: ~14ms

[adrien-bon]

Testing with bevy_ecs_tiled examples, it seems we still have an issue with map removal.
From time to time, when spawning a new map in place a previous one, I see the following logs:

2024-12-04T18:57:56.175238Z ERROR bevy_ecs::system::commands: Failed to 'insert or spawn' bundle of type bevy_ecs_tilemap::render::material::MaterialTilemapHandle<bevy_ecs_tilemap::render::material::StandardTilemapMaterial> into the following invalid entities: [5v2084#8950711844869]
2024-12-04T18:57:56.199393Z ERROR bevy_ecs::system::commands: Failed to 'insert or spawn' bundle of type bevy_ecs_tilemap::render::extract::ExtractedTilemapTextureBundle into the following invalid entities: [5v2084#8950711844869]
2024-12-04T18:57:56.199471Z ERROR bevy_ecs::system::commands: Failed to 'insert or spawn' bundle of type bevy_ecs_tilemap::render::material::MaterialTilemapHandle<bevy_ecs_tilemap::render::material::StandardTilemapMaterial> into the following invalid entities: [5v2084#8950711844869]
2024-12-04T18:57:56.224944Z ERROR bevy_ecs::system::commands: Failed to 'insert or spawn' bundle of type bevy_ecs_tilemap::render::extract::ExtractedTilemapTextureBundle into the following invalid entities: [5v2084#8950711844869]
2024-12-04T18:57:56.225049Z ERROR bevy_ecs::system::commands: Failed to 'insert or spawn' bundle of type bevy_ecs_tilemap::render::material::MaterialTilemapHandle<bevy_ecs_tilemap::render::material::StandardTilemapMaterial> into the following invalid entities: [5v2084#8950711844869]

Will investigate

[adrien-bon]

I can reproduce the issue if I despawn_recursive() a tilemap then spawn a new one. I get the Failed to 'insert or spawn' bundle error and my new tilemap won't render. Issue is not reproduced if I just re-insert a tilemap on the same entity.

I don't fully understand what is happening but it does not happen if I remove the TemporaryRenderEntity component from the Extract* bundles.

I dig a bit into the code, especially around extraction logic (see src/render/extract.rs:373).
From what I see, the RenderEntity I get for the tilemap is not valid anymore.

What is strange is that it works for the first frame but it does not after.
I observe the following sequence:

• spawn a tilemap, with a new RenderEntity = X
• insert_or_spawn_batch(X)
• insert_or_spawn_batch(X)
• ...
• despawn the tilemap, RenderEntity X is properly despawned
• spawn a tilemap, with a new RenderEntity = Y
• insert_or_spawn_batch(Y)
• insert_or_spawn_batch(Y) => error is triggered

Not sure why a RenderEntity I get from an Extract<Query<T>> would suddenly become invalid...

[adrien-bon]

Got it to work by removing the TemporaryRenderEntity component from ExtractedTilemapBundle bundle.

I don't understand because this component should still be present on the tilemap entity since it is also inserted via the ExtractedTilemapTexture bundle on the same entity (which also have this TemporaryRenderEntity component).

Hopefully performances are still OK ?

PR = rparrett#6

[rparrett]

Any chance you could provide a runnable example that reproduces this?

My thinking at the moment is that we

• need to be storing main world entity refs in Extracted* / eventually in RenderChunk2dStorage
  edit: well, maybe, but then we need a per-frame mapping of tilemap main entity -> render entity.
  maybe we could just extract into an EntityHashMap<ExtractedTilemapBundle> or whatever.
• insert_or_spawn is pointless if Extracted* remain TemporaryRenderEntity, so we can remove some main/render entity confusion by just using spawn_batch.

(or, rework things so that we do persistent extraction, and have some separate mechanism for detecting changed tiles/maps/textures or whatever. Would probably prefer fixing up the temporary entities for now.)

(grain of salt, I don't know the renderer super well)

I had the same thought though to do what you just did as a quick workaround. I'll take a closer look in a bit. I feel like there might be a similar bug lurking with tile despawning though.

edit: Hit a bit of a snag with the hashmap approach because we rely on tilemaps getting extracted when checking visibility in material.rs.

[rparrett]

Okay, I think I was misunderstanding things pretty badly -- insert_or_spawn_batch is not pointless, because SyncToRenderWorld creates the entities in the render world.

It is confusing though, and should probably be swapped out for insert_batch.

The thing that's melting my brain at the moment though is reconciling this "custom change-based extraction" with SyncToRenderWorld extracting every entity. It might not be a problem but it just feels weird.

Other stuff in Bevy operates this way... sort of... I think. But instead of deleting the whole entity with TemporaryRenderEntity, the extracted components usually get deleted.

So I think that TemporaryRenderEntity is clearly wrong here. Perhaps instead of clearing the entire entity every frame, we can just clear the extracted components.

Understanding this code more and more though. Will look again a little later.

[rparrett]

I reverted the TemporaryRenderEntity stuff in favor of a new ChangedInMainWorld component. The queries in prepare skip anything without that component and (just that component) is cleared from the render entities every frame.

Perf seems even better (~7.5ms), and map reloading seems to work again.

[dpogorzelski]

Hey, in 0.14 i used to have a custom bevy_mod_picking backend to be able to react when clicking on a tile.
Considering native support for entity picking via observe in 0.15, should it be possible to .observe a TileBundle and get a click trigger to work?
Seems that a simple .observe(on_click_print_hello); and then:

fn on_click_print_hello(click: Trigger<Pointer<Click>>) {
    println!("{} was clicked!", click.entity());
}

doesn't work.
Otherwise the code builds and runs fine.

[rparrett]

That isn't expected to work out of the box. Individual tiles don't have meshes or bounding boxes.

(IMO) A picking backend would be great to add now that picking is built into Bevy. Please see #572 for related discussion.

[rparrett]

Added a despawn_tilemap example to help with testing.

[rparrett]

Okay, no idea why adding that example is breaking the headless CI, but I'll just remove it for now.

[rparrett]

I spot-checked basic, custom_shader, and tiled with

--target=wasm32-unknown-unknown
--target=wasm32-unknown-unknown --features=atlas
--target=wasm32-unknown-unknown --features=bevy/webgpu
--target=wasm32-unknown-unknown --features=bevy/webgpu,atlas

(mac m1/chrome)

And tested for entity leaks with the example from #578

[ChristopherBiscardi]

I see the review request and will start on that later today.

[neocturne]

Based on this PR, the update of bevy_ecs_ldtk is also coming along nicely: Trouv/bevy_ecs_ldtk#340

I've tested the bevy_ecs_tilemap + bevy_ecs_ldtk updates on Linux X11 and Wayland and didn't see any issues.

[ChristopherBiscardi]

Reveiwed and have been using this PR for about a week now to good effect and haven't noticed any issues. Intending to approve, merge, and release later today.