From 5b4f87915856e7ec4b71bbfb507b54f2737aab70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elias=20M=C3=BCller?= Date: Thu, 5 Nov 2026 16:45:46 +0100 Subject: [PATCH] queued photos hold their chunks (reference-counted plugin tickets on the loaded chunks of the view, released after the snapshot or on failure): a photographer who teleported, changed dimension or logged out while the photo waited longer than the unload delay got a partly empty picture --- .../pixelpics/render/ChunkHolds.java | 75 +++++++++++++++++++ .../pixelpics/render/RenderManager.java | 7 ++ .../render/render/DefaultScreenRenderer.java | 27 +++++-- .../render/snapshot/SnapshotBuilder.java | 22 ++++-- .../pixelpics/survival/PhotoService.java | 28 ++++++- 5 files changed, 142 insertions(+), 17 deletions(-) create mode 100644 src/main/java/eu/mhsl/minecraft/pixelpics/render/ChunkHolds.java diff --git a/src/main/java/eu/mhsl/minecraft/pixelpics/render/ChunkHolds.java b/src/main/java/eu/mhsl/minecraft/pixelpics/render/ChunkHolds.java new file mode 100644 index 0000000..6143895 --- /dev/null +++ b/src/main/java/eu/mhsl/minecraft/pixelpics/render/ChunkHolds.java @@ -0,0 +1,75 @@ +package eu.mhsl.minecraft.pixelpics.render; + +import eu.mhsl.minecraft.pixelpics.render.snapshot.SnapshotBuilder; +import java.util.HashMap; +import java.util.Map; +import java.util.UUID; +import org.bukkit.Bukkit; +import org.bukkit.World; +import org.bukkit.plugin.Plugin; + +/** + * Keeps the chunks of a queued photo loaded until its snapshot is taken. The snapshot is only captured when the job + * starts; without a hold, a photographer who leaves meanwhile (teleport, other dimension, logout, flight) lets the + * chunks unload after Paper's unload delay and the photo comes out (partly) empty. Plugin chunk tickets are per plugin + * and not counted, hence the reference count per chunk. Only chunks loaded at the shot get a ticket, so a hold never + * loads a chunk. Main thread only. + */ +public final class ChunkHolds { + + private final Plugin plugin; + private final Map> counts = new HashMap<>(); + + ChunkHolds(Plugin plugin) { + this.plugin = plugin; + } + + /** Holds the loaded chunks of {@code area} in {@code world} until {@link Hold#release}. */ + public Hold hold(World world, SnapshotBuilder.ChunkArea area) { + Map held = counts.computeIfAbsent(world.getUID(), k -> new HashMap<>()); + int w = area.maxCX() - area.minCX() + 1, h = area.maxCZ() - area.minCZ() + 1; + long[] keys = new long[Math.max(0, w) * Math.max(0, h)]; + int n = 0; + for (int cx = area.minCX(); cx <= area.maxCX(); cx++) { + for (int cz = area.minCZ(); cz <= area.maxCZ(); cz++) { + if (!world.isChunkLoaded(cx, cz)) continue; + long key = key(cx, cz); + if (held.merge(key, 1, Integer::sum) == 1) world.addPluginChunkTicket(cx, cz, plugin); + keys[n++] = key; + } + } + return new Hold(world.getUID(), keys, n); + } + + private static long key(int cx, int cz) { + return ((long) cx << 32) | (cz & 0xFFFFFFFFL); + } + + /** The chunks of one photo; {@link #release} is idempotent. */ + public final class Hold { + private final UUID world; + private final long[] keys; + private final int size; + private boolean released; + + private Hold(UUID world, long[] keys, int size) { + this.world = world; + this.keys = keys; + this.size = size; + } + + public void release() { + if (released) return; + released = true; + Map held = counts.get(world); + if (held == null) return; + World w = Bukkit.getWorld(world); // null once unloaded: its tickets are gone with it + for (int i = 0; i < size; i++) { + long key = keys[i]; + Integer left = held.computeIfPresent(key, (k, c) -> c > 1 ? c - 1 : null); + if (left == null && w != null) w.removePluginChunkTicket((int) (key >> 32), (int) key, plugin); + } + if (held.isEmpty()) counts.remove(world); + } + } +} diff --git a/src/main/java/eu/mhsl/minecraft/pixelpics/render/RenderManager.java b/src/main/java/eu/mhsl/minecraft/pixelpics/render/RenderManager.java index 09e94e2..29f9f57 100644 --- a/src/main/java/eu/mhsl/minecraft/pixelpics/render/RenderManager.java +++ b/src/main/java/eu/mhsl/minecraft/pixelpics/render/RenderManager.java @@ -63,6 +63,7 @@ public final class RenderManager { private final ConcurrentLinkedQueue mainTasks = new ConcurrentLinkedQueue<>(); private final BukkitTask mainTaskPump; + private final ChunkHolds chunkHolds; private volatile boolean shuttingDown = false; private final AtomicInteger inFlight = new AtomicInteger(); private final int capacity; @@ -87,6 +88,12 @@ public final class RenderManager { return thread; }); this.mainTaskPump = Bukkit.getScheduler().runTaskTimer(plugin, this::runNextMainTask, 1, 1); + this.chunkHolds = new ChunkHolds(plugin); + } + + /** Keeps the chunks of queued photos loaded until their snapshot (main thread). */ + public ChunkHolds chunkHolds() { + return chunkHolds; } /** The bounded, low-priority pool the ray tracer must run its parallel work on. */ diff --git a/src/main/java/eu/mhsl/minecraft/pixelpics/render/render/DefaultScreenRenderer.java b/src/main/java/eu/mhsl/minecraft/pixelpics/render/render/DefaultScreenRenderer.java index 87c7e9b..0dbbadd 100644 --- a/src/main/java/eu/mhsl/minecraft/pixelpics/render/render/DefaultScreenRenderer.java +++ b/src/main/java/eu/mhsl/minecraft/pixelpics/render/render/DefaultScreenRenderer.java @@ -113,13 +113,8 @@ public class DefaultScreenRenderer { int superH = resolution.height() * SSAA; World world = eyeLocation.getWorld(); // Determined once, here: the FOV needs it before the snapshot exists, and execute's fog uses the same value. - int cameraFluid = FluidRenderer.cameraFluid( - world::getBlockData, eyeLocation.getX(), eyeLocation.getY(), eyeLocation.getZ()); - RayMap rayMap = RayMap.of( - eyeLocation, - superW, - superH, - cameraFluid != FluidRenderer.NONE ? V_FOV_DEG * FLUID_FOV_SCALE : V_FOV_DEG); + int cameraFluid = cameraFluid(eyeLocation); + RayMap rayMap = rayMap(eyeLocation, superW, superH, cameraFluid); FrustumBounds bounds = FrustumBounds.of(eyeLocation, rayMap, MAX_DISTANCE); WorldSnapshot snapshot = SnapshotBuilder.build(world, eyeLocation, bounds, logger); Collection nearby = bounds.nearbyEntities(world); @@ -148,6 +143,24 @@ public class DefaultScreenRenderer { SpecialDates.now()); } + /** + * The chunks {@link #prepare} will capture for this view (with a block of slack: computed from one ray per output + * pixel instead of the supersampled ones). Main thread. + */ + public SnapshotBuilder.ChunkArea chunkArea(Location eyeLocation, Resolution resolution) { + RayMap rayMap = rayMap(eyeLocation, resolution.width(), resolution.height(), cameraFluid(eyeLocation)); + return SnapshotBuilder.chunkArea(eyeLocation, FrustumBounds.of(eyeLocation, rayMap, MAX_DISTANCE), 2); + } + + private static int cameraFluid(Location eye) { + return FluidRenderer.cameraFluid(eye.getWorld()::getBlockData, eye.getX(), eye.getY(), eye.getZ()); + } + + private static RayMap rayMap(Location eye, int width, int height, int cameraFluid) { + return RayMap.of( + eye, width, height, cameraFluid != FluidRenderer.NONE ? V_FOV_DEG * FLUID_FOV_SCALE : V_FOV_DEG); + } + /** Traces every (super)ray in parallel, then downsamples gamma-correctly. Safe off the main thread. */ public BufferedImage execute(RenderJob job) { return execute(job, new AtomicBoolean(false)); diff --git a/src/main/java/eu/mhsl/minecraft/pixelpics/render/snapshot/SnapshotBuilder.java b/src/main/java/eu/mhsl/minecraft/pixelpics/render/snapshot/SnapshotBuilder.java index 3b5084d..410e062 100644 --- a/src/main/java/eu/mhsl/minecraft/pixelpics/render/snapshot/SnapshotBuilder.java +++ b/src/main/java/eu/mhsl/minecraft/pixelpics/render/snapshot/SnapshotBuilder.java @@ -34,15 +34,12 @@ public final class SnapshotBuilder { int minY = world.getMinHeight(); int maxY = world.getMaxHeight(); - int minCX = (int) Math.floor(Math.min(bounds.minX, eye.getX() - CAMERA_MARGIN)) >> 4; - int maxCX = (int) Math.floor(Math.max(bounds.maxX, eye.getX() + CAMERA_MARGIN)) >> 4; - int minCZ = (int) Math.floor(Math.min(bounds.minZ, eye.getZ() - CAMERA_MARGIN)) >> 4; - int maxCZ = (int) Math.floor(Math.max(bounds.maxZ, eye.getZ() + CAMERA_MARGIN)) >> 4; + ChunkArea area = chunkArea(eye, bounds, 0); Map chunks = new HashMap<>(); int skipped = 0; - for (int cx = minCX; cx <= maxCX; cx++) { - for (int cz = minCZ; cz <= maxCZ; cz++) { + for (int cx = area.minCX(); cx <= area.maxCX(); cx++) { + for (int cz = area.minCZ(); cz <= area.maxCZ(); cz++) { if (chunks.size() >= MAX_CHUNKS || !world.isChunkLoaded(cx, cz)) { skipped++; continue; @@ -57,4 +54,17 @@ public final class SnapshotBuilder { } return new WorldSnapshot(chunks, minY, maxY, Material.AIR.createBlockData()); } + + /** Chunk coordinates (inclusive) of a capture area. */ + public record ChunkArea(int minCX, int maxCX, int minCZ, int maxCZ) {} + + /** The chunks {@link #build} captures for these bounds, widened by {@code margin} blocks on every side. */ + public static ChunkArea chunkArea(Location eye, FrustumBounds bounds, double margin) { + double r = CAMERA_MARGIN + margin; + return new ChunkArea( + (int) Math.floor(Math.min(bounds.minX - margin, eye.getX() - r)) >> 4, + (int) Math.floor(Math.max(bounds.maxX + margin, eye.getX() + r)) >> 4, + (int) Math.floor(Math.min(bounds.minZ - margin, eye.getZ() - r)) >> 4, + (int) Math.floor(Math.max(bounds.maxZ + margin, eye.getZ() + r)) >> 4); + } } diff --git a/src/main/java/eu/mhsl/minecraft/pixelpics/survival/PhotoService.java b/src/main/java/eu/mhsl/minecraft/pixelpics/survival/PhotoService.java index 128c3f0..635bce0 100644 --- a/src/main/java/eu/mhsl/minecraft/pixelpics/survival/PhotoService.java +++ b/src/main/java/eu/mhsl/minecraft/pixelpics/survival/PhotoService.java @@ -1,6 +1,7 @@ package eu.mhsl.minecraft.pixelpics.survival; import eu.mhsl.minecraft.pixelpics.Main; +import eu.mhsl.minecraft.pixelpics.render.ChunkHolds; import eu.mhsl.minecraft.pixelpics.render.RenderManager; import eu.mhsl.minecraft.pixelpics.render.render.DefaultScreenRenderer; import eu.mhsl.minecraft.pixelpics.render.render.RenderJob; @@ -65,11 +66,15 @@ public final class PhotoService { } boolean dispatched = false; + ChunkHolds.Hold hold = null; try { // The view is fixed now; the world snapshot is taken (on the main thread) only when the render starts, so - // photos waiting in the queue hold no snapshot. + // photos waiting in the queue hold no snapshot. Its chunks stay loaded until then, also if the + // photographer leaves. Location eye = player.getEyeLocation(); Resolution resolution = new Resolution(MapImageDither.SIZE, MapImageDither.SIZE); + hold = manager.chunkHolds().hold(eye.getWorld(), renderer.chunkArea(eye, resolution)); + ChunkHolds.Hold chunks = hold; // The map is handed over right away and shows blank film until the picture develops. The blank film is // also stored, so the map never turns into a vanilla world map if the render is lost (crash). @@ -97,13 +102,22 @@ public final class PhotoService { manager.dispatch( user, cancelled -> { - RenderJob job = manager.onMainThread(() -> renderer.prepare(eye, resolution, user), cancelled); + RenderJob job = manager.onMainThread( + () -> { + try { + return renderer.prepare(eye, resolution, user); + } finally { + chunks.release(); + } + }, + cancelled); if (job == null) return null; BufferedImage image = renderer.execute(job, cancelled); if (cancelled.get()) return null; return new RenderOutput(image, MapImageDither.dither(image)); }, out -> { + chunks.release(); // already released by prepare MapManager.saveImage(out.image(), id); MapManager.saveIndices(out.indices(), id); plugin.getPendingPhotos().remove(id); @@ -113,10 +127,16 @@ public final class PhotoService { online.playSound(online.getLocation(), Sound.BLOCK_AMETHYST_BLOCK_CHIME, 0.8f, 1.2f); } }, - () -> failed(id, user, usesFilm)); + () -> { + chunks.release(); // the job may have ended before its snapshot + failed(id, user, usesFilm); + }); dispatched = true; } finally { - if (!dispatched) manager.release(user); + if (!dispatched) { + manager.release(user); + if (hold != null) hold.release(); + } } return true;