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

Add a way to specify padding/ margins between sprites in a TextureAtlas. #460

Merged
merged 9 commits into from
Oct 15, 2020

Conversation

sY9sE33
Copy link
Contributor

@sY9sE33 sY9sE33 commented Sep 8, 2020

Occasionally, it's useful to be able to specify margins between sprites in a TextureAtlas, like for using Kenney.nl sprite sheets
e.g.
roguelikeSheet_transparent
(https://www.kenney.nl/assets/roguelike-rpg-pack)

@karroffel karroffel added A-Assets Load files from disk to use for things like images, models, and sounds C-Feature A new feature, making something new possible labels Sep 8, 2020
Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think theres actually a minor bug here. If you try using this with the linked Kenny asset, we get incorrect results. (the asset has 1x1 px padding, 57 columns, and 31 rows)

You might also notice that the kenny asset already works on master with 57 columns and 31 rows. This is because texture_width and texture_height resolve to 16.982456 x 16.967741 and this just happens to render the same as 16x16 (the real tile size).

I think the real "fix" here is to change the api to something a bit more natural:

from_grid(tile_size: Vec2, columns: usize, rows: usize)
from_grid_with_padding(tile_size: Vec2, columns: usize, rows: usize, padding: Vec2)

That should both simplify the calculations and make it easier to reason about

@sY9sE33
Copy link
Contributor Author

sY9sE33 commented Sep 11, 2020

I hope that I understood your comment, @cart. I was also unsure whether the TextureAtlass size should be the size of the texture or the what I currently have (tile size * columns/ rows).

Thanks for the comments!

@cart
Copy link
Member

cart commented Sep 11, 2020

Yeah I think i made the wrong choice when I wrote the initial from_grid api, so thats on me 😄

I'll take a look at the changes today

@cart
Copy link
Member

cart commented Sep 28, 2020

Oh no I forgot to look at this! Sorry about that. Looking now.

@cart
Copy link
Member

cart commented Sep 28, 2020

First, we'll want to update the sprite_sheet.rs example to account for this change. It uses 24x24 sprites, so the setup system should look like this:

fn setup(
    mut commands: Commands,
    asset_server: Res<AssetServer>,
    mut texture_atlases: ResMut<Assets<TextureAtlas>>,
) {
    let texture_handle = asset_server
        .load("assets/textures/rpg/chars/gabe/gabe-idle-run.png")
        .unwrap();
    let texture_atlas = TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1);
    let texture_atlas_handle = texture_atlases.add(texture_atlas);
    commands
        .spawn(Camera2dComponents::default())
        .spawn(SpriteSheetComponents {
            texture_atlas: texture_atlas_handle,
            scale: Scale(6.0),
            ..Default::default()
        })
        .with(Timer::from_seconds(0.1, true));
}

This currently doesn't render correctly because the Rect::min and Rect::max values aren't correct:

                sprites.push(Rect {
                    min: Vec2::new(tile_size.x() + x_padding, tile_size.y() + y_padding),
                    max: Vec2::new(
                        (x + 1) as f32 * tile_size.x(),
                        (y + 1) as f32 * tile_size.y(),
                    ),
                })

here min will always either be "tile_size" or "tile_size + padding". max handles tiles size offsets correctly, but it doesn't handle the padding (which also needs to be accounted for).

@sY9sE33
Copy link
Contributor Author

sY9sE33 commented Oct 3, 2020

I got a little excited last time and pushed before really thinking about it. The code should now work as I was expecting it to. Sorry!

@sY9sE33 sY9sE33 requested a review from cart October 15, 2020 01:58
@cart cart merged commit 9c48e5c into bevyengine:master Oct 15, 2020
joshuajbouw pushed a commit to joshuajbouw/bevy that referenced this pull request Oct 24, 2020
…as. (bevyengine#460)

Add a way to specify padding between sprites in a TextureAtlas
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants