Merge pull request #12759 from calixteman/font_size

Annotation -- Don't compute appearance when nothing has changed

src/core/annotation.js

@@ -1268,18 +1268,25 @@ class WidgetAnnotation extends Annotation {
     if (!annotationStorage || isPassword) {
       return null;
     }
-    const value =
+    let value =
       annotationStorage[this.data.id] && annotationStorage[this.data.id].value;
     if (value === undefined) {
       // The annotation hasn't been rendered so use the appearance
       return null;
     }
 
+    value = value.trim();
+
     if (value === "") {
       // the field is empty: nothing to render
       return "";
     }
 
+    let lineCount = -1;
+    if (this.data.multiLine) {
+      lineCount = value.split(/\r\n|\r|\n/).length;
+    }
+
     const defaultPadding = 2;
     const hPadding = defaultPadding;
     const totalHeight = this.data.rect[3] - this.data.rect[1];
@@ -1297,8 +1304,12 @@ class WidgetAnnotation extends Annotation {
       );
     }
 
+    const [defaultAppearance, fontSize] = this._computeFontSize(
+      totalHeight,
+      lineCount
+    );
+
     const font = await this._getFontData(evaluator, task);
-    const fontSize = this._computeFontSize(font, totalHeight);
 
     let descent = font.descent;
     if (isNaN(descent)) {
@@ -1306,7 +1317,6 @@ class WidgetAnnotation extends Annotation {
     }
 
     const vPadding = defaultPadding + Math.abs(descent) * fontSize;
-    const defaultAppearance = this.data.defaultAppearance;
     const alignment = this.data.textAlignment;
 
     if (this.data.multiLine) {
@@ -1389,35 +1399,44 @@ class WidgetAnnotation extends Annotation {
     return initialState.font;
   }
 
-  _computeFontSize(font, height) {
+  _computeFontSize(height, lineCount) {
-    let fontSize = this.data.defaultAppearanceData.fontSize;
+    let { fontSize } = this.data.defaultAppearanceData;
-    if (!fontSize) {
+    if (fontSize === null || fontSize === 0) {
-      const { fontColor, fontName } = this.data.defaultAppearanceData;
+      // A zero value for size means that the font shall be auto-sized:
-      let capHeight;
+      // its size shall be computed as a function of the height of the
-      if (font.capHeight) {
+      // annotation rectangle (see 12.7.3.3).
-        capHeight = font.capHeight;
+
+      const roundWithOneDigit = x => Math.round(x * 10) / 10;
+
+      // Represent the percentage of the font size over the height
+      // of a single-line field.
+      const FONT_FACTOR = 0.8;
+      if (lineCount === -1) {
+        fontSize = roundWithOneDigit(FONT_FACTOR * height);
       } else {
-        const glyphs = font.charsToGlyphs(font.encodeString("M").join(""));
+        // Hard to guess how many lines there are.
-        if (glyphs.length === 1 && glyphs[0].width) {
+        // The field may have been sized to have 10 lines
-          const em = glyphs[0].width / 1000;
+        // and the user entered only 1 so if we get font size from
-          // According to https://en.wikipedia.org/wiki/Em_(typography)
+        // height and number of lines then we'll get something too big.
-          // an average cap height should be 70% of 1em
+        // So we compute a fake number of lines based on height and
-          capHeight = 0.7 * em;
+        // a font size equal to 10.
-        } else {
+        // Then we'll adjust font size to what we have really.
-          capHeight = 0.7;
+        fontSize = 10;
-        }
+        let lineHeight = fontSize / FONT_FACTOR;
+        let numberOfLines = Math.round(height / lineHeight);
+        numberOfLines = Math.max(numberOfLines, lineCount);
+        lineHeight = height / numberOfLines;
+        fontSize = roundWithOneDigit(FONT_FACTOR * lineHeight);
       }
 
-      // 1.5 * capHeight * fontSize seems to be a good value for lineHeight
+      const { fontName, fontColor } = this.data.defaultAppearanceData;
-      fontSize = Math.max(1, Math.floor(height / (1.5 * capHeight)));
-
       this.data.defaultAppearance = createDefaultAppearance({
         fontSize,
         fontName,
         fontColor,
       });
     }
-    return fontSize;
+    return [this.data.defaultAppearance, fontSize];
   }
 
   _renderText(text, font, fontSize, totalWidth, alignment, hPadding, vPadding) {

src/display/annotation_layer.js

@@ -603,7 +603,7 @@ class TextWidgetAnnotationElement extends WidgetAnnotationElement {
       // NOTE: We cannot set the values using `element.value` below, since it
       //       prevents the AnnotationLayer rasterizer in `test/driver.js`
       //       from parsing the elements correctly for the reference tests.
-      const textContent = storage.getOrCreateValue(id, {
+      const textContent = storage.getValue(id, {
         value: this.data.fieldValue,
       }).value;
       const elementData = {
@@ -873,7 +873,7 @@ class CheckboxWidgetAnnotationElement extends WidgetAnnotationElement {
     const storage = this.annotationStorage;
     const data = this.data;
     const id = data.id;
-    const value = storage.getOrCreateValue(id, {
+    const value = storage.getValue(id, {
       value:
         data.fieldValue &&
         ((data.exportValue && data.exportValue === data.fieldValue) ||
@@ -962,7 +962,7 @@ class RadioButtonWidgetAnnotationElement extends WidgetAnnotationElement {
     const storage = this.annotationStorage;
     const data = this.data;
     const id = data.id;
-    const value = storage.getOrCreateValue(id, {
+    const value = storage.getValue(id, {
       value: data.fieldValue === data.buttonValue,
     }).value;
 
@@ -1072,9 +1072,9 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
     // are not properly printed/saved yet, so we only store the first item in
     // the field value array instead of the entire array. Once support for those
     // two field types is implemented, we should use the same pattern as the
-    // other interactive widgets where the return value of `getOrCreateValue` is
+    // other interactive widgets where the return value of `getValue`
-    // used and the full array of field values is stored.
+    // is used and the full array of field values is stored.
-    storage.getOrCreateValue(id, {
+    storage.getValue(id, {
       value:
         this.data.fieldValue.length > 0 ? this.data.fieldValue[0] : undefined,
     });

src/display/annotation_storage.js

@@ -13,6 +13,7 @@
  * limitations under the License.
  */
 
+import { deprecated } from "./display_utils.js";
 import { objectFromEntries } from "../shared/util.js";
 
 /**
@@ -33,7 +34,7 @@ class AnnotationStorage {
 
   /**
    * Get the value for a given key if it exists
-   * or store and return the default value
+   * or return the default value
    *
    * @public
    * @memberof AnnotationStorage
@@ -41,7 +42,19 @@ class AnnotationStorage {
    * @param {Object} defaultValue
    * @returns {Object}
    */
+  getValue(key, defaultValue) {
+    if (this._storage.has(key)) {
+      return this._storage.get(key);
+    }
+
+    return defaultValue;
+  }
+
+  /**
+   * @deprecated
+   */
   getOrCreateValue(key, defaultValue) {
+    deprecated("Use getValue instead.");
     if (this._storage.has(key)) {
       return this._storage.get(key);
     }

test/unit/annotation_spec.js

@@ -1808,7 +1808,7 @@ describe("annotation", function () {
         }, done.fail)
         .then(appearance => {
           expect(appearance).toEqual(
-            "/Tx BMC q BT /Helv 11 Tf 0 g 1 0 0 1 0 0 Tm" +
+            "/Tx BMC q BT /Helv 8 Tf 0 g 1 0 0 1 0 0 Tm" +
               " 2.00 2.00 Td (test \\(print\\)) Tj ET Q EMC"
           );
           done();
@@ -1848,7 +1848,7 @@ describe("annotation", function () {
             "\x30\x53\x30\x93\x30\x6b\x30\x61" +
             "\x30\x6f\x4e\x16\x75\x4c\x30\x6e";
           expect(appearance).toEqual(
-            "/Tx BMC q BT /Goth 9 Tf 0 g 1 0 0 1 0 0 Tm" +
+            "/Tx BMC q BT /Goth 8 Tf 0 g 1 0 0 1 0 0 Tm" +
               ` 2.00 2.00 Td (${utf16String}) Tj ET Q EMC`
           );
           done();

test/unit/annotation_storage_spec.js

@@ -16,17 +16,21 @@
 import { AnnotationStorage } from "../../src/display/annotation_storage.js";
 
 describe("AnnotationStorage", function () {
-  describe("GetOrCreateValue", function () {
+  describe("GetOrDefaultValue", function () {
     it("should get and set a new value in the annotation storage", function (done) {
       const annotationStorage = new AnnotationStorage();
-      let value = annotationStorage.getOrCreateValue("123A", {
+      let value = annotationStorage.getValue("123A", {
         value: "hello world",
       }).value;
       expect(value).toEqual("hello world");
 
+      annotationStorage.setValue("123A", {
+        value: "hello world",
+      });
+
       // the second argument is the default value to use
       // if the key isn't in the storage
-      value = annotationStorage.getOrCreateValue("123A", {
+      value = annotationStorage.getValue("123A", {
         value: "an other string",
       }).value;
       expect(value).toEqual("hello world");
@@ -50,16 +54,18 @@ describe("AnnotationStorage", function () {
         called = true;
       };
       annotationStorage.onSetModified = callback;
-      annotationStorage.getOrCreateValue("asdf", { value: "original" });
-      expect(called).toBe(false);
 
-      // not changing value
       annotationStorage.setValue("asdf", { value: "original" });
-      expect(called).toBe(false);
+      expect(called).toBe(true);
 
       // changing value
       annotationStorage.setValue("asdf", { value: "modified" });
       expect(called).toBe(true);
+
+      // not changing value
+      called = false;
+      annotationStorage.setValue("asdf", { value: "modified" });
+      expect(called).toBe(false);
       done();
     });
   });
@@ -72,7 +78,10 @@ describe("AnnotationStorage", function () {
         called = true;
       };
       annotationStorage.onResetModified = callback;
-      annotationStorage.getOrCreateValue("asdf", { value: "original" });
+      annotationStorage.setValue("asdf", { value: "original" });
+      annotationStorage.resetModified();
+      expect(called).toBe(true);
+      called = false;
 
       // not changing value
       annotationStorage.setValue("asdf", { value: "original" });