diff --git a/l10n/en-US/viewer.ftl b/l10n/en-US/viewer.ftl index a40842a65fbe26..d0efeec892cbd4 100644 --- a/l10n/en-US/viewer.ftl +++ b/l10n/en-US/viewer.ftl @@ -349,6 +349,8 @@ pdfjs-editor-ink-opacity-input = Opacity pdfjs-editor-stamp-add-image-button = .title = Add image pdfjs-editor-stamp-add-image-button-label = Add image +# This refers to the thickness of the line used for free highlighting (not bound to text) +pdfjs-editor-free-highlight-thickness-input = Thickness pdfjs-free-text = .aria-label = Text Editor diff --git a/src/display/draw_layer.js b/src/display/draw_layer.js index 2f0a3a43eeb60d..4ef018a676dae2 100644 --- a/src/display/draw_layer.js +++ b/src/display/draw_layer.js @@ -155,6 +155,14 @@ class DrawLayer { path.setAttribute("d", line.toSVGPath()); } + updateLine(id, line) { + const root = this.#mapping.get(id); + const defs = root.firstChild; + const path = defs.firstChild; + this.updateBox(id, line.box); + path.setAttribute("d", line.toSVGPath()); + } + removeFreeHighlight(id) { this.remove(id); this.#toUpdate.delete(id); diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js index 5a8bf976d40834..35c4750d65cfd6 100644 --- a/src/display/editor/annotation_editor_layer.js +++ b/src/display/editor/annotation_editor_layer.js @@ -359,7 +359,11 @@ class AnnotationEditorLayer { // Do nothing on right click. return; } - HighlightEditor.startHighlighting(this, event); + HighlightEditor.startHighlighting( + this, + this.#uiManager.direction === "ltr", + event + ); event.preventDefault(); } } diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index cd5f11453acec2..55d7dbd3a7642e 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -472,7 +472,7 @@ class AnnotationEditor { // the position: it'll be done when the user will release the mouse button. let { x, y } = this; - const [bx, by] = this.#getBaseTranslation(); + const [bx, by] = this.getBaseTranslation(); x += bx; y += by; @@ -481,7 +481,14 @@ class AnnotationEditor { this.div.scrollIntoView({ block: "nearest" }); } - #getBaseTranslation() { + /** + * Get the translation to take into account the editor border. + * The CSS engine positions the element in taking into account the border so + * we must apply the opposite translation to have the editor in the right + * position. + * @returns {Array} + */ + getBaseTranslation() { const [parentWidth, parentHeight] = this.parentDimensions; const { _borderLineWidth } = AnnotationEditor; const x = _borderLineWidth / parentWidth; @@ -532,7 +539,7 @@ class AnnotationEditor { this.x = x /= pageWidth; this.y = y /= pageHeight; - const [bx, by] = this.#getBaseTranslation(); + const [bx, by] = this.getBaseTranslation(); x += bx; y += by; diff --git a/src/display/editor/highlight.js b/src/display/editor/highlight.js index 48200534f9049a..18bd01a07b0eda 100644 --- a/src/display/editor/highlight.js +++ b/src/display/editor/highlight.js @@ -19,10 +19,10 @@ import { Util, } from "../../shared/util.js"; import { FreeOutliner, Outliner } from "./outliner.js"; +import { noContextMenu, PixelsPerInch } from "../display_utils.js"; import { AnnotationEditor } from "./editor.js"; import { bindEvents } from "./tools.js"; import { ColorPicker } from "./color_picker.js"; -import { noContextMenu } from "../display_utils.js"; /** * Basic draw editor in order to generate an Highlight annotation. @@ -50,11 +50,13 @@ class HighlightEditor extends AnnotationEditor { #outlineId = null; + #thickness; + static _defaultColor = null; static _defaultOpacity = 1; - static _defaultThickness = 10; + static _defaultThickness = 12; static _l10nPromise; @@ -71,6 +73,7 @@ class HighlightEditor extends AnnotationEditor { constructor(params) { super({ ...params, name: "highlightEditor" }); this.color = params.color || HighlightEditor._defaultColor; + this.#thickness = params.thickness || HighlightEditor._defaultThickness; this.#opacity = params.opacity || HighlightEditor._defaultOpacity; this.#boxes = params.boxes || null; this._isDraggable = false; @@ -113,16 +116,20 @@ class HighlightEditor extends AnnotationEditor { } #createFreeOutlines({ highlight, highlightId, clipPathId }) { - this.#highlightOutlines = highlight.getOutlines( - this._uiManager.direction === "ltr" - ); - this.#id = highlightId; - this.#clipPathId = clipPathId; - const { x, y, width, height, lastPoint } = this.#highlightOutlines.box; + this.#highlightOutlines = highlight; + if (highlightId >= 0) { + this.#id = highlightId; + this.#clipPathId = clipPathId; + } + const { x, y, width, height, lastPoint } = highlight.box; - // We need to redraw the highlight because we change the coordinates to be - // in the box coordinate system. - this.parent.drawLayer.finalizeLine(this.#id, this.#highlightOutlines); + if (highlightId >= 0) { + // We need to redraw the highlight because we change the coordinates to be + // in the box coordinate system. + this.parent.drawLayer.finalizeLine(highlightId, highlight); + } else { + this.parent?.drawLayer.updateLine(this.#id, highlight); + } switch (this.rotation) { case 0: this.x = x; @@ -154,29 +161,43 @@ class HighlightEditor extends AnnotationEditor { } } - const innerMargin = 1.5; - this.#focusOutlines = highlight.getFocusOutline( + const extraThickness = 1.5; + this.#focusOutlines = highlight.getNewOutline( /* Slightly bigger than the highlight in order to have a little space between the highlight and the outline. */ - HighlightEditor._defaultThickness + innerMargin - ); - this.#outlineId = this.parent.drawLayer.highlightOutline( - this.#focusOutlines + this.#thickness / 2 + extraThickness, + /* innerMargin = */ 0.0025 ); + + if (highlightId >= 0) { + this.#outlineId = this.parent.drawLayer.highlightOutline( + this.#focusOutlines + ); + } else { + this.parent?.drawLayer.updateLine(this.#outlineId, this.#focusOutlines); + } this.#lastPoint = lastPoint; } + /** @inheritdoc */ static initialize(l10n, uiManager) { AnnotationEditor.initialize(l10n, uiManager); HighlightEditor._defaultColor ||= uiManager.highlightColors?.values().next().value || "#fff066"; + document + .getElementById("editorHighlightThickness") + .style.setProperty("--pdf-to-css-units", PixelsPerInch.PDF_TO_CSS_UNITS); } + /** @inheritdoc */ static updateDefaultParams(type, value) { switch (type) { case AnnotationEditorParamsType.HIGHLIGHT_DEFAULT_COLOR: HighlightEditor._defaultColor = value; break; + case AnnotationEditorParamsType.HIGHLIGHT_THICKNESS: + HighlightEditor._defaultThickness = value; + break; } } @@ -194,6 +215,9 @@ class HighlightEditor extends AnnotationEditor { case AnnotationEditorParamsType.HIGHLIGHT_COLOR: this.#updateColor(value); break; + case AnnotationEditorParamsType.HIGHLIGHT_THICKNESS: + this.#updateThickness(value); + break; } } @@ -203,6 +227,10 @@ class HighlightEditor extends AnnotationEditor { AnnotationEditorParamsType.HIGHLIGHT_DEFAULT_COLOR, HighlightEditor._defaultColor, ], + [ + AnnotationEditorParamsType.HIGHLIGHT_THICKNESS, + HighlightEditor._defaultThickness, + ], ]; } @@ -213,6 +241,10 @@ class HighlightEditor extends AnnotationEditor { AnnotationEditorParamsType.HIGHLIGHT_COLOR, this.color || HighlightEditor._defaultColor, ], + [ + AnnotationEditorParamsType.HIGHLIGHT_THICKNESS, + this.#thickness || HighlightEditor._defaultThickness, + ], ]; } @@ -240,6 +272,28 @@ class HighlightEditor extends AnnotationEditor { }); } + /** + * Update the thickness and make this action undoable. + * @param {number} thickness + */ + #updateThickness(thickness) { + const savedThickness = this.#thickness; + this.addCommands({ + cmd: () => { + this.#thickness = thickness; + this.#changeThickness(thickness); + }, + undo: () => { + this.#thickness = savedThickness; + this.#changeThickness(savedThickness); + }, + mustExec: true, + type: AnnotationEditorParamsType.INK_THICKNESS, + overwriteIfSameType: true, + keepUndo: true, + }); + } + /** @inheritdoc */ async addEditToolbar() { const toolbar = await super.addEditToolbar(); @@ -270,6 +324,13 @@ class HighlightEditor extends AnnotationEditor { return super.fixAndSetPosition(this.#getRotation()); } + /** @inheritdoc */ + getBaseTranslation() { + // The editor itself doesn't have any CSS border (we're drawing one + // ourselves in using SVG). + return [0, 0]; + } + /** @inheritdoc */ getRect(tx, ty) { return super.getRect(tx, ty, this.#getRotation()); @@ -324,6 +385,18 @@ class HighlightEditor extends AnnotationEditor { } } + #changeThickness(thickness) { + if (!this.#isFreeHighlight) { + return; + } + this.#createFreeOutlines({ + highlight: this.#highlightOutlines.getNewOutline(thickness / 2), + }); + this.fixAndSetPosition(); + const [parentWidth, parentHeight] = this.parentDimensions; + this.setDims(this.width * parentWidth, this.height * parentHeight); + } + #cleanDrawLayer() { if (this.#id === null || !this.parent) { return; @@ -482,7 +555,7 @@ class HighlightEditor extends AnnotationEditor { return this.#highlightOutlines.serialize(rect, this.#getRotation()); } - static startHighlighting(parent, { target: textLayer, x, y }) { + static startHighlighting(parent, isLTR, { target: textLayer, x, y }) { const { x: layerX, y: layerY, @@ -520,7 +593,8 @@ class HighlightEditor extends AnnotationEditor { { x, y }, [layerX, layerY, parentWidth, parentHeight], parent.scale, - this._defaultThickness, + this._defaultThickness / 2, + isLTR, /* innerMargin = */ 0.001 ); ({ id: this._freeHighlightId, clipPathId: this._freeHighlightClipId } = @@ -543,7 +617,7 @@ class HighlightEditor extends AnnotationEditor { if (!this._freeHighlight.isEmpty()) { parent.createAndAddNewEditor(event, false, { highlightId: this._freeHighlightId, - highlight: this._freeHighlight, + highlight: this._freeHighlight.getOutlines(), clipPathId: this._freeHighlightClipId, }); } else { @@ -597,7 +671,7 @@ class HighlightEditor extends AnnotationEditor { annotationType: AnnotationEditorType.HIGHLIGHT, color, opacity: this.#opacity, - thickness: 2 * HighlightEditor._defaultThickness, + thickness: this.#thickness, quadPoints: this.#serializeBoxes(rect), outlines: this.#serializeOutlines(rect), pageIndex: this.pageIndex, diff --git a/src/display/editor/outliner.js b/src/display/editor/outliner.js index 02366fddc24ec8..d3b2c57f7f098a 100644 --- a/src/display/editor/outliner.js +++ b/src/display/editor/outliner.js @@ -350,6 +350,8 @@ class FreeOutliner { #innerMargin; + #isLTR; + #top = []; // The first 6 elements are the last 3 points of the top part of the outline. @@ -377,9 +379,10 @@ class FreeOutliner { static #MIN = FreeOutliner.#MIN_DIST + FreeOutliner.#MIN_DIFF; - constructor({ x, y }, box, scaleFactor, thickness, innerMargin = 0) { + constructor({ x, y }, box, scaleFactor, thickness, isLTR, innerMargin = 0) { this.#box = box; this.#thickness = thickness * scaleFactor; + this.#isLTR = isLTR; this.#last.set([NaN, NaN, NaN, NaN, x, y], 6); this.#innerMargin = innerMargin; this.#min_dist = FreeOutliner.#MIN_DIST * scaleFactor; @@ -571,24 +574,7 @@ class FreeOutliner { return buffer.join(" "); } - getFocusOutline(thickness) { - // Build the outline of the highlight to use as the focus outline. - const [x, y] = this.#points; - const outliner = new FreeOutliner( - { x, y }, - this.#box, - this.#scaleFactor, - thickness, - /* innerMargin = */ 0.0025 - ); - outliner.#points = null; - for (let i = 2; i < this.#points.length; i += 2) { - outliner.add({ x: this.#points[i], y: this.#points[i + 1] }); - } - return outliner.getOutlines(); - } - - getOutlines(isLTR) { + getOutlines() { const top = this.#top; const bottom = this.#bottom; const last = this.#last; @@ -637,8 +623,10 @@ class FreeOutliner { return new FreeHighlightOutline( outline, points, + this.#box, + this.#scaleFactor, this.#innerMargin, - isLTR + this.#isLTR ); } @@ -686,24 +674,40 @@ class FreeOutliner { } } outline.set([NaN, NaN, NaN, NaN, bottom[4], bottom[5]], N); - return new FreeHighlightOutline(outline, points, this.#innerMargin, isLTR); + return new FreeHighlightOutline( + outline, + points, + this.#box, + this.#scaleFactor, + this.#innerMargin, + this.#isLTR + ); } } class FreeHighlightOutline extends Outline { + #box; + #bbox = null; #innerMargin; + #isLTR; + #points; + #scaleFactor; + #outline; - constructor(outline, points, innerMargin, isLTR) { + constructor(outline, points, box, scaleFactor, innerMargin, isLTR) { super(); this.#outline = outline; this.#points = points; + this.#box = box; + this.#scaleFactor = scaleFactor; this.#innerMargin = innerMargin; + this.#isLTR = isLTR; this.#computeMinMax(isLTR); const { x, y, width, height } = this.#bbox; @@ -841,6 +845,34 @@ class FreeHighlightOutline extends Outline { get box() { return this.#bbox; } + + getNewOutline(thickness, innerMargin) { + // Build the outline of the highlight to use as the focus outline. + const { x, y, width, height } = this.#bbox; + const [layerX, layerY, layerWidth, layerHeight] = this.#box; + const sx = width * layerWidth; + const sy = height * layerHeight; + const tx = x * layerWidth + layerX; + const ty = y * layerHeight + layerY; + const outliner = new FreeOutliner( + { + x: this.#points[0] * sx + tx, + y: this.#points[1] * sy + ty, + }, + this.#box, + this.#scaleFactor, + thickness, + this.#isLTR, + innerMargin ?? this.#innerMargin + ); + for (let i = 2; i < this.#points.length; i += 2) { + outliner.add({ + x: this.#points[i] * sx + tx, + y: this.#points[i + 1] * sy + ty, + }); + } + return outliner.getOutlines(); + } } export { FreeOutliner, Outliner }; diff --git a/src/shared/util.js b/src/shared/util.js index eb184a56c37891..77a1592d5ade30 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -88,6 +88,7 @@ const AnnotationEditorParamsType = { INK_OPACITY: 23, HIGHLIGHT_COLOR: 31, HIGHLIGHT_DEFAULT_COLOR: 32, + HIGHLIGHT_THICKNESS: 33, }; // Permission flags from Table 22, Section 7.6.3.2 of the PDF specification. diff --git a/test/integration/highlight_editor_spec.mjs b/test/integration/highlight_editor_spec.mjs index 1c8ba8db40b2a4..6ee7be33ea3a80 100644 --- a/test/integration/highlight_editor_spec.mjs +++ b/test/integration/highlight_editor_spec.mjs @@ -14,7 +14,9 @@ */ import { + awaitPromise, closePages, + createPromise, getEditorSelector, getSerialized, kbBigMoveLeft, @@ -32,6 +34,11 @@ const selectAll = async page => { ); }; +const waitForPointerUp = page => + createPromise(page, resolve => { + window.addEventListener("pointerup", resolve, { once: true }); + }); + const getXY = (page, selector) => page.evaluate(sel => { const bbox = document.querySelector(sel).getBoundingClientRect(); @@ -586,4 +593,92 @@ describe("Highlight Editor", () => { ); }); }); + + describe("Free highlight thickness can be changed", () => { + let pages; + + beforeAll(async () => { + pages = await loadAndWait( + "empty.pdf", + ".annotationEditorLayer", + null, + null, + { highlightEditorColors: "yellow=#000000" } + ); + }); + + afterAll(async () => { + await closePages(pages); + }); + + it("must check that the thickness is correctly updated", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await page.click("#editorHighlight"); + await page.waitForSelector(".annotationEditorLayer.highlightEditing"); + + const rect = await page.$eval(".annotationEditorLayer", el => { + // With Chrome something is wrong when serializing a DomRect, + // hence we extract the values and just return them. + const { x, y } = el.getBoundingClientRect(); + return { x, y }; + }); + + for (let i = 0; i < 3; i++) { + const x = rect.x + 120 + i * 120; + const y = rect.y + 120 + i * 120; + const clickHandle = await waitForPointerUp(page); + await page.mouse.move(x, y); + await page.mouse.down(); + await page.mouse.move(x + 100, y + 100); + await page.mouse.up(); + await awaitPromise(clickHandle); + + await page.waitForSelector(getEditorSelector(i)); + } + + let value = 12; + await waitForSerialized(page, 3); + let serialized = await getSerialized(page); + expect(serialized.map(x => x.thickness)) + .withContext(`In ${browserName}`) + .toEqual([value, value, value]); + + await selectAll(page); + + const prevWidth = await page.evaluate( + sel => document.querySelector(sel).getBoundingClientRect().width, + getEditorSelector(0) + ); + + value = 24; + page.evaluate(val => { + window.PDFViewerApplication.eventBus.dispatch( + "switchannotationeditorparams", + { + source: null, + type: window.pdfjsLib.AnnotationEditorParamsType + .HIGHLIGHT_THICKNESS, + value: val, + } + ); + }, value); + + await page.waitForFunction( + (w, sel) => + document.querySelector(sel).getBoundingClientRect().width !== w, + {}, + prevWidth, + getEditorSelector(0) + ); + + await waitForSerialized(page, 3); + serialized = await getSerialized(page); + expect(serialized.map(x => x.thickness)) + .withContext(`In ${browserName}`) + .toEqual([value, value, value]); + }) + ); + }); + }); }); diff --git a/web/annotation_editor_layer_builder.css b/web/annotation_editor_layer_builder.css index 232886e41b0696..651dcd1eeeb98a 100644 --- a/web/annotation_editor_layer_builder.css +++ b/web/annotation_editor_layer_builder.css @@ -1094,20 +1094,21 @@ height: auto; padding-inline: 10px; padding-block: 10px 16px; + gap: 16px; display: flex; flex-direction: column; box-sizing: border-box; + .editorParamsLabel { + width: fit-content; + inset-inline-start: 0; + } + .colorPicker { display: flex; flex-direction: column; gap: 8px; - #highlightColorPickerLabel { - width: fit-content; - inset-inline-start: 0; - } - .dropdown { display: flex; justify-content: space-between; @@ -1145,4 +1146,58 @@ } } } + + #editorHighlightThickness { + display: flex; + flex-direction: column; + align-items: center; + gap: 4px; + align-self: stretch; + + .editorParamsLabel { + width: 100%; + height: auto; + align-self: stretch; + } + + .thicknessPicker { + display: flex; + justify-content: space-between; + align-items: center; + align-self: stretch; + + --example-color: #bfbfc9; + + @media (prefers-color-scheme: dark) { + --example-color: #80808e; + } + + @media screen and (forced-colors: active) { + --example-color: HighlightText; + } + + &::before { + content: ""; + width: calc(8px * var(--pdf-to-css-units)); + aspect-ratio: 1; + display: block; + border-radius: 100%; + background-color: var(--example-color); + } + + .editorParamsSlider { + width: unset; + height: 14px; + } + + &::after { + content: ""; + width: calc(24px * var(--pdf-to-css-units)); + aspect-ratio: 1; + display: block; + border-radius: 100%; + background-color: var(--example-color); + } + } + } } diff --git a/web/annotation_editor_params.js b/web/annotation_editor_params.js index ddfc17d1ab14c5..f44d24847769d4 100644 --- a/web/annotation_editor_params.js +++ b/web/annotation_editor_params.js @@ -32,6 +32,7 @@ class AnnotationEditorParams { editorInkThickness, editorInkOpacity, editorStampAddImage, + editorFreeHighlightThicknessInput, }) { const dispatchEvent = (typeStr, value) => { this.eventBus.dispatch("switchannotationeditorparams", { @@ -58,6 +59,9 @@ class AnnotationEditorParams { editorStampAddImage.addEventListener("click", () => { dispatchEvent("CREATE"); }); + editorFreeHighlightThicknessInput.addEventListener("input", function () { + dispatchEvent("HIGHLIGHT_THICKNESS", this.valueAsNumber); + }); this.eventBus._on("annotationeditorparamschanged", evt => { for (const [type, value] of evt.details) { @@ -77,6 +81,9 @@ class AnnotationEditorParams { case AnnotationEditorParamsType.INK_OPACITY: editorInkOpacity.value = value; break; + case AnnotationEditorParamsType.HIGHLIGHT_THICKNESS: + editorFreeHighlightThicknessInput.value = value; + break; } } }); diff --git a/web/viewer.html b/web/viewer.html index 5c201db032e4c9..37c2d2aba4542b 100644 --- a/web/viewer.html +++ b/web/viewer.html @@ -176,6 +176,12 @@
Highlight color
+
+ +
+ +
+
diff --git a/web/viewer.js b/web/viewer.js index 65fb35928b41e2..f97457ab684994 100644 --- a/web/viewer.js +++ b/web/viewer.js @@ -172,6 +172,9 @@ function getViewerConfiguration() { editorInkThickness: document.getElementById("editorInkThickness"), editorInkOpacity: document.getElementById("editorInkOpacity"), editorStampAddImage: document.getElementById("editorStampAddImage"), + editorFreeHighlightThicknessInput: document.getElementById( + "editorFreeHighlightThicknessInput" + ), }, printContainer: document.getElementById("printContainer"), openFileInput: