TE and world access thread safety for rendering, and remesh on first TE update packet. (#2014)

This commit is contained in:
Jason Mitchell authored and GitHub committed 2026-08-14 20:21:13 -07:00
1 parent 6af401c834
commit c86e1de9f6
14 files changed
+419 -16

No files matched your search

+1
View File
@@ -202,6 +202,7 @@ afterEvaluate {
glsmTestFixtures(testFixtures(project(":glsm")) as ModuleDependency) { isTransitive = false }
testCompileOnly(testFixtures(project(":glsm")))
testImplementation("mega:fluidlogged-mc1.7.10:0.1.2")
testRuntimeOnly(libs.fastutil)
testRuntimeOnly(libs.joml)
@@ -383,6 +383,10 @@ public enum Mixins implements IMixins {
, "celeritas.biome_blending.MixinBlockLiquid"
, "celeritas.threading.MixinForgeHooksClient"
, "celeritas.terrain.MixinChunk"
, "celeritas.terrain.MixinWorldClient_WorkerAccess"
, "celeritas.terrain.MixinWorld_WorkerMutationGuard"
, "celeritas.terrain.MixinTileEntity_AwaitingDescriptor"
, "celeritas.terrain.MixinNetHandlerPlayClient_DescriptorRepair"
, "celeritas.terrain.MixinRenderRegion"
, "celeritas.terrain.MixinSectionRenderDataStorage"
, "celeritas.terrain.MixinDefaultChunkRenderer"
@@ -0,0 +1,8 @@
package com.gtnewhorizons.angelica.mixins.interfaces;
public interface IAwaitingDescriptorTE {
boolean angelica$isAwaitingDescriptor();
void angelica$setAwaitingDescriptor(boolean awaiting);
}
@@ -1,7 +1,7 @@
package com.gtnewhorizons.angelica.rendering;
import com.gtnewhorizon.gtnhlib.client.renderer.TessellatorManager;
import com.gtnewhorizons.angelica.rendering.celeritas.world.WorldSlice;
import net.minecraft.tileentity.TileEntity;
public class RenderThreadContext {
@@ -15,10 +15,13 @@ public class RenderThreadContext {
currentWorldSlice.remove();
}
public static TileEntity getSnapshotTE(int x, int y, int z) {
final WorldSlice slice = currentWorldSlice.get();
if (slice == null) return null;
return slice.getTileEntity(x, y, z);
public static WorldSlice get() {
return currentWorldSlice.get();
}
public static WorldSlice workerSlice() {
if (TessellatorManager.isOnMainThread()) return null;
return currentWorldSlice.get();
}
public static boolean hasWorldSlice() {
@@ -0,0 +1,120 @@
package com.gtnewhorizons.angelica.rendering;
import java.io.File;
import java.net.MalformedURLException;
import java.net.URL;
import java.security.CodeSource;
import java.util.Arrays;
import net.minecraft.block.Block;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import com.gtnewhorizons.angelica.mixins.interfaces.IRenderingRegistryExt;
import cpw.mods.fml.client.registry.ISimpleBlockRenderingHandler;
import cpw.mods.fml.client.registry.RenderingRegistry;
import cpw.mods.fml.common.Loader;
import cpw.mods.fml.common.ModContainer;
import cpw.mods.fml.common.registry.GameRegistry;
import cpw.mods.fml.common.registry.GameRegistry.UniqueIdentifier;
public final class WorkerWorldAccess {
private static final String REPORT_URL = "https://github.com/GTNewHorizons/angelica/issues";
private static final int INITIAL_SLOTS = 64;
private static final Logger LOGGER = LogManager.getLogger("Angelica");
private static final LoggedRenderTypeErrors READS = new LoggedRenderTypeErrors();
private static final LoggedRenderTypeErrors WRITES = new LoggedRenderTypeErrors();
private WorkerWorldAccess() {}
public static void readOutsideSlice(String worldMethod, int x, int y, int z, Block renderingBlock) {
if (!READS.shouldLog(renderingBlock)) return;
LOGGER.warn(describe(worldMethod, "read outside the chunk slice", x, y, z, renderingBlock), new Throwable());
}
public static void blockedWrite(String worldMethod, int x, int y, int z, Block renderingBlock) {
if (!WRITES.shouldLog(renderingBlock)) return;
LOGGER.error(describe(worldMethod, "ignored, it would corrupt world data", x, y, z, renderingBlock), new Throwable());
}
private static String describe(String worldMethod, String outcome, int x, int y, int z, Block block) {
final StringBuilder sb = new StringBuilder(224);
sb.append("Off-thread World.").append(worldMethod).append(' ').append(outcome).append(" at ").append(x).append(',').append(y).append(',').append(z);
if (block != null) {
sb.append(". Rendering ").append(nameOf(block));
final ISimpleBlockRenderingHandler isbrh = isbrhOf(block);
if (isbrh != null) {
sb.append(" via ").append(isbrh.getClass().getName());
final String owner = modIdOf(isbrh.getClass());
if (owner != null) sb.append(" from ").append(owner);
}
}
return sb.append(". Please report with this trace: ").append(REPORT_URL).toString();
}
private static String nameOf(Block block) {
try {
final UniqueIdentifier id = GameRegistry.findUniqueIdentifierFor(block);
if (id != null) return id.modId + ':' + id.name;
} catch (Throwable ignored) {}
return block.getClass().getName();
}
@SuppressWarnings("deprecation")
private static ISimpleBlockRenderingHandler isbrhOf(Block block) {
try {
return ((IRenderingRegistryExt) RenderingRegistry.instance()).getISBRH(block.getRenderType());
} catch (Throwable ignored) {
return null;
}
}
private static String modIdOf(Class<?> clazz) {
try {
final CodeSource source = clazz.getProtectionDomain() == null ? null : clazz.getProtectionDomain().getCodeSource();
if (source == null || source.getLocation() == null) return null;
final File jar = new File(jarFileUrl(source.getLocation()).toURI());
for (ModContainer mod : Loader.instance().getModList()) {
if (jar.equals(mod.getSource())) return mod.getModId();
}
} catch (Throwable ignored) {}
return null;
}
private static URL jarFileUrl(URL location) throws MalformedURLException {
if (!"jar".equals(location.getProtocol())) return location;
final String nested = location.getPath();
final int separator = nested.indexOf("!/");
return new URL(separator < 0 ? nested : nested.substring(0, separator));
}
private static final class LoggedRenderTypeErrors {
private volatile boolean[] logged = new boolean[INITIAL_SLOTS];
boolean shouldLog(Block block) {
final int renderType = block == null ? -1 : block.getRenderType();
final int slot = renderType < 0 ? 0 : renderType + 1;
final boolean[] current = this.logged;
return (slot >= current.length || !current[slot]) && markLogged(slot);
}
private synchronized boolean markLogged(int slot) {
boolean[] current = this.logged;
if (slot < current.length) {
if (current[slot]) return false;
current[slot] = true;
return true;
}
current = Arrays.copyOf(current, Math.max(slot + 1, current.length * 2));
current[slot] = true;
this.logged = current;
return true;
}
}
}
@@ -9,6 +9,7 @@ import com.gtnewhorizons.angelica.rendering.celeritas.api.IrisShaderProvider;
import com.gtnewhorizons.angelica.rendering.celeritas.api.IrisShaderProviderHolder;
import com.gtnewhorizons.angelica.rendering.celeritas.iris.BlockRenderContext;
import com.gtnewhorizons.angelica.rendering.celeritas.iris.ContextAwareChunkVertexEncoder;
import com.gtnewhorizons.angelica.rendering.celeritas.world.WorldSlice;
import com.prupe.mcpatcher.mal.block.RenderBlocksUtils;
import it.unimi.dsi.fastutil.floats.FloatArrayList;
import it.unimi.dsi.fastutil.objects.Reference2ReferenceMap;
@@ -111,6 +112,8 @@ public abstract class AngelicaChunkBuilderMeshingTask extends ChunkBuilderTask<C
final BlockPos blockPos = new BlockPos(minX, minY, minZ);
final IBlockAccess region = getBlockAccess();
final WorldSlice reportingSlice = region instanceof WorldSlice slice ? slice : null;
if (reportingSlice != null) reportingSlice.setRenderingBlock(null);
final SmoothBiomeColorCache biomeColorCache = getBiomeColorCache();
onEnterExecute();
@@ -155,6 +158,8 @@ public abstract class AngelicaChunkBuilderMeshingTask extends ChunkBuilderTask<C
continue;
}
if (reportingSlice != null) reportingSlice.setRenderingBlock(block);
final int meta = region.getBlockMetadata(x, y, z);
if (block.hasTileEntity(meta)) {
@@ -104,6 +104,7 @@ public class WorldSlice implements IBlockAccessExtended, FLBlockAccess {
// The chunk origin of this slice
private ChunkSectionPos origin;
private StructureBoundingBox volume;
private Block renderingBlock;
private final SmoothBiomeColorCache biomeColorCache;
@@ -530,6 +531,18 @@ public class WorldSlice implements IBlockAccessExtended, FLBlockAccess {
return y << TABLE_BITS << TABLE_BITS | z << TABLE_BITS | x;
}
public boolean containsBlock(int x, int y, int z) {
return blockBoxContains(this.volume, x, y, z);
}
public void setRenderingBlock(Block block) {
this.renderingBlock = block;
}
public Block getRenderingBlock() {
return this.renderingBlock;
}
private static boolean blockBoxContains(StructureBoundingBox box, int x, int y, int z) {
return x >= box.minX && x <= box.maxX &&
y >= box.minY && y <= box.maxY &&
@@ -202,7 +202,8 @@ public class ConcurrentTileEntityMap extends HashMap<ChunkPosition, TileEntity>
if (!isRenderWorkerThread()) return delegate.containsKey(key);
lock.readLock().lock();
try {
return delegate.containsKey(key);
final TileEntity te = delegate.get(key);
return te != null && !te.isInvalid();
} finally {
lock.readLock().unlock();
}
@@ -23,8 +23,10 @@ import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
import com.gtnewhorizon.gtnhlib.hash.Fnv1a64;
import com.gtnewhorizons.angelica.compat.ModStatus;
import com.gtnewhorizons.angelica.event.ChunkBiomeDataChangedEvent;
import com.gtnewhorizons.angelica.mixins.interfaces.IAwaitingDescriptorTE;
import com.gtnewhorizons.angelica.mixins.interfaces.IChunkTileEntityMapHolder;
import com.gtnewhorizons.angelica.rendering.RenderThreadContext;
import com.gtnewhorizons.angelica.rendering.celeritas.world.WorldSlice;
import com.gtnewhorizons.angelica.utils.ConcurrentTileEntityMap;
@Mixin(Chunk.class)
@@ -60,22 +62,29 @@ public abstract class MixinChunk implements IChunkTileEntityMapHolder {
@Redirect(method = "getTileEntityUnsafe", at = @At(value = "INVOKE", target = "Ljava/util/Map;get(Ljava/lang/Object;)Ljava/lang/Object;"), remap = false)
private Object angelica$redirectGetTEUnsafe(Map<ChunkPosition, TileEntity> map, Object key) {
if (RenderThreadContext.hasWorldSlice()) {
return angelica$snapshotFirst(map, key);
}
@Redirect(method = "func_150806_e", at = @At(value = "INVOKE", target = "Ljava/util/Map;get(Ljava/lang/Object;)Ljava/lang/Object;"))
private Object angelica$redirectGetTEFunc(Map<ChunkPosition, TileEntity> map, Object key) {
return angelica$snapshotFirst(map, key);
}
@Unique
private Object angelica$snapshotFirst(Map<ChunkPosition, TileEntity> map, Object key) {
final WorldSlice slice = RenderThreadContext.get();
if (slice != null) {
final ChunkPosition pos = (ChunkPosition) key;
final TileEntity te = RenderThreadContext.getSnapshotTE((this.xPosition << 4) + pos.chunkPosX, pos.chunkPosY, (this.zPosition << 4) + pos.chunkPosZ);
final TileEntity te = slice.getTileEntity((this.xPosition << 4) + pos.chunkPosX, pos.chunkPosY, (this.zPosition << 4) + pos.chunkPosZ);
if (te != null) return te;
}
return map.get(key);
}
@Redirect(method = "func_150806_e", at = @At(value = "INVOKE", target = "Ljava/util/Map;get(Ljava/lang/Object;)Ljava/lang/Object;"))
private Object angelica$redirectGetTEFunc(Map<ChunkPosition, TileEntity> map, Object key) {
if (RenderThreadContext.hasWorldSlice()) {
final ChunkPosition pos = (ChunkPosition) key;
final TileEntity te = RenderThreadContext.getSnapshotTE((this.xPosition << 4) + pos.chunkPosX, pos.chunkPosY, (this.zPosition << 4) + pos.chunkPosZ);
if (te != null) return te;
}
return map.get(key);
@Redirect(method = "func_150806_e", at = @At(value = "INVOKE", target = "Lnet/minecraft/block/Block;hasTileEntity(I)Z"), remap = false)
private boolean angelica$noWorkerTileEntityCreation(Block block, int meta) {
if (RenderThreadContext.hasWorldSlice()) return false;
return block.hasTileEntity(meta);
}
@Redirect(method = "func_150806_e", at = @At(value = "INVOKE", target = "Ljava/util/Map;remove(Ljava/lang/Object;)Ljava/lang/Object;"))
@@ -129,6 +138,7 @@ public abstract class MixinChunk implements IChunkTileEntityMapHolder {
te.xCoord = (this.xPosition << 4) + x;
te.yCoord = worldY;
te.zCoord = (this.zPosition << 4) + z;
((IAwaitingDescriptorTE) te).angelica$setAwaitingDescriptor(true);
this.addTileEntity(te);
}
}
@@ -0,0 +1,26 @@
package com.gtnewhorizons.angelica.mixins.early.celeritas.terrain;
import net.minecraft.client.multiplayer.WorldClient;
import net.minecraft.client.network.NetHandlerPlayClient;
import net.minecraft.tileentity.TileEntity;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Redirect;
import com.gtnewhorizons.angelica.mixins.interfaces.IAwaitingDescriptorTE;
@Mixin(NetHandlerPlayClient.class)
public class MixinNetHandlerPlayClient_DescriptorRepair {
@Redirect(method = "handleUpdateTileEntity",
at = @At(value = "INVOKE", target = "Lnet/minecraft/client/multiplayer/WorldClient;getTileEntity(III)Lnet/minecraft/tileentity/TileEntity;"))
private TileEntity angelica$remeshOnFirstDescriptor(WorldClient world, int x, int y, int z) {
final TileEntity te = world.getTileEntity(x, y, z);
if (te instanceof IAwaitingDescriptorTE awaiting && awaiting.angelica$isAwaitingDescriptor()) {
awaiting.angelica$setAwaitingDescriptor(false);
world.markBlockForUpdate(x, y, z);
}
return te;
}
}
@@ -0,0 +1,24 @@
package com.gtnewhorizons.angelica.mixins.early.celeritas.terrain;
import net.minecraft.tileentity.TileEntity;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Unique;
import com.gtnewhorizons.angelica.mixins.interfaces.IAwaitingDescriptorTE;
@Mixin(TileEntity.class)
public abstract class MixinTileEntity_AwaitingDescriptor implements IAwaitingDescriptorTE {
@Unique private boolean angelica$awaitingDescriptor;
@Override
public boolean angelica$isAwaitingDescriptor() {
return this.angelica$awaitingDescriptor;
}
@Override
public void angelica$setAwaitingDescriptor(boolean awaiting) {
this.angelica$awaitingDescriptor = awaiting;
}
}
@@ -0,0 +1,72 @@
package com.gtnewhorizons.angelica.mixins.early.celeritas.terrain;
import net.minecraft.block.Block;
import net.minecraft.client.multiplayer.WorldClient;
import net.minecraft.profiler.Profiler;
import net.minecraft.tileentity.TileEntity;
import net.minecraft.world.World;
import net.minecraft.world.WorldProvider;
import net.minecraft.world.WorldSettings;
import net.minecraft.world.biome.BiomeGenBase;
import net.minecraft.world.storage.ISaveHandler;
import net.minecraftforge.common.util.ForgeDirection;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Unique;
import com.gtnewhorizons.angelica.rendering.RenderThreadContext;
import com.gtnewhorizons.angelica.rendering.WorkerWorldAccess;
import com.gtnewhorizons.angelica.rendering.celeritas.world.WorldSlice;
@Mixin(WorldClient.class)
public abstract class MixinWorldClient_WorkerAccess extends World {
private MixinWorldClient_WorkerAccess(ISaveHandler saveHandler, String name, WorldProvider provider, WorldSettings settings, Profiler profiler) {
super(saveHandler, name, provider, settings, profiler);
}
@Unique
private static WorldSlice angelica$workerSlice(String method, int x, int y, int z) {
final WorldSlice slice = RenderThreadContext.workerSlice();
if (slice != null && !slice.containsBlock(x, y, z)) {
WorkerWorldAccess.readOutsideSlice(method, x, y, z, slice.getRenderingBlock());
}
return slice;
}
@Override
public Block getBlock(int x, int y, int z) {
final WorldSlice slice = angelica$workerSlice("getBlock", x, y, z);
return slice == null ? super.getBlock(x, y, z) : slice.getBlock(x, y, z);
}
@Override
public int getBlockMetadata(int x, int y, int z) {
final WorldSlice slice = angelica$workerSlice("getBlockMetadata", x, y, z);
return slice == null ? super.getBlockMetadata(x, y, z) : slice.getBlockMetadata(x, y, z);
}
@Override
public TileEntity getTileEntity(int x, int y, int z) {
final WorldSlice slice = angelica$workerSlice("getTileEntity", x, y, z);
return slice == null ? super.getTileEntity(x, y, z) : slice.getTileEntity(x, y, z);
}
@Override
public int getLightBrightnessForSkyBlocks(int x, int y, int z, int min) {
final WorldSlice slice = angelica$workerSlice("getLightBrightnessForSkyBlocks", x, y, z);
return slice == null ? super.getLightBrightnessForSkyBlocks(x, y, z, min) : slice.getLightBrightnessForSkyBlocks(x, y, z, min);
}
@Override
public boolean isSideSolid(int x, int y, int z, ForgeDirection side, boolean _default) {
final WorldSlice slice = angelica$workerSlice("isSideSolid", x, y, z);
return slice == null ? super.isSideSolid(x, y, z, side, _default) : slice.isSideSolid(x, y, z, side, _default);
}
@Override
public BiomeGenBase getBiomeGenForCoords(int x, int z) {
final WorldSlice slice = RenderThreadContext.workerSlice();
return slice == null ? super.getBiomeGenForCoords(x, z) : slice.getBiomeGenForCoords(x, z);
}
}
@@ -0,0 +1,25 @@
package com.gtnewhorizons.angelica.mixins.early.celeritas.terrain;
import net.minecraft.tileentity.TileEntity;
import net.minecraft.world.World;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
import com.gtnewhorizons.angelica.rendering.RenderThreadContext;
import com.gtnewhorizons.angelica.rendering.WorkerWorldAccess;
import com.gtnewhorizons.angelica.rendering.celeritas.world.WorldSlice;
@Mixin(World.class)
public abstract class MixinWorld_WorkerMutationGuard {
@Inject(method = "setTileEntity", at = @At("HEAD"), cancellable = true)
private void angelica$blockWorkerSetTileEntity(int x, int y, int z, TileEntity tileEntity, CallbackInfo ci) {
final WorldSlice slice = RenderThreadContext.workerSlice();
if (slice == null) return;
WorkerWorldAccess.blockedWrite("setTileEntity", x, y, z, slice.getRenderingBlock());
ci.cancel();
}
}
@@ -0,0 +1,91 @@
package com.gtnewhorizons.angelica.utils;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import net.minecraft.tileentity.TileEntity;
import net.minecraft.world.ChunkPosition;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import com.gtnewhorizons.angelica.rendering.RenderThreadContext;
import com.gtnewhorizons.angelica.rendering.celeritas.world.WorldSlice;
class ConcurrentTileEntityMapTest {
private static final ChunkPosition POS = new ChunkPosition(1, 2, 3);
private final ConcurrentTileEntityMap map = new ConcurrentTileEntityMap();
@AfterEach
void leaveWorkerContext() {
RenderThreadContext.clear();
}
private static void enterWorkerContext() {
RenderThreadContext.set(mock(WorldSlice.class));
}
private static TileEntity tileEntity(boolean invalid) {
final TileEntity te = mock(TileEntity.class);
when(te.isInvalid()).thenReturn(invalid);
return te;
}
@Test
void containsKeyAgreesWithGetForAnInvalidTileEntityOnAWorker() {
map.put(POS, tileEntity(true));
enterWorkerContext();
assertNull(map.get(POS), "get must hide an invalid tile entity from a worker");
assertFalse(map.containsKey(POS), "containsKey must agree with get");
}
@Test
void containsKeyAgreesWithGetForAValidTileEntityOnAWorker() {
final TileEntity te = tileEntity(false);
map.put(POS, te);
enterWorkerContext();
assertSame(te, map.get(POS));
assertTrue(map.containsKey(POS));
}
@Test
void containsKeyReportsInvalidTileEntitiesOnTheClientThread() {
map.put(POS, tileEntity(true));
assertTrue(map.containsKey(POS), "the client thread owns invalidation and must still see the entry");
}
@Test
void removeFromAWorkerDefersInsteadOfMutating() {
final TileEntity te = tileEntity(false);
map.put(POS, te);
enterWorkerContext();
assertNull(map.remove(POS));
assertEquals(1, map.size(), "a worker must not mutate the map");
RenderThreadContext.clear();
assertSame(te, map.get(POS), "a valid tile entity survives a deferred invalidation");
}
@Test
void deferredInvalidationEvictsOnTheNextClientThreadWrite() {
map.put(POS, tileEntity(true));
enterWorkerContext();
map.get(POS);
RenderThreadContext.clear();
map.put(new ChunkPosition(4, 5, 6), tileEntity(false));
assertFalse(map.containsKey(POS));
}
}