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

[Merged by Bors] - Added offset parameter to TextureAtlas::from_grid_with_padding #4836

Closed
wants to merge 3 commits into from

Conversation

FraserLee
Copy link
Contributor

@FraserLee FraserLee commented May 24, 2022

Objective

Increase compatibility with a fairly common format of padded spritesheets, in which half the padding value occurs before the first sprite box begins. The original behaviour falls out when Vec2::ZERO is used for offset.

See below unity screenshot for an example of a spritesheet with padding

Screen Shot 2022-05-24 at 4 11 49 PM

Solution

Tiny change to crates/bevy_sprite/src/texture_atlas.rs

Migration Guide

Calls to TextureAtlas::from_grid_with_padding should be modified to include a new parameter, which can be set to Vec2::ZERO to retain old behaviour.

from_grid_with_padding(texture, tile_size, columns, rows, padding)
                                  |
                                  V
from_grid_with_padding(texture, tile_size, columns, rows, padding, Vec2::ZERO)

More compatible with a fairly common format of padded sprite-sheets, in
which half the padding value occurs before the first sprite box begins.
@FraserLee FraserLee changed the title Added offset parameter to TextureAtlas::from_grid_with_padding Added offset parameter to TextureAtlas::from_grid_with_padding May 24, 2022
forgot a trailing space
@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds C-Usability A targeted quality-of-life change that makes Bevy easier to use labels May 25, 2022
@alice-i-cecile alice-i-cecile added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label May 30, 2022
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Tiny doc nit for clarity, then this LGTM. This is technically a breaking change; can you add a tiny note for the migration guide?

@alice-i-cecile's suggestion, & one other of the same type.
@FraserLee
Copy link
Contributor Author

I've edited the PR description. Lmk if there's anywhere else you'd like me to write a note for it :)

@alice-i-cecile
Copy link
Member

bors r+

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 30, 2022
bors bot pushed a commit that referenced this pull request May 30, 2022
…4836)

# Objective
Increase compatibility with a fairly common format of padded spritesheets, in which half the padding value occurs before the first sprite box begins. The original behaviour falls out when `Vec2::ZERO` is used for `offset`.

See below unity screenshot for an example of a spritesheet with padding

![Screen Shot 2022-05-24 at 4 11 49 PM](https://user-images.githubusercontent.com/30442265/170123682-287e5733-b69d-452b-b2e6-46d8d29293fb.png)

## Solution
Tiny change to `crates/bevy_sprite/src/texture_atlas.rs`

## Migration Guide

Calls to `TextureAtlas::from_grid_with_padding` should be modified to include a new parameter, which can be set to `Vec2::ZERO` to retain old behaviour.
```rust
from_grid_with_padding(texture, tile_size, columns, rows, padding)
                                  |
                                  V
from_grid_with_padding(texture, tile_size, columns, rows, padding, Vec2::ZERO)
```


Co-authored-by: FraserLee <[email protected]>
@bors bors bot changed the title Added offset parameter to TextureAtlas::from_grid_with_padding [Merged by Bors] - Added offset parameter to TextureAtlas::from_grid_with_padding May 30, 2022
@bors bors bot closed this May 30, 2022
james7132 pushed a commit to james7132/bevy that referenced this pull request Jun 7, 2022
…evyengine#4836)

# Objective
Increase compatibility with a fairly common format of padded spritesheets, in which half the padding value occurs before the first sprite box begins. The original behaviour falls out when `Vec2::ZERO` is used for `offset`.

See below unity screenshot for an example of a spritesheet with padding

![Screen Shot 2022-05-24 at 4 11 49 PM](https://user-images.githubusercontent.com/30442265/170123682-287e5733-b69d-452b-b2e6-46d8d29293fb.png)

## Solution
Tiny change to `crates/bevy_sprite/src/texture_atlas.rs`

## Migration Guide

Calls to `TextureAtlas::from_grid_with_padding` should be modified to include a new parameter, which can be set to `Vec2::ZERO` to retain old behaviour.
```rust
from_grid_with_padding(texture, tile_size, columns, rows, padding)
                                  |
                                  V
from_grid_with_padding(texture, tile_size, columns, rows, padding, Vec2::ZERO)
```


Co-authored-by: FraserLee <[email protected]>
bors bot pushed a commit that referenced this pull request Sep 24, 2022
…id through option arguments (#6057)

This is an adoption of #3775
This merges `TextureAtlas` `from_grid_with_padding` into `from_grid` , adding optional padding and optional offset.
Since the orignal PR, the offset had already been added to from_grid_with_padding through #4836 

## Changelog

- Added `padding` and `offset` arguments to  `TextureAtlas::from_grid`
- Removed `TextureAtlas::from_grid_with_padding`

## Migration Guide

`TextureAtlas::from_grid_with_padding` was merged into `from_grid` which takes two additional parameters for padding and an offset.
```
// 0.8
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1);
// 0.9
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1, None, None)

// 0.8
TextureAtlas::from_grid_with_padding(texture_handle, Vec2::new(24.0, 24.0), 7, 1, Vec2::new(4.0, 4.0));
// 0.9
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1, Some(Vec2::new(4.0, 4.0)), None)
```

Co-authored-by: olefish <[email protected]>
bors bot pushed a commit that referenced this pull request Sep 24, 2022
…id through option arguments (#6057)

This is an adoption of #3775
This merges `TextureAtlas` `from_grid_with_padding` into `from_grid` , adding optional padding and optional offset.
Since the orignal PR, the offset had already been added to from_grid_with_padding through #4836 

## Changelog

- Added `padding` and `offset` arguments to  `TextureAtlas::from_grid`
- Removed `TextureAtlas::from_grid_with_padding`

## Migration Guide

`TextureAtlas::from_grid_with_padding` was merged into `from_grid` which takes two additional parameters for padding and an offset.
```
// 0.8
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1);
// 0.9
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1, None, None)

// 0.8
TextureAtlas::from_grid_with_padding(texture_handle, Vec2::new(24.0, 24.0), 7, 1, Vec2::new(4.0, 4.0));
// 0.9
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1, Some(Vec2::new(4.0, 4.0)), None)
```

Co-authored-by: olefish <[email protected]>
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 19, 2022
…id through option arguments (bevyengine#6057)

This is an adoption of bevyengine#3775
This merges `TextureAtlas` `from_grid_with_padding` into `from_grid` , adding optional padding and optional offset.
Since the orignal PR, the offset had already been added to from_grid_with_padding through bevyengine#4836 

## Changelog

- Added `padding` and `offset` arguments to  `TextureAtlas::from_grid`
- Removed `TextureAtlas::from_grid_with_padding`

## Migration Guide

`TextureAtlas::from_grid_with_padding` was merged into `from_grid` which takes two additional parameters for padding and an offset.
```
// 0.8
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1);
// 0.9
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1, None, None)

// 0.8
TextureAtlas::from_grid_with_padding(texture_handle, Vec2::new(24.0, 24.0), 7, 1, Vec2::new(4.0, 4.0));
// 0.9
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1, Some(Vec2::new(4.0, 4.0)), None)
```

Co-authored-by: olefish <[email protected]>
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
…id through option arguments (bevyengine#6057)

This is an adoption of bevyengine#3775
This merges `TextureAtlas` `from_grid_with_padding` into `from_grid` , adding optional padding and optional offset.
Since the orignal PR, the offset had already been added to from_grid_with_padding through bevyengine#4836 

## Changelog

- Added `padding` and `offset` arguments to  `TextureAtlas::from_grid`
- Removed `TextureAtlas::from_grid_with_padding`

## Migration Guide

`TextureAtlas::from_grid_with_padding` was merged into `from_grid` which takes two additional parameters for padding and an offset.
```
// 0.8
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1);
// 0.9
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1, None, None)

// 0.8
TextureAtlas::from_grid_with_padding(texture_handle, Vec2::new(24.0, 24.0), 7, 1, Vec2::new(4.0, 4.0));
// 0.9
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1, Some(Vec2::new(4.0, 4.0)), None)
```

Co-authored-by: olefish <[email protected]>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…id through option arguments (bevyengine#6057)

This is an adoption of bevyengine#3775
This merges `TextureAtlas` `from_grid_with_padding` into `from_grid` , adding optional padding and optional offset.
Since the orignal PR, the offset had already been added to from_grid_with_padding through bevyengine#4836 

## Changelog

- Added `padding` and `offset` arguments to  `TextureAtlas::from_grid`
- Removed `TextureAtlas::from_grid_with_padding`

## Migration Guide

`TextureAtlas::from_grid_with_padding` was merged into `from_grid` which takes two additional parameters for padding and an offset.
```
// 0.8
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1);
// 0.9
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1, None, None)

// 0.8
TextureAtlas::from_grid_with_padding(texture_handle, Vec2::new(24.0, 24.0), 7, 1, Vec2::new(4.0, 4.0));
// 0.9
TextureAtlas::from_grid(texture_handle, Vec2::new(24.0, 24.0), 7, 1, Some(Vec2::new(4.0, 4.0)), None)
```

Co-authored-by: olefish <[email protected]>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…evyengine#4836)

# Objective
Increase compatibility with a fairly common format of padded spritesheets, in which half the padding value occurs before the first sprite box begins. The original behaviour falls out when `Vec2::ZERO` is used for `offset`.

See below unity screenshot for an example of a spritesheet with padding

![Screen Shot 2022-05-24 at 4 11 49 PM](https://user-images.githubusercontent.com/30442265/170123682-287e5733-b69d-452b-b2e6-46d8d29293fb.png)

## Solution
Tiny change to `crates/bevy_sprite/src/texture_atlas.rs`

## Migration Guide

Calls to `TextureAtlas::from_grid_with_padding` should be modified to include a new parameter, which can be set to `Vec2::ZERO` to retain old behaviour.
```rust
from_grid_with_padding(texture, tile_size, columns, rows, padding)
                                  |
                                  V
from_grid_with_padding(texture, tile_size, columns, rows, padding, Vec2::ZERO)
```


Co-authored-by: FraserLee <[email protected]>
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-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants