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 padding early to reduce memory #6

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

duckdoom5
Copy link

@duckdoom5 duckdoom5 commented May 14, 2024

You can remove the padding early to fit the culled faces into a u32, reducing the required memory even more.
We can also update the colFaceMasks array to use CHUNK_SIZE, since the padded values are never used.
We can even do the same for axisCols. We only need them to have a u64 row value to store the two additional bits.

I also updated the voxel neighbours checking loop to remove the corner bits from being processed twice. Excuse the amount of code, I personally have it as a function with callback in c++, but I don't know how to do it in Rust.

Note: Did not try to compile nor test the rust version, sorry if it doesn't. (I can help fix it, if so.) I'm using c++, and have verified it works there!

duckdoom5 added 3 commits May 14, 2024 17:24
You can remove the padding early to fit the culled faces into a u32, reducing the required memory even more.

Note: Did not try to compile, sorry if it doesn't
The face mask padded values are unused (Note the x and z are always + 1 in the lower loop)
We only need that to be a uint64 to include the two extra bits in each col
@mastamitche
Copy link

Appreciate the thinking and application.

After I added this to my modified version it needed tweaking pretty significantly a few overflows and other issues but all in all great performance boost!

If anyone would like the rust version ping me.

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