Repository: pygfx/pygfx
Pull request #1021 (2025-03-05 17:11:52)

fix two bugs in atlas allocator
1. The free list shouldn't be cleared. It refers to a pointer array on the GPU which can in fact be readily reused since each "chunk" is independent.
2. There are 0 sized glyphs in the world....

almarklein: I added some tests. Looked into what it actually means when an empty region is selected, because it looks like an oversight, but apparently this Just Works.

Patch:
diff --git a/pygfx/utils/text/_atlas.py b/pygfx/utils/text/_atlas.py
index bdd4fe0..1b7adc0 100644
--- a/pygfx/utils/text/_atlas.py
+++ b/pygfx/utils/text/_atlas.py
@@ -231,8 +231,13 @@ class GlyphAtlas(RectPacker):
             info = self._infos[index]
             x1, y1 = info["origin"]
             w1, h1 = info["size"]
-            if not w1 or not h1:
-                continue  # freed region
+            # An index can have 0 size, for example the
+            # character 32, a non-printable character.
+            # has a glyph that is empty, but it should still
+            # take space and have an index in our atlas.
+            if w1 == 0 and index in self._free_indices:
+                # freed region
+                continue
             x2, y2, w2, h2 = self._select_region(w1, h1)
             info["origin"] = x2, y2
             array2[y2 : y2 + h2, x2 : x2 + w2] = array1[y1 : y1 + h1, x1 : x1 + w1]
@@ -242,7 +247,6 @@ class GlyphAtlas(RectPacker):
 
         self._free_area = 0
         self._allocated_area = allocated_area
-        self._free_indices.clear()
 
     def allocate_region(self, w, h):
         """Allocate a region of the given size. Returns the index for


Test patch:
diff --git a/tests/utils/test_text_atlas.py b/tests/utils/test_text_atlas.py
index f175db5..3656f70 100644
--- a/tests/utils/test_text_atlas.py
+++ b/tests/utils/test_text_atlas.py
@@ -125,6 +125,7 @@ def test_atlas_alloc_resize_with_freeing():
     for _i in range(4):
         atlas.free_region(get_index_in_use())
     assert atlas.allocated_area == 5 * 64
+    assert len(atlas._free_indices) == 4
 
     # Allocate another
     atlas.allocate_region(8, 8)
@@ -134,6 +135,9 @@ def test_atlas_alloc_resize_with_freeing():
     assert atlas.region_count == 6
     assert atlas.allocated_area == 6 * 64
 
+    # The free indices (into the infos array) are still there
+    assert len(atlas._free_indices) == 3  # one it taken by the allocation
+
     # Not only is the atlas not resized, the array is also still the
     # same object. This is important, because this means that the
     # texture that wraps it also has not changed, which means that
@@ -150,6 +154,9 @@ def test_atlas_alloc_resize_with_freeing():
     assert atlas.allocated_area == 12 * 64
     assert atlas._array is not prev_array
 
+    # The free indices (into the infos array) are still there
+    assert len(atlas._free_indices) == 0  # because we used all the free region
+
     # Now we free some regions
     for _i in range(6):
         atlas.free_region(get_index_in_use())
@@ -167,6 +174,7 @@ def test_atlas_alloc_resize_with_freeing():
     assert atlas.region_count == 3
     assert atlas.total_area == 576
     assert atlas.allocated_area == 3 * 64
+    assert len(atlas._free_indices) == 9
 
     # Free all we have
     for _i in range(3):
@@ -224,6 +232,26 @@ def test_atlas_alloc1():
     assert len(indices) == len(set(indices))
 
 
+def test_atlas_alloc_empty():
+    # Allocate empty regions (zero size)
+    # This is not even such a special case; it just works.
+
+    atlas = GlyphAtlas(16, 16)
+
+    i1 = atlas.allocate_region(5, 5)
+    i2 = atlas.allocate_region(0, 0)
+    i3 = atlas.allocate_region(0, 0)
+    i4 = atlas.allocate_region(5, 5)
+
+    # Unique indices
+    assert len({i1, i2, i3, i4}) == 4
+
+    assert atlas.get_region(i1).size == 25
+    assert atlas.get_region(i2).size == 0
+    assert atlas.get_region(i3).size == 0
+    assert atlas.get_region(i4).size == 25
+
+
 def test_atlas_alloc2():
     # Now with de-allocations