From e0031916912f696629a08ee27baed23e55f2dc42 Mon Sep 17 00:00:00 2001
From: RowDaBoat <reardenr@gmail.com>
Date: Tue, 22 Sep 2026 13:45:01 -0300
Subject: [PATCH] Keep live atlas builder from packing over existing sprites

markRegion replaced the skyline over the marked columns, so marking a
shorter entry after a taller one lowered it and new images were packed
on top of existing sprites. It now only raises the skyline and clamps
the region to the atlas width.

Fixes #70.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/silky/allocator.nim  | 42 ++++++++++++++++++++++++++++++++++++++--
 tests/test_atlas_png.nim | 41 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/src/silky/allocator.nim b/src/silky/allocator.nim
index 63b8c40..3a769ae 100644
--- a/src/silky/allocator.nim
+++ b/src/silky/allocator.nim
@@ -117,9 +117,47 @@ proc addToSkyline(allocator: SkylineAllocator, x, y, width, height: int) =
 
 proc markRegion*(allocator: SkylineAllocator, x, y, width, height: int) =
   ## Mark an already-drawn rectangle as used (no margin padding).
-  if width <= 0 or height <= 0:
+  ## Only raises the skyline, so marking order does not matter.
+  let
+    left = max(x, 0)
+    right = min(x + width, allocator.atlasSize)
+    top = y + height
+  if right <= left or height <= 0:
     return
-  allocator.addToSkyline(x, y, width, height)
+
+  var newSkyline: seq[SkylineNode]
+  for node in allocator.skyline:
+    let
+      nodeLeft = node.x
+      nodeRight = node.x + node.width
+    if nodeRight <= left or nodeLeft >= right:
+      newSkyline.add(node)
+      continue
+    # Part of the node before the region.
+    if nodeLeft < left:
+      newSkyline.add(SkylineNode(x: nodeLeft, y: node.y, width: left - nodeLeft))
+    # Part of the node inside the region, raised to the region top.
+    let
+      insideLeft = max(nodeLeft, left)
+      insideRight = min(nodeRight, right)
+    newSkyline.add(SkylineNode(
+      x: insideLeft,
+      y: max(node.y, top),
+      width: insideRight - insideLeft
+    ))
+    # Part of the node after the region.
+    if nodeRight > right:
+      newSkyline.add(SkylineNode(x: right, y: node.y, width: nodeRight - right))
+
+  # Merge adjacent nodes with same height.
+  var mergedSkyline: seq[SkylineNode]
+  for node in newSkyline:
+    if mergedSkyline.len > 0 and mergedSkyline[^1].y == node.y:
+      mergedSkyline[^1].width += node.width
+    else:
+      mergedSkyline.add(node)
+
+  allocator.skyline = mergedSkyline
 
 proc allocate*(allocator: SkylineAllocator, width, height: int): AllocationResult =
   ## Allocate a rectangle using skyline algorithm with margin.
diff --git a/tests/test_atlas_png.nim b/tests/test_atlas_png.nim
index b3cb45a..045b08f 100644
--- a/tests/test_atlas_png.nim
+++ b/tests/test_atlas_png.nim
@@ -3,7 +3,7 @@
 import
   std/[math, os, tables],
   pixie,
-  silky/atlas
+  silky/[allocator, atlas]
 
 proc assertRaisesMissingChunk(path: string) =
   ## Asserts that reading metadata fails when chunk is missing.
@@ -64,4 +64,43 @@ block:
   doAssert entry.boundsHeight.classify in Finite, $entry.boundsHeight
   doAssert entry.advance > 0, $entry.advance
 
+proc overlaps(a, b: Entry): bool =
+  ## True when two atlas entries share any pixels.
+  a.x < b.x + b.width and b.x < a.x + a.width and
+    a.y < b.y + b.height and b.y < a.y + a.height
+
+block:
+  echo "Testing markRegion never lowers the skyline"
+  let allocator = newSkylineAllocator(64, 0)
+  allocator.markRegion(0, 0, 64, 32)
+  allocator.markRegion(0, 0, 64, 8)
+  let allocation = allocator.allocate(8, 8)
+  doAssert allocation.success
+  doAssert allocation.y >= 32, $allocation
+
+block:
+  echo "Testing markRegion raises only the covered columns"
+  let allocator = newSkylineAllocator(64, 0)
+  allocator.markRegion(0, 0, 32, 16)
+  allocator.markRegion(16, 0, 32, 8)
+  # Columns 0..47 are covered up to y 16 or 8, columns 48..63 are free.
+  let free = allocator.allocate(16, 16)
+  doAssert free.success
+  doAssert free.x == 48 and free.y == 0, $free
+  let wide = allocator.allocate(48, 4)
+  doAssert wide.success
+  doAssert wide.y >= 16, $wide
+
+block:
+  echo "Testing live builder does not pack over existing entries"
+  let builder = newAtlasBuilder(64, 1)
+  doAssert builder.addImage("a", newImage(60, 10))
+  doAssert builder.addImage("b", newImage(60, 10))
+  let live = newAtlasBuilderFromAtlas(builder.atlas, builder.atlasImage)
+  doAssert live.addImage("new", newImage(60, 10))
+  let added = live.atlas.entries["new"]
+  for name, entry in live.atlas.entries:
+    if name != "new":
+      doAssert not overlaps(added, entry), name & " " & $entry & " " & $added
+
 echo "All atlas PNG tests passed."