From 96b4ff39a6eb3cfd3d74d8d6dee640830d48c90b Mon Sep 17 00:00:00 2001
From: Charly Delay <0@0xcharly.com>
Date: Sat, 19 Oct 2024 14:08:43 +0900
Subject: [PATCH] Tentative fix for unexpected `font-codepoint-map` behavior
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In particular when configured to replace several ranges with multiple fonts.

Given the following `font-codepoint-map` config:

```
font-codepoint-map=U+0030-U+0039=Monaco       # 0-9
font-codepoint-map=U+0040=mononoki            # @
font-codepoint-map=U+0041-U+005a=Pixel Code   # A-Z
font-codepoint-map=U+0061-U+007a=Victor Mono  # a-z
```

I noticed a couple of unexpected behavior:

1. Codepoint ranges were assigned the wrong font
2. The declaration order had a direct impact on the font assignment
   (seemed to be rotating in some fashion)

If my understanding of the current implementation is correct, for a
given range index `n` in the `MultiArrayList` `CodepointMap.get(…)`
returns the font descriptor at index `len - n - 1`. In other words, it
returns the descriptor symmetrically opposite relative to the middle of
the list.

I've added a couple test cases that I would expect to pass if my
understanding of the expected behavior is correct, verified that they
were broken under the current behavior, and updated the implementation
of `CodepointMap.get(…)` accordingly.

My understanding of the original intent is to give priority to the
latest range match in the list (which is a use case already tested by
the `codepointmap` test, but which I believe happened to pass "by
accident"), so I opted for a reverse traversal of the codepoint list.
---
 src/font/CodepointMap.zig | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/font/CodepointMap.zig b/src/font/CodepointMap.zig
index 8c9ded402..5b174f129 100644
--- a/src/font/CodepointMap.zig
+++ b/src/font/CodepointMap.zig
@@ -54,8 +54,9 @@ pub fn add(self: *CodepointMap, alloc: Allocator, entry: Entry) !void {
 /// Get a descriptor for a codepoint.
 pub fn get(self: *const CodepointMap, cp: u21) ?discovery.Descriptor {
     const items = self.list.items(.range);
-    for (items, 0..) |range, forward_i| {
+    for (0..items.len) |forward_i| {
         const i = items.len - forward_i - 1;
+        const range = items[i];
         if (range[0] <= cp and cp <= range[1]) {
             const descs = self.list.items(.descriptor);
             return descs[i];
@@ -110,4 +111,15 @@ test "codepointmap" {
     // Non-matching
     try testing.expect(m.get(0) == null);
     try testing.expect(m.get(3) == null);
+
+    try m.add(alloc, .{ .range = .{ 3, 4 }, .descriptor = .{ .family = "C" } });
+    try m.add(alloc, .{ .range = .{ 5, 6 }, .descriptor = .{ .family = "D" } });
+    {
+        const d = m.get(3).?;
+        try testing.expectEqualStrings("C", d.family.?);
+    }
+    {
+        const d = m.get(1).?;
+        try testing.expectEqualStrings("B", d.family.?);
+    }
 }