PDF: text colour, strongly-typed render mode, image clip fix (#571)

* font: drop out-of-range glyph ids when re-encoding to PUA

`glyph_for_code` can fall back to "code as GID" (ISO 32000-1 9.6.6.4) and
yield an index past `numGlyphs`. A single cmap entry referencing such a
glyph makes the OTS sanitizer reject the *entire* font, so every glyph
renders as a tofu box rather than just the unmappable code. Skip those
entries in both the CFF (`wrap_to_otf`) and SFNT (`reencode_to_pua`)
paths, with tests covering the drop.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* PDF: text colour, strongly-typed render mode, image clip fix

- Carry the non-stroking (fill) and stroking device colours on each
  TextElement and paint runs in their colour (interned `.k` class) when
  it is not the default black — fill modes use the fill colour, the
  stroke-only modes (Tr 1/5) the stroking colour. The colour rides the
  visible layer in the dual-layer (PUA glyph) cases.
- Replace the bare `int rendering_mode` with a strongly-typed
  `TextRenderingMode` enum (ISO 32000-1 Table 106) across the graphics
  state, page element and renderer, removing the magic 0/1/3/5/7
  comparisons.
- Wrap clipped `<image>` elements in a transform-free `<g clip-path>`:
  a clip-path on the image itself resolves in the image's post-transform
  unit-square space and clips the whole image away.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: andiwand

src/odr/internal/font/cff_transform.cpp

@@ -153,9 +153,15 @@ std::string cff::wrap_to_otf(const CffFont &font,
     pua[pua_code_point(glyph)] = glyph;
   }
   // Real-Unicode entries: caller guarantees BMP, non-PUA keys, so these never
-  // collide with the PUA range filled above.
+  // collide with the PUA range filled above. A glyph id the font does not have
+  // is dropped: `glyph_for_code` can fall back to "code as GID" (ISO 32000-1
+  // 9.6.6.4) and yield an out-of-range index, and a single cmap reference past
+  // `numGlyphs` makes the OTS sanitizer reject the *entire* font (so every
+  // glyph would render as a tofu box, not just the unmappable code).
   for (const auto &[code, glyph] : extra) {
-    pua[code] = glyph;
+    if (glyph < glyphs) {
+      pua[code] = glyph;
+    }
   }
 
   std::uint16_t advance_width_max = 0;

src/odr/internal/font/sfnt_transform.cpp

@@ -282,9 +282,15 @@ void font::reencode_to_pua(sfnt::SfntFont &font,
     map[pua_code_point(glyph)] = glyph;
   }
   // Real-Unicode entries: caller guarantees BMP, non-PUA keys, so these never
-  // collide with the PUA range filled above.
+  // collide with the PUA range filled above. A glyph id the font does not have
+  // is dropped: `glyph_for_code` can fall back to "code as GID" (ISO 32000-1
+  // 9.6.6.4) and yield an out-of-range index, and a single cmap reference past
+  // `numGlyphs` makes the OTS sanitizer reject the *entire* font (so every
+  // glyph would render as a tofu box, not just the unmappable code).
   for (const auto &[code, glyph] : extra) {
-    map[code] = glyph;
+    if (glyph < font.glyph_count()) {
+      map[code] = glyph;
+    }
   }
   font.set_cmap(std::move(map));
 }

src/odr/internal/html/pdf_file.cpp

@@ -181,7 +181,12 @@ std::string svg_path_fragment(const pdf::PathElement &path,
 /// or "" when it carries no pass-through bytes. The image fills the unit square
 /// in user space (ISO 32000-1 8.10.5); the transform maps that square — through
 /// a vertical flip (the image's first row is its top, SVG draws y-down) and the
-/// CTM — into the page box. `clip_id` installs a clip via `clip-path`.
+/// CTM — into the page box. `clip_id`, when non-empty, installs a clip via a
+/// wrapping `<g clip-path>`. The clip geometry is in the page viewBox
+/// (`userSpaceOnUse`), but the `<image>` carries its own `transform`, so a
+/// `clip-path` placed *on the image* would be resolved in the image's
+/// post-transform unit-square space and clip the whole image away. The `<g>`
+/// carries no transform, so the clip is read in the viewBox where it lives.
 std::string svg_image_fragment(const pdf::ImageElement &image,
                                const util::math::Transform2D &to_box,
                                const std::string &clip_id) {
@@ -194,13 +199,18 @@ std::string svg_image_fragment(const pdf::ImageElement &image,
   const util::math::Transform2D m = flip * image.transform * to_box;
 
   std::ostringstream f;
+  // The clip wraps the image in a transform-free `<g>` rather than sitting on
+  // the `<image>`: see the function comment.
+  if (!clip_id.empty()) {
+    f << "<g clip-path=\"url(#" << clip_id << ")\">";
+  }
   f << R"(<image width="1" height="1" preserveAspectRatio="none" transform="matrix()"
     << m.a << ',' << m.b << ',' << m.c << ',' << m.d << ',' << round2(m.e)
     << ',' << round2(m.f) << ")\"";
+  f << " href=\"" << file_to_url(image.data, image.mime) << "\"/>";
   if (!clip_id.empty()) {
-    f << " clip-path=\"url(#" << clip_id << ")\"";
+    f << "</g>";
   }
-  f << " href=\"" << file_to_url(image.data, image.mime) << "\"/>";
   return std::move(f).str();
 }
 
@@ -621,7 +631,29 @@ class HtmlServiceImpl final : public HtmlService {
         // Tr 3 (invisible) and Tr 7 (clip-only) paint nothing; keep them
         // selectable via the transparent `.i` class.
         const bool invisible =
-            text.rendering_mode == 3 || text.rendering_mode == 7;
+            text.rendering_mode == pdf::TextRenderingMode::invisible ||
+            text.rendering_mode == pdf::TextRenderingMode::clip;
+
+        // The run's visible paint colour, folded onto the visible span as an
+        // interned colour class — but only when it is not the default black, so
+        // the overwhelmingly common black run adds nothing. The per-font
+        // `.fvN`/`.gvN` classes declare `color:#000`; this class is emitted
+        // after them in <head> (equal specificity), so it overrides. Invisible
+        // runs (Tr 3/7) stay transparent via `.i`, so they take no colour
+        // class. The fill modes paint with the non-stroking colour, the
+        // stroke-only modes (Tr 1/5) with the stroking colour.
+        std::string color_suffix;
+        if (!invisible) {
+          const pdf::GraphicsState::Color &paint =
+              (text.rendering_mode == pdf::TextRenderingMode::stroke ||
+               text.rendering_mode == pdf::TextRenderingMode::stroke_clip)
+                  ? text.stroke_color
+                  : text.fill_color;
+          if (std::string css = device_color_to_css(paint);
+              css != "rgb(0,0,0)") {
+            color_suffix = ' ' + styles.intern("k", "color:" + std::move(css));
+          }
+        }
 
         // Placement and spacing are shared by both layers of a run; build them
         // once on `base`.
@@ -740,17 +772,18 @@ class HtmlServiceImpl final : public HtmlService {
             std::string classes = std::move(base);
             classes += ' ';
             classes += font_class(font, invisible, /*nested=*/false);
+            classes += color_suffix;
             page_out.items.push_back(
                 SpanOut{std::move(classes), escape_text(text.text), {}, {}});
           } else {
             // Dual layer (a glyph lost its scalar to an earlier one): a
             // transparent selectable Unicode span with the PUA glyph layer
             // nested inside, the latter folded into the combined `.gvN` /
-            // `.giN` class.
-            page_out.items.push_back(
-                SpanOut{base + " i", escape_text(text.text),
-                        font_class(font, invisible, /*nested=*/true),
-                        escape_text(glyph_run(*text.font, text.codes))});
+            // `.giN` class. The colour rides the visible (nested) layer.
+            page_out.items.push_back(SpanOut{
+                base + " i", escape_text(text.text),
+                font_class(font, invisible, /*nested=*/true) + color_suffix,
+                escape_text(glyph_run(*text.font, text.codes))});
           }
         } else if (font != 0) {
           // The visible glyph layer: PUA code points in the embedded font,
@@ -763,16 +796,17 @@ class HtmlServiceImpl final : public HtmlService {
             // Unicode (for copy/search) with the glyph layer nested inside.
             // The nested child overlays the run origin and inherits the
             // placement via the combined `.gvN` / `.giN` class.
-            page_out.items.push_back(
-                SpanOut{base + " i", escape_text(text.text),
-                        font_class(font, invisible, /*nested=*/true),
-                        std::move(glyph_text)});
+            page_out.items.push_back(SpanOut{
+                base + " i", escape_text(text.text),
+                font_class(font, invisible, /*nested=*/true) + color_suffix,
+                std::move(glyph_text)});
           } else {
             // Display-only run: nothing is extractable (the `no_unicode` case),
             // so the glyph layer stands alone and carries the placement itself
             // (`base`), `.g` (unselectable) and the combined paint+font class.
             std::string glyph_classes = base + " g ";
             glyph_classes += font_class(font, invisible, /*nested=*/false);
+            glyph_classes += color_suffix;
             page_out.items.push_back(SpanOut{
                 std::move(glyph_classes), std::move(glyph_text), {}, {}});
           }
@@ -783,6 +817,7 @@ class HtmlServiceImpl final : public HtmlService {
           if (invisible) {
             classes += " i";
           }
+          classes += color_suffix;
           page_out.items.push_back(
               SpanOut{std::move(classes), escape_text(text.text), {}, {}});
         }

src/odr/internal/pdf/pdf_graphics_state.cpp

@@ -250,7 +250,7 @@ void GraphicsState::execute(const GraphicsOperator &op) {
     break;
   case GraphicsOperatorType::set_text_rendering_mode:
     current().text.rendering_mode =
-        static_cast<int>(op.arguments.at(0).as_integer());
+        static_cast<TextRenderingMode>(op.arguments.at(0).as_integer());
     break;
   case GraphicsOperatorType::set_text_rise:
     current().text.rise = op.arguments.at(0).as_real();

src/odr/internal/pdf/pdf_graphics_state.hpp

@@ -18,6 +18,21 @@ enum class ColorSpace {
   device_cmyk,
 };
 
+/// Text rendering mode (`Tr`, ISO 32000-1 Table 106): how shown glyphs are
+/// painted and/or added to the clipping path. The low two bits select the paint
+/// (fill / stroke / both / none); the `_clip` modes additionally add the glyph
+/// outlines to the clip. The integer values are the `Tr` operands.
+enum class TextRenderingMode {
+  fill = 0,
+  stroke = 1,
+  fill_stroke = 2,
+  invisible = 3,
+  fill_clip = 4,
+  stroke_clip = 5,
+  fill_stroke_clip = 6,
+  clip = 7,
+};
+
 /// One segment of a subpath, in user space (the CTM is already applied at
 /// construction time, ISO 32000-1 8.5.2.1). A line carries only `end`; a cubic
 /// Bézier carries its two control points as well.
@@ -72,16 +87,16 @@ struct GraphicsState {
   };
 
   struct Text {
-    double char_spacing{0};              // Tc
-    double word_spacing{0};              // Tw
-    double horizontal_scaling{100};      // Tz, in percent (100 = normal)
-    double leading{0};                   // TL
-    std::string font;                    // Tf resource name
-    double size{};                       // Tf size
-    int rendering_mode{0};               // Tr
-    double rise{0};                      // Ts
-    util::math::Transform2D matrix;      // Tm
-    util::math::Transform2D line_matrix; // Tlm
+    double char_spacing{0};         // Tc
+    double word_spacing{0};         // Tw
+    double horizontal_scaling{100}; // Tz, in percent (100 = normal)
+    double leading{0};              // TL
+    std::string font;               // Tf resource name
+    double size{};                  // Tf size
+    TextRenderingMode rendering_mode{TextRenderingMode::fill}; // Tr
+    double rise{0};                                            // Ts
+    util::math::Transform2D matrix;                            // Tm
+    util::math::Transform2D line_matrix;                       // Tlm
     std::array<double, 2> glyph_width{};
     std::array<double, 4> glyph_bounding_box{};
   };

src/odr/internal/pdf/pdf_page_element.hpp

@@ -21,12 +21,18 @@ struct TextElement {
   util::math::Transform2D transform;
   /// Resolved font, or `nullptr` when the `/Font` resource name was unknown.
   Font *font{nullptr};
-  double size{0};                 // Tf size
-  double char_spacing{0};         // Tc
-  double word_spacing{0};         // Tw
-  double horizontal_scaling{100}; // Tz, percent
-  double rise{0};                 // Ts
-  int rendering_mode{0};          // Tr
+  double size{0};                                            // Tf size
+  double char_spacing{0};                                    // Tc
+  double word_spacing{0};                                    // Tw
+  double horizontal_scaling{100};                            // Tz, percent
+  double rise{0};                                            // Ts
+  TextRenderingMode rendering_mode{TextRenderingMode::fill}; // Tr
+  /// Non-stroking (fill) and stroking colours in force when the run was shown,
+  /// as device colours. The renderer paints the run in whichever its rendering
+  /// mode selects — the fill colour for the common fill modes, the stroking
+  /// colour for the stroke-only modes (Tr 1/5) — defaulting to black.
+  GraphicsState::Color fill_color;
+  GraphicsState::Color stroke_color;
   /// Raw character codes shown by this segment (one `Tj`, or one string of a
   /// `TJ` array).
   std::string codes;

src/odr/internal/pdf/pdf_page_extractor.cpp

@@ -270,6 +270,8 @@ void show(std::vector<PageElement> &out, GraphicsState &state,
   element.horizontal_scaling = text.horizontal_scaling;
   element.rise = text.rise;
   element.rendering_mode = text.rendering_mode;
+  element.fill_color = state.current().other_color;
+  element.stroke_color = state.current().stroke_color;
   ResolvedText resolved = resolve_text(marked, font, codes);
   element.text = std::move(resolved.text);
   element.no_unicode = resolved.no_unicode;
@@ -409,7 +411,11 @@ void paint_path(std::vector<PageElement> &out, GraphicsState &state, bool fill,
     element.dash_array.push_back(on_off * scale);
   }
   element.dash_phase = s.general.dash.phase * scale;
-  out.push_back(std::move(element));
+  // Construct the variant alternative in place: building a temporary variant
+  // and move-constructing it (`push_back(std::move(element))`) trips a spurious
+  // GCC 14 `-Wmaybe-uninitialized` on the string-bearing `ImageElement`
+  // alternative while analyzing the union move, which `-Werror` makes fatal.
+  out.emplace_back(std::in_place_type<PathElement>, std::move(element));
   // Install a pending `W`/`W*` now (it uses the just-painted path) so it scopes
   // the *following* content, then drop the path.
   state.commit_clip();

test/data/reference-output/odr-private

@@ -441,3 +441,18 @@ TEST(CffFontTest, WrapsToLoadableOtf) {
   // The synthesized hmtx carries the CFF advance widths.
   EXPECT_EQ(wrapped.advance_width(1), 250);
 }
+
+TEST(CffFontTest, WrapDropsExtraEntriesPastGlyphCount) {
+  using namespace odr::internal::font;
+  const CffFont cff{build_cff()}; // 2 glyphs: valid ids 0..1
+  // 'A' -> 1 is in range; 'B' -> 5 is past the glyph count. An out-of-range
+  // glyph reference makes the OTS sanitizer reject the whole cmap (and the
+  // font), so it must be dropped rather than written into the cmap.
+  const std::string otf = cff::wrap_to_otf(cff, {{U'A', 1}, {U'B', 5}});
+
+  ASSERT_TRUE(sfnt::SfntFont::is_sfnt(otf));
+  const sfnt::SfntFont wrapped{std::make_unique<std::istringstream>(otf)};
+  EXPECT_EQ(wrapped.glyph_for_code_point('A'), 1);
+  EXPECT_EQ(wrapped.glyph_for_code_point('B'), 0); // dropped: not mapped
+  EXPECT_EQ(wrapped.glyph_for_code_point(pua_code_point(1)), 1);
+}