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

@@ -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);
+    return MathClamp((1 + 0.055) * color ** (1 / 2.4) - 0.055, 0, 1);
-  }
-
-  #adjustToRange(min, max, value) {
-    return Math.max(min, Math.min(max, value));
   }
 
   #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 A = MathClamp(src[srcOffset] * scale, 0, 1);
-    const B = this.#adjustToRange(0, 1, src[srcOffset + 1] * scale);
+    const B = MathClamp(src[srcOffset + 1] * scale, 0, 1);
-    const C = this.#adjustToRange(0, 1, src[srcOffset + 2] * scale);
+    const C = MathClamp(src[srcOffset + 2] * scale, 0, 1);
 
     // A <---> AGR in the spec
     // B <---> BGG in the spec

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":

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(
+        const xi = MathClamp(src[srcOffset + i], domain_2i, domain_2i_1);
-          Math.max(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) {
+      // Clamp to domain.
-        if (v > max) {
+      const v = MathClamp(src[srcOffset], domain[0], domain[1]);
-          v = max;
-        } else if (v < min) {
-          v = min;
-        }
-        return v;
-      };
-
-      // clip to domain
-      const v = clip(src[srcOffset], domain[0], domain[1]);
       // calculate which bound the value is in
       const length = bounds.length;
       let i;

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(
+        // Decode and clamp. The formula is different from the spec because we
-          buffer[index],
+        // don't decode to float range [0,1], we decode it in the [0,max] range.
-          decodeAddends[j],
+        buffer[index] = MathClamp(
-          decodeCoefficients[j],
+          decodeAddends[j] + buffer[index] * decodeCoefficients[j],
+          0,
           max
         );
         index++;

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) {

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[0] = MathClamp(bbox[0] - marginX, 0, 1);
-    bbox[1] = Math.min(1, Math.max(0, bbox[1] - marginY));
+    bbox[1] = MathClamp(bbox[1] - marginY, 0, 1);
-    bbox[2] = Math.min(1, Math.max(0, bbox[2] + marginX));
+    bbox[2] = MathClamp(bbox[2] + marginX, 0, 1);
-    bbox[3] = Math.min(1, Math.max(0, bbox[3] + marginY));
+    bbox[3] = MathClamp(bbox[3] + marginY, 0, 1);
 
     bbox[2] -= bbox[0];
     bbox[3] -= bbox[1];

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));
+          x = MathClamp(x, 0, pageWidth - width);
-          y = Math.max(0, Math.min(pageHeight - height, y));
+          y = MathClamp(y, 0, pageHeight - height);
           break;
         case 90:
-          x = Math.max(0, Math.min(pageWidth - height, x));
+          x = MathClamp(x, 0, pageWidth - height);
-          y = Math.min(pageHeight, Math.max(width, y));
+          y = MathClamp(y, width, pageHeight);
           break;
         case 180:
-          x = Math.min(pageWidth, Math.max(width, x));
+          x = MathClamp(x, width, pageWidth);
-          y = Math.min(pageHeight, Math.max(height, y));
+          y = MathClamp(y, height, pageHeight);
           break;
         case 270:
-          x = Math.min(pageWidth, Math.max(height, x));
+          x = MathClamp(x, height, pageWidth);
-          y = Math.max(0, Math.min(pageHeight - width, y));
+          y = MathClamp(y, 0, pageHeight - width);
           break;
       }
     }

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,

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,

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,

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,

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");