-
Notifications
You must be signed in to change notification settings - Fork 77
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
Improve biome blending in 3D #1637
Conversation
|
||
@Override | ||
public boolean useBiomeTint() { | ||
return true; // (in reality it is only used for fern) |
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 should be fixed then (or does this method only check if the model uses it at all)?
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.
Returning true only for fern would be better. But from the function just above it seem the entityTag is needed to determine whether this is a fern or not. And because returning true when it should be false doesn't lead to a big adverse effect (it may lead to unnecessary work as more biome blending than necessary is performed), I just was lazy and made it simpler
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.
Actually, this shouldn't be a problem at all. We replace all legacy blocks while loading a legacy chunk and let it create a new tag that takes the entity tag's information into account.
All the biome stuff is handled after converting the legacy blocks.
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.
Awesome! 🤩
The third optimization is smart… Could that save memory in 2d biomes too because we could have a "don't care" biome then? Similar how we have any
nodes in the octree?
@@ -989,6 +1003,9 @@ public synchronized void loadChunks(TaskTracker taskTracker, World world, Collec | |||
int octNode = currentBlock; | |||
Block block = palette.get(currentBlock); | |||
|
|||
if(block.useBiomeTint()) |
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.
Maybe?
if(block.useBiomeTint()) | |
if(block.isBiomeDependant()) |
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.
Why not, wasn't too sure of the name myself
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.
How about isBiomeTinted
? Biome dependant sounds like a block "design" can change depending on the biome.
It could be applicable with limited changes to the 2D case as well, if no block in the chunk uses biome color, the color for the biome doesn't need to be stored. We don't even need a "don't care" thing, we just won't store the color in the world texture in the first place. |
…g the blocks to know if they use biome blending
…racking whether biome is used int the chunk or not
…inate are negative had wildy wrong biome colors due to off by 1 error when testing if the chunk is loaded caused by int division rounding behavior
2cdf221
to
53cee6c
Compare
Rebased and resolved conflicts, I'll test again and merge this if it works now. 👍 |
This PR improves performance of doing biome blending during chunk loading.
The combined time of step 3 and 4 (loading chunks and finalization ; biome blending occurs during step 4) has roughly been halved based on my rough timings. (Sorry, no precise timings provided, I've been winging it a bit).
The improved performance and the improved asymptotic complexity of blur make it reasonable to do blur bigger than 333 so that's now possible and exposed in the UI.
As an added bonus that wasn't really intentional, memory usage has decreased when using trivial 3D biome structure (by virtue of not storing biome color for every block, more on that later).
The improved performances are achieved for 3 reasons:
The setting for enabling/disabling biome blending has been replace for a setting to choose the biome blending radius (and a radius of 0 means biome blending is disabled)
Sorry no screenshot of bigger radius, I'm too lazy and my test scene is stupid looking anyway