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

Remove unnecessary cast in DynamicTextureAtlasBuilder #16937

Merged

Conversation

mgi388
Copy link
Contributor

@mgi388 mgi388 commented Dec 22, 2024

Summary

  • I started experimenting if TextureAtlas and friends can be moved to bevy_image. See Discord thread.
  • While doing that, and moving DynamicTextureAtlasBuilder to bevy_image, it revealed that DynamicTextureAtlasBuilder depends on bevy_render::GpuImage, but we can't have bevy_image depend on bevy_render.
  • The reason for the dependency is an assertion introduced in this PR.
  • It doesn't seem like there was a specific reason for that change, so should be safe to change it.
  • So instead of the cast, just look up asset_usage directly on the concrete Image type.
  • Also update the message which referred to a non-existent variable atlas_texture_handle (it was renamed during a subsequent refactor PR).

Testing

  • Checked on Discord if there was any known reason this had to stay like this.
  • CI builds it.

Copy link
Contributor

@kristoff3r kristoff3r left a comment

Choose a reason for hiding this comment

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

Seems reasonable

@kristoff3r kristoff3r added A-Rendering Drawing game state to the screen S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 23, 2024
@BenjaminBrienen BenjaminBrienen added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it D-Straightforward Simple bug fixes and API improvements, docs, test and examples C-Code-Quality A section of code that is hard to understand or change and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 24, 2024
@mockersf mockersf added this pull request to the merge queue Dec 24, 2024
Merged via the queue into bevyengine:main with commit 124f803 Dec 24, 2024
34 checks passed
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-Code-Quality A section of code that is hard to understand or change D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants