From c7e737596c138a34c5943418ac6950028cc3d5f0 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 3 Jul 2026 22:38:18 +0200 Subject: [PATCH] font: return std::string from serializers, drop istream inputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every caller of SfntFont::write / build_sfnt / cff::wrap_to_otf immediately funneled the ostream through an ostringstream to recover the bytes, and build_sfnt already assembled the whole file in memory before the single write — so the ostream bought nothing. Return std::string directly and drop the boilerplate (and one buffer copy) at every call site. Symmetrically, now that construction takes std::string, remove the unique_ptr constructors: CffFont's was dead, and SfntFont's two callers can read the file stream into a string themselves via util::stream::read. Prune the now-unused stream includes throughout. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01GG4WpNTKKR2uk5dBsqyAdG --- src/odr/internal/font/cff_font.cpp | 11 -------- src/odr/internal/font/cff_font.hpp | 4 --- src/odr/internal/font/cff_transform.cpp | 5 +--- src/odr/internal/font/font_file.cpp | 6 ++++- src/odr/internal/font/sfnt_font.cpp | 16 ++---------- src/odr/internal/font/sfnt_font.hpp | 14 +++------- src/odr/internal/font/sfnt_transform.cpp | 15 ++++++----- src/odr/internal/font/sfnt_transform.hpp | 17 +++++++------ src/odr/internal/html/font_file.cpp | 10 ++++---- src/odr/internal/html/pdf_file.cpp | 7 ++--- test/src/internal/font/cff_font.cpp | 6 ++--- test/src/internal/font/font_file.cpp | 4 +-- test/src/internal/font/sfnt_transform.cpp | 31 ++++++++--------------- test/src/internal/pdf/pdf_font.cpp | 4 +-- 14 files changed, 51 insertions(+), 99 deletions(-) diff --git a/src/odr/internal/font/cff_font.cpp b/src/odr/internal/font/cff_font.cpp index 022cccc0..e890df3e 100644 --- a/src/odr/internal/font/cff_font.cpp +++ b/src/odr/internal/font/cff_font.cpp @@ -3,14 +3,11 @@ #include #include #include -#include #include #include -#include #include #include -#include #include #include #include @@ -217,14 +214,6 @@ bool CffFont::is_cff(const std::string_view data) { static_cast(data[3]) <= 4; } -CffFont::CffFont(std::unique_ptr stream) { - if (stream == nullptr) { - throw std::invalid_argument("cff: null input stream"); - } - m_data = util::stream::read(*stream); - parse(); -} - CffFont::CffFont(std::string data) : m_data{std::move(data)} { parse(); } std::vector CffFont::read_index(const std::uint32_t offset, diff --git a/src/odr/internal/font/cff_font.hpp b/src/odr/internal/font/cff_font.hpp index f9e2fda2..53249f07 100644 --- a/src/odr/internal/font/cff_font.hpp +++ b/src/odr/internal/font/cff_font.hpp @@ -3,8 +3,6 @@ #include #include -#include -#include #include #include #include @@ -29,8 +27,6 @@ class CffFont final : public abstract::Font { /// Cheap magic test: a CFF header (major version 1, sane `hdrSize`). [[nodiscard]] static bool is_cff(std::string_view data); - /// Parse the facts from @p stream; the raw bytes are retained for `data()`. - explicit CffFont(std::unique_ptr stream); /// Parse the facts from an in-memory CFF blob (retained for `data()`). explicit CffFont(std::string data); diff --git a/src/odr/internal/font/cff_transform.cpp b/src/odr/internal/font/cff_transform.cpp index 62a4cd6e..ad36240e 100644 --- a/src/odr/internal/font/cff_transform.cpp +++ b/src/odr/internal/font/cff_transform.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include @@ -189,9 +188,7 @@ std::string cff::wrap_to_otf(const CffFont &font, static_cast(first), static_cast(last))); - std::ostringstream out; - build_sfnt(out, 0x4f54544f /* 'OTTO' */, std::move(tables)); - return std::move(out).str(); + return build_sfnt(0x4f54544f /* 'OTTO' */, std::move(tables)); } } // namespace odr::internal::font diff --git a/src/odr/internal/font/font_file.cpp b/src/odr/internal/font/font_file.cpp index aa1d58de..8686bc04 100644 --- a/src/odr/internal/font/font_file.cpp +++ b/src/odr/internal/font/font_file.cpp @@ -1,8 +1,11 @@ #include +#include #include #include +#include +#include #include namespace odr::internal::font { @@ -12,7 +15,8 @@ FontFile::FontFile(std::shared_ptr file, : m_file{std::move(file)}, m_file_type{file_type} { // Parse eagerly: a parse failure is how detection rejects a non-font, so the // open-strategy try/catch can fall through. - m_font = std::make_shared(m_file->stream()); + m_font = + std::make_shared(util::stream::read(*m_file->stream())); } std::shared_ptr FontFile::file() const noexcept { diff --git a/src/odr/internal/font/sfnt_font.cpp b/src/odr/internal/font/sfnt_font.cpp index 083b103a..f6ef706c 100644 --- a/src/odr/internal/font/sfnt_font.cpp +++ b/src/odr/internal/font/sfnt_font.cpp @@ -2,14 +2,10 @@ #include #include -#include #include #include #include -#include -#include -#include #include #include #include @@ -179,14 +175,6 @@ bool SfntFont::is_sfnt(const std::string_view data) { tag == "true" || tag == "ttcf" || tag == "typ1"; } -SfntFont::SfntFont(std::unique_ptr stream) { - if (stream == nullptr) { - throw std::invalid_argument("sfnt: null input stream"); - } - m_data = util::stream::read(*stream); - parse(); -} - SfntFont::SfntFont(std::string data) : m_data{std::move(data)} { parse(); } void SfntFont::parse() { @@ -506,7 +494,7 @@ void SfntFont::set_cmap(std::map cmap) { update_reverse(); } -void SfntFont::write(std::ostream &out) const { +std::string SfntFont::write() const { std::vector> tables; tables.reserve(m_tables.size() + 1); for (const auto &[tag, location] : m_tables) { @@ -543,7 +531,7 @@ void SfntFont::write(std::ostream &out) const { const std::uint32_t version = m_format == FontFormat::opentype_cff ? 0x4f54544fU /* 'OTTO' */ : 0x00010000U; - build_sfnt(out, version, std::move(tables)); + return build_sfnt(version, std::move(tables)); } std::optional diff --git a/src/odr/internal/font/sfnt_font.hpp b/src/odr/internal/font/sfnt_font.hpp index 4a2f1e04..ad8eec94 100644 --- a/src/odr/internal/font/sfnt_font.hpp +++ b/src/odr/internal/font/sfnt_font.hpp @@ -4,9 +4,7 @@ #include #include -#include #include -#include #include #include #include @@ -26,9 +24,6 @@ class SfntFont final : public abstract::Font { /// Cheap magic test: a recognised SFNT version tag at the head of @p data. [[nodiscard]] static bool is_sfnt(std::string_view data); - /// Reads @p stream fully into an in-memory buffer and parses the facts from - /// it; the bytes are retained for pass-through (see `write()`). - explicit SfntFont(std::unique_ptr stream); /// Parses the facts from an in-memory SFNT blob (retained for `write()`). explicit SfntFont(std::string data); @@ -55,11 +50,10 @@ class SfntFont final : public abstract::Font { /// font (see `sfnt_transform.hpp`'s `reencode_to_pua`), then `write()`. void set_cmap(std::map cmap); - /// Serialize the current state to @p out: the (possibly mutated) `cmap` - /// rebuilt from `cmap()`, every other table copied verbatim from the source - /// stream, with a freshly computed table directory and checksums. @p out need - /// only be a forward sink (see `build_sfnt`). - void write(std::ostream &out) const; + /// Serialize the current state and return the bytes: the (possibly mutated) + /// `cmap` rebuilt from `cmap()`, every other table copied verbatim from the + /// source stream, with a freshly computed table directory and checksums. + [[nodiscard]] std::string write() const; private: /// Parse the facts from `m_data` (called by both constructors). diff --git a/src/odr/internal/font/sfnt_transform.cpp b/src/odr/internal/font/sfnt_transform.cpp index f59dfa09..30e9fdf3 100644 --- a/src/odr/internal/font/sfnt_transform.cpp +++ b/src/odr/internal/font/sfnt_transform.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include @@ -71,8 +70,9 @@ char32_t font::pua_code_point(const std::uint16_t glyph) noexcept { return pua_base + glyph; } -void font::build_sfnt(std::ostream &out, const std::uint32_t sfnt_version, - std::vector> tables) { +std::string +font::build_sfnt(const std::uint32_t sfnt_version, + std::vector> tables) { std::ranges::sort( tables, {}, [](const auto &e) -> const std::string & { return e.first; }); @@ -127,11 +127,14 @@ void font::build_sfnt(std::ostream &out, const std::uint32_t sfnt_version, bs::write_u32_be(tables[head_index].second, 8, 0xb1b0afbaU - file_checksum); } - out.write(header.data(), static_cast(header.size())); - out.write(directory.data(), static_cast(directory.size())); + std::string out; + out.reserve(offset); // `offset` has accumulated to the whole-file size + out.append(header); + out.append(directory); for (const auto &data : tables | std::views::values) { - out.write(data.data(), static_cast(data.size())); + out.append(data); } + return out; } std::string font::serialize_cmap(const std::map &map) { diff --git a/src/odr/internal/font/sfnt_transform.hpp b/src/odr/internal/font/sfnt_transform.hpp index 93fff536..8d19e29a 100644 --- a/src/odr/internal/font/sfnt_transform.hpp +++ b/src/odr/internal/font/sfnt_transform.hpp @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include #include @@ -20,16 +19,18 @@ class SfntFont; /// per-font table needed. [[nodiscard]] char32_t pua_code_point(std::uint16_t glyph) noexcept; -/// Serialize an SFNT from its tables to @p out, computing the table directory, -/// per-table checksums and `head.checkSumAdjustment`. @p tables need not be -/// sorted; a `head` table is patched in place with the final adjustment. +/// Serialize an SFNT from its tables, computing the table directory, per-table +/// checksums and `head.checkSumAdjustment`, and return the assembled bytes. +/// @p tables need not be sorted; a `head` table is patched in place with the +/// final adjustment. /// /// The whole-file checksum is additive over the 4-byte-aligned table layout, so /// it equals `checksum(header+directory) + Σ checksum(table)` — computed -/// analytically and the adjustment patched into `head` before the single -/// forward write, so @p out need only be a forward sink (no seek, no tee). -void build_sfnt(std::ostream &out, std::uint32_t sfnt_version, - std::vector> tables); +/// analytically and the adjustment patched into `head` before the bytes are +/// concatenated. +[[nodiscard]] std::string +build_sfnt(std::uint32_t sfnt_version, + std::vector> tables); /// Serialize a code point -> glyph map into a `cmap` table. /// diff --git a/src/odr/internal/html/font_file.cpp b/src/odr/internal/html/font_file.cpp index 2d252480..83c01f14 100644 --- a/src/odr/internal/html/font_file.cpp +++ b/src/odr/internal/html/font_file.cpp @@ -11,10 +11,11 @@ #include #include #include +#include #include +#include #include -#include #include namespace odr::internal::html { @@ -95,11 +96,10 @@ class HtmlServiceImpl final : public HtmlService { // Re-encode a fresh parse: reencode_to_pua mutates the cmap in place, and // the grid below still reads each glyph's original Unicode from // `font`. - font::sfnt::SfntFont embed(m_font_file.impl()->file()->stream()); + font::sfnt::SfntFont embed( + util::stream::read(*m_font_file.impl()->file()->stream())); font::reencode_to_pua(embed); - std::ostringstream sfnt_out; - embed.write(sfnt_out); - reencoded = sfnt_out.str(); + reencoded = embed.write(); } catch (...) { out.out() << "

font has too many glyphs to display

"; out.write_body_end(); diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index 07a250d1..aad74a92 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -658,8 +658,7 @@ class HtmlServiceImpl final : public HtmlService { std::map original_cmap = sfnt->cmap(); try { font::reencode_to_pua(*sfnt); - std::ostringstream sfnt_out; - sfnt->write(sfnt_out); + (void)sfnt->write(); usable = true; } catch (...) { usable = false; @@ -1083,9 +1082,7 @@ class HtmlServiceImpl final : public HtmlService { if (auto sfnt = std::dynamic_pointer_cast( font->embedded_font)) { font::reencode_to_pua(*sfnt, extra); - std::ostringstream sfnt_out; - sfnt->write(sfnt_out); - reencoded = std::move(sfnt_out).str(); + reencoded = sfnt->write(); } else if (auto cff = std::dynamic_pointer_cast( font->embedded_font)) { reencoded = font::cff::wrap_to_otf(*cff, extra); diff --git a/test/src/internal/font/cff_font.cpp b/test/src/internal/font/cff_font.cpp index 50af6021..322875a6 100644 --- a/test/src/internal/font/cff_font.cpp +++ b/test/src/internal/font/cff_font.cpp @@ -10,8 +10,6 @@ #include #include -#include -#include #include #include @@ -432,7 +430,7 @@ TEST(CffFontTest, WrapsToLoadableOtf) { // The wrap is a valid OTTO that parses back as an SFNT carrying the CFF as a // pass-through table and a uniform PUA cmap over every glyph. ASSERT_TRUE(sfnt::SfntFont::is_sfnt(otf)); - const sfnt::SfntFont wrapped{std::make_unique(otf)}; + const sfnt::SfntFont wrapped{otf}; EXPECT_EQ(wrapped.format(), FontFormat::opentype_cff); EXPECT_EQ(wrapped.glyph_count(), cff.glyph_count()); // PUA code point U+E000+glyph maps back to that glyph. @@ -451,7 +449,7 @@ TEST(CffFontTest, WrapDropsExtraEntriesPastGlyphCount) { 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(otf)}; + const sfnt::SfntFont wrapped{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); diff --git a/test/src/internal/font/font_file.cpp b/test/src/internal/font/font_file.cpp index ac5a16cd..0cf15465 100644 --- a/test/src/internal/font/font_file.cpp +++ b/test/src/internal/font/font_file.cpp @@ -110,9 +110,7 @@ std::string name_table(const std::string &ascii) { std::string build_sfnt_bytes(std::uint32_t version, std::vector> tables) { - std::ostringstream out; - build_sfnt(out, version, std::move(tables)); - return out.str(); + return build_sfnt(version, std::move(tables)); } std::string sample_ttf() { diff --git a/test/src/internal/font/sfnt_transform.cpp b/test/src/internal/font/sfnt_transform.cpp index 6ee02b79..155e2016 100644 --- a/test/src/internal/font/sfnt_transform.cpp +++ b/test/src/internal/font/sfnt_transform.cpp @@ -7,9 +7,7 @@ #include #include -#include #include -#include #include #include #include @@ -20,28 +18,23 @@ namespace { namespace bs = odr::internal::util::byte_string; -/// Parse a font from its in-memory bytes (the reader consumes an -/// `std::istream`). +/// Parse a font from its in-memory bytes. sfnt::SfntFont parse(std::string bytes) { - return sfnt::SfntFont(std::make_unique(std::move(bytes))); + return sfnt::SfntFont(std::move(bytes)); } -/// Serialize an SFNT to bytes (the writer emits to an `std::ostream`). +/// Serialize an SFNT to bytes. std::string build_font(std::uint32_t version, std::vector> tables) { - std::ostringstream out; - build_sfnt(out, version, std::move(tables)); - return out.str(); + return build_sfnt(version, std::move(tables)); } /// Parse @p bytes, re-encode it to the PUA in place, and write it back out. std::string reencoded(std::string bytes) { sfnt::SfntFont font = parse(std::move(bytes)); reencode_to_pua(font); - std::ostringstream out; - font.write(out); - return out.str(); + return font.write(); } std::string head_table() { @@ -234,10 +227,8 @@ TEST(SfntTransform, reencode_with_extra_round_trips_through_write) { sfnt::SfntFont font = parse(sample_font()); // 'A' is adjacent to nothing; 'Z' forces a second cmap segment past the PUA. reencode_to_pua(font, {{U'A', 1}, {U'Z', 2}}); - std::ostringstream out; - font.write(out); - const sfnt::SfntFont parsed = parse(std::move(out).str()); + const sfnt::SfntFont parsed = parse(font.write()); EXPECT_EQ(parsed.glyph_for_code_point('A'), 1); EXPECT_EQ(parsed.glyph_for_code_point('Z'), 2); EXPECT_EQ(parsed.glyph_for_code_point(pua_code_point(1)), 1); @@ -286,11 +277,10 @@ TEST(SfntTransform, write_keeps_existing_post) { sfnt::SfntFont parsed = parse(font); reencode_to_pua(parsed); - std::ostringstream out; - parsed.write(out); + const std::string out = parsed.write(); // The source `post` is copied through verbatim, not replaced. - EXPECT_EQ(table(out.str(), "post"), original_post); + EXPECT_EQ(table(out, "post"), original_post); } TEST(SfntTransform, write_synthesizes_os2_when_absent) { @@ -326,9 +316,8 @@ TEST(SfntTransform, write_keeps_existing_os2) { sfnt::SfntFont parsed = parse(font); reencode_to_pua(parsed); - std::ostringstream out; - parsed.write(out); + const std::string out = parsed.write(); // The source `OS/2` is copied through verbatim, not replaced. - EXPECT_EQ(table(out.str(), "OS/2"), original_os2); + EXPECT_EQ(table(out, "OS/2"), original_os2); } diff --git a/test/src/internal/pdf/pdf_font.cpp b/test/src/internal/pdf/pdf_font.cpp index 509785d0..2cb89be6 100644 --- a/test/src/internal/pdf/pdf_font.cpp +++ b/test/src/internal/pdf/pdf_font.cpp @@ -7,7 +7,6 @@ #include #include -#include #include #include #include @@ -120,8 +119,7 @@ std::shared_ptr sample_font() { {"hhea", hhea_table(5)}, {"hmtx", hmtx_table(5)}, {"maxp", maxp_table(5)}}); - return std::make_shared( - std::make_unique(std::move(sfnt))); + return std::make_shared(std::move(sfnt)); } /// Big-endian 2-byte codes, as a composite font's character codes appear.