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.

Sign up for GitHub

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

Jump to bottom

[api-minor] Get rid of CSS transform on each annotation in the annotation layer #15036

Merged
merged 1 commit into from
Jun 19, 2022
Merged

[api-minor] Get rid of CSS transform on each annotation in the annotation layer #15036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions src/core/annotation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ class Annotation {
this.setAppearance(dict);
this.setBorderAndBackgroundColors(dict.get("MK"));

this._hasOwnCanvas = false;
this._streams = [];
if (this.appearance) {
this._streams.push(this.appearance);
Expand All @@ -450,7 +451,6 @@ class Annotation {
modificationDate: this.modificationDate,
rect: this.rectangle,
subtype: params.subtype,
hasOwnCanvas: false,
};

if (params.collectFields) {
Expand Down Expand Up @@ -849,7 +849,7 @@ class Annotation {
const data = this.data;
let appearance = this.appearance;
const isUsingOwnCanvas =
data.hasOwnCanvas && intent & RenderingIntentFlag.DISPLAY;
this._hasOwnCanvas && intent & RenderingIntentFlag.DISPLAY;
if (!appearance) {
if (!isUsingOwnCanvas) {
return Promise.resolve(new OperatorList());
Expand Down Expand Up @@ -2163,7 +2163,7 @@ class ButtonWidgetAnnotation extends WidgetAnnotation {
} else if (this.data.radioButton) {
this._processRadioButton(params);
} else if (this.data.pushButton) {
this.data.hasOwnCanvas = true;
this._hasOwnCanvas = true;
this._processPushButton(params);
} else {
warn("Invalid field flags for button widget annotation");
Expand Down
2 changes: 1 addition & 1 deletion src/core/xfa/template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1425,7 +1425,7 @@ class ChoiceList extends XFAObject {
const field = ui[$getParent]();
const fontSize = (field.font && field.font.size) || 10;
const optionStyle = {
fontSize: `calc(${fontSize}px * var(--zoom-factor))`,
fontSize: `calc(${fontSize}px * var(--scale-factor))`,
};
const children = [];

Expand Down
4 changes: 4 additions & 0 deletions src/core/xfa/xhtml.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,10 @@ function mapStyle(styleStr, node, richText) {
);
}

if (richText && style.fontSize) {
style.fontSize = `calc(${style.fontSize} * var(--scale-factor))`;
}

fixTextIndent(style);
return style;
}
Expand Down
171 changes: 64 additions & 107 deletions src/display/annotation_layer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,11 @@ class AnnotationElement {
page = this.page,
viewport = this.viewport;
const container = document.createElement("section");
let { width, height } = getRectDims(data.rect);
const { width, height } = getRectDims(data.rect);

const [pageLLx, pageLLy, pageURx, pageURy] = viewport.viewBox;
const pageWidth = pageURx - pageLLx;
const pageHeight = pageURy - pageLLy;

container.setAttribute("data-annotation-id", data.id);

Expand All @@ -210,41 +214,13 @@ class AnnotationElement {
page.view[3] - data.rect[3] + page.view[1],
]);

if (data.hasOwnCanvas) {
Snuffleupagus marked this conversation as resolved.
Show resolved Hide resolved
const transform = viewport.transform.slice();
const [scaleX, scaleY] = Util.singularValueDecompose2dScale(transform);
width = Math.ceil(width * scaleX);
height = Math.ceil(height * scaleY);
rect[0] *= scaleX;
rect[1] *= scaleY;
// Reset the scale part of the transform matrix (which must be diagonal
// or anti-diagonal) in order to avoid to rescale the canvas.
// The canvas for the annotation is correctly scaled when it is drawn
// (see `beginAnnotation` in canvas.js).
for (let i = 0; i < 4; i++) {
transform[i] = Math.sign(transform[i]);
}
container.style.transform = `matrix(${transform.join(",")})`;
} else {
container.style.transform = `matrix(${viewport.transform.join(",")})`;
}

container.style.transformOrigin = `${-rect[0]}px ${-rect[1]}px`;

if (!ignoreBorder && data.borderStyle.width > 0) {
container.style.borderWidth = `${data.borderStyle.width}px`;
if (data.borderStyle.style !== AnnotationBorderStyleType.UNDERLINE) {
// Underline styles only have a bottom border, so we do not need
// to adjust for all borders. This yields a similar result as
// Adobe Acrobat/Reader.
width -= 2 * data.borderStyle.width;
height -= 2 * data.borderStyle.width;
}
Comment on lines -236 to -242
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What "changed" in the new implementation to make this hack unnecessary?
Also, do we have proper test-coverage for this code-path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added:
https://github.com/mozilla/pdf.js/pull/15036/files#diff-4748c83592581d85910b4a948677ddfcd25c8dc68ffcd20c524e0f4f0739f51aR53

and I compared the output of annotation-border-styles.pdf (the code path is taken here except in the underline case) in FIrefox and in Acrobat: the rendering is almost the same (even for the underline case).


const horizontalRadius = data.borderStyle.horizontalCornerRadius;
const verticalRadius = data.borderStyle.verticalCornerRadius;
if (horizontalRadius > 0 || verticalRadius > 0) {
const radius = `${horizontalRadius}px / ${verticalRadius}px`;
const radius = `calc(${horizontalRadius}px * var(--scale-factor)) / calc(${verticalRadius}px * var(--scale-factor))`;
Snuffleupagus marked this conversation as resolved.
Show resolved Hide resolved
container.style.borderRadius = radius;
}

Expand Down Expand Up @@ -286,15 +262,11 @@ class AnnotationElement {
}
}

container.style.left = `${rect[0]}px`;
container.style.top = `${rect[1]}px`;
container.style.left = `${(100 * (rect[0] - pageLLx)) / pageWidth}%`;
container.style.top = `${(100 * (rect[1] - pageLLy)) / pageHeight}%`;
container.style.width = `${(100 * width) / pageWidth}%`;
container.style.height = `${(100 * height) / pageHeight}%`;

if (data.hasOwnCanvas) {
container.style.width = container.style.height = "auto";
} else {
container.style.width = `${width}px`;
container.style.height = `${height}px`;
}
return container;
}

Expand Down Expand Up @@ -464,7 +436,7 @@ class AnnotationElement {
const popup = popupElement.render();

// Position the popup next to the annotation's container.
popup.style.left = container.style.width;
popup.style.left = "100%";

container.append(popup);
}
Expand Down Expand Up @@ -820,8 +792,6 @@ class TextAnnotationElement extends AnnotationElement {
this.container.className = "textAnnotation";

const image = document.createElement("img");
image.style.height = this.container.style.height;
image.style.width = this.container.style.width;
image.src =
this.imageResourcesPath +
"annotation-" +
Expand Down Expand Up @@ -925,21 +895,20 @@ class WidgetAnnotationElement extends AnnotationElement {
// it's instead based on the field height.
// If the height is "big" then it could lead to a too big font size
// so in this case use the one we've in the pdf (hence the min).
let computedFontSize;
if (this.data.multiLine) {
const height = Math.abs(this.data.rect[3] - this.data.rect[1]);
const numberOfLines = Math.round(height / (LINE_FACTOR * fontSize)) || 1;
const lineHeight = height / numberOfLines;
style.fontSize = `${Math.min(
computedFontSize = Math.min(
fontSize,
Math.round(lineHeight / LINE_FACTOR)
)}px`;
);
} else {
const height = Math.abs(this.data.rect[3] - this.data.rect[1]);
style.fontSize = `${Math.min(
fontSize,
Math.round(height / LINE_FACTOR)
)}px`;
computedFontSize = Math.min(fontSize, Math.round(height / LINE_FACTOR));
}
style.fontSize = `${computedFontSize}%`;

style.color = Util.makeHexColor(fontColor[0], fontColor[1], fontColor[2]);

Expand Down Expand Up @@ -1227,7 +1196,7 @@ class TextWidgetAnnotationElement extends WidgetAnnotationElement {
const combWidth = fieldWidth / this.data.maxLen;

element.classList.add("comb");
element.style.letterSpacing = `calc(${combWidth}px - 1ch)`;
element.style.letterSpacing = `calc(${combWidth}px * var(--scale-factor) - 1ch)`;
}
} else {
element = document.createElement("div");
Expand Down Expand Up @@ -1464,10 +1433,6 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
value: this.data.fieldValue,
});

const fontSize =
this.data.defaultAppearanceData.fontSize || DEFAULT_FONT_SIZE;
const fontSizeStyle = `calc(${fontSize}px * var(--zoom-factor))`;

const selectElement = document.createElement("select");
GetElementsByNameSet.add(selectElement);
selectElement.setAttribute("data-element-id", id);
Expand Down Expand Up @@ -1499,9 +1464,6 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
const optionElement = document.createElement("option");
optionElement.textContent = option.displayValue;
optionElement.value = option.exportValue;
if (this.data.combo) {
optionElement.style.fontSize = fontSizeStyle;
}
if (storedData.value.includes(option.exportValue)) {
optionElement.setAttribute("selected", true);
addAnEmptyEntry = false;
Expand Down Expand Up @@ -1749,9 +1711,12 @@ class PopupAnnotationElement extends AnnotationElement {
rect[0] + this.data.parentRect[2] - this.data.parentRect[0];
const popupTop = rect[1];

this.container.style.transformOrigin = `${-popupLeft}px ${-popupTop}px`;
this.container.style.left = `${popupLeft}px`;
this.container.style.top = `${popupTop}px`;
const [pageLLx, pageLLy, pageURx, pageURy] = this.viewport.viewBox;
const pageWidth = pageURx - pageLLx;
const pageHeight = pageURy - pageLLy;

this.container.style.left = `${(100 * (popupLeft - pageLLx)) / pageWidth}%`;
this.container.style.top = `${(100 * (popupTop - pageLLy)) / pageHeight}%`;

this.container.append(popup.render());
return this.container;
Expand Down Expand Up @@ -1961,7 +1926,11 @@ class LineAnnotationElement extends AnnotationElement {
// trigger the popup, not the entire container.
const data = this.data;
const { width, height } = getRectDims(data.rect);
const svg = this.svgFactory.create(width, height);
const svg = this.svgFactory.create(
width,
height,
/* skipDimensions = */ true
);

// PDF coordinates are calculated from a bottom left origin, so transform
// the line coordinates to a top left origin for the SVG element.
Expand Down Expand Up @@ -2006,7 +1975,11 @@ class SquareAnnotationElement extends AnnotationElement {
// popup, not the entire container.
const data = this.data;
const { width, height } = getRectDims(data.rect);
const svg = this.svgFactory.create(width, height);
const svg = this.svgFactory.create(
width,
height,
/* skipDimensions = */ true
);

// The browser draws half of the borders inside the square and half of
// the borders outside the square by default. This behavior cannot be
Expand Down Expand Up @@ -2053,7 +2026,11 @@ class CircleAnnotationElement extends AnnotationElement {
// popup, not the entire container.
const data = this.data;
const { width, height } = getRectDims(data.rect);
const svg = this.svgFactory.create(width, height);
const svg = this.svgFactory.create(
width,
height,
/* skipDimensions = */ true
);

// The browser draws half of the borders inside the circle and half of
// the borders outside the circle by default. This behavior cannot be
Expand Down Expand Up @@ -2103,7 +2080,11 @@ class PolylineAnnotationElement extends AnnotationElement {
// popup, not the entire container.
const data = this.data;
const { width, height } = getRectDims(data.rect);
const svg = this.svgFactory.create(width, height);
const svg = this.svgFactory.create(
width,
height,
/* skipDimensions = */ true
);

// Convert the vertices array to a single points string that the SVG
// polyline element expects ("x1,y1 x2,y2 ..."). PDF coordinates are
Expand Down Expand Up @@ -2191,7 +2172,11 @@ class InkAnnotationElement extends AnnotationElement {
// trigger for the popup.
const data = this.data;
const { width, height } = getRectDims(data.rect);
const svg = this.svgFactory.create(width, height);
const svg = this.svgFactory.create(
width,
height,
/* skipDimensions = */ true
);

for (const inkList of data.inkLists) {
// Convert the ink list to a single points string that the SVG
Expand Down Expand Up @@ -2515,55 +2500,27 @@ class AnnotationLayer {
* @memberof AnnotationLayer
*/
static update(parameters) {
const { page, viewport, annotations, annotationCanvasMap, div } =
parameters;
const transform = viewport.transform;
const matrix = `matrix(${transform.join(",")})`;

let scale, ownMatrix;
for (const data of annotations) {
const elements = div.querySelectorAll(
`[data-annotation-id="${data.id}"]`
);
if (elements) {
for (const element of elements) {
if (data.hasOwnCanvas) {
const rect = Util.normalizeRect([
data.rect[0],
page.view[3] - data.rect[1] + page.view[1],
data.rect[2],
page.view[3] - data.rect[3] + page.view[1],
]);

if (!ownMatrix) {
// When an annotation has its own canvas, then
// the scale has been already applied to the canvas,
// so we musn't scale it twice.
scale = Math.abs(transform[0] || transform[1]);
const ownTransform = transform.slice();
for (let i = 0; i < 4; i++) {
ownTransform[i] = Math.sign(ownTransform[i]);
}
ownMatrix = `matrix(${ownTransform.join(",")})`;
}

const left = rect[0] * scale;
const top = rect[1] * scale;
element.style.left = `${left}px`;
element.style.top = `${top}px`;
element.style.transformOrigin = `${-left}px ${-top}px`;
element.style.transform = ownMatrix;
} else {
element.style.transform = matrix;
}
}
}
}
const { annotationCanvasMap, div } = parameters;

this.#setAnnotationCanvasMap(div, annotationCanvasMap);
div.hidden = false;
}

static setDimensions(div, viewport) {
const { width, height, rotation } = viewport;
const { style } = div;

if (rotation === 0 || rotation === 180) {
style.width = `${width}px`;
style.height = `${height}px`;
} else {
style.width = `${height}px`;
style.height = `${width}px`;
}

div.setAttribute("data-annotation-rotation", rotation);
}

static #setAnnotationCanvasMap(div, annotationCanvasMap) {
if (!annotationCanvasMap) {
return;
Expand Down
10 changes: 7 additions & 3 deletions src/display/base_factory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,18 @@ class BaseSVGFactory {
}
}

create(width, height) {
create(width, height, skipDimensions = false) {
if (width <= 0 || height <= 0) {
throw new Error("Invalid SVG dimensions");
}
const svg = this._createSVG("svg:svg");
svg.setAttribute("version", "1.1");
svg.setAttribute("width", `${width}px`);
svg.setAttribute("height", `${height}px`);

if (!skipDimensions) {
svg.setAttribute("width", `${width}px`);
svg.setAttribute("height", `${height}px`);
}

svg.setAttribute("preserveAspectRatio", "none");
svg.setAttribute("viewBox", `0 0 ${width} ${height}`);

Expand Down
3 changes: 0 additions & 3 deletions src/display/canvas.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3021,9 +3021,6 @@ class CanvasGraphics {
canvasHeight
);
const { canvas, context } = this.annotationCanvas;
const viewportScaleFactorStr = `var(--zoom-factor) * ${PixelsPerInch.PDF_TO_CSS_UNITS}`;
canvas.style.width = `calc(${width}px * ${viewportScaleFactorStr})`;
canvas.style.height = `calc(${height}px * ${viewportScaleFactorStr})`;
this.annotationCanvasMap.set(id, canvas);
this.annotationCanvas.savedCtx = this.ctx;
this.ctx = context;
Expand Down
Loading