From 436850c6390ba2ae586046c4bcdf9e29f24425f7 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 2 Apr 2026 03:55:32 -0400 Subject: [PATCH 1/3] fix(pdf-server): import highlight/underline/strike from existing PDFs importPdfjsAnnotation expected nested quadPoints arrays, but pdf.js emits a flat Float32Array (8 numbers per quad). Iterating it yielded numbers, qp.length was undefined, rects stayed empty, and the function returned null - so every quad-based annotation in a loaded PDF was dropped from annotationMap and rendered only as unselectable canvas pixels. Shipped this way in #506; not a regression. Parse the flat array in 8-stride chunks, fall through to ann.rect if that yields nothing. Existing tests used the (wrong) nested fixture shape; switched them to Float32Array and added a 2-quad case + a rect-fallback case. --- .../pdf-server/src/pdf-annotations.test.ts | 53 +++++++++++++++++-- examples/pdf-server/src/pdf-annotations.ts | 47 ++++++++-------- 2 files changed, 71 insertions(+), 29 deletions(-) diff --git a/examples/pdf-server/src/pdf-annotations.test.ts b/examples/pdf-server/src/pdf-annotations.test.ts index fe34d835..d0fabcbb 100644 --- a/examples/pdf-server/src/pdf-annotations.test.ts +++ b/examples/pdf-server/src/pdf-annotations.test.ts @@ -460,7 +460,8 @@ describe("importPdfjsAnnotation", () => { annotationType: 9, ref: { num: 5, gen: 0 }, rect: [72, 700, 272, 712], - quadPoints: [[72, 712, 272, 712, 72, 700, 272, 700]], + // pdf.js emits a FLAT Float32Array, not nested arrays. + quadPoints: new Float32Array([72, 712, 272, 712, 72, 700, 272, 700]), color: new Uint8ClampedArray([255, 255, 0]), contentsObj: { str: "Important" }, }; @@ -474,12 +475,57 @@ describe("importPdfjsAnnotation", () => { expect((result as any).color).toBe("#ffff00"); }); + it("imports a multi-line highlight (multiple quads → multiple rects)", () => { + // Regression: the parser iterated quadPoints as if nested; pdf.js's + // flat array yielded numbers, so rects stayed empty and import bailed. + const ann = { + annotationType: 9, + ref: { num: 8, gen: 0 }, + quadPoints: new Float32Array([ + // line 1 + 72, 712, 272, 712, 72, 700, 272, 700, + // line 2 + 72, 698, 200, 698, 72, 686, 200, 686, + ]), + color: new Uint8ClampedArray([255, 255, 0]), + }; + const result = importPdfjsAnnotation(ann, 1, 0); + expect(result).not.toBeNull(); + expect(result!.type).toBe("highlight"); + expect((result as any).rects).toHaveLength(2); + expect((result as any).rects[0]).toEqual({ + x: 72, + y: 700, + width: 200, + height: 12, + }); + expect((result as any).rects[1]).toEqual({ + x: 72, + y: 686, + width: 128, + height: 12, + }); + }); + + it("falls back to ann.rect when quadPoints is absent", () => { + const ann = { + annotationType: 9, + ref: { num: 9, gen: 0 }, + rect: [72, 700, 272, 712], + color: new Uint8ClampedArray([255, 255, 0]), + }; + const result = importPdfjsAnnotation(ann, 1, 0); + expect(result).not.toBeNull(); + expect((result as any).rects).toHaveLength(1); + }); + it("imports an underline annotation", () => { const ann = { annotationType: 10, ref: { num: 6, gen: 0 }, rect: [72, 700, 272, 712], - quadPoints: [[72, 712, 272, 712, 72, 700, 272, 700]], + // pdf.js emits a FLAT Float32Array, not nested arrays. + quadPoints: new Float32Array([72, 712, 272, 712, 72, 700, 272, 700]), color: new Uint8ClampedArray([255, 0, 0]), }; const result = importPdfjsAnnotation(ann, 1, 0); @@ -493,7 +539,8 @@ describe("importPdfjsAnnotation", () => { annotationType: 12, ref: { num: 7, gen: 0 }, rect: [72, 700, 272, 712], - quadPoints: [[72, 712, 272, 712, 72, 700, 272, 700]], + // pdf.js emits a FLAT Float32Array, not nested arrays. + quadPoints: new Float32Array([72, 712, 272, 712, 72, 700, 272, 700]), color: new Uint8ClampedArray([255, 0, 0]), }; const result = importPdfjsAnnotation(ann, 2, 0); diff --git a/examples/pdf-server/src/pdf-annotations.ts b/examples/pdf-server/src/pdf-annotations.ts index 25d9ebb6..6a7e268e 100644 --- a/examples/pdf-server/src/pdf-annotations.ts +++ b/examples/pdf-server/src/pdf-annotations.ts @@ -948,33 +948,28 @@ export function importPdfjsAnnotation( case "highlight": case "underline": case "strikethrough": { - // PDF.js provides quadPoints as array of arrays [[x1,y1,x2,y2,...], ...] - // or rect as [x1,y1,x2,y2] - let rects: Rect[]; - if (ann.quadPoints && ann.quadPoints.length > 0) { - rects = []; - for (const qp of ann.quadPoints) { - // Each quadPoint is [x1,y1,x2,y2,x3,y3,x4,y4] - // We need the bounding box - if (qp.length >= 8) { - const xs = [qp[0], qp[2], qp[4], qp[6]]; - const ys = [qp[1], qp[3], qp[5], qp[7]]; - const minX = Math.min(...xs); - const minY = Math.min(...ys); - const maxX = Math.max(...xs); - const maxY = Math.max(...ys); - rects.push({ - x: minX, - y: minY, - width: maxX - minX, - height: maxY - minY, - }); - } + // pdf.js emits quadPoints as a FLAT Float32Array [x1,y1,...,x4,y4, …] + // (8 numbers per quad), NOT as nested arrays. Iterating it yields + // numbers, so the old `for (const qp of …) if (qp.length>=8)` never + // matched and every quad-based annotation was dropped (#506). + const rects: Rect[] = []; + const qp = ann.quadPoints as ArrayLike | undefined; + if (qp && qp.length >= 8) { + for (let i = 0; i + 8 <= qp.length; i += 8) { + const xs = [qp[i], qp[i + 2], qp[i + 4], qp[i + 6]]; + const ys = [qp[i + 1], qp[i + 3], qp[i + 5], qp[i + 7]]; + const minX = Math.min(...xs); + const minY = Math.min(...ys); + rects.push({ + x: minX, + y: minY, + width: Math.max(...xs) - minX, + height: Math.max(...ys) - minY, + }); } - } else if (ann.rect) { - rects = [pdfjsRectToRect(ann.rect)]; - } else { - return null; + } + if (rects.length === 0 && ann.rect) { + rects.push(pdfjsRectToRect(ann.rect)); } if (rects.length === 0) return null; From 2f989a757d3aa517fa427c7fd59b462326599bfc Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 2 Apr 2026 04:02:49 -0400 Subject: [PATCH 2/3] fix(pdf-server): own markup annotations in our layer, not the canvas page.render() defaulted to AnnotationMode.ENABLE, painting every annotation's appearance stream onto the canvas. We ALSO import them into annotationMap and render DOM versions in #annotation-layer, so the user saw two representations: an unclickable canvas pixel that looked right, and our clickable DOM element that looked like our styling. Clicking the 'real' one did nothing. Set annotationMode: DISABLE so the canvas is page-content-only and our layer is the single source of truth for markup. Imported annotations now behave identically to user-created ones (select/drag/delete). Form widgets are unaffected (#form-layer via AnnotationLayer.render). get_screenshot keeps ENABLE_STORAGE so screenshots still show annotations. Known trade-offs: - Stamps with image appearance streams degrade to our text-label render (no rasterization yet). - Annotation types we don't import (Ink, Polygon, Caret, ...) no longer ghost on the canvas - they're invisible. They were never editable anyway; loadBaselineAnnotations already logs them. --- examples/pdf-server/src/mcp-app.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/examples/pdf-server/src/mcp-app.ts b/examples/pdf-server/src/mcp-app.ts index fe30b14f..8094fd31 100644 --- a/examples/pdf-server/src/mcp-app.ts +++ b/examples/pdf-server/src/mcp-app.ts @@ -3158,11 +3158,20 @@ async function renderPage() { // Set --scale-factor so CSS font-size/transform rules work correctly. textLayerEl.style.setProperty("--scale-factor", `${scale}`); - // Render canvas - track the task so we can cancel it + // Render canvas - track the task so we can cancel it. + // + // annotationMode: DISABLE — markup annotations are owned by OUR + // annotation layer (renderAnnotationsForPage). Letting page.render + // paint them too gave a doubled-up display where the canvas pixel + // looked like the annotation but wasn't clickable, and our DOM + // version (which IS clickable) sat on top with our own styling. + // Form widgets are unaffected — those live in #form-layer via + // AnnotationLayer.render(), not the canvas. // eslint-disable-next-line @typescript-eslint/no-explicit-any const renderTask = (page.render as any)({ canvasContext: ctx, viewport, + annotationMode: AnnotationMode.DISABLE, }); currentRenderTask = renderTask; From ba71bd1c3fafc12ed1bce93f339bae981956e215 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 2 Apr 2026 04:05:21 -0400 Subject: [PATCH 3/3] Revert "fix(pdf-server): own markup annotations in our layer, not the canvas" This reverts commit 2f989a757d3aa517fa427c7fd59b462326599bfc. --- examples/pdf-server/src/mcp-app.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/examples/pdf-server/src/mcp-app.ts b/examples/pdf-server/src/mcp-app.ts index 8094fd31..fe30b14f 100644 --- a/examples/pdf-server/src/mcp-app.ts +++ b/examples/pdf-server/src/mcp-app.ts @@ -3158,20 +3158,11 @@ async function renderPage() { // Set --scale-factor so CSS font-size/transform rules work correctly. textLayerEl.style.setProperty("--scale-factor", `${scale}`); - // Render canvas - track the task so we can cancel it. - // - // annotationMode: DISABLE — markup annotations are owned by OUR - // annotation layer (renderAnnotationsForPage). Letting page.render - // paint them too gave a doubled-up display where the canvas pixel - // looked like the annotation but wasn't clickable, and our DOM - // version (which IS clickable) sat on top with our own styling. - // Form widgets are unaffected — those live in #form-layer via - // AnnotationLayer.render(), not the canvas. + // Render canvas - track the task so we can cancel it // eslint-disable-next-line @typescript-eslint/no-explicit-any const renderTask = (page.render as any)({ canvasContext: ctx, viewport, - annotationMode: AnnotationMode.DISABLE, }); currentRenderTask = renderTask;