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 all commits
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
85 changes: 44 additions & 41 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,43 @@ 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;
// initialize the heightmap map
for (Heightmap.Types type : Heightmap.Types.values()) {
this.heightmaps.put(type, new SurfaceTrackerLeaf[DIAMETER_IN_SECTIONS*DIAMETER_IN_SECTIONS]);
}

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);
for (int sectionX = 0; sectionX < CubeAccess.DIAMETER_IN_SECTIONS; sectionX++) {
for (int sectionZ = 0; sectionZ < CubeAccess.DIAMETER_IN_SECTIONS; sectionZ++) {

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[] protoLeaves = protoCube.getCubeHeightmaps().get(entry.getKey());
SurfaceTrackerLeaf protoLeaf = protoLeaves != null ? protoLeaves[heightmapIndex] : null;

SurfaceTrackerLeaf levelLeaf = wrapper.loadCube(storage, this, protoLeaf);
this.heightmaps.get(entry.getKey())[heightmapIndex] = levelLeaf;
}

// promote the ProtoCube's light heightmap for this chunk
SurfaceTrackerWrapper lightWrapper = (SurfaceTrackerWrapper) ((LightHeightmapGetter) chunk).getLightHeightmap();
SurfaceTrackerLeaf lightLevelLeaf = lightWrapper.loadCube(storage, this, null);

if (lightLevelLeaf == null)
System.out.println("!");

this.lightHeightmaps[heightmapIndex] = lightLevelLeaf;
}
}

this.setCubeLight(protoCube.hasCubeLight());
this.dirty = true;
}
Expand All @@ -248,18 +270,6 @@ public LevelCube(Level level, ProtoCube protoCube, @Nullable Consumer<LevelCube>
return this.heightmaps;
}

@Override public void sectionLoaded(@Nonnull SurfaceTrackerLeaf surfaceTrackerLeaf, int localSectionX, int localSectionZ) {
int idx = localSectionX + localSectionZ * DIAMETER_IN_SECTIONS;

if (surfaceTrackerLeaf.getRawType() == -1) { //light
this.lightHeightmaps[idx] = surfaceTrackerLeaf;
} else { // normal heightmap
this.heightmaps.computeIfAbsent(surfaceTrackerLeaf.getType(),
type -> new SurfaceTrackerLeaf[DIAMETER_IN_SECTIONS * DIAMETER_IN_SECTIONS]
)[idx] = surfaceTrackerLeaf;
}
}

@Override
public void unloadNode(@Nonnull HeightmapStorage storage) {
for (SurfaceTrackerLeaf[] heightmapLeaves : this.heightmaps.values()) {
Expand Down Expand Up @@ -308,6 +318,17 @@ public int getHighestLight(int x, int z) {
int zSection = blockToCubeLocalSection(z);

int idx = xSection + zSection * DIAMETER_IN_SECTIONS;

SurfaceTrackerLeaf leaf = this.lightHeightmaps[idx];
try {
Thread.sleep(1);
} catch (InterruptedException ex) {
throw new RuntimeException();
}
SurfaceTrackerLeaf leaf2 = this.lightHeightmaps[idx];
if (leaf == null)
System.out.println("!");

SurfaceTrackerLeaf sectionAbove = this.lightHeightmaps[idx].getSectionAbove();

int dy = CubeAccess.DIAMETER_IN_BLOCKS - 1;
Expand Down Expand Up @@ -1090,29 +1111,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
29 changes: 12 additions & 17 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,13 @@ public void onEnteringFeaturesStatus() {
}
}

lightHeightmap.loadCube(((CubicServerLevel) this.levelHeightAccessor).getHeightmapStorage(), this);
SurfaceTrackerLeaf lightLeaf = lightHeightmap.loadCube(((CubicServerLevel) this.levelHeightAccessor).getHeightmapStorage(), this, null);
int heightmapIndex = dx + dz * DIAMETER_IN_SECTIONS;

if (lightLeaf == null)
System.out.println("!");

this.lightHeightmaps[heightmapIndex] = lightLeaf;

for (int z = 0; z < SECTION_DIAMETER; z++) {
for (int x = 0; x < SECTION_DIAMETER; x++) {
Expand All @@ -240,19 +246,6 @@ public void onEnteringFeaturesStatus() {
}
}

@Override
public void sectionLoaded(@Nonnull SurfaceTrackerLeaf surfaceTrackerLeaf, int localSectionX, int localSectionZ) {
int idx = localSectionX + localSectionZ * DIAMETER_IN_SECTIONS;

if (surfaceTrackerLeaf.getRawType() == -1) { //light
this.lightHeightmaps[idx] = surfaceTrackerLeaf;
} else { // normal heightmap
this.heightmaps.computeIfAbsent(surfaceTrackerLeaf.getType(),
type -> new SurfaceTrackerLeaf[DIAMETER_IN_SECTIONS * DIAMETER_IN_SECTIONS]
)[idx] = surfaceTrackerLeaf;
}
}

@Override
public void unloadNode(@Nonnull HeightmapStorage storage) {
for (SurfaceTrackerLeaf[] heightmapLeaves : this.heightmaps.values()) {
Expand Down Expand Up @@ -368,11 +361,9 @@ 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;
}
}
Expand Down Expand Up @@ -539,6 +530,10 @@ public int getHighestLight(int x, int z) {
int zSection = blockToCubeLocalSection(z);

int idx = xSection + zSection * DIAMETER_IN_SECTIONS;

if (this.lightHeightmaps[idx] == null)
System.out.println("!");

SurfaceTrackerLeaf sectionAbove = this.lightHeightmaps[idx].getSectionAbove();

int dy = CubeAccess.DIAMETER_IN_BLOCKS - 1;
Expand Down
4 changes: 0 additions & 4 deletions ...a/io/github/opencubicchunks/cubicchunks/world/level/levelgen/heightmap/HeightmapNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@

public interface HeightmapNode {

default void sectionLoaded(@Nonnull SurfaceTrackerLeaf surfaceTrackerLeaf, int localSectionX, int localSectionZ) {
throw new IllegalStateException("Should not be reached");
}

void unloadNode(@Nonnull HeightmapStorage storage);

int getHighest(int x, int z, byte heightmapType);
Expand Down
46 changes: 38 additions & 8 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(HeightmapStorage storage, HeightmapNode newNode, @Nullable SurfaceTrackerLeaf protoLeaf) {
int newScale = scale - 1;

// Attempt to load all children from storage
Expand All @@ -56,18 +56,48 @@ 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);
} else {

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

assert protoLeaf == null || children[idx] == null : "!";

SurfaceTrackerLeaf newLeaf;

// converting an existing node
if (children[idx] != null) {
newLeaf = new SurfaceTrackerLeaf(newNode, this, (SurfaceTrackerLeaf) children[idx]);
}
// attaching an external leaf
else if (protoLeaf != null) {
newLeaf = new SurfaceTrackerLeaf(newNode, this, protoLeaf);
}
// creating a new leaf
else {
newLeaf = new SurfaceTrackerLeaf(newNode, this, this.heightmapType);
}

children[idx] = newLeaf;
newLeaf.markAncestorsDirty();

onChildLoaded();

return newLeaf;
}

// child is a branch
else {

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

return ((SurfaceTrackerBranch) children[idx]).loadCube(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;
}
}
16 changes: 10 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,22 @@ 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;
}


/**
* 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 +74,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(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(storage, new TestHeightmapNode(0), null);

SurfaceTrackerLeaf leaf = root.getMinScaleNode(0);
assertNotNull(leaf, "Appropriate leaf was null after loading node into root");
Expand Down
Loading