fix empty chunk data handling, improve multithreading in WorldRenderer

doc/rendering/world_renderer.md

@@ -25,12 +25,12 @@
   - Respawn
 - texture animations
 - require neighbour chunks loaded
-  - Also don't load if block changes in chunk (e.g. when movement is disabled and walking to chunk border)
+  - Also don't load if block changes in chunk (e.g. when movement is disabled and walking to chunk border and destroying block)
 - View distance
   - Server side
   - Client side
 - Rewrite renderers
-- Check neighbor positions
+- Check neighbour positions
 - Cache biomes
 - "Fast biome" in 19w36a+
 - Improved biome blending

src/main/java/de/bixilon/minosoft/data/world/World.kt

@@ -124,9 +124,8 @@ class World(
     }
 
     fun unloadChunk(chunkPosition: Vec2i) {
-        chunks.remove(chunkPosition)?.let {
+        val chunk = chunks.remove(chunkPosition) ?: return
-            connection.fireEvent(ChunkUnloadEvent(connection, EventInitiators.UNKNOWN, chunkPosition))
+        connection.fireEvent(ChunkUnloadEvent(connection, EventInitiators.UNKNOWN, chunkPosition, chunk))
-        }
     }
 
     fun replaceChunk(position: Vec2i, chunk: Chunk) {

src/main/java/de/bixilon/minosoft/gui/rendering/block/WorldRenderer.kt

@@ -44,9 +44,10 @@ import de.bixilon.minosoft.protocol.network.connection.play.PlayConnection
 import de.bixilon.minosoft.util.KUtil.synchronizedMapOf
 import de.bixilon.minosoft.util.KUtil.synchronizedSetOf
 import de.bixilon.minosoft.util.KUtil.toResourceLocation
-import de.bixilon.minosoft.util.KUtil.toSynchronizedMap
 import de.bixilon.minosoft.util.KUtil.unsafeCast
-import de.bixilon.minosoft.util.collections.SynchronizedMap
+import de.bixilon.minosoft.util.logging.Log
+import de.bixilon.minosoft.util.logging.LogLevels
+import de.bixilon.minosoft.util.logging.LogMessageType
 import de.bixilon.minosoft.util.task.pool.DefaultThreadPool
 import de.bixilon.minosoft.util.task.pool.ThreadPool.Priorities.LOW
 import de.bixilon.minosoft.util.task.pool.ThreadPoolRunnable
@@ -67,18 +68,22 @@ class WorldRenderer(
     private val world: World = connection.world
     private val sectionPreparer: AbstractSectionPreparer = GenericSectionPreparer(renderWindow)
     private val lightMap = LightMap(connection)
-    private val meshes: SynchronizedMap<Vec2i, SynchronizedMap<Int, ChunkSectionMeshes>> = synchronizedMapOf() // all prepared (and up to date) meshes
+    private val meshes: MutableMap<Vec2i, MutableMap<Int, ChunkSectionMeshes>> = mutableMapOf() // all prepared (and up to date) meshes
-    private var incomplete: MutableSet<Vec2i> = synchronizedSetOf() // Queue of chunk positions that can not be rendered yet (data not complete or neighbours not completed yet)
+    private val incomplete: MutableSet<Vec2i> = synchronizedSetOf() // Queue of chunk positions that can not be rendered yet (data not complete or neighbours not completed yet)
-    private var queue: MutableMap<Vec2i, MutableSet<Int>> = synchronizedMapOf() // Chunk sections that can be prepared or have changed, but are not required to get rendered yet (i.e. culled chunks)
+    private val queue: MutableMap<Vec2i, MutableSet<Int>> = mutableMapOf() // Chunk sections that can be prepared or have changed, but are not required to get rendered yet (i.e. culled chunks)
+    //   private val preparingTasks: SynchronizedMap<Vec2i, SynchronizedMap<Int, ThreadPoolRunnable>> = synchronizedMapOf()
 
     private var visibleOpaque: MutableList<ChunkSectionMesh> = mutableListOf()
     private var visibleTranslucent: MutableList<ChunkSectionMesh> = mutableListOf()
     private var visibleTransparent: MutableList<ChunkSectionMesh> = mutableListOf()
 
 
-    val visibleOpaqueSize: Int by visibleOpaque::size
+    val visibleOpaqueSize: Int
-    val visibleTranslucentSize: Int by visibleTranslucent::size
+        get() = visibleOpaque.size
-    val visibleTransparentSize: Int by visibleTransparent::size
+    val visibleTranslucentSize: Int
+        get() = visibleTranslucent.size
+    val visibleTransparentSize: Int
+        get() = visibleTransparent.size
     val preparedSize: Int by meshes::size
     val queuedSize: Int by queue::size
     val incompleteSize: Int by incomplete::size
@@ -117,13 +122,15 @@ class WorldRenderer(
             if (!neighbourChunks.fullyLoaded) {
                 return@of
             }
-            val meshes = meshes.getOrPut(it.chunkPosition) { synchronizedMapOf() }
             val sectionHeights: MutableSet<Int> = mutableSetOf()
             for (blockPosition in it.blocks.keys) {
                 sectionHeights += blockPosition.sectionHeight
             }
-            for (sectionHeight in sectionHeights) {
+            renderWindow.queue += {
-                updateSection(it.chunkPosition, sectionHeight, chunk, neighbourChunks.unsafeCast(), meshes)
+                val meshes = meshes.getOrPut(it.chunkPosition) { synchronizedMapOf() }
+                for (sectionHeight in sectionHeights) {
+                    updateSection(it.chunkPosition, sectionHeight, chunk, neighbourChunks.unsafeCast(), meshes)
+                }
             }
         })
         connection.registerEvent(CallbackEventInvoker.of<ChunkUnloadEvent> { unloadChunk(it.chunkPosition) })
@@ -131,17 +138,18 @@ class WorldRenderer(
 
     private fun unloadChunk(chunkPosition: Vec2i) {
         incomplete -= chunkPosition
-        queue.remove(chunkPosition)
+        renderWindow.queue += { queue.remove(chunkPosition) }
         for (neighbourPosition in getChunkNeighbourPositions(chunkPosition)) {
-            queue.remove(neighbourPosition)
+            renderWindow.queue += { queue.remove(neighbourPosition) }
             world[neighbourPosition] ?: continue // if chunk is not loaded, we don't need to add it to incomplete
             incomplete += neighbourPosition
         }
-        val meshes = this.meshes.remove(chunkPosition) ?: return
+        renderWindow.queue += add@{
-        if (meshes.isEmpty()) {
+            val meshes = this.meshes.remove(chunkPosition) ?: return@add
-            return
+            if (meshes.isEmpty()) {
-        }
+                return@add
-        renderWindow.queue += {
+            }
+
             for (mesh in meshes.values) {
                 removeMesh(mesh)
                 mesh.unload()
@@ -240,21 +248,30 @@ class WorldRenderer(
             return
         }
         incomplete -= chunkPosition
-        val meshes = this.meshes.getOrPut(chunkPosition) { synchronizedMapOf() }
+        renderWindow.queue += {
+            val meshes = this.meshes.getOrPut(chunkPosition) { synchronizedMapOf() }
 
-        for ((sectionHeight, section) in chunk.sections!!) {
+            for (sectionHeight in chunk.sections!!.keys) {
-            updateSection(chunkPosition, sectionHeight, chunk, neighbourChunks.unsafeCast(), meshes)
+                updateSection(chunkPosition, sectionHeight, chunk, neighbourChunks.unsafeCast(), meshes)
+            }
         }
     }
 
-    private fun updateSection(chunkPosition: Vec2i, sectionHeight: Int, chunk: Chunk = world[chunkPosition]!!, neighbourChunks: Array<Chunk> = getChunkNeighbours(getChunkNeighbourPositions(chunkPosition)).unsafeCast(), meshes: SynchronizedMap<Int, ChunkSectionMeshes> = this.meshes.getOrPut(chunkPosition) { synchronizedMapOf() }) {
+    private fun updateSection(chunkPosition: Vec2i, sectionHeight: Int, chunk: Chunk = world[chunkPosition]!!, neighbourChunks: Array<Chunk> = getChunkNeighbours(getChunkNeighbourPositions(chunkPosition)).unsafeCast(), meshes: MutableMap<Int, ChunkSectionMeshes>? = null) {
-        val task = ThreadPoolRunnable(priority = LOW) {
+        // val chunkTasks = preparingTasks.getOrPut(chunkPosition) { synchronizedMapOf() }
-            updateSectionSync(chunkPosition, sectionHeight, chunk, neighbourChunks, meshes)
+        // chunkTasks.remove(sectionHeight)?.interrupt()
+        val task = ThreadPoolRunnable(priority = LOW, interuptable = false) {
+            try {
+                updateSectionSync(chunkPosition, sectionHeight, chunk, neighbourChunks, meshes)
+            } catch (exception: InterruptedException) {
+                Log.log(LogMessageType.RENDERING_GENERAL, LogLevels.WARN) { exception.message!! }
+            }
         }
+        // chunkTasks[sectionHeight] = task
         DefaultThreadPool += task
     }
 
-    private fun updateSectionSync(chunkPosition: Vec2i, sectionHeight: Int, chunk: Chunk, neighbourChunks: Array<Chunk>, meshes: SynchronizedMap<Int, ChunkSectionMeshes>) {
+    private fun updateSectionSync(chunkPosition: Vec2i, sectionHeight: Int, chunk: Chunk, neighbourChunks: Array<Chunk>, meshes: MutableMap<Int, ChunkSectionMeshes>? = null) {
         if (!chunk.isFullyLoaded || chunkPosition in incomplete) {
             // chunk not loaded and/or neighbours also not fully loaded
             return
@@ -262,29 +279,31 @@ class WorldRenderer(
         val section = chunk.sections!![sectionHeight] ?: return
 
         val visible = isChunkVisible(chunkPosition, sectionHeight, Vec3i.EMPTY, Vec3i(16, 16, 16)) // ToDo: min/maxPosition
-        val previousMesh = meshes[sectionHeight]
 
-        if (previousMesh != null && !visible) {
+        renderWindow.queue += {
-            meshes.remove(sectionHeight)
+            val meshes = meshes ?: this.meshes.getOrPut(chunkPosition) { synchronizedMapOf() }
-            renderWindow.queue += {
+            val previousMesh = meshes[sectionHeight]
+            if (previousMesh != null && !visible) {
+                meshes.remove(sectionHeight)
                 removeMesh(previousMesh)
                 previousMesh.unload()
             }
         }
 
         if (visible) {
-            // ToDo: Possible threading issue
+            renderWindow.queue += {
-            val sectionQueue = queue[chunkPosition]
+                val sectionQueue = queue[chunkPosition]
-            if (sectionQueue != null) {
+                if (sectionQueue != null) {
-                sectionQueue -= sectionHeight
+                    sectionQueue -= sectionHeight
-                if (sectionQueue.isEmpty()) {
+                    if (sectionQueue.isEmpty()) {
-                    queue.remove(chunkPosition)
+                        queue.remove(chunkPosition)
+                    }
                 }
             }
             val neighbours = getSectionNeighbours(neighbourChunks, chunk, sectionHeight)
             prepareSection(chunkPosition, sectionHeight, section, neighbours, meshes)
         } else {
-            queue.getOrPut(chunkPosition) { synchronizedSetOf() } += sectionHeight
+            renderWindow.queue += { queue.getOrPut(chunkPosition) { mutableSetOf() } += sectionHeight }
         }
     }
 
@@ -292,12 +311,12 @@ class WorldRenderer(
     /**
      * Preparse a chunk section, loads in (in the renderQueue) and stores it in the meshes. Should run on another thread
      */
-    private fun prepareSection(chunkPosition: Vec2i, sectionHeight: Int, section: ChunkSection, neighbours: Array<ChunkSection?>, meshes: SynchronizedMap<Int, ChunkSectionMeshes> = this.meshes.getOrPut(chunkPosition) { synchronizedMapOf() }) {
+    private fun prepareSection(chunkPosition: Vec2i, sectionHeight: Int, section: ChunkSection, neighbours: Array<ChunkSection?>, meshes: MutableMap<Int, ChunkSectionMeshes>? = null) {
         val mesh = sectionPreparer.prepare(chunkPosition, sectionHeight, section, neighbours)
 
-        val previousMesh = meshes.remove(sectionHeight)
-
         renderWindow.queue += {
+            val meshes = meshes ?: this.meshes.getOrPut(chunkPosition) { synchronizedMapOf() }
+            val previousMesh = meshes.remove(sectionHeight)
             if (previousMesh != null) {
                 removeMesh(previousMesh)
             }
@@ -353,7 +372,7 @@ class WorldRenderer(
         val visibleTranslucent: MutableList<ChunkSectionMesh> = mutableListOf()
         val visibleTransparent: MutableList<ChunkSectionMesh> = mutableListOf()
 
-        for ((chunkPosition, meshes) in this.meshes.toSynchronizedMap()) {
+        for ((chunkPosition, meshes) in this.meshes) {
             for ((sectionHeight, mesh) in meshes) {
                 if (!isChunkVisible(chunkPosition, sectionHeight, mesh.minPosition, mesh.maxPosition)) {
                     continue
@@ -364,10 +383,11 @@ class WorldRenderer(
             }
         }
 
-        for ((chunkPosition, sectionHeights) in this.queue.toSynchronizedMap()) {
+        val removeFromQueue: MutableSet<Vec2i> = mutableSetOf()
+        for ((chunkPosition, sectionHeights) in this.queue) {
             val chunk = world[chunkPosition]
             if (chunk == null || !chunk.isFullyLoaded || chunkPosition in incomplete) {
-                this.queue.remove(chunkPosition)
+                removeFromQueue += chunkPosition
                 continue
             }
             val neighbours = getChunkNeighbours(getChunkNeighbourPositions(chunkPosition))
@@ -376,6 +396,8 @@ class WorldRenderer(
                 updateSection(chunkPosition, sectionHeight, chunk, neighbours.unsafeCast(), meshes)
             }
         }
+        this.queue -= removeFromQueue
+
         val cameraPositionLength = connection.player.cameraPosition.length2()
 
         visibleOpaque.sortBy { it.centerLength - cameraPositionLength }

src/main/java/de/bixilon/minosoft/gui/rendering/block/mesh/ChunkSectionMesh.kt

@@ -33,7 +33,7 @@ class ChunkSectionMesh(renderWindow: RenderWindow, initialCacheSize: Int, val ce
             transformedUV.x,
             transformedUV.y,
             Float.fromBits(texture.renderData?.shaderTextureId ?: RenderConstants.DEBUG_TEXTURE_ID),
-            Float.fromBits(tintColor or (light shl 24)), // white
+            Float.fromBits(tintColor or (light shl 24)),
         ))
     }
 

src/main/java/de/bixilon/minosoft/gui/rendering/block/mesh/ChunkSectionMeshes.kt

@@ -25,11 +25,11 @@ class ChunkSectionMeshes(
     sectionHeight: Int,
 ) {
     private val centerLength = Vec3d(Vec3i.of(chunkPosition, sectionHeight, Vec3i(8, 8, 8))).length2()
-    var opaqueMesh: ChunkSectionMesh? = ChunkSectionMesh(renderWindow, 200000, centerLength)
+    var opaqueMesh: ChunkSectionMesh? = ChunkSectionMesh(renderWindow, 150000, centerLength)
         private set
-    var translucentMesh: ChunkSectionMesh? = ChunkSectionMesh(renderWindow, 100000, centerLength)
+    var translucentMesh: ChunkSectionMesh? = ChunkSectionMesh(renderWindow, 50000, centerLength)
         private set
-    var transparentMesh: ChunkSectionMesh? = ChunkSectionMesh(renderWindow, 100000, centerLength)
+    var transparentMesh: ChunkSectionMesh? = ChunkSectionMesh(renderWindow, 50000, centerLength)
         private set
 
     // used for frustum culling

src/main/java/de/bixilon/minosoft/modding/event/events/ChunkUnloadEvent.kt

@@ -12,17 +12,15 @@
  */
 package de.bixilon.minosoft.modding.event.events
 
+import de.bixilon.minosoft.data.world.Chunk
 import de.bixilon.minosoft.modding.event.EventInitiators
 import de.bixilon.minosoft.modding.event.events.connection.play.PlayConnectionEvent
 import de.bixilon.minosoft.protocol.network.connection.play.PlayConnection
-import de.bixilon.minosoft.protocol.packets.s2c.play.ChunkUnloadS2CP
 import glm_.vec2.Vec2i
 
 class ChunkUnloadEvent(
     connection: PlayConnection,
     initiator: EventInitiators,
     val chunkPosition: Vec2i,
-) : PlayConnectionEvent(connection, initiator) {
+    val chunk: Chunk,
-
+) : PlayConnectionEvent(connection, initiator)
-    constructor(connection: PlayConnection, packet: ChunkUnloadS2CP) : this(connection, EventInitiators.SERVER, packet.chunkPosition)
-}

src/main/java/de/bixilon/minosoft/protocol/packets/s2c/play/ChunkDataS2CP.kt

@@ -18,9 +18,7 @@ import de.bixilon.minosoft.data.registries.ResourceLocation
 import de.bixilon.minosoft.data.world.ChunkData
 import de.bixilon.minosoft.data.world.biome.source.SpatialBiomeArray
 import de.bixilon.minosoft.datafixer.BlockEntityFixer.fix
-import de.bixilon.minosoft.modding.event.EventInitiators
 import de.bixilon.minosoft.modding.event.events.ChunkDataChangeEvent
-import de.bixilon.minosoft.modding.event.events.ChunkUnloadEvent
 import de.bixilon.minosoft.protocol.network.connection.play.PlayConnection
 import de.bixilon.minosoft.protocol.packets.s2c.PlayS2CPacket
 import de.bixilon.minosoft.protocol.protocol.PlayInByteBuffer
@@ -41,7 +39,8 @@ import java.util.*
 class ChunkDataS2CP(buffer: PlayInByteBuffer) : PlayS2CPacket() {
     val blockEntities: MutableMap<Vec3i, BlockEntity> = mutableMapOf()
     val chunkPosition: Vec2i
-    var chunkData: ChunkData? = ChunkData()
+    val chunkData: ChunkData = ChunkData()
+    var unloadChunk: Boolean = false
         private set
     var heightMap: Map<String, Any>? = null
         private set
@@ -63,11 +62,11 @@ class ChunkDataS2CP(buffer: PlayInByteBuffer) : PlayS2CPacket() {
             } else {
                 buffer
             }
-            ChunkUtil.readChunkPacket(decompressed, dimension, sectionBitMask, addBitMask, !isFullChunk, dimension.hasSkyLight)?.let {
+            val chunkData = ChunkUtil.readChunkPacket(decompressed, dimension, sectionBitMask, addBitMask, !isFullChunk, dimension.hasSkyLight)
-                chunkData!!.replace(it)
+            if (chunkData == null) {
-            } ?: let {
+                unloadChunk = true
-                // unload chunk
+            } else {
-                chunkData = null
+                this.chunkData.replace(chunkData)
             }
         } else {
             if (buffer.versionId >= ProtocolVersions.V_1_16_PRE7 && buffer.versionId < ProtocolVersions.V_1_16_2_PRE2) {
@@ -91,19 +90,18 @@ class ChunkDataS2CP(buffer: PlayInByteBuffer) : PlayS2CPacket() {
                 heightMap = buffer.readNBT()?.compoundCast()
             }
             if (!isFullChunk) {
-                chunkData!!.biomeSource = SpatialBiomeArray(buffer.readBiomeArray())
+                chunkData.biomeSource = SpatialBiomeArray(buffer.readBiomeArray())
             }
             val size = buffer.readVarInt()
             val lastPos = buffer.pointer
-            if (size > 0) {
+            val chunkData = ChunkUtil.readChunkPacket(buffer, dimension, sectionBitMask, null, !isFullChunk, dimension.hasSkyLight)
-                ChunkUtil.readChunkPacket(buffer, dimension, sectionBitMask, null, !isFullChunk, dimension.hasSkyLight)?.let {
+            if (chunkData == null) {
-                    chunkData!!.replace(it)
+                unloadChunk = true
-                } ?: let {
+            } else {
-                    chunkData = null
+                this.chunkData.replace(chunkData)
-                }
-                // set position of the byte buffer, because of some reasons HyPixel makes some weird stuff and sends way to much 0 bytes. (~ 190k), thanks @pokechu22
-                buffer.pointer = size + lastPos
             }
+            // set position of the byte buffer, because of some reasons HyPixel makes some weird stuff and sends way to much 0 bytes. (~ 190k), thanks @pokechu22
+            buffer.pointer = size + lastPos
             if (buffer.versionId >= ProtocolVersions.V_1_9_4) {
                 val blockEntitiesCount = buffer.readVarInt()
                 for (i in 0 until blockEntitiesCount) {
@@ -123,15 +121,14 @@ class ChunkDataS2CP(buffer: PlayInByteBuffer) : PlayS2CPacket() {
     }
 
     override fun handle(connection: PlayConnection) {
-        chunkData?.let {
+        if (unloadChunk) {
-            val chunk = connection.world.getOrCreateChunk(chunkPosition)
-            chunk.setData(chunkData!!)
-            connection.world.setBlockEntities(blockEntities)
-            connection.fireEvent(ChunkDataChangeEvent(connection, this))
-        } ?: let {
             connection.world.unloadChunk(chunkPosition)
-            connection.fireEvent(ChunkUnloadEvent(connection, EventInitiators.SERVER, chunkPosition))
+            return
         }
+        val chunk = connection.world.getOrCreateChunk(chunkPosition)
+        chunk.setData(chunkData)
+        connection.world.setBlockEntities(blockEntities)
+        connection.fireEvent(ChunkDataChangeEvent(connection, this))
     }
 
     override fun log() {

src/main/java/de/bixilon/minosoft/protocol/packets/s2c/play/JoinGameS2CP.kt

@@ -168,7 +168,7 @@ class JoinGameS2CP(buffer: PlayInByteBuffer) : PlayS2CPacket() {
             NoiseBiomeAccessor(connection.world)
         }
         TimeWorker.addTask(TimeWorkerTask(150, true) { // ToDo: Temp workaround
-            connection.sendPacket(ClientSettingsC2SP("en_us"))
+            connection.sendPacket(ClientSettingsC2SP())
 
             val brandName = DefaultRegistries.DEFAULT_PLUGIN_CHANNELS_REGISTRY.forVersion(connection.version)[DefaultPluginChannels.BRAND]!!.resourceLocation
             val buffer = PlayOutByteBuffer(connection)

src/main/java/de/bixilon/minosoft/protocol/packets/s2c/play/MassChunkDataS2CP.kt

@@ -16,7 +16,6 @@ import de.bixilon.minosoft.Minosoft
 import de.bixilon.minosoft.data.world.ChunkData
 import de.bixilon.minosoft.modding.event.EventInitiators
 import de.bixilon.minosoft.modding.event.events.ChunkDataChangeEvent
-import de.bixilon.minosoft.modding.event.events.ChunkUnloadEvent
 import de.bixilon.minosoft.protocol.network.connection.play.PlayConnection
 import de.bixilon.minosoft.protocol.packets.s2c.PlayS2CPacket
 import de.bixilon.minosoft.protocol.protocol.PlayInByteBuffer
@@ -69,16 +68,14 @@ class MassChunkDataS2CP() : PlayS2CPacket() {
     }
 
     override fun handle(connection: PlayConnection) {
-        // transform data
         for ((chunkPosition, data) in data) {
-            data?.let {
+            if (data == null) {
+                // unload chunk
+                connection.world.unloadChunk(chunkPosition)
+            } else {
                 val chunk = connection.world.getOrCreateChunk(chunkPosition)
                 chunk.setData(data)
                 connection.fireEvent(ChunkDataChangeEvent(connection, EventInitiators.SERVER, chunkPosition, chunk))
-            } ?: let {
-                // unload chunk
-                connection.world.unloadChunk(chunkPosition)
-                connection.fireEvent(ChunkUnloadEvent(connection, EventInitiators.SERVER, chunkPosition))
             }
         }
     }