Merge pull request #16642 from calixteman/issue16633

[Editor] Try to make the position of an edited FreeText the more accurated as possible

package-lock.json

@@ -45,6 +45,7 @@
         "mkdirp": "^3.0.1",
         "needle": "^3.2.0",
         "path2d-polyfill": "^2.0.1",
+        "pngjs": "^7.0.0",
         "postcss": "^8.4.24",
         "postcss-dir-pseudo-class": "^7.0.2",
         "prettier": "^2.8.8",
@@ -4337,16 +4338,6 @@
         "url": "https://bevry.me/fund"
       }
     },
-    "node_modules/bindings": {
-      "version": "1.5.0",
-      "resolved": "https://registry.npmjs.org/bindings/-/bindings-1.5.0.tgz",
-      "integrity": "sha512-p2q/t/mhvuOj/UeLlV6566GD/guowlr0hHxClI0W9m7MWYkL1F0hLo+0Aexs9HSPCtR1SXQ0TD3MMKrXZajbiQ==",
-      "dev": true,
-      "optional": true,
-      "dependencies": {
-        "file-uri-to-path": "1.0.0"
-      }
-    },
     "node_modules/bluebird": {
       "version": "3.7.2",
       "resolved": "https://registry.npmjs.org/bluebird/-/bluebird-3.7.2.tgz",
@@ -7329,13 +7320,6 @@
         "node": "^10.12.0 || >=12.0.0"
       }
     },
-    "node_modules/file-uri-to-path": {
-      "version": "1.0.0",
-      "resolved": "https://registry.npmjs.org/file-uri-to-path/-/file-uri-to-path-1.0.0.tgz",
-      "integrity": "sha512-0Zt+s3L7Vf1biwWZ29aARiVYLx7iMGnEUl9x33fbB/j3jR81u/O2LbqK+Bm1CDSNDKVtJ/YjwY7TUd5SkeLQLw==",
-      "dev": true,
-      "optional": true
-    },
     "node_modules/fill-range": {
       "version": "4.0.0",
       "resolved": "https://registry.npmjs.org/fill-range/-/fill-range-4.0.0.tgz",
@@ -7599,25 +7583,6 @@
       "integrity": "sha1-FQStJSMVjKpA20onh8sBQRmU6k8=",
       "dev": true
     },
-    "node_modules/fsevents": {
-      "version": "1.2.13",
-      "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-1.2.13.tgz",
-      "integrity": "sha512-oWb1Z6mkHIskLzEJ/XWX0srkpkTQ7vaopMQkyaEIoq0fmtFVxOthb8cCxeT+p3ynTdkk/RZwbgG4brR5BeWECw==",
-      "deprecated": "fsevents 1 will break on node v14+ and could be using insecure binaries. Upgrade to fsevents 2.",
-      "dev": true,
-      "hasInstallScript": true,
-      "optional": true,
-      "os": [
-        "darwin"
-      ],
-      "dependencies": {
-        "bindings": "^1.5.0",
-        "nan": "^2.12.1"
-      },
-      "engines": {
-        "node": ">= 4.0"
-      }
-    },
     "node_modules/function-bind": {
       "version": "1.1.1",
       "resolved": "https://registry.npmjs.org/function-bind/-/function-bind-1.1.1.tgz",
@@ -13202,7 +13167,7 @@
     },
     "node_modules/npm/node_modules/lodash._baseindexof": {
       "version": "3.1.0",
-      "extraneous": true,
+      "dev": true,
       "inBundle": true,
       "license": "MIT"
     },
@@ -13218,19 +13183,19 @@
     },
     "node_modules/npm/node_modules/lodash._bindcallback": {
       "version": "3.0.1",
-      "extraneous": true,
+      "dev": true,
       "inBundle": true,
       "license": "MIT"
     },
     "node_modules/npm/node_modules/lodash._cacheindexof": {
       "version": "3.0.2",
-      "extraneous": true,
+      "dev": true,
       "inBundle": true,
       "license": "MIT"
     },
     "node_modules/npm/node_modules/lodash._createcache": {
       "version": "3.1.2",
-      "extraneous": true,
+      "dev": true,
       "inBundle": true,
       "license": "MIT",
       "dependencies": {
@@ -13245,7 +13210,7 @@
     },
     "node_modules/npm/node_modules/lodash._getnative": {
       "version": "3.9.1",
-      "extraneous": true,
+      "dev": true,
       "inBundle": true,
       "license": "MIT"
     },
@@ -13263,7 +13228,7 @@
     },
     "node_modules/npm/node_modules/lodash.restparam": {
       "version": "3.6.1",
-      "extraneous": true,
+      "dev": true,
       "inBundle": true,
       "license": "MIT"
     },
@@ -16130,6 +16095,15 @@
         "node": ">=4"
       }
     },
+    "node_modules/pngjs": {
+      "version": "7.0.0",
+      "resolved": "https://registry.npmjs.org/pngjs/-/pngjs-7.0.0.tgz",
+      "integrity": "sha512-LKWqWJRhstyYo9pGvgor/ivk2w94eSjE3RGVuzLGlr3NmD8bf7RcYGze1mNdEHRP6TRP6rMuDHk5t44hnTRyow==",
+      "dev": true,
+      "engines": {
+        "node": ">=14.19.0"
+      }
+    },
     "node_modules/posix-character-classes": {
       "version": "0.1.1",
       "resolved": "https://registry.npmjs.org/posix-character-classes/-/posix-character-classes-0.1.1.tgz",

package.json

@@ -38,6 +38,7 @@
     "mkdirp": "^3.0.1",
     "needle": "^3.2.0",
     "path2d-polyfill": "^2.0.1",
+    "pngjs": "^7.0.0",
     "postcss": "^8.4.24",
     "postcss-dir-pseudo-class": "^7.0.2",
     "prettier": "^2.8.8",

src/core/annotation.js

@@ -546,7 +546,7 @@ class Annotation {
 
     const MK = dict.get("MK");
     this.setBorderAndBackgroundColors(MK);
-    this.setRotation(MK);
+    this.setRotation(MK, dict);
     this.ref = params.ref instanceof Ref ? params.ref : null;
 
     this._streams = [];
@@ -838,10 +838,14 @@ class Annotation {
     }
   }
 
-  setRotation(mk) {
+  setRotation(mk, dict) {
     this.rotation = 0;
+    let angle;
     if (mk instanceof Dict) {
-      let angle = mk.get("R") || 0;
+      angle = mk.get("R") || 0;
+    } else {
+      angle = dict.get("Rotate") || 0;
+    }
     if (Number.isInteger(angle) && angle !== 0) {
       angle %= 360;
       if (angle < 0) {
@@ -852,7 +856,6 @@ class Annotation {
       }
     }
   }
-  }
 
   /**
    * Set the color for background and border if any.
@@ -1069,6 +1072,7 @@ class Annotation {
 
     const text = [];
     const buffer = [];
+    let firstPosition = null;
     const sink = {
       desiredSize: Math.Infinity,
       ready: true,
@@ -1078,6 +1082,7 @@ class Annotation {
           if (item.str === undefined) {
             continue;
           }
+          firstPosition ||= item.transform.slice(-2);
           buffer.push(item.str);
           if (item.hasEOL) {
             text.push(buffer.join(""));
@@ -1102,6 +1107,17 @@ class Annotation {
     }
 
     if (text.length > 1 || text[0]) {
+      const appearanceDict = this.appearance.dict;
+      const bbox = appearanceDict.getArray("BBox") || [0, 0, 1, 1];
+      const matrix = appearanceDict.getArray("Matrix") || [1, 0, 0, 1, 0, 0];
+      const rect = this.data.rect;
+      const transform = getTransformMatrix(rect, bbox, matrix);
+      transform[4] -= rect[0];
+      transform[5] -= rect[1];
+      firstPosition = Util.applyTransform(firstPosition, transform);
+      firstPosition = Util.applyTransform(firstPosition, matrix);
+
+      this.data.textPosition = firstPosition;
       this.data.textContent = text;
     }
   }

src/display/annotation_layer.js

@@ -304,6 +304,9 @@ class AnnotationElement {
   }
 
   setRotation(angle, container = this.container) {
+    if (!this.data.rect) {
+      return;
+    }
     const { pageWidth, pageHeight } = this.parent.viewport.rawDims;
     const { width, height } = getRectDims(this.data.rect);
 
@@ -2210,6 +2213,7 @@ class FreeTextAnnotationElement extends AnnotationElement {
     );
     super(parameters, { isRenderable, ignoreBorder: true });
     this.textContent = parameters.data.textContent;
+    this.textPosition = parameters.data.textPosition;
     this.annotationEditorType = AnnotationEditorType.FREETEXT;
   }
 

src/display/editor/editor.js

@@ -296,6 +296,24 @@ class AnnotationEditor {
     }
   }
 
+  /**
+   * Convert a page translation into a screen one.
+   * @param {number} x
+   * @param {number} y
+   */
+  pageTranslationToScreen(x, y) {
+    switch (this.parentRotation) {
+      case 90:
+        return [-y, x];
+      case 180:
+        return [-x, -y];
+      case 270:
+        return [y, -x];
+      default:
+        return [x, y];
+    }
+  }
+
   get parentScale() {
     return this._uiManager.viewParameters.realScale;
   }
@@ -398,6 +416,9 @@ class AnnotationEditor {
     this.#hasBeenSelected = true;
   }
 
+  /**
+   * Convert the current rect into a page one.
+   */
   getRect(tx, ty) {
     const scale = this.parentScale;
     const [pageWidth, pageHeight] = this.pageDimensions;

src/display/editor/freetext.js

@@ -502,8 +502,47 @@ class FreeTextEditor extends AnnotationEditor {
       // This editor was created in using copy (ctrl+c).
       const [parentWidth, parentHeight] = this.parentDimensions;
       if (this.annotationElementId) {
-        const [tx] = this.getInitialTranslation();
+        // This stuff is hard to test: if something is changed here, please
-        this.setAt(baseX * parentWidth, baseY * parentHeight, tx, tx);
+        // test with the following PDF file:
+        //  - freetexts.pdf
+        //  - rotated_freetexts.pdf
+        // Only small variations between the original annotation and its editor
+        // are allowed.
+
+        // position is the position of the first glyph in the annotation
+        // and it's relative to its container.
+        const { position } = this.#initialData;
+        let [tx, ty] = this.getInitialTranslation();
+        [tx, ty] = this.pageTranslationToScreen(tx, ty);
+        const [pageWidth, pageHeight] = this.pageDimensions;
+        const [pageX, pageY] = this.pageTranslation;
+        let posX, posY;
+        switch (this.rotation) {
+          case 0:
+            posX = baseX + (position[0] - pageX) / pageWidth;
+            posY = baseY + this.height - (position[1] - pageY) / pageHeight;
+            break;
+          case 90:
+            posX = baseX + (position[0] - pageX) / pageWidth;
+            posY = baseY - (position[1] - pageY) / pageHeight;
+            [tx, ty] = [ty, -tx];
+            break;
+          case 180:
+            posX = baseX - this.width + (position[0] - pageX) / pageWidth;
+            posY = baseY - (position[1] - pageY) / pageHeight;
+            [tx, ty] = [-tx, -ty];
+            break;
+          case 270:
+            posX =
+              baseX +
+              (position[0] - pageX - this.height * pageHeight) / pageWidth;
+            posY =
+              baseY +
+              (position[1] - pageY - this.width * pageWidth) / pageHeight;
+            [tx, ty] = [-ty, tx];
+            break;
+        }
+        this.setAt(posX * parentWidth, posY * parentHeight, tx, ty);
       } else {
         this.setAt(
           baseX * parentWidth,
@@ -521,6 +560,10 @@ class FreeTextEditor extends AnnotationEditor {
       this.editorDiv.contentEditable = true;
     }
 
+    if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) {
+      this.div.setAttribute("annotation-id", this.annotationElementId);
+    }
+
     return this.div;
   }
 
@@ -554,6 +597,7 @@ class FreeTextEditor extends AnnotationEditor {
           id,
         },
         textContent,
+        textPosition,
         parent: {
           page: { pageNumber },
         },
@@ -569,6 +613,7 @@ class FreeTextEditor extends AnnotationEditor {
         color: Array.from(fontColor),
         fontSize,
         value: textContent.join("\n"),
+        position: textPosition,
         pageIndex: pageNumber - 1,
         rect,
         rotation,

test/integration/freetext_editor_spec.js

@@ -25,6 +25,8 @@ const {
   waitForStorageEntries,
 } = require("./test_utils.js");
 
+const PNG = require("pngjs").PNG;
+
 const copyPaste = async page => {
   let promise = waitForEvent(page, "copy");
   await page.keyboard.down("Control");
@@ -1379,4 +1381,302 @@ describe("FreeText Editor", () => {
       );
     });
   });
+
+  describe("FreeText (open existing)", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait(
+        "issue16633.pdf",
+        ".annotationEditorLayer",
+        100
+      );
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must open an existing annotation and check that the position are good", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await page.click("#editorFreeText");
+
+          await page.evaluate(() => {
+            document.getElementById("editorFreeTextParamsToolbar").remove();
+          });
+
+          const toBinary = buf => {
+            for (let i = 0; i < buf.length; i += 4) {
+              const gray =
+                (0.2126 * buf[i] + 0.7152 * buf[i + 1] + 0.0722 * buf[i + 2]) /
+                255;
+              buf[i] = buf[i + 1] = buf[i + 2] = gray <= 0.5 ? 0 : 255;
+            }
+          };
+
+          // We want to detect the first non-white pixel in the image.
+          // But we can have some antialiasing...
+          // The idea to just try to detect the beginning of the vertical bar
+          // of the "H" letter.
+          // Hence we just take the first non-white pixel in the image which is
+          // the most repeated one.
+          const getFirstPixel = (buf, width, height) => {
+            toBinary(buf);
+            const firsts = [];
+            const stats = {};
+            // Get the position of the first pixels.
+            // The position of char depends on a lot of different parameters,
+            // hence it's possible to not have a pixel where we expect to have
+            // it. So we just collect the positions of the first black pixel and
+            // take the first one where its abscissa is the most frequent.
+            for (let i = height - 1; i >= 0; i--) {
+              for (let j = 0; j < width; j++) {
+                const idx = (width * i + j) << 2;
+                if (buf[idx] === 0) {
+                  firsts.push([j, i]);
+                  stats[j] = (stats[j] || 0) + 1;
+                  break;
+                }
+              }
+            }
+
+            let maxValue = -Infinity;
+            let maxJ = 0;
+            for (const [j, count] of Object.entries(stats)) {
+              if (count > maxValue) {
+                maxValue = count;
+                maxJ = j;
+              }
+            }
+            maxJ = parseInt(maxJ, 10);
+            for (const [j, i] of firsts) {
+              if (j === maxJ) {
+                return [j, i];
+              }
+            }
+            return null;
+          };
+
+          for (const n of [0, 1, 2, 3, 4]) {
+            const rect = await page.$eval(getEditorSelector(n), el => {
+              // With Chrome something is wrong when serializing a DomRect,
+              // hence we extract the values and just return them.
+              const { x, y, width, height } = el.getBoundingClientRect();
+              return { x, y, width, height };
+            });
+            const editorPng = await page.screenshot({
+              clip: rect,
+              type: "png",
+            });
+            const editorImage = PNG.sync.read(editorPng);
+            const editorFirstPix = getFirstPixel(
+              editorImage.data,
+              editorImage.width,
+              editorImage.height
+            );
+
+            await page.evaluate(N => {
+              const editor = document.getElementById(
+                `pdfjs_internal_editor_${N}`
+              );
+              const annotationId = editor.getAttribute("annotation-id");
+              const annotation = document.querySelector(
+                `[data-annotation-id="${annotationId}"]`
+              );
+              editor.hidden = true;
+              annotation.hidden = false;
+            }, n);
+            await page.waitForTimeout(10);
+            const annotationPng = await page.screenshot({
+              clip: rect,
+              type: "png",
+            });
+            const annotationImage = PNG.sync.read(annotationPng);
+            const annotationFirstPix = getFirstPixel(
+              annotationImage.data,
+              annotationImage.width,
+              annotationImage.height
+            );
+
+            expect(
+              Math.abs(editorFirstPix[0] - annotationFirstPix[0]) <= 3 &&
+                Math.abs(editorFirstPix[1] - annotationFirstPix[1]) <= 3
+            )
+              .withContext(
+                `In ${browserName}, first pix coords in editor: ${editorFirstPix} and in annotation: ${annotationFirstPix}`
+              )
+              .toEqual(true);
+          }
+        })
+      );
+    });
+  });
+
+  describe("FreeText (open existing and rotated)", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait(
+        "rotated_freetexts.pdf",
+        ".annotationEditorLayer",
+        100
+      );
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must open an existing rotated annotation and check that the position are good", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await page.click("#editorFreeText");
+
+          await page.evaluate(() => {
+            document.getElementById("editorFreeTextParamsToolbar").remove();
+          });
+
+          const toBinary = buf => {
+            for (let i = 0; i < buf.length; i += 4) {
+              const gray =
+                (0.2126 * buf[i] + 0.7152 * buf[i + 1] + 0.0722 * buf[i + 2]) /
+                255;
+              buf[i] = buf[i + 1] = buf[i + 2] = gray >= 0.5 ? 255 : 0;
+            }
+          };
+
+          const getFirstPixel = (buf, width, height, start) => {
+            toBinary(buf);
+            const firsts = [];
+            const stats = {};
+            switch (start) {
+              case "TL":
+                for (let j = 0; j < width; j++) {
+                  for (let i = 0; i < height; i++) {
+                    const idx = (width * i + j) << 2;
+                    if (buf[idx] === 0) {
+                      firsts.push([j, i]);
+                      stats[j] = (stats[j] || 0) + 1;
+                      break;
+                    }
+                  }
+                }
+                break;
+              case "TR":
+                for (let i = 0; i < height; i++) {
+                  for (let j = width - 1; j >= 0; j--) {
+                    const idx = (width * i + j) << 2;
+                    if (buf[idx] === 0) {
+                      firsts.push([j, i]);
+                      stats[j] = (stats[j] || 0) + 1;
+                      break;
+                    }
+                  }
+                }
+                break;
+              case "BR":
+                for (let j = width - 1; j >= 0; j--) {
+                  for (let i = height - 1; i >= 0; i--) {
+                    const idx = (width * i + j) << 2;
+                    if (buf[idx] === 0) {
+                      firsts.push([j, i]);
+                      stats[j] = (stats[j] || 0) + 1;
+                      break;
+                    }
+                  }
+                }
+                break;
+              case "BL":
+                for (let i = height - 1; i >= 0; i--) {
+                  for (let j = 0; j < width; j++) {
+                    const idx = (width * i + j) << 2;
+                    if (buf[idx] === 0) {
+                      firsts.push([j, i]);
+                      stats[j] = (stats[j] || 0) + 1;
+                      break;
+                    }
+                  }
+                }
+                break;
+            }
+
+            let maxValue = -Infinity;
+            let maxJ = 0;
+            for (const [j, count] of Object.entries(stats)) {
+              if (count > maxValue) {
+                maxValue = count;
+                maxJ = j;
+              }
+            }
+            maxJ = parseInt(maxJ, 10);
+            for (const [j, i] of firsts) {
+              if (j === maxJ) {
+                return [j, i];
+              }
+            }
+            return null;
+          };
+
+          for (const [n, start] of [
+            [0, "BL"],
+            [1, "BR"],
+            [2, "TR"],
+            [3, "TL"],
+          ]) {
+            const rect = await page.$eval(getEditorSelector(n), el => {
+              // With Chrome something is wrong when serializing a DomRect,
+              // hence we extract the values and just return them.
+              const { x, y, width, height } = el.getBoundingClientRect();
+              return { x, y, width, height };
+            });
+            const editorPng = await page.screenshot({
+              clip: rect,
+              type: "png",
+            });
+            const editorImage = PNG.sync.read(editorPng);
+            const editorFirstPix = getFirstPixel(
+              editorImage.data,
+              editorImage.width,
+              editorImage.height,
+              start
+            );
+
+            await page.evaluate(N => {
+              const editor = document.getElementById(
+                `pdfjs_internal_editor_${N}`
+              );
+              const annotationId = editor.getAttribute("annotation-id");
+              const annotation = document.querySelector(
+                `[data-annotation-id="${annotationId}"]`
+              );
+              editor.hidden = true;
+              annotation.hidden = false;
+            }, n);
+            await page.waitForTimeout(10);
+            const annotationPng = await page.screenshot({
+              clip: rect,
+              type: "png",
+            });
+            const annotationImage = PNG.sync.read(annotationPng);
+            const annotationFirstPix = getFirstPixel(
+              annotationImage.data,
+              annotationImage.width,
+              annotationImage.height,
+              start
+            );
+
+            expect(
+              Math.abs(editorFirstPix[0] - annotationFirstPix[0]) <= 3 &&
+                Math.abs(editorFirstPix[1] - annotationFirstPix[1]) <= 3
+            )
+              .withContext(
+                `In ${browserName}, first pix coords in editor: ${editorFirstPix} and in annotation: ${annotationFirstPix}`
+              )
+              .toEqual(true);
+          }
+        })
+      );
+    });
+  });
 });

test/integration/test_utils.js

@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-exports.loadAndWait = (filename, selector) =>
+exports.loadAndWait = (filename, selector, zoom) =>
   Promise.all(
     global.integrationSessions.map(async session => {
       const page = await session.browser.newPage();
@@ -33,9 +33,11 @@ exports.loadAndWait = (filename, selector) =>
         });
       });
 
-      await page.goto(
+      let url = `${global.integrationBaseUrl}?file=/test/pdfs/${filename}`;
-        `${global.integrationBaseUrl}?file=/test/pdfs/${filename}`
+      if (zoom) {
-      );
+        url += `#zoom=${zoom}`;
+      }
+      await page.goto(url);
       await page.bringToFront();
       await page.waitForSelector(selector, {
         timeout: 0,

test/pdfs/.gitignore

@@ -602,3 +602,5 @@
 !freetexts.pdf
 !issue16553.pdf
 !empty.pdf
+!rotated_freetexts.pdf
+!issue16633.pdf