Skip to content
This repository has been archived by the owner on Oct 4, 2024. It is now read-only.

SurfaceTracker cleanup (WIP) #100

Open
wants to merge 2 commits into
base: MC_1.17
Choose a base branch
from
Open

SurfaceTracker cleanup (WIP) #100

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
89062a8
Removed SurfaceTrackerLeaf.loadCube, because leaves can no longer loa…
Cyclonit Mar 30, 2022
831b729
Intermediate commit to allow debugging by others.
Cyclonit Mar 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 22 additions & 29 deletions src/main/java/io/github/opencubicchunks/cubicchunks/world/level/chunk/LevelCube.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,21 +225,32 @@ public LevelCube(Level level, ProtoCube protoCube, @Nullable Consumer<LevelCube>
this.setAllStarts(protoCube.getAllCubeStructureStarts());
this.setAllReferences(protoCube.getAllReferences());

this.heightmaps.putAll(protoCube.getCubeHeightmaps());
// copy protoCube's heightmaps
ChunkPos pos = this.cubePos.asChunkPos();
HeightmapStorage storage = ((CubicServerLevel) this.level).getHeightmapStorage();

SurfaceTrackerLeaf[] protoCubeLightHeightmaps = protoCube.getLightHeightmaps();
for (int localZ = 0; localZ < CubeAccess.DIAMETER_IN_SECTIONS; localZ++) {
for (int localX = 0; localX < CubeAccess.DIAMETER_IN_SECTIONS; localX++) {
int i = localX + localZ * CubeAccess.DIAMETER_IN_SECTIONS;
for (int sectionX = 0; sectionX < CubeAccess.DIAMETER_IN_SECTIONS; sectionX++) {
for (int sectionZ = 0; sectionZ < CubeAccess.DIAMETER_IN_SECTIONS; sectionZ++) {

this.lightHeightmaps[i] = protoCubeLightHeightmaps[i];
if (this.lightHeightmaps[i] == null) {
System.out.println("Got a null light heightmap while upgrading from CubePrimer at " + this.cubePos);
} else {
this.lightHeightmaps[i].loadCube(localX, localZ, ((CubicServerLevel) this.level).getHeightmapStorage(), this);
LevelChunk chunk = this.level.getChunk(pos.x + sectionX, pos.z + sectionZ);
int heightmapIndex = sectionX + sectionZ * DIAMETER_IN_SECTIONS;

// promote the ProtoCube's vanilla heightmaps for this chunk
for (Map.Entry<Heightmap.Types, Heightmap> entry : chunk.getHeightmaps()) {
SurfaceTrackerWrapper wrapper = (SurfaceTrackerWrapper) entry.getValue();
SurfaceTrackerLeaf protoLeaf = protoCube.getCubeHeightmaps().get(entry.getKey())[heightmapIndex];
SurfaceTrackerLeaf levelLeaf = wrapper.loadCube(storage, this, protoLeaf);
sectionLoaded(levelLeaf, sectionX, sectionZ);
}

// promote the ProtoCube's light heightmap for this chunk
SurfaceTrackerWrapper lightWrapper = (SurfaceTrackerWrapper) ((LightHeightmapGetter) chunk).getLightHeightmap();
SurfaceTrackerLeaf lightProtoLeaf = protoCube.getLightHeightmaps()[heightmapIndex];
SurfaceTrackerLeaf lightLevelLeaf = lightWrapper.loadCube(storage, this, lightProtoLeaf);
sectionLoaded(lightLevelLeaf, sectionX, sectionZ);
}
}

this.setCubeLight(protoCube.hasCubeLight());
this.dirty = true;
}
Expand Down Expand Up @@ -1090,29 +1101,11 @@ public void unpackTicks() {
}

public void postLoad() {

if (this.postLoad != null) {
this.postLoad.accept(this);
this.postLoad = null;
}
// TODO heightmap stuff should probably be elsewhere rather than here.
ChunkPos pos = this.cubePos.asChunkPos();
HeightmapStorage storage = ((CubicServerLevel) this.level).getHeightmapStorage();
for (int x = 0; x < CubeAccess.DIAMETER_IN_SECTIONS; x++) {
for (int z = 0; z < CubeAccess.DIAMETER_IN_SECTIONS; z++) {

// This force-loads the column, but it shouldn't matter if column-cube load order is working properly.
LevelChunk chunk = this.level.getChunk(pos.x + x, pos.z + z);
((ColumnCubeMapGetter) chunk).getCubeMap().markLoaded(this.cubePos.getY());
for (Map.Entry<Heightmap.Types, Heightmap> entry : chunk.getHeightmaps()) {
Heightmap heightmap = entry.getValue();
SurfaceTrackerWrapper tracker = (SurfaceTrackerWrapper) heightmap;
tracker.loadCube(storage, this);
}

// TODO probably don't want to do this if the cube was already loaded as a CubePrimer
((LightHeightmapGetter) chunk).getServerLightHeightmap().loadCube(storage, this);
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic was partly duplicated between the promotion constructor and postLoad. I moved all of it into the promotion constructor for now.

}

public void invalidateAllBlockEntities() {
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/io/github/opencubicchunks/cubicchunks/world/level/chunk/ProtoCube.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ public void onEnteringFeaturesStatus() {
}
}

lightHeightmap.loadCube(((CubicServerLevel) this.levelHeightAccessor).getHeightmapStorage(), this);
SurfaceTrackerLeaf lightLeaf = lightHeightmap.loadCube(((CubicServerLevel) this.levelHeightAccessor).getHeightmapStorage(), this, null);
sectionLoaded(lightLeaf, dx, dz);

for (int z = 0; z < SECTION_DIAMETER; z++) {
for (int x = 0; x < SECTION_DIAMETER; x++) {
Expand Down Expand Up @@ -368,12 +369,11 @@ private SurfaceTrackerLeaf[] getHeightmapSections(Heightmap.Types type) {
for (int dx = 0; dx < CubeAccess.DIAMETER_IN_SECTIONS; dx++) {
for (int dz = 0; dz < CubeAccess.DIAMETER_IN_SECTIONS; dz++) {
int idx = dx + dz * CubeAccess.DIAMETER_IN_SECTIONS;
SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(cubePos.getY(), null, (byte) type.ordinal());
leaf.loadCube(dx, dz, ((CubicServerLevel) ((ServerLevelAccessor) this.levelHeightAccessor).getLevel()).getHeightmapStorage(), this);
SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(this, null, (byte) type.ordinal());
// On creation of a new node for a cube, both the node and its parents must be marked dirty
leaf.setAllDirty();
leaf.markAncestorsDirty();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leaf is an unattached SurfaceTrackerLeaf. There is no point in calling "markAncestorsDirty" as it does not have any.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc this is used elsewhere too. It's really a "mark any positions dirty if they are below the new height in myself and ancestors"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setAllDirty() already sets everything within the leaf dirty without any checks. markAncestorsDirty() doesn't have any additional effect.

surfaceTrackerLeaves[idx] = leaf;
sectionLoaded(leaf, dx, dz);
}
}
return surfaceTrackerLeaves;
Expand Down
35 changes: 28 additions & 7 deletions ...s/cubicchunks/world/level/levelgen/heightmap/surfacetrackertree/SurfaceTrackerBranch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ protected int updateHeight(int x, int z, int idx) {
}
}

@Override public void loadCube(int localSectionX, int localSectionZ, HeightmapStorage storage, HeightmapNode newNode) {
public SurfaceTrackerLeaf loadCube(int localSectionX, int localSectionZ, HeightmapStorage storage, HeightmapNode newNode, @Nullable SurfaceTrackerLeaf protoLeaf) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loadCube now accepts a protoLeaf to copy its data and it returns the newly created leaf. This enable promotion of "proto leaves" from ProtoCube into LevelCube.

int newScale = scale - 1;

// Attempt to load all children from storage
Expand All @@ -56,18 +56,39 @@ protected int updateHeight(int x, int z, int idx) {

int idx = indexOfRawHeightNode(newNode.getNodeY(), scale, scaledY);
int newScaledY = indexToScaledY(idx, scale, scaledY);
if (children[idx] == null) {
// If the child containing new node has not been loaded from storage, create it
// Scale 1 nodes create leaf node children
if (newScale == 0) {
children[idx] = new SurfaceTrackerLeaf(newScaledY, this, this.heightmapType);

// child is a leaf
if (newScale == 0) {

assert children[idx] == null : "Duplicate leaf!";

SurfaceTrackerLeaf newLeaf;
if (protoLeaf == null) {
newLeaf = new SurfaceTrackerLeaf(newNode, this, this.heightmapType);
} else {
newLeaf = new SurfaceTrackerLeaf(newNode, this, protoLeaf);
}
children[idx] = newLeaf;
newNode.sectionLoaded(newLeaf, localSectionX, localSectionZ);
newLeaf.markAncestorsDirty();

onChildLoaded();

return newLeaf;
}

// child is a branch
else {

if (children[idx] == null) {
children[idx] = new SurfaceTrackerBranch(newScale, newScaledY, this, this.heightmapType);
}

return ((SurfaceTrackerBranch) children[idx]).loadCube(localSectionX, localSectionZ, storage, newNode, protoLeaf);
}
children[idx].loadCube(localSectionX, localSectionZ, storage, newNode);
}


@Override protected void unload(HeightmapStorage storage) {
for (SurfaceTrackerNode child : this.children) {
assert child == null : "Heightmap branch being unloaded while holding a child?!";
Expand Down
44 changes: 14 additions & 30 deletions ...nks/cubicchunks/world/level/levelgen/heightmap/surfacetrackertree/SurfaceTrackerLeaf.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,22 @@
import io.github.opencubicchunks.cubicchunks.world.level.levelgen.heightmap.HeightmapStorage;

public class SurfaceTrackerLeaf extends SurfaceTrackerNode {
protected HeightmapNode node;

public SurfaceTrackerLeaf(int y, @Nullable SurfaceTrackerBranch parent, byte heightmapType) {
super(0, y, parent, heightmapType);
protected final HeightmapNode node;

public SurfaceTrackerLeaf(HeightmapNode node, @Nullable SurfaceTrackerBranch parent, byte heightmapType) {
super(0, node.getNodeY(), parent, heightmapType);

this.node = node;
}

public SurfaceTrackerLeaf(HeightmapNode node, @Nullable SurfaceTrackerBranch parent, SurfaceTrackerLeaf protoLeaf) {
super(0, node.getNodeY(), parent, protoLeaf.heightmapType, protoLeaf.heights, protoLeaf.dirtyPositions);

this.node = node;
}


@Override
protected int updateHeight(int x, int z, int idx) {
synchronized(this) {
Expand All @@ -29,27 +39,6 @@ protected int updateHeight(int x, int z, int idx) {
}
}

@Override
public synchronized void loadCube(int localSectionX, int localSectionZ, HeightmapStorage storage, @Nonnull HeightmapNode newNode) {
boolean isBeingInitialized = this.node == null;

this.node = newNode;
newNode.sectionLoaded(this, localSectionX, localSectionZ);

// Parent might be null for proto-cube leaf nodes
// If we are inserting a new node (it's parent is null), the parents must be updated.
// The parent can already be set for LevelCubes, their heights are inherited from their ProtoCubes
// and do not need to be updated
if (this.parent != null) {
this.markAncestorsDirty();
if (isBeingInitialized) {
// If this is the first node inserted into this leaf, we inform the parent node.
// Both ProtoCube and LevelCube will call loadCube, this avoids invalid reference counting
this.parent.onChildLoaded();
}
}
}

@Override protected void unload(@Nonnull HeightmapStorage storage) {
assert this.node == null : "Heightmap leaf being unloaded while holding a cube?!";

Expand All @@ -68,7 +57,7 @@ public void cubeUnloaded(int localSectionX, int localSectionZ, HeightmapStorage
// On unloading the node, the leaf must have no dirty positions
updateDirtyHeights(localSectionX, localSectionZ);

this.node = null;
// TODO: this.node = null;

// Parent can be null for a protocube that hasn't been added to the global heightmap
if (parent != null) {
Expand Down Expand Up @@ -149,9 +138,4 @@ public SurfaceTrackerLeaf getSectionAbove() {
// TODO this can be optimized - don't need to go to the root every time, just the lowest node that is a parent of both this node and the node above.
return this.getRoot().getMinScaleNode(scaledY + 1);
}

@VisibleForTesting
public void setNode(@Nullable HeightmapNode node) {
this.node = node;
}
}
24 changes: 18 additions & 6 deletions ...nks/cubicchunks/world/level/levelgen/heightmap/surfacetrackertree/SurfaceTrackerNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,30 @@ public abstract class SurfaceTrackerNode {
protected final byte heightmapType;

public SurfaceTrackerNode(int scale, int scaledY, @Nullable SurfaceTrackerBranch parent, byte heightmapType) {
// super((ChunkAccess) node, types);
// +1 in bit size to make room for null values
this.heights = new BitStorage(BASE_SIZE_BITS + 1 + scale * NODE_COUNT_BITS, WIDTH_BLOCKS * WIDTH_BLOCKS);
this.dirtyPositions = new long[WIDTH_BLOCKS * WIDTH_BLOCKS / Long.SIZE];
this(scale, scaledY, parent, heightmapType,
new BitStorage(BASE_SIZE_BITS + 1 + scale * NODE_COUNT_BITS, WIDTH_BLOCKS * WIDTH_BLOCKS),
new long[WIDTH_BLOCKS * WIDTH_BLOCKS / Long.SIZE]
);
}

protected SurfaceTrackerNode(int scale, int scaledY, @Nullable SurfaceTrackerBranch parent, byte heightmapType, BitStorage heights, long[] dirtyPositions) {
this.heights = heights;
this.dirtyPositions = dirtyPositions;
this.parent = parent;
this.scaledY = scaledY;
this.scale = (byte) scale;
this.heightmapType = heightmapType;
}


private BitStorage getRawHeights() {
return heights;
}

private long[] getDirtyPositions() {
return dirtyPositions;
}

/**
* Get the height for a given position. Recomputes the height if the column is marked dirty in this section.
* x and z are <b>GLOBAL</b> coordinates (cube local is also fine, but section/chunk local is WRONG).
Expand All @@ -68,8 +82,6 @@ public int getHeight(int x, int z) {
*/
protected abstract int updateHeight(int x, int z, int idx);

public abstract void loadCube(int localSectionX, int localSectionZ, HeightmapStorage storage, HeightmapNode newNode);

/**
* Tells a node to unload itself, nulling its parent, and passing itself to the provided storage
*/
Expand Down
2 changes: 1 addition & 1 deletion ...hunks/world/level/levelgen/heightmap/surfacetrackertree/SurfaceTrackerSectionStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public void unloadNode(SurfaceTrackerNode surfaceTrackerNode) {
saved.put(new PackedTypeScaleScaledY(surfaceTrackerNode.heightmapType, surfaceTrackerNode.scale, surfaceTrackerNode.scaledY), surfaceTrackerNode);

if (surfaceTrackerNode.scale == 0) {
((SurfaceTrackerLeaf) surfaceTrackerNode).node = null;
// TODO: ((SurfaceTrackerLeaf) surfaceTrackerNode).node = null;
} else {
Arrays.fill(((SurfaceTrackerBranch) surfaceTrackerNode).children, null);
}
Expand Down
4 changes: 2 additions & 2 deletions .../cubicchunks/world/level/levelgen/heightmap/surfacetrackertree/SurfaceTrackerWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ public long[] getRawData() {
return data.getRaw();
}

public synchronized void loadCube(HeightmapStorage storage, HeightmapNode node) {
this.surfaceTracker.loadCube(blockToCubeLocalSection(dx), blockToCubeLocalSection(dz), storage, node);
public synchronized SurfaceTrackerLeaf loadCube(HeightmapStorage storage, HeightmapNode node, @Nullable SurfaceTrackerLeaf protoLeaf) {
return this.surfaceTracker.loadCube(blockToCubeLocalSection(dx), blockToCubeLocalSection(dz), storage, node, protoLeaf);
}

@Nullable
Expand Down
2 changes: 1 addition & 1 deletion ...va/io/github/opencubicchunks/cubicchunks/levelgen/heightmap/SurfaceTrackerBranchTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public void testValidScaleBounds() {
public void testLeafInsertionIntoRoot() {
NullHeightmapStorage storage = new NullHeightmapStorage();
SurfaceTrackerBranch root = new SurfaceTrackerBranch(SurfaceTrackerNode.MAX_SCALE, 0, null, (byte) 0);
root.loadCube(0, 0, storage, new TestHeightmapNode(0));
root.loadCube(0, 0, storage, new TestHeightmapNode(0), null);

SurfaceTrackerLeaf leaf = root.getMinScaleNode(0);
assertNotNull(leaf, "Appropriate leaf was null after loading node into root");
Expand Down
31 changes: 10 additions & 21 deletions ...java/io/github/opencubicchunks/cubicchunks/levelgen/heightmap/SurfaceTrackerLeafTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ public class SurfaceTrackerLeafTest {
*/
@Test
public void testCubeLoadUnload() {
SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(0, null, (byte) 0);

leaf.loadCube(0, 0, null, new TestHeightmapNode(0));
TestHeightmapNode testNode = new TestHeightmapNode(0);
SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(testNode, null, (byte) 0);

assertNotNull(leaf.getNode(), "Leaf had null HeightmapNode after being loaded");

leaf.cubeUnloaded(0, 0, null);
Expand All @@ -48,7 +49,7 @@ public void testLeafUnload() {

//Set up leaf and node with parent
SurfaceTrackerBranch parent = new SurfaceTrackerBranch(SurfaceTrackerNode.MAX_SCALE, 0, null, (byte) 0);
parent.loadCube(0, 0, storage, new TestHeightmapNode(0));
parent.loadCube(0, 0, storage, new TestHeightmapNode(0), null);
SurfaceTrackerLeaf leaf = parent.getMinScaleNode(0);

//Unload the node
Expand All @@ -63,11 +64,9 @@ public void testLeafUnload() {
*/
@Test
public void testNoValidHeights() {
NullHeightmapStorage storage = new NullHeightmapStorage();

SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(0, null, (byte) 0);
TestHeightmapNode testNode = new TestHeightmapNode(0);
leaf.loadCube(0, 0, storage, testNode);
SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(testNode, null, (byte) 0);

Consumer<HeightmapBlock> setHeight = block -> testNode.setBlock(block.x(), block.y() & (SurfaceTrackerLeaf.SCALE_0_NODE_HEIGHT - 1), block.z(), block.isOpaque());

Expand All @@ -92,11 +91,8 @@ public void testNoValidHeights() {
public void testBasicFunctionality() {
ReferenceHeightmap reference = new ReferenceHeightmap(0);

NullHeightmapStorage storage = new NullHeightmapStorage();

SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(0, null, (byte) 0);
TestHeightmapNode testNode = new TestHeightmapNode(0);
leaf.loadCube(0, 0, storage, testNode);
SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(testNode, null, (byte) 0);

Consumer<HeightmapBlock> setHeight = block -> {
reference.set(block.y(), block.isOpaque());
Expand Down Expand Up @@ -135,11 +131,8 @@ public void testBasicFunctionality() {
public void testManyPositions(int nodeY) {
ReferenceHeightmap reference = new ReferenceHeightmap(nodeY);

NullHeightmapStorage storage = new NullHeightmapStorage();

SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(nodeY, null, (byte) 0);
TestHeightmapNode testNode = new TestHeightmapNode(nodeY);
leaf.loadCube(0, 0, storage, testNode);
SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(testNode, null, (byte) 0);

Consumer<HeightmapBlock> setHeight = block -> {
reference.set(block.y(), block.isOpaque());
Expand Down Expand Up @@ -177,11 +170,8 @@ public void testManyPositions(int nodeY) {
public void testSeededRandom() {
ReferenceHeightmap reference = new ReferenceHeightmap(0);

NullHeightmapStorage storage = new NullHeightmapStorage();

SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(0, null, (byte) 0);
TestHeightmapNode testNode = new TestHeightmapNode(0);
leaf.loadCube(0, 0, storage, testNode);
SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(testNode, null, (byte) 0);

Consumer<HeightmapBlock> setHeight = block -> {
reference.set(block.y(), block.isOpaque());
Expand All @@ -208,10 +198,9 @@ public void testSeededRandom() {
*/
@Test
public void testBounds() {
NullHeightmapStorage storage = new NullHeightmapStorage();

SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(0, null, (byte) 0);
leaf.loadCube(0, 0, storage, new TestHeightmapNode(0));
TestHeightmapNode node = new TestHeightmapNode(0);
SurfaceTrackerLeaf leaf = new SurfaceTrackerLeaf(node, null, (byte) 0);

Consumer<HeightmapBlock> setHeight = block -> {
leaf.onSetBlock(block.x(), block.y(), block.z(), type -> block.isOpaque());
Expand Down
Loading