Fix Y-slices with num_emerge_threads > 1 #16224

ExeVirus · 2025-06-04T03:22:18Z

Goal

Allow reliable multi-threaded mapgen (outside pure lua mapgen) by removing issues arising from race conditions related to Y direction overgeneration and more than one emerge thread, Solved in this PR by always setting those two overgenerated layers back to CONTENT_IGNORE after using it to generate dust/biome/etc.

Resolves #9357

To do

Not probably following coding standards
The removeOvergeneratedCStone() function probably needs a better name
The removeOvergeneratedCStone() function also probably needs to be made more effecient - I'm rusty on Vmanip.

This PR is Ready for Review.

How to test

Load up a new world, with num_emerge_threads > 1 in your favorite game (mtg or devtest, or otherwise), toggle on fly,fast and start flying around. Look for bad y-slices in master vs this branch.

Other ways to test:

You can use this especially made "game" just for hunting this bug:

https://codeberg.org/perfect_city_game_studio/mapgen_test

I'm not the author, this is from the original issue thread

OR

You can load up any game, with a world seed of "2" and select mapgen v7, have a high num emerge threads, and teleport to 50,50,500.

kno10 · 2025-06-04T10:03:21Z

I had been considering something similar, but only setting these back to IGNORE if they have (A) not been generated by the other chunk, (B) not been changed after initial generation.
But I did not have time to do this in code - make a copy of the y slices overgenerated, if not generated already - and set to IGNORE if they are the same just before writing.
The reason why this may be necessary are decorations.

Consider tall grass, two nodes high, placed at the top "valid" slice. It will write the upper half into this overgenerated area. Unconditionally setting that back to IGNORE might clip this. This might also apply to trees.

sfan5 · 2025-06-04T10:35:16Z

To me this looks like a bad hack. If we're placing too much stone, then removing it after the fact, why not fix the placement in the first place?

kno10 · 2025-06-04T10:57:49Z

To me this looks like a bad hack. If we're placing too much stone, then removing it after the fact, why not fix the placement in the first place?

Overgeneration is necessary for some decoration placement, such as dust. Dust placement depends on the node below, hence if the chunk below is not yet generated, one y slice needs to be overgenerated.

However, I do not think overgenerated base terrain should be written.

Minecraft may (judging by some youtube videos on MC terrain generation) be doing a multi-step emerge to avoid such reasons.

For this, every block would have an integer emerge state phase. To emerge a block to a state, all neighboring blocks must at least be in the previous state. Phases could be: base terrain, then cavegen, then dust, then decorations. Or maybe even dust after decorations to add snow cover? This will then allow such phases to reliably touch neighboring chunks.
This would also be great for lua mapgens to either do only the first, or only the later phases.

ExeVirus · 2025-06-04T11:02:23Z

kno10: very good point, had not considered the tall grass and trees being why we probably do this Y slice overgeneration in the first place.

Will take a look tonight, but you are probably correct and the fix is simply to check if the node is c_stone, and if so, set it back to content ignore.

Sfan5: don't think we can simply get rid of overgeneration in general, it's required for our algorithms to know if the topmost node is covered or exposed, so things like dust and top layer fills work correctly. After those pieces of generation are complete, there's no reason to keep the left over c_stone, so removing it eliminates that complexity.

Haven't confirmed yet, but our mapblock writing must skip Content ignores, because I expected that doing this would have caused an entire empty layer.

ExeVirus · 2025-06-04T11:39:20Z

kno10: very good point, had not considered the tall grass and trees being why we probably do this Y slice overgeneration in the first place.

Will take a look tonight, but you are probably correct and the fix is simply to check if the node is c_stone, and if so, set it back to content ignore.

Sfan5: don't think we can simply get rid of overgeneration in general, it's required for our algorithms to know if the topmost node is covered or exposed, so things like dust and top layer fills work correctly. After those pieces of generation are complete, there's no reason to keep the left over c_stone nodes, so removing them eliminates that complexity.

Haven't confirmed yet, but our mapblock writing must skip Content ignores, because I expected that doing this would have caused an entire empty layer.

Edit: I'm also personally against the stages thing short term.

sfan5 · 2025-06-04T12:47:06Z

Overgeneration is necessary for some decoration placement, such as dust. Dust placement depends on the node below, hence if the chunk below is not yet generated, one y slice needs to be overgenerated.

However, I do not think overgenerated base terrain should be written.

Right, that makes sense.
Perhaps we should be marking overgenerated terrain nodes with a flag (e.g. VOXELFLAG_CHECKED2) and then delete only those afterwards.

ExeVirus · 2025-06-04T13:02:00Z

We want some* over generated nodes (decor, dust, structures, etc.), just not the default terrain perlin noise generation step nodes. Is the vmanip able to store more information than just content_id? I know we are calculating lighting too, and that depends on those overgenerated nodes as well, so we can't use paramtype1.

Perhaps we could use paramtype2 in combination with what node type it is (c_stone, c_water, etc.) to flag for deletion. Of course, structures often use stone, so not a perfect solution.

sfan5 · 2025-06-04T13:15:15Z

param0, param1 and param2 all need to be preserved but like I said we have a flag array.

luanti/src/voxel.h

Lines 350 to 356 in fde6384

    
           enum : u8 { 
        
           	VOXELFLAG_NO_DATA  = 1 << 0, // no data about that node 
        
           	VOXELFLAG_CHECKED1 = 1 << 1, // Algorithm-dependent 
        
           	VOXELFLAG_CHECKED2 = 1 << 2, // Algorithm-dependent 
        
           	VOXELFLAG_CHECKED3 = 1 << 3, // Algorithm-dependent 
        
           	VOXELFLAG_CHECKED4 = 1 << 4, // Algorithm-dependent 
        
           };

luanti/src/voxel.h

Lines 508 to 511 in fde6384

    
           	/* 
        
           		Flags of all nodes 
        
           	*/ 
        
           	u8 *m_flags = nullptr;

ExeVirus · 2025-06-04T14:05:46Z

Will have to look at it later, but my first thought with marking "over-generated" is how will we know lua callbacks during mapgen have changed the vmanip and ensure we remove the flag. For example, say my lua callbacks set that layer to stone on purpose, then I would want to remove that flag - but we won't be able to tell an edit has been made if the whole layer was previously stone.

I.e. if paramtype0, 1, 2 don't change after lua mapgen, how will we know NOT to remove that node if it was intentionally meant to be that paramtype 0,1,2?

sfan5 · 2025-06-04T14:09:27Z

True, no idea. Would removing overgenerated stuff before handing things to Lua work?

ExeVirus · 2025-06-04T14:20:34Z

True, no idea. Would removing overgenerated stuff before handing things to Lua work?

Just took a look:

luanti/src/emerge.cpp

Lines 723 to 736 in fde6384

    
           	m_mapgen->makeChunk(&bmdata); 
        
           } 
        
           { 
        
           	ScopeProfiler sp(g_profiler, 
        
           		"EmergeThread: Lua on_generated", SPT_AVG); 
        
           	try { 
        
           		m_script->on_generated(&bmdata, m_mapgen->blockseed); 
        
           	} catch (const LuaError &e) { 
        
           		m_server->setAsyncFatalError(e); 
        
           		error = true; 
        
           	} 
        
           }

Yep, that totally works: lua comes into the picture after we have completed C++ mapgen makeChunk() entirely (via gennotify callbacks), so will be looking to implement the flag solution when I have time tonight to address.

Desour · 2025-06-04T14:31:40Z

Instead of removing the nodes there, what about not iterating one too much:

luanti/src/mapgen/mapgen_v7.cpp

Line 540 in fde6384

for (s16 y = node_min.Y - 1; y <= node_max.Y + 1;

ExeVirus · 2025-06-04T14:40:45Z

Instead of removing the nodes there, what about not iterating one too much:

See above reasons:

Overgeneration is necessary for some decoration placement, such as dust. Dust placement depends on the node below, hence if the chunk below is not yet generated, one y slice needs to be overgenerated.

It's required for our algorithms to know if the topmost node is covered or exposed, so things like dust and top layer fills work correctly. After those pieces of generation are complete, there's no reason to keep the left over c_stone, so removing it eliminates that complexity.

To walk you through a full example, imagine we're doing normal MTG mapgen_v7. I have a big mountain, and I'm generating a middle chunk of that mountain as my very first mapcunk for the entire map. The topmost layer will be generating below an entire layer of CONTENT_IGNORE. For biome generation to work, it would check the node above, see if it's AIR or IGNORE, and if so - assume that this is the top of the mountain, and set all those nodes to grass nodes. But that would be wrong, it should still be stone because the next mapchunk above (that hasn't been generated yet) will be covering that layer.

So to make each mapchunk aware of what is above and below it, we overgenerate +-1 Y direction with the base terrain (stone + water). Now biome and dust and decorations actually know if there is AIR above a node or not.

Since we must keep the overgeneration to keep everything else aware of what is going to be above or below, this solution thread aims to remove those crutches afterwards to clean up what is effectively the "worksite scrap material"

The complexity we're trying to address is to ensure that we only remove scrap and not the nodes we meant to actually put in that layer (decor, structures, trees)

ExeVirus · 2025-06-05T01:11:20Z

param0, param1 and param2 all need to be preserved but like I said we have a flag array.

Not going to help in the mapgen world, in mapgen world we only have a voxelarea, which only has a vector of mapnodes, which only have param0, param1, and param2.

Will look at what methods are available to us to flag such nodes, hopefully it's glaringly obvious how to accomplish such a feat in a bit.

ExeVirus · 2025-06-05T01:33:58Z

Update 1:

flat mapgen

num_emerge = 8

mgflat_ground_level = 46

map_persistence in mtg game.conf set to off (dummy backend)

As suspected, just setting all overgenerated layers to content ignore results in bad things for decor:

void MapgenBasic::removeOvergeneratedCStone()
{
	for (s16 z = node_min.Z; z <= node_max.Z; z++)
	for (s16 x = node_min.X; x <= node_max.X; x++) {
		u32 vi = vm->m_area.index(x, node_max.Y + 1, z); // top
		vm->m_data[vi].setContent(CONTENT_IGNORE);
		vi = vm->m_area.index(x, node_min.Y - 1, z);     // bottom
		vm->m_data[vi].setContent(CONTENT_IGNORE);
	}
}

And as proposed, only removing the overgenerated, leftover c_stone resolves that particular issue:

void MapgenBasic::removeOvergeneratedCStone()
{
	for (s16 z = node_min.Z; z <= node_max.Z; z++)
	for (s16 x = node_min.X; x <= node_max.X; x++) {
		u32 vi = vm->m_area.index(x, node_max.Y + 1, z); // top
		if (vm->m_data[vi].getContent() == c_stone) {
			vm->m_data[vi].setContent(CONTENT_IGNORE);
		}
		vi = vm->m_area.index(x, node_min.Y - 1, z);     // bottom
		if (vm->m_data[vi].getContent() == c_stone) {
			vm->m_data[vi].setContent(CONTENT_IGNORE);
		}
	}
}

Next steps

Need to go underground, hunting for dungeons, and see if this breaks them

ExeVirus · 2025-06-05T01:45:35Z

Update 2:

New failure mode: when overlapping mapchunks are both generating at the same time, and the one below makes big jungle trees, and the one above is not aware of those nodes so it just places air:

So wait, I understand overlapping at the +-1 boundary, WHY do we overlap entire chunk generation along x/z axes? For example, top down view really brings this into focus:

ExeVirus · 2025-06-05T01:52:19Z

Update 3:

Dungeons are fine, mostly because they use cobble, don't have a good test setup yet for finding the fallback stone-only dungeons, will try to get that going now.

Edit:

Confirmed with the mapgen "game" that was made, enabled dungeons, floated around at +48, -48 in flat mapgen with the fixes: seeing no issues (pink or yellow nodes are issues in this mapgen):

Currently testing with caves, nothing to report there yet.

ExeVirus · 2025-06-05T02:22:39Z

Update 4:

Caves seem absolutely fine, confirmed ores and air and water are all showing up correctly.

So with that, the +-y overgeneration issue seems resolved, but not the X/Z full mapblock overlap I'm seeing?

Any tips out there about where to look for that bad boy?

ExeVirus · 2025-06-05T02:43:45Z

Confirmed the X/Z bug is happening with both mapgen V7 and flat:

It always starts at 47 (in my tests), which is [Node Min.Y - 1], i.e. overgenerated terrain but also the rest of those mapblocks get written wrong (i.e. air).

Perhaps this could be fixed on-writeback to mapblocks: Don't allow air to replace non-air.

ExeVirus · 2025-06-05T03:05:58Z

Will address tomorrow I guess, out of time for now. Thinking I'll make changes to BlitBackAll(), so that instead of just taking the received mapgen blocks as gospel:

luanti/src/map.cpp

Lines 875 to 880 in fde6384

    
           if (!overwrite_generated && block->isGenerated()) 
        
           	continue; 
        
           block->copyFrom(*this); 
        
           block->raiseModified(MOD_STATE_WRITE_NEEDED, MOD_REASON_VMANIP); 
        
           block->expireIsAirCache();

I would do a "mergeFrom" instead of "copyFrom":

block->mergeFrom(*this);
block->raiseModified(MOD_STATE_WRITE_NEEDED, MOD_REASON_VMANIP);
block->expireIsAirCache();

Which this mergeFrom would not allow AIR or CONTENT_IGNORE to overwrite other terrain is my thinking. This way existing decor/map cannot be overwritten by air. And I would only use mergeFrom if BlitBackAll is provided some boolean flag like it is for overwrite_generated.

My immediate concern with such a change is not performance but actually wondering how this would affect re-generation requests from lua-side - would require resetting that chunk to content_ignore, or something.

sfan5 · 2025-06-05T07:24:22Z

Not going to help in the mapgen world, in mapgen world we only have a voxelarea, which only has a vector of mapnodes, which only have param0, param1, and param2.

❓
A VoxelArea is just two vectors that define the area and it contains no data. A VoxelManip always has a flag array, and this is what the mapgen is working with.

Which this mergeFrom would not allow AIR or CONTENT_IGNORE to overwrite other terrain is my thinking. This way existing decor/map cannot be overwritten by air.

I hope I'm not misunderstanding the problem but this again sounds like bandaid to me.
Why are we writing overgenerated terrain data at all?
Imagine this:

all nodes generated including overgeneration
decor placement and other stuff happens
overgenerated terrain (not decor) is erased again
everyone is happy

for a PoC you could implement this solution the way @kno10 said:

But I did not have time to do this in code - make a copy of the y slices overgenerated, if not generated already - and set to IGNORE if they are the same just before writing.

kno10 · 2025-06-05T09:12:33Z

Will take a look tonight, but you are probably correct and the fix is simply to check if the node is c_stone, and if so, set it back to content ignore.

I do not think you can rely on stone being the only thing overgenerated.

What if, e.g., a schematic places additional stone intentionally?

ExeVirus · 2025-06-05T10:30:07Z

I do not think you can rely on stone being the only thing overgenerated.

I think you are correct, I would assume c_water would also have an effect, I just haven't found a good way to induce that to be an issue.

What if, e.g., a schematic places additional stone intentionally?

Exactly my concern, and why I was originally excited about what sfan was talking about with a flag per node outside param0,1,2 ~~If such a thing exists~~ I can mark nodes from overgeneration as set for removal, and then after decor and friends, go back and remove anything not overwritten. Edit: I finally see them, didn't notice that MMVManip is a derived VoxelManipulator and that does have all such flags.

I hope I'm not misunderstanding the problem but this again sounds like bandaid to me.

I had hoped you or another core dev could shed some light on it too. I don't understand yet if/why we would have mapchunks offset slightly to overlap generation. Hopefully I'm wrong about that.

If I'm wrong about that guess, the answer lies in "not only is overgeneration an issue" but also "order of mapblock write-back matters with multi threaded mapgen". In my screenshots, that would mean the lower with jungle trees wrote first, and then the other mapchunk that was doing generation simultaneously wrote 2nd, which it thought all that space was air (correct for it's algorithm), since it started before the mapchunk below was done and had written back.

In that scenario, my proposed solution of not just blindly blitbackall() every mapblock from mapchunk generation when num_emerge_threads > 1 ~~, and instead treat air as a lower class citizen to be ignored~~ should work.

Why are we writing overgenerated terrain data at all?

Tree is made in one mapchunk at the very top, Y=46 in my pictures. It needs to extend far into the next mapchunk above. That's not overgeneration, per se, but same effect and issue with multi threaded mapgen.

In single threaded mapgen, those tree nodes are copied into the next mapchunk's starting vmanip, and our mapgen correctly knows to ignore and leave that alone. "Leave alone" is effectively saying that those nodes, whatever they are, should trump the air that was going to be written there.

The only other thing to keep in mind is underground "structure" decorations, that might include air in the schematic. I'd have to check, but I assume we would overwrite stone with air from the schematic, and if so... The same issue of a top most layer decor placement would get cut off by the stone write back from the multi threaded mapgen chunk above.

~~In that case, my solution could become only CONTENT_IGNORE may be overwritten in the lower mapblocks of mapchunk generation. Which would be what I put in mergefrom().~~

~~Then someone would have to explain the maximum height decor we support, otherwise this solution may limit the max height to mapchunk height - 1 x mapblock height~~

Edit: After finally understanding I have the voxelManip flags available during all this, we may be able to pull this off correctly with flags alone like you both keep proposing:

During all non-decor, non-lua base mapgen - set all those nodes with a flag
All decor, dust, and lua mapgen effects would be written back without that flag
When writing back in BlitBackAll, decorations and lua manipulations have different flags that are allowed to overwrite terrain, but ones still flagged are only allowed to overwrite CONTENT_IGNORE.
Only do this flagging and tracking and special write back when num_emerge_threads > 1

Issue still lies in "what if they want to re-emerge a mapchunk" - which I would have to look into

ExeVirus · 2025-06-05T11:03:30Z

And for the others following along since the beginning... Yes using flags to say what gets priority is exactly the same thing as a layering system, so you were right, whoever kept proposing that, I'm now oboard for that same solution, using the vmanip flags to create a 2-level priority system.

ExeVirus · 2025-06-06T02:59:25Z

Holy moly, our mapgen decoration placement is a wild thing.

So, from what I understand after 2 more hours, besides getting flagging to work properly, is that a tree placed at the top of a mapchunk, will only generate within our maps limits.

What i don't understand yet, is how the heck that information is passed to the mapchunk above for generation.

Like, does the mapgen above just look and see that there's some tree stumps sitting there at the bottom, and keep on generating an L-tree based on that? If so... How does it know how far along in the turtle axiom it was? I.e. if a tree generates near such a boundary, does it just get taller than normal since it might have a base that is 3 nodes high already or something...

For multi threaded mapgen, if we need to know where those tree stumps were to keep on generating, I can see how we have a mess on our hands. We'd have to overgenerate to the full height of the tallest tree, which is game specific, to pull that off without bugs.

I guess the only alternative is to just not emerge any mapchunk that is adjacent to another that is currently generating... Which would really cut down on parallelism, and result in a sort of checkerboard emerge pattern.

For example, say we're generating 27 mapchunks in a 3x3x3 cube. The maximum simultaneous mapchunks that can be generated would be 14, followed by 13.

Which, I mean 13 is a lot more than 1.

Oh wait, even that is not safe, because the mapchunk that fills in the middle, could make trees, that the mapchunk above wouldn't have known about.... We must, even with single threaded mapgen have a method to deal with this madness? Like if I have 2 players, teleport far apart veritcally, and they then get close together in ungenerated space, I assume our algorithm would eventually be forced to make a middle mapchunk, that may have trees that never finish generating....

Does this happen with single threaded mapgen??

Thoughts from the smarter people than me?

kromka-chleba · 2025-06-07T17:41:59Z

Like, does the mapgen above just look and see that there's some tree stumps sitting there at the bottom, and keep on generating an L-tree based on that? If so... How does it know how far along in the turtle axiom it was? I.e. if a tree generates near such a boundary, does it just get taller than normal since it might have a base that is 3 nodes high already or something...

I don't know how it works under the hood but I suppose it could simply place a decoration several times in, once in each adjacent mapchunk? At least that's how I do overgeneration in my game's road generator - simply mirror all node placements for adjacent map division units. But idk how it is implemented in Luanti.

Either way simply redoing node placements in each mapchunk is a simple alternative to actual overgeneration. Then instead of a shell of 1 mapblock around each mapchunk there would be only mapchunks and nodes written to areas outside of the mapchunk would get ignored.

First Fixed Version

17e21a9

ExeVirus mentioned this pull request Jun 4, 2025

Mapgen: "Unfinished" y-slices with num_emerge_threads > 1 #9357

Open

Zughy added @ Mapgen Bugfix 🐛 PRs that fix a bug labels Jun 4, 2025

Fix for decor so they aren't sniped by the fix

e877bd6

kno10 mentioned this pull request Jun 5, 2025

Multi-stage map generation #16229

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix Y-slices with num_emerge_threads > 1 #16224

Fix Y-slices with num_emerge_threads > 1 #16224

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fix Y-slices with num_emerge_threads > 1 #16224

Are you sure you want to change the base?

Fix Y-slices with num_emerge_threads > 1 #16224

Uh oh!

Conversation

Goal

To do

How to test

Other ways to test:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Update 1:

Next steps

Uh oh!

Uh oh!

Update 2:

Uh oh!

Uh oh!

Update 3:

Edit:

Uh oh!

Uh oh!

Update 4:

So with that, the +-y overgeneration issue seems resolved, but not the X/Z full mapblock overlap I'm seeing?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!