Heavily reworked the render service priority ordering

src/main/java/me/cortex/voxy/client/core/rendering/RenderService.java

@@ -79,7 +79,7 @@ public class RenderService<T extends AbstractSectionRenderer<J, ?>, J extends Vi
         this.viewportSelector = new ViewportSelector<>(this.sectionRenderer::createViewport);
         this.renderGen = new RenderGenerationService(world, this.modelService, serviceThreadPool,
                 this.geometryUpdateQueue::push, this.sectionRenderer.getGeometryManager() instanceof IUsesMeshlets,
-                ()->this.geometryUpdateQueue.count()<1000 && this.modelService.getProcessingCount()< 750);
+                ()->this.geometryUpdateQueue.count()<1000 && this.modelService.getProcessingCount()< 1000);
 
         router.setCallbacks(this.renderGen::enqueueTask, section -> {
             section.acquire();

src/main/java/me/cortex/voxy/client/core/rendering/VoxyRenderSystem.java

@@ -220,8 +220,6 @@ public class VoxyRenderSystem {
             TimingStatistics.update();
             debug.add("Voxy frame runtime (millis): " + TimingStatistics.setup.pVal() + ", " + TimingStatistics.dynamic.pVal() + ", " + TimingStatistics.main.pVal()+ ", " + TimingStatistics.all.pVal());
         }
-        int val = RenderGenerationService.FC.getAndSet(0);
-        debug.add("FC: " + val);
         PrintfDebugUtil.addToOut(debug);
     }
 

src/main/java/me/cortex/voxy/client/core/rendering/building/RenderGenerationService.java

@@ -3,6 +3,7 @@ package me.cortex.voxy.client.core.rendering.building;
 import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
 import it.unimi.dsi.fastutil.longs.Long2ObjectFunction;
 import it.unimi.dsi.fastutil.longs.Long2ObjectLinkedOpenHashMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap;
 import me.cortex.voxy.client.core.model.IdNotYetComputedException;
 import me.cortex.voxy.client.core.model.ModelBakerySubsystem;
 import me.cortex.voxy.common.Logger;
@@ -13,25 +14,44 @@ import me.cortex.voxy.common.world.other.Mapper;
 import me.cortex.voxy.common.thread.ServiceSlice;
 import me.cortex.voxy.common.thread.ServiceThreadPool;
 
+import java.util.Comparator;
 import java.util.List;
+import java.util.PriorityQueue;
+import java.util.concurrent.PriorityBlockingQueue;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.BooleanSupplier;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
 //TODO: Add a render cache
+
+
+//TODO: to add remove functionallity add a "defunked" variable to the build task and set it to true on remove
+// and process accordingly
 public class RenderGenerationService {
+    private static final AtomicInteger COUNTER = new AtomicInteger();
     private static final class BuildTask {
         WorldSection section;
         final long position;
         boolean hasDoneModelRequestInner;
         boolean hasDoneModelRequestOuter;
+        int attempts;
+        int addin;
+        long priority = Long.MIN_VALUE;
         private BuildTask(long position) {
             this.position = position;
         }
+        private void updatePriority() {
+            int unique = COUNTER.incrementAndGet();
+            int lvl = WorldEngine.MAX_LOD_LAYER-WorldEngine.getLevel(this.position);
+            lvl = Math.min(lvl, 3);//Make the 2 highest quality have equal priority
+            this.priority = (((lvl*3L + this.attempts)*2 + this.addin) <<32) + Integer.toUnsignedLong(unique);
+            this.addin = 0;
+        }
     }
 
-    private final Long2ObjectLinkedOpenHashMap<BuildTask> taskQueue = new Long2ObjectLinkedOpenHashMap<>();
+    private final PriorityBlockingQueue<BuildTask> taskQueue = new PriorityBlockingQueue<>(320000, (a,b)-> Long.compareUnsigned(a.priority, b.priority));
+    private final Long2ObjectOpenHashMap<BuildTask> taskMap = new Long2ObjectOpenHashMap<>(320000);
 
     private final WorldEngine world;
     private final ModelBakerySubsystem modelBakery;
@@ -101,14 +121,10 @@ public class RenderGenerationService {
         return WorldEngine.getLevel(pos) > 2;
     }
 
-    public static final AtomicInteger FC = new AtomicInteger(0);
     //TODO: add a generated render data cache
     private void processJob(RenderDataFactory45 factory, IntOpenHashSet seenMissedIds) {
-        BuildTask task;
+        BuildTask task = this.taskQueue.poll();
-        synchronized (this.taskQueue) {
+
-            task = this.taskQueue.removeFirst();
-            //task = (Math.random() < 0.1)?this.taskQueue.removeLast():this.taskQueue.removeFirst();
-        }
         //long time = BuiltSection.getTime();
         boolean shouldFreeSection = true;
 
@@ -125,74 +141,93 @@ public class RenderGenerationService {
         }
         section.assertNotFree();
         BuiltSection mesh = null;
+
+        synchronized (this.taskMap) {
+            var rtask = this.taskMap.remove(task.position);
+            if (rtask != task) {
+                throw new IllegalStateException();
+            }
+        }
+
         try {
             mesh = factory.generateMesh(section);
         } catch (IdNotYetComputedException e) {
-            if (e.isIdBlockId) {
-                //TODO: maybe move this to _after_ task as been readded to queue??
-                if (!this.modelBakery.factory.hasModelForBlockId(e.id)) {
-                    if (seenMissedIds.add(e.id)) {
-                        this.modelBakery.requestBlockBake(e.id);
-                    }
-                }
-            }
-
-            if (task.hasDoneModelRequestInner && task.hasDoneModelRequestOuter) {
-                FC.addAndGet(1);
-                try {
-                    Thread.sleep(1);
-                } catch (InterruptedException ex) {
-                    throw new RuntimeException(ex);
-                }
-            }
-
-            if (!task.hasDoneModelRequestInner) {
-                //The reason for the extra id parameter is that we explicitly add/check against the exception id due to e.g. requesting accross a chunk boarder wont be captured in the request
-                if (e.auxData == null)//the null check this is because for it to be, the inner must already be computed
-                    this.computeAndRequestRequiredModels(seenMissedIds, section);
-                task.hasDoneModelRequestInner = true;
-            }
-            if ((!task.hasDoneModelRequestOuter) && e.auxData != null) {
-                this.computeAndRequestRequiredModels(seenMissedIds, e.auxBitMsk, e.auxData);
-                task.hasDoneModelRequestOuter = true;
-            }
-
-
-            {//Keep the lock on the section, and attach it to the task, this prevents needing to re-aquire it later
-                task.section = section;
-            }
             {
-                //We need to reinsert the build task into the queue
+                BuildTask other;
-                BuildTask queuedTask;
+                synchronized (this.taskMap) {
-                synchronized (this.taskQueue) {
+                    other = this.taskMap.putIfAbsent(task.position, task);
-                    queuedTask = this.taskQueue.putIfAbsent(section.key, task);
-                    if (queuedTask == null) {
-                        queuedTask = task;
-                    }
-
-                    if (queuedTask.hasDoneModelRequestInner && queuedTask.hasDoneModelRequestOuter && putTaskFirst(section.key)) {//Force higher priority
-                        this.taskQueue.getAndMoveToFirst(section.key);
-                    }
                 }
-
+                if (other != null) {//Weve been replaced
-                if (queuedTask == task) {//use the == not .equal to see if we need to release a permit
+                    //Request the block
-                    if (this.threads.isAlive()) {//Only execute if were not dead
+                    if (e.isIdBlockId) {
-                        this.threads.execute();//Since we put in queue, release permit
+                        //TODO: maybe move this to _after_ task as been readded to queue??
+                        if (!this.modelBakery.factory.hasModelForBlockId(e.id)) {
+                            if (seenMissedIds.add(e.id)) {
+                                this.modelBakery.requestBlockBake(e.id);
+                            }
+                        }
+                    }
+                    //Exchange info
+                    if (task.hasDoneModelRequestInner) {
+                        other.hasDoneModelRequestInner = true;
+                    }
+                    if (task.hasDoneModelRequestOuter) {
+                        other.hasDoneModelRequestOuter = true;
                     }
-
-                    //If we did put it in the queue, dont release the section
-                    shouldFreeSection = false;
-                } else {
-                    //Mark (or remark) the section as having models requested
-                    if (task.hasDoneModelRequestInner)
-                        queuedTask.hasDoneModelRequestInner = true;
-
-                    if (task.hasDoneModelRequestOuter)
-                        queuedTask.hasDoneModelRequestOuter = true;
-
-                    //Things went bad, set section to null and ensure section is freed
                     task.section = null;
                     shouldFreeSection = true;
+                    task = null;
+                }
+            }
+            if (task != null) {
+                //This is our task
+
+                //Request the block
+                if (e.isIdBlockId) {
+                    //TODO: maybe move this to _after_ task as been readded to queue??
+                    if (!this.modelBakery.factory.hasModelForBlockId(e.id)) {
+                        if (seenMissedIds.add(e.id)) {
+                            this.modelBakery.requestBlockBake(e.id);
+                        }
+                    }
+                }
+
+                if (task.hasDoneModelRequestInner && task.hasDoneModelRequestOuter) {
+                    task.attempts++;
+                    try {
+                        Thread.sleep(1);
+                    } catch (InterruptedException ex) {
+                        throw new RuntimeException(ex);
+                    }
+                } else {
+                    if (!task.hasDoneModelRequestInner) {
+                        //The reason for the extra id parameter is that we explicitly add/check against the exception id due to e.g. requesting accross a chunk boarder wont be captured in the request
+                        if (e.auxData == null)//the null check this is because for it to be, the inner must already be computed
+                            this.computeAndRequestRequiredModels(seenMissedIds, section);
+                        task.hasDoneModelRequestInner = true;
+                    }
+                    //If this happens... aahaha painnnn
+                    if (task.hasDoneModelRequestOuter) {
+                        task.attempts++;
+                    }
+
+                    if ((!task.hasDoneModelRequestOuter) && e.auxData != null) {
+                        this.computeAndRequestRequiredModels(seenMissedIds, e.auxBitMsk, e.auxData);
+                        task.hasDoneModelRequestOuter = true;
+                    }
+
+                    task.addin = 3;//Single time addin which gives the models time to bake before the task executes
+                }
+
+                //Keep the lock on the section, and attach it to the task, this prevents needing to re-aquire it later
+                task.section = section;
+                shouldFreeSection = false;
+
+                task.updatePriority();
+                this.taskQueue.add(task);
+
+                if (this.threads.isAlive()) {//Only execute if were not dead
+                    this.threads.execute();//Since we put in queue, release permit
                 }
             }
         }
@@ -208,27 +243,19 @@ public class RenderGenerationService {
 
 
     public void enqueueTask(long pos) {
-        synchronized (this.taskQueue) {
+        boolean[] isOurs = new boolean[1];
-            this.taskQueue.computeIfAbsent(pos, key->{
-                this.threads.execute();
-                return new BuildTask(key);
-            });
-            //Prioritize lower detail builds
-            if (putTaskFirst(pos)) {
-                this.taskQueue.getAndMoveToFirst(pos);
-            }
-        }
-    }
-
-    public void removeTask(long pos) {
         BuildTask task;
-        synchronized (this.taskQueue) {
+        synchronized (this.taskMap) {
-            task = this.taskQueue.remove(pos);
+            task = this.taskMap.computeIfAbsent(pos, p->{
+                isOurs[0] = true;
+                return new BuildTask(p);
+            });
         }
-        if (task != null) {
+        if (isOurs[0]) {//If its not ours we dont care about it
-            if (!this.threads.steal()) {
+            //Set priority and insert into queue and execute
-                throw new IllegalStateException("Failed to steal a task!!!");
+            task.updatePriority();
-            }
+            this.taskQueue.add(task);
+            this.threads.execute();
         }
     }
 
@@ -244,12 +271,15 @@ public class RenderGenerationService {
             int i = this.threads.drain();
             if (i == 0) break;
 
-            synchronized (this.taskQueue) {
+            synchronized (this.taskMap) {
                 for (int j = 0; j < i; j++) {
-                    var task = this.taskQueue.removeFirst();
+                    var task = this.taskQueue.remove();
                     if (task.section != null) {
                         task.section.release();
                     }
+                    if (this.taskMap.remove(task.position) != task) {
+                        throw new IllegalStateException();
+                    }
                 }
             }
         }
@@ -259,10 +289,15 @@ public class RenderGenerationService {
 
         //Cleanup any remaining data
         while (!this.taskQueue.isEmpty()) {
-            var task = this.taskQueue.removeFirst();
+            var task = this.taskQueue.remove();
             if (task.section != null) {
                 task.section.release();
             }
+            synchronized (this.taskMap) {
+                if (this.taskMap.remove(task.position) != task) {
+                    throw new IllegalStateException();
+                }
+            }
         }
     }