Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid to have the text layer mismatching the rendered text with mismatching locales (bug 1869001) #17476

Merged
merged 1 commit into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/display/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import {
NodeStandardFontDataFactory,
} from "display-node_utils";
import { CanvasGraphics } from "./canvas.js";
import { cleanupTextLayer } from "./text_layer.js";
import { GlobalWorkerOptions } from "./worker_options.js";
import { MessageHandler } from "../shared/message_handler.js";
import { Metadata } from "./metadata.js";
Expand Down Expand Up @@ -2481,6 +2482,7 @@ class WorkerTransport {
this.fontLoader.clear();
this.#methodPromises.clear();
this.filterFactory.destroy();
cleanupTextLayer();

this._networkStream?.cancelAllRequests(
new AbortException("Worker was terminated.")
Expand Down Expand Up @@ -3065,6 +3067,7 @@ class WorkerTransport {
}
this.#methodPromises.clear();
this.filterFactory.destroy(/* keepHCM = */ true);
cleanupTextLayer();
}

get loadingParams() {
Expand Down
67 changes: 38 additions & 29 deletions src/display/text_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,7 @@
/** @typedef {import("./display_utils").PageViewport} PageViewport */
/** @typedef {import("./api").TextContent} TextContent */

import {
AbortException,
FeatureTest,
PromiseCapability,
Util,
} from "../shared/util.js";
import { AbortException, PromiseCapability, Util } from "../shared/util.js";
import { setLayerDimensions } from "./display_utils.js";

/**
Expand All @@ -43,8 +38,6 @@ import { setLayerDimensions } from "./display_utils.js";
* @property {Array<string>} [textContentItemsStr] - Strings that correspond to
* the `str` property of the text items of the textContent input.
* This is output and shall initially be set to an empty array.
* @property {boolean} [isOffscreenCanvasSupported] true if we can use
* OffscreenCanvas to measure string widths.
*/

/**
Expand All @@ -60,8 +53,6 @@ import { setLayerDimensions } from "./display_utils.js";
* This is output and shall initially be set to an empty array.
* @property {WeakMap<HTMLElement,Object>} [textDivProperties] - Some properties
* weakly mapped to the HTML elements used to render the text.
* @property {boolean} [isOffscreenCanvasSupported] true if we can use
* OffscreenCanvas to measure string widths.
* @property {boolean} [mustRotate] true if the text layer must be rotated.
* @property {boolean} [mustRescale] true if the text layer contents must be
* rescaled.
Expand All @@ -71,28 +62,43 @@ const MAX_TEXT_DIVS_TO_RENDER = 100000;
const DEFAULT_FONT_SIZE = 30;
const DEFAULT_FONT_ASCENT = 0.8;
const ascentCache = new Map();

function getCtx(size, isOffscreenCanvasSupported) {
let ctx;
if (isOffscreenCanvasSupported && FeatureTest.isOffscreenCanvasSupported) {
ctx = new OffscreenCanvas(size, size).getContext("2d", { alpha: false });
} else {
let _canvasContext = null;

function getCtx() {
if (!_canvasContext) {
// We don't use an OffscreenCanvas here because we use serif/sans serif
// fonts with it and they depends on the locale.
// In Firefox, the <html> element get a lang attribute that depends on what
// Fluent returns for the locale and the OffscreenCanvas uses the OS locale.
// Those two locales can be different and consequently the used fonts will
// be different (see bug 1869001).
// Ideally, we should use in the text layer the fonts we've in the pdf (or
// their replacements when they aren't embedded) and then we can use an
// OffscreenCanvas.
const canvas = document.createElement("canvas");
canvas.width = canvas.height = size;
ctx = canvas.getContext("2d", { alpha: false });
canvas.className = "hiddenCanvasElement";
document.body.append(canvas);
calixteman marked this conversation as resolved.
Show resolved Hide resolved
_canvasContext = canvas.getContext("2d", { alpha: false });
}

return ctx;
return _canvasContext;
}

function cleanupTextLayer() {
_canvasContext?.canvas.remove();
_canvasContext = null;
}

function getAscent(fontFamily, isOffscreenCanvasSupported) {
function getAscent(fontFamily) {
const cachedAscent = ascentCache.get(fontFamily);
if (cachedAscent) {
return cachedAscent;
}

const ctx = getCtx(DEFAULT_FONT_SIZE, isOffscreenCanvasSupported);
const ctx = getCtx();

const savedFont = ctx.font;
ctx.canvas.width = ctx.canvas.height = DEFAULT_FONT_SIZE;
ctx.font = `${DEFAULT_FONT_SIZE}px ${fontFamily}`;
const metrics = ctx.measureText("");

Expand All @@ -104,6 +110,7 @@ function getAscent(fontFamily, isOffscreenCanvasSupported) {
ascentCache.set(fontFamily, ratio);

ctx.canvas.width = ctx.canvas.height = 0;
ctx.font = savedFont;
return ratio;
}

Expand Down Expand Up @@ -143,6 +150,7 @@ function getAscent(fontFamily, isOffscreenCanvasSupported) {
}

ctx.canvas.width = ctx.canvas.height = 0;
ctx.font = savedFont;

if (ascent) {
const ratio = ascent / (ascent + descent);
Expand Down Expand Up @@ -176,8 +184,7 @@ function appendText(task, geom, styles) {
const fontFamily =
(task._fontInspectorEnabled && style.fontSubstitution) || style.fontFamily;
const fontHeight = Math.hypot(tx[2], tx[3]);
const fontAscent =
fontHeight * getAscent(fontFamily, task._isOffscreenCanvasSupported);
const fontAscent = fontHeight * getAscent(fontFamily);

let left, top;
if (angle === 0) {
Expand Down Expand Up @@ -308,14 +315,12 @@ class TextLayerRenderTask {
textDivs,
textDivProperties,
textContentItemsStr,
isOffscreenCanvasSupported,
}) {
this._textContentSource = textContentSource;
this._isReadableStream = textContentSource instanceof ReadableStream;
this._container = this._rootContainer = container;
this._textDivs = textDivs || [];
this._textContentItemsStr = textContentItemsStr || [];
this._isOffscreenCanvasSupported = isOffscreenCanvasSupported;
this._fontInspectorEnabled = !!globalThis.FontInspector?.enabled;

this._reader = null;
Expand All @@ -328,7 +333,7 @@ class TextLayerRenderTask {
div: null,
scale: viewport.scale * (globalThis.devicePixelRatio || 1),
properties: null,
ctx: getCtx(0, isOffscreenCanvasSupported),
ctx: getCtx(),
};
const { pageWidth, pageHeight, pageX, pageY } = viewport.rawDims;
this._transform = [1, 0, 0, -1, -pageX, pageY + pageHeight];
Expand Down Expand Up @@ -474,7 +479,6 @@ function updateTextLayer({
viewport,
textDivs,
textDivProperties,
isOffscreenCanvasSupported,
mustRotate = true,
mustRescale = true,
}) {
Expand All @@ -483,7 +487,7 @@ function updateTextLayer({
}

if (mustRescale) {
const ctx = getCtx(0, isOffscreenCanvasSupported);
const ctx = getCtx();
const scale = viewport.scale * (globalThis.devicePixelRatio || 1);
const params = {
prevFontSize: null,
Expand All @@ -501,4 +505,9 @@ function updateTextLayer({
}
}

export { renderTextLayer, TextLayerRenderTask, updateTextLayer };
export {
cleanupTextLayer,
renderTextLayer,
TextLayerRenderTask,
updateTextLayer,
};
1 change: 0 additions & 1 deletion web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,6 @@ const PDFViewerApplication = {
annotationEditorHighlightColors: AppOptions.get("highlightEditorColors"),
imageResourcesPath: AppOptions.get("imageResourcesPath"),
enablePrintAutoRotate: AppOptions.get("enablePrintAutoRotate"),
isOffscreenCanvasSupported,
maxCanvasPixels: AppOptions.get("maxCanvasPixels"),
enablePermissions: AppOptions.get("enablePermissions"),
pageColors,
Expand Down
5 changes: 0 additions & 5 deletions web/pdf_page_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ import { XfaLayerBuilder } from "./xfa_layer_builder.js";
* The default value is `AnnotationMode.ENABLE_FORMS`.
* @property {string} [imageResourcesPath] - Path for image resources, mainly
* for annotation icons. Include trailing slash.
* @property {boolean} [isOffscreenCanvasSupported] - Allows to use an
* OffscreenCanvas if needed.
* @property {number} [maxCanvasPixels] - The maximum supported canvas size in
* total pixels, i.e. width * height. Use `-1` for no limit, or `0` for
* CSS-only zooming. The default value is 4096 * 4096 (16 mega-pixels).
Expand Down Expand Up @@ -154,8 +152,6 @@ class PDFPageView {
this.#annotationMode =
options.annotationMode ?? AnnotationMode.ENABLE_FORMS;
this.imageResourcesPath = options.imageResourcesPath || "";
this.isOffscreenCanvasSupported =
options.isOffscreenCanvasSupported ?? true;
this.maxCanvasPixels = options.maxCanvasPixels ?? MAX_CANVAS_PIXELS;
this.pageColors = options.pageColors || null;

Expand Down Expand Up @@ -879,7 +875,6 @@ class PDFPageView {
this.textLayer = new TextLayerBuilder({
highlighter: this._textHighlighter,
accessibilityManager: this._accessibilityManager,
isOffscreenCanvasSupported: this.isOffscreenCanvasSupported,
calixteman marked this conversation as resolved.
Show resolved Hide resolved
enablePermissions:
this.#textLayerMode === TextLayerMode.ENABLE_PERMISSIONS,
});
Expand Down
3 changes: 2 additions & 1 deletion web/pdf_viewer.css
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
transform: rotate(270deg) translateX(-100%);
}

#hiddenCopyElement {
#hiddenCopyElement,
.hiddenCanvasElement {
position: absolute;
top: 0;
left: 0;
Expand Down
5 changes: 0 additions & 5 deletions web/pdf_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,6 @@ function isValidAnnotationEditorMode(mode) {
* mainly for annotation icons. Include trailing slash.
* @property {boolean} [enablePrintAutoRotate] - Enables automatic rotation of
* landscape pages upon printing. The default is `false`.
* @property {boolean} [isOffscreenCanvasSupported] - Allows to use an
* OffscreenCanvas if needed.
* @property {number} [maxCanvasPixels] - The maximum supported canvas size in
* total pixels, i.e. width * height. Use `-1` for no limit, or `0` for
* CSS-only zooming. The default value is 4096 * 4096 (16 mega-pixels).
Expand Down Expand Up @@ -287,8 +285,6 @@ class PDFViewer {
if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
this.removePageBorders = options.removePageBorders || false;
}
this.isOffscreenCanvasSupported =
options.isOffscreenCanvasSupported ?? true;
this.maxCanvasPixels = options.maxCanvasPixels;
this.l10n = options.l10n || NullL10n;
this.#enablePermissions = options.enablePermissions || false;
Expand Down Expand Up @@ -919,7 +915,6 @@ class PDFViewer {
textLayerMode,
annotationMode,
imageResourcesPath: this.imageResourcesPath,
isOffscreenCanvasSupported: this.isOffscreenCanvasSupported,
maxCanvasPixels: this.maxCanvasPixels,
pageColors: this.pageColors,
l10n: this.l10n,
Expand Down
6 changes: 0 additions & 6 deletions web/text_layer_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ import { removeNullCharacters } from "./ui_utils.js";
* @property {TextHighlighter} highlighter - Optional object that will handle
* highlighting text from the find controller.
* @property {TextAccessibilityManager} [accessibilityManager]
* @property {boolean} [isOffscreenCanvasSupported] - Allows to use an
* OffscreenCanvas if needed.
*/

/**
Expand All @@ -49,7 +47,6 @@ class TextLayerBuilder {
constructor({
highlighter = null,
accessibilityManager = null,
isOffscreenCanvasSupported = true,
enablePermissions = false,
}) {
this.textContentItemsStr = [];
Expand All @@ -59,7 +56,6 @@ class TextLayerBuilder {
this.textLayerRenderTask = null;
this.highlighter = highlighter;
this.accessibilityManager = accessibilityManager;
this.isOffscreenCanvasSupported = isOffscreenCanvasSupported;
this.#enablePermissions = enablePermissions === true;

/**
Expand Down Expand Up @@ -107,7 +103,6 @@ class TextLayerBuilder {
viewport,
textDivs: this.textDivs,
textDivProperties: this.textDivProperties,
isOffscreenCanvasSupported: this.isOffscreenCanvasSupported,
mustRescale,
mustRotate,
});
Expand All @@ -129,7 +124,6 @@ class TextLayerBuilder {
textDivs: this.textDivs,
textDivProperties: this.textDivProperties,
textContentItemsStr: this.textContentItemsStr,
isOffscreenCanvasSupported: this.isOffscreenCanvasSupported,
});

await this.textLayerRenderTask.promise;
Expand Down