fix: render visible watermarks with libpdf and Noto Sans#2086
Open
fix: render visible watermarks with libpdf and Noto Sans#2086
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughRefactors the annotate-document API to use Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/api/mupdf/annotate-document.ts (1)
427-439:⚠️ Potential issue | 🟠 MajorAlways clear the fetch timeout in a
finallyblock.If
fetchthrows before Line 438,timeoutIdis never cleared. Under failure bursts, this leaves many pending timers alive until they fire.💡 Proposed fix
const fetchStart = Date.now(); const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), 60000); - - response = await fetch(validatedUrl.toString(), { - signal: controller.signal, - headers: { - Accept: "application/pdf", - }, - }); - - clearTimeout(timeoutId); + try { + response = await fetch(validatedUrl.toString(), { + signal: controller.signal, + headers: { + Accept: "application/pdf", + }, + }); + } finally { + clearTimeout(timeoutId); + }Also applies to: 445-458
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/api/mupdf/annotate-document.ts` around lines 427 - 439, The fetch timeout (timeoutId) must be cleared in a finally block so it is always removed even if fetch throws; update the code around the fetch call that uses controller/timeoutId (the block that sets const controller = new AbortController(); const timeoutId = setTimeout(...); response = await fetch(validatedUrl.toString(), { signal: controller.signal, ... })) to wrap the fetch in try { ... } finally { clearTimeout(timeoutId); } and also ensure controller.abort() is not left dangling (you can still call controller.abort() from the timer callback). Apply the same try/finally pattern to the second fetch block (the similar code at the later fetch around lines 445-458) so both timeoutId variables are always cleared.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pages/api/mupdf/annotate-document.ts`:
- Around line 270-277: The handler currently allows fractional numPages (e.g.,
1.2) which rounds up when used later; fix this by normalizing and validating
numPages at the start of the exported function (e.g., in annotateDocument or the
top of this module) — compute const integerNumPages = Math.max(0,
Math.floor(Number(numPages))) and then use integerNumPages everywhere (replace
numPages in the invalid-page check and in pagesToProcess calculation), and throw
or handle as before if integerNumPages > sourcePageCount; this ensures
fractional inputs are coerced to an integer and prevents extra pages being
processed.
- Around line 509-515: The branch that checks errorMessage currently groups
"Failed to load Noto Sans font" with client errors; update the conditional so
font-load failures are treated as server errors: remove "Failed to load Noto
Sans font" from the Invalid-request branch and add a separate branch that checks
errorMessage.includes("Failed to load Noto Sans font") (or more general font
load messages) and sets statusCode = 500 and errorType = "Server error" (keeping
the existing variables statusCode and errorType used in this handler) so
font/configuration failures return 5xx instead of 400.
---
Outside diff comments:
In `@pages/api/mupdf/annotate-document.ts`:
- Around line 427-439: The fetch timeout (timeoutId) must be cleared in a
finally block so it is always removed even if fetch throws; update the code
around the fetch call that uses controller/timeoutId (the block that sets const
controller = new AbortController(); const timeoutId = setTimeout(...); response
= await fetch(validatedUrl.toString(), { signal: controller.signal, ... })) to
wrap the fetch in try { ... } finally { clearTimeout(timeoutId); } and also
ensure controller.abort() is not left dangling (you can still call
controller.abort() from the timer callback). Apply the same try/finally pattern
to the second fetch block (the similar code at the later fetch around lines
445-458) so both timeoutId variables are always cleared.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/fonts/NotoSans-Regular.ttfis excluded by!**/*.ttf
📒 Files selected for processing (1)
pages/api/mupdf/annotate-document.ts
Comment on lines
+270
to
+277
| if (numPages > sourcePageCount) { | ||
| throw new Error( | ||
| `Invalid page count: requested ${numPages}, document has ${sourcePageCount}`, | ||
| ); | ||
| } | ||
|
|
||
| const pagesToProcess = Math.min(numPages, sourcePageCount); | ||
|
|
Contributor
There was a problem hiding this comment.
Guard against fractional numPages to prevent extra pages being watermarked.
At Line 276 and Line 324, a fractional value (for example 1.2) results in 2 pages being processed. Since this function is exported and callable outside this handler, it should enforce integer input itself.
💡 Proposed fix
const sourcePageCount = pdfDoc.getPageCount();
+ if (!Number.isInteger(numPages) || numPages <= 0) {
+ throw new Error("Invalid page count: must be a positive integer");
+ }
if (numPages > sourcePageCount) {
throw new Error(
`Invalid page count: requested ${numPages}, document has ${sourcePageCount}`,
);
}Also applies to: 324-324
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pages/api/mupdf/annotate-document.ts` around lines 270 - 277, The handler
currently allows fractional numPages (e.g., 1.2) which rounds up when used
later; fix this by normalizing and validating numPages at the start of the
exported function (e.g., in annotateDocument or the top of this module) —
compute const integerNumPages = Math.max(0, Math.floor(Number(numPages))) and
then use integerNumPages everywhere (replace numPages in the invalid-page check
and in pagesToProcess calculation), and throw or handle as before if
integerNumPages > sourcePageCount; this ensures fractional inputs are coerced to
an integer and prevents extra pages being processed.
pages/api/mupdf/annotate-document.ts
Outdated
Comment on lines
+509
to
+515
| } else if ( | ||
| errorMessage.includes("Invalid page count") || | ||
| errorMessage.includes("Watermark text is empty") || | ||
| errorMessage.includes("Failed to load Noto Sans font") | ||
| ) { | ||
| statusCode = 400; | ||
| errorType = "Invalid request"; |
Contributor
There was a problem hiding this comment.
Map font-load failures to 5xx, not 400.
Failed to load Noto Sans font is a server/runtime configuration failure, not invalid client input. Returning 400 can mislead clients and hide operational issues.
💡 Proposed fix
} else if (
errorMessage.includes("Invalid page count") ||
- errorMessage.includes("Watermark text is empty") ||
- errorMessage.includes("Failed to load Noto Sans font")
+ errorMessage.includes("Watermark text is empty")
) {
statusCode = 400;
errorType = "Invalid request";
+ } else if (errorMessage.includes("Failed to load Noto Sans font")) {
+ statusCode = 500;
+ errorType = "Watermark service misconfigured";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } else if ( | |
| errorMessage.includes("Invalid page count") || | |
| errorMessage.includes("Watermark text is empty") || | |
| errorMessage.includes("Failed to load Noto Sans font") | |
| ) { | |
| statusCode = 400; | |
| errorType = "Invalid request"; | |
| } else if ( | |
| errorMessage.includes("Invalid page count") || | |
| errorMessage.includes("Watermark text is empty") | |
| ) { | |
| statusCode = 400; | |
| errorType = "Invalid request"; | |
| } else if (errorMessage.includes("Failed to load Noto Sans font")) { | |
| statusCode = 500; | |
| errorType = "Watermark service misconfigured"; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pages/api/mupdf/annotate-document.ts` around lines 509 - 515, The branch that
checks errorMessage currently groups "Failed to load Noto Sans font" with client
errors; update the conditional so font-load failures are treated as server
errors: remove "Failed to load Noto Sans font" from the Invalid-request branch
and add a separate branch that checks errorMessage.includes("Failed to load Noto
Sans font") (or more general font load messages) and sets statusCode = 500 and
errorType = "Server error" (keeping the existing variables statusCode and
errorType used in this handler) so font/configuration failures return 5xx
instead of 400.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pages/api/mupdf/annotate-document.ts (2)
459-472:⚠️ Potential issue | 🟡 MinorServer misconfiguration (
No distribution hosts configured) returns400.
validateUrlthrows"No distribution hosts configured"when no environment variables are set. This is caught by the dedicated URL try/catch at Line 461 and returned as a400 Invalid URLto the client, masking a server-side misconfiguration as a client error.💡 Proposed fix
} catch (error) { const errorMsg = error instanceof Error ? error.message : String(error); + const isServerConfig = errorMsg.includes("No distribution hosts configured"); log({ message: `URL validation failed: ${errorMsg}\nAttempted URL: ${url}`, type: "error", mention: false, }); + if (isServerConfig) { + return res.status(500).json({ error: "Server misconfiguration", details: errorMsg }); + } return res.status(400).json({ error: "Invalid URL", details: errorMsg, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/api/mupdf/annotate-document.ts` around lines 459 - 472, The current try/catch around validateUrl(url) in annotate-document.ts treats all thrown errors as client 400s; detect server-side misconfiguration errors (e.g. the thrown message "No distribution hosts configured" or a specific ServerConfigError if available) and return a 500 instead. Update the catch to inspect the caught error (error instanceof Error ? error.message : String(error)) and if it matches the misconfiguration indicator, log it as a server error and send res.status(500).json({ error: "Server misconfiguration", details: errorMsg }); otherwise keep the existing 400 Invalid URL response; refer to validateUrl and the surrounding URL validation try/catch in the request handler.
138-141:⚠️ Potential issue | 🟡 Minor
opacityJSDoc comment says0 to 0.8but the clamp uses[0, 1].The
WatermarkConfiginterface documents// 0 to 0.8whilecreateEmbeddedWatermarkPageclamps opacity toMath.max(0, Math.min(watermarkConfig.opacity, 1))(Line 236). Callers relying on the interface comment won't know that values above 0.8 are actually accepted.💡 Proposed fix
- opacity: number; // 0 to 0.8 + opacity: number; // 0 to 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/api/mupdf/annotate-document.ts` around lines 138 - 141, The JSDoc for WatermarkConfig.opacity mismatches the runtime clamp in createEmbeddedWatermarkPage (the code clamps to [0,1] but the comment says "0 to 0.8"); update the JSDoc on opacity to "0 to 1" to reflect the actual accepted range (or if you want to enforce the intended 0.8 max, change the clamp in createEmbeddedWatermarkPage to Math.max(0, Math.min(watermarkConfig.opacity, 0.8))); reference WatermarkConfig.opacity and createEmbeddedWatermarkPage when making the change so the docs and runtime behavior stay consistent.
♻️ Duplicate comments (2)
pages/api/mupdf/annotate-document.ts (2)
314-322:⚠️ Potential issue | 🟡 MinorFractional
numPagesis still not guarded inannotatePdfWithEmbeddedWatermark.
numPagesis accepted asnumberand only checked againstsourcePageCount; a fractional value (e.g.,1.7) makespagesToProcess = 1.7, causing theforloop to process 2 pages unexpectedly. The handler check at Line 445 (typeof numPages !== "number" || numPages <= 0) also does not enforce an integer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/api/mupdf/annotate-document.ts` around lines 314 - 322, The function annotatePdfWithEmbeddedWatermark accepts numPages as a number but doesn't enforce it being an integer, so fractional values (e.g., 1.7) make pagesToProcess fractional and break the page loop; update validation in annotatePdfWithEmbeddedWatermark (and the request handler check that currently tests typeof numPages !== "number" || numPages <= 0) to require an integer (use Number.isInteger or coerce via Math.floor with explicit validation) and ensure pagesToProcess is an integer (e.g., Math.min(sourcePageCount, Math.floor(numPages))) before the for loop to prevent processing fractional pages.
65-83:⚠️ Potential issue | 🟠 MajorFont-load failure is still mapped to
400 Invalid request.This is unaddressed from a previous review: a filesystem failure loading
NotoSans-Regular.ttfis a server-side configuration error, not a client error.💡 Proposed fix
.catch((error) => { notoSansFontBytesPromise = null; const message = error instanceof Error ? error.message : String(error); throw new AnnotateDocumentError({ message: `Failed to load Noto Sans font: ${message}`, - statusCode: 400, - errorType: "Invalid request", + statusCode: 500, + errorType: "Failed to apply watermark", }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/api/mupdf/annotate-document.ts` around lines 65 - 83, The font-load failure in getNotoSansFontBytes currently maps filesystem errors to a 400/“Invalid request”; change the error mapping to a server-side error (e.g., statusCode: 500 and errorType: "Server error") so failures reading NOTO_SANS_FONT_PATH are reported as internal server errors. Update the catch block that resets notoSansFontBytesPromise and throws AnnotateDocumentError to use the new statusCode and errorType, and ensure the thrown message still includes the original error.message for diagnostics.
🧹 Nitpick comments (3)
pages/api/mupdf/annotate-document.ts (3)
529-538:pagesToProcesson Line 535 is redundant.It mirrors
numPagesverbatim and is only used once in the log on Line 538.💡 Proposed fix
- const pagesToProcess = numPages; - console.log( - `Total processing time: ${Date.now() - startTime}ms for ${pagesToProcess} pages`, + `Total processing time: ${Date.now() - startTime}ms for ${numPages} pages`, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/api/mupdf/annotate-document.ts` around lines 529 - 538, The variable pagesToProcess is redundant — remove its declaration and use numPages directly in the console.log; update the annotatePdfWithEmbeddedWatermark usage remains the same (annotatedPdf, annotatePdfWithEmbeddedWatermark) and change the log to reference numPages and startTime instead of pagesToProcess.
328-332:rawWatermarkText→watermarkTextis a no-op assignment.Line 332 re-assigns without transformation. Either consolidate or remove the intermediate variable.
💡 Proposed fix
- const rawWatermarkText = safeTemplateReplace( + const watermarkText = safeTemplateReplace( watermarkConfig.text, viewerData, ); - const watermarkText = rawWatermarkText;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/api/mupdf/annotate-document.ts` around lines 328 - 332, The assignment rawWatermarkText → watermarkText is redundant; replace the two-step declaration with a single binding by assigning the result of safeTemplateReplace directly to watermarkText (e.g., const watermarkText = safeTemplateReplace(watermarkConfig.text, viewerData)); remove rawWatermarkText and ensure any later references use watermarkText. If other code relied on rawWatermarkText, rename usages accordingly to watermarkText.
499-513: AbortError is detected via fragile string matching.
errorMsg.includes("aborted")(Line 507) relies on the error message text, which varies across runtimes and locales. Prefer checkingerror.name === "AbortError"orerror instanceof DOMExceptionbefore falling through to the string check.💡 Proposed fix
- if (errorMsg.includes("aborted")) { + if ( + (error instanceof Error && error.name === "AbortError") || + errorMsg.includes("aborted") + ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/api/mupdf/annotate-document.ts` around lines 499 - 513, The current abort detection uses a fragile string match on errorMsg; update the catch to detect aborts by checking the actual error type/name first (e.g., if error instanceof DOMException or error?.name === "AbortError") before falling back to the existing string check, and if matched throw the same AnnotateDocumentError with the 504/Request timeout payload; locate this logic in the catch block where error is captured and where AnnotateDocumentError is thrown to replace the brittle includes("aborted") condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pages/api/mupdf/annotate-document.ts`:
- Around line 247-272: The tiling calculation uses patternWidth = safeTextWidth
/ 1.1 which makes horizontal spacing smaller than the text and causes heavy
overlap; update the tiling logic in the block guarded by watermarkConfig.isTiled
to set patternWidth to be larger than safeTextWidth (e.g., multiply by 1.5 or
add explicit horizontal padding) so each tile has clear separation, and keep
patternHeight logic or adjust similarly if needed; ensure you only change the
assignment of patternWidth (referenced in patternWidth, safeTextWidth,
watermarkPage.drawText and getRotateOptions) so the for-loops produce
non-overlapping horizontal tiles.
- Around line 3-5: The dependency `@libpdf/core` is currently using a caret range
which allows minor updates that can break pre-1.0 APIs; update package.json to
pin the version to an exact release (e.g., "0.2.8") instead of "^0.2.8" to
ensure stability for imports like PDF and rgb used in
pages/api/mupdf/annotate-document.ts, then run npm install/yarn to update
lockfile and verify the build/tests still pass.
---
Outside diff comments:
In `@pages/api/mupdf/annotate-document.ts`:
- Around line 459-472: The current try/catch around validateUrl(url) in
annotate-document.ts treats all thrown errors as client 400s; detect server-side
misconfiguration errors (e.g. the thrown message "No distribution hosts
configured" or a specific ServerConfigError if available) and return a 500
instead. Update the catch to inspect the caught error (error instanceof Error ?
error.message : String(error)) and if it matches the misconfiguration indicator,
log it as a server error and send res.status(500).json({ error: "Server
misconfiguration", details: errorMsg }); otherwise keep the existing 400 Invalid
URL response; refer to validateUrl and the surrounding URL validation try/catch
in the request handler.
- Around line 138-141: The JSDoc for WatermarkConfig.opacity mismatches the
runtime clamp in createEmbeddedWatermarkPage (the code clamps to [0,1] but the
comment says "0 to 0.8"); update the JSDoc on opacity to "0 to 1" to reflect the
actual accepted range (or if you want to enforce the intended 0.8 max, change
the clamp in createEmbeddedWatermarkPage to Math.max(0,
Math.min(watermarkConfig.opacity, 0.8))); reference WatermarkConfig.opacity and
createEmbeddedWatermarkPage when making the change so the docs and runtime
behavior stay consistent.
---
Duplicate comments:
In `@pages/api/mupdf/annotate-document.ts`:
- Around line 314-322: The function annotatePdfWithEmbeddedWatermark accepts
numPages as a number but doesn't enforce it being an integer, so fractional
values (e.g., 1.7) make pagesToProcess fractional and break the page loop;
update validation in annotatePdfWithEmbeddedWatermark (and the request handler
check that currently tests typeof numPages !== "number" || numPages <= 0) to
require an integer (use Number.isInteger or coerce via Math.floor with explicit
validation) and ensure pagesToProcess is an integer (e.g.,
Math.min(sourcePageCount, Math.floor(numPages))) before the for loop to prevent
processing fractional pages.
- Around line 65-83: The font-load failure in getNotoSansFontBytes currently
maps filesystem errors to a 400/“Invalid request”; change the error mapping to a
server-side error (e.g., statusCode: 500 and errorType: "Server error") so
failures reading NOTO_SANS_FONT_PATH are reported as internal server errors.
Update the catch block that resets notoSansFontBytesPromise and throws
AnnotateDocumentError to use the new statusCode and errorType, and ensure the
thrown message still includes the original error.message for diagnostics.
---
Nitpick comments:
In `@pages/api/mupdf/annotate-document.ts`:
- Around line 529-538: The variable pagesToProcess is redundant — remove its
declaration and use numPages directly in the console.log; update the
annotatePdfWithEmbeddedWatermark usage remains the same (annotatedPdf,
annotatePdfWithEmbeddedWatermark) and change the log to reference numPages and
startTime instead of pagesToProcess.
- Around line 328-332: The assignment rawWatermarkText → watermarkText is
redundant; replace the two-step declaration with a single binding by assigning
the result of safeTemplateReplace directly to watermarkText (e.g., const
watermarkText = safeTemplateReplace(watermarkConfig.text, viewerData)); remove
rawWatermarkText and ensure any later references use watermarkText. If other
code relied on rawWatermarkText, rename usages accordingly to watermarkText.
- Around line 499-513: The current abort detection uses a fragile string match
on errorMsg; update the catch to detect aborts by checking the actual error
type/name first (e.g., if error instanceof DOMException or error?.name ===
"AbortError") before falling back to the existing string check, and if matched
throw the same AnnotateDocumentError with the 504/Request timeout payload;
locate this logic in the catch block where error is captured and where
AnnotateDocumentError is thrown to replace the brittle includes("aborted")
condition.
Comment on lines
+3
to
+5
| import { PDF, rgb } from "@libpdf/core"; | ||
| import { readFile } from "node:fs/promises"; | ||
| import path from "node:path"; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
grep -n "@libpdf/core" package.jsonRepository: mfts/papermark
Length of output: 90
🌐 Web query:
@libpdf/core npm package beta status stable version
💡 Result:
- Current published (“latest”) version:
@libpdf/core@0.2.7. [1] - Beta vs stable: It’s not at a 1.x “stable” SemVer release yet (still 0.x, i.e., generally treated as pre-1.0 / potentially breaking changes). [1]
- “Stable” (≥1.0.0) version: I couldn’t find evidence of any 1.0.0+ release for
@libpdf/corein public metadata; the official docs even show an example using^1.0.0, which appears aspirational / forward-looking rather than reflecting a currently published stable release. [2]
Sources: [1] jsDelivr package page for @libpdf/core (shows version 0.2.7). [2] LibPDF docs page showing @libpdf/core usage and an example referencing ^1.0.0.
Pin @libpdf/core to an exact version. The package is currently declared as "@libpdf/core": "^0.2.8" in package.json, which permits minor version updates. Since the library is still at 0.x (pre-1.0), minor version increments may include breaking API changes. Change to "@libpdf/core": "0.2.8" (or "0.2.7" if preferred) to ensure predictable behavior and prevent silent breaking changes to the watermarking functionality.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pages/api/mupdf/annotate-document.ts` around lines 3 - 5, The dependency
`@libpdf/core` is currently using a caret range which allows minor updates that
can break pre-1.0 APIs; update package.json to pin the version to an exact
release (e.g., "0.2.8") instead of "^0.2.8" to ensure stability for imports like
PDF and rgb used in pages/api/mupdf/annotate-document.ts, then run npm
install/yarn to update lockfile and verify the build/tests still pass.
Comment on lines
+247
to
272
| if (watermarkConfig.isTiled) { | ||
| const safeTextWidth = Math.max(textWidth, 1); | ||
| const safeTextHeight = Math.max(textHeight, 1); | ||
|
|
||
| const patternWidth = safeTextWidth / 1.1; | ||
| const patternHeight = safeTextHeight * 15; | ||
|
|
||
| // Calculate the offset to center the pattern | ||
| const offsetX = -patternWidth / 4; | ||
| const offsetY = -patternHeight / 4; | ||
|
|
||
| const maxTilesPerRow = Math.ceil(width / patternWidth) + 1; | ||
| const maxTilesPerColumn = Math.ceil(height / patternHeight) + 1; | ||
|
|
||
| for (let i = 0; i < maxTilesPerRow; i++) { | ||
| for (let j = 0; j < maxTilesPerColumn; j++) { | ||
| const x = i * patternWidth + offsetX; | ||
| const y = j * patternHeight + offsetY; | ||
|
|
||
| try { | ||
| page.drawText(watermarkText, { | ||
| x, | ||
| y, | ||
| size: fontSize, | ||
| font, | ||
| color: hexToRgb(config.color) ?? rgb(0, 0, 0), | ||
| opacity: config.opacity, | ||
| rotate: degrees(config.rotation), | ||
| }); | ||
| } catch (error) { | ||
| console.error("Error drawing tiled watermark text:", error); | ||
| throw new Error( | ||
| `Failed to apply watermark to page ${pageIndex + 1}: ${error}`, | ||
| ); | ||
| } | ||
| const maxTilesPerRow = Math.ceil(pageWidth / patternWidth) + 1; | ||
| const maxTilesPerColumn = Math.ceil(pageHeight / patternHeight) + 1; | ||
|
|
||
| for (let row = 0; row < maxTilesPerRow; row++) { | ||
| for (let col = 0; col < maxTilesPerColumn; col++) { | ||
| const x = row * patternWidth + offsetX; | ||
| const y = col * patternHeight + offsetY; | ||
|
|
||
| watermarkPage.drawText(watermarkText, { | ||
| ...drawOptions, | ||
| x, | ||
| y, | ||
| rotate: getRotateOptions(x, y), | ||
| }); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Tiling patternWidth is narrower than the text, causing heavy horizontal overlap.
patternWidth = safeTextWidth / 1.1 (Line 251) sets the horizontal repeat interval to ~91% of the text width. Each successive tile overlaps the previous by ~9% of its width, making horizontal tiles nearly indistinguishable from a single run of solid text. This is likely unintentional — the pattern probably needs extra padding beyond the text width.
💡 Proposed fix (example: 1.5× horizontal spacing)
- const patternWidth = safeTextWidth / 1.1;
+ const patternWidth = safeTextWidth * 1.5;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (watermarkConfig.isTiled) { | |
| const safeTextWidth = Math.max(textWidth, 1); | |
| const safeTextHeight = Math.max(textHeight, 1); | |
| const patternWidth = safeTextWidth / 1.1; | |
| const patternHeight = safeTextHeight * 15; | |
| // Calculate the offset to center the pattern | |
| const offsetX = -patternWidth / 4; | |
| const offsetY = -patternHeight / 4; | |
| const maxTilesPerRow = Math.ceil(width / patternWidth) + 1; | |
| const maxTilesPerColumn = Math.ceil(height / patternHeight) + 1; | |
| for (let i = 0; i < maxTilesPerRow; i++) { | |
| for (let j = 0; j < maxTilesPerColumn; j++) { | |
| const x = i * patternWidth + offsetX; | |
| const y = j * patternHeight + offsetY; | |
| try { | |
| page.drawText(watermarkText, { | |
| x, | |
| y, | |
| size: fontSize, | |
| font, | |
| color: hexToRgb(config.color) ?? rgb(0, 0, 0), | |
| opacity: config.opacity, | |
| rotate: degrees(config.rotation), | |
| }); | |
| } catch (error) { | |
| console.error("Error drawing tiled watermark text:", error); | |
| throw new Error( | |
| `Failed to apply watermark to page ${pageIndex + 1}: ${error}`, | |
| ); | |
| } | |
| const maxTilesPerRow = Math.ceil(pageWidth / patternWidth) + 1; | |
| const maxTilesPerColumn = Math.ceil(pageHeight / patternHeight) + 1; | |
| for (let row = 0; row < maxTilesPerRow; row++) { | |
| for (let col = 0; col < maxTilesPerColumn; col++) { | |
| const x = row * patternWidth + offsetX; | |
| const y = col * patternHeight + offsetY; | |
| watermarkPage.drawText(watermarkText, { | |
| ...drawOptions, | |
| x, | |
| y, | |
| rotate: getRotateOptions(x, y), | |
| }); | |
| } | |
| } | |
| if (watermarkConfig.isTiled) { | |
| const safeTextWidth = Math.max(textWidth, 1); | |
| const safeTextHeight = Math.max(textHeight, 1); | |
| const patternWidth = safeTextWidth * 1.5; | |
| const patternHeight = safeTextHeight * 15; | |
| const offsetX = -patternWidth / 4; | |
| const offsetY = -patternHeight / 4; | |
| const maxTilesPerRow = Math.ceil(pageWidth / patternWidth) + 1; | |
| const maxTilesPerColumn = Math.ceil(pageHeight / patternHeight) + 1; | |
| for (let row = 0; row < maxTilesPerRow; row++) { | |
| for (let col = 0; col < maxTilesPerColumn; col++) { | |
| const x = row * patternWidth + offsetX; | |
| const y = col * patternHeight + offsetY; | |
| watermarkPage.drawText(watermarkText, { | |
| ...drawOptions, | |
| x, | |
| y, | |
| rotate: getRotateOptions(x, y), | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pages/api/mupdf/annotate-document.ts` around lines 247 - 272, The tiling
calculation uses patternWidth = safeTextWidth / 1.1 which makes horizontal
spacing smaller than the text and causes heavy overlap; update the tiling logic
in the block guarded by watermarkConfig.isTiled to set patternWidth to be larger
than safeTextWidth (e.g., multiply by 1.5 or add explicit horizontal padding) so
each tile has clear separation, and keep patternHeight logic or adjust similarly
if needed; ensure you only change the assignment of patternWidth (referenced in
patternWidth, safeTextWidth, watermarkPage.drawText and getRotateOptions) so the
for-loops produce non-overlapping horizontal tiles.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pages/api/mupdf/annotate-document.tsfrompdf-libto@libpdf/corefor watermarking and render watermarks as embedded page XObjectsNotoSans-Regular.ttffrompublic/fonts/NotoSans-Regular.ttfso watermark text supports broader character sets without ASCII transliterationValidation
npm run lint -- --file pages/api/mupdf/annotate-document.tsmutool draw) confirms visible centered 45-degree watermarkSummary by CodeRabbit
New Features
Bug Fixes