Keep zoomed PDF detail text sharp (#36173)

* Keep zoomed PDF detail text at device resolution

* Test PDF detail update dimension-read bounds

* Account for browser layout units in PDF geometry test

config/patches/pdfjs-dist@6.3.289.patch

@@ -0,0 +1,84 @@
+diff --git a/web/pdf_viewer.mjs b/web/pdf_viewer.mjs
+--- a/web/pdf_viewer.mjs
++++ b/web/pdf_viewer.mjs
+@@ -5192,11 +5192,11 @@
+     if (visibleArea.minX < minDetailX || visibleArea.minY < minDetailY || visibleArea.maxX > maxDetailX || visibleArea.maxY > maxDetailY) {
+       return true;
+     }
++    const { scale } = this.pageView.viewport;
+     const {
+-      width: maxWidth,
+-      height: maxHeight,
+-      scale
+-    } = this.pageView.viewport;
++      clientWidth: maxWidth,
++      clientHeight: maxHeight
++    } = this.pageView._ensureCanvasWrapper();
+     if (this.#detailArea.scale !== scale) {
+       return true;
+     }
+@@ -5237,12 +5237,15 @@
+     if (overflowScale < 0) {
+       overflowScale = 0;
+     }
++    const canvasWrapper = this.pageView._ensureCanvasWrapper();
++    const { pixelRatio } = OutputScale;
+     const overflowWidth = visibleWidth * overflowScale;
+     const overflowHeight = visibleHeight * overflowScale;
+-    const minX = Math.max(0, visibleArea.minX - overflowWidth);
+-    const maxX = Math.min(viewport.width, visibleArea.maxX + overflowWidth);
+-    const minY = Math.max(0, visibleArea.minY - overflowHeight);
+-    const maxY = Math.min(viewport.height, visibleArea.maxY + overflowHeight);
++    // Visible bounds and tile coverage share the rounded page's device pixels.
++    const minX = Math.floor(Math.max(0, visibleArea.minX - overflowWidth) * pixelRatio) / pixelRatio;
++    const maxX = Math.ceil(Math.min(canvasWrapper.clientWidth, visibleArea.maxX + overflowWidth) * pixelRatio) / pixelRatio;
++    const minY = Math.floor(Math.max(0, visibleArea.minY - overflowHeight) * pixelRatio) / pixelRatio;
++    const maxY = Math.ceil(Math.min(canvasWrapper.clientHeight, visibleArea.maxY + overflowHeight) * pixelRatio) / pixelRatio;
+     const width = maxX - minX;
+     const height = maxY - minY;
+     this.#detailArea = {
+@@ -5263,11 +5266,9 @@
+       return baseContext;
+     }
+     const {
+-      viewport: {
+-        width: vWidth,
+-        height: vHeight
+-      }
+-    } = this.pageView;
++      clientWidth: vWidth,
++      clientHeight: vHeight
++    } = this.pageView._ensureCanvasWrapper();
+     const {
+       width: aWidth,
+       height: aHeight,
+@@ -5330,16 +5331,22 @@
+     const {
+       pixelRatio
+     } = OutputScale;
+-    const transform = [pixelRatio, 0, 0, pixelRatio, -area.minX * pixelRatio, -area.minY * pixelRatio];
+-    canvas.width = area.width * pixelRatio;
+-    canvas.height = area.height * pixelRatio;
++    const pageWidth = canvasWrapper.clientWidth;
++    const pageHeight = canvasWrapper.clientHeight;
++    const scaleX = pageWidth * pixelRatio / width;
++    const scaleY = pageHeight * pixelRatio / height;
++    const left = Math.round(area.minX * pixelRatio);
++    const top = Math.round(area.minY * pixelRatio);
++    const transform = [scaleX, 0, 0, scaleY, -left, -top];
++    canvas.width = Math.round(area.width * pixelRatio);
++    canvas.height = Math.round(area.height * pixelRatio);
+     const {
+       style
+     } = canvas;
+-    style.width = `${area.width * 100 / width}%`;
+-    style.height = `${area.height * 100 / height}%`;
+-    style.top = `${area.minY * 100 / height}%`;
+-    style.left = `${area.minX * 100 / width}%`;
++    style.width = `${canvas.width * 100 / (pageWidth * pixelRatio)}%`;
++    style.height = `${canvas.height * 100 / (pageHeight * pixelRatio)}%`;
++    style.top = `${top * 100 / (pageHeight * pixelRatio)}%`;
++    style.left = `${left * 100 / (pageWidth * pixelRatio)}%`;
+     const renderingPromise = this._drawCanvas(this._getRenderingContext(canvas, transform), () => {
+       this.canvas?.remove();
+       this.canvas = prevCanvas;

pnpm-lock.yaml

@@ -187,6 +187,7 @@ patchedDependencies:
   monaco-editor@0.55.1: 53b71f7d8a7c00b76ffdb73ce83d18e31692f193f5fafa6a390e48f5b3fdd636
   node-pty@1.1.0: b56279c3729be0e79c21f4d6ab207052ed949e9a792ae326fa9bd473e23347a1
   oxlint-plugin-anti-slop@0.1.2: c10c871702365a0c2c59e9f1c5fb92df97cae5755d534c0a3d83c6368f6feafe
+  pdfjs-dist@6.3.289: 424fde648e16c965ce45c55bdda49ecb3a4aa992c856900714c841c3f13f8415
   sprintf-js@1.1.3: 05316a8b01f155fe19a1e0cb4042c81d43d798a76755c407684e1199a18b1e08
   vitest@5.0.5: 166c034c864f98267e7d479f083b8d37ea9a7b08d6662313dd38e7c69c9a3eb4
   ws@8.22.0: 9393e04dd9a90b01eb7aa35fcd5b1431b5038ce999fbbd82651136193ba51f14
@@ -540,7 +541,7 @@ importers:
         version: 7.0.2005
       pdfjs-dist:
         specifier: ^6.3.289
-        version: 6.3.289
+        version: 6.3.289(patch_hash=424fde648e16c965ce45c55bdda49ecb3a4aa992c856900714c841c3f13f8415)
       playwright-core:
         specifier: 1.64.1
         version: 1.64.1(electron@43.8.1(supports-color@7.2.0))
@@ -14001,7 +14002,7 @@ snapshots:
 
   pathe@2.0.3: {}
 
-  pdfjs-dist@6.3.289:
+  pdfjs-dist@6.3.289(patch_hash=424fde648e16c965ce45c55bdda49ecb3a4aa992c856900714c841c3f13f8415):
     optionalDependencies:
       '@napi-rs/canvas': 1.0.3
 

pnpm-workspace.yaml

@@ -133,3 +133,4 @@ patchedDependencies:
   '@tiptap/extension-list@3.31.3': config/patches/@tiptap__extension-list@3.31.3.patch
   '@tiptap/markdown@3.31.3': config/patches/@tiptap__markdown@3.31.3.patch
   vitest@5.0.5: config/patches/vitest@5.0.5.patch
+  pdfjs-dist@6.3.289: config/patches/pdfjs-dist@6.3.289.patch

tests/e2e/pdf-detail-device-pixels.spec.ts

@@ -0,0 +1,119 @@
+import { rmSync, writeFileSync } from 'node:fs'
+import path from 'node:path'
+import { expect, test } from './helpers/orca-app'
+import { createPdfFindFixture } from './helpers/pdf-find-fixture'
+import {
+  buildPdfScalePreferenceKey,
+  PDF_SCALE_PREFERENCES_STORAGE_KEY
+} from '../../src/renderer/src/components/editor/pdf-scale-preference-storage'
+
+test.use({ orcaAppExtraArgs: ['--force-device-scale-factor=1'] })
+
+test('PDF detail tiles keep device resolution after consecutive capped zooms and scrolling', async ({
+  orcaPage,
+  seededRepoPath,
+  registerPostElectronShutdownCleanup
+}, testInfo) => {
+  // The canvas cap is screen-relative; keep its trigger independent of the host display.
+  const cdp = await orcaPage.context().newCDPSession(orcaPage)
+  await cdp.send('Emulation.setDeviceMetricsOverride', {
+    width: 1280,
+    height: 900,
+    screenWidth: 1920,
+    screenHeight: 1050,
+    deviceScaleFactor: 1,
+    mobile: false
+  })
+  const filePath = path.join(seededRepoPath, 'pdf-detail-device-pixels.pdf')
+  writeFileSync(filePath, createPdfFindFixture())
+  registerPostElectronShutdownCleanup(async () => rmSync(filePath, { force: true }))
+  const worktreeId = await orcaPage.evaluate(() => window.__store?.getState().activeWorktreeId)
+  if (!worktreeId) {
+    throw new Error('Missing fixture worktree')
+  }
+  const preferenceKey = buildPdfScalePreferenceKey({ worktreeId, filePath })
+  await orcaPage.evaluate(
+    ({ filePath, worktreeId, preferenceKey, storageKey }) => {
+      localStorage.setItem(storageKey, JSON.stringify({ [preferenceKey]: 1 }))
+      window.__store?.getState().openFile({
+        filePath,
+        relativePath: 'pdf-detail-device-pixels.pdf',
+        worktreeId,
+        language: 'plaintext',
+        mode: 'edit'
+      })
+    },
+    { filePath, worktreeId, preferenceKey, storageKey: PDF_SCALE_PREFERENCES_STORAGE_KEY }
+  )
+  const page = orcaPage.locator('.pdfViewer .page').first()
+  const canvas = page.locator('.canvasWrapper > canvas').last()
+  await expect(page.locator('.textLayer')).toContainText('needle result 1')
+  expect(await orcaPage.evaluate(() => devicePixelRatio)).toBe(1)
+
+  // Each completed capped step preserves the earlier page rounding before the next zoom.
+  for (let step = 0; step < 6; step += 1) {
+    const previous = await canvas.elementHandle()
+    if (!previous) {
+      throw new Error('Missing rendered canvas')
+    }
+    await orcaPage.getByTitle('Zoom in', { exact: true }).click()
+    await expect.poll(() => canvas.evaluate((current, old) => current !== old, previous)).toBe(true)
+    await expect(page).not.toHaveClass(/loadingIcon/)
+    await previous.dispose()
+  }
+  await expect(orcaPage.getByText('381%', { exact: true })).toBeVisible()
+  await expect(page.locator('.canvasWrapper > canvas')).toHaveCount(2)
+
+  const observations: object[] = []
+  const checkTile = async (): Promise<void> => {
+    const geometry = await canvas.evaluate((element) => {
+      if (!(element instanceof HTMLCanvasElement)) {
+        throw new Error('Missing detail canvas')
+      }
+      const bounds = element.getBoundingClientRect()
+      const wrapper = element.parentElement?.getBoundingClientRect()
+      if (!wrapper) {
+        throw new Error('Missing canvas wrapper')
+      }
+      const actualDPR = devicePixelRatio
+      return {
+        actualDPR,
+        backing: [element.width, element.height],
+        displayed: [bounds.width * actualDPR, bounds.height * actualDPR],
+        offset: [(bounds.left - wrapper.left) * actualDPR, (bounds.top - wrapper.top) * actualDPR]
+      }
+    })
+    // CSS rects use 1/64px layout units; relative origins combine two rect measurements.
+    const layoutQuantum = geometry.actualDPR / 64
+    for (let axis = 0; axis < 2; axis += 1) {
+      expect(Math.abs(geometry.displayed[axis] - geometry.backing[axis])).toBeLessThanOrEqual(
+        layoutQuantum + 1e-7
+      )
+      expect(
+        Math.abs(geometry.offset[axis] - Math.round(geometry.offset[axis]))
+      ).toBeLessThanOrEqual(2 * layoutQuantum + 1e-7)
+    }
+    observations.push(geometry)
+  }
+  await checkTile()
+  const previous = await canvas.elementHandle()
+  if (!previous) {
+    throw new Error('Missing rendered detail canvas')
+  }
+  await page.evaluate((element) => {
+    if (!(element instanceof HTMLElement)) {
+      throw new Error('Missing PDF page element')
+    }
+    const container = element.closest('.pdfViewer')?.parentElement
+    if (!container) {
+      throw new Error('Missing PDF scroll container')
+    }
+    container.scrollLeft = container.scrollWidth
+    container.scrollTop = element.offsetTop + element.clientHeight - container.clientHeight
+  })
+  await expect.poll(() => canvas.evaluate((current, old) => current !== old, previous)).toBe(true)
+  await expect(page).not.toHaveClass(/loadingIcon/)
+  await previous.dispose()
+  await checkTile()
+  writeFileSync(testInfo.outputPath('detail-geometry.json'), JSON.stringify(observations, null, 2))
+})

tests/e2e/pdf-detail-dimension-reads.unit.test.ts

@@ -0,0 +1,132 @@
+// @vitest-environment happy-dom
+
+import type { PDFPageProxy } from 'pdfjs-dist'
+// The legacy entry supplies the same Node polyfills used by the PDF find tests.
+import { getDocument } from 'pdfjs-dist/legacy/build/pdf.mjs'
+import { EventBus, PDFPageView, RenderingStates } from 'pdfjs-dist/web/pdf_viewer.mjs'
+import { afterAll, afterEach, beforeAll, expect, it, vi } from 'vitest'
+import { createPdfFindFixture } from './helpers/pdf-find-fixture'
+
+let loadingTask: ReturnType<typeof getDocument> | undefined
+let pdfPage: PDFPageProxy
+const pageViews: PDFPageView[] = []
+
+beforeAll(async () => {
+  loadingTask = getDocument({ data: new Uint8Array(createPdfFindFixture()) })
+  pdfPage = await (await loadingTask.promise).getPage(1)
+})
+
+afterEach(() => {
+  for (const pageView of pageViews) {
+    pageView.destroy()
+  }
+  pageViews.length = 0
+  vi.restoreAllMocks()
+  vi.unstubAllGlobals()
+})
+
+afterAll(async () => {
+  await loadingTask?.destroy()
+})
+
+it.each([1, 10])('bounds detail update dimension reads across %i eligible owners', (population) => {
+  vi.stubGlobal('devicePixelRatio', 1)
+  const initialArea = { minX: 400, minY: 400, maxX: 500, maxY: 500 }
+  const owners = Array.from({ length: population }, (_, index) => {
+    const pageView = new PDFPageView({
+      id: index + 1,
+      eventBus: new EventBus(),
+      defaultViewport: pdfPage.getViewport({ scale: 1 }),
+      maxCanvasPixels: 1_000_000,
+      capCanvasAreaFactor: -1,
+      enableSelectionRendering: false
+    })
+    pageViews.push(pageView)
+    pageView.setPdfPage(pdfPage)
+    pageView.update({ scale: 4 })
+    pageView._ensureCanvasWrapper()
+    const wrapper = pageView.div.querySelector('.canvasWrapper')
+    if (!(wrapper instanceof HTMLElement)) {
+      throw new Error('Missing PDF canvas wrapper')
+    }
+    const reads = { count: 0 }
+    Object.defineProperties(wrapper, {
+      clientWidth: {
+        get() {
+          reads.count += 1
+          return 3264
+        }
+      },
+      clientHeight: {
+        get() {
+          reads.count += 1
+          return 4224
+        }
+      }
+    })
+    expect(wrapper.clientWidth).toBe(3264)
+    expect(wrapper.clientHeight).toBe(4224)
+    expect(reads.count).toBe(2)
+    pageView.updateVisibleArea(initialArea)
+    const detail = pageView.detailView
+    expect(detail).not.toBeNull()
+    // Only update admission is measured; raster completion is the workload's precondition.
+    detail.renderingState = RenderingStates.FINISHED
+    const reset = vi.spyOn(detail, 'reset')
+    reads.count = 0
+    return { pageView, detail, reset, reads, area: initialArea }
+  })
+  const countReads = (): number => owners.reduce((sum, owner) => sum + owner.reads.count, 0)
+  const clearCounts = (): void => {
+    for (const owner of owners) {
+      owner.reads.count = 0
+      owner.reset.mockClear()
+    }
+  }
+  const updateAll = (): void => {
+    for (const owner of owners) {
+      owner.pageView.updateVisibleArea(owner.area)
+    }
+  }
+  const expectUnchanged = (): void => {
+    for (const owner of owners) {
+      expect(owner.pageView.detailView).toBe(owner.detail)
+      expect(owner.detail.renderingState).toBe(RenderingStates.FINISHED)
+      expect(owner.reset).not.toHaveBeenCalled()
+    }
+  }
+
+  updateAll()
+  expect(countReads()).toBeLessThanOrEqual(2 * population)
+  expectUnchanged()
+
+  clearCounts()
+  owners[0].area = { ...initialArea, minX: 480, maxX: 580 }
+  updateAll()
+  expect(countReads()).toBeLessThanOrEqual(2 * (population - 1) + 4)
+  for (const [index, owner] of owners.entries()) {
+    expect(owner.pageView.detailView).toBe(owner.detail)
+    expect(owner.reset).toHaveBeenCalledTimes(index === 0 ? 1 : 0)
+    expect(owner.detail.renderingState).toBe(
+      index === 0 ? RenderingStates.INITIAL : RenderingStates.FINISHED
+    )
+  }
+
+  owners[0].detail.renderingState = RenderingStates.FINISHED
+  clearCounts()
+  for (let pass = 0; pass < 10; pass += 1) {
+    updateAll()
+  }
+  expect(countReads()).toBeLessThanOrEqual(20 * population)
+  expectUnchanged()
+
+  clearCounts()
+  for (const owner of owners) {
+    owner.pageView.updateVisibleArea(null)
+    expect(owner.pageView.detailView).toBeNull()
+    expect(owner.reset).toHaveBeenCalledTimes(1)
+    owner.pageView.updateVisibleArea(null)
+    expect(owner.reset).toHaveBeenCalledTimes(1)
+  }
+  expect(countReads()).toBe(0)
+})