Merge pull request #19619 from Snuffleupagus/fix-#savedHasOwnCanvas

Fix the `#savedHasOwnCanvas` handling in the `StampAnnotation` class

src/core/annotation.js

@@ -4905,13 +4905,13 @@ class StrikeOutAnnotation extends MarkupAnnotation {
 }
 
 class StampAnnotation extends MarkupAnnotation {
-  #savedHasOwnCanvas;
+  #savedHasOwnCanvas = null;
 
   constructor(params) {
     super(params);
 
     this.data.annotationType = AnnotationType.STAMP;
-    this.#savedHasOwnCanvas = this.data.hasOwnCanvas = this.data.noRotate;
+    this.data.hasOwnCanvas = this.data.noRotate;
     this.data.isEditable = !this.data.noHTML;
     // We want to be able to add mouse listeners to the annotation.
     this.data.noHTML = false;
@@ -4920,15 +4920,18 @@ class StampAnnotation extends MarkupAnnotation {
   mustBeViewedWhenEditing(isEditing, modifiedIds = null) {
     if (isEditing) {
       if (!this.data.isEditable) {
-        return false;
+        return true;
       }
       // When we're editing, we want to ensure that the stamp annotation is
       // drawn on a canvas in order to use it in the annotation editor layer.
-      this.#savedHasOwnCanvas = this.data.hasOwnCanvas;
+      this.#savedHasOwnCanvas ??= this.data.hasOwnCanvas;
       this.data.hasOwnCanvas = true;
       return true;
     }
-    this.data.hasOwnCanvas = this.#savedHasOwnCanvas;
+    if (this.#savedHasOwnCanvas !== null) {
+      this.data.hasOwnCanvas = this.#savedHasOwnCanvas;
+      this.#savedHasOwnCanvas = null;
+    }
 
     return !modifiedIds?.has(this.data.id);
   }

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

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(
-          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;

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++;

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[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];

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;
       }
     }

src/display/editor/tools.js

@@ -42,17 +42,6 @@ function bindEvents(obj, element, names) {
   }
 }
 
-/**
- * Convert a number between 0 and 100 into an hex number between 0 and 255.
- * @param {number} opacity
- * @return {string}
- */
-function opacityToHex(opacity) {
-  return Math.round(Math.min(255, Math.max(1, 255 * opacity)))
-    .toString(16)
-    .padStart(2, "0");
-}
-
 /**
  * Class to create some unique ids for the different editors.
  */
@@ -2562,5 +2551,4 @@ export {
   ColorManager,
   CommandManager,
   KeyboardManager,
-  opacityToHex,
 };

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/integration/freetext_editor_spec.mjs

@@ -28,7 +28,7 @@ import {
   getRect,
   getSerialized,
   hover,
-  isCanvasWhite,
+  isCanvasMonochrome,
   kbBigMoveDown,
   kbBigMoveLeft,
   kbBigMoveRight,
@@ -837,7 +837,7 @@ describe("FreeText Editor", () => {
           await switchToFreeText(page);
 
           // The page has been re-rendered but with no freetext annotations.
-          let isWhite = await isCanvasWhite(page, 1);
+          let isWhite = await isCanvasMonochrome(page, 1, null, 0xffffffff);
           expect(isWhite).withContext(`In ${browserName}`).toBeTrue();
 
           let editorIds = await getEditors(page, "freeText");
@@ -880,7 +880,7 @@ describe("FreeText Editor", () => {
           editorIds = await getEditors(page, "freeText");
           expect(editorIds.length).withContext(`In ${browserName}`).toEqual(1);
 
-          isWhite = await isCanvasWhite(page, 1, editorRect);
+          isWhite = await isCanvasMonochrome(page, 1, editorRect, 0xffffffff);
           expect(isWhite).withContext(`In ${browserName}`).toBeTrue();
 
           // Check we've now a div containing the text.

test/integration/ink_editor_spec.mjs

@@ -23,7 +23,7 @@ import {
   getEditorSelector,
   getRect,
   getSerialized,
-  isCanvasWhite,
+  isCanvasMonochrome,
   kbRedo,
   kbUndo,
   loadAndWait,
@@ -747,7 +747,12 @@ describe("Ink Editor", () => {
           await switchToInk(page);
 
           // The page has been re-rendered but with no ink annotations.
-          let isWhite = await isCanvasWhite(page, 1, annotationsRect);
+          let isWhite = await isCanvasMonochrome(
+            page,
+            1,
+            annotationsRect,
+            0xffffffff
+          );
           expect(isWhite).withContext(`In ${browserName}`).toBeTrue();
 
           let editorIds = await getEditors(page, "ink");
@@ -781,7 +786,7 @@ describe("Ink Editor", () => {
           editorIds = await getEditors(page, "ink");
           expect(editorIds.length).withContext(`In ${browserName}`).toEqual(1);
 
-          isWhite = await isCanvasWhite(page, 1, editorRect);
+          isWhite = await isCanvasMonochrome(page, 1, editorRect, 0xffffffff);
           expect(isWhite).withContext(`In ${browserName}`).toBeTrue();
 
           // Check we've now a svg with a red stroke.

test/integration/stamp_editor_spec.mjs

@@ -30,6 +30,7 @@ import {
   getFirstSerialized,
   getRect,
   getSerialized,
+  isCanvasMonochrome,
   isVisible,
   kbBigMoveDown,
   kbBigMoveRight,
@@ -45,6 +46,7 @@ import {
   waitForAnnotationEditorLayer,
   waitForAnnotationModeChanged,
   waitForEntryInStorage,
+  waitForPageRendered,
   waitForSelectedEditor,
   waitForSerialized,
   waitForTimeout,
@@ -1819,4 +1821,57 @@ describe("Stamp Editor", () => {
       );
     });
   });
+
+  describe("Switch to edit mode, zoom and check that the non-editable stamp is still there", () => {
+    const annotationSelector = getAnnotationSelector("14R");
+
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait("red_stamp.pdf", annotationSelector, 20);
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must check if the canvas is still red", async () => {
+      await Promise.all(
+        pages.map(async ([, page]) => {
+          expect(
+            await isCanvasMonochrome(page, 1, null, 0xff0000ff)
+          ).toBeTrue();
+
+          await switchToStamp(page);
+
+          expect(
+            await isCanvasMonochrome(page, 1, null, 0xff0000ff)
+          ).toBeTrue();
+
+          const rectPage = await getRect(
+            page,
+            `.page[data-page-number = "1"] .annotationEditorLayer`
+          );
+
+          const handle = await waitForPageRendered(page, 1);
+          const originX = rectPage.x + rectPage.width / 2;
+          const originY = rectPage.y + rectPage.height / 2;
+          await page.evaluate(
+            origin => {
+              window.PDFViewerApplication.pdfViewer.increaseScale({
+                scaleFactor: 2,
+                origin,
+              });
+            },
+            [originX, originY]
+          );
+          await awaitPromise(handle);
+
+          expect(
+            await isCanvasMonochrome(page, 1, null, 0xff0000ff)
+          ).toBeTrue();
+        })
+      );
+    });
+  });
 });

test/integration/test_utils.mjs

@@ -838,9 +838,9 @@ function waitForNoElement(page, selector) {
   );
 }
 
-function isCanvasWhite(page, pageNumber, rectangle) {
+function isCanvasMonochrome(page, pageNumber, rectangle, color) {
   return page.evaluate(
-    (rect, pageN) => {
+    (rect, pageN, col) => {
       const canvas = document.querySelector(
         `.page[data-page-number = "${pageN}"] .canvasWrapper canvas`
       );
@@ -853,10 +853,11 @@ function isCanvasWhite(page, pageNumber, rectangle) {
         rect.width,
         rect.height
       );
-      return new Uint32Array(data.buffer).every(x => x === 0xffffffff);
+      return new Uint32Array(data.buffer).every(x => x === col);
     },
     rectangle,
-    pageNumber
+    pageNumber,
+    color
   );
 }
 
@@ -925,7 +926,7 @@ export {
   getSpanRectFromText,
   getXY,
   hover,
-  isCanvasWhite,
+  isCanvasMonochrome,
   isVisible,
   kbBigMoveDown,
   kbBigMoveLeft,

test/pdfs/.gitignore

@@ -710,3 +710,4 @@
 !bug1019475_2.pdf
 !issue19505.pdf
 !colors.pdf
+!red_stamp.pdf

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