fix(html): the underline and the squiggle sit under the text

CHANGELOG.md

@@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- **Fix**: a pdf underline and squiggle were drawn through the text rather than
+  under it, both in the view and in the saved file. They now hang below the
+  glyphs. A strike-out still crosses the text.
+
 - **Fix**: a pdf mark cut the tails off the text it marked and broke apart at
   every word gap, because it took the selection layer's em box. It now covers
   the glyphs under the selection, as one box per line.

src/odr/internal/html/frontend/pdf-annotation.js

@@ -148,17 +148,19 @@
       if (type === "highlight") {
         return rect(b[0], b[1], b[2] - b[0], h);
       }
+      // `b[3]` is the bottom of the glyph boxes, so an underline and a
+      // squiggle hang below it. A strike-out is the one meant to cross.
       if (type === "underline") {
-        return rect(b[0], b[3] - h / 16, b[2] - b[0], Math.max(h / 16, 0.5));
+        return rect(b[0], b[3], b[2] - b[0], Math.max(h / 16, 0.5));
       }
       if (type === "strikeOut") {
         return rect(b[0], b[1] + h / 2, b[2] - b[0], Math.max(h / 16, 0.5));
       }
       var step = Math.max(h / 8, 1);
-      var d = "M " + b[0] + " " + (b[3] - step);
+      var d = "M " + b[0] + " " + b[3];
       var up = true;
       for (var x = b[0] + step; x < b[2]; x += step, up = !up) {
-        d += " L " + x + " " + (up ? b[3] - step * 2 : b[3] - step);
+        d += " L " + x + " " + (up ? b[3] + step : b[3]);
       }
       return d;
     };

src/odr/internal/pdf/pdf_annotation.cpp

@@ -145,16 +145,18 @@ void bar(std::ostringstream &out, const QuadCorners &quad, const double bottom,
       << number(quad.right - quad.left) << ' ' << number(height) << " re\n";
 }
 
-/// A wave along the bottom of `quad`, as a stroked zigzag of `amplitude`.
+/// A wave under `quad`, as a stroked zigzag of `amplitude` hanging off its
+/// bottom edge: `quad.bottom` is where the glyphs end, so the wave belongs
+/// below it rather than through the descenders.
 void wave(std::ostringstream &out, const QuadCorners &quad,
           const double amplitude) {
-  const double base = quad.bottom + amplitude;
-  out << number(quad.left) << ' ' << number(base) << " m\n";
+  const double base = quad.bottom - amplitude;
+  out << number(quad.left) << ' ' << number(quad.bottom) << " m\n";
   const auto steps = static_cast<std::size_t>(
       std::max(1.0, std::floor((quad.right - quad.left) / amplitude)));
   for (std::size_t i = 1; i <= steps; ++i) {
     out << number(quad.left + static_cast<double>(i) * amplitude) << ' '
-        << number(i % 2 == 1 ? base + amplitude : base) << " l\n";
+        << number(i % 2 == 1 ? base : quad.bottom) << " l\n";
   }
   out << "S\n";
 }
@@ -176,7 +178,9 @@ std::string text_markup_appearance(const TextMarkup &markup) {
       bar(out, c, c.bottom, height);
       break;
     case TextMarkupKind::underline:
-      bar(out, c, c.bottom + height / 16, std::max(height / 16, 0.5));
+      // Under the glyphs, not through them: `c.bottom` is where they end.
+      bar(out, c, c.bottom - std::max(height / 16, 0.5),
+          std::max(height / 16, 0.5));
       break;
     case TextMarkupKind::strike_out:
       bar(out, c, c.bottom + height / 2, std::max(height / 16, 0.5));

test/browser/annotation/tests.html

@@ -513,6 +513,60 @@
       line.boxes[0]
     );
 
+    // --- what is drawn under the text stays under it ----------------------
+
+    // Every y the path names, in the page-box points the box is stated in.
+    function pathRows(id) {
+      const d = document
+        .querySelector('[data-odr-annotation="' + id + '"]')
+        .getAttribute("d");
+      const rows = [];
+      let y = 0;
+      d.replace(/([MLhv]) *(-?[\d.]+)(?: +(-?[\d.]+))?/g, (all, op, a, b) => {
+        if (op === "M" || op === "L") {
+          y = +b;
+          rows.push(y);
+        } else if (op === "v") {
+          y += +a;
+          rows.push(y);
+        }
+        return all;
+      });
+      return rows;
+    }
+
+    function markDual(type) {
+      api.clear();
+      api.setTool(type);
+      window.getSelection().removeAllRanges();
+      window.getSelection().addRange(both.cloneRange());
+      api.mark();
+      return api.list()[0];
+    }
+
+    const under = markDual("underline");
+    check(
+      "an underline starts where the glyphs end",
+      Math.min.apply(null, pathRows(under.id)) >= under.boxes[0][3] - 0.01,
+      [Math.min.apply(null, pathRows(under.id)), under.boxes[0][3]]
+    );
+
+    const wave = markDual("squiggly");
+    check(
+      "and so does a squiggle",
+      Math.min.apply(null, pathRows(wave.id)) >= wave.boxes[0][3] - 0.01,
+      [Math.min.apply(null, pathRows(wave.id)), wave.boxes[0][3]]
+    );
+
+    const struck = markDual("strikeOut");
+    const strikeRows = pathRows(struck.id);
+    check(
+      "a strike-out is the one that crosses the text",
+      Math.min.apply(null, strikeRows) > struck.boxes[0][1] &&
+        Math.max.apply(null, strikeRows) < struck.boxes[0][3],
+      [strikeRows, struck.boxes[0]]
+    );
+
     window.odr.onAnnotationChange = () => {};
     api.clear();
     api.setTool(null);

test/src/internal/pdf/pdf_annotation.cpp

@@ -10,6 +10,8 @@
 
 #include <internal/pdf/pdf_test_file_builder.hpp>
 
+#include <algorithm>
+#include <iterator>
 #include <memory>
 #include <sstream>
 #include <string>
@@ -146,6 +148,54 @@ TEST(PdfAnnotation, only_highlight_multiplies) {
   }
 }
 
+// The quad is the text, so an underline and a squiggle hang below it and only
+// a strike-out crosses it. `pdf-annotation.js` draws the same three shapes for
+// the view, and the two have to agree.
+TEST(PdfAnnotation, an_underline_and_a_squiggle_stay_under_the_quad) {
+  // Every y the appearance stream names, in pdf user space.
+  const auto rows = [](const TextMarkupKind kind) {
+    TextMarkup markup = one_line_highlight();
+    markup.kind = kind;
+    const std::string pdf = with_markup(markup);
+    DocumentParser parser(std::make_unique<std::istringstream>(pdf));
+    const std::unique_ptr<Document> document = parser.parse_document();
+    const Dictionary &dictionary =
+        first_page(*document)->annotations.front()->object.as_dictionary();
+    const std::string stream = parser.read_object_stream(
+        dictionary.get("AP").as_dictionary().get("N").as_reference());
+    std::vector<double> out;
+    std::istringstream in(stream);
+    std::vector<std::string> words{std::istream_iterator<std::string>(in),
+                                   std::istream_iterator<std::string>()};
+    for (std::size_t i = 0; i < words.size(); ++i) {
+      if (words[i] == "l" || words[i] == "m") {
+        out.push_back(std::stod(words[i - 1]));
+      } else if (words[i] == "re") {
+        out.push_back(std::stod(words[i - 3]));
+        out.push_back(std::stod(words[i - 3]) + std::stod(words[i - 1]));
+      }
+    }
+    return out;
+  };
+
+  // `one_line_highlight` states a quad from y=688 to y=700.
+  constexpr double bottom = 688;
+  constexpr double top = 700;
+
+  for (const TextMarkupKind kind :
+       {TextMarkupKind::underline, TextMarkupKind::squiggly}) {
+    const std::vector<double> y = rows(kind);
+    ASSERT_FALSE(y.empty());
+    EXPECT_LE(*std::max_element(y.begin(), y.end()), bottom)
+        << "kind " << static_cast<int>(kind);
+  }
+
+  const std::vector<double> struck = rows(TextMarkupKind::strike_out);
+  ASSERT_FALSE(struck.empty());
+  EXPECT_GT(*std::min_element(struck.begin(), struck.end()), bottom);
+  EXPECT_LT(*std::max_element(struck.begin(), struck.end()), top);
+}
+
 TEST(PdfAnnotation, text_markup_subtypes) {
   const auto subtype = [](const TextMarkupKind kind) {
     TextMarkup markup = one_line_highlight();