From 07bbbf75a5fd3d7dafa8c040dba862f6b823ef27 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 6 Mar 2025 12:57:14 +0100 Subject: [PATCH] Introduce a helper function for clamping a value to a range Currently we have a number of spots in the code-base where we need to clamp a value to a [min, max] range. This is either implemented using `Math.min`/`Math.max` or with a local helper function, which leads to some unnecessary duplication. Hence this patch adds and re-uses a single helper function for this, which we'll hopefully be able to remove in the future once https://github.com/tc39/proposal-math-clamp/ becomes generally available. --- src/core/colorspace.js | 15 ++++++--------- src/core/colorspace_utils.js | 4 ++-- src/core/function.js | 23 ++++++----------------- src/core/image.js | 25 ++++++------------------- src/core/xfa/utils.js | 4 ++-- src/display/editor/drawers/inkdraw.js | 10 +++++----- src/display/editor/editor.js | 23 ++++++++++++++--------- src/pdf.js | 2 ++ src/shared/util.js | 7 +++++++ test/unit/pdf_spec.js | 2 ++ web/pdfjs.js | 2 ++ web/ui_utils.js | 8 +++----- 12 files changed, 57 insertions(+), 68 deletions(-) diff --git a/src/core/colorspace.js b/src/core/colorspace.js index 1f33f628c310b..287b6cb0be88a 100644 --- a/src/core/colorspace.js +++ b/src/core/colorspace.js @@ -18,6 +18,7 @@ import { FeatureTest, FormatError, info, + MathClamp, shadow, unreachable, warn, @@ -946,7 +947,7 @@ class CalRGBCS extends ColorSpace { #sRGBTransferFunction(color) { // See http://en.wikipedia.org/wiki/SRGB. if (color <= 0.0031308) { - return this.#adjustToRange(0, 1, 12.92 * color); + return MathClamp(12.92 * color, 0, 1); } // Optimization: // If color is close enough to 1, skip calling the following transform @@ -957,11 +958,7 @@ class CalRGBCS extends ColorSpace { if (color >= 0.99554525) { return 1; } - return this.#adjustToRange(0, 1, (1 + 0.055) * color ** (1 / 2.4) - 0.055); - } - - #adjustToRange(min, max, value) { - return Math.max(min, Math.min(max, value)); + return MathClamp((1 + 0.055) * color ** (1 / 2.4) - 0.055, 0, 1); } #decodeL(L) { @@ -1057,9 +1054,9 @@ class CalRGBCS extends ColorSpace { #toRgb(src, srcOffset, dest, destOffset, scale) { // A, B and C represent a red, green and blue components of a calibrated // rgb space. - const A = this.#adjustToRange(0, 1, src[srcOffset] * scale); - const B = this.#adjustToRange(0, 1, src[srcOffset + 1] * scale); - const C = this.#adjustToRange(0, 1, src[srcOffset + 2] * scale); + const A = MathClamp(src[srcOffset] * scale, 0, 1); + const B = MathClamp(src[srcOffset + 1] * scale, 0, 1); + const C = MathClamp(src[srcOffset + 2] * scale, 0, 1); // A <---> AGR in the spec // B <---> BGG in the spec diff --git a/src/core/colorspace_utils.js b/src/core/colorspace_utils.js index 03122738b2ac7..73fe625293a48 100644 --- a/src/core/colorspace_utils.js +++ b/src/core/colorspace_utils.js @@ -26,7 +26,7 @@ import { PatternCS, } from "./colorspace.js"; import { Dict, Name, Ref } from "./primitives.js"; -import { shadow, unreachable, warn } from "../shared/util.js"; +import { MathClamp, shadow, unreachable, warn } from "../shared/util.js"; import { IccColorSpace } from "./icc_colorspace.js"; import { MissingDataException } from "./core_utils.js"; @@ -245,7 +245,7 @@ class ColorSpaceUtils { case "I": case "Indexed": baseCS = this.#subParse(cs[1], options); - const hiVal = Math.max(0, Math.min(xref.fetchIfRef(cs[2]), 255)); + const hiVal = MathClamp(xref.fetchIfRef(cs[2]), 0, 255); const lookup = xref.fetchIfRef(cs[3]); return new IndexedCS(baseCS, hiVal, lookup); case "Separation": diff --git a/src/core/function.js b/src/core/function.js index 6864b13d4980b..7cd9eee429faf 100644 --- a/src/core/function.js +++ b/src/core/function.js @@ -18,6 +18,7 @@ import { FeatureTest, FormatError, info, + MathClamp, shadow, unreachable, } from "../shared/util.js"; @@ -263,10 +264,7 @@ class PDFFunction { // x_i' = min(max(x_i, Domain_2i), Domain_2i+1) const domain_2i = domain[i][0]; const domain_2i_1 = domain[i][1]; - const xi = Math.min( - Math.max(src[srcOffset + i], domain_2i), - domain_2i_1 - ); + const xi = MathClamp(src[srcOffset + i], domain_2i, domain_2i_1); // e_i = Interpolate(x_i', Domain_2i, Domain_2i+1, // Encode_2i, Encode_2i+1) @@ -280,7 +278,7 @@ class PDFFunction { // e_i' = min(max(e_i, 0), Size_i - 1) const size_i = size[i]; - e = Math.min(Math.max(e, 0), size_i - 1); + e = MathClamp(e, 0, size_i - 1); // Adjusting the cube: N and vertex sample index const e0 = e < size_i - 1 ? Math.floor(e) : e - 1; // e1 = e0 + 1; @@ -314,7 +312,7 @@ class PDFFunction { rj = interpolate(rj, 0, 1, decode[j][0], decode[j][1]); // y_j = min(max(r_j, range_2j), range_2j+1) - dest[destOffset + j] = Math.min(Math.max(rj, range[j][0]), range[j][1]); + dest[destOffset + j] = MathClamp(rj, range[j][0], range[j][1]); } }; } @@ -361,17 +359,8 @@ class PDFFunction { const tmpBuf = new Float32Array(1); return function constructStichedFn(src, srcOffset, dest, destOffset) { - const clip = function constructStichedFromIRClip(v, min, max) { - if (v > max) { - v = max; - } else if (v < min) { - v = min; - } - return v; - }; - - // clip to domain - const v = clip(src[srcOffset], domain[0], domain[1]); + // Clamp to domain. + const v = MathClamp(src[srcOffset], domain[0], domain[1]); // calculate which bound the value is in const length = bounds.length; let i; diff --git a/src/core/image.js b/src/core/image.js index e3e99907fac61..454671fa03bfb 100644 --- a/src/core/image.js +++ b/src/core/image.js @@ -18,6 +18,7 @@ import { FeatureTest, FormatError, ImageKind, + MathClamp, warn, } from "../shared/util.js"; import { @@ -33,21 +34,6 @@ import { JpegStream } from "./jpeg_stream.js"; import { JpxImage } from "./jpx.js"; import { Name } from "./primitives.js"; -/** - * Decode and clamp a value. The formula is different from the spec because we - * don't decode to float range [0,1], we decode it in the [0,max] range. - */ -function decodeAndClamp(value, addend, coefficient, max) { - value = addend + value * coefficient; - // Clamp the value to the range - if (value < 0) { - value = 0; - } else if (value > max) { - value = max; - } - return value; -} - /** * Resizes an image mask with 1 component. * @param {TypedArray} src - The source buffer. @@ -487,10 +473,11 @@ class PDFImage { let index = 0; for (i = 0, ii = this.width * this.height; i < ii; i++) { for (let j = 0; j < numComps; j++) { - buffer[index] = decodeAndClamp( - buffer[index], - decodeAddends[j], - decodeCoefficients[j], + // Decode and clamp. The formula is different from the spec because we + // don't decode to float range [0,1], we decode it in the [0,max] range. + buffer[index] = MathClamp( + decodeAddends[j] + buffer[index] * decodeCoefficients[j], + 0, max ); index++; diff --git a/src/core/xfa/utils.js b/src/core/xfa/utils.js index 6db780e4a06f9..327464a4da531 100644 --- a/src/core/xfa/utils.js +++ b/src/core/xfa/utils.js @@ -13,7 +13,7 @@ * limitations under the License. */ -import { shadow } from "../../shared/util.js"; +import { MathClamp, shadow } from "../../shared/util.js"; const dimConverters = { pt: x => x, @@ -143,7 +143,7 @@ function getColor(data, def = [0, 0, 0]) { const color = data .trim() .split(/\s*,\s*/) - .map(c => Math.min(Math.max(0, parseInt(c.trim(), 10)), 255)) + .map(c => MathClamp(parseInt(c.trim(), 10), 0, 255)) .map(c => (isNaN(c) ? 0 : c)); if (color.length < 3) { diff --git a/src/display/editor/drawers/inkdraw.js b/src/display/editor/drawers/inkdraw.js index 2af5a48f17c1b..0be1c7bde7f2d 100644 --- a/src/display/editor/drawers/inkdraw.js +++ b/src/display/editor/drawers/inkdraw.js @@ -13,8 +13,8 @@ * limitations under the License. */ +import { MathClamp, Util } from "../../../shared/util.js"; import { Outline } from "./outline.js"; -import { Util } from "../../../shared/util.js"; class InkDrawOutliner { // The last 3 points of the line. @@ -616,10 +616,10 @@ class InkDrawOutline extends Outline { } const [marginX, marginY] = this.#getMarginComponents(); - bbox[0] = Math.min(1, Math.max(0, bbox[0] - marginX)); - bbox[1] = Math.min(1, Math.max(0, bbox[1] - marginY)); - bbox[2] = Math.min(1, Math.max(0, bbox[2] + marginX)); - bbox[3] = Math.min(1, Math.max(0, bbox[3] + marginY)); + bbox[0] = MathClamp(bbox[0] - marginX, 0, 1); + bbox[1] = MathClamp(bbox[1] - marginY, 0, 1); + bbox[2] = MathClamp(bbox[2] + marginX, 0, 1); + bbox[3] = MathClamp(bbox[3] + marginY, 0, 1); bbox[2] -= bbox[0]; bbox[3] -= bbox[1]; diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index 5ffa85d5a8417..8cf46aa0adf44 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -22,7 +22,12 @@ import { ColorManager, KeyboardManager, } from "./tools.js"; -import { FeatureTest, shadow, unreachable } from "../../shared/util.js"; +import { + FeatureTest, + MathClamp, + shadow, + unreachable, +} from "../../shared/util.js"; import { noContextMenu, stopEvent } from "../display_utils.js"; import { AltText } from "./alt_text.js"; import { EditorToolbar } from "./toolbar.js"; @@ -613,20 +618,20 @@ class AnnotationEditor { if (this._mustFixPosition) { switch (rotation) { case 0: - x = Math.max(0, Math.min(pageWidth - width, x)); - y = Math.max(0, Math.min(pageHeight - height, y)); + x = MathClamp(x, 0, pageWidth - width); + y = MathClamp(y, 0, pageHeight - height); break; case 90: - x = Math.max(0, Math.min(pageWidth - height, x)); - y = Math.min(pageHeight, Math.max(width, y)); + x = MathClamp(x, 0, pageWidth - height); + y = MathClamp(y, width, pageHeight); break; case 180: - x = Math.min(pageWidth, Math.max(width, x)); - y = Math.min(pageHeight, Math.max(height, y)); + x = MathClamp(x, width, pageWidth); + y = MathClamp(y, height, pageHeight); break; case 270: - x = Math.min(pageWidth, Math.max(height, x)); - y = Math.max(0, Math.min(pageHeight - width, y)); + x = MathClamp(x, height, pageWidth); + y = MathClamp(y, 0, pageHeight - width); break; } } diff --git a/src/pdf.js b/src/pdf.js index eea401cd6c893..e9a1b974c9020 100644 --- a/src/pdf.js +++ b/src/pdf.js @@ -33,6 +33,7 @@ import { getUuid, ImageKind, InvalidPDFException, + MathClamp, normalizeUnicode, OPS, PasswordResponses, @@ -119,6 +120,7 @@ export { isDataScheme, isPdfFile, isValidExplicitDest, + MathClamp, noContextMenu, normalizeUnicode, OPS, diff --git a/src/shared/util.js b/src/shared/util.js index 5775c0d4bfb6e..e3c3ad2dcf59a 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -1128,6 +1128,12 @@ function _isValidExplicitDest(validRef, validName, dest) { return true; } +// TOOD: Replace all occurrences of this function with `Math.clamp` once +// https://github.com/tc39/proposal-math-clamp/ is generally available. +function MathClamp(v, min, max) { + return Math.min(Math.max(v, min), max); +} + // TODO: Remove this once `Uint8Array.prototype.toHex` is generally available. function toHexUtil(arr) { if (Uint8Array.prototype.toHex) { @@ -1202,6 +1208,7 @@ export { isNodeJS, LINE_DESCENT_FACTOR, LINE_FACTOR, + MathClamp, normalizeUnicode, objectFromMap, objectSize, diff --git a/test/unit/pdf_spec.js b/test/unit/pdf_spec.js index 95267085ea6cf..184afaecbb5ba 100644 --- a/test/unit/pdf_spec.js +++ b/test/unit/pdf_spec.js @@ -24,6 +24,7 @@ import { getUuid, ImageKind, InvalidPDFException, + MathClamp, normalizeUnicode, OPS, PasswordResponses, @@ -96,6 +97,7 @@ const expectedAPI = Object.freeze({ isDataScheme, isPdfFile, isValidExplicitDest, + MathClamp, noContextMenu, normalizeUnicode, OPS, diff --git a/web/pdfjs.js b/web/pdfjs.js index 2c8f7fff7e48f..5ba83c1411f37 100644 --- a/web/pdfjs.js +++ b/web/pdfjs.js @@ -40,6 +40,7 @@ const { isDataScheme, isPdfFile, isValidExplicitDest, + MathClamp, noContextMenu, normalizeUnicode, OPS, @@ -92,6 +93,7 @@ export { isDataScheme, isPdfFile, isValidExplicitDest, + MathClamp, noContextMenu, normalizeUnicode, OPS, diff --git a/web/ui_utils.js b/web/ui_utils.js index dd2f250eae2f1..99c4ff58a636f 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -13,6 +13,8 @@ * limitations under the License. */ +import { MathClamp } from "pdfjs-lib"; + const DEFAULT_SCALE_VALUE = "auto"; const DEFAULT_SCALE = 1.0; const DEFAULT_SCALE_DELTA = 1.1; @@ -676,10 +678,6 @@ const docStyle = ? null : document.documentElement.style; -function clamp(v, min, max) { - return Math.min(Math.max(v, min), max); -} - class ProgressBar { #classList = null; @@ -701,7 +699,7 @@ class ProgressBar { } set percent(val) { - this.#percent = clamp(val, 0, 100); + this.#percent = MathClamp(val, 0, 100); if (isNaN(val)) { this.#classList.add("indeterminate");