Skip to content

Remove warning from Image::resize #19116

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
merged 2 commits into from
May 26, 2025

Conversation

SpecificProtagonist
Copy link
Contributor

Objective

Image::resize currently prints a warning when resizing an uninitialized Image, even though the resizing works correctly. For code that doesn't care about whether the image is initialized CP-side, this is inconvenient and unnecessary.

@SpecificProtagonist SpecificProtagonist added A-Rendering Drawing game state to the screen S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 7, 2025
@@ -850,16 +849,14 @@ impl Image {
}

/// Resizes the image to the new size, by removing information or appending 0 to the `data`.
/// Does not properly resize the contents of the image, but only its internal `data` buffer.
/// Does not properly scale the contents of the image.
pub fn resize(&mut self, size: Extent3d) {
self.texture_descriptor.size = size;
Copy link
Member

@Vrixyz Vrixyz May 17, 2025

Choose a reason for hiding this comment

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

Nit: It's probably out of scope for this PR, but setting the size when we don't have any data feels weird. I'd put this in the condition 🤔 (and comment that it does nothing when called with a self.data equal to None.

The best would probably to return an Error, so user can react, but as self.data is exposed the test can just exist in user space.

@Vrixyz Vrixyz added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 17, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 26, 2025
Merged via the queue into bevyengine:main with commit 57eda2e May 26, 2025
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 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