Skip to content

Commit 3d826b4

Browse files
art804DutchSailor
andauthored
fix(search): position match highlights from item geometry (correct at zoom + across text-layer builders) (#299)
* fix(search): position highlights from the matched items, not a visual re-scan Highlights were positioned by re-scanning the DOM for the match text and pairing the Nth stream-order result with the Nth visual-order occurrence; whenever content-stream order differs from reading order (multi-column pages, tables, headers drawn last) the box lands on the wrong occurrence, and matches spanning multiple text items were missed entirely. Build the highlight from the result's own items via span[data-item-index] and a character Range, which is exact in both orders and across item boundaries. Also fix the continuous-mode page selectors (data-page, not data-page-num), which returned null and silently skipped highlight/scroll there. * fix(search): scale highlight rects into the text layer's coordinate space Search highlights are positioned inside .textLayer, which is laid out in unscaled PDF points and zoomed via a CSS matrix transform. The highlight rects were measured with getBoundingClientRect() (visual, post-transform pixels) and applied as-is, so the layer's transform scaled them a second time — at any zoom other than 100% every highlight drifted away from its match, proportionally to its distance from the page origin. Divide the measured offsets and sizes by the layer's effective scale (bounding-rect width over layout width) before positioning. Works in both single-page and continuous mode and is a no-op at scale 1. Also remove findAllMatchPositions(), dead since the switch to item-based positioning. * fix(search): position highlights from item geometry, not DOM spans Highlighting depended on span[data-item-index], which only the custom single-page PDF.js text layer sets. The Rust-extracted vector-mode layer and the stock PDF.js TextLayer (continuous mode) carry no such attribute, so every match on those pages silently produced zero highlights. DOM measurement also broke on zoom: rects captured at one zoom level went stale the moment the layer was rebuilt or re-scaled. Position highlights directly from the matched items' PDF-space geometry (transform/width/height captured at text extraction), converted to layer-local coordinates via --total-scale-factor, which every layer builder sets. Layer-local rects ride the viewport zoom transform, so zooming no longer displaces them. Also sort results by visual position (top-to-bottom, left-to-right) instead of content-stream order, so "1 of N" starts at the top of the page, and remove the now-unused findDomSpanForItem(). * fix(search): start at the visually-first match on the current page The initial current match is chosen during the progressive search, from a page's raw results — which were in content-stream order, so "1 of N" could land mid-page (e.g. starting at 3/14). Sort each page's results visually at discovery time so the starting match is the topmost one on the current page (or the next page with matches). --------- Co-authored-by: Maarten Vroegindeweij <30430941+DutchSailor@users.noreply.github.com>
1 parent 6cdbdf7 commit 3d826b4

2 files changed

Lines changed: 83 additions & 155 deletions

File tree

open-pdf-studio/js/search/find-bar.js

Lines changed: 50 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ async function navigateToResult(result) {
303303

304304
if (getActiveDocument()?.viewMode === 'continuous') {
305305
// Scroll to page in continuous mode
306-
const pageWrapper = document.querySelector(`[data-page-num="${result.pageNum}"]`);
306+
const pageWrapper = document.querySelector(`.page-wrapper[data-page="${result.pageNum}"]`);
307307
if (pageWrapper) {
308308
pageWrapper.scrollIntoView({ behavior: 'smooth', block: 'center' });
309309
}
@@ -395,147 +395,71 @@ export function highlightResults() {
395395
}
396396

397397
/**
398-
* Find all occurrences of search text in the text layer and return their positions
399-
*/
400-
function findAllMatchPositions(textLayer, searchText, matchCase, wholeWord) {
401-
const positions = [];
402-
const textSpans = textLayer.querySelectorAll('span');
403-
const compareSearchText = matchCase ? searchText : searchText.toLowerCase();
404-
405-
for (const span of textSpans) {
406-
const spanText = span.textContent;
407-
if (!spanText) continue;
408-
409-
const compareSpanText = matchCase ? spanText : spanText.toLowerCase();
410-
let startIndex = 0;
411-
412-
while (true) {
413-
const matchIndex = compareSpanText.indexOf(compareSearchText, startIndex);
414-
if (matchIndex === -1) break;
415-
416-
// Whole word check: verify word boundaries in the span text
417-
if (wholeWord) {
418-
const before = matchIndex > 0 ? compareSpanText[matchIndex - 1] : ' ';
419-
const after = matchIndex + compareSearchText.length < compareSpanText.length
420-
? compareSpanText[matchIndex + compareSearchText.length] : ' ';
421-
const isWordChar = (c) => /\w/.test(c);
422-
if (isWordChar(before) || isWordChar(after)) {
423-
startIndex = matchIndex + 1;
424-
continue;
425-
}
426-
}
427-
428-
const textNode = span.firstChild;
429-
if (textNode && textNode.nodeType === Node.TEXT_NODE) {
430-
try {
431-
// Get span's position within the text layer (from its style)
432-
const spanLeft = parseFloat(span.style.left) || 0;
433-
const spanTop = parseFloat(span.style.top) || 0;
434-
435-
// Get the scaleX factor from transform if present
436-
let scaleX = 1;
437-
const transform = span.style.transform;
438-
if (transform) {
439-
const scaleMatch = transform.match(/scaleX\(([^)]+)\)/);
440-
if (scaleMatch) {
441-
scaleX = parseFloat(scaleMatch[1]) || 1;
442-
}
443-
}
444-
445-
// Measure the width of text before the match (in original coordinates)
446-
let preWidth = 0;
447-
if (matchIndex > 0) {
448-
const preRange = document.createRange();
449-
preRange.setStart(textNode, 0);
450-
preRange.setEnd(textNode, matchIndex);
451-
preWidth = preRange.getBoundingClientRect().width;
452-
}
453-
454-
// Measure the width of the match itself
455-
const matchRange = document.createRange();
456-
matchRange.setStart(textNode, matchIndex);
457-
matchRange.setEnd(textNode, matchIndex + searchText.length);
458-
const matchRect = matchRange.getBoundingClientRect();
459-
460-
// Get span's visual bounding rect
461-
const spanRect = span.getBoundingClientRect();
462-
const textLayerRect = span.parentElement.getBoundingClientRect();
463-
464-
// Calculate position relative to text layer using visual coordinates
465-
const highlightLeft = matchRect.left - textLayerRect.left;
466-
const highlightTop = matchRect.top - textLayerRect.top;
467-
468-
// Store position data
469-
positions.push({
470-
span,
471-
highlightLeft,
472-
highlightTop,
473-
matchWidth: matchRect.width,
474-
matchHeight: matchRect.height,
475-
matchIndex,
476-
spanText,
477-
// Also store viewport rect for sorting
478-
viewportTop: matchRect.top,
479-
viewportLeft: matchRect.left
480-
});
481-
} catch (e) {
482-
console.warn('Range error:', e);
483-
}
484-
}
485-
486-
startIndex = matchIndex + 1;
487-
}
488-
}
489-
490-
// Sort by position (top to bottom, left to right)
491-
positions.sort((a, b) => {
492-
if (Math.abs(a.viewportTop - b.viewportTop) > 5) {
493-
return a.viewportTop - b.viewportTop;
494-
}
495-
return a.viewportLeft - b.viewportLeft;
496-
});
497-
498-
return positions;
499-
}
500-
501-
/**
502-
* Highlight search results on a page
398+
* Highlight search results on a page.
399+
*
400+
* Highlights are positioned from the matched items' own PDF-space geometry
401+
* (transform/width/height captured at text extraction), NOT from measuring
402+
* DOM spans. The three text-layer builders (custom single-page PDF.js,
403+
* stock PDF.js TextLayer in continuous mode, Rust-extracted spans in vector
404+
* mode) produce different span structures — only one of them carries
405+
* data-item-index — so any DOM-based lookup breaks on the other two.
406+
* Item geometry is layer-type independent, and because the rects live in
407+
* layer-local coordinates they ride along with the viewport's zoom
408+
* transform instead of needing re-measurement.
503409
*/
504410
function highlightMatch(result, isCurrent) {
505-
if (!result || !result.matchText) return;
411+
if (!result || !result.items || result.items.length === 0) return;
506412

507413
const pageNum = result.pageNum;
414+
const doc = getActiveDocument();
508415

509416
// Get the text layer for this page
510417
let textLayer;
511-
if (getActiveDocument()?.viewMode === 'continuous') {
512-
textLayer = document.querySelector(`[data-page-num="${pageNum}"] .textLayer`);
418+
if (doc?.viewMode === 'continuous') {
419+
const wrapper = document.querySelector(`.page-wrapper[data-page="${pageNum}"]`);
420+
textLayer = wrapper?.querySelector('.textLayer');
513421
} else {
422+
if (doc && doc.currentPage !== pageNum) return;
514423
textLayer = document.querySelector('.textLayer');
515424
}
516-
517425
if (!textLayer) return;
518426

519-
// Find all match positions in the text layer
520-
const positions = findAllMatchPositions(textLayer, result.matchText, state.search.matchCase, state.search.wholeWord);
521-
522-
// Count which occurrence this result is on this page
523-
const pageResults = state.search.results.filter(r => r.pageNum === pageNum);
524-
const occurrenceIndex = pageResults.findIndex(r => r.index === result.index);
525-
526-
if (occurrenceIndex >= 0 && occurrenceIndex < positions.length) {
527-
const pos = positions[occurrenceIndex];
427+
// Layer-local px per PDF point. Every layer builder sets
428+
// --total-scale-factor on the layer or an ancestor: 1 in vector mode
429+
// (layout px = PDF pt, zoom applied via CSS transform), viewport.scale
430+
// for the PDF.js-built layers (laid out at scaled size).
431+
const scale = parseFloat(
432+
getComputedStyle(textLayer).getPropertyValue('--total-scale-factor')
433+
) || 1;
434+
const pageHeightPt = textLayer.offsetHeight / scale;
435+
436+
for (const item of result.items) {
437+
const t = item.transform;
438+
if (!t) continue; // synthetic (Add Text) items carry no geometry
439+
440+
const startInItem = Math.max(0, result.startPos - item.startPos);
441+
const endInItem = Math.min(item.str.length, result.endPos - item.startPos);
442+
if (endInItem <= startInItem) continue;
443+
444+
// Partial matches inside an item: slice the run width proportionally
445+
// by character count. Approximate for proportional fonts, but close
446+
// enough for a highlight and independent of DOM/font availability.
447+
const len = item.str.length || 1;
448+
const itemH = item.height || Math.abs(t[3]) || 10;
449+
const itemW = item.width || 0;
450+
const x0 = t[4] + itemW * (startInItem / len);
451+
const x1 = t[4] + itemW * (endInItem / len);
452+
// t[5] is the baseline; ascent ≈ 0.8em above it (same convention the
453+
// vector-mode span builder uses), Y flipped into top-left space.
454+
const topPt = pageHeightPt - t[5] - itemH * 0.8;
528455

529456
const highlight = document.createElement('div');
530457
highlight.className = 'search-highlight' + (isCurrent ? ' current' : '');
531458
highlight.dataset.resultIndex = result.index;
532-
533-
// Position using calculated visual coordinates
534-
highlight.style.left = pos.highlightLeft + 'px';
535-
highlight.style.top = pos.highlightTop + 'px';
536-
highlight.style.width = pos.matchWidth + 'px';
537-
highlight.style.height = pos.matchHeight + 'px';
538-
459+
highlight.style.left = (x0 * scale) + 'px';
460+
highlight.style.top = (topPt * scale) + 'px';
461+
highlight.style.width = (Math.max(x1 - x0, 2) * scale) + 'px';
462+
highlight.style.height = (itemH * scale) + 'px';
539463
textLayer.appendChild(highlight);
540464
}
541465
}

open-pdf-studio/js/search/find-controller.js

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -128,20 +128,48 @@ function searchPage(pageData, pattern, query) {
128128
);
129129

130130
if (matchItems.length > 0) {
131+
// Visual anchor (PDF space, Y-up) of the first geometric item, used
132+
// to order results top-to-bottom on the page rather than in
133+
// content-stream order.
134+
const anchor = matchItems.find(item => item.transform);
131135
results.push({
132136
pageNum,
133137
startPos,
134138
endPos,
135139
matchText: text.substring(startPos, endPos),
136140
items: matchItems,
141+
anchorX: anchor ? anchor.transform[4] : null,
142+
anchorY: anchor ? anchor.transform[5] : null,
137143
index: 0 // will be re-indexed later
138144
});
139145
}
140146
}
141147

148+
// Sort visually at discovery time, not just in the final pass: the
149+
// progressive search picks the initial current match from a page's raw
150+
// results, and stream order would make "1 of N" land mid-page.
151+
results.sort(compareResultsVisually);
152+
142153
return results;
143154
}
144155

156+
/**
157+
* Order results the way a reader scans the page: by page, then top to
158+
* bottom (PDF Y is up, so larger anchorY first), then left to right.
159+
* Content-stream order (startPos) is only the tiebreaker — streams often
160+
* draw headers/footers/cards out of visual order.
161+
*/
162+
function compareResultsVisually(a, b) {
163+
if (a.pageNum !== b.pageNum) return a.pageNum - b.pageNum;
164+
if (a.anchorY != null && b.anchorY != null) {
165+
if (Math.abs(a.anchorY - b.anchorY) > 2) return b.anchorY - a.anchorY;
166+
if (a.anchorX != null && b.anchorX != null && a.anchorX !== b.anchorX) {
167+
return a.anchorX - b.anchorX;
168+
}
169+
}
170+
return a.startPos - b.startPos;
171+
}
172+
145173
/**
146174
* Build the search regex from query and options
147175
*/
@@ -167,13 +195,12 @@ export async function performSearch(query, options = {}) {
167195
const results = [];
168196

169197
for (const pageData of pagesText) {
170-
const pageResults = searchPage(pageData, pattern, query);
171-
for (const r of pageResults) {
172-
r.index = results.length;
173-
results.push(r);
174-
}
198+
results.push(...searchPage(pageData, pattern, query));
175199
}
176200

201+
results.sort(compareResultsVisually);
202+
results.forEach((r, i) => r.index = i);
203+
177204
return results;
178205
} finally {
179206
state.search.isSearching = false;
@@ -251,7 +278,7 @@ export function executeProgressiveSearch(onProgress) {
251278
textCache.set(docId, pagesText);
252279
}
253280

254-
allResults.sort((a, b) => a.pageNum - b.pageNum || a.startPos - b.startPos);
281+
allResults.sort(compareResultsVisually);
255282
allResults.forEach((r, i) => r.index = i);
256283

257284
onProgress(allResults, totalPages, totalPages, true);
@@ -386,29 +413,6 @@ function replaceInTextEdit(doc, result, replaceText) {
386413
return { type: 'textEdit', id: edit.id, oldText, newText: edit.newText };
387414
}
388415

389-
/**
390-
* Find the DOM span for a search result item by matching dataset.itemIndex.
391-
* This is the ONLY reliable way to map search results to DOM spans.
392-
*/
393-
function findDomSpanForItem(item, pageNum, doc) {
394-
if (item.itemIndex < 0) return null; // synthetic item (text edit)
395-
396-
let textLayer;
397-
if (doc.viewMode === 'continuous') {
398-
const wrapper = document.querySelector(`.page-wrapper[data-page="${pageNum}"]`);
399-
textLayer = wrapper?.querySelector('.textLayer');
400-
} else {
401-
// In single-page mode, verify we're on the correct page
402-
if (doc.currentPage !== pageNum) return null;
403-
textLayer = document.querySelector('.textLayer');
404-
}
405-
if (!textLayer) return null;
406-
407-
// Direct lookup by data-item-index attribute
408-
const span = textLayer.querySelector(`span[data-item-index="${item.itemIndex}"]`);
409-
return span || null;
410-
}
411-
412416
/**
413417
* Replace a match in base PDF content.
414418
*

0 commit comments

Comments
 (0)