-
Notifications
You must be signed in to change notification settings - Fork 809
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
Optimize Chunkmap Rendering #2785
base: dev
Are you sure you want to change the base?
Optimize Chunkmap Rendering #2785
Conversation
Drawing a rectangle of NULL_STATUS_COLOR covering the entire map background allows ignoring any null ChunkStatus entries. By iterating in a spiral inwards->outwards (the same order chunks will appear when loading a world) and counting the amount of statuses we can terminate the loop early, saving a lot of expensive ChunkStatus lookups. Additionally, this commit implements a system to combine tiles of the same status to drastically reduce the amount of quads rendered. This does break vanilla parity when mapPadding is non-zero, however vanilla only ever sets it to 0 anyway. It's probably left over code from debugging as it separates tiles visually. We could even use this to implement our own debugging view, showing how tiles are combined, by replacing '+ tileSize' with '+ mapScale' and subtracting 'mapPadding' from x2 and y2 when drawing the inner rectangle. This preserves the same rendering when mapPadding is 0. Also fixed DEFAULT_STATUS_COLOR status being packed as the wrong format, causing the (only visible with mapPadding != 0 and never with this commit) blue square outline to instead be red.
@@ -35,16 +35,20 @@ public class LevelLoadingScreenMixin { | |||
private static final int NULL_STATUS_COLOR = ColorABGR.pack(0, 0, 0, 0xFF); | |||
|
|||
@Unique | |||
private static final int DEFAULT_STATUS_COLOR = ColorARGB.pack(0, 0x11, 0xFF, 0xFF); | |||
private static final int DEFAULT_STATUS_COLOR = ColorABGR.pack(0, 0x11, 0xFF, 0xFF); |
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 it intentional that the color format was changed but rgb values did not? this means the color would be a different one now.
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.
Yes, it was encoded in the wrong format before, causing it to be rendered red instead of the blue in vanilla. nobody ever noticed because this only ever gets rendered when mapPadding is non-zero, which it never is in vanilla
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.
cool, thanks for fixing it
Unless there's some obvious way in which another mod uses mapPadding that conflicts with this interpretation, I think what you showed for mapPadding looks fine. |
This was originally written and profiled in Minecraft 1.16.1, where it was 3x faster. (72000ms -> 24000ms over 10 minutes, rendering 4 chunkmaps at once with unlimited framerate)
Drawing a rectangle of NULL_STATUS_COLOR covering the entire map background allows ignoring any null ChunkStatus entries. By iterating in a spiral inwards->outwards (the same order chunks will appear when loading a world) and counting the amount of statuses we can terminate the loop early, saving a lot of expensive ChunkStatus lookups. Additionally, this commit implements a system to combine tiles of the same status to drastically reduce the amount of quads rendered.
This does break vanilla parity when mapPadding is non-zero, however vanilla only ever sets it to 0 anyway. It's probably left over code from debugging as it separates tiles visually. We could even use this to implement our own debugging view, showing how tiles are combined, by replacing '+ tileSize' with '+ mapScale' and subtracting 'mapPadding' from x2 and y2 when drawing the inner rectangle. This preserves the same rendering when mapPadding is 0.
Also fixed DEFAULT_STATUS_COLOR status being packed as the wrong format, causing the (only visible with mapPadding != 0 and never with this commit) blue square outline to instead be red.
Considerations:
It's never used in vanilla code (and as far as I know also not by any mods), so it probably shouldn't be an issue? Current implementation will simply render a normal chunkmap with increased size and it breaks centering (vanilla does that too).
It could also be switched to the mentioned debugging view, then it would still be consistent with vanilla in being a debug tool, while also looking kind of cool (atleast to me who knows why it looks the way it does :P).
If its necessary for Sodium to render this identical to vanilla the best solution would probably be to just add a different rendering path, trying to disable batching in the same loop would become messy.
Because of reduced spawn chunks, the chunkmap now covers a lot less chunks than it used to in 1.16.1 where I profiled. I'll try to figure out a good way to profile this and report results.
Code things I might do: