From 16318850d6d1765d4d2610252012cf42f3056734 Mon Sep 17 00:00:00 2001
From: silverweed <silverweed14@proton.me>
Date: Tue, 19 Nov 2024 16:24:05 +0100
Subject: [PATCH] fix other_root_obj list containing known sections

---
 src/lists.h       | 33 ++++++++++++++++++++++++++++++++
 src/render.cpp    | 48 ++++++++++++++++++++++++-----------------------
 src/rntuple.cpp   |  4 ++--
 src/rntuple.h     |  4 ++--
 src/rntviewer.cpp |  1 +
 src/tfile.cpp     | 36 +++++++++++++++--------------------
 6 files changed, 78 insertions(+), 48 deletions(-)
 create mode 100644 src/lists.h

diff --git a/src/lists.h b/src/lists.h
new file mode 100644
index 0000000..040b9a5
--- /dev/null
+++ b/src/lists.h
@@ -0,0 +1,33 @@
+#define push_to_sll(lhead, ltail, new) \
+  if ((ltail)) { \
+    assert((lhead)); \
+    (ltail)->next = (new); \
+  } else { \
+    assert(!(ltail)); \
+    (lhead) = (new); \
+  } \
+  (ltail) = (new)
+
+#define push_to_dll(lhead, ltail, new) \
+  if ((ltail)) { \
+    assert((lhead)); \
+    (ltail)->next = (new); \
+    (new)->prev = (ltail); \
+  } else { \
+    assert(!(ltail)); \
+    (lhead) = (new); \
+  } \
+  (ltail) = (new)
+
+#define pop_from_dll(lhead, ltail, popped) do { \
+  if ((popped)->prev) { \
+    (popped)->prev->next = (popped)->next; \
+  } else { \
+    (lhead) = (popped)->next; \
+  } \
+  if ((popped)->next) { \
+    (popped)->next->prev = (popped)->prev; \
+  } else { \
+    (ltail) = (popped)->prev; \
+  } \
+} while (0)
diff --git a/src/render.cpp b/src/render.cpp
index 40f5524..ed77bba 100644
--- a/src/render.cpp
+++ b/src/render.cpp
@@ -359,33 +359,35 @@ void update_and_render(Arena *arena, App_State &app, f32 delta_time_ms)
       ImGui::SameLine(); 
       if (ImGui::Button("TKey Header")) {} // TODO: jump to next key
 
-      ImGui::ColorEdit3("_Page Start", app.viewer.col_page_start, edit_flags);
-      ImGui::SameLine(); 
-      if (ImGui::Button("Page Start"))
-        app.viewer.latest_page = viewer_jump_to_page(app, 0);
+      if (app.rndata.n_pages) {
+        ImGui::ColorEdit3("_Page Start", app.viewer.col_page_start, edit_flags);
+        ImGui::SameLine(); 
+        if (ImGui::Button("Page Start"))
+          app.viewer.latest_page = viewer_jump_to_page(app, 0);
 
-      ImGui::ColorEdit3("_Page", app.viewer.col_section[Sec_Page], edit_flags);
-      ImGui::SameLine();
-      if (ImGui::Button("Page"))
-        app.viewer.latest_page = viewer_jump_to_page(app, app.viewer.latest_page_gone_to);
-      ImGui::SameLine();
-      {
-        const i64 step_fast_i64 = app.rndata.n_pages / 100;
-        i64 page_to_go_to = app.viewer.latest_page_gone_to;
-        ImGui::PushItemWidth(100.f);
-        if (ImGui::InputScalar("##page_viewed", ImGuiDataType_S64, &page_to_go_to, &step_i64, &step_fast_i64, "%u") && 
-            ImGui::IsItemDeactivatedAfterEdit())
+        ImGui::ColorEdit3("_Page", app.viewer.col_section[Sec_Page], edit_flags);
+        ImGui::SameLine();
+        if (ImGui::Button("Page"))
+          app.viewer.latest_page = viewer_jump_to_page(app, app.viewer.latest_page_gone_to);
+        ImGui::SameLine();
         {
-          app.viewer.latest_page = viewer_jump_to_page(app, page_to_go_to);
+          const i64 step_fast_i64 = app.rndata.n_pages / 100;
+          i64 page_to_go_to = app.viewer.latest_page_gone_to;
+          ImGui::PushItemWidth(100.f);
+          if (ImGui::InputScalar("##page_viewed", ImGuiDataType_S64, &page_to_go_to, &step_i64, &step_fast_i64, "%u") && 
+              ImGui::IsItemDeactivatedAfterEdit())
+          {
+            app.viewer.latest_page = viewer_jump_to_page(app, page_to_go_to);
+          }
+          ImGui::PopItemWidth();
         }
-        ImGui::PopItemWidth();
+        ImGui::SameLine();
+        String8 this_page_width = app.viewer.latest_page 
+                                  ? push_str8f(scratch.arena, " (this page: %s)", 
+                                               to_pretty_size(scratch.arena, app.viewer.latest_page->range.len).c()) 
+                                  : str8("");
+        ImGui::Text("%s%s", to_pretty_size(scratch.arena, app.rndata.tot_page_comp_size).c(), this_page_width.c());
       }
-      ImGui::SameLine();
-      String8 this_page_width = app.viewer.latest_page 
-                                ? push_str8f(scratch.arena, " (this page: %s)", 
-                                             to_pretty_size(scratch.arena, app.viewer.latest_page->range.len).c()) 
-                                : str8("");
-      ImGui::Text("%s%s", to_pretty_size(scratch.arena, app.rndata.tot_page_comp_size).c(), this_page_width.c());
 
       ImGui::ColorEdit3("_Checksum", app.viewer.col_checksum, edit_flags);
       ImGui::SameLine(); 
diff --git a/src/rntuple.cpp b/src/rntuple.cpp
index ee241cc..f6d6839 100644
--- a/src/rntuple.cpp
+++ b/src/rntuple.cpp
@@ -452,7 +452,7 @@ Section find_section(App_State &app, u64 off, i64 hilite_cluster = -1)
   const RNTuple_Data &rdata = app.rndata;
   const TKeys_Data &tdata = app.tfile_data.tkeys_data;
 
-  for (u32 i = 1; i < Sec_First_Non_Unique; ++i) {   
+  for (u32 i = 1; i < Sec_COUNT_Non_Unique; ++i) {   
     const Section &sec = tdata.sections[i];
     if (sec.range.start - sec.pre_size <= off && off < sec.range.end()) {
       return sec;
@@ -536,7 +536,7 @@ Section find_section(App_State &app, u64 off, i64 hilite_cluster = -1)
   }
 
   // Copypasted from the fast page lookup
-  if (tdata.other_root_obj) {
+  if (tdata.n_other_root_obj > 0) {
     // fast case: `off` is in the same page info as previous `off`.
     for (i32 i = 0; i < 2; ++i) {
       const Section &other_sec = app.last_other_root_obj->section;
diff --git a/src/rntuple.h b/src/rntuple.h
index bd51773..0f1204d 100644
--- a/src/rntuple.h
+++ b/src/rntuple.h
@@ -66,8 +66,8 @@ enum Section_Id {
   Sec_RNTuple_Footer,
   Sec_Page_List,
 
-  Sec_First_Non_Unique,
-  Sec_Page = Sec_First_Non_Unique, 
+  Sec_COUNT_Non_Unique,
+  Sec_Page = Sec_COUNT_Non_Unique, 
   // any other object stored in a TKey
   Sec_Other,
 
diff --git a/src/rntviewer.cpp b/src/rntviewer.cpp
index 45b87f3..70106e5 100644
--- a/src/rntviewer.cpp
+++ b/src/rntviewer.cpp
@@ -66,6 +66,7 @@ using ROOT::Experimental::Internal::RNTupleSerializer;
 // Internal headers
 // --------------------------------------------------
 #include "types.h"
+#include "lists.h"
 #include "defer.h"
 #include "prof.h"
 #include "mem.h"
diff --git a/src/tfile.cpp b/src/tfile.cpp
index d4d7de1..9622601 100644
--- a/src/tfile.cpp
+++ b/src/tfile.cpp
@@ -204,7 +204,7 @@ b8 walk_tkeys(Arena *arena, const u8 *data, u64 data_len, u32 flags, TFile_Data
       ++n_keys;
     }
 
-    if (cur == tkeys_data.sections[Sec_TKey_List].range.start) {
+    if (cur == tkeys_data.sections[Sec_TKey_List].range.start - tkeys_data.sections[Sec_TKey_List].pre_size) {
       if (keylen != tkeys_data.sections[Sec_TKey_List].pre_size ||
           (u64)(n_bytes - keylen) != tkeys_data.sections[Sec_TKey_List].range.len) 
       {
@@ -217,6 +217,10 @@ b8 walk_tkeys(Arena *arena, const u8 *data, u64 data_len, u32 flags, TFile_Data
       tkeys_data.sections[Sec_TFile_Info].pre_size = keylen;
       tkeys_data.sections[Sec_TFile_Info].range.start += keylen;
       tkeys_data.sections[Sec_TFile_Info].range.len = n_bytes - keylen;
+    } else if (cur == tkeys_data.sections[Sec_TFile_FreeList].range.start) {
+      tkeys_data.sections[Sec_TFile_FreeList].pre_size = keylen;
+      tkeys_data.sections[Sec_TFile_FreeList].range.start += keylen;
+      tkeys_data.sections[Sec_TFile_FreeList].range.len = n_bytes - keylen;
     } else {
       // Check if this is a RNTuple anchor
       u64 key_header_size = (key_version >= 1000) ? 18 + 16 : 18 + 8;
@@ -271,27 +275,17 @@ b8 walk_tkeys(Arena *arena, const u8 *data, u64 data_len, u32 flags, TFile_Data
         rblob_key->next = tkeys_data.rblob_keys;
         tkeys_data.rblob_keys = rblob_key;
 
-      } else if ((flags & WTK_COLLECT_OTHER_ROOT_OBJS) && 
-                 !key_has_class_name("TFile")) // @Cleanup: this is here to avoid adding stuff like the tkeys list
-                                               // or the free list to the "other root obj" list. We might instead want
-                                               // to do a later pass to remove all objects that are in a "well-known"
-                                               // section from that list.
-      {
-        Other_Root_Obj_Info *binfo = arena_push<Other_Root_Obj_Info>(arena);
+      } else if (flags & WTK_COLLECT_OTHER_ROOT_OBJS) {
+        Other_Root_Obj_Info *oth_info = arena_push<Other_Root_Obj_Info>(arena);
         if (cname_len) {
-          binfo->class_name = str8_from_buf(arena, data + cname_off + 1, cname_len);
+          oth_info->class_name = str8_from_buf(arena, data + cname_off + 1, cname_len);
         }
-        binfo->section.id = Sec_Other;
-        binfo->section.info = binfo;
-        binfo->section.range.start = cur + keylen;
-        binfo->section.range.len = n_bytes - keylen;
-        binfo->section.pre_size = keylen;
-        if (tkeys_data.other_root_obj) {
-          other_root_obj_tail->next = binfo;
-        } else {
-          tkeys_data.other_root_obj = binfo;
-        }
-        other_root_obj_tail = binfo;
+        oth_info->section.id = Sec_Other;
+        oth_info->section.info = oth_info;
+        oth_info->section.range.start = cur + keylen;
+        oth_info->section.range.len = n_bytes - keylen;
+        oth_info->section.pre_size = keylen;
+        push_to_sll(tkeys_data.other_root_obj, other_root_obj_tail, oth_info);
         tkeys_data.n_other_root_obj++;
       }
     }
@@ -301,7 +295,7 @@ b8 walk_tkeys(Arena *arena, const u8 *data, u64 data_len, u32 flags, TFile_Data
   if (flags & WTK_PRINT_KEYS_INFO)
     printf("Found %u TKeys.\n", n_keys);
 
-  for (u32 id = 1; id < Sec_COUNT; ++id)
+  for (u32 id = 1; id < Sec_COUNT_Non_Unique; ++id)
     tkeys_data.sections[id].id = (Section_Id)id;
 
   // adjust header section padding