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 TextureAtlasLoader #11873

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

lynn-lumen
Copy link
Contributor

@lynn-lumen lynn-lumen commented Feb 15, 2024

Objective

Solution

  • Adding an AssetLoader for TextureAtlasLayout The loader has three extensions:
    • atlas.ron: Creates a TextureAtlasLayout with the specified size and texture rectangles.
    • atlas.grid.ron: Creates a TextureAtlasLayout with the specified grid, similar to TextureAtlasLayout::from_grid.
    • atlas.composed.ron: Creates a TextureAtlasLayout from the specified source textures by composing them using TextureAtlasBuilder. The composed image returned by the TextureAtlasBuilder is added as a labeled asset: composed_texture .

The AssetLoader implementation is based on @viridia's code as posted in the original issue

@lynn-lumen lynn-lumen changed the title Add TextureAtlasLayoutLoader Add TextureAtlasLoader Feb 15, 2024
@alice-i-cecile alice-i-cecile added C-Enhancement A new feature A-Rendering Drawing game state to the screen A-Assets Load files from disk to use for things like images, models, and sounds labels Feb 15, 2024
@viridia
Copy link
Contributor

viridia commented Feb 16, 2024

I think this would go a lot better if there was an example showing how to use it. For example, imagine a novice user who wants to spawn an AtlasImageBundle. That accepts a TextureAtlas, not a TextureAtlasLayout, but it won't be obvious to the novice how to get one from the other (it's not obvious to me either).

My original PR assumed that you could spawn an AtlasImageBundle in just two steps:

  • Load a TextureAtlas resource and get a handle. This would automatically load the image as a dependency of the atlas, since the texture atlas resource would include a relative path to the image.
  • Spawn an AtlasImageBundle using the returned handle.

However, that no longer works due to the changes in the way texture atlases work.

@lynn-lumen
Copy link
Contributor Author

I think this would go a lot better if there was an example showing how to use it. For example, imagine a novice user who wants to spawn an AtlasImageBundle. That accepts a TextureAtlas, not a TextureAtlasLayout, but it won't be obvious to the novice how to get one from the other (it's not obvious to me either).

There already are some examples demonstrating how to use TextureAtlas and TextureAtlasLayout. However I agree, that an example demonstrating how to load a TextureAtlasLayout is useful.

I have added an example like that now, but I would appreciate someone reading over that since my English is not that good.

Copy link
Contributor

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@lynn-lumen lynn-lumen marked this pull request as ready for review February 16, 2024 13:39
@viridia
Copy link
Contributor

viridia commented Feb 17, 2024

Looks like there are some problems with the dependencies. Otherwise looks good.

@lynn-lumen
Copy link
Contributor Author

lynn-lumen commented Feb 17, 2024

Looks like there are some problems with the dependencies.

This is apparently an issue on main. The test will get triggered anytime you modify a Cargo.toml and failing should be non-blocking for merging. I asked on discord because I was confused 😅

There's also a related issue: #11917

Comment on lines +4 to +7
(min: (0, 8), max: (16, 24)),
(min: (16, 8), max: (32, 24)),
(min: (32, 8), max: (48, 24)),
(min: (48, 8), max: (64, 24)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Having a name for each packed sprite here might be useful too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind, this is out of scope for this PR.

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 A-Rendering Drawing game state to the screen C-Enhancement A new feature
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

AssetLoader for TextureAtlas
4 participants