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

[rtextures] implemented fill color TODO in ImageResizeCanvas() #3720

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

LievenPetersen
Copy link
Contributor

ImageResizeCanvas() accepts a fill color, that was so far unused, with a TODO to implement it.

This is a fairly simple implementation that fills in the entire canvas, with a minor optimization to copy entire rows once the first one is filled.
The implementation is similar to ImageClearBackground(), this could possibly be unified in the future.

I was unsure about the desired level of code commenting, so I opted for extensive comments, in line with the contributing guide.

Tested on Linux:

  • build
  • functionality
  • with valgrind (for invalid read/writes)

@LievenPetersen
Copy link
Contributor Author

I could also upgrade this to either use ImageClearBackground() or to not fill the pixels that will be occupied by the old image's content.

@raysan5 raysan5 merged commit a820c37 into raysan5:master Jan 11, 2024
@raysan5
Copy link
Owner

raysan5 commented Jan 11, 2024

@LievenPetersen Thanks for the review!

I could also upgrade this to either use ImageClearBackground() or to not fill the pixels that will be occupied by the old image's content.

I'm thinking about that... actually, if image canvas is smaller than current canvas, it wouldn't be needed to fill anything... but I also think if the filling process is fast enough to skip that sequence of checks with offset/size...

I'm merging for now. If it can be optimized in the future, it can be reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants