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

Panic without atlas feature after v0.6.0 upgrade #176

Open
johanhelsing opened this issue Apr 7, 2023 · 16 comments
Open

Panic without atlas feature after v0.6.0 upgrade #176

johanhelsing opened this issue Apr 7, 2023 · 16 comments
Labels
bug Something isn't working

Comments

@johanhelsing
Copy link
Contributor

johanhelsing commented Apr 7, 2023

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 ?

@Trouv Trouv added the bug Something isn't working label Apr 7, 2023
@johanhelsing
Copy link
Contributor Author

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
Copy link
Owner

Trouv commented Apr 7, 2023

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
Copy link
Contributor Author

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
Copy link
Owner

Trouv commented Apr 7, 2023

What about max_texture_array_layers? Or both?

@johanhelsing
Copy link
Contributor Author

johanhelsing commented Apr 7, 2023

What about max_texture_array_layers? Or both?

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

@Trouv
Copy link
Owner

Trouv commented Apr 7, 2023

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

@JoshLambda
Copy link

According to bevyengine/bevy#8248 (comment) this is a hardware limit that you cannot increase (only decrease).

@Mattincho
Copy link

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
Copy link
Owner

Trouv commented Apr 18, 2023

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

@johanhelsing
Copy link
Contributor Author

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
Copy link

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
Copy link
Owner

Trouv commented Apr 19, 2023

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
Copy link
Owner

Trouv commented Apr 19, 2023

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

@Mattincho
Copy link

Works like a charm 👍

Trouv added a commit that referenced this issue Apr 24, 2023
…183)

Closes #172, and resolves an issue brought up by @Mattincho in #176 

# Summary
The white image generated for levels is now only used for intgrid
coloring. This means it no longer needs to be the size of the level - it
can just be the size of the maximum-sized intgrid layer. The
maximum-sized intgrid tile is easy to determine during asset loading.
This PR creates a small white-image for the maximum-sized intgrid tile
once for the entire asset, rather than a large one once per level. This
should improve performance, and also hopefully resolve issues with
hitting hardware limits on larger levels.

BREAKING CHANGE: Most likely won't affect users - `LdtkAsset` has gained
a `int_grid_image_handle` field, breaking any manual construction of it.
@Trouv
Copy link
Owner

Trouv commented Apr 24, 2023

@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
Copy link
Owner

Trouv commented Apr 29, 2023

@Mattincho now this change is in 0.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants