Re-factor and simplify the getQuadPoints helper function

The use of `Array.prototype.reduce()` is, in my opinion, hurting overall readability since it's not particularly easy to look at the relevant code and immediately understand what's going on here. Furthermore this code leads to strictly speaking unnecessary allocations and parsing, since we could just track the min/max values directly in the relevant loop instead.
This commit is contained in:
Jonas Jenwald 2022-11-20 18:16:27 +01:00
parent 67741aeaa9
commit 4b02610e8c

View File

@ -367,10 +367,6 @@ function getPdfColorArray(color) {
} }
function getQuadPoints(dict, rect) { function getQuadPoints(dict, rect) {
if (!dict.has("QuadPoints")) {
return null;
}
// The region is described as a number of quadrilaterals. // The region is described as a number of quadrilaterals.
// Each quadrilateral must consist of eight coordinates. // Each quadrilateral must consist of eight coordinates.
const quadPoints = dict.getArray("QuadPoints"); const quadPoints = dict.getArray("QuadPoints");
@ -387,54 +383,49 @@ function getQuadPoints(dict, rect) {
// Each series of eight numbers represents the coordinates for one // Each series of eight numbers represents the coordinates for one
// quadrilateral in the order [x1, y1, x2, y2, x3, y3, x4, y4]. // quadrilateral in the order [x1, y1, x2, y2, x3, y3, x4, y4].
// Convert this to an array of objects with x and y coordinates. // Convert this to an array of objects with x and y coordinates.
quadPointsLists.push([]); let minX = Infinity,
maxX = -Infinity,
minY = Infinity,
maxY = -Infinity;
for (let j = i * 8, jj = i * 8 + 8; j < jj; j += 2) { for (let j = i * 8, jj = i * 8 + 8; j < jj; j += 2) {
const x = quadPoints[j]; const x = quadPoints[j];
const y = quadPoints[j + 1]; const y = quadPoints[j + 1];
// The quadpoints should be ignored if any coordinate in the array minX = Math.min(x, minX);
// lies outside the region specified by the rectangle. The rectangle maxX = Math.max(x, maxX);
// can be `null` for markup annotations since their rectangle may be minY = Math.min(y, minY);
// incorrect (fixes bug 1538111). maxY = Math.max(y, maxY);
if (
rect !== null &&
(x < rect[0] || x > rect[2] || y < rect[1] || y > rect[3])
) {
return null;
}
quadPointsLists[i].push({ x, y });
} }
} // The quadpoints should be ignored if any coordinate in the array
// lies outside the region specified by the rectangle. The rectangle
// The PDF specification states in section 12.5.6.10 (figure 64) that the // can be `null` for markup annotations since their rectangle may be
// order of the quadpoints should be bottom left, bottom right, top right // incorrect (fixes bug 1538111).
// and top left. However, in practice PDF files use a different order, if (
// namely bottom left, bottom right, top left and top right (this is also rect !== null &&
// mentioned on https://github.com/highkite/pdfAnnotate#QuadPoints), so (minX < rect[0] || maxX > rect[2] || minY < rect[1] || maxY > rect[3])
// this is the actual order we should work with. However, the situation is ) {
// even worse since Adobe's own applications and other applications violate return null;
// the specification and create annotations with other orders, namely top }
// left, top right, bottom left and bottom right or even top left, top right, // The PDF specification states in section 12.5.6.10 (figure 64) that the
// bottom right and bottom left. To avoid inconsistency and broken rendering, // order of the quadpoints should be bottom left, bottom right, top right
// we normalize all lists to put the quadpoints in the same standard order // and top left. However, in practice PDF files use a different order,
// (see https://stackoverflow.com/a/10729881). // namely bottom left, bottom right, top left and top right (this is also
return quadPointsLists.map(quadPointsList => { // mentioned on https://github.com/highkite/pdfAnnotate#QuadPoints), so
const [minX, maxX, minY, maxY] = quadPointsList.reduce( // this is the actual order we should work with. However, the situation is
([mX, MX, mY, MY], quadPoint) => [ // even worse since Adobe's own applications and other applications violate
Math.min(mX, quadPoint.x), // the specification and create annotations with other orders, namely top
Math.max(MX, quadPoint.x), // left, top right, bottom left and bottom right or even top left,
Math.min(mY, quadPoint.y), // top right, bottom right and bottom left. To avoid inconsistency and
Math.max(MY, quadPoint.y), // broken rendering, we normalize all lists to put the quadpoints in the
], // same standard order (see https://stackoverflow.com/a/10729881).
[Number.MAX_VALUE, Number.MIN_VALUE, Number.MAX_VALUE, Number.MIN_VALUE] quadPointsLists.push([
);
return [
{ x: minX, y: maxY }, { x: minX, y: maxY },
{ x: maxX, y: maxY }, { x: maxX, y: maxY },
{ x: minX, y: minY }, { x: minX, y: minY },
{ x: maxX, y: minY }, { x: maxX, y: minY },
]; ]);
}); }
return quadPointsLists;
} }
function getTransformMatrix(rect, bbox, matrix) { function getTransformMatrix(rect, bbox, matrix) {