Skip to content

Commit 77b017f

Browse files
kopporclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 232e061 commit 77b017f

1 file changed

Lines changed: 56 additions & 13 deletions

File tree

src/RefHover.cpp

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,15 @@
1212

1313
#define REF_HOVER_CLASS L"SumatraPDFRefHover"
1414

15-
// Fallback strip dimensions (used when text-based detection can't find boundaries).
16-
static constexpr float kStripHeightPt = 130.f;
1715
// Larger fallback used when detection found something tiny (likely a figure
1816
// fragment) — gives enough height to capture a diagram + caption.
1917
static constexpr float kFigureStripHeightPt = 280.f;
18+
// Non-reference targets (TOC, topbar links, generic cross-refs that don't
19+
// resolve to a bibliography-shaped entry) get a fixed-ratio box anchored on
20+
// the destination — keeps the popup visually consistent regardless of what
21+
// happens to be near the goto target.
22+
static constexpr float kNonRefBoxWidthPt = 360.f;
23+
static constexpr float kNonRefBoxHeightPt = 260.f;
2024
static constexpr float kAnchorTopMarginPt = 6.f;
2125
static constexpr float kRightMarginPt = 4.f;
2226
// pt of padding around the detected entry box.
@@ -235,10 +239,10 @@ void RefHoverHide(RefHoverState* s, HWND hwndCanvas) {
235239
}
236240
}
237241

238-
// Fallback box when text-based detection can't find boundaries: a strip from
239-
// destX to the right page margin. heightPt controls how tall — small for the
240-
// "no text near destY" case, larger for figures/diagrams.
241-
static RectF FallbackBox(RectF mediabox, float destX, float destY, float heightPt) {
242+
// Wide strip from destX to the right page margin — used only when detection
243+
// found a small box that looks like a figure caption number, so that the
244+
// popup captures the surrounding figure + caption.
245+
static RectF FigureStripBox(RectF mediabox, float destX, float destY, float heightPt) {
242246
float lx = (destX >= 0.f) ? destX - 12.f : 0.f;
243247
if (lx < 0.f) {
244248
lx = 0.f;
@@ -262,24 +266,63 @@ static RectF FallbackBox(RectF mediabox, float destX, float destY, float heightP
262266
return RectF{lx, ty, w, heightPt};
263267
}
264268

269+
// Used when the link doesn't resolve to a bibliography-shaped entry (TOC
270+
// targets, topbar links, image-only PDFs). Returns a fixed-pt box anchored
271+
// near the destination so that all "non-reference" popups have the same
272+
// shape regardless of what happens to be near the target.
273+
static RectF FixedRatioBox(RectF mediabox, float destX, float destY) {
274+
float w = kNonRefBoxWidthPt;
275+
float h = kNonRefBoxHeightPt;
276+
if (w > mediabox.dx) {
277+
w = mediabox.dx;
278+
}
279+
if (h > mediabox.dy) {
280+
h = mediabox.dy;
281+
}
282+
float lx = (destX >= 0.f) ? destX - 12.f : 0.f;
283+
float ty = (destY >= 0.f) ? destY - kAnchorTopMarginPt : 0.f;
284+
if (lx < 0.f) {
285+
lx = 0.f;
286+
}
287+
if (ty < 0.f) {
288+
ty = 0.f;
289+
}
290+
if (lx + w > mediabox.dx) {
291+
lx = mediabox.dx - w;
292+
}
293+
if (ty + h > mediabox.dy) {
294+
ty = mediabox.dy - h;
295+
}
296+
if (lx < 0.f) {
297+
lx = 0.f;
298+
}
299+
if (ty < 0.f) {
300+
ty = 0.f;
301+
}
302+
return RectF{lx, ty, w, h};
303+
}
304+
265305
// Find the bounding box of a single bibliography entry on the destination
266306
// page. Uses per-glyph text+coords from the engine's text cache:
267307
// 1. Locate the leftmost glyph with y in a small band around destY (entry start).
268308
// 2. Scan forward; stop at "[N" near the same left margin (next entry) or
269309
// a vertical paragraph gap.
270310
// 3. Return the min/max bounding box of glyphs in [start, end), padded.
271-
// Falls back to FallbackBox() if no glyphs are near destY.
311+
// Falls back to FixedRatioBox() if no glyphs are near destY (the link is not
312+
// a bibliography reference — TOC / topbar / cross-ref). The fixed-ratio box
313+
// gives every non-reference target the same popup shape, which avoids the
314+
// thin wide popups produced when the detection latched onto a topbar row.
272315
static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float destY) {
273316
RectF mediabox = engine->PageMediabox(destPage);
274317
if (destY < 0.f) {
275-
return FallbackBox(mediabox, destX, destY, kStripHeightPt);
318+
return FixedRatioBox(mediabox, destX, destY);
276319
}
277320

278321
int textLen = 0;
279322
Rect* coords = nullptr;
280323
const WCHAR* text = engine->GetTextForPage(destPage, &textLen, &coords);
281324
if (!text || textLen <= 0 || !coords) {
282-
return FallbackBox(mediabox, destX, destY, kStripHeightPt);
325+
return FixedRatioBox(mediabox, destX, destY);
283326
}
284327

285328
int dY = (int)destY;
@@ -314,7 +357,7 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float
314357
}
315358
}
316359
if (startIdx < 0) {
317-
return FallbackBox(mediabox, destX, destY, kStripHeightPt);
360+
return FixedRatioBox(mediabox, destX, destY);
318361
}
319362

320363
int firstLineLeftX = coords[startIdx].x;
@@ -447,7 +490,7 @@ static RectF DetectEntryBox(EngineBase* engine, int destPage, float destX, float
447490
}
448491
}
449492
if (minX == INT_MAX) {
450-
return FallbackBox(mediabox, destX, destY, kStripHeightPt);
493+
return FixedRatioBox(mediabox, destX, destY);
451494
}
452495

453496
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
467510
box.dy = mediabox.dy - box.y;
468511
}
469512
if (box.dx < 50.f || box.dy < 20.f) {
470-
return FallbackBox(mediabox, destX, destY, kStripHeightPt);
513+
return FixedRatioBox(mediabox, destX, destY);
471514
}
472515
// If detection produced a small box (text-only scan got trapped in one
473516
// fragment of a figure / diagram), expand to a generous strip so the
474517
// surrounding visual content is included in the render.
475518
if (box.dx < 150.f && box.dy < 60.f) {
476-
return FallbackBox(mediabox, destX, destY, kFigureStripHeightPt);
519+
return FigureStripBox(mediabox, destX, destY, kFigureStripHeightPt);
477520
}
478521
return box;
479522
}

0 commit comments

Comments
 (0)