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

texture atlas builder: use correct pixel size instead of 4 #2920

Closed
wants to merge 2 commits into from

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented Oct 6, 2021

Objective

Solution

  • Replace the hard coded pixel with one using the texture pixel size

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Oct 6, 2021
@mockersf mockersf added C-Bug An unexpected or incorrect behavior and removed S-Needs-Triage This issue needs to be labelled labels Oct 6, 2021
@DJMcNab
Copy link
Member

DJMcNab commented Oct 6, 2021

Obviously it's possible to avoid this allocation, but it might need to change the interface of new_fill

Alternatively, we could just have a static array of zeroes long enough and just slice into it.

@mockersf
Copy link
Member Author

mockersf commented Oct 6, 2021

Obviously it's possible to avoid this allocation, but it might need to change the interface of new_fill

new_fill is able to take more than one pixel and repeat it across the texture. It seems hard to modify it for this without losing this functionality... but then it's something that was done a long time ago 6164ea6 and I'm not sure anyone used it that way or is even aware of that...

Alternatively, we could just have a static array of zeroes long enough and just slice into it.

Today the max size is 16, but I suppose that could change and this would be forgotten.

@DJMcNab
Copy link
Member

DJMcNab commented Oct 6, 2021

We could add a comment to the pixel_size function saying to check this method, or else make the pixel size a macro and have a way to get the largest one as a constant

1 similar comment
@DJMcNab
Copy link
Member

DJMcNab commented Oct 6, 2021

We could add a comment to the pixel_size function saying to check this method, or else make the pixel size a macro and have a way to get the largest one as a constant

@mockersf
Copy link
Member Author

mockersf commented Oct 6, 2021

using the new method instead of new_fill, now we're back to only one alloc 👍

@DJMcNab
Copy link
Member

DJMcNab commented Oct 6, 2021

Awesome, thanks!

@mockersf
Copy link
Member Author

replaced by #2977 on pipelined rendering branch

@mockersf mockersf closed this Oct 15, 2021
bors bot pushed a commit that referenced this pull request Oct 28, 2021
# Objective

- Fixes #2919 
- Initial pixel was hard coded and not dependent on texture format
- Replace #2920 as I noticed this needed to be done also on pipeline rendering branch

## Solution

- Replace the hard coded pixel with one using the texture pixel size
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-Bug An unexpected or incorrect behavior
Projects
None yet
3 participants