Skip to content

feat(bevy_image): Support tileset grids to be created through the ImageArrayLayout #24132

Open
Blue-Pilkinton-Ching wants to merge 2 commits intobevyengine:mainfrom
Blue-Pilkinton-Ching:main
Open

feat(bevy_image): Support tileset grids to be created through the ImageArrayLayout #24132
Blue-Pilkinton-Ching wants to merge 2 commits intobevyengine:mainfrom
Blue-Pilkinton-Ching:main

Conversation

@Blue-Pilkinton-Ching
Copy link
Copy Markdown

@Blue-Pilkinton-Ching Blue-Pilkinton-Ching commented May 5, 2026

Objective

  • This issue explains how it's not possible to construct a tileset that has tiles arranged in a grid. Currently it's only possible to construct a tileset using a single column of vertical layers.

Fixes #23492.

Solution

  • This PR fixes that by giving the user the ability to pass the number of rows / columns in an image/tileset OR the pixel width/height of each tile in the image, and then applying a transformation step to the image data to have it ordered correctly in the texture data Vector.

Eg:

    let tileset_handle = asset_server
        .load_builder()
        .with_settings::<ImageLoaderSettings>(|s| {
            s.array_layout = Some(ImageArrayLayout::GridCount {
                columns: 5,
                rows: 5,
            })
        })
        .load("tilesets/my_tileset.png");

or

    let tileset_handle = asset_server
        .load_builder()
        .with_settings::<ImageLoaderSettings>(|s| {
            s.array_layout = Some(ImageArrayLayout::GridSize {
                tile_width_pixels: 16,
                tile_height_pixels: 16,
            })
        })
        .load("tilesets/my_tileset.png");

Testing

  • Both enum varients tested with a grid based tileset and working on Macos.

This PR could be expanded upon by supporting padding on tilesets or gaps between tiles.

This is my first contribution to open source 🫣

…rayLayout struct to support tileset textures
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Welcome, new contributor!

Please make sure you've read our contributing guide, as well as our policy regarding AI usage, and we look forward to reviewing your pull request shortly ✨

@ChristopherBiscardi
Copy link
Copy Markdown
Contributor

I'm walking away from my computer but saw the PR go up :D People will love having this, and the function on Image is the right place for the functionality IMO. CI isn't passing yet but I grabbed a kenney tilemap and used the new GridCount variant to successfully get the tilemap rendering

image image image

@ChristopherBiscardi ChristopherBiscardi self-requested a review May 5, 2026 05:44
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 5, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Rendering May 5, 2026
@alice-i-cecile alice-i-cecile added the C-Feature A new feature, making something new possible label May 5, 2026
Copy link
Copy Markdown
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.

Could we add a few tests for this? This is the sort of "tricky math" code that would really benefit from good unit testing.

@beicause
Copy link
Copy Markdown
Member

beicause commented May 6, 2026

The name reinterpret_grid_2d_as_array feels off - it's not free like other reinterpret functions.

@Blue-Pilkinton-Ching
Copy link
Copy Markdown
Author

Actually what I've currently got here I think is a bit of a sloppy solution.

  1. I suggest I consolidate reinterpret_stacked_2d_as_array and reinterpret_grid_2d_as_array into a single function which transforms the pixel data of the image into the correct array format to be read by the gpu.
  • In this function I can add a check to see if the grid only contains a single column in which case we can skip most of the overhead from the pixel data transformation logic.
  1. I also suggest that we rename the array_layout field of ImageLoaderSettings to grid_layout, have it be a type of Option<ImageGridLayout>, which is an enum of two varients: TileCount and TileSize. This would be breaking, but I think would be simpler and feel more intuitive to users of the api. In my experience most tilesets are structured in grids not vertical columns.

Do these changes sound like the right approach? I'll see if I can get to adding an option for setting padding & gaps on tiles as well.

@ChristopherBiscardi
Copy link
Copy Markdown
Contributor

I think is a bit of a sloppy solution

I think adding a function instead of making breaking changes is fine, especially this late in 0.19's cycle if you want this to get in for the rc. We can always refactor in the next cycle to deprecate/unify functions.

In my experience most tilesets are structured in grids not vertical columns.

reinterpret_stacked_2d_as_array is not a function specific to tilesets. For example, this asset which is used to showcase 2d array textures with a 3d cube rendering


In the interest of landing this, IMO the only things that are necessary vs the current version of the PR are:

  • a passing CI (this PR hasn't passed CI yet)
  • the tests Alice asked for
  • a function name that doesn't use reinterpret, since we are currently using that word to mean "only mutates Extent3d". "convert` is another option but I don't have strong opinions on naming, and we don't have much of a convention here.

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 C-Feature A new feature, making something new possible S-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

A TilemapChunk should accept a grid arranged tileset

4 participants