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

Error loading two different texture atlases back to back #1056

Closed
tommygoris opened this issue Dec 13, 2020 · 16 comments · Fixed by #1208
Closed

Error loading two different texture atlases back to back #1056

tommygoris opened this issue Dec 13, 2020 · 16 comments · Fixed by #1208
Labels
A-Rendering Drawing game state to the screen P-Crash A sudden unexpected crash

Comments

@tommygoris
Copy link

Bevy version

0.3.0 509b138

Operating system & version

Windows 10

What you did

Load 2 sprite texture atlas back to back in the setup like so... (This is a modified version of the sprite_sheet.rs example that is not animated and loads an additional and different sprite sheet.)
I am not sure if a specific sprite sheet is required to trigger the error.
Note: The error can be inconsistent to trigger.

use bevy::prelude::*;

fn main() {
App::build()
.add_plugins(DefaultPlugins)
.add_startup_system(setup)
.run();
}

fn setup(
commands: &mut Commands,
asset_server: Res,
mut texture_atlases: ResMut<Assets>,
) {

let texture_handle1: Handle<Texture> = asset_server.load("sprite_sheet1");
let texture_handle2: Handle<Texture> = asset_server.load("sprite_sheet1");

let texture_atlas1 = TextureAtlas::from_grid(texture_handle1, Vec2::new(x, y), c, r);
let texture_atlas2 = TextureAtlas::from_grid(texture_handle2, Vec2::new(x, y), c, r);

let texture_atlas_handle1 = texture_atlases.add(texture_atlas1);
let texture_atlas_handle2 = texture_atlases.add(texture_atlas2);

commands
    .spawn(Camera2dBundle::default())
    .spawn(SpriteSheetBundle {
        texture_atlas: texture_atlas_handle1,
        transform: Transform::from_scale(Vec3::splat(6.0)),
        ..Default::default()
    })
    .with(Timer::from_seconds(0.1, true));

}

What you expected to happen

No crash

What actually happened

An error occurred.
thread 'main' panicked at 'range end index 1768 out of range for slice of length 1024', ...\bevy\crates\bevy_render\src\render_graph\nodes\render_resources_node.rs:336:26

Additional information
Error occurs in render_resources_node.rs file

I am not too familiar with rendering code, but after doing some investigating, the first part of the problem occurs in the loop in (lines 681-687). In the loop, when the prepare_uniform_buffers() function is called, it updates the field required_staging_buffer_size field with the size required determined by the byte len of the buffer. The problem is that if one of the render_resource buffer byte len differs from the buffer_array's item size then if the buffer array needs to be resized then it will resize the buffer to a value less than what is needed because the buffer_arrays item size is not correct.

The second part of the issue seems to be caused by the set_required_staging_buffer_size_to_max() function in the render_resources_node.rs file called from line ~692 in the same file. The required_staging_buffer_size field is updated/resized with a size that is less than what the buffer requires. This is because the item_size field is incorrect and so when the new_size is calculated (by multiplying the buffer len), it is less than what is required by the buffer.

For example, the prepare_uniform_buffers() is called and the required_staging_buffer_size is set to a new size of 8 + 1760 to get a value of 1768 for the buffer. The buffer_array.item_size = 256. Then when the set_required_staging_buffer_size_to_max() is called, it will update/resize the required_staging_buffer_size to item_size(256) * buffer_len(4) = 1024. The error will happen because of this, 1768 > 1024 and an index out of range will occur down the line.

Seems to me that the item_size for the buffer_array needs to be updated properly.

@Moxinilian Moxinilian added P-Crash A sudden unexpected crash A-Rendering Drawing game state to the screen labels Dec 15, 2020
@davidlgj
Copy link

davidlgj commented Jan 1, 2021

Just wanted to chime in that I have the exact same problem on Bevy 0.4.0 on Arch Linux.

I have nearly exactly the same code, 2 sprite texture atlases and not much more. I'm trying to learn Bevy so I'm just playing with the examples.

@rparrett
Copy link
Contributor

rparrett commented Jan 2, 2021

Same issue here, Bevy 0.4.0/master on wasm after adding a second sprite atlas to the mix.

@mockersf
Copy link
Member

mockersf commented Jan 2, 2021

couldn't reproduce on macOS with master adapting example sprite_sheet.rs, either with using the same sprite sheet or using two different

Code used ```rust use bevy::prelude::*;

fn main() {
App::build()
.add_plugins(DefaultPlugins)
.add_startup_system(setup.system())
.add_system(animate_sprite_system.system())
.run();
}

fn animate_sprite_system(
time: Res,
texture_atlases: Res<Assets>,
mut query: Query<(&mut Timer, &mut TextureAtlasSprite, &Handle)>,
) {
for (mut timer, mut sprite, texture_atlas_handle) in query.iter_mut() {
timer.tick(time.delta_seconds());
if timer.finished() {
let texture_atlas = texture_atlases.get(texture_atlas_handle).unwrap();
sprite.index = ((sprite.index as usize + 1) % texture_atlas.textures.len()) as u32;
}
}
}

fn setup(
commands: &mut Commands,
asset_server: Res,
mut texture_atlases: ResMut<Assets>,
) {
let texture_handle = asset_server.load("textures/rpg/chars/gabe/gabe-idle-run.png");
let texture_handle2 = asset_server.load("textures/rpg/chars/gabe/gabe-idle-run2.png");
let texture_atlas = TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1);
let texture_atlas2 = TextureAtlas::from_grid(texture_handle2, Vec2::new(24.0, 24.0), 7, 1);
let texture_atlas_handle = texture_atlases.add(texture_atlas);
let texture_atlas_handle2 = texture_atlases.add(texture_atlas2);
commands
.spawn(Camera2dBundle::default())
.spawn(SpriteSheetBundle {
texture_atlas: texture_atlas_handle,
transform: Transform::from_scale(Vec3::splat(6.0)),
..Default::default()
})
.with(Timer::from_seconds(0.1, true));
commands
.spawn(Camera2dBundle::default())
.spawn(SpriteSheetBundle {
texture_atlas: texture_atlas_handle2,
transform: Transform::from_scale(Vec3::splat(6.0)),
..Default::default()
})
.with(Timer::from_seconds(0.15, true));
}

</details>

@rparrett
Copy link
Contributor

rparrett commented Jan 2, 2021

I tried for a minimal reproduction but failed.

But I managed to get my project back into a state where the intermittent panic is happening again. Sometime in the wee hours last night I moved one of the sprite sheet loads into a different system and it just stopped happening for whatever reason.

https:/rparrett/taipo/tree/spritesheet-panic

cargo make serve --profile=release

Also using rust 1.49.0 on a mac to compile the thing in case that makes a difference.

@davidlgj
Copy link

davidlgj commented Jan 2, 2021

Here is another example, I think I got it down to as minimal as it get 😄
https:/davidlgj/bevy_assets_intermittent_crash

Here is the main.rs

use bevy::prelude::*;

fn setup(
    asset_server: Res<AssetServer>,
    mut texture_atlases: ResMut<Assets<TextureAtlas>>,
) {

    let texture_handle = asset_server.load("hero.png");
    let texture_atlas = TextureAtlas::from_grid(texture_handle, Vec2::new(16.0, 16.0), 2, 4);
    let _texture_atlas_handle = texture_atlases.add(texture_atlas);

    let tiles_handle = asset_server.load("alltiles.png");
    let tiles_texture_atlas = TextureAtlas::from_grid(tiles_handle, Vec2::new(16.0, 16.0), 3, 59);
    let _tiles_atlas_handle = texture_atlases.add(tiles_texture_atlas);
}

fn main() {
    App::build()
        .add_plugins(DefaultPlugins)
        .add_startup_system(setup.system())
        .run();
}

Running it with cargo run crashes more often than it works for me. Same "out of range" error as @tommygoris
Tried moving the loading of the atlases to two different setup systems as @rparrett got working, but that did not make any difference for me.

Rust 1.49.0 on Arch Linux.

@rparrett
Copy link
Contributor

I'm currently seeing this error again despite

[dependencies]
bevy = { git = "https:/mockersf/bevy/", branch = "biggest-resource", default-features = false }
[patch.crates-io]
bevy = { git = "https:/mockersf/bevy/", branch = "biggest-resource", default-features = false }

after introducing several new texture atlases. So #1208 may not be fixing the problem entirely.

It's possible that I'm doing something silly with the atlases or Cargo.toml, but as far as I can tell, the patch is working.

Compiling bevy v0.4.0 (https:/mockersf/bevy/?branch=biggest-resource#0acb0a60)

I threw the code online at https:/rparrett/taipo/tree/spritesheet-panic-two but I'm finding the panic to be stubbornly resisting my attempts to create an easier/faster way to reproduce it. It panics pretty reliably, but only as I am actively playing the game and building towers while enemies are moving/spawning. So that's probably not useful to anybody.

wasm.js:382 panicked at 'range end index 8952 out of range for slice of length 8160', /Users/robparrett/.cargo/git/checkouts/bevy-e5386b669060b10b/0acb0a6/crates/bevy_render/src/render_graph/nodes/render_resources_node.rs:329:26

@rparrett
Copy link
Contributor

rparrett commented Jan 11, 2021

This really just defies reproduction.

Curiously, in my case, maybe high dpi text is a factor? Or something else related to scaling. I wrote quick puppeteer script to play the game a bit, and absolutely could not reproduce this until I added defaultViewport: null to the browser options.

Without that, puppeteer runs the game in an 800x600 viewport, so it is not scaled.

This currently reproduces the issue on my system shortly after the third crab starts taking damage, about 120 seconds into play.

https:/rparrett/taipo-puppeteer/blob/main/index.js

Again, probably not useful to anyone, but might get me closer to producing a real test case of some sort.

@cart cart reopened this Jan 11, 2021
@cart
Copy link
Member

cart commented Jan 11, 2021

I'm leaving this open until we are sure the issue is actually resolved.

@rparrett
Copy link
Contributor

rparrett commented Jan 11, 2021

I won't be able to dig back into this for a little while, but I rebuilt with c434f57 and re-ran my puppeteer script and saw a panic in the same spot, at the same time.

Only new insight is that there's probably a small non-spritesheet sprite (shuriken) being despawned at that exact time or very close to it.

@rparrett
Copy link
Contributor

Managed to capture a big ol' stack trace, in case that's helpful. Curiously, the panic occurs in debug builds, but 20 seconds further into the game well after the last crab in the first wave has died.

http://vpaste.net/ONh22

@rparrett
Copy link
Contributor

rparrett commented Jan 12, 2021

Got the game running natively to confirm that it's not wasm-only.

Here's a log with tracing included. This consistently seems to happen immediately after pressing enter to complete a "typing target," but nothing interesting should be happening with sprites or spritesheets at that time. 3 text fields get updated and presumably some flex layout stuff might be going on.

I can't really make sense of the bevy_render traces, but maybe there's something interesting in there.

http://vpaste.net/RHMDB

edit: swapped pastebin link for one with the entire frame (but this frame is pretty much only processing that one event though, I think)

@rparrett
Copy link
Contributor

The one interesting thing I am doing with fonts, which isn't very interesting is that I am using two fonts.

There's one small font that gets loaded for some text on the loading screen.

And another 4mb thing with all the JP glyphs that's used for literally everything else.

After the game is loaded, the loading screen is despawned and that first font is never used again. (But the handle is held onto forever)

However, as a quick test, I just swapped that small font's name for the big one in its load call, and the panic doesn't seem to happen anymore.

So that'll be the avenue I explore if I'm able to later tonight.

@rparrett
Copy link
Contributor

I plowed into render_resources_node.rs a bit and think maybe there's something fishy going on around here:

https:/mockersf/bevy/blob/0acb0a60cdf4dab96b45cf3a99bba62acada3af8/crates/bevy_render/src/render_graph/nodes/render_resources_node.rs#L293

Where we seem to be creating new larger buffers for resources with non-aligned buffer sizes, but maybe not making space in the staging buffer for them?

I'm 98% sure though that the above sentence is nonsense because I have no idea what's going on in there. I attempted a few fixes based on that theory, but was unsuccessful.

Instead I de-wasmed my game and wrote a new plugin that makes it automatically play itself until it panics. I hope someone with better rendering credentials might take pity on me and check it out.

I do think this might have something to do with UI text as well. It seems like the various text fields actually getting updated/turned green is important to reproducing the panic.

https:/rparrett/taipo/tree/spritesheet-panic-two

@rparrett
Copy link
Contributor

rparrett commented Jan 24, 2021

Here is a much more minimal reproduction of this (or a very similar) panic using text only.

https:/rparrett/bevy-test/tree/render-panic

@rparrett
Copy link
Contributor

rparrett commented Jan 26, 2021

I pushed a commit to the branch above that reproduces the same panic in a very simple system.

The gist is that I render a bunch of different glyphs in a single text bundle, one at a time. 14 seconds in (on my system) it panics.

If I understand text rendering properly, this may still be some sort of a "multiple texture atlases" issue.

rparrett added a commit to rparrett/bevy that referenced this issue Jan 27, 2021
rparrett added a commit to rparrett/bevy that referenced this issue Jan 27, 2021
rparrett added a commit to rparrett/bevy that referenced this issue Feb 3, 2021
rparrett added a commit to rparrett/bevy that referenced this issue Feb 10, 2021
rparrett added a commit to rparrett/bevy that referenced this issue Feb 19, 2021
rmsc added a commit to rmsc/bevy that referenced this issue Feb 23, 2021
The required staging buffer size is already properly calculated in
prepare_staging_buffers(). Whenever a resize was needed, this value
got overwritten by set_required_staging_buffer_size_to_max(), which
computed the wrong (smaller) size. This in turn prevented the staging
buffers from being resized in resize_staging_buffer().

By removing set_required_staging_buffer_size_to_max() entiery, the value
from prepare_staging_buffers() is used.
@rmsc
Copy link
Contributor

rmsc commented Feb 23, 2021

This should be fixed by #1509, could you please try it out and see if the problem disappears for good?

rmsc added a commit to rmsc/bevy that referenced this issue Feb 26, 2021
    The required_staging_buffer_size is currently calculated differently
    in two places, each will be correct in different situations:

    * prepare_staging_buffers() based on actual buffer_byte_len()
    * set_required_staging_buffer_size_to_max() based on item_size

    Up to this commit, and in case of a resize, the latter always took
    precedence, even if the resulting buffer size would be smaller than
    calculated by the former. This commit always uses the largest
    calculated size.
rmsc added a commit to rmsc/bevy that referenced this issue Feb 28, 2021
* prepare_uniform_buffers() now computes the required uniform buffer
  size over all assets, rather than just changed ones.

* set_required_staging_buffer_size_to_max() now doesn't overwrite the
  value computed by prepare_uniform_buffers() if the resulting size
  would be smaller.
rmsc added a commit to rmsc/bevy that referenced this issue Mar 3, 2021
* when doing a full asset copy (resize), prepare_uniform_buffers() is
  now called on all assets rather than just changed ones. This makes
  sure the staging buffer is large enough.

* set_required_staging_buffer_size_to_max() now doesn't overwrite the
  value computed by prepare_uniform_buffers() if the resulting size
  would be smaller.

Co-authored-by: François <[email protected]>
rmsc added a commit to rmsc/bevy that referenced this issue Mar 3, 2021
* when doing a full asset copy (resize), prepare_uniform_buffers() is
  now called on all assets rather than just changed ones. This makes
  sure the staging buffer is large enough.

* set_required_staging_buffer_size_to_max() now doesn't overwrite the
  value computed by prepare_uniform_buffers() if the resulting size
  would be smaller.

Co-authored-by: François <[email protected]>
rmsc added a commit to rmsc/bevy that referenced this issue Mar 4, 2021
* when doing a full asset copy (resize), prepare_uniform_buffers() is
  now called on all assets rather than just changed ones. This makes
  sure the staging buffer is large enough.

* set_required_staging_buffer_size_to_max() now doesn't overwrite the
  value computed by prepare_uniform_buffers() if the resulting size
  would be smaller.

Co-authored-by: Renato Caldas <[email protected]>
Co-authored-by: François <[email protected]>
@bors bors bot closed this as completed in 87399c3 Mar 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen P-Crash A sudden unexpected crash
Projects
None yet
7 participants