From c3e45021f8209d559192cb849a5d0e13db75337d Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 6 May 2026 11:13:31 +0200 Subject: [PATCH 01/21] Add citation/reference hover preview Hovering an internal-document link (citation, figure, footnote, etc.) now shows a small popup that renders the destination region of the target page, so the bibliography entry / figure / footnote is visible without leaving the current page. Inspired by PDFRefPreview. Detection: - IsCitationLink: any internal kindPageElementDest link. - DetectEntryBox: per-glyph text+coords scan from the destination anchor, ending at one of: "[N" at the entry's first-line X, indent change back to that X, an X that matches neither first-line nor continuation X, vertical paragraph break, single-line-entry pattern, or column wrap. - Falls back to a fixed strip when text-based detection finds nothing near destY, and to a taller strip when the detected box is too small (figure / diagram fragment). - Result is rendered via engine->RenderPage and blitted into a yellow popup, letterboxed to fit max bounds. Engine API: - IPageDestination::GetDestPoint2() (default returns GetRect2()). - PageDestGetDestPoint() helper. - PageDestinationMupdf caches resolved (destX, destY) from fz_resolve_link at construction. Plumbing: - New RefHover.cpp/.h. - MainWindow gets a refHover member, destroyed in dtor. - Canvas OnMouseMove schedules / hides the popup; OnTimer renders. Configurable via the EnableCitationHover advanced setting (default true). fixes #128 fixes #4221 Co-Authored-By: Claude Opus 4.7 --- cmd/gen-settings.ts | 9 + docs/md/Advanced-options-settings.md | 5 + docs/md/Version-history.md | 1 + src/Canvas.cpp | 48 +++ src/EngineBase.h | 12 + src/EngineMupdf.cpp | 23 +- src/MainWindow.cpp | 2 + src/MainWindow.h | 2 + src/RefHover.cpp | 443 ++++++++++++++++++++++++++ src/RefHover.h | 27 ++ src/Settings.h | 18 +- vs2022/SumatraPDF-dll.vcxproj | 1 + vs2022/SumatraPDF-dll.vcxproj.filters | 6 + vs2022/SumatraPDF.vcxproj | 1 + vs2022/SumatraPDF.vcxproj.filters | 6 + 15 files changed, 596 insertions(+), 8 deletions(-) create mode 100644 src/RefHover.cpp create mode 100644 src/RefHover.h diff --git a/cmd/gen-settings.ts b/cmd/gen-settings.ts index 4e3c362e49a..1e24f601545 100644 --- a/cmd/gen-settings.ts +++ b/cmd/gen-settings.ts @@ -746,6 +746,15 @@ const globalPrefs: Field[] = [ ), setVersion(mkField("ScrollbarInSinglePage", Bool, false, "if true, we show scrollbar in single page mode"), "3.6"), setVersion(mkField("SmoothScroll", Bool, false, "if true, implements smooth scrolling"), "3.6"), + setVersion( + mkField( + "EnableCitationHover", + Bool, + true, + "if true, hovering an internal-document link shows a popup rendering the destination region (citation entry, figure, footnote)", + ), + "3.7", + ), setVersion( mkField( "FastScrollOverScrollbar", diff --git a/docs/md/Advanced-options-settings.md b/docs/md/Advanced-options-settings.md index 9b405391aa6..956602d8640 100644 --- a/docs/md/Advanced-options-settings.md +++ b/docs/md/Advanced-options-settings.md @@ -140,6 +140,11 @@ ScrollbarInSinglePage = false ; if true, implements smooth scrolling (introduced in version 3.6) SmoothScroll = false +; if true, hovering an internal-document link shows a popup rendering the +; destination region (citation entry, figure, footnote) (introduced in version +; 3.7) +EnableCitationHover = true + ; if true, mouse wheel scrolling is faster when mouse is over a scrollbar ; (introduced in version 3.6) FastScrollOverScrollbar = false diff --git a/docs/md/Version-history.md b/docs/md/Version-history.md index f5ab5fbc8cd..d8811dc6036 100644 --- a/docs/md/Version-history.md +++ b/docs/md/Version-history.md @@ -61,6 +61,7 @@ Available in [pre-release](https://www.sumatrapdfreader.org/prerelease) builds. - fix Edit Annotations window not restoring to the correct monitor in multi-monitor setups - use `GetFileAttributesEx` instead of opening files for change detection on network drives, avoiding Windows Defender re-scans - fix toolbar page number misalignment when `PrinterAccess` is revoked in `sumatrapdfrestrict.ini` +- add citation/reference hover preview: hovering an internal-document link (e.g. a `[1]` citation, figure reference, or footnote marker) now shows a small popup rendering the destination region, so you can see the bibliography entry / figure / footnote without leaving the current page. Toggle with the `EnableCitationHover` advanced setting (fixes [#128](https://github.com/sumatrapdfreader/sumatrapdf/issues/128), [#4221](https://github.com/sumatrapdfreader/sumatrapdf/issues/4221)) ## 3.6.1 (2026-04-06) diff --git a/src/Canvas.cpp b/src/Canvas.cpp index 3344e44ec7b..06aa1ae3960 100644 --- a/src/Canvas.cpp +++ b/src/Canvas.cpp @@ -57,6 +57,8 @@ #include "Toolbar.h" #include "Translations.h" +#include "RefHover.h" + #include "utils/Log.h" // if set instead of trying to render pages we don't have, we simply do nothing @@ -793,6 +795,24 @@ static bool gShowAnnotationNotification = true; // Forward declaration static RectF CalculateResizedRect(MainWindow* win, int x, int y); +// Returns true when el is an internal-document link (not an external URL or +// file launch). Used as a heuristic for "this is probably a citation link". +static bool IsCitationLink(IPageElement* el) { + if (!el || !el->Is(kindPageElementDest)) { + return false; + } + IPageDestination* dest = el->AsLink(); + if (!dest) { + return false; + } + Kind k = dest->GetKind(); + if (k == kindDestinationLaunchURL || k == kindDestinationLaunchFile) { + return false; + } + int destPage = PageDestGetPageNo(dest); + return destPage > 0; +} + static void OnMouseMove(MainWindow* win, int x, int y, WPARAM) { DisplayModel* dm = win->AsFixed(); // ReportIf(!dm); // can happen if reload fails, we delete DisplayModel @@ -902,6 +922,27 @@ static void OnMouseMove(MainWindow* win, int x, int y, WPARAM) { RemoveNotificationsForGroup(win->hwndCanvas, kNotifAnnotation); } win->annotationUnderCursor = annot; + + // Citation hover: render the destination region of an internal + // link (typically the bibliography entry) into a popup. + if (gGlobalPrefs->enableCitationHover) { + if (!win->refHover) { + win->refHover = RefHoverCreate(win->hwndCanvas); + } + IPageElement* el = dm->GetElementAtPos(pos, nullptr); + if (win->refHover && IsCitationLink(el)) { + IPageDestination* dest = el->AsLink(); + int destPage = PageDestGetPageNo(dest); + RectF destPt = PageDestGetDestPoint(dest); + Point screenPt = {x, y}; + ClientToScreen(win->hwndCanvas, (POINT*)&screenPt); + RefHoverSchedule(win->refHover, win->hwndCanvas, screenPt, destPage, destPt.x, destPt.y); + } else if (win->refHover) { + RefHoverHide(win->refHover, win->hwndCanvas); + } + } else if (win->refHover) { + RefHoverHide(win->refHover, win->hwndCanvas); + } break; } @@ -2848,6 +2889,13 @@ static void OnTimer(MainWindow* win, HWND hwnd, WPARAM timerId) { } break; + case kRefHoverTimerID: { + DisplayModel* dm = win->AsFixed(); + EngineBase* engine = dm ? dm->GetEngine() : nullptr; + RefHoverOnTimer(win->refHover, hwnd, engine); + break; + } + case HIDE_FWDSRCHMARK_TIMER_ID: win->fwdSearchMark.hideStep++; if (1 == win->fwdSearchMark.hideStep) { diff --git a/src/EngineBase.h b/src/EngineBase.h index f33254a2fe4..9e3801ccde9 100644 --- a/src/EngineBase.h +++ b/src/EngineBase.h @@ -91,6 +91,9 @@ struct IPageDestination : KindBase { virtual RectF GetRect2() { return rect; } // optional zoom level on the above returned page virtual float GetZoom2() { return zoom; } + // anchor point (x, y) on the destination page; rect's dx/dy may be 0. + // Default falls back to GetRect2 (callers should still tolerate (0,0)). + virtual RectF GetDestPoint2() { return GetRect2(); } // string value associated with the destination (e.g. a path or a URL) virtual char* GetValue2() { return nullptr; } @@ -119,6 +122,15 @@ static inline RectF PageDestGetRect(IPageDestination* dest) { return dest->GetRect2(); } +// anchor point on the destination page (x, y in user-space). Returns {0,0,0,0} +// when the destination has no specific anchor. +static inline RectF PageDestGetDestPoint(IPageDestination* dest) { + if (!dest) { + return {}; + } + return dest->GetDestPoint2(); +} + // optional zoom level on the above returned page static inline float PageDestGetZoom(IPageDestination* dest) { return dest->GetZoom2(); diff --git a/src/EngineMupdf.cpp b/src/EngineMupdf.cpp index a54c88c7f09..b1ef784eef4 100644 --- a/src/EngineMupdf.cpp +++ b/src/EngineMupdf.cpp @@ -92,6 +92,11 @@ struct PageDestinationMupdf : IPageDestination { char* value = nullptr; char* name = nullptr; + // anchor (x, y) on the destination page resolved from the link URI; + // -1 means "not resolved" (e.g. external URL or file launch). + float destX = -1.f; + float destY = -1.f; + PageDestinationMupdf(fz_link* l, fz_outline* o) { // exactly one must be provided kind = kindDestinationMupdf; @@ -107,6 +112,16 @@ struct PageDestinationMupdf : IPageDestination { } return rect; } + + RectF GetDestPoint2() override { + if (outline) { + return RectF{outline->x, outline->y, 0, 0}; + } + if (destY >= 0.f) { + return RectF{destX, destY, 0, 0}; + } + return {}; + } ~PageDestinationMupdf() override { str::Free(value); str::Free(name); @@ -223,7 +238,13 @@ static IPageDestination* NewPageDestinationMupdf(fz_context* ctx, fz_document* d auto dest = new PageDestinationMupdf(link, outline); dest->rect = FzGetRectF(link, outline); - dest->pageNo = FzGetPageNo(ctx, doc, link, outline); + { + float x = 0, y = 0; + const char* destUri = link ? link->uri : (outline ? outline->uri : nullptr); + dest->pageNo = ResolveLink(ctx, doc, destUri, &x, &y); + dest->destX = x; + dest->destY = y; + } return dest; } diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp index 488254364b7..7986dae2703 100644 --- a/src/MainWindow.cpp +++ b/src/MainWindow.cpp @@ -33,6 +33,7 @@ #include "OverlayScrollbar.h" #include "SumatraPDF.h" #include "MainWindow.h" +#include "RefHover.h" #include "WindowTab.h" #include "TableOfContents.h" #include "resource.h" @@ -109,6 +110,7 @@ void CreateMovePatternLazy(MainWindow* win) { MainWindow::~MainWindow() { KillTimer(hwndCanvas, kSmoothScrollTimerID); + RefHoverDestroy(refHover); FinishStressTest(this); ReportIf(TabCount() > 0); diff --git a/src/MainWindow.h b/src/MainWindow.h index f9b811d1087..4e160099ad7 100644 --- a/src/MainWindow.h +++ b/src/MainWindow.h @@ -52,6 +52,7 @@ struct WindowTab; struct Annotation; struct ILinkHandler; +struct RefHoverState; // Current action being performed with a mouse enum class MouseAction { @@ -268,6 +269,7 @@ struct MainWindow { IPageElement* linkOnLastButtonDown = nullptr; AutoFreeStr urlOnLastButtonDown; Annotation* annotationUnderCursor = nullptr; + RefHoverState* refHover = nullptr; // highlight rectangle for element under cursor during context menu (in page coordinates) RectF contextMenuHighlightRect{}; int contextMenuHighlightPageNo = 0; diff --git a/src/RefHover.cpp b/src/RefHover.cpp new file mode 100644 index 00000000000..dcbd200ff50 --- /dev/null +++ b/src/RefHover.cpp @@ -0,0 +1,443 @@ +/* Copyright 2024 the SumatraPDF project authors (see AUTHORS file). + License: GPLv3 */ + +#include "utils/BaseUtil.h" +#include "utils/WinUtil.h" + +#include "wingui/UIModels.h" + +#include "DocController.h" +#include "EngineBase.h" +#include "RefHover.h" + +#define REF_HOVER_CLASS L"SumatraPDFRefHover" + +// Fallback strip dimensions (used when text-based detection can't find boundaries). +static constexpr float kStripHeightPt = 130.f; +// Larger fallback used when detection found something tiny (likely a figure +// fragment) — gives enough height to capture a diagram + caption. +static constexpr float kFigureStripHeightPt = 280.f; +static constexpr float kAnchorTopMarginPt = 6.f; +static constexpr float kRightMarginPt = 4.f; +// pt of padding around the detected entry box. +static constexpr float kEntryPadPt = 6.f; +// zoom for the rendered strip — higher = sharper but larger popup. +static constexpr float kRenderZoom = 1.5f; +// upper bounds for the popup window in screen pixels. +static constexpr int kMaxPopupWidth = 800; +static constexpr int kMaxPopupHeight = 320; +static constexpr int kBorder = 4; + +static bool gClassRegistered = false; + +static LRESULT CALLBACK RefHoverWndProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) { + if (msg == WM_PAINT) { + PAINTSTRUCT ps; + HDC hdc = BeginPaint(hwnd, &ps); + + RECT rc; + GetClientRect(hwnd, &rc); + + HBRUSH hbg = CreateSolidBrush(RGB(255, 252, 200)); + FillRect(hdc, &rc, hbg); + DeleteObject(hbg); + + RefHoverState* s = (RefHoverState*)GetWindowLongPtrW(hwnd, GWLP_USERDATA); + if (s && s->bmp) { + Size bmpSize = s->bmp->GetSize(); + int destW = rc.right - rc.left - 2 * kBorder; + int destH = rc.bottom - rc.top - 2 * kBorder; + float sx = (float)destW / (float)bmpSize.dx; + float sy = (float)destH / (float)bmpSize.dy; + float scale = sx < sy ? sx : sy; + int drawW = (int)(bmpSize.dx * scale); + int drawH = (int)(bmpSize.dy * scale); + int dx = (rc.right - rc.left - drawW) / 2; + int dy = (rc.bottom - rc.top - drawH) / 2; + s->bmp->Blit(hdc, Rect{dx, dy, drawW, drawH}); + } + + EndPaint(hwnd, &ps); + return 0; + } + if (msg == WM_ERASEBKGND) { + return 1; + } + return DefWindowProc(hwnd, msg, wp, lp); +} + +static void RegisterClassIfNeeded() { + if (gClassRegistered) { + return; + } + WNDCLASSW wc{}; + wc.lpfnWndProc = RefHoverWndProc; + wc.hInstance = GetModuleHandleW(nullptr); + wc.lpszClassName = REF_HOVER_CLASS; + wc.hCursor = LoadCursorW(nullptr, IDC_ARROW); + RegisterClassW(&wc); + gClassRegistered = true; +} + +RefHoverState* RefHoverCreate(HWND hwndCanvas) { + RegisterClassIfNeeded(); + auto* s = new RefHoverState(); + HWND hwnd = CreateWindowExW(WS_EX_TOOLWINDOW, REF_HOVER_CLASS, nullptr, WS_POPUP | WS_BORDER, 0, 0, 10, 10, + hwndCanvas, nullptr, GetModuleHandleW(nullptr), nullptr); + if (!hwnd) { + delete s; + return nullptr; + } + SetWindowLongPtrW(hwnd, GWLP_USERDATA, (LONG_PTR)s); + s->hwndPopup = hwnd; + return s; +} + +void RefHoverDestroy(RefHoverState* s) { + if (!s) { + return; + } + if (s->hwndPopup) { + DestroyWindow(s->hwndPopup); + s->hwndPopup = nullptr; + } + delete s->bmp; + s->bmp = nullptr; + delete s; +} + +static void ShowPopup(RefHoverState* s, Point screenPt) { + if (!s || !s->hwndPopup || !s->bmp) { + return; + } + Size bmpSize = s->bmp->GetSize(); + int popupW = bmpSize.dx + 2 * kBorder; + int popupH = bmpSize.dy + 2 * kBorder; + + // Letterbox to max bounds. + if (popupW > kMaxPopupWidth) { + float scale = (float)(kMaxPopupWidth - 2 * kBorder) / (float)bmpSize.dx; + popupW = kMaxPopupWidth; + popupH = (int)(bmpSize.dy * scale) + 2 * kBorder; + } + if (popupH > kMaxPopupHeight) { + float scale = (float)(kMaxPopupHeight - 2 * kBorder) / (float)bmpSize.dy; + popupH = kMaxPopupHeight; + popupW = (int)(bmpSize.dx * scale) + 2 * kBorder; + } + + int x = screenPt.x + 16; + int y = screenPt.y + 16; + HMONITOR hmon = MonitorFromPoint({screenPt.x, screenPt.y}, MONITOR_DEFAULTTONEAREST); + MONITORINFO mi{}; + mi.cbSize = sizeof(mi); + GetMonitorInfoW(hmon, &mi); + if (x + popupW > mi.rcWork.right) { + x = screenPt.x - popupW - 4; + } + if (y + popupH > mi.rcWork.bottom) { + y = screenPt.y - popupH - 4; + } + + SetWindowPos(s->hwndPopup, HWND_TOPMOST, x, y, popupW, popupH, SWP_NOACTIVATE | SWP_SHOWWINDOW); + InvalidateRect(s->hwndPopup, nullptr, TRUE); +} + +void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int destPage, float destX, float destY) { + if (!s) { + return; + } + KillTimer(hwndCanvas, kRefHoverTimerID); + + if (IsWindowVisible(s->hwndPopup) && s->displayedDestPage == destPage) { + return; + } + s->pendingScreenPt = screenPt; + s->pendingDestPage = destPage; + s->pendingDestX = destX; + s->pendingDestY = destY; + SetTimer(hwndCanvas, kRefHoverTimerID, kRefHoverDelayMs, nullptr); +} + +void RefHoverHide(RefHoverState* s, HWND hwndCanvas) { + if (!s) { + return; + } + KillTimer(hwndCanvas, kRefHoverTimerID); + s->pendingDestPage = -1; + if (s->hwndPopup && IsWindowVisible(s->hwndPopup)) { + ShowWindow(s->hwndPopup, SW_HIDE); + s->displayedDestPage = -1; + } +} + +// Fallback box when text-based detection can't find boundaries: a strip from +// destX to the right page margin. heightPt controls how tall — small for the +// "no text near destY" case, larger for figures/diagrams. +static RectF FallbackBox(RectF mediabox, float destX, float destY, float heightPt) { + float lx = (destX >= 0.f) ? destX - 12.f : 0.f; + if (lx < 0.f) { + lx = 0.f; + } + float ty = (destY >= 0.f) ? destY - kAnchorTopMarginPt : 0.f; + if (ty < 0.f) { + ty = 0.f; + } + float w = mediabox.dx - lx - kRightMarginPt; + if (w < 100.f) { + lx = 0.f; + w = mediabox.dx; + } + if (ty + heightPt > mediabox.dy) { + heightPt = mediabox.dy - ty; + if (heightPt < 0.f) { + heightPt = mediabox.dy; + ty = 0.f; + } + } + return RectF{lx, ty, w, heightPt}; +} + +// Find the bounding box of a single bibliography entry on the destination +// page. Uses per-glyph text+coords from the engine's text cache: +// 1. Locate the leftmost glyph with y in a small band around destY (entry start). +// 2. Scan forward; stop at "[N" near the same left margin (next entry) or +// a vertical paragraph gap. +// 3. Return the min/max bounding box of glyphs in [start, end), padded. +// Falls back to FallbackBox() if no glyphs are near destY. +static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float destY) { + RectF mediabox = engine->PageMediabox(destPage); + if (destY < 0.f) { + return FallbackBox(mediabox, destX, destY, kStripHeightPt); + } + + int textLen = 0; + Rect* coords = nullptr; + const WCHAR* text = engine->GetTextForPage(destPage, &textLen, &coords); + if (!text || textLen <= 0 || !coords) { + return FallbackBox(mediabox, destX, destY, kStripHeightPt); + } + + int dY = (int)destY; + int dX = (int)destX; + // Constrain to the destination's column — for 2-column layouts this + // prevents the search from latching onto same-Y body text in another + // column. We allow a small left tolerance so a "[1]" whose [ starts + // a few pt left of destX still matches. + int columnLeft = (destX >= 0.f) ? dX - 15 : INT_MIN; + + // 1. Find the start glyph: top-left non-whitespace glyph with + // y in [destY-5, destY+30] and x at-or-right-of columnLeft. + int startIdx = -1; + int bestY = INT_MAX; + int bestX = INT_MAX; + for (int i = 0; i < textLen; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + Rect r = coords[i]; + if (r.y < dY - 5 || r.y > dY + 30) { + continue; + } + if (r.x < columnLeft) { + continue; + } + if (r.y < bestY || (r.y == bestY && r.x < bestX)) { + bestY = r.y; + bestX = r.x; + startIdx = i; + } + } + if (startIdx < 0) { + return FallbackBox(mediabox, destX, destY, kStripHeightPt); + } + + int firstLineLeftX = coords[startIdx].x; + int firstLineY = coords[startIdx].y; + int firstLineDy = coords[startIdx].dy; + if (firstLineDy <= 0) { + firstLineDy = 12; + } + + // 2. Scan forward to find the end of the entry. + int endIdx = textLen; + int prevY = firstLineY; + int prevBottom = firstLineY + firstLineDy; + int lineHeight = firstLineDy; + + // Track leftmost X on the current line vs the previous line so we can + // detect indent changes (the most reliable signal for author-year bibs). + int currentLineLeftX = firstLineLeftX; + int prevLineLeftX = INT_MAX; + // X of the entry's continuation lines (captured from line 2). -1 = unknown. + int indentX = -1; + + for (int i = startIdx + 1; i < textLen; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + Rect r = coords[i]; + + // Stop on column wrap: y goes significantly above the current row. + if (r.y < firstLineY - 5) { + endIdx = i; + break; + } + // Skip glyphs in other columns (left of the entry's column). + if (r.x < firstLineLeftX - 20) { + continue; + } + + bool isNewLine = (r.y > prevY + 2); + if (isNewLine) { + prevLineLeftX = currentLineLeftX; + currentLineLeftX = r.x; + } else if (r.x < currentLineLeftX) { + currentLineLeftX = r.x; + } + + bool pastFirstLine = (r.y > firstLineY + firstLineDy * 3 / 4 + 2); + bool atFirstLineLeftX = (r.x >= firstLineLeftX - 5 && r.x <= firstLineLeftX + 5); + + // Capture the continuation X from the entry's second line. + if (isNewLine && pastFirstLine && indentX < 0 && !atFirstLineLeftX) { + indentX = r.x; + } + bool atIndentX = (indentX > 0) && (r.x >= indentX - 5 && r.x <= indentX + 5); + + // (a) "[N" at the entry's first-line X = next numeric entry. + if (c == L'[' && atFirstLineLeftX && i + 1 < textLen && text[i + 1] >= L'0' && text[i + 1] <= L'9') { + endIdx = i; + break; + } + + // (b) Indent change: a new line back at the entry's first-line X + // after a continuation line at a different X. Catches author-year + // hanging-indent bibliographies where there's no [N] marker. + if (isNewLine && atFirstLineLeftX && pastFirstLine && prevLineLeftX != INT_MAX && + (prevLineLeftX < firstLineLeftX - 5 || prevLineLeftX > firstLineLeftX + 5)) { + endIdx = i; + break; + } + + // (c) Vertical paragraph break (no-indent style fallback). + if (r.y > prevBottom + lineHeight * 5 / 4) { + endIdx = i; + break; + } + + // (d) Once we know indentX, a new line at any X other than firstLineLeftX + // or indentX is external content (author bios, footers, etc.) — end here. + if (isNewLine && pastFirstLine && indentX > 0 && !atFirstLineLeftX && !atIndentX) { + endIdx = i; + break; + } + + // (e) Single-line-entry case: a new line back at firstLineLeftX before + // we discovered a continuation indent. The previous "entry" was one + // line. Common pattern: stacked numbered footnotes "¹url\n²url\n³url". + if (isNewLine && pastFirstLine && atFirstLineLeftX && indentX < 0 && prevLineLeftX != INT_MAX) { + endIdx = i; + break; + } + + // Track current line height as we go (catches changing leading). + if (isNewLine) { + int dy = r.y - prevY; + if (dy > 4 && dy < 60) { + lineHeight = dy; + } + prevY = r.y; + prevBottom = r.y + r.dy; + } + } + + // 3. Compute bounding box of glyphs in [startIdx, endIdx). + int minX = INT_MAX, minY = INT_MAX, maxX = INT_MIN, maxY = INT_MIN; + for (int i = startIdx; i < endIdx; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + Rect r = coords[i]; + // Exclude glyphs that aren't in the entry's column. + if (r.x < firstLineLeftX - 20) { + continue; + } + if (r.y < firstLineY - 5) { + continue; + } + if (r.x < minX) { + minX = r.x; + } + if (r.y < minY) { + minY = r.y; + } + if (r.x + r.dx > maxX) { + maxX = r.x + r.dx; + } + if (r.y + r.dy > maxY) { + maxY = r.y + r.dy; + } + } + if (minX == INT_MAX) { + return FallbackBox(mediabox, destX, destY, kStripHeightPt); + } + + RectF box{(float)minX - kEntryPadPt, (float)minY - kEntryPadPt, (float)(maxX - minX) + 2.f * kEntryPadPt, + (float)(maxY - minY) + 2.f * kEntryPadPt}; + if (box.x < 0.f) { + box.dx += box.x; + box.x = 0.f; + } + if (box.y < 0.f) { + box.dy += box.y; + box.y = 0.f; + } + if (box.x + box.dx > mediabox.dx) { + box.dx = mediabox.dx - box.x; + } + if (box.y + box.dy > mediabox.dy) { + box.dy = mediabox.dy - box.y; + } + if (box.dx < 50.f || box.dy < 20.f) { + return FallbackBox(mediabox, destX, destY, kStripHeightPt); + } + // If detection produced a small box (text-only scan got trapped in one + // fragment of a figure / diagram), expand to a generous strip so the + // surrounding visual content is included in the render. + if (box.dx < 150.f && box.dy < 60.f) { + return FallbackBox(mediabox, destX, destY, kFigureStripHeightPt); + } + return box; +} + +void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine) { + KillTimer(hwndCanvas, kRefHoverTimerID); + if (!s || !engine || s->pendingDestPage <= 0) { + return; + } + int destPage = s->pendingDestPage; + float destX = s->pendingDestX; + float destY = s->pendingDestY; + + RectF mediabox = engine->PageMediabox(destPage); + if (mediabox.dx <= 0.f || mediabox.dy <= 0.f) { + return; + } + + RectF region = DetectEntryBox(engine, destPage, destX, destY); + RenderPageArgs args(destPage, kRenderZoom, 0, ®ion); + RenderedBitmap* bmp = engine->RenderPage(args); + if (!bmp) { + return; + } + + delete s->bmp; + s->bmp = bmp; + s->displayedDestPage = destPage; + + ShowPopup(s, s->pendingScreenPt); +} diff --git a/src/RefHover.h b/src/RefHover.h new file mode 100644 index 00000000000..4c8813915b6 --- /dev/null +++ b/src/RefHover.h @@ -0,0 +1,27 @@ +/* Copyright 2024 the SumatraPDF project authors (see AUTHORS file). + License: GPLv3 */ + +class EngineBase; +struct RenderedBitmap; + +struct RefHoverState { + HWND hwndPopup = nullptr; + // currently shown rendered destination strip (owned) + RenderedBitmap* bmp = nullptr; + int displayedDestPage = -1; + + // pending request: set by RefHoverSchedule, consumed by RefHoverOnTimer + Point pendingScreenPt{}; + int pendingDestPage = -1; + float pendingDestX = -1.f; + float pendingDestY = -1.f; +}; + +constexpr int kRefHoverDelayMs = 300; +constexpr UINT_PTR kRefHoverTimerID = 9; + +RefHoverState* RefHoverCreate(HWND hwndCanvas); +void RefHoverDestroy(RefHoverState* s); +void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int destPage, float destX, float destY); +void RefHoverHide(RefHoverState* s, HWND hwndCanvas); +void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine); diff --git a/src/Settings.h b/src/Settings.h index b9c8906d46b..72ec43d2029 100644 --- a/src/Settings.h +++ b/src/Settings.h @@ -484,6 +484,9 @@ struct GlobalPrefs { bool scrollbarInSinglePage; // if true, implements smooth scrolling bool smoothScroll; + // if true, hovering an internal-document link shows a popup rendering + // the destination region (citation entry, figure, footnote) + bool enableCitationHover; // if true, mouse wheel scrolling is faster when mouse is over a // scrollbar bool fastScrollOverScrollbar; @@ -901,6 +904,7 @@ static const FieldInfo gGlobalPrefsFields[] = { {offsetof(GlobalPrefs, scrollbars), SettingType::String, (intptr_t)"windows"}, {offsetof(GlobalPrefs, scrollbarInSinglePage), SettingType::Bool, false}, {offsetof(GlobalPrefs, smoothScroll), SettingType::Bool, false}, + {offsetof(GlobalPrefs, enableCitationHover), SettingType::Bool, true}, {offsetof(GlobalPrefs, fastScrollOverScrollbar), SettingType::Bool, false}, {offsetof(GlobalPrefs, preventSleepInFullscreen), SettingType::Bool, true}, {offsetof(GlobalPrefs, tabWidth), SettingType::Int, 300}, @@ -961,17 +965,17 @@ static const FieldInfo gGlobalPrefsFields[] = { {(size_t)-1, SettingType::Comment, (intptr_t)"Settings below are not recognized by the current version"}, }; static const StructInfo gGlobalPrefsInfo = { - sizeof(GlobalPrefs), 90, gGlobalPrefsFields, + sizeof(GlobalPrefs), 91, gGlobalPrefsFields, "\0\0CheckForUpdates\0CustomScreenDPI\0DefaultDisplayMode\0DefaultZoom\0EnableTeXEnhancements\0EscToExit\0FullPathI" "nTitle\0InverseSearchCmdLine\0LazyLoading\0MainWindowBackground\0NoHomeTab\0HomePageSortByFrequentlyRead\0ReloadMo" "difiedDocuments\0RememberOpenedFiles\0RememberStatePerDocument\0RestoreSession\0ReuseInstance\0ShowMenubar\0ShowMe" "nubarWithTabs\0ShowTips\0CustomColors\0ShowToolbar\0ShowFavorites\0ShowToc\0ShowLinks\0ShowStartPage\0SidebarDx\0S" - "crollbars\0ScrollbarInSinglePage\0SmoothScroll\0FastScrollOverScrollbar\0PreventSleepInFullscreen\0TabWidth\0Theme" - "\0TocDy\0ToolbarSize\0TreeFontName\0TreeFontSize\0UIFontSize\0DisableAntiAlias\0UseSysColors\0UseTabs\0TabsMru\0Zo" - "omLevels\0ZoomIncrement\0\0FixedPageUI\0\0EBookUI\0\0ComicBookUI\0\0ImageUI\0\0ChmUI\0\0Annotations\0\0ExternalVie" - "wers\0\0ForwardSearch\0\0PrinterDefaults\0\0Fullscreen\0\0SelectionHandlers\0\0Shortcuts\0\0Themes\0\0TabGroups\0" - "\0\0DefaultPasswords\0UiLanguage\0VersionToSkip\0WindowState\0WindowPos\0FileStates\0SessionData\0ReopenOnce\0Time" - "OfLastUpdateCheck\0OpenCountWeek\0PropWinPos\0\0"}; + "crollbars\0ScrollbarInSinglePage\0SmoothScroll\0EnableCitationHover\0FastScrollOverScrollbar\0PreventSleepInFullsc" + "reen\0TabWidth\0Theme\0TocDy\0ToolbarSize\0TreeFontName\0TreeFontSize\0UIFontSize\0DisableAntiAlias\0UseSysColors" + "\0UseTabs\0TabsMru\0ZoomLevels\0ZoomIncrement\0\0FixedPageUI\0\0EBookUI\0\0ComicBookUI\0\0ImageUI\0\0ChmUI\0\0Anno" + "tations\0\0ExternalViewers\0\0ForwardSearch\0\0PrinterDefaults\0\0Fullscreen\0\0SelectionHandlers\0\0Shortcuts\0\0" + "Themes\0\0TabGroups\0\0\0DefaultPasswords\0UiLanguage\0VersionToSkip\0WindowState\0WindowPos\0FileStates\0SessionD" + "ata\0ReopenOnce\0TimeOfLastUpdateCheck\0OpenCountWeek\0PropWinPos\0\0"}; static const FieldInfo gTheme_1_Fields[] = { {offsetof(Theme, name), SettingType::String, (intptr_t)""}, {offsetof(Theme, textColor), SettingType::Color, (intptr_t)""}, diff --git a/vs2022/SumatraPDF-dll.vcxproj b/vs2022/SumatraPDF-dll.vcxproj index 4f3ab7d01b2..8a32f90b1c3 100644 --- a/vs2022/SumatraPDF-dll.vcxproj +++ b/vs2022/SumatraPDF-dll.vcxproj @@ -1244,6 +1244,7 @@ cd ../out/rel64_prefast_asan & ..\..\bin\MakeLZSA.exe InstallerData.dat libm + diff --git a/vs2022/SumatraPDF-dll.vcxproj.filters b/vs2022/SumatraPDF-dll.vcxproj.filters index 73ddf32a62b..fdad19414f6 100644 --- a/vs2022/SumatraPDF-dll.vcxproj.filters +++ b/vs2022/SumatraPDF-dll.vcxproj.filters @@ -234,6 +234,9 @@ src + + src + src @@ -551,6 +554,9 @@ src + + src + src diff --git a/vs2022/SumatraPDF.vcxproj b/vs2022/SumatraPDF.vcxproj index 1b955f90fca..1f3eec1908b 100644 --- a/vs2022/SumatraPDF.vcxproj +++ b/vs2022/SumatraPDF.vcxproj @@ -1227,6 +1227,7 @@ + diff --git a/vs2022/SumatraPDF.vcxproj.filters b/vs2022/SumatraPDF.vcxproj.filters index 6eba3292947..9f9ded03701 100644 --- a/vs2022/SumatraPDF.vcxproj.filters +++ b/vs2022/SumatraPDF.vcxproj.filters @@ -234,6 +234,9 @@ src + + src + src @@ -548,6 +551,9 @@ src + + src + src From 0334fbc9bba51108102e75e91e4cf9fb12c9a863 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 7 May 2026 14:36:17 +0200 Subject: [PATCH 02/21] Add wheel-zoom to citation hover popup Initial render uses page zoom so popup text height matches visible page text. Wheel on the citation link re-renders a region sized to fill the popup at the new zoom (anchored at detection top-left, clamped to page). Wheeling out brings in new page content; wheeling in crops to detail. Co-Authored-By: Claude Opus 4.7 --- src/Canvas.cpp | 22 +++++++- src/RefHover.cpp | 138 +++++++++++++++++++++++++++++++++++++---------- src/RefHover.h | 18 ++++++- 3 files changed, 148 insertions(+), 30 deletions(-) diff --git a/src/Canvas.cpp b/src/Canvas.cpp index 06aa1ae3960..1e10bc23d85 100644 --- a/src/Canvas.cpp +++ b/src/Canvas.cpp @@ -2134,6 +2134,22 @@ static LRESULT CanvasOnMouseWheel(MainWindow* win, UINT msg, WPARAM wp, LPARAM l return res; } + // Wheel-zoom the citation-hover popup while the cursor is still on the + // citation link that opened it. Avoids moving the cursor onto the popup + // (which would dismiss the hover). + if (win->refHover && win->refHover->hwndPopup && IsWindowVisible(win->refHover->hwndPopup)) { + DisplayModel* dmHover = win->AsFixed(); + if (dmHover) { + Point pt = HwndGetCursorPos(win->hwndCanvas); + IPageElement* elHover = dmHover->GetElementAtPos(pt, nullptr); + if (IsCitationLink(elHover)) { + short delta = GET_WHEEL_DELTA_WPARAM(wp); + RefHoverWheelZoom(win->refHover, dmHover->GetEngine(), delta); + return 0; + } + } + } + DisplayModel* dm = win->AsFixed(); // Note: not all mouse drivers correctly report the Ctrl key's state @@ -2892,7 +2908,11 @@ static void OnTimer(MainWindow* win, HWND hwnd, WPARAM timerId) { case kRefHoverTimerID: { DisplayModel* dm = win->AsFixed(); EngineBase* engine = dm ? dm->GetEngine() : nullptr; - RefHoverOnTimer(win->refHover, hwnd, engine); + float pageZoom = 1.f; + if (dm && win->refHover && win->refHover->pendingDestPage > 0) { + pageZoom = dm->GetZoomReal(win->refHover->pendingDestPage); + } + RefHoverOnTimer(win->refHover, hwnd, engine, pageZoom); break; } diff --git a/src/RefHover.cpp b/src/RefHover.cpp index dcbd200ff50..7e3b172f78d 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -21,12 +21,18 @@ static constexpr float kAnchorTopMarginPt = 6.f; static constexpr float kRightMarginPt = 4.f; // pt of padding around the detected entry box. static constexpr float kEntryPadPt = 6.f; -// zoom for the rendered strip — higher = sharper but larger popup. +// upper bound for the auto-fit base zoom. We render at min(kRenderZoom, +// fit-to-popup-max), then multiply by RefHoverState::userZoom (the +// mouse-wheel adjustment). static constexpr float kRenderZoom = 1.5f; // upper bounds for the popup window in screen pixels. -static constexpr int kMaxPopupWidth = 800; -static constexpr int kMaxPopupHeight = 320; +static constexpr int kMaxPopupWidth = 1200; +static constexpr int kMaxPopupHeight = 600; static constexpr int kBorder = 4; +// user-zoom (mouse-wheel) bounds and step. +static constexpr float kMinUserZoom = 0.4f; +static constexpr float kMaxUserZoom = 3.0f; +static constexpr float kUserZoomStep = 1.15f; static bool gClassRegistered = false; @@ -44,17 +50,21 @@ static LRESULT CALLBACK RefHoverWndProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM l RefHoverState* s = (RefHoverState*)GetWindowLongPtrW(hwnd, GWLP_USERDATA); if (s && s->bmp) { + // Draw the bitmap at native pixel size, top-left aligned. + // GDI clips at the popup edges when the bitmap exceeds the + // client area (which happens when the user wheel-zooms in — + // that's how zoomed-in content "fills" the previously blank + // bottom/right of the popup). Size bmpSize = s->bmp->GetSize(); - int destW = rc.right - rc.left - 2 * kBorder; - int destH = rc.bottom - rc.top - 2 * kBorder; - float sx = (float)destW / (float)bmpSize.dx; - float sy = (float)destH / (float)bmpSize.dy; - float scale = sx < sy ? sx : sy; - int drawW = (int)(bmpSize.dx * scale); - int drawH = (int)(bmpSize.dy * scale); - int dx = (rc.right - rc.left - drawW) / 2; - int dy = (rc.bottom - rc.top - drawH) / 2; - s->bmp->Blit(hdc, Rect{dx, dy, drawW, drawH}); + HDC bmpDC = CreateCompatibleDC(hdc); + HGDIOBJ oldBmp = bmpDC ? SelectObject(bmpDC, s->bmp->GetBitmap()) : nullptr; + if (oldBmp) { + BitBlt(hdc, kBorder, kBorder, bmpSize.dx, bmpSize.dy, bmpDC, 0, 0, SRCCOPY); + SelectObject(bmpDC, oldBmp); + } + if (bmpDC) { + DeleteDC(bmpDC); + } } EndPaint(hwnd, &ps); @@ -114,24 +124,21 @@ static void ShowPopup(RefHoverState* s, Point screenPt) { int popupW = bmpSize.dx + 2 * kBorder; int popupH = bmpSize.dy + 2 * kBorder; - // Letterbox to max bounds. - if (popupW > kMaxPopupWidth) { - float scale = (float)(kMaxPopupWidth - 2 * kBorder) / (float)bmpSize.dx; - popupW = kMaxPopupWidth; - popupH = (int)(bmpSize.dy * scale) + 2 * kBorder; + HMONITOR hmon = MonitorFromPoint({screenPt.x, screenPt.y}, MONITOR_DEFAULTTONEAREST); + MONITORINFO mi{}; + mi.cbSize = sizeof(mi); + GetMonitorInfoW(hmon, &mi); + int monW = mi.rcWork.right - mi.rcWork.left; + int monH = mi.rcWork.bottom - mi.rcWork.top; + if (popupW > monW) { + popupW = monW; } - if (popupH > kMaxPopupHeight) { - float scale = (float)(kMaxPopupHeight - 2 * kBorder) / (float)bmpSize.dy; - popupH = kMaxPopupHeight; - popupW = (int)(bmpSize.dx * scale) + 2 * kBorder; + if (popupH > monH) { + popupH = monH; } int x = screenPt.x + 16; int y = screenPt.y + 16; - HMONITOR hmon = MonitorFromPoint({screenPt.x, screenPt.y}, MONITOR_DEFAULTTONEAREST); - MONITORINFO mi{}; - mi.cbSize = sizeof(mi); - GetMonitorInfoW(hmon, &mi); if (x + popupW > mi.rcWork.right) { x = screenPt.x - popupW - 4; } @@ -143,6 +150,63 @@ static void ShowPopup(RefHoverState* s, Point screenPt) { InvalidateRect(s->hwndPopup, nullptr, TRUE); } +// Re-render at the adjusted zoom; popup window keeps its initial size. +// The rendered region is sized to exactly fill the popup at the new zoom +// (anchored at the original detected top-left), so wheel-down brings in +// new page content from below/right and wheel-up shows less of the page +// at higher detail. Either way the popup stays full — no blank area. +// Returns true if the zoom actually changed. +bool RefHoverWheelZoom(RefHoverState* s, EngineBase* engine, int wheelDelta) { + if (!s || !s->hwndPopup || s->displayedDestPage <= 0 || !engine) { + return false; + } + float factor = (wheelDelta > 0) ? kUserZoomStep : (1.f / kUserZoomStep); + float newZoom = s->userZoom * factor; + if (newZoom < kMinUserZoom) { + newZoom = kMinUserZoom; + } else if (newZoom > kMaxUserZoom) { + newZoom = kMaxUserZoom; + } + if (newZoom == s->userZoom) { + return false; + } + s->userZoom = newZoom; + + RECT rc; + GetClientRect(s->hwndPopup, &rc); + float clientW = (float)((rc.right - rc.left) - 2 * kBorder); + float clientH = (float)((rc.bottom - rc.top) - 2 * kBorder); + float zoom = s->baseZoom * s->userZoom; + if (zoom <= 0.f || clientW <= 0.f || clientH <= 0.f) { + return false; + } + + RectF mediabox = engine->PageMediabox(s->displayedDestPage); + RectF region = s->lastRegion; + region.dx = clientW / zoom; + region.dy = clientH / zoom; + // Clamp to page bounds. + if (region.x + region.dx > mediabox.dx) { + region.dx = mediabox.dx - region.x; + } + if (region.y + region.dy > mediabox.dy) { + region.dy = mediabox.dy - region.y; + } + if (region.dx <= 0.f || region.dy <= 0.f) { + return false; + } + + RenderPageArgs args(s->displayedDestPage, zoom, 0, ®ion); + RenderedBitmap* bmp = engine->RenderPage(args); + if (!bmp) { + return false; + } + delete s->bmp; + s->bmp = bmp; + InvalidateRect(s->hwndPopup, nullptr, TRUE); + return true; +} + void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int destPage, float destX, float destY) { if (!s) { return; @@ -414,7 +478,7 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float return box; } -void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine) { +void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, float pageZoom) { KillTimer(hwndCanvas, kRefHoverTimerID); if (!s || !engine || s->pendingDestPage <= 0) { return; @@ -429,7 +493,24 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine) { } RectF region = DetectEntryBox(engine, destPage, destX, destY); - RenderPageArgs args(destPage, kRenderZoom, 0, ®ion); + // New destination — reset user-driven zoom. baseZoom matches the + // document's current display zoom for the destination page, so popup + // text height is comparable to the visible page text. If the region + // is too tall to fit at that zoom, shrink baseZoom by height only — + // we deliberately don't constrain by width (the popup may be wider + // than the default max bound; ShowPopup caps it at the monitor work + // area). + s->userZoom = 1.f; + float baseZoom = (pageZoom > 0.f) ? pageZoom : kRenderZoom; + float availH = (float)(kMaxPopupHeight - 2 * kBorder); + if (region.dy > 0.f && region.dy * baseZoom > availH) { + baseZoom = availH / region.dy; + } + if (baseZoom < kMinUserZoom) { + baseZoom = kMinUserZoom; + } + s->baseZoom = baseZoom; + RenderPageArgs args(destPage, s->baseZoom * s->userZoom, 0, ®ion); RenderedBitmap* bmp = engine->RenderPage(args); if (!bmp) { return; @@ -438,6 +519,7 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine) { delete s->bmp; s->bmp = bmp; s->displayedDestPage = destPage; + s->lastRegion = region; ShowPopup(s, s->pendingScreenPt); } diff --git a/src/RefHover.h b/src/RefHover.h index 4c8813915b6..f5bb7920802 100644 --- a/src/RefHover.h +++ b/src/RefHover.h @@ -15,6 +15,15 @@ struct RefHoverState { int pendingDestPage = -1; float pendingDestX = -1.f; float pendingDestY = -1.f; + + // re-render context, kept so mouse-wheel can zoom the popup without + // re-running detection. Reset on every new destination. + RectF lastRegion{}; + // baseZoom matches the document's current page zoom on first show so + // popup text height is comparable to page text. userZoom is the + // multiplier driven by the user's mouse-wheel. + float baseZoom = 1.f; + float userZoom = 1.f; }; constexpr int kRefHoverDelayMs = 300; @@ -24,4 +33,11 @@ RefHoverState* RefHoverCreate(HWND hwndCanvas); void RefHoverDestroy(RefHoverState* s); void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int destPage, float destX, float destY); void RefHoverHide(RefHoverState* s, HWND hwndCanvas); -void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine); +// pageZoom is the destination page's current display zoom (px-per-pt) — +// used as the initial render zoom so popup text height matches the page. +void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, float pageZoom); +// Re-render the popup at adjusted zoom in response to a mouse-wheel event. +// Popup window keeps its initial size; only the rendered content scales. +// Positive delta zooms in, negative zooms out. Returns true if the zoom +// changed and a re-render happened. +bool RefHoverWheelZoom(RefHoverState* s, EngineBase* engine, int wheelDelta); From 77b017f3c4c14810d93a141bf0e86cb5f794faa2 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 7 May 2026 16:35:45 +0200 Subject: [PATCH 03/21] Use fixed-ratio popup for non-reference hover targets Reviewer noted that internal links pointing to non-bibliography destinations (TOC entries, topbar goto targets, page anchors) produced ugly wide-thin popups because the entry-detection fallback returned a strip that ran from destX to the right page margin. Replace those fallback paths with a fixed-pt box anchored on the destination so every non-reference target gets the same popup shape. Bibliography entries that the detector can identify keep their existing (content-shaped) box. Co-Authored-By: Claude Opus 4.7 --- src/RefHover.cpp | 69 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 13 deletions(-) diff --git a/src/RefHover.cpp b/src/RefHover.cpp index 7e3b172f78d..c3dbd07a615 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -12,11 +12,15 @@ #define REF_HOVER_CLASS L"SumatraPDFRefHover" -// Fallback strip dimensions (used when text-based detection can't find boundaries). -static constexpr float kStripHeightPt = 130.f; // Larger fallback used when detection found something tiny (likely a figure // fragment) — gives enough height to capture a diagram + caption. static constexpr float kFigureStripHeightPt = 280.f; +// Non-reference targets (TOC, topbar links, generic cross-refs that don't +// resolve to a bibliography-shaped entry) get a fixed-ratio box anchored on +// the destination — keeps the popup visually consistent regardless of what +// happens to be near the goto target. +static constexpr float kNonRefBoxWidthPt = 360.f; +static constexpr float kNonRefBoxHeightPt = 260.f; static constexpr float kAnchorTopMarginPt = 6.f; static constexpr float kRightMarginPt = 4.f; // pt of padding around the detected entry box. @@ -235,10 +239,10 @@ void RefHoverHide(RefHoverState* s, HWND hwndCanvas) { } } -// Fallback box when text-based detection can't find boundaries: a strip from -// destX to the right page margin. heightPt controls how tall — small for the -// "no text near destY" case, larger for figures/diagrams. -static RectF FallbackBox(RectF mediabox, float destX, float destY, float heightPt) { +// Wide strip from destX to the right page margin — used only when detection +// found a small box that looks like a figure caption number, so that the +// popup captures the surrounding figure + caption. +static RectF FigureStripBox(RectF mediabox, float destX, float destY, float heightPt) { float lx = (destX >= 0.f) ? destX - 12.f : 0.f; if (lx < 0.f) { lx = 0.f; @@ -262,24 +266,63 @@ static RectF FallbackBox(RectF mediabox, float destX, float destY, float heightP return RectF{lx, ty, w, heightPt}; } +// Used when the link doesn't resolve to a bibliography-shaped entry (TOC +// targets, topbar links, image-only PDFs). Returns a fixed-pt box anchored +// near the destination so that all "non-reference" popups have the same +// shape regardless of what happens to be near the target. +static RectF FixedRatioBox(RectF mediabox, float destX, float destY) { + float w = kNonRefBoxWidthPt; + float h = kNonRefBoxHeightPt; + if (w > mediabox.dx) { + w = mediabox.dx; + } + if (h > mediabox.dy) { + h = mediabox.dy; + } + float lx = (destX >= 0.f) ? destX - 12.f : 0.f; + float ty = (destY >= 0.f) ? destY - kAnchorTopMarginPt : 0.f; + if (lx < 0.f) { + lx = 0.f; + } + if (ty < 0.f) { + ty = 0.f; + } + if (lx + w > mediabox.dx) { + lx = mediabox.dx - w; + } + if (ty + h > mediabox.dy) { + ty = mediabox.dy - h; + } + if (lx < 0.f) { + lx = 0.f; + } + if (ty < 0.f) { + ty = 0.f; + } + return RectF{lx, ty, w, h}; +} + // Find the bounding box of a single bibliography entry on the destination // page. Uses per-glyph text+coords from the engine's text cache: // 1. Locate the leftmost glyph with y in a small band around destY (entry start). // 2. Scan forward; stop at "[N" near the same left margin (next entry) or // a vertical paragraph gap. // 3. Return the min/max bounding box of glyphs in [start, end), padded. -// Falls back to FallbackBox() if no glyphs are near destY. +// Falls back to FixedRatioBox() if no glyphs are near destY (the link is not +// a bibliography reference — TOC / topbar / cross-ref). The fixed-ratio box +// gives every non-reference target the same popup shape, which avoids the +// thin wide popups produced when the detection latched onto a topbar row. static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float destY) { RectF mediabox = engine->PageMediabox(destPage); if (destY < 0.f) { - return FallbackBox(mediabox, destX, destY, kStripHeightPt); + return FixedRatioBox(mediabox, destX, destY); } int textLen = 0; Rect* coords = nullptr; const WCHAR* text = engine->GetTextForPage(destPage, &textLen, &coords); if (!text || textLen <= 0 || !coords) { - return FallbackBox(mediabox, destX, destY, kStripHeightPt); + return FixedRatioBox(mediabox, destX, destY); } int dY = (int)destY; @@ -314,7 +357,7 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float } } if (startIdx < 0) { - return FallbackBox(mediabox, destX, destY, kStripHeightPt); + return FixedRatioBox(mediabox, destX, destY); } int firstLineLeftX = coords[startIdx].x; @@ -447,7 +490,7 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float } } if (minX == INT_MAX) { - return FallbackBox(mediabox, destX, destY, kStripHeightPt); + return FixedRatioBox(mediabox, destX, destY); } RectF box{(float)minX - kEntryPadPt, (float)minY - kEntryPadPt, (float)(maxX - minX) + 2.f * kEntryPadPt, @@ -467,13 +510,13 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float box.dy = mediabox.dy - box.y; } if (box.dx < 50.f || box.dy < 20.f) { - return FallbackBox(mediabox, destX, destY, kStripHeightPt); + return FixedRatioBox(mediabox, destX, destY); } // If detection produced a small box (text-only scan got trapped in one // fragment of a figure / diagram), expand to a generous strip so the // surrounding visual content is included in the render. if (box.dx < 150.f && box.dy < 60.f) { - return FallbackBox(mediabox, destX, destY, kFigureStripHeightPt); + return FigureStripBox(mediabox, destX, destY, kFigureStripHeightPt); } return box; } From 577c69fd5b8f881b8ba30df5abed4a330f6834a1 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 8 May 2026 10:27:42 +0200 Subject: [PATCH 04/21] Loosen continuation-indent tolerance in citation detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The atIndentX check used a ±5pt tolerance to decide whether a new line was still part of the current bibliography entry. That's too tight when a continuation line switches from roman to italic (or vice versa) — the per-glyph bbox left edge shifts with the new side-bearings, even though both lines actually start at the same pen position. The result was that 3+ line entries with italic continuation (e.g. titles followed by an italicized journal/conference name on a new line) got truncated at the italic line's first glyph, with only the 6pt of padding leaking the top of that line into the popup. Bumping tolerance to ±25pt covers any realistic side-bearing variance while still catching truly external content (footers, author bios) which sit at a meaningfully different X. Co-Authored-By: Claude Opus 4.7 --- src/RefHover.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/RefHover.cpp b/src/RefHover.cpp index c3dbd07a615..ceadcef1305 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -412,7 +412,14 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float if (isNewLine && pastFirstLine && indentX < 0 && !atFirstLineLeftX) { indentX = r.x; } - bool atIndentX = (indentX > 0) && (r.x >= indentX - 5 && r.x <= indentX + 5); + // Tolerance is generous because continuation lines share the same pen + // position but per-glyph bbox left edge varies with side-bearings — + // a roman "i" and an italic "C" can differ by several pt even though + // both lines start at the same indent. Without this, multi-line + // entries that switch into italics on a continuation line get cut off + // (rule (d) below would otherwise terminate at that line's first + // glyph). + bool atIndentX = (indentX > 0) && (r.x >= indentX - 25 && r.x <= indentX + 25); // (a) "[N" at the entry's first-line X = next numeric entry. if (c == L'[' && atFirstLineLeftX && i + 1 < textLen && text[i + 1] >= L'0' && text[i + 1] <= L'9') { From 3a8facb041804ee92cc95be7bf304bc34f484e7a Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 8 May 2026 11:12:05 +0200 Subject: [PATCH 05/21] Drop overly-aggressive 'external content' termination rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removed the rule that terminated entry detection when a continuation line's first glyph X drifted from the captured indentX. Even with the recently bumped tolerance, font-metric variance on continuation lines (especially when one continuation line starts in italic and another in roman/digit) could still trigger premature termination, cutting off the last line of long bibliography entries. The remaining rules already cover all the realistic next-entry signals: - "[N" at firstLineLeftX (numeric bibs) - new line back at firstLineLeftX after a continuation (hanging-indent) - vertical paragraph break - single-line-entry case If a future PDF has external content following a bibliography entry without any of those signals, we'd over-include — but that's vanishingly rare and clearly preferable to silently truncating real entries. Co-Authored-By: Claude Opus 4.7 --- src/RefHover.cpp | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/src/RefHover.cpp b/src/RefHover.cpp index ceadcef1305..68f1f8b4ed9 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -412,14 +412,6 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float if (isNewLine && pastFirstLine && indentX < 0 && !atFirstLineLeftX) { indentX = r.x; } - // Tolerance is generous because continuation lines share the same pen - // position but per-glyph bbox left edge varies with side-bearings — - // a roman "i" and an italic "C" can differ by several pt even though - // both lines start at the same indent. Without this, multi-line - // entries that switch into italics on a continuation line get cut off - // (rule (d) below would otherwise terminate at that line's first - // glyph). - bool atIndentX = (indentX > 0) && (r.x >= indentX - 25 && r.x <= indentX + 25); // (a) "[N" at the entry's first-line X = next numeric entry. if (c == L'[' && atFirstLineLeftX && i + 1 < textLen && text[i + 1] >= L'0' && text[i + 1] <= L'9') { @@ -429,7 +421,8 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float // (b) Indent change: a new line back at the entry's first-line X // after a continuation line at a different X. Catches author-year - // hanging-indent bibliographies where there's no [N] marker. + // hanging-indent bibliographies where there's no [N] marker — this + // is the primary signal for the *next* entry's start. if (isNewLine && atFirstLineLeftX && pastFirstLine && prevLineLeftX != INT_MAX && (prevLineLeftX < firstLineLeftX - 5 || prevLineLeftX > firstLineLeftX + 5)) { endIdx = i; @@ -442,14 +435,7 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float break; } - // (d) Once we know indentX, a new line at any X other than firstLineLeftX - // or indentX is external content (author bios, footers, etc.) — end here. - if (isNewLine && pastFirstLine && indentX > 0 && !atFirstLineLeftX && !atIndentX) { - endIdx = i; - break; - } - - // (e) Single-line-entry case: a new line back at firstLineLeftX before + // (d) Single-line-entry case: a new line back at firstLineLeftX before // we discovered a continuation indent. The previous "entry" was one // line. Common pattern: stacked numbered footnotes "¹url\n²url\n³url". if (isNewLine && pastFirstLine && atFirstLineLeftX && indentX < 0 && prevLineLeftX != INT_MAX) { From 3d9e269264d2b8b3db673a1a7d10e960ea23dea9 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 8 May 2026 11:33:44 +0200 Subject: [PATCH 06/21] Use page-half landscape view for non-reference hover targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer noted that the small fixed-ratio popup (360×260pt) used for non-reference targets often shows too little context — e.g. a 'Table N' link surfaces only the caption text, not the table itself; a section- heading link shows only the heading; an image-only PDF link shows a narrow patch of art. Replace it with a landscape view: full page width × half page height, anchored at the destination. Capped by kMaxPopupWidth/Height (the latter already enforced; this commit also wires the width cap into the auto- fit). Also recognise single-line detected entries with no continuation indent as ambiguous (likely caption / heading rather than a real bibliography entry) and route them through the same landscape view. Co-Authored-By: Claude Opus 4.7 --- src/RefHover.cpp | 79 ++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 40 deletions(-) diff --git a/src/RefHover.cpp b/src/RefHover.cpp index 68f1f8b4ed9..4ddbeb76ca2 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -15,14 +15,12 @@ // Larger fallback used when detection found something tiny (likely a figure // fragment) — gives enough height to capture a diagram + caption. static constexpr float kFigureStripHeightPt = 280.f; -// Non-reference targets (TOC, topbar links, generic cross-refs that don't -// resolve to a bibliography-shaped entry) get a fixed-ratio box anchored on -// the destination — keeps the popup visually consistent regardless of what -// happens to be near the goto target. -static constexpr float kNonRefBoxWidthPt = 360.f; -static constexpr float kNonRefBoxHeightPt = 260.f; static constexpr float kAnchorTopMarginPt = 6.f; static constexpr float kRightMarginPt = 4.f; +// Single-line detected entries below this point-height are treated as +// ambiguous (likely a table caption or section header rather than a real +// bibliography entry) and rendered with the full-page landscape box. +static constexpr float kSingleLinePt = 30.f; // pt of padding around the detected entry box. static constexpr float kEntryPadPt = 6.f; // upper bound for the auto-fit base zoom. We render at min(kRenderZoom, @@ -266,36 +264,26 @@ static RectF FigureStripBox(RectF mediabox, float destX, float destY, float heig return RectF{lx, ty, w, heightPt}; } -// Used when the link doesn't resolve to a bibliography-shaped entry (TOC -// targets, topbar links, image-only PDFs). Returns a fixed-pt box anchored -// near the destination so that all "non-reference" popups have the same -// shape regardless of what happens to be near the target. -static RectF FixedRatioBox(RectF mediabox, float destX, float destY) { - float w = kNonRefBoxWidthPt; - float h = kNonRefBoxHeightPt; - if (w > mediabox.dx) { - w = mediabox.dx; - } +// Used when the link doesn't resolve to a recognizable bibliography entry — +// TOC targets, topbar/section links, table or figure captions, image-only +// PDFs. Returns a landscape view of the page (full page width × half page +// height) anchored at destY, so the popup shows enough surrounding context +// for the user to recognize where the link points (e.g. the table rows +// below a caption, the section content below a heading). +static RectF LandscapeBox(RectF mediabox, float destX, float destY) { + float w = mediabox.dx; + float h = mediabox.dy / 2.f; if (h > mediabox.dy) { h = mediabox.dy; } - float lx = (destX >= 0.f) ? destX - 12.f : 0.f; + float lx = 0.f; float ty = (destY >= 0.f) ? destY - kAnchorTopMarginPt : 0.f; - if (lx < 0.f) { - lx = 0.f; - } if (ty < 0.f) { ty = 0.f; } - if (lx + w > mediabox.dx) { - lx = mediabox.dx - w; - } if (ty + h > mediabox.dy) { ty = mediabox.dy - h; } - if (lx < 0.f) { - lx = 0.f; - } if (ty < 0.f) { ty = 0.f; } @@ -308,21 +296,21 @@ static RectF FixedRatioBox(RectF mediabox, float destX, float destY) { // 2. Scan forward; stop at "[N" near the same left margin (next entry) or // a vertical paragraph gap. // 3. Return the min/max bounding box of glyphs in [start, end), padded. -// Falls back to FixedRatioBox() if no glyphs are near destY (the link is not -// a bibliography reference — TOC / topbar / cross-ref). The fixed-ratio box -// gives every non-reference target the same popup shape, which avoids the -// thin wide popups produced when the detection latched onto a topbar row. +// Falls back to LandscapeBox() when the link is not a bibliography reference +// (TOC, topbar, cross-ref, table caption). The landscape box renders a half- +// page-tall slice of the page anchored on the destination so the user sees +// surrounding context (e.g. the table rows under a caption). static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float destY) { RectF mediabox = engine->PageMediabox(destPage); if (destY < 0.f) { - return FixedRatioBox(mediabox, destX, destY); + return LandscapeBox(mediabox, destX, destY); } int textLen = 0; Rect* coords = nullptr; const WCHAR* text = engine->GetTextForPage(destPage, &textLen, &coords); if (!text || textLen <= 0 || !coords) { - return FixedRatioBox(mediabox, destX, destY); + return LandscapeBox(mediabox, destX, destY); } int dY = (int)destY; @@ -357,7 +345,7 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float } } if (startIdx < 0) { - return FixedRatioBox(mediabox, destX, destY); + return LandscapeBox(mediabox, destX, destY); } int firstLineLeftX = coords[startIdx].x; @@ -483,7 +471,7 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float } } if (minX == INT_MAX) { - return FixedRatioBox(mediabox, destX, destY); + return LandscapeBox(mediabox, destX, destY); } RectF box{(float)minX - kEntryPadPt, (float)minY - kEntryPadPt, (float)(maxX - minX) + 2.f * kEntryPadPt, @@ -503,7 +491,15 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float box.dy = mediabox.dy - box.y; } if (box.dx < 50.f || box.dy < 20.f) { - return FixedRatioBox(mediabox, destX, destY); + return LandscapeBox(mediabox, destX, destY); + } + // Single-line detected entries with no continuation indent are usually + // not bibliography references — could be a table/figure caption, a + // section heading, or an in-text cross-ref destination. Show the + // landscape view so the user sees what's below the caption (the table, + // the section content, etc.) rather than just the caption text itself. + if (box.dy < kSingleLinePt && indentX < 0) { + return LandscapeBox(mediabox, destX, destY); } // If detection produced a small box (text-only scan got trapped in one // fragment of a figure / diagram), expand to a generous strip so the @@ -531,17 +527,20 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa RectF region = DetectEntryBox(engine, destPage, destX, destY); // New destination — reset user-driven zoom. baseZoom matches the // document's current display zoom for the destination page, so popup - // text height is comparable to the visible page text. If the region - // is too tall to fit at that zoom, shrink baseZoom by height only — - // we deliberately don't constrain by width (the popup may be wider - // than the default max bound; ShowPopup caps it at the monitor work - // area). + // text height is comparable to the visible page text. Shrink baseZoom + // if either dimension would exceed the popup max; landscape-style + // regions for non-reference targets are typically wider than tall, so + // the width cap matters here. s->userZoom = 1.f; float baseZoom = (pageZoom > 0.f) ? pageZoom : kRenderZoom; float availH = (float)(kMaxPopupHeight - 2 * kBorder); + float availW = (float)(kMaxPopupWidth - 2 * kBorder); if (region.dy > 0.f && region.dy * baseZoom > availH) { baseZoom = availH / region.dy; } + if (region.dx > 0.f && region.dx * baseZoom > availW) { + baseZoom = availW / region.dx; + } if (baseZoom < kMinUserZoom) { baseZoom = kMinUserZoom; } From 5c94e9cf2bcb7c2e7295aff38f7badd6e5509a04 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 8 May 2026 11:47:43 +0200 Subject: [PATCH 07/21] Keep fitted box for single-line description-list bib entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related fixes: 1. Relax rule (a): a "[" at firstLineLeftX is now treated as a next-entry marker regardless of what follows. Previously it required a digit ("[1]", "[2]"), which missed alphanumeric description-list styles like "[Nyg11]", "[Foo+09]", "[KAZ18]". 2. The "single-line + no indent → landscape view" heuristic was firing on single-line description-list bibliography entries, treating them as non-references and replacing the fitted entry box with a half-page landscape slice (which then included the next several entries below). Skip that redirect when the detected entry starts with "[" — that's a strong bibliography signal even on a one-line entry. The destY-at-page-top case (where the citation link's destination has no specific Y, just page-level) still falls back to the landscape view — that one needs source-text extraction on the source page to identify the citation key, which is a larger change. Co-Authored-By: Claude Opus 4.7 --- src/RefHover.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/RefHover.cpp b/src/RefHover.cpp index 4ddbeb76ca2..7185865899b 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -401,8 +401,11 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float indentX = r.x; } - // (a) "[N" at the entry's first-line X = next numeric entry. - if (c == L'[' && atFirstLineLeftX && i + 1 < textLen && text[i + 1] >= L'0' && text[i + 1] <= L'9') { + // (a) "[" at the entry's first-line X = next entry marker. Works for + // both numeric "[123]" and alphanumeric "[Foo+09]" / "[Bib05]" styles + // — body-text "[…]" can't trigger this because body sits at indentX, + // not firstLineLeftX. + if (c == L'[' && atFirstLineLeftX) { endIdx = i; break; } @@ -498,7 +501,10 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float // section heading, or an in-text cross-ref destination. Show the // landscape view so the user sees what's below the caption (the table, // the section content, etc.) rather than just the caption text itself. - if (box.dy < kSingleLinePt && indentX < 0) { + // Exception: an entry starting with "[" is a description-list-style + // citation marker ("[Nyg11]", "[1]", …) — those are real references + // even when single-line, keep their fitted box. + if (box.dy < kSingleLinePt && indentX < 0 && text[startIdx] != L'[') { return LandscapeBox(mediabox, destX, destY); } // If detection produced a small box (text-only scan got trapped in one From 5915fb57f838d325c4afa95b8813aefc755afea4 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 8 May 2026 22:58:58 +0200 Subject: [PATCH 08/21] Handle figures, headings, code listings, and abbreviation links in hover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reshape the citation-hover popup so it picks the right region for non- bibliography destinations: - "Figure / Table / Listing / Algorithm N.M" caption anywhere below the detected box: the destination is figure / table / listing body. Show landscape view so the popup includes the caption. Catches code/console listings whose lines start with "[TAG]" (linter output, etc.) that would otherwise short-circuit through the bracket-bib check. - Code-listing detector via brace/semicolon/paren density: figures whose body is example code route to landscape view. - Numbered / labeled headings ("6.2 Foo", "Figure 2.2 ...", "Section 7"): routed to landscape so the popup includes the heading + content below. - Tabular indent (continuation X far right of firstLineLeftX): routed to landscape to show the whole table. Description-list bibliography improvements: - Track a descListSibling flag on rule (a) "[" at firstLineLeftX, rule (b) indent-change-back, rule (c) vertical paragraph break with the next glyph at firstLineLeftX, and rule (d) single-line case. When set, the post-loop returns the fitted box even for single-line entries with no continuation indent — fixes abbreviation-list hovers (JVM, AKM, ADR, OMT, ...) where blank lines separate sibling entries. - Recover startIdx via leftmost-on-line walk after the y-band scan: a poorly-authored link destX can land startIdx mid-line on hanging- indent layouts, dropping the "[KOS06]" / "Philippe Kruchten" leading portion from the popup. Walking to the line's actual leftmost glyph recovers it. - Trim trailing blank page margin in LandscapeBox so the popup ends just below the last text glyph instead of including the full page footer. - Cap popup height by monitor work-area (~90%, max 1400px) and clamp popup position to the work-area edges so it doesn't get clipped at the top/left when the cursor is near a screen corner. Resolve page-level link destinations using the source link's text: - PageDestGetDestPoint returns {0,0,0,0} when the link has no specific anchor — common for body-text abbreviation / glossary links. Plumb the source page + source rect through RefHoverSchedule, then on destY <= 0 extract the longest alphanumeric run from the source rect (preferring "(XYZ)"-flanked tokens over bracketed citation keys) and search the destination page for it. The matching glyph's Y becomes destY, so DetectEntryBox can crop to the matching entry. Hovering "(AKM)" body text now yields just the AKM entry, not the whole abbreviations page. Re-render guard tightened: the popup short-circuit now also compares destX/destY, so two adjacent links that share a destination page but land at different positions (e.g. "Section 7" link + a bib ref both on page 41) each render their own content rather than reuse a stale popup. A manual-test checklist comment at the top of RefHover.cpp documents the case classes covered and the remaining known limits. Co-Authored-By: Claude Opus 4.7 --- src/Canvas.cpp | 5 +- src/RefHover.cpp | 549 ++++++++++++++++++++++++++++++++++++++++------- src/RefHover.h | 16 +- 3 files changed, 487 insertions(+), 83 deletions(-) diff --git a/src/Canvas.cpp b/src/Canvas.cpp index 1e10bc23d85..391a821fe9b 100644 --- a/src/Canvas.cpp +++ b/src/Canvas.cpp @@ -936,7 +936,10 @@ static void OnMouseMove(MainWindow* win, int x, int y, WPARAM) { RectF destPt = PageDestGetDestPoint(dest); Point screenPt = {x, y}; ClientToScreen(win->hwndCanvas, (POINT*)&screenPt); - RefHoverSchedule(win->refHover, win->hwndCanvas, screenPt, destPage, destPt.x, destPt.y); + int srcPage = el->GetPageNo(); + RectF srcRect = el->GetRect(); + RefHoverSchedule(win->refHover, win->hwndCanvas, screenPt, destPage, destPt.x, destPt.y, srcPage, + srcRect); } else if (win->refHover) { RefHoverHide(win->refHover, win->hwndCanvas); } diff --git a/src/RefHover.cpp b/src/RefHover.cpp index 7185865899b..5c8ed07ec3c 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -1,6 +1,59 @@ /* Copyright 2024 the SumatraPDF project authors (see AUTHORS file). License: GPLv3 */ +// Citation / reference hover — manual test checklist. +// Each item names a hover target and the popup behavior we want. +// Verify by hovering the described link and watching the popup. +// +// Bibliography / reference list: +// [ ] [Nyg11]-style description-list (numeric "[1]" or alphanumeric +// "[Foo+09]"): popup shows just the one entry, not adjacent ones. +// [ ] Author-year hanging-indent bib ("Smith, J. (2020). ..."): popup +// shows just the entry, including all wrapped continuation lines. +// [ ] Single-line author-year entry: same as above. +// [ ] Abbreviation / glossary list ("JVM Java Virtual Machine. 19, 36"): +// popup shows just the one entry, even when single-line with no +// bracket prefix. +// [ ] Numbered footnotes / endnotes: popup shows just the one entry. +// +// Caption / heading / table / code-listing destination: +// [ ] "Figure N.M" / "Table N.M" reference whose dest lands at the +// caption text: landscape view (full page width) with caption + +// figure body. +// [ ] "Figure N.M" reference whose dest lands at a code-listing body +// above the caption: landscape view — popup includes the caption +// below the code. +// [ ] "Section N.M" / "Chapter N" reference (numbered heading): +// landscape view, anchored at the heading down to the page text. +// [ ] Table reference (rows arranged in columns at the dest): landscape. +// [ ] Trailing blank page margin trimmed off the popup bottom (region +// ends just below the last text glyph, not at the page boundary). +// +// Positioning / interaction: +// [ ] Popup not clipped at screen edges when cursor is near a corner +// and the popup has to flip to the alternate side. +// [ ] Two adjacent links to the same destination page but different +// positions each render their own content (no stale popup on the +// second hover). +// [ ] Popup hides on mouse-out, re-appears on re-enter. +// [ ] Mouse-wheel over popup zooms in / out. +// [ ] Popup height grows on bigger monitors (capped at ~90% of work +// area, max 1400px). +// +// Page-level link destinations (no specific destY) — we extract the source +// link's text from its source rect and search the destination page for that +// text, using the leftmost match's Y as destY. This covers the common +// abbreviation / glossary case (hovering "AKM" in body text → popup crops +// to the AKM entry on the abbreviations page). +// +// Known limits: +// - Page-level link whose source rect doesn't isolate a unique key (e.g. +// a TOC line "1.2 Foo .... 12" — many such lines may share a prefix +// across the doc): falls back to full-page landscape view. +// - PDFs whose link destinations are authored at an unexpected Y (e.g. +// mid-paragraph): popup anchors there; this code can't fix a +// misauthored destination. + #include "utils/BaseUtil.h" #include "utils/WinUtil.h" @@ -12,15 +65,7 @@ #define REF_HOVER_CLASS L"SumatraPDFRefHover" -// Larger fallback used when detection found something tiny (likely a figure -// fragment) — gives enough height to capture a diagram + caption. -static constexpr float kFigureStripHeightPt = 280.f; static constexpr float kAnchorTopMarginPt = 6.f; -static constexpr float kRightMarginPt = 4.f; -// Single-line detected entries below this point-height are treated as -// ambiguous (likely a table caption or section header rather than a real -// bibliography entry) and rendered with the full-page landscape box. -static constexpr float kSingleLinePt = 30.f; // pt of padding around the detected entry box. static constexpr float kEntryPadPt = 6.f; // upper bound for the auto-fit base zoom. We render at min(kRenderZoom, @@ -147,6 +192,23 @@ static void ShowPopup(RefHoverState* s, Point screenPt) { if (y + popupH > mi.rcWork.bottom) { y = screenPt.y - popupH - 4; } + // Final clamp: when neither below-cursor nor above-cursor has enough + // room for the full popup (cursor near a screen edge and popup taller + // than half the work area), the alternate position can land off-screen. + // Pull the popup back into the work area; it may overlap the cursor + // but at least it won't be clipped at the top/left edge. + if (x < mi.rcWork.left) { + x = mi.rcWork.left; + } + if (y < mi.rcWork.top) { + y = mi.rcWork.top; + } + if (x + popupW > mi.rcWork.right) { + x = mi.rcWork.right - popupW; + } + if (y + popupH > mi.rcWork.bottom) { + y = mi.rcWork.bottom - popupH; + } SetWindowPos(s->hwndPopup, HWND_TOPMOST, x, y, popupW, popupH, SWP_NOACTIVATE | SWP_SHOWWINDOW); InvalidateRect(s->hwndPopup, nullptr, TRUE); @@ -209,19 +271,23 @@ bool RefHoverWheelZoom(RefHoverState* s, EngineBase* engine, int wheelDelta) { return true; } -void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int destPage, float destX, float destY) { +void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int destPage, float destX, float destY, + int srcPage, RectF srcRect) { if (!s) { return; } KillTimer(hwndCanvas, kRefHoverTimerID); - if (IsWindowVisible(s->hwndPopup) && s->displayedDestPage == destPage) { + if (IsWindowVisible(s->hwndPopup) && s->displayedDestPage == destPage && s->displayedDestX == destX && + s->displayedDestY == destY) { return; } s->pendingScreenPt = screenPt; s->pendingDestPage = destPage; s->pendingDestX = destX; s->pendingDestY = destY; + s->pendingSrcPage = srcPage; + s->pendingSrcRect = srcRect; SetTimer(hwndCanvas, kRefHoverTimerID, kRefHoverDelayMs, nullptr); } @@ -237,57 +303,53 @@ void RefHoverHide(RefHoverState* s, HWND hwndCanvas) { } } -// Wide strip from destX to the right page margin — used only when detection -// found a small box that looks like a figure caption number, so that the -// popup captures the surrounding figure + caption. -static RectF FigureStripBox(RectF mediabox, float destX, float destY, float heightPt) { - float lx = (destX >= 0.f) ? destX - 12.f : 0.f; - if (lx < 0.f) { - lx = 0.f; - } - float ty = (destY >= 0.f) ? destY - kAnchorTopMarginPt : 0.f; - if (ty < 0.f) { - ty = 0.f; - } - float w = mediabox.dx - lx - kRightMarginPt; - if (w < 100.f) { - lx = 0.f; - w = mediabox.dx; - } - if (ty + heightPt > mediabox.dy) { - heightPt = mediabox.dy - ty; - if (heightPt < 0.f) { - heightPt = mediabox.dy; - ty = 0.f; - } - } - return RectF{lx, ty, w, heightPt}; -} - // Used when the link doesn't resolve to a recognizable bibliography entry — // TOC targets, topbar/section links, table or figure captions, image-only -// PDFs. Returns a landscape view of the page (full page width × half page -// height) anchored at destY, so the popup shows enough surrounding context -// for the user to recognize where the link points (e.g. the table rows -// below a caption, the section content below a heading). -static RectF LandscapeBox(RectF mediabox, float destX, float destY) { - float w = mediabox.dx; - float h = mediabox.dy / 2.f; - if (h > mediabox.dy) { - h = mediabox.dy; - } - float lx = 0.f; +// PDFs. Returns a region that spans the full page width and goes from the +// destination Y down to the bottom of the last text glyph on the page — +// captures table caption / figure caption / section content below the +// link target without leaving a long blank margin at the popup bottom. +// Auto-fit in RefHoverOnTimer + the monitor-based popup height cap keep +// the popup a sensible size; the user can wheel-zoom in if text is too +// small. +static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* text, const Rect* coords, + int textLen) { float ty = (destY >= 0.f) ? destY - kAnchorTopMarginPt : 0.f; if (ty < 0.f) { ty = 0.f; } - if (ty + h > mediabox.dy) { - ty = mediabox.dy - h; - } - if (ty < 0.f) { + float h = mediabox.dy - ty; + if (h <= 0.f) { + h = mediabox.dy; ty = 0.f; } - return RectF{lx, ty, w, h}; + // Trim trailing blank margin: find the bottom of the last text glyph + // on the page (page numbers / footers count) and end the region just + // below it so the popup doesn't render a tall empty page footer. + if (text && coords && textLen > 0) { + int boxTop = (int)ty; + int boxBottom = (int)(ty + h); + int lastTextBottom = boxTop; + for (int i = 0; i < textLen; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + Rect r = coords[i]; + if (r.y < boxTop || r.y >= boxBottom) { + continue; + } + int glyphBottom = r.y + r.dy; + if (glyphBottom > lastTextBottom) { + lastTextBottom = glyphBottom; + } + } + float trimmedH = (float)lastTextBottom + kAnchorTopMarginPt - ty; + if (trimmedH > 20.f && trimmedH < h) { + h = trimmedH; + } + } + return RectF{0.f, ty, mediabox.dx, h}; } // Find the bounding box of a single bibliography entry on the destination @@ -302,15 +364,14 @@ static RectF LandscapeBox(RectF mediabox, float destX, float destY) { // surrounding context (e.g. the table rows under a caption). static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float destY) { RectF mediabox = engine->PageMediabox(destPage); - if (destY < 0.f) { - return LandscapeBox(mediabox, destX, destY); - } - int textLen = 0; Rect* coords = nullptr; const WCHAR* text = engine->GetTextForPage(destPage, &textLen, &coords); + if (destY < 0.f) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } if (!text || textLen <= 0 || !coords) { - return LandscapeBox(mediabox, destX, destY); + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); } int dY = (int)destY; @@ -345,7 +406,34 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float } } if (startIdx < 0) { - return LandscapeBox(mediabox, destX, destY); + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + + // PDF link destX is unreliable: poorly-authored links carry the source + // page's body-text X, not the destination-page entry-start X. That lands + // startIdx mid-line on hanging-indent description-list bibs, dropping + // the leading "[KOS06]" / "Philippe Kruchten" portion from the popup. + // Walk to the leftmost glyph on the same line as startIdx so the entry + // bounds always include the line's left edge. + { + int sy = coords[startIdx].y; + int leftmostX = coords[startIdx].x; + int leftmostIdx = startIdx; + for (int i = 0; i < textLen; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + Rect r = coords[i]; + if (r.y < sy - 3 || r.y > sy + 3) { + continue; + } + if (r.x < leftmostX) { + leftmostX = r.x; + leftmostIdx = i; + } + } + startIdx = leftmostIdx; } int firstLineLeftX = coords[startIdx].x; @@ -367,6 +455,12 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float int prevLineLeftX = INT_MAX; // X of the entry's continuation lines (captured from line 2). -1 = unknown. int indentX = -1; + // Set when we observe another sibling entry start at firstLineLeftX with + // no continuation indent in between — strong "this is a description list" + // signal (e.g. "JVM Java Virtual Machine. 19, 36" / "LLM Large Language + // Model. 45" abbreviation lists) that survives even when the current + // entry is a single line. + bool descListSibling = false; for (int i = startIdx + 1; i < textLen; i++) { WCHAR c = text[i]; @@ -406,6 +500,7 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float // — body-text "[…]" can't trigger this because body sits at indentX, // not firstLineLeftX. if (c == L'[' && atFirstLineLeftX) { + descListSibling = true; endIdx = i; break; } @@ -416,20 +511,30 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float // is the primary signal for the *next* entry's start. if (isNewLine && atFirstLineLeftX && pastFirstLine && prevLineLeftX != INT_MAX && (prevLineLeftX < firstLineLeftX - 5 || prevLineLeftX > firstLineLeftX + 5)) { + descListSibling = true; endIdx = i; break; } - // (c) Vertical paragraph break (no-indent style fallback). + // (c) Vertical paragraph break (no-indent style fallback). When the + // glyph that triggered the gap is back at firstLineLeftX, the gap + // is a blank line between description-list siblings (typical + // abbreviation lists where each entry is separated by extra + // vertical space) — treat as a sibling entry boundary. if (r.y > prevBottom + lineHeight * 5 / 4) { + if (atFirstLineLeftX) { + descListSibling = true; + } endIdx = i; break; } // (d) Single-line-entry case: a new line back at firstLineLeftX before // we discovered a continuation indent. The previous "entry" was one - // line. Common pattern: stacked numbered footnotes "¹url\n²url\n³url". + // line. Common pattern: stacked numbered footnotes "¹url\n²url\n³url" + // or abbreviation lists ("JVM Java Virtual Machine. 19, 36"). if (isNewLine && pastFirstLine && atFirstLineLeftX && indentX < 0 && prevLineLeftX != INT_MAX) { + descListSibling = true; endIdx = i; break; } @@ -474,7 +579,7 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float } } if (minX == INT_MAX) { - return LandscapeBox(mediabox, destX, destY); + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); } RectF box{(float)minX - kEntryPadPt, (float)minY - kEntryPadPt, (float)(maxX - minX) + 2.f * kEntryPadPt, @@ -494,28 +599,275 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float box.dy = mediabox.dy - box.y; } if (box.dx < 50.f || box.dy < 20.f) { - return LandscapeBox(mediabox, destX, destY); - } - // Single-line detected entries with no continuation indent are usually - // not bibliography references — could be a table/figure caption, a - // section heading, or an in-text cross-ref destination. Show the - // landscape view so the user sees what's below the caption (the table, - // the section content, etc.) rather than just the caption text itself. - // Exception: an entry starting with "[" is a description-list-style - // citation marker ("[Nyg11]", "[1]", …) — those are real references - // even when single-line, keep their fitted box. - if (box.dy < kSingleLinePt && indentX < 0 && text[startIdx] != L'[') { - return LandscapeBox(mediabox, destX, destY); - } - // If detection produced a small box (text-only scan got trapped in one - // fragment of a figure / diagram), expand to a generous strip so the - // surrounding visual content is included in the render. - if (box.dx < 150.f && box.dy < 60.f) { - return FigureStripBox(mediabox, destX, destY, kFigureStripHeightPt); + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + // "Figure N.M" / "Table N.M" / "Listing N.M" / "Algorithm N.M" caption + // anywhere below the detected box: the destination is a figure / table + // / listing body. Override all other heuristics so the popup uses the + // landscape view (caption included). Catches code/console listings + // where each line happens to start with "[TAG]" — those would otherwise + // be misclassified as description-list bibliography entries. + { + auto isCaptionLabelAt = [&](int idx) -> bool { + if (idx > 0) { + WCHAR prev = text[idx - 1]; + bool prevAlnum = (prev >= L'a' && prev <= L'z') || (prev >= L'A' && prev <= L'Z') || + (prev >= L'0' && prev <= L'9'); + if (prevAlnum) { + return false; + } + } + auto matchWord = [&](const WCHAR* w, int n) -> bool { + if (idx + n + 1 >= textLen) { + return false; + } + for (int j = 0; j < n; j++) { + WCHAR c = text[idx + j]; + if (c >= L'A' && c <= L'Z') { + c = (WCHAR)(c + 32); + } + if (c != w[j]) { + return false; + } + } + int k = idx + n; + while (k < textLen && (text[k] == L' ' || text[k] == L'\t')) { + k++; + } + return k < textLen && text[k] >= L'0' && text[k] <= L'9'; + }; + return matchWord(L"figure", 6) || matchWord(L"table", 5) || matchWord(L"listing", 7) || + matchWord(L"algorithm", 9); + }; + int boxBottomY = (int)(box.y + box.dy); + for (int i = 0; i < textLen; i++) { + if (coords[i].y <= boxBottomY) { + continue; + } + if (isCaptionLabelAt(i)) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + } + } + // Description-list bibliography ("[Smith2020]", "[1]", …) — unambiguous, + // keep the fitted box. + if (text[startIdx] == L'[') { + return box; + } + // Tabular layout: continuation X far right of firstLineLeftX is a + // column gap, not a hanging indent. Detection terminated at the first + // data row; show the landscape view so the user sees the full table. + if (indentX > 0 && (indentX - firstLineLeftX) > 80) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + // Section heading or caption-style label. Body paragraph below the + // heading has first-line indent, so detection captures heading + body + // line 1 and `indentX` lands in the same range as a hanging-indent bib. + // Use the entry's first character / first word to disambiguate: real + // bibliographies rarely start with a digit or with a label word like + // "Figure"/"Table"/"Section". Catches "6.2 Foo", "Figure 2.2: …", etc. + auto matchPrefix = [&](const WCHAR* word, int n) { + if (startIdx + n >= textLen) { + return false; + } + for (int j = 0; j < n; j++) { + WCHAR c = text[startIdx + j]; + if (c >= L'A' && c <= L'Z') { + c = (WCHAR)(c + 32); + } + if (c != word[j]) { + return false; + } + } + return true; + }; + WCHAR firstC = text[startIdx]; + bool digitStart = (firstC >= L'0' && firstC <= L'9'); + bool labelStart = matchPrefix(L"figure", 6) || matchPrefix(L"table", 5) || + matchPrefix(L"section", 7) || matchPrefix(L"chapter", 7) || + matchPrefix(L"algorithm", 9); + if (digitStart || labelStart) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + // Code-listing detector: a high density of braces / semicolons / parens + // within the detected box means the destination is most likely a code + // listing presented as a figure. Bibliography prose almost never has + // these characters at this density. Show the landscape view so the + // popup also includes the figure caption below the code. + { + int codeChars = 0; + int totalChars = 0; + for (int i = startIdx; i < endIdx; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + totalChars++; + if (c == L'{' || c == L'}' || c == L';' || c == L'(' || c == L')') { + codeChars++; + } + } + if (totalChars > 50 && codeChars * 12 > totalChars) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + } + // Description-list / glossary / footnote-style entry: rule (a) or (d) + // fired, meaning we saw a *sibling* entry start at firstLineLeftX. That + // is a strong "this is a list of entries" signal even when the current + // entry is a single line (abbreviations: "JVM Java Virtual Machine."). + if (descListSibling) { + return box; + } + // Single-line entry with no continuation indent and no sibling entry + // detected — caption / heading / in-text cross-ref destination. + if (box.dy < 30.f && indentX < 0) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); } + // Default: looks like a multi-line author-year bibliography entry, + // keep the fitted box. return box; } +// When the link's destination is page-level (destY < 0), try to recover a +// specific Y by reading the source link's text from srcPage at srcRect, then +// searching for that text on destPage. Returns -1 if no match. +// +// Typical case: hovering an abbreviation in body text (e.g. "AKM") whose link +// points to the abbreviations page without a specific Y. Source rect covers +// "AKM"; we look for "AKM" at the leftmost position on destPage (the +// description-list entry start) and return its Y. With a recovered destY, +// DetectEntryBox can crop the popup to the matching entry. +static float ResolveDestYFromSourceText(EngineBase* engine, int srcPage, RectF srcRect, int destPage) { + if (srcPage <= 0 || destPage <= 0 || srcRect.dx <= 0.f || srcRect.dy <= 0.f) { + return -1.f; + } + int srcLen = 0; + Rect* srcCoords = nullptr; + const WCHAR* srcText = engine->GetTextForPage(srcPage, &srcLen, &srcCoords); + if (!srcText || srcLen <= 0 || !srcCoords) { + return -1.f; + } + // Tight margin: include glyphs partially overlapping the rect (handles + // 1-2pt slop from tool-authored rects) without sweeping in adjacent + // body text. A wider margin would let the longest-alnum heuristic + // latch onto unrelated nearby words ("ADR" near "linking" → picks + // "linking" → no match on dest page → fall to landscape view). + int srcL = (int)srcRect.x - 2; + int srcT = (int)srcRect.y - 2; + int srcR = (int)(srcRect.x + srcRect.dx) + 2; + int srcB = (int)(srcRect.y + srcRect.dy) + 2; + + WCHAR rawText[512]; + int rawLen = 0; + for (int i = 0; i < srcLen && rawLen < 511; i++) { + Rect r = srcCoords[i]; + // Include glyph if its bounding box overlaps the search rect — more + // forgiving than a center-in-rect check when the link rect is + // tightly authored or slightly offset from the actual glyph. + if (r.x + r.dx < srcL || r.x > srcR) { + continue; + } + if (r.y + r.dy < srcT || r.y > srcB) { + continue; + } + rawText[rawLen++] = srcText[i]; + } + + auto isAlnum = [](WCHAR c) { + return (c >= L'a' && c <= L'z') || (c >= L'A' && c <= L'Z') || (c >= L'0' && c <= L'9'); + }; + + // Pick the best alphanumeric run from the source rect as the search + // needle. Strips surrounding punctuation ("(AKM)" → "AKM"). Tokens + // flanked by parentheses (the typical "definition" convention for + // expanding a phrase, e.g. "Architectural Knowledge Management (AKM)") + // win over equally / longer-but-non-flanked tokens — that keeps the + // resolver from latching onto a citation key like "KLV06" inside + // "[KLV06]" when both appear near the link rect. + int bestStart = -1; + int bestLen = 0; + bool bestIsParens = false; + int curStart = -1; + int curLen = 0; + for (int i = 0; i <= rawLen; i++) { + bool alnum = (i < rawLen) && isAlnum(rawText[i]); + if (alnum) { + if (curStart < 0) { + curStart = i; + } + curLen++; + } else { + if (curLen >= 2) { + bool flanked = (curStart > 0 && rawText[curStart - 1] == L'(' && i < rawLen && rawText[i] == L')'); + bool take = false; + if (flanked && !bestIsParens) { + take = true; + } else if (flanked == bestIsParens && curLen > bestLen) { + take = true; + } + if (take) { + bestStart = curStart; + bestLen = curLen; + bestIsParens = flanked; + } + } + curStart = -1; + curLen = 0; + } + } + if (bestLen < 2) { + return -1.f; + } + + int destLen = 0; + Rect* destCoords = nullptr; + const WCHAR* destText = engine->GetTextForPage(destPage, &destLen, &destCoords); + if (!destText || destLen <= 0 || !destCoords) { + return -1.f; + } + auto matchAt = [&](int idx) -> bool { + if (idx + bestLen > destLen) { + return false; + } + for (int j = 0; j < bestLen; j++) { + WCHAR a = destText[idx + j]; + WCHAR b = rawText[bestStart + j]; + if (a >= L'A' && a <= L'Z') { + a = (WCHAR)(a + 32); + } + if (b >= L'A' && b <= L'Z') { + b = (WCHAR)(b + 32); + } + if (a != b) { + return false; + } + } + if (idx > 0 && isAlnum(destText[idx - 1])) { + return false; + } + if (idx + bestLen < destLen && isAlnum(destText[idx + bestLen])) { + return false; + } + return true; + }; + + // Prefer the leftmost match — entry-list starts (e.g. "AKM" at the left + // margin of the abbreviations page) sit at the smallest X on the page. + int bestX = INT_MAX; + int bestY = -1; + for (int i = 0; i < destLen; i++) { + if (!matchAt(i)) { + continue; + } + Rect r = destCoords[i]; + if (r.x < bestX) { + bestX = r.x; + bestY = r.y; + } + } + return (bestY >= 0) ? (float)bestY : -1.f; +} + void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, float pageZoom) { KillTimer(hwndCanvas, kRefHoverTimerID); if (!s || !engine || s->pendingDestPage <= 0) { @@ -524,6 +876,20 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa int destPage = s->pendingDestPage; float destX = s->pendingDestX; float destY = s->pendingDestY; + // PageDestGetDestPoint returns {0,0,0,0} when the link has no specific + // anchor (page-level destination) — that's the typical case for body-text + // abbreviation / glossary links and for some TOC-derived bib refs. In + // those cases destY == 0 (not < 0), so we have to treat <= 0 as "no + // anchor" and try to recover a specific Y from the source link's text. + if (destY <= 0.f) { + float resolved = ResolveDestYFromSourceText(engine, s->pendingSrcPage, s->pendingSrcRect, destPage); + if (resolved >= 0.f) { + destY = resolved; + if (destX < 0.f) { + destX = 0.f; + } + } + } RectF mediabox = engine->PageMediabox(destPage); if (mediabox.dx <= 0.f || mediabox.dy <= 0.f) { @@ -539,7 +905,26 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa // the width cap matters here. s->userZoom = 1.f; float baseZoom = (pageZoom > 0.f) ? pageZoom : kRenderZoom; - float availH = (float)(kMaxPopupHeight - 2 * kBorder); + // Cap popup height by monitor work area so big screens get bigger + // popups (more of a long region — full table + caption etc. — fits). + int popupHCap = kMaxPopupHeight; + { + POINT mp = {s->pendingScreenPt.x, s->pendingScreenPt.y}; + HMONITOR hmon = MonitorFromPoint(mp, MONITOR_DEFAULTTONEAREST); + MONITORINFO mi{}; + mi.cbSize = sizeof(mi); + if (GetMonitorInfoW(hmon, &mi)) { + int monH = mi.rcWork.bottom - mi.rcWork.top; + int dyn = monH * 9 / 10; + if (dyn > popupHCap) { + popupHCap = dyn; + } + if (popupHCap > 1400) { + popupHCap = 1400; + } + } + } + float availH = (float)(popupHCap - 2 * kBorder); float availW = (float)(kMaxPopupWidth - 2 * kBorder); if (region.dy > 0.f && region.dy * baseZoom > availH) { baseZoom = availH / region.dy; @@ -560,6 +945,8 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa delete s->bmp; s->bmp = bmp; s->displayedDestPage = destPage; + s->displayedDestX = destX; + s->displayedDestY = destY; s->lastRegion = region; ShowPopup(s, s->pendingScreenPt); diff --git a/src/RefHover.h b/src/RefHover.h index f5bb7920802..057506276de 100644 --- a/src/RefHover.h +++ b/src/RefHover.h @@ -9,12 +9,25 @@ struct RefHoverState { // currently shown rendered destination strip (owned) RenderedBitmap* bmp = nullptr; int displayedDestPage = -1; + // Destination point used for the currently-displayed bitmap. Compared + // against incoming requests so adjacent links that target the same page + // but different positions (e.g. a section-7 link and a bib ref both + // landing on page 41) re-render rather than reuse the stale popup. + float displayedDestX = -1.f; + float displayedDestY = -1.f; // pending request: set by RefHoverSchedule, consumed by RefHoverOnTimer Point pendingScreenPt{}; int pendingDestPage = -1; float pendingDestX = -1.f; float pendingDestY = -1.f; + // Source link location, used to recover a more specific destY when the + // PDF link is page-level (destY < 0). We extract the source link's text + // from srcPage at srcRect and search for that text on destPage to find + // the matching entry's Y. Without this, page-level abbreviation / + // glossary links render the whole abbreviations page from top. + int pendingSrcPage = -1; + RectF pendingSrcRect{}; // re-render context, kept so mouse-wheel can zoom the popup without // re-running detection. Reset on every new destination. @@ -31,7 +44,8 @@ constexpr UINT_PTR kRefHoverTimerID = 9; RefHoverState* RefHoverCreate(HWND hwndCanvas); void RefHoverDestroy(RefHoverState* s); -void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int destPage, float destX, float destY); +void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int destPage, float destX, float destY, + int srcPage, RectF srcRect); void RefHoverHide(RefHoverState* s, HWND hwndCanvas); // pageZoom is the destination page's current display zoom (px-per-pt) — // used as the initial render zoom so popup text height matches the page. From 93ab2c64974e63b719d5bdca794aa42ce237193f Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 11 May 2026 10:56:23 +0200 Subject: [PATCH 09/21] Tighter popup geometry and figure/caption-aware region detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Popup positioning & sizing: - Center popup horizontally on the source page (using the source page's on-screen rect, plumbed through from Canvas.cpp via dm->GetPageInfo) so the popup expands symmetrically into gray margins rather than drifting toward one edge. - Width capped at ~95% of monitor work area; height capped at 45% of source page on screen (default) or 75% / spaceAbove-based cap for the taller figure-with-caption case so the figure body + caption both fit. - 30px gap above/below cursor (was 16) so 1-2 lines of context around the hovered word stay visible. - Popup never overlaps the cursor: prefer below, flip above on overflow; if neither side has room for the full popup, shrink popup height to the larger side rather than clamping into the cursor. - Vertical bounds clamp the popup to the source page screen rect, so it doesn't escape into the next-page area; horizontal bounds use the monitor work area so the popup can span beyond the page text column. LandscapeBox region & caption detection (used for figures, tables, listings, sections — anywhere DetectEntryBox falls back to a landscape view): - Cap base region to 200pt tall (was the destY → bottom-of-page span) so the popup is wide and short rather than narrow and tall. - Caption-extension: when a "Figure N.M" / "Table N.M" / "Listing N.M" / "Algorithm N.M" line-start is detected on the page (search range extended to bottom of page so tall figures with captions far below the initial cap still match), extend the region downward to include the full caption block. Logic moved out of DetectEntryBox into LandscapeBox itself, so it applies both when DetectEntryBox produces a fitted box AND when it falls through (image-only figures with no text at destY). - Caption end via line-by-line Y-range scan (stream order is unreliable for figure-float text) with a justification check: walk up to 3 lines from caption start; stop at any line whose right edge reaches within 30pt of the page text margin (= justified body paragraph). Avoids sweeping body text in when caption is followed by a no-parskip body paragraph (Figure 2.1 case) while still extending through the full 3-line caption when caption lines are raggedright (typical LaTeX). Co-Authored-By: Claude Opus 4.7 --- src/Canvas.cpp | 11 +- src/RefHover.cpp | 323 ++++++++++++++++++++++++++++++++++++++++------- src/RefHover.h | 6 +- 3 files changed, 292 insertions(+), 48 deletions(-) diff --git a/src/Canvas.cpp b/src/Canvas.cpp index 391a821fe9b..738e2d53f66 100644 --- a/src/Canvas.cpp +++ b/src/Canvas.cpp @@ -938,8 +938,17 @@ static void OnMouseMove(MainWindow* win, int x, int y, WPARAM) { ClientToScreen(win->hwndCanvas, (POINT*)&screenPt); int srcPage = el->GetPageNo(); RectF srcRect = el->GetRect(); + Rect pageScreenRect{}; + PageInfo* pi = (srcPage > 0) ? dm->GetPageInfo(srcPage) : nullptr; + if (pi && !pi->pageOnScreen.IsEmpty()) { + pageScreenRect = pi->pageOnScreen; + POINT topLeft = {pageScreenRect.x, pageScreenRect.y}; + ClientToScreen(win->hwndCanvas, &topLeft); + pageScreenRect.x = topLeft.x; + pageScreenRect.y = topLeft.y; + } RefHoverSchedule(win->refHover, win->hwndCanvas, screenPt, destPage, destPt.x, destPt.y, srcPage, - srcRect); + srcRect, pageScreenRect); } else if (win->refHover) { RefHoverHide(win->refHover, win->hwndCanvas); } diff --git a/src/RefHover.cpp b/src/RefHover.cpp index 5c8ed07ec3c..5dff7b79c5f 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -175,39 +175,79 @@ static void ShowPopup(RefHoverState* s, Point screenPt) { MONITORINFO mi{}; mi.cbSize = sizeof(mi); GetMonitorInfoW(hmon, &mi); - int monW = mi.rcWork.right - mi.rcWork.left; - int monH = mi.rcWork.bottom - mi.rcWork.top; - if (popupW > monW) { - popupW = monW; + + // Horizontal bounds: monitor work area (popup is allowed to extend + // into the gray margins beyond the page text column). + // Vertical bounds: monitor work area intersected with the page screen + // rect, so the popup stays within the visible page vertically and + // doesn't spill into the next-page area or the page header / footer + // gap above / below. + int leftBound = mi.rcWork.left; + int rightBound = mi.rcWork.right; + int topBound = mi.rcWork.top; + int bottomBound = mi.rcWork.bottom; + Rect pr = s->pendingPageScreenRect; + if (pr.dy > 0) { + if (pr.y > topBound) { + topBound = pr.y; + } + if (pr.y + pr.dy < bottomBound) { + bottomBound = pr.y + pr.dy; + } } - if (popupH > monH) { - popupH = monH; + int boundW = rightBound - leftBound; + int boundH = bottomBound - topBound; + if (popupW > boundW) { + popupW = boundW; } - - int x = screenPt.x + 16; - int y = screenPt.y + 16; - if (x + popupW > mi.rcWork.right) { - x = screenPt.x - popupW - 4; + if (popupH > boundH) { + popupH = boundH; } - if (y + popupH > mi.rcWork.bottom) { - y = screenPt.y - popupH - 4; + + // Horizontally: center the popup on the source page (not the canvas / + // monitor edge) so when the popup is wider than the page text column it + // expands symmetrically into the gray margins. The popup's X is + // independent of the cursor so consecutive hovers on the same page + // don't make the popup jump horizontally. + int pageCenterX = (pr.dx > 0) ? (pr.x + pr.dx / 2) : screenPt.x; + int x = pageCenterX - popupW / 2; + // Vertically: gap above/below the cursor so 1-2 lines of context around + // the hovered word stay visible. Prefer below cursor; flip above if + // overflow. If neither side fits the full popup, shrink popupH to + // whichever side has more room — popup is cut (bitmap clipped at popup + // edges in WM_PAINT) rather than overlapping the cursor. + constexpr int kCursorPad = 30; + int spaceBelow = bottomBound - (screenPt.y + kCursorPad); + int spaceAbove = (screenPt.y - kCursorPad) - topBound; + int y; + if (spaceBelow >= popupH) { + y = screenPt.y + kCursorPad; + } else if (spaceAbove >= popupH) { + y = screenPt.y - popupH - kCursorPad; + } else if (spaceBelow >= spaceAbove) { + if (spaceBelow > 0) { + popupH = spaceBelow; + } + y = screenPt.y + kCursorPad; + } else { + if (spaceAbove > 0) { + popupH = spaceAbove; + } + y = screenPt.y - popupH - kCursorPad; } - // Final clamp: when neither below-cursor nor above-cursor has enough - // room for the full popup (cursor near a screen edge and popup taller - // than half the work area), the alternate position can land off-screen. - // Pull the popup back into the work area; it may overlap the cursor - // but at least it won't be clipped at the top/left edge. - if (x < mi.rcWork.left) { - x = mi.rcWork.left; + // Horizontal clamp to monitor work area. + if (x < leftBound) { + x = leftBound; } - if (y < mi.rcWork.top) { - y = mi.rcWork.top; + if (x + popupW > rightBound) { + x = rightBound - popupW; } - if (x + popupW > mi.rcWork.right) { - x = mi.rcWork.right - popupW; + // Final vertical clamp (defensive). + if (y < topBound) { + y = topBound; } - if (y + popupH > mi.rcWork.bottom) { - y = mi.rcWork.bottom - popupH; + if (y + popupH > bottomBound) { + popupH = bottomBound - y; } SetWindowPos(s->hwndPopup, HWND_TOPMOST, x, y, popupW, popupH, SWP_NOACTIVATE | SWP_SHOWWINDOW); @@ -272,7 +312,7 @@ bool RefHoverWheelZoom(RefHoverState* s, EngineBase* engine, int wheelDelta) { } void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int destPage, float destX, float destY, - int srcPage, RectF srcRect) { + int srcPage, RectF srcRect, Rect pageScreenRect) { if (!s) { return; } @@ -288,6 +328,7 @@ void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int des s->pendingDestY = destY; s->pendingSrcPage = srcPage; s->pendingSrcRect = srcRect; + s->pendingPageScreenRect = pageScreenRect; SetTimer(hwndCanvas, kRefHoverTimerID, kRefHoverDelayMs, nullptr); } @@ -323,9 +364,135 @@ static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* h = mediabox.dy; ty = 0.f; } + // Cap to a focused region size so the popup is wide and short rather + // than narrow and tall. + constexpr float kMaxLandscapePt = 200.f; + if (h > kMaxLandscapePt) { + h = kMaxLandscapePt; + } + // Caption extension: if a "Figure N.M" / "Table N.M" / "Listing N.M" / + // "Algorithm N.M" caption appears within ~250pt below the capped region + // bottom (typical figure body height), extend the region downward to + // include the full caption block. Necessary for image-only figures + // where the figure body has no extractable text at destY — the caller + // falls to LandscapeBox without ever running the caption-aware + // DetectEntryBox path. + if (text && coords && textLen > 0) { + auto isCaptionAt = [&](int idx) -> bool { + if (idx > 0) { + WCHAR prev = text[idx - 1]; + bool prevAlnum = (prev >= L'a' && prev <= L'z') || (prev >= L'A' && prev <= L'Z') || + (prev >= L'0' && prev <= L'9'); + if (prevAlnum) { + return false; + } + } + auto matchWord = [&](const WCHAR* w, int n) -> bool { + if (idx + n + 1 >= textLen) { + return false; + } + for (int j = 0; j < n; j++) { + WCHAR c = text[idx + j]; + if (c >= L'A' && c <= L'Z') { + c = (WCHAR)(c + 32); + } + if (c != w[j]) { + return false; + } + } + int k = idx + n; + while (k < textLen && (text[k] == L' ' || text[k] == L'\t')) { + k++; + } + return k < textLen && text[k] >= L'0' && text[k] <= L'9'; + }; + return matchWord(L"figure", 6) || matchWord(L"table", 5) || matchWord(L"listing", 7) || + matchWord(L"algorithm", 9); + }; + // Search to end of page so tall figures with captions far below the + // initial 200pt cap still match. First "Figure N.M" line-start on the + // page below the cap wins — typically the relevant caption. + int searchTop = (int)(ty + h); + int searchBot = (int)mediabox.dy; + int capStartIdx = -1; + for (int i = 0; i < textLen; i++) { + if (coords[i].y < searchTop || coords[i].y > searchBot) { + continue; + } + if (isCaptionAt(i)) { + capStartIdx = i; + break; + } + } + if (capStartIdx >= 0) { + int capStartY = coords[capStartIdx].y; + int capLineH = coords[capStartIdx].dy; + if (capLineH < 10) { + capLineH = 12; + } + int lineSpacing = capLineH * 14 / 10; + if (lineSpacing < 14) { + lineSpacing = 14; + } + // Page right text margin: max right-X across all text glyphs on + // the page. Justified body lines (filling the text column) end + // within a few pt of this; LaTeX-style captions (raggedright) + // typically don't reach it. Used below to distinguish caption + // continuation from a justified body paragraph that follows. + int pageRightX = 0; + for (int j = 0; j < textLen; j++) { + int rx = coords[j].x + coords[j].dx; + if (rx > pageRightX) { + pageRightX = rx; + } + } + // Scan line by line from capStartY. Always accept line 0 + // (the caption start itself). For each subsequent line within + // capStartY + 3·lineSpacing, stop if its right edge reaches + // the page right margin (= justified body paragraph). Captions + // up to 3 lines are accepted as long as no line is justified. + int captionEndY = capStartY + capLineH; + for (int lineIdx = 0; lineIdx < 3; lineIdx++) { + int expectedY = capStartY + lineIdx * lineSpacing; + int rangeTop = expectedY - 3; + int rangeBot = expectedY + 3; + bool foundLine = false; + int lineBottomY = expectedY + capLineH; + int lineRightX = 0; + for (int j = 0; j < textLen; j++) { + int gy = coords[j].y; + if (gy < rangeTop || gy > rangeBot) { + continue; + } + foundLine = true; + int gb = gy + coords[j].dy; + if (gb > lineBottomY) { + lineBottomY = gb; + } + int rx = coords[j].x + coords[j].dx; + if (rx > lineRightX) { + lineRightX = rx; + } + } + if (!foundLine) { + break; + } + if (lineIdx > 0 && lineRightX > pageRightX - 30) { + // Justified body line — stop before extending region + // into the next paragraph. + break; + } + captionEndY = lineBottomY; + } + float extendedH = (float)captionEndY + kAnchorTopMarginPt - ty; + if (extendedH > h) { + h = extendedH; + } + } + } // Trim trailing blank margin: find the bottom of the last text glyph - // on the page (page numbers / footers count) and end the region just - // below it so the popup doesn't render a tall empty page footer. + // inside the candidate region and end the region just below it so the + // popup doesn't render an empty trailing margin. if (text && coords && textLen > 0) { int boxTop = (int)ty; int boxBottom = (int)(ty + h); @@ -645,6 +812,9 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float continue; } if (isCaptionLabelAt(i)) { + // Let LandscapeBox handle the caption-extension — it has a + // tighter, line-count-capped walk that doesn't sweep into + // following body paragraphs. return LandscapeBox(mediabox, destX, destY, text, coords, textLen); } } @@ -851,20 +1021,51 @@ static float ResolveDestYFromSourceText(EngineBase* engine, int srcPage, RectF s return true; }; - // Prefer the leftmost match — entry-list starts (e.g. "AKM" at the left - // margin of the abbreviations page) sit at the smallest X on the page. - int bestX = INT_MAX; - int bestY = -1; + // Prefer line-start matches (no other glyph at smaller x with same y) + // over mid-line matches. A "Figure 7.1" caption sits at line start; a + // body-text mention "in Figure 7.1 below" sits mid-line. Same logic + // benefits abbreviation entries vs. body mentions of an abbreviation. + auto isLineStartMatch = [&](int idx) -> bool { + int sy = destCoords[idx].y; + int sx = destCoords[idx].x; + for (int i = 0; i < destLen; i++) { + if (i == idx) { + continue; + } + if (destCoords[i].y != sy) { + continue; + } + WCHAR c = destText[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + if (destCoords[i].x < sx) { + return false; + } + } + return true; + }; + + int bestX_lineStart = INT_MAX; + int bestY_lineStart = -1; + int bestX_any = INT_MAX; + int bestY_any = -1; for (int i = 0; i < destLen; i++) { if (!matchAt(i)) { continue; } Rect r = destCoords[i]; - if (r.x < bestX) { - bestX = r.x; - bestY = r.y; + if (isLineStartMatch(i)) { + if (r.x < bestX_lineStart) { + bestX_lineStart = r.x; + bestY_lineStart = r.y; + } + } else if (r.x < bestX_any) { + bestX_any = r.x; + bestY_any = r.y; } } + int bestY = (bestY_lineStart >= 0) ? bestY_lineStart : bestY_any; return (bestY >= 0) ? (float)bestY : -1.f; } @@ -905,27 +1106,57 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa // the width cap matters here. s->userZoom = 1.f; float baseZoom = (pageZoom > 0.f) ? pageZoom : kRenderZoom; - // Cap popup height by monitor work area so big screens get bigger - // popups (more of a long region — full table + caption etc. — fits). - int popupHCap = kMaxPopupHeight; + // Popup max size: + // width ~95% of monitor work area — popup may span beyond the page + // text column into the surrounding gray margins so the + // rendered figure / caption / table is at a readable + // size, not shrunk to fit a narrow text column. + // height 45% of source page height — keeps the bottom of the page + // visible below the popup so the line under the cursor + // and surrounding context stay readable. + int popupWCap = kMaxPopupWidth; { POINT mp = {s->pendingScreenPt.x, s->pendingScreenPt.y}; HMONITOR hmon = MonitorFromPoint(mp, MONITOR_DEFAULTTONEAREST); MONITORINFO mi{}; mi.cbSize = sizeof(mi); if (GetMonitorInfoW(hmon, &mi)) { - int monH = mi.rcWork.bottom - mi.rcWork.top; - int dyn = monH * 9 / 10; - if (dyn > popupHCap) { - popupHCap = dyn; + int monW = mi.rcWork.right - mi.rcWork.left; + int dyn = monW * 95 / 100; + if (dyn > popupWCap) { + popupWCap = dyn; } - if (popupHCap > 1400) { - popupHCap = 1400; + } + } + // Default popup height cap: 45% of source page height, fall back to + // kMaxPopupHeight when page rect is unknown. For tall regions (figure + // with caption), grow the cap into whichever side of the cursor has + // more available space within the page rect, so the figure body and + // its full caption fit. Cursor at the bottom of the page → popup + // expands upward into the page top; cursor at top → expands downward. + int popupHCap; + if (region.dy > 250.f && s->pendingPageScreenRect.dy > 0) { + Rect pr = s->pendingPageScreenRect; + int curY = s->pendingScreenPt.y; + int spaceAbove = curY - pr.y - 30; + int spaceBelow = (pr.y + pr.dy) - curY - 30; + int maxSpace = (spaceAbove > spaceBelow) ? spaceAbove : spaceBelow; + if (maxSpace < 0) { + maxSpace = 0; + } + int pageBased = pr.dy * 75 / 100; + popupHCap = (pageBased > maxSpace) ? pageBased : maxSpace; + } else { + popupHCap = kMaxPopupHeight; + if (s->pendingPageScreenRect.dy > 0) { + int pageBased = s->pendingPageScreenRect.dy * 45 / 100; + if (pageBased < popupHCap) { + popupHCap = pageBased; } } } float availH = (float)(popupHCap - 2 * kBorder); - float availW = (float)(kMaxPopupWidth - 2 * kBorder); + float availW = (float)(popupWCap - 2 * kBorder); if (region.dy > 0.f && region.dy * baseZoom > availH) { baseZoom = availH / region.dy; } diff --git a/src/RefHover.h b/src/RefHover.h index 057506276de..299b6420ab9 100644 --- a/src/RefHover.h +++ b/src/RefHover.h @@ -28,6 +28,10 @@ struct RefHoverState { // glossary links render the whole abbreviations page from top. int pendingSrcPage = -1; RectF pendingSrcRect{}; + // Screen rect of the source page (visible portion). Used to clamp the + // popup so it stays within the document area and doesn't drift into + // the gray margins outside the page. + Rect pendingPageScreenRect{}; // re-render context, kept so mouse-wheel can zoom the popup without // re-running detection. Reset on every new destination. @@ -45,7 +49,7 @@ constexpr UINT_PTR kRefHoverTimerID = 9; RefHoverState* RefHoverCreate(HWND hwndCanvas); void RefHoverDestroy(RefHoverState* s); void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int destPage, float destX, float destY, - int srcPage, RectF srcRect); + int srcPage, RectF srcRect, Rect pageScreenRect); void RefHoverHide(RefHoverState* s, HWND hwndCanvas); // pageZoom is the destination page's current display zoom (px-per-pt) — // used as the initial render zoom so popup text height matches the page. From 576c23addb22c8a99c9e5792ef00d03c785d4e26 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 12 May 2026 16:44:11 +0200 Subject: [PATCH 10/21] Scroll popup with mouse wheel; Ctrl+wheel zooms Plain wheel over the citation popup now scrolls the rendered region, rolling over to the previous/next page when it crosses a page edge. Zoom moves to Ctrl+wheel. Also persist the popup-fitting region after a zoom so a subsequent scroll keeps filling the popup. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Canvas.cpp | 15 +++++-- src/RefHover.cpp | 111 +++++++++++++++++++++++++++++++++++++++++++---- src/RefHover.h | 6 +++ 3 files changed, 120 insertions(+), 12 deletions(-) diff --git a/src/Canvas.cpp b/src/Canvas.cpp index 738e2d53f66..7d8576324ae 100644 --- a/src/Canvas.cpp +++ b/src/Canvas.cpp @@ -2146,9 +2146,11 @@ static LRESULT CanvasOnMouseWheel(MainWindow* win, UINT msg, WPARAM wp, LPARAM l return res; } - // Wheel-zoom the citation-hover popup while the cursor is still on the - // citation link that opened it. Avoids moving the cursor onto the popup - // (which would dismiss the hover). + // Mouse-wheel on the citation-hover popup (cursor still on the citation + // link that opened it). Avoids moving the cursor onto the popup itself, + // which would dismiss the hover. + // plain wheel → scroll popup content (rolls over to prev/next page) + // ctrl+wheel → zoom popup content if (win->refHover && win->refHover->hwndPopup && IsWindowVisible(win->refHover->hwndPopup)) { DisplayModel* dmHover = win->AsFixed(); if (dmHover) { @@ -2156,7 +2158,12 @@ static LRESULT CanvasOnMouseWheel(MainWindow* win, UINT msg, WPARAM wp, LPARAM l IPageElement* elHover = dmHover->GetElementAtPos(pt, nullptr); if (IsCitationLink(elHover)) { short delta = GET_WHEEL_DELTA_WPARAM(wp); - RefHoverWheelZoom(win->refHover, dmHover->GetEngine(), delta); + bool isCtrl = (LOWORD(wp) & MK_CONTROL) || IsCtrlPressed(); + if (isCtrl) { + RefHoverWheelZoom(win->refHover, dmHover->GetEngine(), delta); + } else { + RefHoverWheelScroll(win->refHover, dmHover->GetEngine(), delta); + } return 0; } } diff --git a/src/RefHover.cpp b/src/RefHover.cpp index 5dff7b79c5f..1a8542309a9 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -36,7 +36,9 @@ // positions each render their own content (no stale popup on the // second hover). // [ ] Popup hides on mouse-out, re-appears on re-enter. -// [ ] Mouse-wheel over popup zooms in / out. +// [ ] Mouse-wheel over popup scrolls; rolls over to the prev / next page +// when the viewport reaches a page edge. +// [ ] Ctrl+mouse-wheel over popup zooms in / out. // [ ] Popup height grows on bigger monitors (capped at ~90% of work // area, max 1400px). // @@ -307,6 +309,100 @@ bool RefHoverWheelZoom(RefHoverState* s, EngineBase* engine, int wheelDelta) { } delete s->bmp; s->bmp = bmp; + // Persist the popup-fitting dx/dy so a subsequent scroll keeps rendering + // a bitmap that fills the popup. Without this, scroll would fall back to + // the original (smaller) entry-box dimensions and leave visible padding. + s->lastRegion = region; + InvalidateRect(s->hwndPopup, nullptr, TRUE); + return true; +} + +// Scroll the rendered region by one wheel notch. Rolls over to the previous +// or next page when the region would cross a page edge — the popup behaves +// like a small continuous-scroll viewport into the document. +bool RefHoverWheelScroll(RefHoverState* s, EngineBase* engine, int wheelDelta) { + if (!s || !s->hwndPopup || s->displayedDestPage <= 0 || !engine) { + return false; + } + float zoom = s->baseZoom * s->userZoom; + if (zoom <= 0.f) { + return false; + } + int pageCount = engine->PageCount(); + int page = s->displayedDestPage; + RectF region = s->lastRegion; + RectF mediabox = engine->PageMediabox(page); + if (mediabox.dx <= 0.f || mediabox.dy <= 0.f) { + return false; + } + + // ~60 screen pixels per WHEEL_DELTA notch, expressed in page points so the + // perceived scroll speed stays roughly constant across zoom levels. + // Positive wheelDelta → wheel forward → scroll toward earlier content + // (region.y decreases). Negative → later content (region.y increases). + float scrollPt = 60.f * ((float)wheelDelta / (float)WHEEL_DELTA) / zoom; + float newY = region.y - scrollPt; + + if (newY < 0.f) { + // Overflow off the page top — carry the remainder onto the previous + // page if there is one. Position the new region near the prev page's + // bottom and offset upward by the overflow so the seam between pages + // feels continuous across the wheel. + if (page > 1) { + float overflow = -newY; + page--; + mediabox = engine->PageMediabox(page); + newY = mediabox.dy - region.dy - overflow; + if (newY < 0.f) { + newY = 0.f; + } + } else { + newY = 0.f; + } + } else if (newY + region.dy > mediabox.dy) { + if (page < pageCount) { + float overflow = (newY + region.dy) - mediabox.dy; + page++; + mediabox = engine->PageMediabox(page); + newY = overflow; + if (newY + region.dy > mediabox.dy) { + newY = mediabox.dy - region.dy; + } + if (newY < 0.f) { + newY = 0.f; + } + } else { + newY = mediabox.dy - region.dy; + if (newY < 0.f) { + newY = 0.f; + } + } + } + + if (page == s->displayedDestPage && newY == region.y) { + return false; + } + region.y = newY; + if (region.dy > mediabox.dy) { + region.dy = mediabox.dy; + } + if (region.x + region.dx > mediabox.dx) { + region.x = mediabox.dx - region.dx; + if (region.x < 0.f) { + region.x = 0.f; + region.dx = mediabox.dx; + } + } + + RenderPageArgs args(page, zoom, 0, ®ion); + RenderedBitmap* bmp = engine->RenderPage(args); + if (!bmp) { + return false; + } + delete s->bmp; + s->bmp = bmp; + s->displayedDestPage = page; + s->lastRegion = region; InvalidateRect(s->hwndPopup, nullptr, TRUE); return true; } @@ -381,8 +477,8 @@ static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* auto isCaptionAt = [&](int idx) -> bool { if (idx > 0) { WCHAR prev = text[idx - 1]; - bool prevAlnum = (prev >= L'a' && prev <= L'z') || (prev >= L'A' && prev <= L'Z') || - (prev >= L'0' && prev <= L'9'); + bool prevAlnum = + (prev >= L'a' && prev <= L'z') || (prev >= L'A' && prev <= L'Z') || (prev >= L'0' && prev <= L'9'); if (prevAlnum) { return false; } @@ -778,8 +874,8 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float auto isCaptionLabelAt = [&](int idx) -> bool { if (idx > 0) { WCHAR prev = text[idx - 1]; - bool prevAlnum = (prev >= L'a' && prev <= L'z') || (prev >= L'A' && prev <= L'Z') || - (prev >= L'0' && prev <= L'9'); + bool prevAlnum = + (prev >= L'a' && prev <= L'z') || (prev >= L'A' && prev <= L'Z') || (prev >= L'0' && prev <= L'9'); if (prevAlnum) { return false; } @@ -853,9 +949,8 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float }; WCHAR firstC = text[startIdx]; bool digitStart = (firstC >= L'0' && firstC <= L'9'); - bool labelStart = matchPrefix(L"figure", 6) || matchPrefix(L"table", 5) || - matchPrefix(L"section", 7) || matchPrefix(L"chapter", 7) || - matchPrefix(L"algorithm", 9); + bool labelStart = matchPrefix(L"figure", 6) || matchPrefix(L"table", 5) || matchPrefix(L"section", 7) || + matchPrefix(L"chapter", 7) || matchPrefix(L"algorithm", 9); if (digitStart || labelStart) { return LandscapeBox(mediabox, destX, destY, text, coords, textLen); } diff --git a/src/RefHover.h b/src/RefHover.h index 299b6420ab9..05df1048647 100644 --- a/src/RefHover.h +++ b/src/RefHover.h @@ -59,3 +59,9 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa // Positive delta zooms in, negative zooms out. Returns true if the zoom // changed and a re-render happened. bool RefHoverWheelZoom(RefHoverState* s, EngineBase* engine, int wheelDelta); +// Scroll the popup's rendered region by a wheel notch. Positive delta scrolls +// toward earlier content (up); negative scrolls toward later content (down). +// Rolls over to the previous / next page when the viewport hits a page edge +// (continuous scrolling). Popup window keeps its initial size; only the +// rendered region's Y (and possibly page number) changes. +bool RefHoverWheelScroll(RefHoverState* s, EngineBase* engine, int wheelDelta); From eebf044d66344e06f552cb5767105c2c4e59365f Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 13 May 2026 07:40:25 +0200 Subject: [PATCH 11/21] Robust bracket-entry detection, German labels, figure-body popups Bracket-style bib entries ("[ZM12]"): compute the bounding box from a y-range bounded by the next "[" in the label column, independent of text-array order. Some PDFs draw labels and body text in non-monotonic order, which made the iterative scan terminate after the first line. Tightened the y-boundary by 6pt and widened the label-column search to +30pt so layouts with a page-number/section prefix before the label still register the next entry. German labels recognized everywhere caption / heading words are: added Abbildung, Tabelle, Listing, Abschnitt, Kapitel, Algorithmus. LandscapeBox extends upward by 250pt and uses a 360pt height cap when destY anchors on a caption line, so figure / table cross-references whose target lands at the caption include the figure body above, not just the caption plus the following paragraph. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/RefHover.cpp | 179 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 170 insertions(+), 9 deletions(-) diff --git a/src/RefHover.cpp b/src/RefHover.cpp index 1a8542309a9..6bd172d22c3 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -455,16 +455,78 @@ static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* if (ty < 0.f) { ty = 0.f; } + // When destY anchors at a "Figure N.M" / "Abbildung N.M" / "Table N.M" + // caption line, the figure / table *body* sits above the caption — but + // ty currently starts at the caption. Extend upward so the popup + // includes the figure body, not just the caption + the paragraph + // following it. + bool destAtCaption = false; + if (text && coords && textLen > 0 && destY > 0.f) { + auto isCaptionAt = [&](int idx) -> bool { + if (idx > 0) { + WCHAR prev = text[idx - 1]; + bool prevAlnum = + (prev >= L'a' && prev <= L'z') || (prev >= L'A' && prev <= L'Z') || (prev >= L'0' && prev <= L'9'); + if (prevAlnum) { + return false; + } + } + auto matchWord = [&](const WCHAR* w, int n) -> bool { + if (idx + n + 1 >= textLen) { + return false; + } + for (int j = 0; j < n; j++) { + WCHAR c = text[idx + j]; + if (c >= L'A' && c <= L'Z') { + c = (WCHAR)(c + 32); + } + if (c != w[j]) { + return false; + } + } + int k = idx + n; + while (k < textLen && (text[k] == L' ' || text[k] == L'\t')) { + k++; + } + return k < textLen && text[k] >= L'0' && text[k] <= L'9'; + }; + return matchWord(L"figure", 6) || matchWord(L"abbildung", 9) || matchWord(L"table", 5) || + matchWord(L"tabelle", 7) || matchWord(L"listing", 7) || matchWord(L"algorithm", 9) || + matchWord(L"algorithmus", 11); + }; + int dY = (int)destY; + for (int i = 0; i < textLen; i++) { + int gy = coords[i].y; + if (gy < dY - 5 || gy > dY + 15) { + continue; + } + if (isCaptionAt(i)) { + destAtCaption = true; + break; + } + } + } + if (destAtCaption) { + constexpr float kFigureBodyExtendPt = 250.f; + float newTy = ty - kFigureBodyExtendPt; + if (newTy < 0.f) { + newTy = 0.f; + } + ty = newTy; + } float h = mediabox.dy - ty; if (h <= 0.f) { h = mediabox.dy; ty = 0.f; } // Cap to a focused region size so the popup is wide and short rather - // than narrow and tall. + // than narrow and tall. Captions get a taller cap so the figure body + // above and the caption text below both fit. constexpr float kMaxLandscapePt = 200.f; - if (h > kMaxLandscapePt) { - h = kMaxLandscapePt; + constexpr float kMaxLandscapeCaptionPt = 360.f; + float maxLandscape = destAtCaption ? kMaxLandscapeCaptionPt : kMaxLandscapePt; + if (h > maxLandscape) { + h = maxLandscape; } // Caption extension: if a "Figure N.M" / "Table N.M" / "Listing N.M" / // "Algorithm N.M" caption appears within ~250pt below the capped region @@ -502,8 +564,9 @@ static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* } return k < textLen && text[k] >= L'0' && text[k] <= L'9'; }; - return matchWord(L"figure", 6) || matchWord(L"table", 5) || matchWord(L"listing", 7) || - matchWord(L"algorithm", 9); + return matchWord(L"figure", 6) || matchWord(L"abbildung", 9) || matchWord(L"table", 5) || + matchWord(L"tabelle", 7) || matchWord(L"listing", 7) || matchWord(L"algorithm", 9) || + matchWord(L"algorithmus", 11); }; // Search to end of page so tall figures with captions far below the // initial 200pt cap still match. First "Figure N.M" line-start on the @@ -706,6 +769,101 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float firstLineDy = 12; } + // Bracket-style entry ("[ZM12]", "[1]", …): build the bounding box from + // a y-range whose upper bound is the next "[" at firstLineLeftX. The + // iterative scan below depends on text-array order, but some PDFs draw + // labels and body in non-monotonic order — that made rule (a) terminate + // on a *later* entry's "[" appearing early in the text array, before our + // entry's body lines 2+. The y-range approach is order-independent. + if (text[startIdx] == L'[') { + int entryYBoundary = (int)mediabox.dy; + for (int i = 0; i < textLen; i++) { + if (i == startIdx) { + continue; + } + if (text[i] != L'[') { + continue; + } + Rect r = coords[i]; + // Accept "[" up to 30pt right of firstLineLeftX: some layouts + // prefix entries with a page number or section index (e.g. a + // "2" left of "[VB25]"), so the label "[" isn't exactly at + // firstLineLeftX. Body-text "[…]" sits at indentX (≥ ~60pt + // right of firstLineLeftX) so it's still excluded. + if (r.x < firstLineLeftX - 5 || r.x > firstLineLeftX + 30) { + continue; + } + if (r.y <= firstLineY + firstLineDy) { + continue; + } + if (r.y < entryYBoundary) { + entryYBoundary = r.y; + } + } + // Cap to a reasonable entry height so a last-on-page entry (no next + // "[") doesn't sweep the page footer / page number into the popup. + constexpr int kMaxBracketEntryPt = 250; + int capY = firstLineY + kMaxBracketEntryPt; + if (capY < entryYBoundary) { + entryYBoundary = capY; + } + // Pull the boundary up by ~half a line height so the next entry's + // first line — whose glyph tops can round to within 1–2 pt of the + // "[" we picked — is reliably excluded. + entryYBoundary -= 6; + int bMinX = INT_MAX, bMinY = INT_MAX, bMaxX = INT_MIN, bMaxY = INT_MIN; + for (int i = 0; i < textLen; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + Rect r = coords[i]; + if (r.x < firstLineLeftX - 20) { + continue; + } + if (r.y < firstLineY - 5) { + continue; + } + if (r.y >= entryYBoundary) { + continue; + } + if (r.x < bMinX) { + bMinX = r.x; + } + if (r.y < bMinY) { + bMinY = r.y; + } + if (r.x + r.dx > bMaxX) { + bMaxX = r.x + r.dx; + } + if (r.y + r.dy > bMaxY) { + bMaxY = r.y + r.dy; + } + } + if (bMinX != INT_MAX && (bMaxX - bMinX) >= 50 && (bMaxY - bMinY) >= 12) { + RectF box{(float)bMinX - kEntryPadPt, (float)bMinY - kEntryPadPt, + (float)(bMaxX - bMinX) + 2.f * kEntryPadPt, (float)(bMaxY - bMinY) + 2.f * kEntryPadPt}; + if (box.x < 0.f) { + box.dx += box.x; + box.x = 0.f; + } + if (box.y < 0.f) { + box.dy += box.y; + box.y = 0.f; + } + if (box.x + box.dx > mediabox.dx) { + box.dx = mediabox.dx - box.x; + } + if (box.y + box.dy > mediabox.dy) { + box.dy = mediabox.dy - box.y; + } + if (box.dx >= 50.f && box.dy >= 20.f) { + return box; + } + } + // Fall through to the iterative-scan logic on degenerate result. + } + // 2. Scan forward to find the end of the entry. int endIdx = textLen; int prevY = firstLineY; @@ -899,8 +1057,9 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float } return k < textLen && text[k] >= L'0' && text[k] <= L'9'; }; - return matchWord(L"figure", 6) || matchWord(L"table", 5) || matchWord(L"listing", 7) || - matchWord(L"algorithm", 9); + return matchWord(L"figure", 6) || matchWord(L"abbildung", 9) || matchWord(L"table", 5) || + matchWord(L"tabelle", 7) || matchWord(L"listing", 7) || matchWord(L"algorithm", 9) || + matchWord(L"algorithmus", 11); }; int boxBottomY = (int)(box.y + box.dy); for (int i = 0; i < textLen; i++) { @@ -949,8 +1108,10 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float }; WCHAR firstC = text[startIdx]; bool digitStart = (firstC >= L'0' && firstC <= L'9'); - bool labelStart = matchPrefix(L"figure", 6) || matchPrefix(L"table", 5) || matchPrefix(L"section", 7) || - matchPrefix(L"chapter", 7) || matchPrefix(L"algorithm", 9); + bool labelStart = matchPrefix(L"figure", 6) || matchPrefix(L"abbildung", 9) || matchPrefix(L"table", 5) || + matchPrefix(L"tabelle", 7) || matchPrefix(L"listing", 7) || matchPrefix(L"section", 7) || + matchPrefix(L"abschnitt", 9) || matchPrefix(L"chapter", 7) || matchPrefix(L"kapitel", 7) || + matchPrefix(L"algorithm", 9) || matchPrefix(L"algorithmus", 11); if (digitStart || labelStart) { return LandscapeBox(mediabox, destX, destY, text, coords, textLen); } From 929de360c018199cd69e3a1927177ea2c3b74919 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 13 May 2026 10:57:42 +0200 Subject: [PATCH 12/21] Hyphenated multi-line captions; bracket-label baseline tolerance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caption-extension: replace justification check with a gap + shortness dual-signal. The old "stop on any line reaching the right margin" rule truncated German captions like "...Verdecchia und Bo-/gner [VB25]..." because hyphenation made every caption line reach the column edge. Now: paragraph-break gap (> 70% line height) ends the caption; a short caption line followed by a justified line also ends it. Both common patterns (parskip-spaced captions and short-caption-then-body) are still covered. Bracket-entry detection: after the same-y walk-leftmost step, if the chosen start glyph still isn't "[", search for "[" within ~one line height of the start glyph at any smaller x. Catches description-list bibs where the "[VB25]" label sits on a slightly different baseline than its body line 1 — previously the popup rendered only a narrow strip from mid-body-text rather than the full entry. Co-Authored-By: Claude Opus 4.7 --- src/RefHover.cpp | 94 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 72 insertions(+), 22 deletions(-) diff --git a/src/RefHover.cpp b/src/RefHover.cpp index 6bd172d22c3..8514af526c3 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -589,15 +589,9 @@ static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* if (capLineH < 10) { capLineH = 12; } - int lineSpacing = capLineH * 14 / 10; - if (lineSpacing < 14) { - lineSpacing = 14; - } // Page right text margin: max right-X across all text glyphs on - // the page. Justified body lines (filling the text column) end - // within a few pt of this; LaTeX-style captions (raggedright) - // typically don't reach it. Used below to distinguish caption - // continuation from a justified body paragraph that follows. + // the page. A line reaching within ~30pt of pageRightX is at the + // column edge (justified body, or a hyphenated caption line). int pageRightX = 0; for (int j = 0; j < textLen; j++) { int rx = coords[j].x + coords[j].dx; @@ -605,25 +599,41 @@ static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* pageRightX = rx; } } - // Scan line by line from capStartY. Always accept line 0 - // (the caption start itself). For each subsequent line within - // capStartY + 3·lineSpacing, stop if its right edge reaches - // the page right margin (= justified body paragraph). Captions - // up to 3 lines are accepted as long as no line is justified. + // Walk subsequent lines below capStartY. Stop when we hit a + // paragraph break (vertical gap above inter-line leading) or + // a body-shape line. Two signals to detect body: + // 1) gap > ~70% of capLineH (parskip / float-separator) = + // new paragraph. + // 2) a "short" caption line seen earlier and the current line + // fills the column (raggedright-then-justified transition). + // Either signal alone catches a common case; together they cover + // hyphenated multi-line German captions (e.g. "...Bo-/gner...") + // where every caption line happens to reach the right margin. int captionEndY = capStartY + capLineH; + int prevLineBottom = capStartY + capLineH - 1; + bool seenShortLine = false; for (int lineIdx = 0; lineIdx < 3; lineIdx++) { - int expectedY = capStartY + lineIdx * lineSpacing; - int rangeTop = expectedY - 3; - int rangeBot = expectedY + 3; + int capTop, capBot; + if (lineIdx == 0) { + capTop = capStartY - 3; + capBot = capStartY + 3; + } else { + capTop = prevLineBottom + 1; + capBot = prevLineBottom + capLineH * 18 / 10; + } bool foundLine = false; - int lineBottomY = expectedY + capLineH; + int lineTopY = INT_MAX; + int lineBottomY = -1; int lineRightX = 0; for (int j = 0; j < textLen; j++) { int gy = coords[j].y; - if (gy < rangeTop || gy > rangeBot) { + if (gy < capTop || gy > capBot) { continue; } foundLine = true; + if (gy < lineTopY) { + lineTopY = gy; + } int gb = gy + coords[j].dy; if (gb > lineBottomY) { lineBottomY = gb; @@ -636,12 +646,21 @@ static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* if (!foundLine) { break; } - if (lineIdx > 0 && lineRightX > pageRightX - 30) { - // Justified body line — stop before extending region - // into the next paragraph. - break; + bool isShort = lineRightX < pageRightX - 30; + if (lineIdx >= 1) { + int gap = lineTopY - prevLineBottom; + if (gap > capLineH * 7 / 10) { + break; + } + if (!isShort && seenShortLine) { + break; + } } captionEndY = lineBottomY; + prevLineBottom = lineBottomY; + if (isShort) { + seenShortLine = true; + } } float extendedH = (float)captionEndY + kAnchorTopMarginPt - ty; if (extendedH > h) { @@ -762,6 +781,37 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float startIdx = leftmostIdx; } + // Tight-y walk above can miss a "[VB25]"-style label that sits on a + // slightly different baseline than its body line 1 (description-list + // layouts where label and body use different fonts/sizes). If the + // current leftmost still isn't a "[", search for one within roughly a + // line height of destY at any smaller x — that's the bracket label of + // the entry the link points at. + if (text[startIdx] != L'[') { + int sy = coords[startIdx].y; + int sDy = coords[startIdx].dy; + int yTol = sDy > 10 ? sDy : 10; + int bracketIdx = -1; + int bracketX = coords[startIdx].x; + for (int i = 0; i < textLen; i++) { + if (text[i] != L'[') { + continue; + } + Rect r = coords[i]; + if (r.y < sy - yTol || r.y > sy + yTol) { + continue; + } + if (r.x >= bracketX) { + continue; + } + bracketX = r.x; + bracketIdx = i; + } + if (bracketIdx >= 0) { + startIdx = bracketIdx; + } + } + int firstLineLeftX = coords[startIdx].x; int firstLineY = coords[startIdx].y; int firstLineDy = coords[startIdx].dy; From c76ea35d8caa0b8c1f449a8779f6df02586f6f37 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 14 May 2026 21:50:01 +0200 Subject: [PATCH 13/21] Handle malformed /XYZ dests and sparse-text dest pages in hover preview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes for image-heavy PDFs (e.g. Bluey.pdf "JUMP TO ALL (A-Z)" button, /Dest /allcharacters → /XYZ 0 -2.580017 0): - RefHoverOnTimer: treat destY past page bottom as page-level (destY=0) so the source-text resolver runs. PDF /XYZ y just past page bottom is a common authoring mistake when "top of page" was intended. - ResolveDestYFromSourceText: try every alnum candidate run in priority order (parens-flanked first, then length desc), first dest-page match wins. Previously only the longest run was tried, so "Jump to all (a-z)" picked "Jump" (not on dest) and gave up; "all" now matches "All Characters" on the dest page. - DetectEntryBox: when the dest page has very little text (< 50 chars, i.e. image-only with a heading), return the full page rectangle instead of fitting to the heading line — auto-fit in RefHoverOnTimer then scales the bitmap to popup limits. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/RefHover.cpp | 178 +++++++++++++++++++++++++++-------------------- 1 file changed, 104 insertions(+), 74 deletions(-) diff --git a/src/RefHover.cpp b/src/RefHover.cpp index 8514af526c3..a594ff6d2a3 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -712,10 +712,17 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float int textLen = 0; Rect* coords = nullptr; const WCHAR* text = engine->GetTextForPage(destPage, &textLen, &coords); - if (destY < 0.f) { - return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + // Sparse-text dest page (image-only or near-image-only — e.g. a + // children's PDF overview with character thumbnails plus a single + // heading). Fitting to the heading line gives a thin sliver and hides + // the actual content. Show the whole page so the user sees what they + // would navigate to; the auto-fit in RefHoverOnTimer scales the bitmap + // to popup limits. + constexpr int kSparsePageTextLen = 50; + if (!text || textLen < kSparsePageTextLen || !coords) { + return RectF{0.f, 0.f, mediabox.dx, mediabox.dy}; } - if (!text || textLen <= 0 || !coords) { + if (destY < 0.f) { return LandscapeBox(mediabox, destX, destY, text, coords, textLen); } @@ -1253,16 +1260,21 @@ static float ResolveDestYFromSourceText(EngineBase* engine, int srcPage, RectF s return (c >= L'a' && c <= L'z') || (c >= L'A' && c <= L'Z') || (c >= L'0' && c <= L'9'); }; - // Pick the best alphanumeric run from the source rect as the search - // needle. Strips surrounding punctuation ("(AKM)" → "AKM"). Tokens + // Collect alphanumeric candidate runs from the source rect as search + // needles. Strips surrounding punctuation ("(AKM)" → "AKM"). Tokens // flanked by parentheses (the typical "definition" convention for // expanding a phrase, e.g. "Architectural Knowledge Management (AKM)") - // win over equally / longer-but-non-flanked tokens — that keeps the - // resolver from latching onto a citation key like "KLV06" inside - // "[KLV06]" when both appear near the link rect. - int bestStart = -1; - int bestLen = 0; - bool bestIsParens = false; + // are preferred — that keeps the resolver from latching onto a citation + // key like "KLV06" inside "[KLV06]" when both appear near the link rect. + // Each candidate is tried against the dest page in priority order; + // first match wins. Trying multiple candidates handles cases where the + // longest run isn't on the dest page but a shorter run is (e.g. Bluey + // "Jump to all (a-z)" → "Jump" not on dest, "all" matches "All + // Characters"). + struct Cand { int start; int len; bool flanked; }; + constexpr int kMaxCands = 16; + Cand cands[kMaxCands]; + int ncands = 0; int curStart = -1; int curLen = 0; for (int i = 0; i <= rawLen; i++) { @@ -1273,27 +1285,33 @@ static float ResolveDestYFromSourceText(EngineBase* engine, int srcPage, RectF s } curLen++; } else { - if (curLen >= 2) { + if (curLen >= 2 && ncands < kMaxCands) { bool flanked = (curStart > 0 && rawText[curStart - 1] == L'(' && i < rawLen && rawText[i] == L')'); - bool take = false; - if (flanked && !bestIsParens) { - take = true; - } else if (flanked == bestIsParens && curLen > bestLen) { - take = true; - } - if (take) { - bestStart = curStart; - bestLen = curLen; - bestIsParens = flanked; - } + cands[ncands++] = {curStart, curLen, flanked}; } curStart = -1; curLen = 0; } } - if (bestLen < 2) { + if (ncands == 0) { return -1.f; } + // Sort: parens-flanked first, then by length descending. + for (int i = 0; i < ncands - 1; i++) { + for (int j = i + 1; j < ncands; j++) { + bool swap = false; + if (cands[j].flanked && !cands[i].flanked) { + swap = true; + } else if (cands[j].flanked == cands[i].flanked && cands[j].len > cands[i].len) { + swap = true; + } + if (swap) { + Cand t = cands[i]; + cands[i] = cands[j]; + cands[j] = t; + } + } + } int destLen = 0; Rect* destCoords = nullptr; @@ -1301,32 +1319,6 @@ static float ResolveDestYFromSourceText(EngineBase* engine, int srcPage, RectF s if (!destText || destLen <= 0 || !destCoords) { return -1.f; } - auto matchAt = [&](int idx) -> bool { - if (idx + bestLen > destLen) { - return false; - } - for (int j = 0; j < bestLen; j++) { - WCHAR a = destText[idx + j]; - WCHAR b = rawText[bestStart + j]; - if (a >= L'A' && a <= L'Z') { - a = (WCHAR)(a + 32); - } - if (b >= L'A' && b <= L'Z') { - b = (WCHAR)(b + 32); - } - if (a != b) { - return false; - } - } - if (idx > 0 && isAlnum(destText[idx - 1])) { - return false; - } - if (idx + bestLen < destLen && isAlnum(destText[idx + bestLen])) { - return false; - } - return true; - }; - // Prefer line-start matches (no other glyph at smaller x with same y) // over mid-line matches. A "Figure 7.1" caption sits at line start; a // body-text mention "in Figure 7.1 below" sits mid-line. Same logic @@ -1352,27 +1344,60 @@ static float ResolveDestYFromSourceText(EngineBase* engine, int srcPage, RectF s return true; }; - int bestX_lineStart = INT_MAX; - int bestY_lineStart = -1; - int bestX_any = INT_MAX; - int bestY_any = -1; - for (int i = 0; i < destLen; i++) { - if (!matchAt(i)) { - continue; + for (int ci = 0; ci < ncands; ci++) { + int bestStart = cands[ci].start; + int bestLen = cands[ci].len; + auto matchAt = [&](int idx) -> bool { + if (idx + bestLen > destLen) { + return false; + } + for (int j = 0; j < bestLen; j++) { + WCHAR a = destText[idx + j]; + WCHAR b = rawText[bestStart + j]; + if (a >= L'A' && a <= L'Z') { + a = (WCHAR)(a + 32); + } + if (b >= L'A' && b <= L'Z') { + b = (WCHAR)(b + 32); + } + if (a != b) { + return false; + } + } + if (idx > 0 && isAlnum(destText[idx - 1])) { + return false; + } + if (idx + bestLen < destLen && isAlnum(destText[idx + bestLen])) { + return false; + } + return true; + }; + + int bestX_lineStart = INT_MAX; + int bestY_lineStart = -1; + int bestX_any = INT_MAX; + int bestY_any = -1; + for (int i = 0; i < destLen; i++) { + if (!matchAt(i)) { + continue; + } + Rect r = destCoords[i]; + if (isLineStartMatch(i)) { + if (r.x < bestX_lineStart) { + bestX_lineStart = r.x; + bestY_lineStart = r.y; + } + } else if (r.x < bestX_any) { + bestX_any = r.x; + bestY_any = r.y; + } } - Rect r = destCoords[i]; - if (isLineStartMatch(i)) { - if (r.x < bestX_lineStart) { - bestX_lineStart = r.x; - bestY_lineStart = r.y; - } - } else if (r.x < bestX_any) { - bestX_any = r.x; - bestY_any = r.y; + int bestY = (bestY_lineStart >= 0) ? bestY_lineStart : bestY_any; + if (bestY >= 0) { + return (float)bestY; } } - int bestY = (bestY_lineStart >= 0) ? bestY_lineStart : bestY_any; - return (bestY >= 0) ? (float)bestY : -1.f; + return -1.f; } void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, float pageZoom) { @@ -1383,12 +1408,22 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa int destPage = s->pendingDestPage; float destX = s->pendingDestX; float destY = s->pendingDestY; + + RectF mediabox = engine->PageMediabox(destPage); + if (mediabox.dx <= 0.f || mediabox.dy <= 0.f) { + return; + } // PageDestGetDestPoint returns {0,0,0,0} when the link has no specific // anchor (page-level destination) — that's the typical case for body-text // abbreviation / glossary links and for some TOC-derived bib refs. In // those cases destY == 0 (not < 0), so we have to treat <= 0 as "no // anchor" and try to recover a specific Y from the source link's text. - if (destY <= 0.f) { + // Some PDFs author /XYZ with y just past the page bottom (negative in + // PDF user space, top-down flips it past mediabox.dy) when they mean + // "top of page" — e.g. Bluey.pdf "JUMP TO ALL (A-Z)" uses + // /XYZ 0 -2.58 0. Treat past-page-bottom destY the same as page-level. + if (destY <= 0.f || destY >= mediabox.dy - 1.f) { + destY = 0.f; float resolved = ResolveDestYFromSourceText(engine, s->pendingSrcPage, s->pendingSrcRect, destPage); if (resolved >= 0.f) { destY = resolved; @@ -1398,11 +1433,6 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa } } - RectF mediabox = engine->PageMediabox(destPage); - if (mediabox.dx <= 0.f || mediabox.dy <= 0.f) { - return; - } - RectF region = DetectEntryBox(engine, destPage, destX, destY); // New destination — reset user-driven zoom. baseZoom matches the // document's current display zoom for the destination page, so popup From bcdc9200fb6085108c9a0b3c200aad442372fb3b Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 14 May 2026 21:52:01 +0200 Subject: [PATCH 14/21] Fix copyright year in RefHover.* (2024 -> 2026) Co-Authored-By: Claude Opus 4.7 (1M context) --- src/RefHover.cpp | 2 +- src/RefHover.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/RefHover.cpp b/src/RefHover.cpp index a594ff6d2a3..6de9977b624 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -1,4 +1,4 @@ -/* Copyright 2024 the SumatraPDF project authors (see AUTHORS file). +/* Copyright 2026 the SumatraPDF project authors (see AUTHORS file). License: GPLv3 */ // Citation / reference hover — manual test checklist. diff --git a/src/RefHover.h b/src/RefHover.h index 05df1048647..cab4177f670 100644 --- a/src/RefHover.h +++ b/src/RefHover.h @@ -1,4 +1,4 @@ -/* Copyright 2024 the SumatraPDF project authors (see AUTHORS file). +/* Copyright 2026 the SumatraPDF project authors (see AUTHORS file). License: GPLv3 */ class EngineBase; From 400c37cb1bfb0f2b964dec249968d5f28a6f115f Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 26 May 2026 23:38:51 +0200 Subject: [PATCH 15/21] RefHover: suppress annotation tooltip when overlapping internal link AddLink.js-style PDFs overlay a FreeText annotation (the visible label) on top of a Link annotation (the clickable goto-target). The annotation tooltip "Free Text annotation. Ctrl+click to edit." was shown instead of the link preview, hiding the more useful information. Skip the annotation notification when an internal goto-link is at the same position; RefHover already prioritises kindPageElementDest in PickBestElement, so the link is what the user is really targeting. Rename IsCitationLink -> IsInternalLinkDest to match its actual scope (any internal-document link, not just citations). --- src/Canvas.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Canvas.cpp b/src/Canvas.cpp index 7d8576324ae..022a350802f 100644 --- a/src/Canvas.cpp +++ b/src/Canvas.cpp @@ -796,8 +796,8 @@ static bool gShowAnnotationNotification = true; static RectF CalculateResizedRect(MainWindow* win, int x, int y); // Returns true when el is an internal-document link (not an external URL or -// file launch). Used as a heuristic for "this is probably a citation link". -static bool IsCitationLink(IPageElement* el) { +// file launch). Such links are eligible for RefHover destination preview. +static bool IsInternalLinkDest(IPageElement* el) { if (!el || !el->Is(kindPageElementDest)) { return false; } @@ -894,13 +894,15 @@ static void OnMouseMove(MainWindow* win, int x, int y, WPARAM) { case MouseAction::None: { Annotation* annot = dm->GetAnnotationAtPos(pos, nullptr); Annotation* prev = win->annotationUnderCursor; + IPageElement* el = dm->GetElementAtPos(pos, nullptr); + bool hasInternalLink = IsInternalLinkDest(el); if (annot != prev) { #if 0 TempStr name = annot ? AnnotationReadableNameTemp(annot->type) : (TempStr) "none"; TempStr prevName = prev ? AnnotationReadableNameTemp(prev->type) : (TempStr) "none"; logf("different annot under cursor. prev: %s, new: %s\n", prevName, name); #endif - if (gShowAnnotationNotification) { + if (gShowAnnotationNotification && !hasInternalLink) { if (annot) { // auto r = annot->bounds; // logf("new pos: %d-%d, size: %d-%d\n", (int)r.x, (int)r.y, (int)r.dx, (int)r.dy); @@ -918,19 +920,18 @@ static void OnMouseMove(MainWindow* win, int x, int y, WPARAM) { } } } - if (!annot) { + if (!annot || hasInternalLink) { RemoveNotificationsForGroup(win->hwndCanvas, kNotifAnnotation); } win->annotationUnderCursor = annot; - // Citation hover: render the destination region of an internal - // link (typically the bibliography entry) into a popup. + // RefHover: render the destination region of an internal link + // (bibliography entry, glossary, generic goto-link) into a popup. if (gGlobalPrefs->enableCitationHover) { if (!win->refHover) { win->refHover = RefHoverCreate(win->hwndCanvas); } - IPageElement* el = dm->GetElementAtPos(pos, nullptr); - if (win->refHover && IsCitationLink(el)) { + if (win->refHover && hasInternalLink) { IPageDestination* dest = el->AsLink(); int destPage = PageDestGetPageNo(dest); RectF destPt = PageDestGetDestPoint(dest); @@ -2156,7 +2157,7 @@ static LRESULT CanvasOnMouseWheel(MainWindow* win, UINT msg, WPARAM wp, LPARAM l if (dmHover) { Point pt = HwndGetCursorPos(win->hwndCanvas); IPageElement* elHover = dmHover->GetElementAtPos(pt, nullptr); - if (IsCitationLink(elHover)) { + if (IsInternalLinkDest(elHover)) { short delta = GET_WHEEL_DELTA_WPARAM(wp); bool isCtrl = (LOWORD(wp) & MK_CONTROL) || IsCtrlPressed(); if (isCtrl) { From 239220efe5010e4b691c1e8b92d0834e99ae47df Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 May 2026 07:32:47 +0200 Subject: [PATCH 16/21] RefHover: honor /XYZ zoom hint from the link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hover preview previously ignored the link's /XYZ zoom factor and rendered the destination at the popup's auto-fit zoom. Clicking the same link navigates to the page at the requested zoom, so the preview should match. Plumb the link's zoom through EngineMupdf (via fz_resolve_link_dest) into PageDestinationMupdf::destZoom, expose it via GetZoom2(), and pass it from Canvas to RefHoverSchedule / RefHoverOnTimer. When the link supplies a zoom hint, render the destination region anchored at the link's destY at that zoom (clamped to at least the document's current display zoom, so the preview never feels smaller than the page itself). Skip the DetectEntryBox auto-fit heuristic in that case — the link author already said how much to enlarge. Use the full page width starting at x=0 rather than cropping at destX: strict /XYZ semantics chop the left-most letters of the target lines, which makes for a poor preview. --- src/Canvas.cpp | 5 ++-- src/EngineMupdf.cpp | 37 +++++++++++++++++++------ src/RefHover.cpp | 66 +++++++++++++++++++++++++++++++++++++++------ src/RefHover.h | 8 +++++- 4 files changed, 97 insertions(+), 19 deletions(-) diff --git a/src/Canvas.cpp b/src/Canvas.cpp index 022a350802f..34b395e2332 100644 --- a/src/Canvas.cpp +++ b/src/Canvas.cpp @@ -935,6 +935,7 @@ static void OnMouseMove(MainWindow* win, int x, int y, WPARAM) { IPageDestination* dest = el->AsLink(); int destPage = PageDestGetPageNo(dest); RectF destPt = PageDestGetDestPoint(dest); + float destZoom = PageDestGetZoom(dest); Point screenPt = {x, y}; ClientToScreen(win->hwndCanvas, (POINT*)&screenPt); int srcPage = el->GetPageNo(); @@ -948,8 +949,8 @@ static void OnMouseMove(MainWindow* win, int x, int y, WPARAM) { pageScreenRect.x = topLeft.x; pageScreenRect.y = topLeft.y; } - RefHoverSchedule(win->refHover, win->hwndCanvas, screenPt, destPage, destPt.x, destPt.y, srcPage, - srcRect, pageScreenRect); + RefHoverSchedule(win->refHover, win->hwndCanvas, screenPt, destPage, destPt.x, destPt.y, destZoom, + srcPage, srcRect, pageScreenRect); } else if (win->refHover) { RefHoverHide(win->refHover, win->hwndCanvas); } diff --git a/src/EngineMupdf.cpp b/src/EngineMupdf.cpp index fe60b2594c1..be93de63b23 100644 --- a/src/EngineMupdf.cpp +++ b/src/EngineMupdf.cpp @@ -96,6 +96,9 @@ struct PageDestinationMupdf : IPageDestination { // -1 means "not resolved" (e.g. external URL or file launch). float destX = -1.f; float destY = -1.f; + // /XYZ zoom level requested by the link (1.0 = 100%). 0 means + // "not specified" — caller should use document default. + float destZoom = 0.f; PageDestinationMupdf(fz_link* l, fz_outline* o) { // exactly one must be provided @@ -122,6 +125,11 @@ struct PageDestinationMupdf : IPageDestination { } return {}; } + + float GetZoom2() override { + return destZoom; + } + ~PageDestinationMupdf() override { str::Free(value); str::Free(name); @@ -160,27 +168,39 @@ static NO_INLINE RectF FzGetRectF(fz_link* link, fz_outline* outline) { return {}; } -static int ResolveLink(fz_context* ctx, fz_document* doc, const char* uri, float* xp, float* yp) { +static int ResolveLink(fz_context* ctx, fz_document* doc, const char* uri, float* xp, float* yp, + float* zoomp = nullptr) { if (!uri) { return -1; } int pageNo = -1; - fz_location loc; + fz_link_dest ldest{}; - fz_var(loc); + fz_var(ldest); fz_var(pageNo); fz_try(ctx) { - loc = fz_resolve_link(ctx, doc, uri, xp, yp); - pageNo = fz_page_number_from_location(ctx, doc, loc); + ldest = fz_resolve_link_dest(ctx, doc, uri); + pageNo = fz_page_number_from_location(ctx, doc, ldest.loc); } fz_catch(ctx) { - fz_warn(ctx, "fz_resolve_link failed"); + fz_warn(ctx, "fz_resolve_link_dest failed"); fz_report_error(ctx); pageNo = -1; } if (pageNo < 0) { return -1; } + if (xp) { + *xp = isnan(ldest.x) ? 0.f : ldest.x; + } + if (yp) { + *yp = isnan(ldest.y) ? 0.f : ldest.y; + } + if (zoomp) { + float z = isnan(ldest.zoom) ? 0.f : ldest.zoom; + // mupdf reports zoom as percentage (100 = 100%); we use 1.0 as 100%. + *zoomp = z / 100.f; + } return pageNo + 1; } @@ -239,11 +259,12 @@ static IPageDestination* NewPageDestinationMupdf(fz_context* ctx, fz_document* d auto dest = new PageDestinationMupdf(link, outline); dest->rect = FzGetRectF(link, outline); { - float x = 0, y = 0; + float x = 0, y = 0, z = 0; const char* destUri = link ? link->uri : (outline ? outline->uri : nullptr); - dest->pageNo = ResolveLink(ctx, doc, destUri, &x, &y); + dest->pageNo = ResolveLink(ctx, doc, destUri, &x, &y, &z); dest->destX = x; dest->destY = y; + dest->destZoom = z; } return dest; } diff --git a/src/RefHover.cpp b/src/RefHover.cpp index 6de9977b624..cc3ed5537bd 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -408,7 +408,7 @@ bool RefHoverWheelScroll(RefHoverState* s, EngineBase* engine, int wheelDelta) { } void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int destPage, float destX, float destY, - int srcPage, RectF srcRect, Rect pageScreenRect) { + float destZoom, int srcPage, RectF srcRect, Rect pageScreenRect) { if (!s) { return; } @@ -422,6 +422,7 @@ void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int des s->pendingDestPage = destPage; s->pendingDestX = destX; s->pendingDestY = destY; + s->pendingDestZoom = destZoom; s->pendingSrcPage = srcPage; s->pendingSrcRect = srcRect; s->pendingPageScreenRect = pageScreenRect; @@ -1433,7 +1434,32 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa } } - RectF region = DetectEntryBox(engine, destPage, destX, destY); + // When the link supplies an /XYZ zoom hint, honour it: render the + // destination region anchored at the link's (destX, destY) at the + // requested zoom rather than auto-fitting a detected entry box. + // This matches the navigation behaviour (DisplayModel::ScrollTo + // also reads the link zoom). + float linkZoom = s->pendingDestZoom; + bool useLinkZoom = (linkZoom > 0.f); + // Popup must not look smaller than the page does on screen — if the + // user is already viewing the document at a higher zoom than the + // link's /XYZ hint, render the popup at the current display zoom + // instead (still anchored at the link's top). Otherwise the preview + // would feel like a zoom-OUT, defeating the point of XYZ. + if (useLinkZoom && pageZoom > linkZoom) { + linkZoom = pageZoom; + } + + RectF region; + if (useLinkZoom) { + // Span full page width — strict /XYZ would crop at destX, but for + // a hover preview that just chops the left-most letters of the + // target lines. Top anchor (destY) is preserved. + // dx/dy placeholders; resized against popup caps below. + region = RectF{0.f, destY, mediabox.dx, mediabox.dy - destY}; + } else { + region = DetectEntryBox(engine, destPage, destX, destY); + } // New destination — reset user-driven zoom. baseZoom matches the // document's current display zoom for the destination page, so popup // text height is comparable to the visible page text. Shrink baseZoom @@ -1441,7 +1467,7 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa // regions for non-reference targets are typically wider than tall, so // the width cap matters here. s->userZoom = 1.f; - float baseZoom = (pageZoom > 0.f) ? pageZoom : kRenderZoom; + float baseZoom = useLinkZoom ? linkZoom : ((pageZoom > 0.f) ? pageZoom : kRenderZoom); // Popup max size: // width ~95% of monitor work area — popup may span beyond the page // text column into the surrounding gray margins so the @@ -1493,11 +1519,35 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa } float availH = (float)(popupHCap - 2 * kBorder); float availW = (float)(popupWCap - 2 * kBorder); - if (region.dy > 0.f && region.dy * baseZoom > availH) { - baseZoom = availH / region.dy; - } - if (region.dx > 0.f && region.dx * baseZoom > availW) { - baseZoom = availW / region.dx; + if (useLinkZoom) { + // Keep the link's requested zoom; size the region so it fills the + // popup without overflowing. Clamp to what's left of the page so + // we don't render past the mediabox edge. + float wantW = availW / baseZoom; + float wantH = availH / baseZoom; + float maxW = mediabox.dx - region.x; + float maxH = mediabox.dy - region.y; + if (wantW > maxW) { + wantW = maxW; + } + if (wantH > maxH) { + wantH = maxH; + } + if (wantW < 1.f) { + wantW = 1.f; + } + if (wantH < 1.f) { + wantH = 1.f; + } + region.dx = wantW; + region.dy = wantH; + } else { + if (region.dy > 0.f && region.dy * baseZoom > availH) { + baseZoom = availH / region.dy; + } + if (region.dx > 0.f && region.dx * baseZoom > availW) { + baseZoom = availW / region.dx; + } } if (baseZoom < kMinUserZoom) { baseZoom = kMinUserZoom; diff --git a/src/RefHover.h b/src/RefHover.h index cab4177f670..eca2a4c3c97 100644 --- a/src/RefHover.h +++ b/src/RefHover.h @@ -21,6 +21,12 @@ struct RefHoverState { int pendingDestPage = -1; float pendingDestX = -1.f; float pendingDestY = -1.f; + // /XYZ zoom hint from the link (1.0 = 100%). 0 means "no zoom hint"; + // RefHover then falls back to its auto-fit DetectEntryBox heuristic. + // When non-zero, the popup renders the destination region centred on + // (destX, destY) at this zoom — honouring the link author's intent + // (e.g. "goto top-left at 2x"). + float pendingDestZoom = 0.f; // Source link location, used to recover a more specific destY when the // PDF link is page-level (destY < 0). We extract the source link's text // from srcPage at srcRect and search for that text on destPage to find @@ -49,7 +55,7 @@ constexpr UINT_PTR kRefHoverTimerID = 9; RefHoverState* RefHoverCreate(HWND hwndCanvas); void RefHoverDestroy(RefHoverState* s); void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int destPage, float destX, float destY, - int srcPage, RectF srcRect, Rect pageScreenRect); + float destZoom, int srcPage, RectF srcRect, Rect pageScreenRect); void RefHoverHide(RefHoverState* s, HWND hwndCanvas); // pageZoom is the destination page's current display zoom (px-per-pt) — // used as the initial render zoom so popup text height matches the page. From af7c7fea72cc6dad96a1b3816b803cf75b94cf53 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 28 May 2026 08:33:05 +0200 Subject: [PATCH 17/21] RefHover: hide popup when cursor leaves canvas Register for WM_MOUSELEAVE while scheduling a RefHover preview and dismiss the popup on receipt, so the hover doesn't linger when the user moves to a tab strip or another application. Co-Authored-By: Claude Opus 4.7 --- src/Canvas.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Canvas.cpp b/src/Canvas.cpp index 34b395e2332..abf2f554ad8 100644 --- a/src/Canvas.cpp +++ b/src/Canvas.cpp @@ -932,6 +932,8 @@ static void OnMouseMove(MainWindow* win, int x, int y, WPARAM) { win->refHover = RefHoverCreate(win->hwndCanvas); } if (win->refHover && hasInternalLink) { + // request WM_MOUSELEAVE so popup hides when cursor leaves canvas + TrackMouseLeave(win->hwndCanvas); IPageDestination* dest = el->AsLink(); int destPage = PageDestGetPageNo(dest); RectF destPt = PageDestGetDestPoint(dest); @@ -2681,6 +2683,12 @@ static LRESULT WndProcCanvasFixedPageUI(MainWindow* win, HWND hwnd, UINT msg, WP OnMouseMove(win, x, y, wp); return 0; + case WM_MOUSELEAVE: + if (win->refHover) { + RefHoverHide(win->refHover, win->hwndCanvas); + } + return 0; + case WM_LBUTTONDOWN: OnMouseLeftButtonDown(win, x, y, wp); return 0; From a26c4c73f0daf1891a5ef57cc74c90886c8adc3d Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 28 May 2026 13:57:14 +0200 Subject: [PATCH 18/21] RefHover: tighter region for equation cross-refs; DRY helpers Add DetectEquationBox: when the destination's nearest line carries a trailing "(N)" or "(N.M)" label past the page's right half and nothing sits further right on that line, render only the equation row plus a small vertical padding instead of the 200pt landscape slice (which swept in the following paragraph and the next equation below). Also dedupe the popup region detectors: - Extract IsCaptionLabelAt(text, textLen, idx) from three identical copies in LandscapeBox and DetectEntryBox. - Extract ClipToMediabox(box, mediabox) from three near-identical RectF clamp blocks. - Use str::IsDigit / str::IsWs and std::abs in DetectEquationBox. Co-Authored-By: Claude Opus 4.7 --- src/RefHover.cpp | 306 +++++++++++++++++++++++++++-------------------- 1 file changed, 178 insertions(+), 128 deletions(-) diff --git a/src/RefHover.cpp b/src/RefHover.cpp index cc3ed5537bd..e6da3b4a1f4 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -85,6 +85,64 @@ static constexpr float kUserZoomStep = 1.15f; static bool gClassRegistered = false; +static bool IsAsciiAlnum(WCHAR c) { + return (c >= L'a' && c <= L'z') || (c >= L'A' && c <= L'Z') || (c >= L'0' && c <= L'9'); +} + +// True if `text[idx..]` starts a caption label like "Figure 1.2", "Table 3", +// "Listing 4.1", "Algorithm 5" (en/de). Matches case-insensitively and +// requires the previous glyph to be a word boundary and the word to be +// followed by whitespace and a digit. Used by the popup region detectors to +// recognize a figure/table/caption destination so the popup includes the +// figure body above the caption. +static bool IsCaptionLabelAt(const WCHAR* text, int textLen, int idx) { + if (idx > 0 && IsAsciiAlnum(text[idx - 1])) { + return false; + } + auto matchWord = [&](const WCHAR* w, int n) -> bool { + if (idx + n + 1 >= textLen) { + return false; + } + for (int j = 0; j < n; j++) { + WCHAR c = text[idx + j]; + if (c >= L'A' && c <= L'Z') { + c = (WCHAR)(c + 32); + } + if (c != w[j]) { + return false; + } + } + int k = idx + n; + while (k < textLen && (text[k] == L' ' || text[k] == L'\t')) { + k++; + } + return k < textLen && text[k] >= L'0' && text[k] <= L'9'; + }; + return matchWord(L"figure", 6) || matchWord(L"abbildung", 9) || matchWord(L"table", 5) || + matchWord(L"tabelle", 7) || matchWord(L"listing", 7) || matchWord(L"algorithm", 9) || + matchWord(L"algorithmus", 11); +} + +// Clip a region to the page mediabox: shifts a negative x/y to 0 (shrinking +// the box by the same amount) and trims any overhang past the right/bottom +// edge. Used everywhere a detected region must be passed to RenderPage. +static void ClipToMediabox(RectF& box, RectF mediabox) { + if (box.x < 0.f) { + box.dx += box.x; + box.x = 0.f; + } + if (box.y < 0.f) { + box.dy += box.y; + box.y = 0.f; + } + if (box.x + box.dx > mediabox.dx) { + box.dx = mediabox.dx - box.x; + } + if (box.y + box.dy > mediabox.dy) { + box.dy = mediabox.dy - box.y; + } +} + static LRESULT CALLBACK RefHoverWndProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) { if (msg == WM_PAINT) { PAINTSTRUCT ps; @@ -463,45 +521,13 @@ static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* // following it. bool destAtCaption = false; if (text && coords && textLen > 0 && destY > 0.f) { - auto isCaptionAt = [&](int idx) -> bool { - if (idx > 0) { - WCHAR prev = text[idx - 1]; - bool prevAlnum = - (prev >= L'a' && prev <= L'z') || (prev >= L'A' && prev <= L'Z') || (prev >= L'0' && prev <= L'9'); - if (prevAlnum) { - return false; - } - } - auto matchWord = [&](const WCHAR* w, int n) -> bool { - if (idx + n + 1 >= textLen) { - return false; - } - for (int j = 0; j < n; j++) { - WCHAR c = text[idx + j]; - if (c >= L'A' && c <= L'Z') { - c = (WCHAR)(c + 32); - } - if (c != w[j]) { - return false; - } - } - int k = idx + n; - while (k < textLen && (text[k] == L' ' || text[k] == L'\t')) { - k++; - } - return k < textLen && text[k] >= L'0' && text[k] <= L'9'; - }; - return matchWord(L"figure", 6) || matchWord(L"abbildung", 9) || matchWord(L"table", 5) || - matchWord(L"tabelle", 7) || matchWord(L"listing", 7) || matchWord(L"algorithm", 9) || - matchWord(L"algorithmus", 11); - }; int dY = (int)destY; for (int i = 0; i < textLen; i++) { int gy = coords[i].y; if (gy < dY - 5 || gy > dY + 15) { continue; } - if (isCaptionAt(i)) { + if (IsCaptionLabelAt(text, textLen, i)) { destAtCaption = true; break; } @@ -537,38 +563,6 @@ static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* // falls to LandscapeBox without ever running the caption-aware // DetectEntryBox path. if (text && coords && textLen > 0) { - auto isCaptionAt = [&](int idx) -> bool { - if (idx > 0) { - WCHAR prev = text[idx - 1]; - bool prevAlnum = - (prev >= L'a' && prev <= L'z') || (prev >= L'A' && prev <= L'Z') || (prev >= L'0' && prev <= L'9'); - if (prevAlnum) { - return false; - } - } - auto matchWord = [&](const WCHAR* w, int n) -> bool { - if (idx + n + 1 >= textLen) { - return false; - } - for (int j = 0; j < n; j++) { - WCHAR c = text[idx + j]; - if (c >= L'A' && c <= L'Z') { - c = (WCHAR)(c + 32); - } - if (c != w[j]) { - return false; - } - } - int k = idx + n; - while (k < textLen && (text[k] == L' ' || text[k] == L'\t')) { - k++; - } - return k < textLen && text[k] >= L'0' && text[k] <= L'9'; - }; - return matchWord(L"figure", 6) || matchWord(L"abbildung", 9) || matchWord(L"table", 5) || - matchWord(L"tabelle", 7) || matchWord(L"listing", 7) || matchWord(L"algorithm", 9) || - matchWord(L"algorithmus", 11); - }; // Search to end of page so tall figures with captions far below the // initial 200pt cap still match. First "Figure N.M" line-start on the // page below the cap wins — typically the relevant caption. @@ -579,7 +573,7 @@ static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* if (coords[i].y < searchTop || coords[i].y > searchBot) { continue; } - if (isCaptionAt(i)) { + if (IsCaptionLabelAt(text, textLen, i)) { capStartIdx = i; break; } @@ -698,6 +692,115 @@ static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* return RectF{0.f, ty, mediabox.dx, h}; } +// Detect a labelled display equation at (destX, destY): a "(N)" or "(N.M)" +// glyph cluster sitting near the right column edge on or near destY, with +// no other text further right on that line. Returns the equation's tight +// bounding box (full page width, ~one eq line tall) when found, empty rect +// otherwise. Used to avoid the landscape-style 200pt slice that sweeps in +// the paragraph and the next equation below an equation cross-reference. +static RectF DetectEquationBox(EngineBase* engine, int destPage, float destX, float destY) { + (void)destX; + RectF empty{}; + if (destY <= 0.f) { + return empty; + } + int textLen = 0; + Rect* coords = nullptr; + const WCHAR* text = engine->GetTextForPage(destPage, &textLen, &coords); + if (!text || textLen <= 0 || !coords) { + return empty; + } + RectF mediabox = engine->PageMediabox(destPage); + int dY = (int)destY; + + // Scan glyphs in a band around destY. Find a ')' whose right edge is the + // rightmost in its line, preceded by digits and an opening '('. + int bestLabelY = -1; + int bestLabelDy = 0; + int bestDist = INT_MAX; + for (int i = 0; i < textLen; i++) { + if (text[i] != L')') { + continue; + } + int ly = coords[i].y; + if (ly < dY - 40 || ly > dY + 40) { + continue; + } + // Walk backward through digits on the same line. + int p = i - 1; + int digits = 0; + while (p >= 0 && str::IsDigit(text[p]) && coords[p].y == ly) { + p--; + digits++; + } + if (digits == 0) { + continue; + } + // Optional ".M" form. + if (p >= 0 && text[p] == L'.' && coords[p].y == ly) { + p--; + int d2 = 0; + while (p >= 0 && str::IsDigit(text[p]) && coords[p].y == ly) { + p--; + d2++; + } + if (d2 == 0) { + continue; + } + } + if (p < 0 || text[p] != L'(' || coords[p].y != ly) { + continue; + } + int labelLeftX = coords[p].x; + int labelRightX = coords[i].x + coords[i].dx; + // Reject if any non-space glyph on the same line sits further right + // than the label — equation labels are line-trailing by construction. + bool hasRightOf = false; + for (int j = 0; j < textLen; j++) { + if (j >= p && j <= i) { + continue; + } + if (str::IsWs(text[j])) { + continue; + } + if (coords[j].y != ly) { + continue; + } + if (coords[j].x + coords[j].dx > labelRightX) { + hasRightOf = true; + break; + } + } + if (hasRightOf) { + continue; + } + // Reject if the label sits in the left half of the page (likely a + // body-text "(N)" footnote marker, not a display-eq label). + if (labelLeftX < (int)(mediabox.dx * 0.5f)) { + continue; + } + int dist = std::abs(ly - dY); + if (dist < bestDist) { + bestDist = dist; + bestLabelY = ly; + bestLabelDy = coords[i].dy; + } + } + if (bestLabelY < 0) { + return empty; + } + if (bestLabelDy <= 0) { + bestLabelDy = 12; + } + // Region: one eq line — labeled row + small vertical padding. Multi-row + // align environments are rare in cross-refs; a tight box is the right + // default and the user can wheel-scroll if context is needed. + float pad = (float)bestLabelDy + 6.f; + RectF box{0.f, (float)bestLabelY - pad, mediabox.dx, (float)bestLabelDy + 2.f * pad}; + ClipToMediabox(box, mediabox); + return box; +} + // Find the bounding box of a single bibliography entry on the destination // page. Uses per-glyph text+coords from the engine's text cache: // 1. Locate the leftmost glyph with y in a small band around destY (entry start). @@ -901,20 +1004,7 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float if (bMinX != INT_MAX && (bMaxX - bMinX) >= 50 && (bMaxY - bMinY) >= 12) { RectF box{(float)bMinX - kEntryPadPt, (float)bMinY - kEntryPadPt, (float)(bMaxX - bMinX) + 2.f * kEntryPadPt, (float)(bMaxY - bMinY) + 2.f * kEntryPadPt}; - if (box.x < 0.f) { - box.dx += box.x; - box.x = 0.f; - } - if (box.y < 0.f) { - box.dy += box.y; - box.y = 0.f; - } - if (box.x + box.dx > mediabox.dx) { - box.dx = mediabox.dx - box.x; - } - if (box.y + box.dy > mediabox.dy) { - box.dy = mediabox.dy - box.y; - } + ClipToMediabox(box, mediabox); if (box.dx >= 50.f && box.dy >= 20.f) { return box; } @@ -1063,20 +1153,7 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float RectF box{(float)minX - kEntryPadPt, (float)minY - kEntryPadPt, (float)(maxX - minX) + 2.f * kEntryPadPt, (float)(maxY - minY) + 2.f * kEntryPadPt}; - if (box.x < 0.f) { - box.dx += box.x; - box.x = 0.f; - } - if (box.y < 0.f) { - box.dy += box.y; - box.y = 0.f; - } - if (box.x + box.dx > mediabox.dx) { - box.dx = mediabox.dx - box.x; - } - if (box.y + box.dy > mediabox.dy) { - box.dy = mediabox.dy - box.y; - } + ClipToMediabox(box, mediabox); if (box.dx < 50.f || box.dy < 20.f) { return LandscapeBox(mediabox, destX, destY, text, coords, textLen); } @@ -1087,44 +1164,12 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float // where each line happens to start with "[TAG]" — those would otherwise // be misclassified as description-list bibliography entries. { - auto isCaptionLabelAt = [&](int idx) -> bool { - if (idx > 0) { - WCHAR prev = text[idx - 1]; - bool prevAlnum = - (prev >= L'a' && prev <= L'z') || (prev >= L'A' && prev <= L'Z') || (prev >= L'0' && prev <= L'9'); - if (prevAlnum) { - return false; - } - } - auto matchWord = [&](const WCHAR* w, int n) -> bool { - if (idx + n + 1 >= textLen) { - return false; - } - for (int j = 0; j < n; j++) { - WCHAR c = text[idx + j]; - if (c >= L'A' && c <= L'Z') { - c = (WCHAR)(c + 32); - } - if (c != w[j]) { - return false; - } - } - int k = idx + n; - while (k < textLen && (text[k] == L' ' || text[k] == L'\t')) { - k++; - } - return k < textLen && text[k] >= L'0' && text[k] <= L'9'; - }; - return matchWord(L"figure", 6) || matchWord(L"abbildung", 9) || matchWord(L"table", 5) || - matchWord(L"tabelle", 7) || matchWord(L"listing", 7) || matchWord(L"algorithm", 9) || - matchWord(L"algorithmus", 11); - }; int boxBottomY = (int)(box.y + box.dy); for (int i = 0; i < textLen; i++) { if (coords[i].y <= boxBottomY) { continue; } - if (isCaptionLabelAt(i)) { + if (IsCaptionLabelAt(text, textLen, i)) { // Let LandscapeBox handle the caption-extension — it has a // tighter, line-count-capped walk that doesn't sweep into // following body paragraphs. @@ -1458,7 +1503,12 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa // dx/dy placeholders; resized against popup caps below. region = RectF{0.f, destY, mediabox.dx, mediabox.dy - destY}; } else { - region = DetectEntryBox(engine, destPage, destX, destY); + // Equation cross-reference: tight box around the labelled line. + // Falls through to DetectEntryBox when no eq label is found. + region = DetectEquationBox(engine, destPage, destX, destY); + if (region.dx <= 0.f || region.dy <= 0.f) { + region = DetectEntryBox(engine, destPage, destX, destY); + } } // New destination — reset user-driven zoom. baseZoom matches the // document's current display zoom for the destination page, so popup From 5b661b8bef95ca6ef9289a27665d0fa1c7df6bfc Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 28 May 2026 14:24:16 +0200 Subject: [PATCH 19/21] RefHover: extract pure detectors and add unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move LandscapeBox / DetectEquationBox / DetectEntryBox out of RefHover.cpp into a new translation unit RefHoverDetect.{h,cpp}. The detectors now take the per-glyph text + coords arrays and mediabox directly instead of an EngineBase pointer, which makes them callable from test_util without pulling in the engine, HWND, or rendering layers. Add src/utils/tests/RefHover_ut.cpp with synthetic-glyph regression tests covering: - sparse-text page → whole page - destY < 0 fallback to LandscapeBox - bracket-style bib fits to one entry, excludes the next - equation label "(N)" at right column edge → tight band returned - body-text "(N)" in left half → rejected - non-line-trailing "(N)" → rejected - null / empty / negative-destY inputs handled without crash - LandscapeBox returned shape (full width, anchored, bounded height) Wire RefHoverTest() into the test_util alphabetical run order. Co-Authored-By: Claude Opus 4.7 --- src/RefHover.cpp | 827 +------------------------------- src/RefHoverDetect.cpp | 819 +++++++++++++++++++++++++++++++ src/RefHoverDetect.h | 32 ++ src/tools/test_util.cpp | 2 + src/utils/tests/RefHover_ut.cpp | 178 +++++++ vs2022/SumatraPDF-dll.vcxproj | 1 + vs2022/SumatraPDF.vcxproj | 1 + vs2022/test_util.vcxproj | 2 + 8 files changed, 1041 insertions(+), 821 deletions(-) create mode 100644 src/RefHoverDetect.cpp create mode 100644 src/RefHoverDetect.h create mode 100644 src/utils/tests/RefHover_ut.cpp diff --git a/src/RefHover.cpp b/src/RefHover.cpp index e6da3b4a1f4..954f9b8db87 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -64,12 +64,10 @@ #include "DocController.h" #include "EngineBase.h" #include "RefHover.h" +#include "RefHoverDetect.h" #define REF_HOVER_CLASS L"SumatraPDFRefHover" -static constexpr float kAnchorTopMarginPt = 6.f; -// pt of padding around the detected entry box. -static constexpr float kEntryPadPt = 6.f; // upper bound for the auto-fit base zoom. We render at min(kRenderZoom, // fit-to-popup-max), then multiply by RefHoverState::userZoom (the // mouse-wheel adjustment). @@ -85,64 +83,6 @@ static constexpr float kUserZoomStep = 1.15f; static bool gClassRegistered = false; -static bool IsAsciiAlnum(WCHAR c) { - return (c >= L'a' && c <= L'z') || (c >= L'A' && c <= L'Z') || (c >= L'0' && c <= L'9'); -} - -// True if `text[idx..]` starts a caption label like "Figure 1.2", "Table 3", -// "Listing 4.1", "Algorithm 5" (en/de). Matches case-insensitively and -// requires the previous glyph to be a word boundary and the word to be -// followed by whitespace and a digit. Used by the popup region detectors to -// recognize a figure/table/caption destination so the popup includes the -// figure body above the caption. -static bool IsCaptionLabelAt(const WCHAR* text, int textLen, int idx) { - if (idx > 0 && IsAsciiAlnum(text[idx - 1])) { - return false; - } - auto matchWord = [&](const WCHAR* w, int n) -> bool { - if (idx + n + 1 >= textLen) { - return false; - } - for (int j = 0; j < n; j++) { - WCHAR c = text[idx + j]; - if (c >= L'A' && c <= L'Z') { - c = (WCHAR)(c + 32); - } - if (c != w[j]) { - return false; - } - } - int k = idx + n; - while (k < textLen && (text[k] == L' ' || text[k] == L'\t')) { - k++; - } - return k < textLen && text[k] >= L'0' && text[k] <= L'9'; - }; - return matchWord(L"figure", 6) || matchWord(L"abbildung", 9) || matchWord(L"table", 5) || - matchWord(L"tabelle", 7) || matchWord(L"listing", 7) || matchWord(L"algorithm", 9) || - matchWord(L"algorithmus", 11); -} - -// Clip a region to the page mediabox: shifts a negative x/y to 0 (shrinking -// the box by the same amount) and trims any overhang past the right/bottom -// edge. Used everywhere a detected region must be passed to RenderPage. -static void ClipToMediabox(RectF& box, RectF mediabox) { - if (box.x < 0.f) { - box.dx += box.x; - box.x = 0.f; - } - if (box.y < 0.f) { - box.dy += box.y; - box.y = 0.f; - } - if (box.x + box.dx > mediabox.dx) { - box.dx = mediabox.dx - box.x; - } - if (box.y + box.dy > mediabox.dy) { - box.dy = mediabox.dy - box.y; - } -} - static LRESULT CALLBACK RefHoverWndProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) { if (msg == WM_PAINT) { PAINTSTRUCT ps; @@ -499,764 +439,6 @@ void RefHoverHide(RefHoverState* s, HWND hwndCanvas) { } } -// Used when the link doesn't resolve to a recognizable bibliography entry — -// TOC targets, topbar/section links, table or figure captions, image-only -// PDFs. Returns a region that spans the full page width and goes from the -// destination Y down to the bottom of the last text glyph on the page — -// captures table caption / figure caption / section content below the -// link target without leaving a long blank margin at the popup bottom. -// Auto-fit in RefHoverOnTimer + the monitor-based popup height cap keep -// the popup a sensible size; the user can wheel-zoom in if text is too -// small. -static RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* text, const Rect* coords, - int textLen) { - float ty = (destY >= 0.f) ? destY - kAnchorTopMarginPt : 0.f; - if (ty < 0.f) { - ty = 0.f; - } - // When destY anchors at a "Figure N.M" / "Abbildung N.M" / "Table N.M" - // caption line, the figure / table *body* sits above the caption — but - // ty currently starts at the caption. Extend upward so the popup - // includes the figure body, not just the caption + the paragraph - // following it. - bool destAtCaption = false; - if (text && coords && textLen > 0 && destY > 0.f) { - int dY = (int)destY; - for (int i = 0; i < textLen; i++) { - int gy = coords[i].y; - if (gy < dY - 5 || gy > dY + 15) { - continue; - } - if (IsCaptionLabelAt(text, textLen, i)) { - destAtCaption = true; - break; - } - } - } - if (destAtCaption) { - constexpr float kFigureBodyExtendPt = 250.f; - float newTy = ty - kFigureBodyExtendPt; - if (newTy < 0.f) { - newTy = 0.f; - } - ty = newTy; - } - float h = mediabox.dy - ty; - if (h <= 0.f) { - h = mediabox.dy; - ty = 0.f; - } - // Cap to a focused region size so the popup is wide and short rather - // than narrow and tall. Captions get a taller cap so the figure body - // above and the caption text below both fit. - constexpr float kMaxLandscapePt = 200.f; - constexpr float kMaxLandscapeCaptionPt = 360.f; - float maxLandscape = destAtCaption ? kMaxLandscapeCaptionPt : kMaxLandscapePt; - if (h > maxLandscape) { - h = maxLandscape; - } - // Caption extension: if a "Figure N.M" / "Table N.M" / "Listing N.M" / - // "Algorithm N.M" caption appears within ~250pt below the capped region - // bottom (typical figure body height), extend the region downward to - // include the full caption block. Necessary for image-only figures - // where the figure body has no extractable text at destY — the caller - // falls to LandscapeBox without ever running the caption-aware - // DetectEntryBox path. - if (text && coords && textLen > 0) { - // Search to end of page so tall figures with captions far below the - // initial 200pt cap still match. First "Figure N.M" line-start on the - // page below the cap wins — typically the relevant caption. - int searchTop = (int)(ty + h); - int searchBot = (int)mediabox.dy; - int capStartIdx = -1; - for (int i = 0; i < textLen; i++) { - if (coords[i].y < searchTop || coords[i].y > searchBot) { - continue; - } - if (IsCaptionLabelAt(text, textLen, i)) { - capStartIdx = i; - break; - } - } - if (capStartIdx >= 0) { - int capStartY = coords[capStartIdx].y; - int capLineH = coords[capStartIdx].dy; - if (capLineH < 10) { - capLineH = 12; - } - // Page right text margin: max right-X across all text glyphs on - // the page. A line reaching within ~30pt of pageRightX is at the - // column edge (justified body, or a hyphenated caption line). - int pageRightX = 0; - for (int j = 0; j < textLen; j++) { - int rx = coords[j].x + coords[j].dx; - if (rx > pageRightX) { - pageRightX = rx; - } - } - // Walk subsequent lines below capStartY. Stop when we hit a - // paragraph break (vertical gap above inter-line leading) or - // a body-shape line. Two signals to detect body: - // 1) gap > ~70% of capLineH (parskip / float-separator) = - // new paragraph. - // 2) a "short" caption line seen earlier and the current line - // fills the column (raggedright-then-justified transition). - // Either signal alone catches a common case; together they cover - // hyphenated multi-line German captions (e.g. "...Bo-/gner...") - // where every caption line happens to reach the right margin. - int captionEndY = capStartY + capLineH; - int prevLineBottom = capStartY + capLineH - 1; - bool seenShortLine = false; - for (int lineIdx = 0; lineIdx < 3; lineIdx++) { - int capTop, capBot; - if (lineIdx == 0) { - capTop = capStartY - 3; - capBot = capStartY + 3; - } else { - capTop = prevLineBottom + 1; - capBot = prevLineBottom + capLineH * 18 / 10; - } - bool foundLine = false; - int lineTopY = INT_MAX; - int lineBottomY = -1; - int lineRightX = 0; - for (int j = 0; j < textLen; j++) { - int gy = coords[j].y; - if (gy < capTop || gy > capBot) { - continue; - } - foundLine = true; - if (gy < lineTopY) { - lineTopY = gy; - } - int gb = gy + coords[j].dy; - if (gb > lineBottomY) { - lineBottomY = gb; - } - int rx = coords[j].x + coords[j].dx; - if (rx > lineRightX) { - lineRightX = rx; - } - } - if (!foundLine) { - break; - } - bool isShort = lineRightX < pageRightX - 30; - if (lineIdx >= 1) { - int gap = lineTopY - prevLineBottom; - if (gap > capLineH * 7 / 10) { - break; - } - if (!isShort && seenShortLine) { - break; - } - } - captionEndY = lineBottomY; - prevLineBottom = lineBottomY; - if (isShort) { - seenShortLine = true; - } - } - float extendedH = (float)captionEndY + kAnchorTopMarginPt - ty; - if (extendedH > h) { - h = extendedH; - } - } - } - // Trim trailing blank margin: find the bottom of the last text glyph - // inside the candidate region and end the region just below it so the - // popup doesn't render an empty trailing margin. - if (text && coords && textLen > 0) { - int boxTop = (int)ty; - int boxBottom = (int)(ty + h); - int lastTextBottom = boxTop; - for (int i = 0; i < textLen; i++) { - WCHAR c = text[i]; - if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { - continue; - } - Rect r = coords[i]; - if (r.y < boxTop || r.y >= boxBottom) { - continue; - } - int glyphBottom = r.y + r.dy; - if (glyphBottom > lastTextBottom) { - lastTextBottom = glyphBottom; - } - } - float trimmedH = (float)lastTextBottom + kAnchorTopMarginPt - ty; - if (trimmedH > 20.f && trimmedH < h) { - h = trimmedH; - } - } - return RectF{0.f, ty, mediabox.dx, h}; -} - -// Detect a labelled display equation at (destX, destY): a "(N)" or "(N.M)" -// glyph cluster sitting near the right column edge on or near destY, with -// no other text further right on that line. Returns the equation's tight -// bounding box (full page width, ~one eq line tall) when found, empty rect -// otherwise. Used to avoid the landscape-style 200pt slice that sweeps in -// the paragraph and the next equation below an equation cross-reference. -static RectF DetectEquationBox(EngineBase* engine, int destPage, float destX, float destY) { - (void)destX; - RectF empty{}; - if (destY <= 0.f) { - return empty; - } - int textLen = 0; - Rect* coords = nullptr; - const WCHAR* text = engine->GetTextForPage(destPage, &textLen, &coords); - if (!text || textLen <= 0 || !coords) { - return empty; - } - RectF mediabox = engine->PageMediabox(destPage); - int dY = (int)destY; - - // Scan glyphs in a band around destY. Find a ')' whose right edge is the - // rightmost in its line, preceded by digits and an opening '('. - int bestLabelY = -1; - int bestLabelDy = 0; - int bestDist = INT_MAX; - for (int i = 0; i < textLen; i++) { - if (text[i] != L')') { - continue; - } - int ly = coords[i].y; - if (ly < dY - 40 || ly > dY + 40) { - continue; - } - // Walk backward through digits on the same line. - int p = i - 1; - int digits = 0; - while (p >= 0 && str::IsDigit(text[p]) && coords[p].y == ly) { - p--; - digits++; - } - if (digits == 0) { - continue; - } - // Optional ".M" form. - if (p >= 0 && text[p] == L'.' && coords[p].y == ly) { - p--; - int d2 = 0; - while (p >= 0 && str::IsDigit(text[p]) && coords[p].y == ly) { - p--; - d2++; - } - if (d2 == 0) { - continue; - } - } - if (p < 0 || text[p] != L'(' || coords[p].y != ly) { - continue; - } - int labelLeftX = coords[p].x; - int labelRightX = coords[i].x + coords[i].dx; - // Reject if any non-space glyph on the same line sits further right - // than the label — equation labels are line-trailing by construction. - bool hasRightOf = false; - for (int j = 0; j < textLen; j++) { - if (j >= p && j <= i) { - continue; - } - if (str::IsWs(text[j])) { - continue; - } - if (coords[j].y != ly) { - continue; - } - if (coords[j].x + coords[j].dx > labelRightX) { - hasRightOf = true; - break; - } - } - if (hasRightOf) { - continue; - } - // Reject if the label sits in the left half of the page (likely a - // body-text "(N)" footnote marker, not a display-eq label). - if (labelLeftX < (int)(mediabox.dx * 0.5f)) { - continue; - } - int dist = std::abs(ly - dY); - if (dist < bestDist) { - bestDist = dist; - bestLabelY = ly; - bestLabelDy = coords[i].dy; - } - } - if (bestLabelY < 0) { - return empty; - } - if (bestLabelDy <= 0) { - bestLabelDy = 12; - } - // Region: one eq line — labeled row + small vertical padding. Multi-row - // align environments are rare in cross-refs; a tight box is the right - // default and the user can wheel-scroll if context is needed. - float pad = (float)bestLabelDy + 6.f; - RectF box{0.f, (float)bestLabelY - pad, mediabox.dx, (float)bestLabelDy + 2.f * pad}; - ClipToMediabox(box, mediabox); - return box; -} - -// Find the bounding box of a single bibliography entry on the destination -// page. Uses per-glyph text+coords from the engine's text cache: -// 1. Locate the leftmost glyph with y in a small band around destY (entry start). -// 2. Scan forward; stop at "[N" near the same left margin (next entry) or -// a vertical paragraph gap. -// 3. Return the min/max bounding box of glyphs in [start, end), padded. -// Falls back to LandscapeBox() when the link is not a bibliography reference -// (TOC, topbar, cross-ref, table caption). The landscape box renders a half- -// page-tall slice of the page anchored on the destination so the user sees -// surrounding context (e.g. the table rows under a caption). -static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float destY) { - RectF mediabox = engine->PageMediabox(destPage); - int textLen = 0; - Rect* coords = nullptr; - const WCHAR* text = engine->GetTextForPage(destPage, &textLen, &coords); - // Sparse-text dest page (image-only or near-image-only — e.g. a - // children's PDF overview with character thumbnails plus a single - // heading). Fitting to the heading line gives a thin sliver and hides - // the actual content. Show the whole page so the user sees what they - // would navigate to; the auto-fit in RefHoverOnTimer scales the bitmap - // to popup limits. - constexpr int kSparsePageTextLen = 50; - if (!text || textLen < kSparsePageTextLen || !coords) { - return RectF{0.f, 0.f, mediabox.dx, mediabox.dy}; - } - if (destY < 0.f) { - return LandscapeBox(mediabox, destX, destY, text, coords, textLen); - } - - int dY = (int)destY; - int dX = (int)destX; - // Constrain to the destination's column — for 2-column layouts this - // prevents the search from latching onto same-Y body text in another - // column. We allow a small left tolerance so a "[1]" whose [ starts - // a few pt left of destX still matches. - int columnLeft = (destX >= 0.f) ? dX - 15 : INT_MIN; - - // 1. Find the start glyph: top-left non-whitespace glyph with - // y in [destY-5, destY+30] and x at-or-right-of columnLeft. - int startIdx = -1; - int bestY = INT_MAX; - int bestX = INT_MAX; - for (int i = 0; i < textLen; i++) { - WCHAR c = text[i]; - if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { - continue; - } - Rect r = coords[i]; - if (r.y < dY - 5 || r.y > dY + 30) { - continue; - } - if (r.x < columnLeft) { - continue; - } - if (r.y < bestY || (r.y == bestY && r.x < bestX)) { - bestY = r.y; - bestX = r.x; - startIdx = i; - } - } - if (startIdx < 0) { - return LandscapeBox(mediabox, destX, destY, text, coords, textLen); - } - - // PDF link destX is unreliable: poorly-authored links carry the source - // page's body-text X, not the destination-page entry-start X. That lands - // startIdx mid-line on hanging-indent description-list bibs, dropping - // the leading "[KOS06]" / "Philippe Kruchten" portion from the popup. - // Walk to the leftmost glyph on the same line as startIdx so the entry - // bounds always include the line's left edge. - { - int sy = coords[startIdx].y; - int leftmostX = coords[startIdx].x; - int leftmostIdx = startIdx; - for (int i = 0; i < textLen; i++) { - WCHAR c = text[i]; - if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { - continue; - } - Rect r = coords[i]; - if (r.y < sy - 3 || r.y > sy + 3) { - continue; - } - if (r.x < leftmostX) { - leftmostX = r.x; - leftmostIdx = i; - } - } - startIdx = leftmostIdx; - } - - // Tight-y walk above can miss a "[VB25]"-style label that sits on a - // slightly different baseline than its body line 1 (description-list - // layouts where label and body use different fonts/sizes). If the - // current leftmost still isn't a "[", search for one within roughly a - // line height of destY at any smaller x — that's the bracket label of - // the entry the link points at. - if (text[startIdx] != L'[') { - int sy = coords[startIdx].y; - int sDy = coords[startIdx].dy; - int yTol = sDy > 10 ? sDy : 10; - int bracketIdx = -1; - int bracketX = coords[startIdx].x; - for (int i = 0; i < textLen; i++) { - if (text[i] != L'[') { - continue; - } - Rect r = coords[i]; - if (r.y < sy - yTol || r.y > sy + yTol) { - continue; - } - if (r.x >= bracketX) { - continue; - } - bracketX = r.x; - bracketIdx = i; - } - if (bracketIdx >= 0) { - startIdx = bracketIdx; - } - } - - int firstLineLeftX = coords[startIdx].x; - int firstLineY = coords[startIdx].y; - int firstLineDy = coords[startIdx].dy; - if (firstLineDy <= 0) { - firstLineDy = 12; - } - - // Bracket-style entry ("[ZM12]", "[1]", …): build the bounding box from - // a y-range whose upper bound is the next "[" at firstLineLeftX. The - // iterative scan below depends on text-array order, but some PDFs draw - // labels and body in non-monotonic order — that made rule (a) terminate - // on a *later* entry's "[" appearing early in the text array, before our - // entry's body lines 2+. The y-range approach is order-independent. - if (text[startIdx] == L'[') { - int entryYBoundary = (int)mediabox.dy; - for (int i = 0; i < textLen; i++) { - if (i == startIdx) { - continue; - } - if (text[i] != L'[') { - continue; - } - Rect r = coords[i]; - // Accept "[" up to 30pt right of firstLineLeftX: some layouts - // prefix entries with a page number or section index (e.g. a - // "2" left of "[VB25]"), so the label "[" isn't exactly at - // firstLineLeftX. Body-text "[…]" sits at indentX (≥ ~60pt - // right of firstLineLeftX) so it's still excluded. - if (r.x < firstLineLeftX - 5 || r.x > firstLineLeftX + 30) { - continue; - } - if (r.y <= firstLineY + firstLineDy) { - continue; - } - if (r.y < entryYBoundary) { - entryYBoundary = r.y; - } - } - // Cap to a reasonable entry height so a last-on-page entry (no next - // "[") doesn't sweep the page footer / page number into the popup. - constexpr int kMaxBracketEntryPt = 250; - int capY = firstLineY + kMaxBracketEntryPt; - if (capY < entryYBoundary) { - entryYBoundary = capY; - } - // Pull the boundary up by ~half a line height so the next entry's - // first line — whose glyph tops can round to within 1–2 pt of the - // "[" we picked — is reliably excluded. - entryYBoundary -= 6; - int bMinX = INT_MAX, bMinY = INT_MAX, bMaxX = INT_MIN, bMaxY = INT_MIN; - for (int i = 0; i < textLen; i++) { - WCHAR c = text[i]; - if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { - continue; - } - Rect r = coords[i]; - if (r.x < firstLineLeftX - 20) { - continue; - } - if (r.y < firstLineY - 5) { - continue; - } - if (r.y >= entryYBoundary) { - continue; - } - if (r.x < bMinX) { - bMinX = r.x; - } - if (r.y < bMinY) { - bMinY = r.y; - } - if (r.x + r.dx > bMaxX) { - bMaxX = r.x + r.dx; - } - if (r.y + r.dy > bMaxY) { - bMaxY = r.y + r.dy; - } - } - if (bMinX != INT_MAX && (bMaxX - bMinX) >= 50 && (bMaxY - bMinY) >= 12) { - RectF box{(float)bMinX - kEntryPadPt, (float)bMinY - kEntryPadPt, - (float)(bMaxX - bMinX) + 2.f * kEntryPadPt, (float)(bMaxY - bMinY) + 2.f * kEntryPadPt}; - ClipToMediabox(box, mediabox); - if (box.dx >= 50.f && box.dy >= 20.f) { - return box; - } - } - // Fall through to the iterative-scan logic on degenerate result. - } - - // 2. Scan forward to find the end of the entry. - int endIdx = textLen; - int prevY = firstLineY; - int prevBottom = firstLineY + firstLineDy; - int lineHeight = firstLineDy; - - // Track leftmost X on the current line vs the previous line so we can - // detect indent changes (the most reliable signal for author-year bibs). - int currentLineLeftX = firstLineLeftX; - int prevLineLeftX = INT_MAX; - // X of the entry's continuation lines (captured from line 2). -1 = unknown. - int indentX = -1; - // Set when we observe another sibling entry start at firstLineLeftX with - // no continuation indent in between — strong "this is a description list" - // signal (e.g. "JVM Java Virtual Machine. 19, 36" / "LLM Large Language - // Model. 45" abbreviation lists) that survives even when the current - // entry is a single line. - bool descListSibling = false; - - for (int i = startIdx + 1; i < textLen; i++) { - WCHAR c = text[i]; - if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { - continue; - } - Rect r = coords[i]; - - // Stop on column wrap: y goes significantly above the current row. - if (r.y < firstLineY - 5) { - endIdx = i; - break; - } - // Skip glyphs in other columns (left of the entry's column). - if (r.x < firstLineLeftX - 20) { - continue; - } - - bool isNewLine = (r.y > prevY + 2); - if (isNewLine) { - prevLineLeftX = currentLineLeftX; - currentLineLeftX = r.x; - } else if (r.x < currentLineLeftX) { - currentLineLeftX = r.x; - } - - bool pastFirstLine = (r.y > firstLineY + firstLineDy * 3 / 4 + 2); - bool atFirstLineLeftX = (r.x >= firstLineLeftX - 5 && r.x <= firstLineLeftX + 5); - - // Capture the continuation X from the entry's second line. - if (isNewLine && pastFirstLine && indentX < 0 && !atFirstLineLeftX) { - indentX = r.x; - } - - // (a) "[" at the entry's first-line X = next entry marker. Works for - // both numeric "[123]" and alphanumeric "[Foo+09]" / "[Bib05]" styles - // — body-text "[…]" can't trigger this because body sits at indentX, - // not firstLineLeftX. - if (c == L'[' && atFirstLineLeftX) { - descListSibling = true; - endIdx = i; - break; - } - - // (b) Indent change: a new line back at the entry's first-line X - // after a continuation line at a different X. Catches author-year - // hanging-indent bibliographies where there's no [N] marker — this - // is the primary signal for the *next* entry's start. - if (isNewLine && atFirstLineLeftX && pastFirstLine && prevLineLeftX != INT_MAX && - (prevLineLeftX < firstLineLeftX - 5 || prevLineLeftX > firstLineLeftX + 5)) { - descListSibling = true; - endIdx = i; - break; - } - - // (c) Vertical paragraph break (no-indent style fallback). When the - // glyph that triggered the gap is back at firstLineLeftX, the gap - // is a blank line between description-list siblings (typical - // abbreviation lists where each entry is separated by extra - // vertical space) — treat as a sibling entry boundary. - if (r.y > prevBottom + lineHeight * 5 / 4) { - if (atFirstLineLeftX) { - descListSibling = true; - } - endIdx = i; - break; - } - - // (d) Single-line-entry case: a new line back at firstLineLeftX before - // we discovered a continuation indent. The previous "entry" was one - // line. Common pattern: stacked numbered footnotes "¹url\n²url\n³url" - // or abbreviation lists ("JVM Java Virtual Machine. 19, 36"). - if (isNewLine && pastFirstLine && atFirstLineLeftX && indentX < 0 && prevLineLeftX != INT_MAX) { - descListSibling = true; - endIdx = i; - break; - } - - // Track current line height as we go (catches changing leading). - if (isNewLine) { - int dy = r.y - prevY; - if (dy > 4 && dy < 60) { - lineHeight = dy; - } - prevY = r.y; - prevBottom = r.y + r.dy; - } - } - - // 3. Compute bounding box of glyphs in [startIdx, endIdx). - int minX = INT_MAX, minY = INT_MAX, maxX = INT_MIN, maxY = INT_MIN; - for (int i = startIdx; i < endIdx; i++) { - WCHAR c = text[i]; - if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { - continue; - } - Rect r = coords[i]; - // Exclude glyphs that aren't in the entry's column. - if (r.x < firstLineLeftX - 20) { - continue; - } - if (r.y < firstLineY - 5) { - continue; - } - if (r.x < minX) { - minX = r.x; - } - if (r.y < minY) { - minY = r.y; - } - if (r.x + r.dx > maxX) { - maxX = r.x + r.dx; - } - if (r.y + r.dy > maxY) { - maxY = r.y + r.dy; - } - } - if (minX == INT_MAX) { - return LandscapeBox(mediabox, destX, destY, text, coords, textLen); - } - - RectF box{(float)minX - kEntryPadPt, (float)minY - kEntryPadPt, (float)(maxX - minX) + 2.f * kEntryPadPt, - (float)(maxY - minY) + 2.f * kEntryPadPt}; - ClipToMediabox(box, mediabox); - if (box.dx < 50.f || box.dy < 20.f) { - return LandscapeBox(mediabox, destX, destY, text, coords, textLen); - } - // "Figure N.M" / "Table N.M" / "Listing N.M" / "Algorithm N.M" caption - // anywhere below the detected box: the destination is a figure / table - // / listing body. Override all other heuristics so the popup uses the - // landscape view (caption included). Catches code/console listings - // where each line happens to start with "[TAG]" — those would otherwise - // be misclassified as description-list bibliography entries. - { - int boxBottomY = (int)(box.y + box.dy); - for (int i = 0; i < textLen; i++) { - if (coords[i].y <= boxBottomY) { - continue; - } - if (IsCaptionLabelAt(text, textLen, i)) { - // Let LandscapeBox handle the caption-extension — it has a - // tighter, line-count-capped walk that doesn't sweep into - // following body paragraphs. - return LandscapeBox(mediabox, destX, destY, text, coords, textLen); - } - } - } - // Description-list bibliography ("[Smith2020]", "[1]", …) — unambiguous, - // keep the fitted box. - if (text[startIdx] == L'[') { - return box; - } - // Tabular layout: continuation X far right of firstLineLeftX is a - // column gap, not a hanging indent. Detection terminated at the first - // data row; show the landscape view so the user sees the full table. - if (indentX > 0 && (indentX - firstLineLeftX) > 80) { - return LandscapeBox(mediabox, destX, destY, text, coords, textLen); - } - // Section heading or caption-style label. Body paragraph below the - // heading has first-line indent, so detection captures heading + body - // line 1 and `indentX` lands in the same range as a hanging-indent bib. - // Use the entry's first character / first word to disambiguate: real - // bibliographies rarely start with a digit or with a label word like - // "Figure"/"Table"/"Section". Catches "6.2 Foo", "Figure 2.2: …", etc. - auto matchPrefix = [&](const WCHAR* word, int n) { - if (startIdx + n >= textLen) { - return false; - } - for (int j = 0; j < n; j++) { - WCHAR c = text[startIdx + j]; - if (c >= L'A' && c <= L'Z') { - c = (WCHAR)(c + 32); - } - if (c != word[j]) { - return false; - } - } - return true; - }; - WCHAR firstC = text[startIdx]; - bool digitStart = (firstC >= L'0' && firstC <= L'9'); - bool labelStart = matchPrefix(L"figure", 6) || matchPrefix(L"abbildung", 9) || matchPrefix(L"table", 5) || - matchPrefix(L"tabelle", 7) || matchPrefix(L"listing", 7) || matchPrefix(L"section", 7) || - matchPrefix(L"abschnitt", 9) || matchPrefix(L"chapter", 7) || matchPrefix(L"kapitel", 7) || - matchPrefix(L"algorithm", 9) || matchPrefix(L"algorithmus", 11); - if (digitStart || labelStart) { - return LandscapeBox(mediabox, destX, destY, text, coords, textLen); - } - // Code-listing detector: a high density of braces / semicolons / parens - // within the detected box means the destination is most likely a code - // listing presented as a figure. Bibliography prose almost never has - // these characters at this density. Show the landscape view so the - // popup also includes the figure caption below the code. - { - int codeChars = 0; - int totalChars = 0; - for (int i = startIdx; i < endIdx; i++) { - WCHAR c = text[i]; - if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { - continue; - } - totalChars++; - if (c == L'{' || c == L'}' || c == L';' || c == L'(' || c == L')') { - codeChars++; - } - } - if (totalChars > 50 && codeChars * 12 > totalChars) { - return LandscapeBox(mediabox, destX, destY, text, coords, textLen); - } - } - // Description-list / glossary / footnote-style entry: rule (a) or (d) - // fired, meaning we saw a *sibling* entry start at firstLineLeftX. That - // is a strong "this is a list of entries" signal even when the current - // entry is a single line (abbreviations: "JVM Java Virtual Machine."). - if (descListSibling) { - return box; - } - // Single-line entry with no continuation indent and no sibling entry - // detected — caption / heading / in-text cross-ref destination. - if (box.dy < 30.f && indentX < 0) { - return LandscapeBox(mediabox, destX, destY, text, coords, textLen); - } - // Default: looks like a multi-line author-year bibliography entry, - // keep the fitted box. - return box; -} - // When the link's destination is page-level (destY < 0), try to recover a // specific Y by reading the source link's text from srcPage at srcRect, then // searching for that text on destPage. Returns -1 if no match. @@ -1503,11 +685,14 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa // dx/dy placeholders; resized against popup caps below. region = RectF{0.f, destY, mediabox.dx, mediabox.dy - destY}; } else { + int textLen = 0; + Rect* coords = nullptr; + const WCHAR* text = engine->GetTextForPage(destPage, &textLen, &coords); // Equation cross-reference: tight box around the labelled line. // Falls through to DetectEntryBox when no eq label is found. - region = DetectEquationBox(engine, destPage, destX, destY); + region = DetectEquationBox(text, coords, textLen, mediabox, destX, destY); if (region.dx <= 0.f || region.dy <= 0.f) { - region = DetectEntryBox(engine, destPage, destX, destY); + region = DetectEntryBox(text, coords, textLen, mediabox, destX, destY); } } // New destination — reset user-driven zoom. baseZoom matches the diff --git a/src/RefHoverDetect.cpp b/src/RefHoverDetect.cpp new file mode 100644 index 00000000000..f7eaa56aec0 --- /dev/null +++ b/src/RefHoverDetect.cpp @@ -0,0 +1,819 @@ +/* Copyright 2026 the SumatraPDF project authors (see AUTHORS file). + License: GPLv3 */ + +// Pure-function popup-region detectors used by RefHover. Kept in a separate +// translation unit so the heuristics can be unit-tested with synthetic glyph +// arrays (see src/utils/tests/RefHover_ut.cpp) without pulling in the engine, +// HWND, or rendering layers. + +#include "utils/BaseUtil.h" +#include "RefHoverDetect.h" + +static constexpr float kAnchorTopMarginPt = 6.f; +// pt of padding around the detected entry box. +static constexpr float kEntryPadPt = 6.f; + +static bool IsAsciiAlnum(WCHAR c) { + return (c >= L'a' && c <= L'z') || (c >= L'A' && c <= L'Z') || (c >= L'0' && c <= L'9'); +} + +// True if `text[idx..]` starts a caption label like "Figure 1.2", "Table 3", +// "Listing 4.1", "Algorithm 5" (en/de). Matches case-insensitively and +// requires the previous glyph to be a word boundary and the word to be +// followed by whitespace and a digit. Used by the popup region detectors to +// recognize a figure/table/caption destination so the popup includes the +// figure body above the caption. +static bool IsCaptionLabelAt(const WCHAR* text, int textLen, int idx) { + if (idx > 0 && IsAsciiAlnum(text[idx - 1])) { + return false; + } + auto matchWord = [&](const WCHAR* w, int n) -> bool { + if (idx + n + 1 >= textLen) { + return false; + } + for (int j = 0; j < n; j++) { + WCHAR c = text[idx + j]; + if (c >= L'A' && c <= L'Z') { + c = (WCHAR)(c + 32); + } + if (c != w[j]) { + return false; + } + } + int k = idx + n; + while (k < textLen && (text[k] == L' ' || text[k] == L'\t')) { + k++; + } + return k < textLen && text[k] >= L'0' && text[k] <= L'9'; + }; + return matchWord(L"figure", 6) || matchWord(L"abbildung", 9) || matchWord(L"table", 5) || + matchWord(L"tabelle", 7) || matchWord(L"listing", 7) || matchWord(L"algorithm", 9) || + matchWord(L"algorithmus", 11); +} + +// Clip a region to the page mediabox: shifts a negative x/y to 0 (shrinking +// the box by the same amount) and trims any overhang past the right/bottom +// edge. Used everywhere a detected region must be passed to RenderPage. +static void ClipToMediabox(RectF& box, RectF mediabox) { + if (box.x < 0.f) { + box.dx += box.x; + box.x = 0.f; + } + if (box.y < 0.f) { + box.dy += box.y; + box.y = 0.f; + } + if (box.x + box.dx > mediabox.dx) { + box.dx = mediabox.dx - box.x; + } + if (box.y + box.dy > mediabox.dy) { + box.dy = mediabox.dy - box.y; + } +} + +// Used when the link doesn't resolve to a recognizable bibliography entry — +// TOC targets, topbar/section links, table or figure captions, image-only +// PDFs. Returns a region that spans the full page width and goes from the +// destination Y down to the bottom of the last text glyph on the page — +// captures table caption / figure caption / section content below the +// link target without leaving a long blank margin at the popup bottom. +// Auto-fit in RefHoverOnTimer + the monitor-based popup height cap keep +// the popup a sensible size; the user can wheel-zoom in if text is too +// small. +RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* text, const Rect* coords, int textLen) { + (void)destX; + float ty = (destY >= 0.f) ? destY - kAnchorTopMarginPt : 0.f; + if (ty < 0.f) { + ty = 0.f; + } + // When destY anchors at a "Figure N.M" / "Abbildung N.M" / "Table N.M" + // caption line, the figure / table *body* sits above the caption — but + // ty currently starts at the caption. Extend upward so the popup + // includes the figure body, not just the caption + the paragraph + // following it. + bool destAtCaption = false; + if (text && coords && textLen > 0 && destY > 0.f) { + int dY = (int)destY; + for (int i = 0; i < textLen; i++) { + int gy = coords[i].y; + if (gy < dY - 5 || gy > dY + 15) { + continue; + } + if (IsCaptionLabelAt(text, textLen, i)) { + destAtCaption = true; + break; + } + } + } + if (destAtCaption) { + constexpr float kFigureBodyExtendPt = 250.f; + float newTy = ty - kFigureBodyExtendPt; + if (newTy < 0.f) { + newTy = 0.f; + } + ty = newTy; + } + float h = mediabox.dy - ty; + if (h <= 0.f) { + h = mediabox.dy; + ty = 0.f; + } + // Cap to a focused region size so the popup is wide and short rather + // than narrow and tall. Captions get a taller cap so the figure body + // above and the caption text below both fit. + constexpr float kMaxLandscapePt = 200.f; + constexpr float kMaxLandscapeCaptionPt = 360.f; + float maxLandscape = destAtCaption ? kMaxLandscapeCaptionPt : kMaxLandscapePt; + if (h > maxLandscape) { + h = maxLandscape; + } + // Caption extension: if a "Figure N.M" / "Table N.M" / "Listing N.M" / + // "Algorithm N.M" caption appears within ~250pt below the capped region + // bottom (typical figure body height), extend the region downward to + // include the full caption block. Necessary for image-only figures + // where the figure body has no extractable text at destY — the caller + // falls to LandscapeBox without ever running the caption-aware + // DetectEntryBox path. + if (text && coords && textLen > 0) { + // Search to end of page so tall figures with captions far below the + // initial 200pt cap still match. First "Figure N.M" line-start on the + // page below the cap wins — typically the relevant caption. + int searchTop = (int)(ty + h); + int searchBot = (int)mediabox.dy; + int capStartIdx = -1; + for (int i = 0; i < textLen; i++) { + if (coords[i].y < searchTop || coords[i].y > searchBot) { + continue; + } + if (IsCaptionLabelAt(text, textLen, i)) { + capStartIdx = i; + break; + } + } + if (capStartIdx >= 0) { + int capStartY = coords[capStartIdx].y; + int capLineH = coords[capStartIdx].dy; + if (capLineH < 10) { + capLineH = 12; + } + // Page right text margin: max right-X across all text glyphs on + // the page. A line reaching within ~30pt of pageRightX is at the + // column edge (justified body, or a hyphenated caption line). + int pageRightX = 0; + for (int j = 0; j < textLen; j++) { + int rx = coords[j].x + coords[j].dx; + if (rx > pageRightX) { + pageRightX = rx; + } + } + // Walk subsequent lines below capStartY. Stop when we hit a + // paragraph break (vertical gap above inter-line leading) or + // a body-shape line. Two signals to detect body: + // 1) gap > ~70% of capLineH (parskip / float-separator) = + // new paragraph. + // 2) a "short" caption line seen earlier and the current line + // fills the column (raggedright-then-justified transition). + // Either signal alone catches a common case; together they cover + // hyphenated multi-line German captions (e.g. "...Bo-/gner...") + // where every caption line happens to reach the right margin. + int captionEndY = capStartY + capLineH; + int prevLineBottom = capStartY + capLineH - 1; + bool seenShortLine = false; + for (int lineIdx = 0; lineIdx < 3; lineIdx++) { + int capTop, capBot; + if (lineIdx == 0) { + capTop = capStartY - 3; + capBot = capStartY + 3; + } else { + capTop = prevLineBottom + 1; + capBot = prevLineBottom + capLineH * 18 / 10; + } + bool foundLine = false; + int lineTopY = INT_MAX; + int lineBottomY = -1; + int lineRightX = 0; + for (int j = 0; j < textLen; j++) { + int gy = coords[j].y; + if (gy < capTop || gy > capBot) { + continue; + } + foundLine = true; + if (gy < lineTopY) { + lineTopY = gy; + } + int gb = gy + coords[j].dy; + if (gb > lineBottomY) { + lineBottomY = gb; + } + int rx = coords[j].x + coords[j].dx; + if (rx > lineRightX) { + lineRightX = rx; + } + } + if (!foundLine) { + break; + } + bool isShort = lineRightX < pageRightX - 30; + if (lineIdx >= 1) { + int gap = lineTopY - prevLineBottom; + if (gap > capLineH * 7 / 10) { + break; + } + if (!isShort && seenShortLine) { + break; + } + } + captionEndY = lineBottomY; + prevLineBottom = lineBottomY; + if (isShort) { + seenShortLine = true; + } + } + float extendedH = (float)captionEndY + kAnchorTopMarginPt - ty; + if (extendedH > h) { + h = extendedH; + } + } + } + // Trim trailing blank margin: find the bottom of the last text glyph + // inside the candidate region and end the region just below it so the + // popup doesn't render an empty trailing margin. + if (text && coords && textLen > 0) { + int boxTop = (int)ty; + int boxBottom = (int)(ty + h); + int lastTextBottom = boxTop; + for (int i = 0; i < textLen; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + Rect r = coords[i]; + if (r.y < boxTop || r.y >= boxBottom) { + continue; + } + int glyphBottom = r.y + r.dy; + if (glyphBottom > lastTextBottom) { + lastTextBottom = glyphBottom; + } + } + float trimmedH = (float)lastTextBottom + kAnchorTopMarginPt - ty; + if (trimmedH > 20.f && trimmedH < h) { + h = trimmedH; + } + } + return RectF{0.f, ty, mediabox.dx, h}; +} + +// Detect a labelled display equation at (destX, destY): a "(N)" or "(N.M)" +// glyph cluster sitting near the right column edge on or near destY, with +// no other text further right on that line. Returns the equation's tight +// bounding box (full page width, ~one eq line tall) when found, empty rect +// otherwise. Used to avoid the landscape-style 200pt slice that sweeps in +// the paragraph and the next equation below an equation cross-reference. +RectF DetectEquationBox(const WCHAR* text, const Rect* coords, int textLen, RectF mediabox, float destX, float destY) { + (void)destX; + RectF empty{}; + if (destY <= 0.f || !text || textLen <= 0 || !coords) { + return empty; + } + int dY = (int)destY; + + // Scan glyphs in a band around destY. Find a ')' whose right edge is the + // rightmost in its line, preceded by digits and an opening '('. + int bestLabelY = -1; + int bestLabelDy = 0; + int bestDist = INT_MAX; + for (int i = 0; i < textLen; i++) { + if (text[i] != L')') { + continue; + } + int ly = coords[i].y; + if (ly < dY - 40 || ly > dY + 40) { + continue; + } + // Walk backward through digits on the same line. + int p = i - 1; + int digits = 0; + while (p >= 0 && str::IsDigit(text[p]) && coords[p].y == ly) { + p--; + digits++; + } + if (digits == 0) { + continue; + } + // Optional ".M" form. + if (p >= 0 && text[p] == L'.' && coords[p].y == ly) { + p--; + int d2 = 0; + while (p >= 0 && str::IsDigit(text[p]) && coords[p].y == ly) { + p--; + d2++; + } + if (d2 == 0) { + continue; + } + } + if (p < 0 || text[p] != L'(' || coords[p].y != ly) { + continue; + } + int labelLeftX = coords[p].x; + int labelRightX = coords[i].x + coords[i].dx; + // Reject if any non-space glyph on the same line sits further right + // than the label — equation labels are line-trailing by construction. + bool hasRightOf = false; + for (int j = 0; j < textLen; j++) { + if (j >= p && j <= i) { + continue; + } + if (str::IsWs(text[j])) { + continue; + } + if (coords[j].y != ly) { + continue; + } + if (coords[j].x + coords[j].dx > labelRightX) { + hasRightOf = true; + break; + } + } + if (hasRightOf) { + continue; + } + // Reject if the label sits in the left half of the page (likely a + // body-text "(N)" footnote marker, not a display-eq label). + if (labelLeftX < (int)(mediabox.dx * 0.5f)) { + continue; + } + int dist = std::abs(ly - dY); + if (dist < bestDist) { + bestDist = dist; + bestLabelY = ly; + bestLabelDy = coords[i].dy; + } + } + if (bestLabelY < 0) { + return empty; + } + if (bestLabelDy <= 0) { + bestLabelDy = 12; + } + // Region: one eq line — labeled row + small vertical padding. Multi-row + // align environments are rare in cross-refs; a tight box is the right + // default and the user can wheel-scroll if context is needed. + float pad = (float)bestLabelDy + 6.f; + RectF box{0.f, (float)bestLabelY - pad, mediabox.dx, (float)bestLabelDy + 2.f * pad}; + ClipToMediabox(box, mediabox); + return box; +} + +// Find the bounding box of a single bibliography entry on the destination +// page. Uses per-glyph text+coords from the engine's text cache: +// 1. Locate the leftmost glyph with y in a small band around destY (entry start). +// 2. Scan forward; stop at "[N" near the same left margin (next entry) or +// a vertical paragraph gap. +// 3. Return the min/max bounding box of glyphs in [start, end), padded. +// Falls back to LandscapeBox() when the link is not a bibliography reference +// (TOC, topbar, cross-ref, table caption). The landscape box renders a half- +// page-tall slice of the page anchored on the destination so the user sees +// surrounding context (e.g. the table rows under a caption). +RectF DetectEntryBox(const WCHAR* text, const Rect* coords, int textLen, RectF mediabox, float destX, float destY) { + // Sparse-text dest page (image-only or near-image-only — e.g. a + // children's PDF overview with character thumbnails plus a single + // heading). Fitting to the heading line gives a thin sliver and hides + // the actual content. Show the whole page so the user sees what they + // would navigate to; the auto-fit in RefHoverOnTimer scales the bitmap + // to popup limits. + constexpr int kSparsePageTextLen = 50; + if (!text || textLen < kSparsePageTextLen || !coords) { + return RectF{0.f, 0.f, mediabox.dx, mediabox.dy}; + } + if (destY < 0.f) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + + int dY = (int)destY; + int dX = (int)destX; + // Constrain to the destination's column — for 2-column layouts this + // prevents the search from latching onto same-Y body text in another + // column. We allow a small left tolerance so a "[1]" whose [ starts + // a few pt left of destX still matches. + int columnLeft = (destX >= 0.f) ? dX - 15 : INT_MIN; + + // 1. Find the start glyph: top-left non-whitespace glyph with + // y in [destY-5, destY+30] and x at-or-right-of columnLeft. + int startIdx = -1; + int bestY = INT_MAX; + int bestX = INT_MAX; + for (int i = 0; i < textLen; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + Rect r = coords[i]; + if (r.y < dY - 5 || r.y > dY + 30) { + continue; + } + if (r.x < columnLeft) { + continue; + } + if (r.y < bestY || (r.y == bestY && r.x < bestX)) { + bestY = r.y; + bestX = r.x; + startIdx = i; + } + } + if (startIdx < 0) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + + // PDF link destX is unreliable: poorly-authored links carry the source + // page's body-text X, not the destination-page entry-start X. That lands + // startIdx mid-line on hanging-indent description-list bibs, dropping + // the leading "[KOS06]" / "Philippe Kruchten" portion from the popup. + // Walk to the leftmost glyph on the same line as startIdx so the entry + // bounds always include the line's left edge. + { + int sy = coords[startIdx].y; + int leftmostX = coords[startIdx].x; + int leftmostIdx = startIdx; + for (int i = 0; i < textLen; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + Rect r = coords[i]; + if (r.y < sy - 3 || r.y > sy + 3) { + continue; + } + if (r.x < leftmostX) { + leftmostX = r.x; + leftmostIdx = i; + } + } + startIdx = leftmostIdx; + } + + // Tight-y walk above can miss a "[VB25]"-style label that sits on a + // slightly different baseline than its body line 1 (description-list + // layouts where label and body use different fonts/sizes). If the + // current leftmost still isn't a "[", search for one within roughly a + // line height of destY at any smaller x — that's the bracket label of + // the entry the link points at. + if (text[startIdx] != L'[') { + int sy = coords[startIdx].y; + int sDy = coords[startIdx].dy; + int yTol = sDy > 10 ? sDy : 10; + int bracketIdx = -1; + int bracketX = coords[startIdx].x; + for (int i = 0; i < textLen; i++) { + if (text[i] != L'[') { + continue; + } + Rect r = coords[i]; + if (r.y < sy - yTol || r.y > sy + yTol) { + continue; + } + if (r.x >= bracketX) { + continue; + } + bracketX = r.x; + bracketIdx = i; + } + if (bracketIdx >= 0) { + startIdx = bracketIdx; + } + } + + int firstLineLeftX = coords[startIdx].x; + int firstLineY = coords[startIdx].y; + int firstLineDy = coords[startIdx].dy; + if (firstLineDy <= 0) { + firstLineDy = 12; + } + + // Bracket-style entry ("[ZM12]", "[1]", …): build the bounding box from + // a y-range whose upper bound is the next "[" at firstLineLeftX. The + // iterative scan below depends on text-array order, but some PDFs draw + // labels and body in non-monotonic order — that made rule (a) terminate + // on a *later* entry's "[" appearing early in the text array, before our + // entry's body lines 2+. The y-range approach is order-independent. + if (text[startIdx] == L'[') { + int entryYBoundary = (int)mediabox.dy; + for (int i = 0; i < textLen; i++) { + if (i == startIdx) { + continue; + } + if (text[i] != L'[') { + continue; + } + Rect r = coords[i]; + // Accept "[" up to 30pt right of firstLineLeftX: some layouts + // prefix entries with a page number or section index (e.g. a + // "2" left of "[VB25]"), so the label "[" isn't exactly at + // firstLineLeftX. Body-text "[…]" sits at indentX (≥ ~60pt + // right of firstLineLeftX) so it's still excluded. + if (r.x < firstLineLeftX - 5 || r.x > firstLineLeftX + 30) { + continue; + } + if (r.y <= firstLineY + firstLineDy) { + continue; + } + if (r.y < entryYBoundary) { + entryYBoundary = r.y; + } + } + // Cap to a reasonable entry height so a last-on-page entry (no next + // "[") doesn't sweep the page footer / page number into the popup. + constexpr int kMaxBracketEntryPt = 250; + int capY = firstLineY + kMaxBracketEntryPt; + if (capY < entryYBoundary) { + entryYBoundary = capY; + } + // Pull the boundary up by ~half a line height so the next entry's + // first line — whose glyph tops can round to within 1–2 pt of the + // "[" we picked — is reliably excluded. + entryYBoundary -= 6; + int bMinX = INT_MAX, bMinY = INT_MAX, bMaxX = INT_MIN, bMaxY = INT_MIN; + for (int i = 0; i < textLen; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + Rect r = coords[i]; + if (r.x < firstLineLeftX - 20) { + continue; + } + if (r.y < firstLineY - 5) { + continue; + } + if (r.y >= entryYBoundary) { + continue; + } + if (r.x < bMinX) { + bMinX = r.x; + } + if (r.y < bMinY) { + bMinY = r.y; + } + if (r.x + r.dx > bMaxX) { + bMaxX = r.x + r.dx; + } + if (r.y + r.dy > bMaxY) { + bMaxY = r.y + r.dy; + } + } + if (bMinX != INT_MAX && (bMaxX - bMinX) >= 50 && (bMaxY - bMinY) >= 12) { + RectF box{(float)bMinX - kEntryPadPt, (float)bMinY - kEntryPadPt, + (float)(bMaxX - bMinX) + 2.f * kEntryPadPt, (float)(bMaxY - bMinY) + 2.f * kEntryPadPt}; + ClipToMediabox(box, mediabox); + if (box.dx >= 50.f && box.dy >= 20.f) { + return box; + } + } + // Fall through to the iterative-scan logic on degenerate result. + } + + // 2. Scan forward to find the end of the entry. + int endIdx = textLen; + int prevY = firstLineY; + int prevBottom = firstLineY + firstLineDy; + int lineHeight = firstLineDy; + + // Track leftmost X on the current line vs the previous line so we can + // detect indent changes (the most reliable signal for author-year bibs). + int currentLineLeftX = firstLineLeftX; + int prevLineLeftX = INT_MAX; + // X of the entry's continuation lines (captured from line 2). -1 = unknown. + int indentX = -1; + // Set when we observe another sibling entry start at firstLineLeftX with + // no continuation indent in between — strong "this is a description list" + // signal (e.g. "JVM Java Virtual Machine. 19, 36" / "LLM Large Language + // Model. 45" abbreviation lists) that survives even when the current + // entry is a single line. + bool descListSibling = false; + + for (int i = startIdx + 1; i < textLen; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + Rect r = coords[i]; + + // Stop on column wrap: y goes significantly above the current row. + if (r.y < firstLineY - 5) { + endIdx = i; + break; + } + // Skip glyphs in other columns (left of the entry's column). + if (r.x < firstLineLeftX - 20) { + continue; + } + + bool isNewLine = (r.y > prevY + 2); + if (isNewLine) { + prevLineLeftX = currentLineLeftX; + currentLineLeftX = r.x; + } else if (r.x < currentLineLeftX) { + currentLineLeftX = r.x; + } + + bool pastFirstLine = (r.y > firstLineY + firstLineDy * 3 / 4 + 2); + bool atFirstLineLeftX = (r.x >= firstLineLeftX - 5 && r.x <= firstLineLeftX + 5); + + // Capture the continuation X from the entry's second line. + if (isNewLine && pastFirstLine && indentX < 0 && !atFirstLineLeftX) { + indentX = r.x; + } + + // (a) "[" at the entry's first-line X = next entry marker. Works for + // both numeric "[123]" and alphanumeric "[Foo+09]" / "[Bib05]" styles + // — body-text "[…]" can't trigger this because body sits at indentX, + // not firstLineLeftX. + if (c == L'[' && atFirstLineLeftX) { + descListSibling = true; + endIdx = i; + break; + } + + // (b) Indent change: a new line back at the entry's first-line X + // after a continuation line at a different X. Catches author-year + // hanging-indent bibliographies where there's no [N] marker — this + // is the primary signal for the *next* entry's start. + if (isNewLine && atFirstLineLeftX && pastFirstLine && prevLineLeftX != INT_MAX && + (prevLineLeftX < firstLineLeftX - 5 || prevLineLeftX > firstLineLeftX + 5)) { + descListSibling = true; + endIdx = i; + break; + } + + // (c) Vertical paragraph break (no-indent style fallback). When the + // glyph that triggered the gap is back at firstLineLeftX, the gap + // is a blank line between description-list siblings (typical + // abbreviation lists where each entry is separated by extra + // vertical space) — treat as a sibling entry boundary. + if (r.y > prevBottom + lineHeight * 5 / 4) { + if (atFirstLineLeftX) { + descListSibling = true; + } + endIdx = i; + break; + } + + // (d) Single-line-entry case: a new line back at firstLineLeftX before + // we discovered a continuation indent. The previous "entry" was one + // line. Common pattern: stacked numbered footnotes "¹url\n²url\n³url" + // or abbreviation lists ("JVM Java Virtual Machine. 19, 36"). + if (isNewLine && pastFirstLine && atFirstLineLeftX && indentX < 0 && prevLineLeftX != INT_MAX) { + descListSibling = true; + endIdx = i; + break; + } + + // Track current line height as we go (catches changing leading). + if (isNewLine) { + int dy = r.y - prevY; + if (dy > 4 && dy < 60) { + lineHeight = dy; + } + prevY = r.y; + prevBottom = r.y + r.dy; + } + } + + // 3. Compute bounding box of glyphs in [startIdx, endIdx). + int minX = INT_MAX, minY = INT_MAX, maxX = INT_MIN, maxY = INT_MIN; + for (int i = startIdx; i < endIdx; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + Rect r = coords[i]; + // Exclude glyphs that aren't in the entry's column. + if (r.x < firstLineLeftX - 20) { + continue; + } + if (r.y < firstLineY - 5) { + continue; + } + if (r.x < minX) { + minX = r.x; + } + if (r.y < minY) { + minY = r.y; + } + if (r.x + r.dx > maxX) { + maxX = r.x + r.dx; + } + if (r.y + r.dy > maxY) { + maxY = r.y + r.dy; + } + } + if (minX == INT_MAX) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + + RectF box{(float)minX - kEntryPadPt, (float)minY - kEntryPadPt, (float)(maxX - minX) + 2.f * kEntryPadPt, + (float)(maxY - minY) + 2.f * kEntryPadPt}; + ClipToMediabox(box, mediabox); + if (box.dx < 50.f || box.dy < 20.f) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + // "Figure N.M" / "Table N.M" / "Listing N.M" / "Algorithm N.M" caption + // anywhere below the detected box: the destination is a figure / table + // / listing body. Override all other heuristics so the popup uses the + // landscape view (caption included). Catches code/console listings + // where each line happens to start with "[TAG]" — those would otherwise + // be misclassified as description-list bibliography entries. + { + int boxBottomY = (int)(box.y + box.dy); + for (int i = 0; i < textLen; i++) { + if (coords[i].y <= boxBottomY) { + continue; + } + if (IsCaptionLabelAt(text, textLen, i)) { + // Let LandscapeBox handle the caption-extension — it has a + // tighter, line-count-capped walk that doesn't sweep into + // following body paragraphs. + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + } + } + // Description-list bibliography ("[Smith2020]", "[1]", …) — unambiguous, + // keep the fitted box. + if (text[startIdx] == L'[') { + return box; + } + // Tabular layout: continuation X far right of firstLineLeftX is a + // column gap, not a hanging indent. Detection terminated at the first + // data row; show the landscape view so the user sees the full table. + if (indentX > 0 && (indentX - firstLineLeftX) > 80) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + // Section heading or caption-style label. Body paragraph below the + // heading has first-line indent, so detection captures heading + body + // line 1 and `indentX` lands in the same range as a hanging-indent bib. + // Use the entry's first character / first word to disambiguate: real + // bibliographies rarely start with a digit or with a label word like + // "Figure"/"Table"/"Section". Catches "6.2 Foo", "Figure 2.2: …", etc. + auto matchPrefix = [&](const WCHAR* word, int n) { + if (startIdx + n >= textLen) { + return false; + } + for (int j = 0; j < n; j++) { + WCHAR c = text[startIdx + j]; + if (c >= L'A' && c <= L'Z') { + c = (WCHAR)(c + 32); + } + if (c != word[j]) { + return false; + } + } + return true; + }; + WCHAR firstC = text[startIdx]; + bool digitStart = (firstC >= L'0' && firstC <= L'9'); + bool labelStart = matchPrefix(L"figure", 6) || matchPrefix(L"abbildung", 9) || matchPrefix(L"table", 5) || + matchPrefix(L"tabelle", 7) || matchPrefix(L"listing", 7) || matchPrefix(L"section", 7) || + matchPrefix(L"abschnitt", 9) || matchPrefix(L"chapter", 7) || matchPrefix(L"kapitel", 7) || + matchPrefix(L"algorithm", 9) || matchPrefix(L"algorithmus", 11); + if (digitStart || labelStart) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + // Code-listing detector: a high density of braces / semicolons / parens + // within the detected box means the destination is most likely a code + // listing presented as a figure. Bibliography prose almost never has + // these characters at this density. Show the landscape view so the + // popup also includes the figure caption below the code. + { + int codeChars = 0; + int totalChars = 0; + for (int i = startIdx; i < endIdx; i++) { + WCHAR c = text[i]; + if (c == L' ' || c == L'\t' || c == L'\n' || c == L'\r') { + continue; + } + totalChars++; + if (c == L'{' || c == L'}' || c == L';' || c == L'(' || c == L')') { + codeChars++; + } + } + if (totalChars > 50 && codeChars * 12 > totalChars) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + } + // Description-list / glossary / footnote-style entry: rule (a) or (d) + // fired, meaning we saw a *sibling* entry start at firstLineLeftX. That + // is a strong "this is a list of entries" signal even when the current + // entry is a single line (abbreviations: "JVM Java Virtual Machine."). + if (descListSibling) { + return box; + } + // Single-line entry with no continuation indent and no sibling entry + // detected — caption / heading / in-text cross-ref destination. + if (box.dy < 30.f && indentX < 0) { + return LandscapeBox(mediabox, destX, destY, text, coords, textLen); + } + // Default: looks like a multi-line author-year bibliography entry, + // keep the fitted box. + return box; +} diff --git a/src/RefHoverDetect.h b/src/RefHoverDetect.h new file mode 100644 index 00000000000..18e41b44bd3 --- /dev/null +++ b/src/RefHoverDetect.h @@ -0,0 +1,32 @@ +/* Copyright 2026 the SumatraPDF project authors (see AUTHORS file). + License: GPLv3 */ + +// Pure-function region detectors used by RefHover to decide what slice of the +// destination page to render into the hover popup. Kept engine-independent so +// the heuristics can be unit-tested with synthetic glyph arrays (see +// src/utils/tests/RefHover_ut.cpp). +// +// All three functions take: +// text — per-glyph WCHAR array (engine->GetTextForPage's first out-ptr) +// coords — per-glyph Rect array, parallel to `text` (second out-ptr) +// textLen — glyph count +// mediabox — page bounds in PDF user space +// destX, destY — link's destination coordinates (PDF user space) +// +// Returned RectF is in PDF user space, clipped to mediabox. + +// Landscape view: full page width strip anchored at destY, extending downward +// to the last text glyph or a recognised caption block. Fallback when no +// recognisable entry or equation is found. +RectF LandscapeBox(RectF mediabox, float destX, float destY, const WCHAR* text, const Rect* coords, int textLen); + +// Equation cross-ref: tight one-line box around a "(N)" or "(N.M)" label +// sitting at the right column edge near destY. Returns empty rect when no +// equation label is detected. +RectF DetectEquationBox(const WCHAR* text, const Rect* coords, int textLen, RectF mediabox, float destX, float destY); + +// Bibliography / glossary / abbreviation entry box. Tries bracket-style +// ("[Foo+09]"), hanging-indent author-year, and single-line description-list +// layouts. Falls back to LandscapeBox when the destination doesn't look like +// a list entry. +RectF DetectEntryBox(const WCHAR* text, const Rect* coords, int textLen, RectF mediabox, float destX, float destY); diff --git a/src/tools/test_util.cpp b/src/tools/test_util.cpp index 81318a2ecdd..d2e43bf47e5 100644 --- a/src/tools/test_util.cpp +++ b/src/tools/test_util.cpp @@ -19,6 +19,7 @@ extern void FileUtilTest(); extern void HtmlPrettyPrintTest(); extern void HtmlPullParser_UnitTests(); extern void JsonTest(); +extern void RefHoverTest(); extern void SettingsUtilTest(); extern void SimpleLogTest(); extern void SquareTreeTest(); @@ -51,6 +52,7 @@ int main(int, char**) { HtmlPrettyPrintTest(); HtmlPullParser_UnitTests(); JsonTest(); + RefHoverTest(); SettingsUtilTest(); SimpleLogTest(); SquareTreeTest(); diff --git a/src/utils/tests/RefHover_ut.cpp b/src/utils/tests/RefHover_ut.cpp new file mode 100644 index 00000000000..1ce33f603bf --- /dev/null +++ b/src/utils/tests/RefHover_ut.cpp @@ -0,0 +1,178 @@ +/* Copyright 2026 the SumatraPDF project authors (see AUTHORS file). + License: GPLv3 */ + +// Unit tests for the popup region detectors in RefHoverDetect. Each test +// constructs a synthetic per-glyph (text + coords) array that mimics what +// EngineBase::GetTextForPage produces, then asserts the detector returns a +// region matching the documented behaviour. + +#include "utils/BaseUtil.h" +#include "RefHoverDetect.h" + +// must be last due to assert() over-write +#include "utils/UtAssert.h" + +constexpr float kPageW = 612.f; +constexpr float kPageH = 792.f; +constexpr int kCharW = 6; +constexpr int kLineH = 12; + +// Helper: append the WCHARs of `s` starting at (x, y) with fixed-width glyphs. +// Caller passes pre-allocated text/coords buffers and the current length. +static void AddText(WCHAR* text, Rect* coords, int& len, int cap, const WCHAR* s, int x, int y) { + for (int i = 0; s[i] && len < cap; i++) { + text[len] = s[i]; + coords[len] = Rect{x + i * kCharW, y, kCharW, kLineH}; + len++; + } +} + +static RectF Mediabox() { + return RectF{0.f, 0.f, kPageW, kPageH}; +} + +static bool IsEmpty(RectF r) { + return r.dx <= 0.f || r.dy <= 0.f; +} + +// (1) Sparse-text destination page (< 50 non-empty glyphs): DetectEntryBox +// returns the whole page so an image-heavy page renders fully. +static void SparseTextReturnsWholePage() { + WCHAR text[64]; + Rect coords[64]; + int len = 0; + AddText(text, coords, len, 64, L"Heading only", 100, 100); + RectF box = DetectEntryBox(text, coords, len, Mediabox(), 100.f, 100.f); + utassert(box.x == 0.f); + utassert(box.y == 0.f); + utassert(box.dx == kPageW); + utassert(box.dy == kPageH); +} + +// (2) destY < 0 with rich text: DetectEntryBox delegates to LandscapeBox, +// which returns a full-width strip anchored at the page top. +static void NegativeDestYFallsToLandscape() { + WCHAR text[512]; + Rect coords[512]; + int len = 0; + for (int i = 0; i < 10; i++) { + AddText(text, coords, len, 512, L"Body text line content here.", 72, 100 + i * 14); + } + RectF box = DetectEntryBox(text, coords, len, Mediabox(), 72.f, -1.f); + utassert(box.x == 0.f); + utassert(box.y == 0.f); + utassert(box.dx == kPageW); + utassert(box.dy > 0.f); + utassert(box.dy < kPageH); +} + +// (3) Bracket-style bibliography "[Foo10]" / "[Bar11]": DetectEntryBox fits +// to the first entry and does not include the second. +static void BracketEntryFitsToOneEntry() { + WCHAR text[512]; + Rect coords[512]; + int len = 0; + // Entry 1 at y=200, two lines (line 2 indented). + AddText(text, coords, len, 512, L"[Foo10] Smith J., 2010, Some title.", 72, 200); + AddText(text, coords, len, 512, L"continuation line of the entry.", 92, 215); + // Entry 2 at y=240, sibling start back at x=72. + AddText(text, coords, len, 512, L"[Bar11] Doe J., 2011, Another title.", 72, 240); + AddText(text, coords, len, 512, L"continuation line of the entry.", 92, 255); + RectF box = DetectEntryBox(text, coords, len, Mediabox(), 72.f, 200.f); + utassert(!IsEmpty(box)); + // Box should end before entry 2 starts at y=240. + utassert(box.y + box.dy < 240.f); + // Box should start at or after entry 1's first line. + utassert(box.y <= 200.f + 6.f); + utassert(box.y >= 200.f - 12.f); +} + +// (4) Equation label "(14)" at right column edge near destY: DetectEquationBox +// returns a tight, full-width strip near the label. +static void EquationLabelDetected() { + WCHAR text[512]; + Rect coords[512]; + int len = 0; + // Equation row at y=300, label "(14)" at x≈540 (right of mediabox.dx*0.5). + AddText(text, coords, len, 512, L"dH/dt = -sum( ... )", 200, 300); + AddText(text, coords, len, 512, L"(14)", 540, 300); + // A paragraph below. + for (int i = 0; i < 3; i++) { + AddText(text, coords, len, 512, L"Paragraph text below the equation here.", 72, 330 + i * 14); + } + RectF box = DetectEquationBox(text, coords, len, Mediabox(), 72.f, 300.f); + utassert(!IsEmpty(box)); + utassert(box.x == 0.f); + utassert(box.dx == kPageW); + // Tight vertical band around y=300 (within ~3 line heights). + utassert(box.y < 300.f); + utassert(box.y + box.dy < 300.f + 3 * (float)kLineH + 20.f); +} + +// (5) "(N)" sitting in the left half of the page (body-text marker, not a +// display-eq label): DetectEquationBox returns empty. +static void BodyTextParenRejected() { + WCHAR text[512]; + Rect coords[512]; + int len = 0; + // "(14)" at x=80 — left half of 612-wide page. + AddText(text, coords, len, 512, L"(14) inline text continuing past the label.", 80, 300); + RectF box = DetectEquationBox(text, coords, len, Mediabox(), 80.f, 300.f); + utassert(IsEmpty(box)); +} + +// (6) "(N)" with more text further right on the same line: DetectEquationBox +// returns empty (label must be line-trailing). +static void NonTrailingParenRejected() { + WCHAR text[512]; + Rect coords[512]; + int len = 0; + // Label "(14)" at x=540 followed by text at x=560 (further right). + AddText(text, coords, len, 512, L"dH/dt = ...", 200, 300); + AddText(text, coords, len, 512, L"(14)", 540, 300); + AddText(text, coords, len, 512, L"trail", 560, 300); + RectF box = DetectEquationBox(text, coords, len, Mediabox(), 200.f, 300.f); + utassert(IsEmpty(box)); +} + +// (7) Empty / null inputs: DetectEquationBox returns empty rect, never crashes. +static void EmptyInputsHandled() { + RectF box1 = DetectEquationBox(nullptr, nullptr, 0, Mediabox(), 0.f, 100.f); + utassert(IsEmpty(box1)); + WCHAR text[1] = {0}; + Rect coords[1]{}; + RectF box2 = DetectEquationBox(text, coords, 0, Mediabox(), 0.f, 100.f); + utassert(IsEmpty(box2)); + // destY <= 0 short-circuit. + RectF box3 = DetectEquationBox(text, coords, 0, Mediabox(), 0.f, -1.f); + utassert(IsEmpty(box3)); +} + +// (8) LandscapeBox: returned region spans the full mediabox width, starts at +// destY-margin, height is positive and bounded. +static void LandscapeBoxBasicShape() { + WCHAR text[256]; + Rect coords[256]; + int len = 0; + for (int i = 0; i < 5; i++) { + AddText(text, coords, len, 256, L"some body content.", 72, 400 + i * 14); + } + RectF box = LandscapeBox(Mediabox(), 72.f, 400.f, text, coords, len); + utassert(box.x == 0.f); + utassert(box.dx == kPageW); + utassert(box.y >= 0.f); + utassert(box.y < 400.f); + utassert(box.dy > 0.f); + utassert(box.y + box.dy <= kPageH); +} + +void RefHoverTest() { + SparseTextReturnsWholePage(); + NegativeDestYFallsToLandscape(); + BracketEntryFitsToOneEntry(); + EquationLabelDetected(); + BodyTextParenRejected(); + NonTrailingParenRejected(); + EmptyInputsHandled(); + LandscapeBoxBasicShape(); +} diff --git a/vs2022/SumatraPDF-dll.vcxproj b/vs2022/SumatraPDF-dll.vcxproj index 50afc624393..5ae05c4a5bb 100644 --- a/vs2022/SumatraPDF-dll.vcxproj +++ b/vs2022/SumatraPDF-dll.vcxproj @@ -1245,6 +1245,7 @@ cd ../out/rel64_prefast_asan & ..\..\bin\MakeLZSA.exe InstallerData.dat libm + diff --git a/vs2022/SumatraPDF.vcxproj b/vs2022/SumatraPDF.vcxproj index 9cb89a39f78..ff2e3b84a3d 100644 --- a/vs2022/SumatraPDF.vcxproj +++ b/vs2022/SumatraPDF.vcxproj @@ -1228,6 +1228,7 @@ + diff --git a/vs2022/test_util.vcxproj b/vs2022/test_util.vcxproj index 85a94a11367..0bcdacdd4a4 100644 --- a/vs2022/test_util.vcxproj +++ b/vs2022/test_util.vcxproj @@ -910,6 +910,7 @@ + @@ -958,6 +959,7 @@ + From 83954802f1ac565647c9cda6a779968a8b41e899 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 28 May 2026 14:31:06 +0200 Subject: [PATCH 20/21] RefHover: data-driven caption/heading dictionaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hoist the language-specific caption and heading-prefix word lists out of IsCaptionLabelAt / DetectEntryBox and into two file-static tables in RefHoverDetect.cpp. Adding support for a new language is now a one-line addition to the table instead of editing the call sites. Switch the case-fold step from ASCII-only `c + 32` to `towlower` so the match works for capitalised non-ASCII first letters (e.g. "Sección", "Capítulo"). Accented dict entries are stored already-lowercased (NFC), matching the form PDF text extraction produces. Seed the tables with Spanish/Portuguese/Italian (figura, tabla, algoritmo, sección, capítulo) and French (tableau, chapitre, algorithme) in addition to the existing English and German. Add FrenchCaptionDetected test exercising the new dictionary path. Re-sort RefHoverTest call list alphabetically. Co-Authored-By: Claude Opus 4.7 --- src/RefHoverDetect.cpp | 127 ++++++++++++++++++++------------ src/utils/tests/RefHover_ut.cpp | 32 ++++++-- 2 files changed, 107 insertions(+), 52 deletions(-) diff --git a/src/RefHoverDetect.cpp b/src/RefHoverDetect.cpp index f7eaa56aec0..35ad1bb8e94 100644 --- a/src/RefHoverDetect.cpp +++ b/src/RefHoverDetect.cpp @@ -17,38 +17,88 @@ static bool IsAsciiAlnum(WCHAR c) { return (c >= L'a' && c <= L'z') || (c >= L'A' && c <= L'Z') || (c >= L'0' && c <= L'9'); } -// True if `text[idx..]` starts a caption label like "Figure 1.2", "Table 3", -// "Listing 4.1", "Algorithm 5" (en/de). Matches case-insensitively and -// requires the previous glyph to be a word boundary and the word to be -// followed by whitespace and a digit. Used by the popup region detectors to -// recognize a figure/table/caption destination so the popup includes the -// figure body above the caption. -static bool IsCaptionLabelAt(const WCHAR* text, int textLen, int idx) { - if (idx > 0 && IsAsciiAlnum(text[idx - 1])) { +// Caption / heading keyword tables. Each entry is a lowercase word recognised +// at the start of a "Figure 1.2" / "Tableau 2" style label. Add a language by +// appending entries; the call sites loop the table so no other code changes. +// Trailing nullptr terminates the list. +// +// Entries are matched case-insensitively against the input glyph (via +// towlower) so capitalised or all-caps PDF text matches too. Accented dict +// words must be stored already-lowercased (NFC) — PDF text extraction +// produces NFC most of the time. +static const WCHAR* const kCaptionWords[] = { + // en + L"figure", L"table", L"listing", L"algorithm", + // de + L"abbildung", L"tabelle", L"algorithmus", + // es / it / pt (shared roots) + L"figura", L"tabla", L"algoritmo", + // fr + L"tableau", L"algorithme", + nullptr, +}; + +// Heading prefixes recognised at the start of a destination glyph run. Used +// to disambiguate a section heading / figure caption destination from a +// description-list bibliography entry. Superset of kCaptionWords — includes +// "section" / "chapter" / locale equivalents that aren't captions but are +// heading destinations. +static const WCHAR* const kHeadingPrefixWords[] = { + // en + L"figure", L"table", L"listing", L"section", L"chapter", L"algorithm", + // de + L"abbildung", L"tabelle", L"abschnitt", L"kapitel", L"algorithmus", + // es + L"figura", L"tabla", L"sección", L"capítulo", L"algoritmo", + // fr + L"tableau", L"chapitre", L"algorithme", + nullptr, +}; + +// Match `text[idx..]` case-insensitively against the lowercase dictionary +// word `w`. Returns true on full word match. When requireTrailingDigit is +// set, also requires optional whitespace then a digit immediately after the +// word (the "Figure 1.2" trailing-number constraint). +static bool MatchWordAt(const WCHAR* text, int textLen, int idx, const WCHAR* w, bool requireTrailingDigit) { + int n = 0; + while (w[n]) { + n++; + } + if (idx + n > textLen) { return false; } - auto matchWord = [&](const WCHAR* w, int n) -> bool { - if (idx + n + 1 >= textLen) { + if (requireTrailingDigit && idx + n + 1 >= textLen) { + return false; + } + for (int j = 0; j < n; j++) { + WCHAR c = (WCHAR)towlower(text[idx + j]); + if (c != w[j]) { return false; } - for (int j = 0; j < n; j++) { - WCHAR c = text[idx + j]; - if (c >= L'A' && c <= L'Z') { - c = (WCHAR)(c + 32); - } - if (c != w[j]) { - return false; - } - } - int k = idx + n; - while (k < textLen && (text[k] == L' ' || text[k] == L'\t')) { - k++; + } + if (!requireTrailingDigit) { + return true; + } + int k = idx + n; + while (k < textLen && (text[k] == L' ' || text[k] == L'\t')) { + k++; + } + return k < textLen && text[k] >= L'0' && text[k] <= L'9'; +} + +// True if `text[idx..]` starts a caption label like "Figure 1.2", "Tableau 2". +// See kCaptionWords for the language list. Requires the previous glyph to be +// a word boundary and the word to be followed by whitespace and a digit. +static bool IsCaptionLabelAt(const WCHAR* text, int textLen, int idx) { + if (idx > 0 && IsAsciiAlnum(text[idx - 1])) { + return false; + } + for (int i = 0; kCaptionWords[i]; i++) { + if (MatchWordAt(text, textLen, idx, kCaptionWords[i], /*requireTrailingDigit=*/true)) { + return true; } - return k < textLen && text[k] >= L'0' && text[k] <= L'9'; - }; - return matchWord(L"figure", 6) || matchWord(L"abbildung", 9) || matchWord(L"table", 5) || - matchWord(L"tabelle", 7) || matchWord(L"listing", 7) || matchWord(L"algorithm", 9) || - matchWord(L"algorithmus", 11); + } + return false; } // Clip a region to the page mediabox: shifts a negative x/y to 0 (shrinking @@ -755,27 +805,12 @@ RectF DetectEntryBox(const WCHAR* text, const Rect* coords, int textLen, RectF m // Use the entry's first character / first word to disambiguate: real // bibliographies rarely start with a digit or with a label word like // "Figure"/"Table"/"Section". Catches "6.2 Foo", "Figure 2.2: …", etc. - auto matchPrefix = [&](const WCHAR* word, int n) { - if (startIdx + n >= textLen) { - return false; - } - for (int j = 0; j < n; j++) { - WCHAR c = text[startIdx + j]; - if (c >= L'A' && c <= L'Z') { - c = (WCHAR)(c + 32); - } - if (c != word[j]) { - return false; - } - } - return true; - }; WCHAR firstC = text[startIdx]; bool digitStart = (firstC >= L'0' && firstC <= L'9'); - bool labelStart = matchPrefix(L"figure", 6) || matchPrefix(L"abbildung", 9) || matchPrefix(L"table", 5) || - matchPrefix(L"tabelle", 7) || matchPrefix(L"listing", 7) || matchPrefix(L"section", 7) || - matchPrefix(L"abschnitt", 9) || matchPrefix(L"chapter", 7) || matchPrefix(L"kapitel", 7) || - matchPrefix(L"algorithm", 9) || matchPrefix(L"algorithmus", 11); + bool labelStart = false; + for (int i = 0; !labelStart && kHeadingPrefixWords[i]; i++) { + labelStart = MatchWordAt(text, textLen, startIdx, kHeadingPrefixWords[i], /*requireTrailingDigit=*/false); + } if (digitStart || labelStart) { return LandscapeBox(mediabox, destX, destY, text, coords, textLen); } diff --git a/src/utils/tests/RefHover_ut.cpp b/src/utils/tests/RefHover_ut.cpp index 1ce33f603bf..60755389b6a 100644 --- a/src/utils/tests/RefHover_ut.cpp +++ b/src/utils/tests/RefHover_ut.cpp @@ -148,7 +148,26 @@ static void EmptyInputsHandled() { utassert(IsEmpty(box3)); } -// (8) LandscapeBox: returned region spans the full mediabox width, starts at +// (8a) Caption-detection table covers non-English label words: a "Tableau 2" +// French caption above the destination triggers the upward figure-body +// extension (region top moves above destY). +static void FrenchCaptionDetected() { + WCHAR text[512]; + Rect coords[512]; + int len = 0; + // Caption line "Tableau 2: Données" at y=300 — this is the destY. + AddText(text, coords, len, 512, L"Tableau 2: Donnees", 72, 300); + // Body paragraph below. + for (int i = 0; i < 5; i++) { + AddText(text, coords, len, 512, L"Paragraphe de texte courant.", 72, 320 + i * 14); + } + RectF box = LandscapeBox(Mediabox(), 72.f, 300.f, text, coords, len); + // destAtCaption pulls region top above destY (figure body extension). + utassert(box.y < 300.f - 50.f); + utassert(box.dx == kPageW); +} + +// (9) LandscapeBox: returned region spans the full mediabox width, starts at // destY-margin, height is positive and bounded. static void LandscapeBoxBasicShape() { WCHAR text[256]; @@ -167,12 +186,13 @@ static void LandscapeBoxBasicShape() { } void RefHoverTest() { - SparseTextReturnsWholePage(); - NegativeDestYFallsToLandscape(); - BracketEntryFitsToOneEntry(); - EquationLabelDetected(); BodyTextParenRejected(); - NonTrailingParenRejected(); + BracketEntryFitsToOneEntry(); EmptyInputsHandled(); + EquationLabelDetected(); + FrenchCaptionDetected(); LandscapeBoxBasicShape(); + NegativeDestYFallsToLandscape(); + NonTrailingParenRejected(); + SparseTextReturnsWholePage(); } From 9b706f411fa1ca9971468adfa28ad8ba9667457a Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 28 May 2026 14:34:49 +0200 Subject: [PATCH 21/21] RefHover: group RefHoverState fields into Pending / Displayed structs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Twelve fields had pending* / displayed* prefixes carrying their grouping in the name. Nest them under two struct members instead so the grouping is structural — RefHoverSchedule writes s->pending.*, RefHoverOnTimer reads s->pending.* and writes s->displayed.*, and the wheel handlers read s->displayed.* alone. Top-level hwndPopup / bmp stay flat (touched by every code path). No behaviour change; mechanical s/pendingDestPage/pending.destPage/- style rename plus the struct definition. Updates the two call sites in Canvas.cpp. Co-Authored-By: Claude Opus 4.7 --- src/Canvas.cpp | 4 +- src/RefHover.cpp | 102 +++++++++++++++++++++++------------------------ src/RefHover.h | 80 ++++++++++++++++++++----------------- 3 files changed, 96 insertions(+), 90 deletions(-) diff --git a/src/Canvas.cpp b/src/Canvas.cpp index abf2f554ad8..a38415355a8 100644 --- a/src/Canvas.cpp +++ b/src/Canvas.cpp @@ -2938,8 +2938,8 @@ static void OnTimer(MainWindow* win, HWND hwnd, WPARAM timerId) { DisplayModel* dm = win->AsFixed(); EngineBase* engine = dm ? dm->GetEngine() : nullptr; float pageZoom = 1.f; - if (dm && win->refHover && win->refHover->pendingDestPage > 0) { - pageZoom = dm->GetZoomReal(win->refHover->pendingDestPage); + if (dm && win->refHover && win->refHover->pending.destPage > 0) { + pageZoom = dm->GetZoomReal(win->refHover->pending.destPage); } RefHoverOnTimer(win->refHover, hwnd, engine, pageZoom); break; diff --git a/src/RefHover.cpp b/src/RefHover.cpp index 954f9b8db87..c1f8767b2c2 100644 --- a/src/RefHover.cpp +++ b/src/RefHover.cpp @@ -69,8 +69,8 @@ #define REF_HOVER_CLASS L"SumatraPDFRefHover" // upper bound for the auto-fit base zoom. We render at min(kRenderZoom, -// fit-to-popup-max), then multiply by RefHoverState::userZoom (the -// mouse-wheel adjustment). +// fit-to-popup-max), then multiply by RefHoverState::Displayed::userZoom +// (the mouse-wheel adjustment). static constexpr float kRenderZoom = 1.5f; // upper bounds for the popup window in screen pixels. static constexpr int kMaxPopupWidth = 1200; @@ -186,7 +186,7 @@ static void ShowPopup(RefHoverState* s, Point screenPt) { int rightBound = mi.rcWork.right; int topBound = mi.rcWork.top; int bottomBound = mi.rcWork.bottom; - Rect pr = s->pendingPageScreenRect; + Rect pr = s->pending.pageScreenRect; if (pr.dy > 0) { if (pr.y > topBound) { topBound = pr.y; @@ -261,32 +261,32 @@ static void ShowPopup(RefHoverState* s, Point screenPt) { // at higher detail. Either way the popup stays full — no blank area. // Returns true if the zoom actually changed. bool RefHoverWheelZoom(RefHoverState* s, EngineBase* engine, int wheelDelta) { - if (!s || !s->hwndPopup || s->displayedDestPage <= 0 || !engine) { + if (!s || !s->hwndPopup || s->displayed.destPage <= 0 || !engine) { return false; } float factor = (wheelDelta > 0) ? kUserZoomStep : (1.f / kUserZoomStep); - float newZoom = s->userZoom * factor; + float newZoom = s->displayed.userZoom * factor; if (newZoom < kMinUserZoom) { newZoom = kMinUserZoom; } else if (newZoom > kMaxUserZoom) { newZoom = kMaxUserZoom; } - if (newZoom == s->userZoom) { + if (newZoom == s->displayed.userZoom) { return false; } - s->userZoom = newZoom; + s->displayed.userZoom = newZoom; RECT rc; GetClientRect(s->hwndPopup, &rc); float clientW = (float)((rc.right - rc.left) - 2 * kBorder); float clientH = (float)((rc.bottom - rc.top) - 2 * kBorder); - float zoom = s->baseZoom * s->userZoom; + float zoom = s->displayed.baseZoom * s->displayed.userZoom; if (zoom <= 0.f || clientW <= 0.f || clientH <= 0.f) { return false; } - RectF mediabox = engine->PageMediabox(s->displayedDestPage); - RectF region = s->lastRegion; + RectF mediabox = engine->PageMediabox(s->displayed.destPage); + RectF region = s->displayed.region; region.dx = clientW / zoom; region.dy = clientH / zoom; // Clamp to page bounds. @@ -300,7 +300,7 @@ bool RefHoverWheelZoom(RefHoverState* s, EngineBase* engine, int wheelDelta) { return false; } - RenderPageArgs args(s->displayedDestPage, zoom, 0, ®ion); + RenderPageArgs args(s->displayed.destPage, zoom, 0, ®ion); RenderedBitmap* bmp = engine->RenderPage(args); if (!bmp) { return false; @@ -310,7 +310,7 @@ bool RefHoverWheelZoom(RefHoverState* s, EngineBase* engine, int wheelDelta) { // Persist the popup-fitting dx/dy so a subsequent scroll keeps rendering // a bitmap that fills the popup. Without this, scroll would fall back to // the original (smaller) entry-box dimensions and leave visible padding. - s->lastRegion = region; + s->displayed.region = region; InvalidateRect(s->hwndPopup, nullptr, TRUE); return true; } @@ -319,16 +319,16 @@ bool RefHoverWheelZoom(RefHoverState* s, EngineBase* engine, int wheelDelta) { // or next page when the region would cross a page edge — the popup behaves // like a small continuous-scroll viewport into the document. bool RefHoverWheelScroll(RefHoverState* s, EngineBase* engine, int wheelDelta) { - if (!s || !s->hwndPopup || s->displayedDestPage <= 0 || !engine) { + if (!s || !s->hwndPopup || s->displayed.destPage <= 0 || !engine) { return false; } - float zoom = s->baseZoom * s->userZoom; + float zoom = s->displayed.baseZoom * s->displayed.userZoom; if (zoom <= 0.f) { return false; } int pageCount = engine->PageCount(); - int page = s->displayedDestPage; - RectF region = s->lastRegion; + int page = s->displayed.destPage; + RectF region = s->displayed.region; RectF mediabox = engine->PageMediabox(page); if (mediabox.dx <= 0.f || mediabox.dy <= 0.f) { return false; @@ -377,7 +377,7 @@ bool RefHoverWheelScroll(RefHoverState* s, EngineBase* engine, int wheelDelta) { } } - if (page == s->displayedDestPage && newY == region.y) { + if (page == s->displayed.destPage && newY == region.y) { return false; } region.y = newY; @@ -399,8 +399,8 @@ bool RefHoverWheelScroll(RefHoverState* s, EngineBase* engine, int wheelDelta) { } delete s->bmp; s->bmp = bmp; - s->displayedDestPage = page; - s->lastRegion = region; + s->displayed.destPage = page; + s->displayed.region = region; InvalidateRect(s->hwndPopup, nullptr, TRUE); return true; } @@ -412,18 +412,18 @@ void RefHoverSchedule(RefHoverState* s, HWND hwndCanvas, Point screenPt, int des } KillTimer(hwndCanvas, kRefHoverTimerID); - if (IsWindowVisible(s->hwndPopup) && s->displayedDestPage == destPage && s->displayedDestX == destX && - s->displayedDestY == destY) { + if (IsWindowVisible(s->hwndPopup) && s->displayed.destPage == destPage && s->displayed.destX == destX && + s->displayed.destY == destY) { return; } - s->pendingScreenPt = screenPt; - s->pendingDestPage = destPage; - s->pendingDestX = destX; - s->pendingDestY = destY; - s->pendingDestZoom = destZoom; - s->pendingSrcPage = srcPage; - s->pendingSrcRect = srcRect; - s->pendingPageScreenRect = pageScreenRect; + s->pending.screenPt = screenPt; + s->pending.destPage = destPage; + s->pending.destX = destX; + s->pending.destY = destY; + s->pending.destZoom = destZoom; + s->pending.srcPage = srcPage; + s->pending.srcRect = srcRect; + s->pending.pageScreenRect = pageScreenRect; SetTimer(hwndCanvas, kRefHoverTimerID, kRefHoverDelayMs, nullptr); } @@ -432,10 +432,10 @@ void RefHoverHide(RefHoverState* s, HWND hwndCanvas) { return; } KillTimer(hwndCanvas, kRefHoverTimerID); - s->pendingDestPage = -1; + s->pending.destPage = -1; if (s->hwndPopup && IsWindowVisible(s->hwndPopup)) { ShowWindow(s->hwndPopup, SW_HIDE); - s->displayedDestPage = -1; + s->displayed.destPage = -1; } } @@ -630,12 +630,12 @@ static float ResolveDestYFromSourceText(EngineBase* engine, int srcPage, RectF s void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, float pageZoom) { KillTimer(hwndCanvas, kRefHoverTimerID); - if (!s || !engine || s->pendingDestPage <= 0) { + if (!s || !engine || s->pending.destPage <= 0) { return; } - int destPage = s->pendingDestPage; - float destX = s->pendingDestX; - float destY = s->pendingDestY; + int destPage = s->pending.destPage; + float destX = s->pending.destX; + float destY = s->pending.destY; RectF mediabox = engine->PageMediabox(destPage); if (mediabox.dx <= 0.f || mediabox.dy <= 0.f) { @@ -652,7 +652,7 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa // /XYZ 0 -2.58 0. Treat past-page-bottom destY the same as page-level. if (destY <= 0.f || destY >= mediabox.dy - 1.f) { destY = 0.f; - float resolved = ResolveDestYFromSourceText(engine, s->pendingSrcPage, s->pendingSrcRect, destPage); + float resolved = ResolveDestYFromSourceText(engine, s->pending.srcPage, s->pending.srcRect, destPage); if (resolved >= 0.f) { destY = resolved; if (destX < 0.f) { @@ -666,7 +666,7 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa // requested zoom rather than auto-fitting a detected entry box. // This matches the navigation behaviour (DisplayModel::ScrollTo // also reads the link zoom). - float linkZoom = s->pendingDestZoom; + float linkZoom = s->pending.destZoom; bool useLinkZoom = (linkZoom > 0.f); // Popup must not look smaller than the page does on screen — if the // user is already viewing the document at a higher zoom than the @@ -701,7 +701,7 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa // if either dimension would exceed the popup max; landscape-style // regions for non-reference targets are typically wider than tall, so // the width cap matters here. - s->userZoom = 1.f; + s->displayed.userZoom = 1.f; float baseZoom = useLinkZoom ? linkZoom : ((pageZoom > 0.f) ? pageZoom : kRenderZoom); // Popup max size: // width ~95% of monitor work area — popup may span beyond the page @@ -713,7 +713,7 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa // and surrounding context stay readable. int popupWCap = kMaxPopupWidth; { - POINT mp = {s->pendingScreenPt.x, s->pendingScreenPt.y}; + POINT mp = {s->pending.screenPt.x, s->pending.screenPt.y}; HMONITOR hmon = MonitorFromPoint(mp, MONITOR_DEFAULTTONEAREST); MONITORINFO mi{}; mi.cbSize = sizeof(mi); @@ -732,9 +732,9 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa // its full caption fit. Cursor at the bottom of the page → popup // expands upward into the page top; cursor at top → expands downward. int popupHCap; - if (region.dy > 250.f && s->pendingPageScreenRect.dy > 0) { - Rect pr = s->pendingPageScreenRect; - int curY = s->pendingScreenPt.y; + if (region.dy > 250.f && s->pending.pageScreenRect.dy > 0) { + Rect pr = s->pending.pageScreenRect; + int curY = s->pending.screenPt.y; int spaceAbove = curY - pr.y - 30; int spaceBelow = (pr.y + pr.dy) - curY - 30; int maxSpace = (spaceAbove > spaceBelow) ? spaceAbove : spaceBelow; @@ -745,8 +745,8 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa popupHCap = (pageBased > maxSpace) ? pageBased : maxSpace; } else { popupHCap = kMaxPopupHeight; - if (s->pendingPageScreenRect.dy > 0) { - int pageBased = s->pendingPageScreenRect.dy * 45 / 100; + if (s->pending.pageScreenRect.dy > 0) { + int pageBased = s->pending.pageScreenRect.dy * 45 / 100; if (pageBased < popupHCap) { popupHCap = pageBased; } @@ -787,8 +787,8 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa if (baseZoom < kMinUserZoom) { baseZoom = kMinUserZoom; } - s->baseZoom = baseZoom; - RenderPageArgs args(destPage, s->baseZoom * s->userZoom, 0, ®ion); + s->displayed.baseZoom = baseZoom; + RenderPageArgs args(destPage, s->displayed.baseZoom * s->displayed.userZoom, 0, ®ion); RenderedBitmap* bmp = engine->RenderPage(args); if (!bmp) { return; @@ -796,10 +796,10 @@ void RefHoverOnTimer(RefHoverState* s, HWND hwndCanvas, EngineBase* engine, floa delete s->bmp; s->bmp = bmp; - s->displayedDestPage = destPage; - s->displayedDestX = destX; - s->displayedDestY = destY; - s->lastRegion = region; + s->displayed.destPage = destPage; + s->displayed.destX = destX; + s->displayed.destY = destY; + s->displayed.region = region; - ShowPopup(s, s->pendingScreenPt); + ShowPopup(s, s->pending.screenPt); } diff --git a/src/RefHover.h b/src/RefHover.h index eca2a4c3c97..d21adfadac7 100644 --- a/src/RefHover.h +++ b/src/RefHover.h @@ -8,45 +8,51 @@ struct RefHoverState { HWND hwndPopup = nullptr; // currently shown rendered destination strip (owned) RenderedBitmap* bmp = nullptr; - int displayedDestPage = -1; - // Destination point used for the currently-displayed bitmap. Compared - // against incoming requests so adjacent links that target the same page - // but different positions (e.g. a section-7 link and a bib ref both - // landing on page 41) re-render rather than reuse the stale popup. - float displayedDestX = -1.f; - float displayedDestY = -1.f; - // pending request: set by RefHoverSchedule, consumed by RefHoverOnTimer - Point pendingScreenPt{}; - int pendingDestPage = -1; - float pendingDestX = -1.f; - float pendingDestY = -1.f; - // /XYZ zoom hint from the link (1.0 = 100%). 0 means "no zoom hint"; - // RefHover then falls back to its auto-fit DetectEntryBox heuristic. - // When non-zero, the popup renders the destination region centred on - // (destX, destY) at this zoom — honouring the link author's intent - // (e.g. "goto top-left at 2x"). - float pendingDestZoom = 0.f; - // Source link location, used to recover a more specific destY when the - // PDF link is page-level (destY < 0). We extract the source link's text - // from srcPage at srcRect and search for that text on destPage to find - // the matching entry's Y. Without this, page-level abbreviation / - // glossary links render the whole abbreviations page from top. - int pendingSrcPage = -1; - RectF pendingSrcRect{}; - // Screen rect of the source page (visible portion). Used to clamp the - // popup so it stays within the document area and doesn't drift into - // the gray margins outside the page. - Rect pendingPageScreenRect{}; + // Pending hover request: set by RefHoverSchedule, consumed by + // RefHoverOnTimer when the hover-delay timer fires. + struct Pending { + Point screenPt{}; + int destPage = -1; + float destX = -1.f; + float destY = -1.f; + // /XYZ zoom hint from the link (1.0 = 100%). 0 means "no zoom hint"; + // RefHover then falls back to its auto-fit DetectEntryBox heuristic. + // When non-zero, the popup renders the destination region centred on + // (destX, destY) at this zoom — honouring the link author's intent + // (e.g. "goto top-left at 2x"). + float destZoom = 0.f; + // Source link location, used to recover a more specific destY when + // the PDF link is page-level (destY < 0). We extract the source + // link's text from srcPage at srcRect and search for that text on + // destPage to find the matching entry's Y. Without this, page-level + // abbreviation / glossary links render the whole abbreviations page + // from top. + int srcPage = -1; + RectF srcRect{}; + // Screen rect of the source page (visible portion). Used to clamp + // the popup so it stays within the document area and doesn't drift + // into the gray margins outside the page. + Rect pageScreenRect{}; + } pending; - // re-render context, kept so mouse-wheel can zoom the popup without - // re-running detection. Reset on every new destination. - RectF lastRegion{}; - // baseZoom matches the document's current page zoom on first show so - // popup text height is comparable to page text. userZoom is the - // multiplier driven by the user's mouse-wheel. - float baseZoom = 1.f; - float userZoom = 1.f; + // Currently-displayed bitmap context. Compared against incoming hover + // requests to skip a re-render when the destination hasn't changed, and + // re-used by the wheel-zoom / wheel-scroll handlers so they can re-render + // without re-running detection. + struct Displayed { + int destPage = -1; + float destX = -1.f; + float destY = -1.f; + // Region of the page rendered into the popup bitmap, kept so the + // wheel handlers can shift / scale it without re-running detection. + RectF region{}; + // baseZoom matches the document's current page zoom on first show + // so popup text height is comparable to page text. userZoom is the + // multiplier driven by the user's mouse-wheel. + float baseZoom = 1.f; + float userZoom = 1.f; + } displayed; }; constexpr int kRefHoverDelayMs = 300;