-
Notifications
You must be signed in to change notification settings - Fork 828
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
Sort render regions by distance from camera #2133
Conversation
double dz = cameraPos.z - region.getCenterZ(); | ||
renderList.setDistanceFromCamera((dx * dx) + (dy * dy) + (dz * dz)); | ||
} | ||
this.lists.sort(LIST_DISTANCE_COMPARATOR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it might be faster to use unstableSort
here because that can use quicksort which is allocation free. I can't think of a reason for a stable sort to be necessary. ObjectArrayList
does a stable sort by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this implementation of quicksort O(n) on sorted inputs? I think sort
uses timsort which should have that behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah the default Arrays.sort
that it uses for stable sort might just be better. The quicksort is kinda hard to read so I'm not sure it'd actually fare better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm in favour of this PR
src/main/java/me/jellysquid/mods/sodium/client/render/chunk/lists/ChunkRenderList.java
Outdated
Show resolved
Hide resolved
The render regions should always be sorted as a matter of how the chunk traversal algorithm works. If that isn't happening, it means the traversal algorithm is broken. I would much rather fix the underlying issue rather than try to workaround it. |
Fixes #2132 by ensuring the list of render regions is always sorted by distance from the camera.