fix(portals): skip redundant live crop (#190) + consolidate surface lifecycle (#192)

#190 — While the portal pop-out is popped out AND hosting the live confined
viewport, the docked preview is hidden and the pop-out shows the live
DocumentView, so the saved-tracking / auto-pin 300-DPI block crop has no
on-screen consumer. RenderPortalTarget and PinAutoReference now elide the
PDFium rasterise in that state (new PortalCropRedundant gate = popped-out &&
live), set _trackingCropSkipped, and still pin + aim the live viewport.
Docking re-materialises the current target via RerenderDisplayedCrop so the
re-revealed docked preview is never stale; _autoPin now carries the caption
block so the float+caption union crop can be reproduced without re-parsing.
A docked temporary peek is live but NOT popped out, so its visible docked
preview still renders normally. PortalsView's empty-state hint now keys off
DisplayedPortalId (a target is pinned) instead of the crop bitmap, which is
now null while elided.

#192 — Extracted BuildSecondarySurface / DisposeSecondarySurface in
MainWindow.Panes.cs as the single shared core (new DocumentView + Initialize +
BindViewport(ownsImages) + RegisterSurface / unregister + Teardown +
SafeRemoveViewport). Split panes, tear-off windows, and the live portal
viewport all route through them, so a future surface-lifecycle change lands in
one place. The portal path keeps its genuine differences (own viewport
creation/seating, not focused on create) at the call site.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: sjvrensburg

src/RailReader2/ViewModels/MainWindowViewModel.PortalViewport.cs

@@ -47,6 +47,14 @@ private set
     /// <summary>The "no active portal" hint shows only when not live and there is no crop to show.</summary>
     public bool ShowPortalHint => !_isPortalLive && PortalWindowImage is null;
 
+    /// <summary>True when the saved-tracking / auto-pin crop (<see cref="ActivePortalImage"/>) has no
+    /// on-screen consumer: the pop-out is popped out (the docked preview is hidden, showing the Dock
+    /// button) AND hosting the live confined viewport (the pop-out's static crop is hidden). Rasterising
+    /// the 300-DPI block crop is then pure waste (#190) — the render paths skip it and docking
+    /// re-materialises the current target. NB: a docked temporary peek IS live but NOT popped out, so the
+    /// visible docked preview still renders its saved-tracking crop.</summary>
+    private bool PortalCropRedundant => IsPortalPoppedOut && _isPortalLive;
+
     /// <summary>Raised when the pop-out's desired live target may have changed (a pin, peek, clear, or
     /// pop-out/dock). MainWindow re-syncs the hosted DocumentView (creating, re-aiming, or tearing it
     /// down). Raised only at genuine content-change points — never per animation frame — so the user can

src/RailReader2/ViewModels/MainWindowViewModel.Portals.cs

@@ -82,11 +82,20 @@ public bool ShouldShowPortalWindow
     partial void OnIsPortalPoppedOutChanged(bool value)
     {
         OnPropertyChanged(nameof(ShouldShowPortalWindow));
-        if (!value && _isPortalViewLocked)
+        if (value) return;
+
+        // Docking. A per-pop-out view lock is released and tracking catches up to the reading position;
+        // EvaluatePortals re-pins (and renders) if the position moved to a different portal while locked.
+        if (_isPortalViewLocked)
         {
             ReleasePortalLock();
             EvaluatePortals(forceRender: true);
         }
+        // The docked preview is visible again. While popped-out + live the tracking crop was skipped
+        // (#190) — re-materialise the current target now so the preview isn't blank/stale. A no-op (no
+        // re-rasterise) unless a crop was actually elided; if the unlock above already re-pinned to a
+        // different target it cleared the flag, so this won't double-render.
+        RerenderDisplayedCrop();
     }
 
     // The pop-out window's content (PortalWindowImage/Label/Hint) and visibility (ShouldShowPortalWindow)
@@ -108,6 +117,11 @@ private void RaisePortalWindowProps()
     // True when the active portal's target page was not yet analysed at render time, so a forced pass
     // (target page just analysed) should retry instead of being debounced away.
     private bool _portalTargetPending;
+    // True when the currently displayed target's tracking crop was deliberately NOT rasterised because
+    // it had no on-screen consumer (popped-out + live: PortalCropRedundant) — so ActivePortalImage does
+    // not reflect _displayedPortalId. Docking re-materialises it (RerenderDisplayedCrop, #190). Cleared
+    // whenever the crop is actually (re-)rendered or the displayed pin is cleared.
+    private bool _trackingCropSkipped;
     // Reading position (page, block, line) at the last evaluation — lets steady reading on one line
     // skip the whole match. Includes the line because line-precise sources can change the match as the
     // reading position advances line-by-line within a block.
@@ -133,10 +147,11 @@ private void RaisePortalWindowProps()
     // failure cannot re-trigger a full-page rasterisation on every pass. Cleared on tab switch.
     private readonly HashSet<string> _failedAutoPins = [];
     // Source/target blocks of the currently displayed auto pin, kept so the on-page markers can be
-    // drawn for it (auto pins are never saved Portals, so BuildPortalMarkers has nothing else to read).
-    // One field, set and cleared in lockstep with the "auto:" _displayedPortalId via SetAutoPin /
-    // ClearAutoPin, so the two can never drift out of sync.
-    private (int SrcPage, int SrcBlock, int SrcLine, int TgtPage, int TgtBlock)? _autoPin;
+    // drawn for it (auto pins are never saved Portals, so BuildPortalMarkers has nothing else to read)
+    // and so the float+caption union crop can be reproduced on a dock re-render (#190 — hence CaptionBlock).
+    // One field, set and cleared in lockstep with the "auto:" _displayedPortalId, so the two can never
+    // drift out of sync.
+    private (int SrcPage, int SrcBlock, int SrcLine, int TgtPage, int TgtBlock, int CaptionBlock)? _autoPin;
     // _displayedPortalId values for auto pins, so they share the pin-until-different machinery with
     // saved portals without ever colliding with a saved portal's guid. The resolved target is part
     // of the identity, so a same-labelled but different float (numbering restarts across chapters)
@@ -309,6 +324,7 @@ private void ClearActivePortal()
         _displayedPortalId = null;
         _autoPin = null;
         _portalTargetPending = false;
+        _trackingCropSkipped = false;
         SetPortalImage(null);
         ActivePortalLabel = null;
         PortalHint = NoActivePortalHint;
@@ -325,6 +341,7 @@ private void ResetDisplayedPortal()
         _displayedPortalId = null;
         _autoPin = null;
         _portalTargetPending = false;
+        _trackingCropSkipped = false;
         ActivePortalLabel = null;
         PortalHint = NoActivePortalHint;
         InvalidatePortalMarkers();
@@ -350,7 +367,7 @@ internal void EvaluatePortals(bool forceRender = false)
         if (_controller.FocusedViewport?.Owner is not { } doc || ActiveTab is not { } tab)
         {
             ClearPortalPeek();
-            ClearActivePortal();
+            ClearActivePortal();   // also resets _trackingCropSkipped
             _portalImageOwner = null;
             _lastEvalReadingBlock = null;
             _autoRefPending = false;
@@ -386,6 +403,7 @@ internal void EvaluatePortals(bool forceRender = false)
             _displayedPortalId = null;
             _autoPin = null;
             _portalTargetPending = false;
+            _trackingCropSkipped = false;
             _pendingTargetForLink = null;
             _pageSizeCache.Clear();
             _referenceIndex.Clear();
@@ -517,21 +535,30 @@ private void PinAutoReference(DocumentModel doc, ReferenceIndex.Reference refere
         string autoId = AutoPinId(reference, target);
         if (!doc.TryGetAnalysis(target.Page, out var targetAnalysis)) return;
         var blocks = targetAnalysis.Blocks;
-        var region = Union(blocks[target.TargetBlock].BBox, blocks[target.CaptionBlock].BBox);
-        var bitmap = RenderRegionCrop(doc, target.Page, region);
-        if (bitmap is null)
+
+        // Render the float+caption union crop into the tracking preview — unless that preview has no
+        // on-screen consumer right now (#190: popped-out + live shows the confined viewport instead and
+        // re-renders on dock). The skip still pins + aims the live viewport; only the rasterise is elided.
+        Bitmap? bitmap = null;
+        if (!PortalCropRedundant)
         {
-            // Remember the failure: a deterministic render failure must not re-trigger a full-page
-            // rasterisation on every line advance / forced pass. Cleared on tab switch.
-            _failedAutoPins.Add(autoId);
-            return;   // keep whatever was shown
+            var region = Union(blocks[target.TargetBlock].BBox, blocks[target.CaptionBlock].BBox);
+            bitmap = RenderRegionCrop(doc, target.Page, region);
+            if (bitmap is null)
+            {
+                // Remember the failure: a deterministic render failure must not re-trigger a full-page
+                // rasterisation on every line advance / forced pass. Cleared on tab switch.
+                _failedAutoPins.Add(autoId);
+                return;   // keep whatever was shown
+            }
         }
 
         _displayedPortalId = autoId;
-        _autoPin = (srcPage, srcBlock, srcLine, target.Page, target.TargetBlock);
+        _autoPin = (srcPage, srcBlock, srcLine, target.Page, target.TargetBlock, target.CaptionBlock);
         _portalTargetPending = false;
+        _trackingCropSkipped = bitmap is null;   // skipped while redundant → re-render on dock
         ActivePortalLabel = $"{reference} (p.{target.Page + 1}) · auto";
-        SetPortalImage(bitmap);
+        SetPortalImage(bitmap);   // null while redundant: releases the prior crop (nothing shows it now)
         PortalHint = null;
         InvalidatePortalMarkers();   // a previously accented saved portal loses the accent
         RaisePortalView();   // auto pin changed → (re-)aim the live portal viewport at the float
@@ -554,27 +581,84 @@ private void RenderPortalTarget(DocumentModel doc, int page, int block)
         if (!resolvable)
         {
             _portalTargetPending = true;
+            _trackingCropSkipped = false;   // nothing to skip — waiting on analysis
             SetPortalImage(null);
             PortalHint = "Resolving target…";
             RaisePortalView();   // unresolved → live view shows hint until the page is analysed
             return;
         }
 
         _portalTargetPending = false;
+
+        // #190: while popped-out AND live, the docked preview is hidden and the pop-out shows the live
+        // confined viewport — the 300-DPI crop has no on-screen consumer, so skip the rasterise. Docking
+        // re-materialises it (RerenderDisplayedCrop). The live viewport renders the block itself, so it's
+        // still aimed below via RaisePortalView.
+        if (PortalCropRedundant)
+        {
+            _trackingCropSkipped = true;
+            SetPortalImage(null);   // release the prior crop (nothing on-screen consumes it now)
+            PortalHint = null;
+            RaisePortalView();
+            return;
+        }
+
         var bitmap = RenderBlockCrop(doc, page, block);
         if (bitmap is null)
         {
+            _trackingCropSkipped = false;
             SetPortalImage(null);
             PortalHint = "Could not render portal target.";
             // The live viewport renders the block directly (it doesn't need the crop), so still aim it.
             RaisePortalView();
             return;
         }
+        _trackingCropSkipped = false;
         SetPortalImage(bitmap);
         PortalHint = null;
         RaisePortalView();   // saved target resolved → (re-)aim the live portal viewport at it
     }
 
+    /// <summary>Materialise the currently displayed target's tracking crop into <see cref="ActivePortalImage"/>
+    /// when it was elided (#190). The render paths skip the rasterise while popped-out + live (the docked
+    /// preview is hidden); docking re-reveals that preview, so the crop must be produced now or it would
+    /// stay blank/stale. A no-op unless a crop was actually skipped (so a normal dock never re-rasterises).
+    /// UI thread only (PDFium).</summary>
+    private void RerenderDisplayedCrop()
+    {
+        if (!_trackingCropSkipped) return;
+        if (_controller.FocusedViewport?.Owner is not { } doc || ActiveTab is not { } tab
+            || _displayedPortalId is null)
+        {
+            _trackingCropSkipped = false;
+            return;
+        }
+
+        if (_autoPin is { } a)
+        {
+            // Reproduce the auto pin's float+caption union crop (the stored CaptionBlock makes this
+            // possible without re-parsing the reference). A render failure here just leaves the docked
+            // preview blank; the flag clears either way so it can't retry-loop on every dock.
+            _trackingCropSkipped = false;
+            if (doc.TryGetAnalysis(a.TgtPage, out var analysis)
+                && a.TgtBlock >= 0 && a.TgtBlock < analysis.Blocks.Count
+                && a.CaptionBlock >= 0 && a.CaptionBlock < analysis.Blocks.Count)
+            {
+                var region = Union(analysis.Blocks[a.TgtBlock].BBox, analysis.Blocks[a.CaptionBlock].BBox);
+                SetPortalImage(RenderRegionCrop(doc, a.TgtPage, region));
+            }
+        }
+        else if (tab.Portals.Portals.FirstOrDefault(p => p.Id == _displayedPortalId) is { } portal)
+        {
+            // Now docked (not redundant), RenderPortalTarget rasterises and clears _trackingCropSkipped.
+            RenderPortalTarget(doc, portal.Target.Page, ResolveAnchorBlock(doc, portal.Target));
+        }
+        else
+        {
+            _trackingCropSkipped = false;   // displayed id no longer matches a saved/auto pin
+        }
+    }
+
     /// <summary>Rasterise a detected block region and decode it to an Avalonia <see cref="Bitmap"/>,
     /// or null if the page/block is unavailable or rendering fails. UI thread only (PDFium).</summary>
     private Bitmap? RenderBlockCrop(DocumentModel doc, int page, int block)

src/RailReader2/Views/MainWindow.DocumentWindows.cs

@@ -80,7 +80,7 @@ private void OnDocumentWindowClosed(object? sender, EventArgs e)
         // Was this window's viewport the focused one? Capture before disposal re-points focus.
         bool wasFocused = Vm is { } vm0
             && ReferenceEquals(vm0.Controller.FocusedViewport, view.SurfaceViewport);
-        DisposeSecondaryView(view);
+        DisposeSecondarySurface(view);
 
         // Closing the FOCUSED window: Core re-points FocusedViewport to the primary, but the desktop
         // focus projections (ambient size, status-bar zoom, rail toolbar, menu gating) are only
@@ -98,7 +98,7 @@ private bool CloseFocusedDocumentWindow()
         foreach (var win in _documentWindows)
             if (win.HostedView is { } v && ReferenceEquals(v.SurfaceViewport, focused))
             {
-                win.Close(); // → OnDocumentWindowClosed → DisposeSecondaryView
+                win.Close(); // → OnDocumentWindowClosed → DisposeSecondarySurface
                 return true;
             }
         return false;

src/RailReader2/Views/MainWindow.Panes.cs

@@ -63,10 +63,43 @@ private void TeardownPanes(MainWindowViewModel vm)
         CloseAllDocumentWindows();
     }
 
-    // ── Shared surface lifecycle (reused by split panes and tear-off windows) ──────────────
+    // ── Shared surface lifecycle (split panes, tear-off windows, AND the live portal viewport) ──────
+    //
+    // The DocumentView wiring half of "spin up a secondary surface" is identical for all three: a fresh
+    // DocumentView, model/pref wiring, its own ViewportImages (ownsImages), then surface registration.
+    // BuildSecondarySurface / DisposeSecondarySurface are that single shared core (#192) — the callers
+    // own the genuinely different parts: how the viewport is created + seated, whether the new surface
+    // is focused, and where the view is hosted (PaneGrid, a tear-off window, or the portal pop-out).
+
+    /// <summary>Bind a fresh chrome-carrying DocumentView to an already-created <paramref name="vp"/> and
+    /// register it as a tickable surface. The shared core of every secondary surface; the caller created
+    /// + seated the viewport and decides focus + hosting. Null if there's no active tab.</summary>
+    private DocumentView? BuildSecondarySurface(Viewport vp)
+    {
+        if (Vm is not { } vm || vm.ActiveTab is not { } tab) return null;
+        var view = new DocumentView();
+        view.Initialize(vm, tab);                                       // model wiring (annotations / prefs)
+        view.BindViewport(vp, new ViewportImages(vp), ownsImages: true); // its own page/minimap images
+        vm.RegisterSurface(view);
+        return view;
+    }
 
-    /// <summary>Create a fresh viewport on the active document and a DocumentView bound to it,
-    /// opening on the focused view's current page. Registers it as a surface. Null if no document.</summary>
+    /// <summary>Tear down a surface built by <see cref="BuildSecondarySurface"/>: unregister it from the
+    /// frame loop, retire its owned images on the composition thread, then remove + dispose its viewport
+    /// from the owning model (Core re-points focus off a removed focused view to that doc's primary).
+    /// Shared by split panes, tear-offs, and the live portal viewport (#192).</summary>
+    private void DisposeSecondarySurface(DocumentView view)
+    {
+        if (Vm is not { } vm) return;
+        var vp = view.SurfaceViewport;
+        vm.UnregisterSurface(view);
+        view.Teardown(); // retires its owned page/minimap images + freeze crops on the composition thread (#180, #191)
+        if (vp is { } v)
+            vm.SafeRemoveViewport(v); // safe whether the model is alive or already disposed
+    }
+
+    /// <summary>Create a fresh viewport on the active document and a DocumentView bound to it, opening on
+    /// the focused view's current page (split panes / tear-offs). Null if no document.</summary>
     private DocumentView? CreateSecondaryView()
     {
         if (Vm is not { } vm || vm.ActiveTab is not { } tab) return null;
@@ -82,23 +115,7 @@ private void TeardownPanes(MainWindowViewModel vm)
         // rail synchronously, otherwise it schedules analysis and the fan-out seats it when it arrives.
         doc.SubmitAnalysis(vp, vm.Controller.Worker, vm.Controller.Config.NavigableRoles);
 
-        var view = new DocumentView();
-        view.Initialize(vm, tab);                                   // model wiring (annotations / prefs)
-        view.BindViewport(vp, new ViewportImages(vp), ownsImages: true); // its own page/minimap images
-        vm.RegisterSurface(view);
-        return view;
-    }
-
-    /// <summary>Unregister a secondary surface, dispose its images, and remove + dispose its viewport
-    /// from its owning document. Core re-points focus off a removed focused view to that doc's primary.</summary>
-    private void DisposeSecondaryView(DocumentView view)
-    {
-        if (Vm is not { } vm) return;
-        var vp = view.SurfaceViewport;
-        vm.UnregisterSurface(view);
-        view.Teardown(); // retires its owned page/minimap images + freeze crops on the composition thread (#180, #191)
-        if (vp is { } v)
-            vm.SafeRemoveViewport(v); // shared with the portal viewport teardown (MainWindowViewModel)
+        return BuildSecondarySurface(vp);
     }
 
     // ── Split panes ────────────────────────────────────────────────────────────────────────
@@ -132,7 +149,7 @@ private void RemovePane(DocumentView pane)
         if (idx < 0) return;
 
         _panes.RemoveAt(idx);
-        DisposeSecondaryView(pane); // Core re-points focus to Primary if this pane was focused
+        DisposeSecondarySurface(pane); // Core re-points focus to Primary if this pane was focused
         RebuildPaneGrid();
 
         // Move focus to a neighbouring remaining pane (Core already fell back to Primary; refine it).
@@ -147,7 +164,7 @@ private void OnCloseExtraSurfaces()
         // Remove every docked pane except the primary.
         for (int i = _panes.Count - 1; i >= 1; i--)
         {
-            DisposeSecondaryView(_panes[i]);
+            DisposeSecondarySurface(_panes[i]);
             _panes.RemoveAt(i);
         }
         RebuildPaneGrid();