Trouv / bevy_ecs_ldtk

ECS-friendly ldtk plugin for bevy, leveraging bevy_ecs_tilemap
Other
703 stars 81 forks source link

Panic without `atlas` feature after v0.6.0 upgrade #176

Open johanhelsing opened 1 year ago

johanhelsing commented 1 year ago

I'm not really sure if this is intentional or not, but I get a panic after upgrading from v0.5.0 to v0.6.0 (on native).

2023-04-07T15:28:53.676302Z ERROR wgpu::backend::direct: Handling wgpu errors as fatal by default
thread 'Compute Task Pool (11)' panicked at 'wgpu error: Validation Error

Caused by:
    In Device::create_texture
      note: label = `texture_array`
    Dimension Z value 4096 exceeds the limit of 2048

', C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\wgpu-0.15.1\src\backend\direct.rs:3024:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
2023-04-07T15:28:53.677043Z  INFO cargo_space::ldtk_integration: Level event: Transformed("58e02ac0-5110-11ed-b070-cdccc74eaccd")
2023-04-07T15:28:53.677162Z  INFO cargo_space::ldtk_integration: Level event: Transformed("58e02ac0-5110-11ed-b070-cdccc74eaccd")
thread 'Compute Task Pool (11)' panicked at 'A system has panicked so the executor cannot continue.: RecvError', C:\Users\Johan\.cargo\git\checkouts\bevy-899673624796a25d\7b9124d\crates\bevy_ecs\src\schedule\executor\multi_threaded.rs:194:60
thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', C:\Users\Johan\.cargo\git\checkouts\bevy-899673624796a25d\7b9124d\crates\bevy_tasks\src\task_pool.rs:376:49
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: RecvError', C:\Users\Johan\.cargo\git\checkouts\bevy-899673624796a25d\7b9124d\crates\bevy_render\src\pipelined_rendering.rs:136:45
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', C:\Users\Johan\.cargo\git\checkouts\bevy-899673624796a25d\7b9124d\crates\bevy_tasks\src\task_pool.rs:376:49
error: process didn't exit successfully: `target\debug\cargo_space.exe --dev --join-private 123 --next 1` (exit code: 101)

The panics go away if I enable the atlas feature.

If intentional, maybe either provide a more helpful error/panic message, or put a note in the breaking changes section of the release notes: https://github.com/Trouv/bevy_ecs_ldtk/releases/tag/v0.6.0 ?

johanhelsing commented 1 year ago

fwiw I was on @geieredgar 's bevy-track branch for a while without the feature and without issues. Can do a bisect if that's interesting, but thought I'd ask first

Trouv commented 1 year ago

That's unfortunate. Definitely not intentional. I've seen this wgpu error before. How big is your biggest tileset, in terms of tiles? You may want to try increasing some wgpu limits to 4096, like max_texture_dimension_3d or maybe max_texture_array_layers: https://docs.rs/bevy/latest/bevy/render/settings/struct.WgpuLimits.html

You can do so by manually setting WgpuSettings::limits on RenderPlugin:

app.add_plugins(DefaultPlugins.set(RenderPlugin { wgpu_settings: WgpuSettings { ... }}))
johanhelsing commented 1 year ago

It's 1024x1024 pixels with 16 pixels per tile, so 64x64=4096 tiles.

I tried:

            .set(RenderPlugin {
                wgpu_settings: WgpuSettings {
                    limits: {
                        let mut limits = WgpuSettings::default().limits;
                        limits.max_texture_dimension_3d = 4096;
                        limits
                    },
                    ..default()
                },
            })

But didn't make a difference.

Trouv commented 1 year ago

What about max_texture_array_layers? Or both?

johanhelsing commented 1 year ago

What about max_texture_array_layers? Or both?

Sorry, was a bit quick forgot about that one. Tried both now, still panics.

Trouv commented 1 year ago

Interesting, does the error still say Dimension Z value 4096 exceeds the limit of 2048? Or has 2048 increased to 4096?

JoshLambda commented 1 year ago

According to https://github.com/bevyengine/bevy/issues/8248#issuecomment-1488780916 this is a hardware limit that you cannot increase (only decrease).

Mattincho commented 1 year ago

I'm seeing the same. Using v0.6.0 too.

The weird thing, is that I see the issue, when I just increase the canvas size of the level:

2023-04-17T19:01:55.996649Z ERROR wgpu::backend::direct: Handling wgpu errors as fatal by default
thread 'Compute Task Pool (7)' panicked at 'wgpu error: Validation Error

Caused by:
    In Device::create_texture
      note: label = `texture_array`
    Dimension Z value 4680 exceeds the limit of 2048

', C:\Users\Administrador\.cargo\registry\src\index.crates.io-6f17d22bba15001f\wgpu-0.15.1\src\backend\direct.rs:3024:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'Compute Task Pool (7)' panicked at 'A system has panicked so the executor cannot continue.: RecvError', C:\Users\Administrador\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_ecs-0.10.1\src\schedule\executor\multi_threaded.rs:194:60
thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', C:\Users\Administrador\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_tasks-0.10.1\src\task_pool.rs:376:49
thread 'Compute Task Pool (7)' panicked at 'called `Result::unwrap()` on an `Err` value: RecvError', C:\Users\Administrador\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_render-0.10.1\src\pipelined_rendering.rs:136:45
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', C:\Users\Administrador\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_tasks-0.10.1\src\task_pool.rs:376:49
error: process didn't exit successfully: `target\debug\gm.exe` (exit code: 101)
Trouv commented 1 year ago

Hmm, it only happens when you increase the canvas size of the level? Do you have a tileset w/ 4680 tiles in it?

johanhelsing commented 1 year ago

I have a few other non-ldtk bevy_ecs_tilemaps at least, though not sure how big they are. I can try disabling them and see if that makes a difference. Might have some time for this towards the end of the week.

Mattincho commented 1 year ago

I'm just starting a little project, only with two tilesets for now. One is 256x256, and the other is 512x512. My tiles are 16x16 I'm using one in an entitys layer, and the other one in an integer grid. Without enabling the atlas feature of bevy_ecs_tilemap, increasing the canvas size, to somewhat around 700x700 (I can disable the feature and check again if that's needed), results in the error mentioned earlier. Also, the z dimension value mentioned in the error, increases as I increase the canvas.

I added this to the Cargo.toml, and I no longer get the error increasing the canvas size:

bevy_ecs_tilemap = { version = "0.10", default-features = false, features = ["atlas"]}
Trouv commented 1 year ago

Without enabling the atlas feature of bevy_ecs_tilemap, increasing the canvas size, to somewhat around 700x700 (I can disable the feature and check again if that's needed), results in the error mentioned earlier. Also, the z dimension value mentioned in the error, increases as I increase the canvas.

I'm able to reproduce this, and I know what the problem is. It has to do with how the plugin handles rendering intgrid colors for exposed intgrid layers w/o autotiles. Unfortunately, the way the plugin handles it at the moment there's not really a way to disable it. The true fix is #172. I'll prioritize this.

I'm doubtful that this is the same issue that @johanhelsing is running into.

Trouv commented 1 year ago

@Mattincho could you try depending on the branch in #183?

Mattincho commented 1 year ago

Works like a charm 👍

Trouv commented 1 year ago

@Mattincho Now you will need to depend on the main branch - since that branch has been merged (and deleted).

[patch.crates-io]
bevy_ecs_ldtk = { git = "https://github.com/Trouv/bevy_ecs_ldtk", branch = "main" }
Trouv commented 1 year ago

@Mattincho now this change is in 0.7