StarArawn / bevy_ecs_tilemap

A tilemap rendering crate for bevy which is more ECS friendly.
MIT License
943 stars 198 forks source link

Bevy 0.13 race condition resulting in tiles not rendering #521

Closed MeoMix closed 5 months ago

MeoMix commented 8 months ago

Hello! I'm trying to look into this but I don't have a ton of ideas just yet. I wanted to report it just to promote awareness.

After updating to Bevy 0.13 and pointing to main on bevy_ecs_tilemap I intermittently have an issue where no tiles render.

Here are some screenshots:

Incorrect: image

Correct: image

Other assets which don't rely on bevy_ecs_tilemap render fine. The issue reproduces on both native builds and WASM builds.

Additionally, I can confirm that the handle is strong and is fully loaded by the time I give it to TilemapBundle as a texture.

I reviewed the PR which brought bevy_ecs_tilemap up to 0.13 and didn't immediately spot anything concerning. I am going to try and test the examples locally now and see if I can reproduce the issue with any of them.

I wonder if this has to do with changes to apply_deferred such that it attempts to apply itself automatically? Although I would think that would cause less issues to occur, but reduce performance, rather than introducing new race conditions. Since it reproduces in WASM I don't think it has anything to do with multithreading.

MeoMix commented 8 months ago

https://github.com/MeoMix/bevy_bug_example/blob/master/src/main.rs

This minimally reproduces the issue. It's some conflict with the bevy_egui render feature and bevy_ecs_tilemap but I haven't tracked it down further yet.

MeoMix commented 8 months ago

Okay, I put up a fix!

For historical tracking purposes (I reuse my bug example repo sometimes), here's relevant code to reproduce the issue:

// This code should draw a white square in the center of the window if it runs properly.
// One in every 5-10 runs no white square will render.
// If the `multithreaded` feature is enabled in bevy's Cargo.toml then the square "always" renders (if it's a timing issue it becomes sufficiently difficult to surface with multithreading)
use bevy::prelude::*;
use bevy::render::{Render, RenderApp, RenderSet};
use bevy_ecs_tilemap::prelude::*;

fn main() {
    App::new()
    .add_plugins((DefaultPlugins, TilemapPlugin))
    .add_systems(Startup, (spawn_camera, spawn_tilemap).chain())
    .add_plugins(ExamplePlugin)
    .run();
}

fn spawn_camera(mut commands: Commands) {
    commands.spawn(Camera2dBundle::default());
}

pub fn spawn_tilemap(mut commands: Commands) {
    let tilemap_entity = commands.spawn_empty().id();
    let map_size =  TilemapSize { x: 144, y: 144 };
    let mut tile_storage = TileStorage::empty(map_size);

    let tile_pos = TilePos { x: 0, y: 0 };

    let tile_entity = commands.spawn(TileBundle {
        position: tile_pos,
        tilemap_id: TilemapId(tilemap_entity),
        ..default()
    }).id();

    tile_storage.set(&tile_pos, tile_entity);

    commands.entity(tilemap_entity).insert(
        TilemapBundle {
            size: map_size,
            storage: tile_storage,
            tile_size: TilemapTileSize { x: 128.0, y: 128.0 },
            ..default()
        },
    );
}

pub struct ExamplePlugin;

impl Plugin for ExamplePlugin {
    fn build(&self, app: &mut App) {
    }

    fn finish(&self, app: &mut App) {
        if let Ok(render_app) = app.get_sub_app_mut(RenderApp) {
            render_app.add_systems(Render, test_system.in_set(RenderSet::Queue));
        }
    }
}

// IMPORTANT: need to have commands as an argument here or bug doesn't reproduce!
pub fn test_system(mut commands: Commands) {}

This code was problematic because it introduced a system to Queue which depended on Commands. This caused Bevy's scheduler to reorder systems such that queue_material_tilemap_meshes was running before prepare which resulted in bad outcomes.

This wasn't occurring previously due to complete chance. The scheduler was simply picking an ordering that was compatible with the architecture, but there wasn't a guarantee enforced.