Fix the native items from the review

native/src/deview.cpp

@@ -19,6 +19,7 @@
 
 #include <algorithm>
 #include <cstdint>
+#include <cstdio>
 #include <cstring>
 #include <filesystem>
 #include <map>
@@ -101,6 +102,9 @@ struct CachedTexture
     bool loaded = false;
     std::uintmax_t length = 0;
     std::filesystem::file_time_type written{};
+
+    /* Whether the frame being built asked for this picture. What ForgetUnusedPictures keeps. */
+    bool used = false;
 };
 
 struct State
@@ -337,6 +341,7 @@ const Texture2D* Picture(const std::string& path)
         if (found->second.written == written &&
             found->second.length == length)
         {
+            found->second.used = true;
             return found->second.loaded ? &found->second.texture : nullptr;
         }
 
@@ -346,6 +351,7 @@ const Texture2D* Picture(const std::string& path)
     CachedTexture entry;
     entry.written = written;
     entry.length = length;
+    entry.used = true;
     const Texture2D texture = LoadTexture(path.c_str());
     if (IsTextureValid(texture))
     {
@@ -360,6 +366,34 @@ const Texture2D* Picture(const std::string& path)
     return inserted->second.loaded ? &inserted->second.texture : nullptr;
 }
 
+/*
+ * Drops every picture the frame just drawn did not ask for, once the frame has been rendered and
+ * the draw data naming those textures has been consumed.
+ *
+ * Otherwise an entry went only when its own path was asked for again and had changed or gone, so
+ * every image reviewed in a session stayed decoded, on the GPU, until the session ended. A picture
+ * scrolled or navigated back to is decoded again, which is one file read.
+ */
+void ForgetUnusedPictures()
+{
+    for (auto entry = state.pictures.begin(); entry != state.pictures.end();)
+    {
+        if (entry->second.used)
+        {
+            entry->second.used = false;
+            ++entry;
+            continue;
+        }
+
+        if (entry->second.loaded)
+        {
+            UnloadTexture(entry->second.texture);
+        }
+
+        entry = state.pictures.erase(entry);
+    }
+}
+
 void UnloadPictures()
 {
     for (auto& entry : state.pictures)
@@ -467,7 +501,10 @@ void RenderDrawData(ImDrawData* drawData)
     rlDrawRenderBatchActive();
     rlDisableBackfaceCulling();
 
-    const float height = static_cast<float>(GetScreenHeight());
+    /* The height of what is being drawn to, which is not the window's for a capture: that draws to
+     * a render texture of its own size, and BeginTextureMode changes the target without changing
+     * what GetScreenHeight reports. */
+    const float height = drawData->DisplaySize.y;
     for (int list = 0; list < drawData->CmdListsCount; list++)
     {
         const ImDrawList* commands = drawData->CmdLists[list];
@@ -606,8 +643,9 @@ struct PaneHit
     /* The left edge of the column, which is where the gutter starts. */
     float cellLeft = -1.0f;
 
-    /* Where the row text starts, past that gutter, read from the first row that draws any. Stays
-     * -1 for a pane of nothing but filler, which has nothing to select either. */
+    /* Where the row text starts, past that gutter, read from the first row that draws any, and
+     * true of every row because GutterDigits gives them all one width. Stays -1 for a pane of
+     * nothing but filler, which has nothing to select either. */
     float textLeft = -1.0f;
 
     /* The top of row zero and the pitch between rows, read from the first two rows the way
@@ -616,7 +654,38 @@ struct PaneHit
     float pitch = 0.0f;
 };
 
-void DrawRow(const DeviewScreen* screen, const DeviewPane& pane, int index, int column, PaneHit& hit)
+/*
+ * How many digits the line numbers of this frame take: four, the width every other renderer
+ * gives them, or more when a row drawn in either pane needs it.
+ *
+ * One width for every row of both panes, which is what lets the text start read from the first
+ * row stand for all of them. Formatted per row, a five digit number pushed its own row's text a
+ * cell right of the rows above it, and a drag across them selected a cell off.
+ */
+int GutterDigits(const DeviewScreen* screen)
+{
+    int digits = 4;
+    for (int side = 0; side < 2 && side < screen->paneCount; side++)
+    {
+        const DeviewPane& pane = screen->panes[side];
+        for (int index = 0; index < pane.rowCount; index++)
+        {
+            int32_t number = screen->rows[pane.rowOffset + index].lineNumber;
+            int length = 1;
+            while (number >= 10)
+            {
+                number /= 10;
+                length++;
+            }
+
+            digits = std::max(digits, length);
+        }
+    }
+
+    return digits;
+}
+
+void DrawRow(const DeviewScreen* screen, const DeviewPane& pane, int index, int column, int digits, PaneHit& hit)
 {
     /* Before the row count check, so a pane shorter than the body still reports where its rows
      * begin and how far apart they are. */
@@ -652,11 +721,11 @@ void DrawRow(const DeviewScreen* screen, const DeviewPane& pane, int index, int
     ImGui::PushStyleColor(ImGuiCol_Text, IM_COL32(130, 130, 130, 255));
     if (row.lineNumber >= 0)
     {
-        ImGui::Text("%c %4d", RowMarker(row.kind), row.lineNumber);
+        ImGui::Text("%c %*d", RowMarker(row.kind), digits, row.lineNumber);
     }
     else
     {
-        ImGui::Text("%c     ", RowMarker(row.kind));
+        ImGui::Text("%c %*s", RowMarker(row.kind), digits, "");
     }
 
     ImGui::PopStyleColor();
@@ -1001,6 +1070,8 @@ void BuildFrame(const DeviewScreen* screen)
     PaneImage leftImage;
     PaneImage rightImage;
 
+    const int digits = GutterDigits(screen);
+
     /* Filled by the same pass that draws the rows, and read after it by UpdateSelection. */
     PaneHit leftHit;
     PaneHit rightHit;
@@ -1011,7 +1082,11 @@ void BuildFrame(const DeviewScreen* screen)
         const DeviewPane& right = screen->panes[1];
         if (hasQueue)
         {
-            ImGui::TableSetupColumn("Pending", ImGuiTableColumnFlags_WidthFixed, queueWidth);
+            /* The count every other renderer puts in this header, with the column's id kept apart
+             * from it so a count that changes is still the same column. */
+            char pending[48];
+            std::snprintf(pending, sizeof pending, "Pending (%d)###Pending", screen->pendingCount);
+            ImGui::TableSetupColumn(pending, ImGuiTableColumnFlags_WidthFixed, queueWidth);
         }
 
         ImGui::TableSetupColumn(Copy(screen, left.headerOffset, left.headerLength).c_str());
@@ -1111,10 +1186,10 @@ void BuildFrame(const DeviewScreen* screen)
             }
 
             RecordPaneImage(leftImage, left, index);
-            DrawRow(screen, left, index, column, leftHit);
+            DrawRow(screen, left, index, column, digits, leftHit);
             ImGui::TableSetColumnIndex(column + 1);
             RecordPaneImage(rightImage, right, index);
-            DrawRow(screen, right, index, column + 1, rightHit);
+            DrawRow(screen, right, index, column + 1, digits, rightHit);
         }
 
         ImGui::EndTable();
@@ -1428,6 +1503,7 @@ int32_t deview_present(const DeviewScreen* screen)
     ClearBackground(Color{24, 24, 24, 255});
     RenderDrawData(ImGui::GetDrawData());
     EndDrawing();
+    ForgetUnusedPictures();
 
     MeasureGrid();
     return 1;
@@ -1511,6 +1587,7 @@ int32_t deview_capture(const DeviewScreen* screen, int32_t width, int32_t height
     ClearBackground(Color{24, 24, 24, 255});
     RenderDrawData(ImGui::GetDrawData());
     EndTextureMode();
+    ForgetUnusedPictures();
 
     Image image = LoadImageFromTexture(target.texture);
     /* Render textures come back bottom up. */

native/swift/Sources/Deview/Exports.swift

@@ -8,6 +8,13 @@ import ImageIO
 ///
 /// The header is imported for its struct layouts only, with DEVIEW_TYPES_ONLY, so these are the
 /// definitions of those symbols rather than a second declaration of them.
+///
+/// Every one that touches AppKit runs inside its own autorelease pool. `NSApplication.run` drains
+/// a pool per event, but this app never calls it: the managed loop calls in instead, and with no
+/// pool pushed objc4 parks everything autoreleased in one it creates for the thread, which drains
+/// only when the thread exits. That is the main thread, so everything a frame autoreleased - the
+/// events, the attributed strings, the drawing - stayed until the process did. GLFW, which also
+/// pumps by hand, does the same.
 
 @_cdecl("deview_version")
 public func deviewVersion() -> Int32 {
@@ -16,6 +23,19 @@ public func deviewVersion() -> Int32 {
 
 @_cdecl("deview_init")
 public func deviewInit(
+    _ width: Int32,
+    _ height: Int32,
+    _ title: UnsafePointer<CChar>?,
+    _ fontTtf: UnsafePointer<UInt8>?,
+    _ fontLength: Int32,
+    _ fontSize: Float,
+    _ hidden: Int32) -> Int32 {
+    autoreleasepool {
+        initialise(width, height, title, fontTtf, fontLength, fontSize, hidden)
+    }
+}
+
+private func initialise(
     _ width: Int32,
     _ height: Int32,
     _ title: UnsafePointer<CChar>?,
@@ -45,7 +65,10 @@ public func deviewPresent(_ screen: UnsafePointer<DeviewScreen>?) -> Int32 {
         return 0
     }
 
-    runtime.present(Frame.decode(screen))
+    autoreleasepool {
+        runtime.present(Frame.decode(screen))
+    }
+
     return 1
 }
 
@@ -56,8 +79,10 @@ public func deviewPollInput(_ input: UnsafeMutablePointer<DeviewInput>?) {
     }
 
     let runtime = Runtime.shared
-    if runtime.initialised {
-        runtime.measureGrid()
+    autoreleasepool {
+        if runtime.initialised {
+            runtime.measureGrid()
+        }
     }
 
     input.pointee = runtime.input
@@ -66,10 +91,12 @@ public func deviewPollInput(_ input: UnsafeMutablePointer<DeviewInput>?) {
 
 @_cdecl("deview_set_hidden")
 public func deviewSetHidden(_ hidden: Int32) {
-    if hidden == 0 {
-        Runtime.shared.show()
-    } else {
-        Runtime.shared.hide()
+    autoreleasepool {
+        if hidden == 0 {
+            Runtime.shared.show()
+        } else {
+            Runtime.shared.hide()
+        }
     }
 }
 
@@ -81,19 +108,25 @@ public func deviewSetClipboard(_ text: UnsafePointer<CChar>?) {
 
     // Cleared first: NSPasteboard keeps whatever types were declared before, so writing a string
     // over an image would otherwise leave both on the board and paste the wrong one.
-    let board = NSPasteboard.general
-    board.clearContents()
-    board.setString(String(cString: text), forType: .string)
+    autoreleasepool {
+        let board = NSPasteboard.general
+        board.clearContents()
+        board.setString(String(cString: text), forType: .string)
+    }
 }
 
 @_cdecl("deview_focus")
 public func deviewFocus() {
-    Runtime.shared.show()
+    autoreleasepool {
+        Runtime.shared.show()
+    }
 }
 
 @_cdecl("deview_shutdown")
 public func deviewShutdown() {
-    Runtime.shared.shutdown()
+    autoreleasepool {
+        Runtime.shared.shutdown()
+    }
 }
 
 /// Renders into a bitmap of this side's own making rather than asking the view for one.
@@ -105,6 +138,16 @@ public func deviewShutdown() {
 /// snapshot tests do not need a window server.
 @_cdecl("deview_capture")
 public func deviewCapture(
+    _ screen: UnsafePointer<DeviewScreen>?,
+    _ width: Int32,
+    _ height: Int32,
+    _ pngPath: UnsafePointer<CChar>?) -> Int32 {
+    autoreleasepool {
+        capture(screen, width, height, pngPath)
+    }
+}
+
+private func capture(
     _ screen: UnsafePointer<DeviewScreen>?,
     _ width: Int32,
     _ height: Int32,

native/swift/Sources/Deview/Renderer.swift

@@ -212,6 +212,11 @@ final class Renderer {
 
         let bodyBottom = bodyTop + CGFloat(capacity) * line
 
+        // Only the pictures this frame names stay decoded. An entry used to go only when its own
+        // path was asked for again and had changed or gone, so every image reviewed in a session
+        // was held until the session ended.
+        pictures = pictures.filter { $0.key == frame.left.imagePath || $0.key == frame.right.imagePath }
+
         // Under the rows rather than instead of them. The rows are what every head draws — format,
         // size and byte count, coloured against the other side — and this one can afford to also
         // show the thing they describe.

native/swift/Sources/Deview/Runtime.swift

@@ -11,6 +11,9 @@ final class Runtime {
     static let shared = Runtime()
 
     private var delegate: WindowDelegate?
+
+    /// Held here because `NSApplication.delegate` does not keep it alive.
+    private let applicationDelegate = ApplicationDelegate()
     private var size = CGSize(width: 1100, height: 700)
     private var title = "DiffEngineViewer"
 
@@ -80,6 +83,7 @@ final class Runtime {
 
         // Before finishLaunching, which is when the bar is first read.
         application.mainMenu = MainMenu.build(target)
+        application.delegate = applicationDelegate
         application.finishLaunching()
 
         let bounds = NSRect(origin: .zero, size: size)
@@ -309,3 +313,24 @@ final class Runtime {
         input.dragFocusColumn = 0
     }
 }
+
+/// Answers a quit that comes from outside the managed loop: Quit in the Dock, and logout.
+///
+/// Both arrive as `terminate:`, which does not return. With no delegate to ask, AppKit exits from
+/// inside it, so nothing after the pump call runs, the managed `finally` included. For a viewer
+/// that owns the queue, that `finally` is `PersistOwned`, and macOS has no tray to hold the queue
+/// instead, so the queue was lost.
+///
+/// The quit is refused and reported as a close, which the managed side answers the way it answers
+/// the window's close button: it leaves the loop, persists, and exits. That is what GLFW does.
+/// Not `.terminateLater`, whose reply AppKit waits for in a modal loop inside the pump, while the
+/// thread that would send the reply is the one blocked waiting for the pump to return.
+///
+/// A refused quit also cancels the logout that asked for it. By the time the logout is tried
+/// again the viewer has exited.
+final class ApplicationDelegate: NSObject, NSApplicationDelegate {
+    func applicationShouldTerminate(_ sender: NSApplication) -> NSApplication.TerminateReply {
+        Runtime.shared.input.closeRequested = 1
+        return .terminateCancel
+    }
+}