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

Optimize Chunkmap Rendering #2785

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

KingContaria
Copy link

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:

  • How should mapPadding be handled?

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.

  • How big is the impact with smaller chunkmaps in modern versions?

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:

  • reimplement caching the previous STATUS_TO_COLOR_FAST access
  • try to merge the x and y loops, there's a lot of duplicate code there and it seems doable

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.
@KingContaria
Copy link
Author

Screenshot of the mentioned debug view because it does really show how tiles are combined very well:

grafik

@@ -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);
Copy link
Collaborator

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.

Copy link
Author

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

Copy link
Collaborator

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

@douira
Copy link
Collaborator

douira commented Oct 1, 2024

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.

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