From 253de1f3433d71ae0789f1699deb506061ab6ee9 Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Sun, 30 Jun 2024 20:45:52 -0700 Subject: [PATCH] Lightly observed - Pass light updates to LightStorage so that we don't have to re-upload every tracked section every frame - Slightly optimize light section writing, still room for improvement - Remove dead code in LightStorage --- .../backend/engine/embed/LightStorage.java | 91 +++++++++---------- .../engine/embed/LightUpdateHolder.java | 32 +++++++ .../backend/mixin/ClientChunkCacheMixin.java | 29 ++++++ .../resources/flywheel.backend.mixins.json | 1 + 4 files changed, 104 insertions(+), 49 deletions(-) create mode 100644 common/src/backend/java/dev/engine_room/flywheel/backend/engine/embed/LightUpdateHolder.java create mode 100644 common/src/backend/java/dev/engine_room/flywheel/backend/mixin/ClientChunkCacheMixin.java diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/embed/LightStorage.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/embed/LightStorage.java index 8095e7cdb..8521b1835 100644 --- a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/embed/LightStorage.java +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/embed/LightStorage.java @@ -63,10 +63,28 @@ public class LightStorage { removeUnusedSections(allLightSections); - // Only add the new sections. - allLightSections.removeAll(section2ArenaIndex.keySet()); + var knownSections = section2ArenaIndex.keySet(); - for (var section : allLightSections) { + var updatedSections = LightUpdateHolder.get(level) + .getUpdatedSections(); + + // Only add the new sections. + allLightSections.removeAll(knownSections); + + for (long updatedSection : updatedSections) { + for (int x = -1; x <= 1; x++) { + for (int y = -1; y <= 1; y++) { + for (int z = -1; z <= 1; z++) { + long section = SectionPos.offset(updatedSection, x, y, z); + if (knownSections.contains(section)) { + allLightSections.add(section); + } + } + } + } + } + + for (long section : allLightSections) { addSection(section); } }); @@ -103,50 +121,38 @@ public class LightStorage { int yMin = SectionPos.sectionToBlockCoord(SectionPos.y(section)); int zMin = SectionPos.sectionToBlockCoord(SectionPos.z(section)); + var sectionPos = SectionPos.of(section); + var blockData = blockLight.getDataLayerData(sectionPos); + var skyData = skyLight.getDataLayerData(sectionPos); + int index = indexForSection(section); changed.set(index); long ptr = arena.indexToPointer(index); + // Not sure of a way to iterate over the surface of a cube, so branch in the inner loop to take the fast path. + // Adding the fast path is about 8x faster than having only the slow path. + // There's still room for optimization, as the slow path takes about 3x the cpu time as the fast path despite + // being called an order of magnitude less. for (int y = -1; y < 17; y++) { for (int z = -1; z < 17; z++) { for (int x = -1; x < 17; x++) { - blockPos.set(xMin + x, yMin + y, zMin + z); - var block = blockLight.getLightValue(blockPos); - var sky = skyLight.getLightValue(blockPos); + if (x == -1 || y == -1 || z == -1 || x == 16 || y == 16 || z == 16) { + // Slow path, collect the surface of our section. + blockPos.set(xMin + x, yMin + y, zMin + z); + var block = blockLight.getLightValue(blockPos); + var sky = skyLight.getLightValue(blockPos); - write(ptr, x, y, z, block, sky); - } - } - } - } + write(ptr, x, y, z, block, sky); + } else { + // Fast path, read directly from the data layer for the main section. + // Would be nice to move the null check elsewhere. + var block = blockData == null ? 0 : blockData.get(x, y, z); + var sky = skyData == null ? 0 : skyData.get(x, y, z); - void addSectionFast(long section) { - // TODO: get this to work. it should be MUCH faster to read directly from the data layer - // though it's more complicated to manage which section datas we fetch - var lightEngine = level.getLightEngine(); - - var blockLight = lightEngine.getLayerListener(LightLayer.BLOCK); - var skyLight = lightEngine.getLayerListener(LightLayer.SKY); - - var sectionPos = SectionPos.of(section); - var blockData = blockLight.getDataLayerData(sectionPos); - var skyData = skyLight.getDataLayerData(sectionPos); - - if (blockData == null || skyData == null) { - return; - } - - long ptr = ptrForSection(section); - - for (int y = 0; y < 16; y++) { - for (int z = 0; z < 16; z++) { - for (int x = 0; x < 16; x++) { - var block = blockData.get(x, y, z); - var sky = skyData.get(x, y, z); - - write(ptr, x, y, z, block, sky); + write(ptr, x, y, z, block, sky); + } } } } @@ -173,19 +179,6 @@ public class LightStorage { MemoryUtil.memPutByte(ptr + offset, (byte) packedByte); } - private void writeFor2Cubed(long ptr, int x, int y, int z, int block, int sky) { - int x1 = x + 1; - int y1 = y + 1; - int z1 = z + 1; - - int longIndex = (x1 >> 1) + (z1 >> 1) * 9 + (y1 >> 1) * 9 * 9; - int byteIndexInLong = (x1 & 1) + ((z1 & 1) << 1) + ((y1 & 1) << 2); - - long packedByte = (block & 0xF) | ((sky & 0xF) << 4); - - MemoryUtil.memPutByte(ptr + longIndex * 8L + byteIndexInLong, (byte) packedByte); - } - /** * Get a pointer to the base of the given section. *

If the section is not yet reserved, allocate a chunk in the arena. diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/engine/embed/LightUpdateHolder.java b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/embed/LightUpdateHolder.java new file mode 100644 index 000000000..b61dfc197 --- /dev/null +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/engine/embed/LightUpdateHolder.java @@ -0,0 +1,32 @@ +package dev.engine_room.flywheel.backend.engine.embed; + +import dev.engine_room.flywheel.lib.util.LevelAttached; +import it.unimi.dsi.fastutil.longs.LongArraySet; +import it.unimi.dsi.fastutil.longs.LongSet; +import net.minecraft.world.level.LevelAccessor; + +/** + * Stores the set of updates light sections for LightStorage to poll in its frame plan. + */ +public class LightUpdateHolder { + private static final LevelAttached HOLDERS = new LevelAttached<>(level -> new LightUpdateHolder()); + + public static LightUpdateHolder get(LevelAccessor level) { + return HOLDERS.get(level); + } + + private final LongSet updatedSections = new LongArraySet(); + + public LongSet getUpdatedSections() { + var out = new LongArraySet(updatedSections); + updatedSections.clear(); + return out; + } + + public void add(long section) { + updatedSections.add(section); + } + + private LightUpdateHolder() { + } +} diff --git a/common/src/backend/java/dev/engine_room/flywheel/backend/mixin/ClientChunkCacheMixin.java b/common/src/backend/java/dev/engine_room/flywheel/backend/mixin/ClientChunkCacheMixin.java new file mode 100644 index 000000000..11ea0cea2 --- /dev/null +++ b/common/src/backend/java/dev/engine_room/flywheel/backend/mixin/ClientChunkCacheMixin.java @@ -0,0 +1,29 @@ +package dev.engine_room.flywheel.backend.mixin; + +import org.spongepowered.asm.mixin.Final; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.Shadow; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Inject; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; + +import dev.engine_room.flywheel.backend.engine.embed.LightUpdateHolder; +import net.minecraft.client.multiplayer.ClientChunkCache; +import net.minecraft.client.multiplayer.ClientLevel; +import net.minecraft.core.SectionPos; +import net.minecraft.world.level.LightLayer; + +@Mixin(ClientChunkCache.class) +abstract class ClientChunkCacheMixin { + @Shadow + @Final + ClientLevel level; + + @Inject(method = "onLightUpdate", at = @At("HEAD")) + private void flywheel$backend$onLightUpdate(LightLayer layer, SectionPos pos, CallbackInfo ci) { + // This is duplicated from code in impl, but I'm not sure that it + // makes sense to be generically passed to backends. + LightUpdateHolder.get(level) + .add(pos.asLong()); + } +} diff --git a/common/src/backend/resources/flywheel.backend.mixins.json b/common/src/backend/resources/flywheel.backend.mixins.json index 2d924bef0..e303f7b03 100644 --- a/common/src/backend/resources/flywheel.backend.mixins.json +++ b/common/src/backend/resources/flywheel.backend.mixins.json @@ -6,6 +6,7 @@ "refmap": "backend-flywheel.refmap.json", "client": [ "AbstractClientPlayerAccessor", + "ClientChunkCacheMixin", "GlStateManagerMixin", "LevelRendererAccessor", "OptionsMixin",