From f5d3b79ac2b66eaedc2287b1066d4b32ba644592 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 19 May 2026 18:38:12 +0000 Subject: [PATCH 1/6] Extract shared text-layout primitives Pull font_for, Token, Line, tokenize_runs, wrap_tokens/wrap_runs and total_height out of render.cpp and table_layout.cpp into a new private header src/text_layout.h. Both translation units used to carry their own byte-identical copies and a near-clone wrap engine; the templated wrap_tokens in render.cpp already had the right shape but wasn't reachable from table_layout.cpp. The shared template now coalesces consecutive same-style spans on a line (an optimisation that previously only existed in table_layout's copy). render.cpp's path inherits it for free, dropping Tj operator count ~4x in the example renders without changing the visible output. The wrap API now takes line_height_pt directly instead of (size_pt, leading_factor); render.cpp call sites pre-multiply at the call, table_layout passes style.text_leading_pt unchanged. src/render.cpp | -190 lines src/table_layout.cpp | -197 lines src/text_layout.h | +213 lines (new) --- src/render.cpp | 190 +++----------------------------------- src/table_layout.cpp | 197 ++++++--------------------------------- src/text_layout.h | 213 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 254 insertions(+), 346 deletions(-) create mode 100644 src/text_layout.h diff --git a/src/render.cpp b/src/render.cpp index ec585fb..a2af0e6 100644 --- a/src/render.cpp +++ b/src/render.cpp @@ -4,13 +4,12 @@ #include #include "io_helpers.h" -#include "utf8_decode.h" +#include "text_layout.h" #include #include #include #include -#include #include namespace mark2haru { @@ -18,171 +17,12 @@ namespace { namespace fs = std::filesystem; -Pdf_font font_for(Inline_style style) -{ - switch (style) { - case Inline_style::BOLD: return Pdf_font::BOLD; - case Inline_style::ITALIC: return Pdf_font::ITALIC; - case Inline_style::BOLD_ITALIC: return Pdf_font::BOLD_ITALIC; - case Inline_style::CODE: return Pdf_font::MONO; - case Inline_style::NORMAL: - default: return Pdf_font::REGULAR; - } -} - -struct Token -{ - std::string text; - Inline_style style = Inline_style::NORMAL; - bool newline = false; -}; - -struct Line -{ - std::vector> - spans; - double height_pt = 0.0; -}; - -std::vector tokenize_runs(const std::vector& runs) -{ - std::vector tokens; - for (const auto& run : runs) { - size_t start = 0; - while (start <= run.text.size()) { - const size_t nl = run.text.find('\n', start); - const std::string chunk = nl == std::string::npos - ? run.text.substr(start) - : run.text.substr(start, nl - start); - - size_t piece = 0; - while (piece <= chunk.size()) { - const size_t sp = chunk.find(' ', piece); - if (sp == std::string::npos) { - if (piece < chunk.size()) { - tokens.push_back({ chunk.substr(piece), run.style, false }); - } - break; - } - if (sp > piece) { - tokens.push_back({ chunk.substr(piece, sp - piece), run.style, false }); - } - tokens.push_back({ " ", run.style, false }); - piece = sp + 1; - } - - if (nl == std::string::npos) { - break; - } - tokens.push_back({ "", run.style, true }); - start = nl + 1; - } - } - return tokens; -} - -template -std::vector wrap_tokens( - const std::vector& tokens, - double max_width_pt, - double size_pt, - double leading, - MeasureFn&& measure) -{ - std::vector lines; - Line current; - double current_width = 0.0; - - auto finish_line = [&]() { - current.height_pt = size_pt * leading; - lines.push_back(current); - current = Line{}; - current_width = 0.0; - }; - - auto add_word = [&](const std::string& word, Inline_style style) { - const double word_width = measure(font_for(style), word, size_pt); - if (current_width > 0.0 && current_width + word_width > max_width_pt) { - finish_line(); - } - if (word_width <= max_width_pt) { - current.spans.emplace_back(word, style); - current_width += word_width; - return; - } - - // Word is wider than a full line: break it at code-point boundaries. - std::string fragment; - double fragment_width = 0.0; - for (const auto& piece : utf8::split_pieces(word)) { - const double ch_width = measure(font_for(style), piece, size_pt); - const double projected = current_width + fragment_width + ch_width; - const bool any_content_on_line = current_width > 0.0 || fragment_width > 0.0; - if (any_content_on_line && projected > max_width_pt) { - if (!fragment.empty()) { - current.spans.emplace_back(fragment, style); - current_width += fragment_width; - fragment.clear(); - fragment_width = 0.0; - } - finish_line(); - } - fragment += piece; - fragment_width += ch_width; - } - if (!fragment.empty()) { - current.spans.emplace_back(fragment, style); - current_width += fragment_width; - } - }; - - for (const auto& token : tokens) { - if (token.newline) { - finish_line(); - continue; - } - if (token.text == " ") { - const double space_width = measure(font_for(token.style), token.text, size_pt); - if (!current.spans.empty() && current_width + space_width > max_width_pt) { - finish_line(); - } - else - if (!current.spans.empty()) { - current.spans.emplace_back(token.text, token.style); - current_width += space_width; - } - continue; - } - if (!token.text.empty()) { - add_word(token.text, token.style); - } - } - - if (!current.spans.empty() || lines.empty()) { - finish_line(); - } - return lines; -} - -template -std::vector wrap_runs( - const std::vector& runs, - double max_width_pt, - double size_pt, - double leading, - MeasureFn&& measure) -{ - return wrap_tokens(tokenize_runs(runs), max_width_pt, size_pt, leading, measure); -} - -double total_height(const std::vector& lines) -{ - double h = 0.0; - for (const auto& line : lines) { - h += line.height_pt; - } - return h; -} +using text_layout::font_for; +using text_layout::Line; +using text_layout::Token; +using text_layout::total_height; +using text_layout::wrap_tokens; +using text_layout::wrap_runs; // Standard "overloaded" idiom to combine a set of single-type lambdas // into one callable that std::visit can dispatch on. Adding a new block @@ -239,18 +79,18 @@ std::vector code_lines( const std::string& text, double max_width_pt, double size_pt, - double leading, + double line_height_pt, MeasureFn&& measure) { std::vector lines; for (const auto& raw : split_lines(text)) { std::vector tokens; tokens.push_back({ raw, Inline_style::CODE, false }); - auto wrapped = wrap_tokens(tokens, max_width_pt, size_pt, leading, measure); + auto wrapped = wrap_tokens(tokens, max_width_pt, size_pt, line_height_pt, measure); lines.insert(lines.end(), wrapped.begin(), wrapped.end()); } if (lines.empty()) { - lines.push_back({ {}, size_pt * leading }); + lines.push_back({ {}, line_height_pt }); } return lines; } @@ -326,7 +166,7 @@ bool render_markdown_to_pdf( auto on_paragraph = [&](const Paragraph_block& pb) { const auto lines = wrap_runs(pb.runs, content_width, options.body_size_pt, - options.line_spacing, measure); + options.body_size_pt * options.line_spacing, measure); ensure_space(total_height(lines) + options.body_size_pt * 0.25); cursor_y = draw_lines_at(lines, options.body_size_pt, options.margin_left_pt, cursor_y); cursor_y += options.body_size_pt * 0.35; @@ -337,7 +177,7 @@ bool render_markdown_to_pdf( const double size = options.body_size_pt * hm.size_factor; const double space_before = size * hm.space_before_factor; const double space_after = size * hm.space_after_factor; - const auto lines = wrap_runs(hb.runs, content_width, size, 1.15, measure); + const auto lines = wrap_runs(hb.runs, content_width, size, size * 1.15, measure); ensure_space(space_before + total_height(lines) + space_after); cursor_y += space_before; cursor_y = draw_lines_at(lines, size, options.margin_left_pt, cursor_y); @@ -355,7 +195,7 @@ bool render_markdown_to_pdf( lb.items[i].runs, item_width, options.body_size_pt, - options.line_spacing, + options.body_size_pt * options.line_spacing, measure); ensure_space(total_height(lines) + options.body_size_pt * 0.2); writer.draw_text( @@ -407,7 +247,7 @@ bool render_markdown_to_pdf( const double size = options.body_size_pt * 0.92; const double pad = options.body_size_pt * 0.45; const double available_width = content_width - pad * 2.0; - const auto lines = code_lines(cb.text, available_width, size, 1.25, measure); + const auto lines = code_lines(cb.text, available_width, size, size * 1.25, measure); const double height = total_height(lines) + pad * 2.0; ensure_space(height + options.body_size_pt * 0.2); writer.fill_rect(options.margin_left_pt, cursor_y, content_width, height, @@ -444,7 +284,7 @@ bool render_markdown_to_pdf( for (size_t col = 0; col < column_count; ++col) { const std::vector empty; const auto& runs = col < row.cells.size() ? row.cells[col].runs : empty; - cell_lines[r][col] = wrap_runs(runs, inner_width, cell_size, 1.22, measure); + cell_lines[r][col] = wrap_runs(runs, inner_width, cell_size, cell_size * 1.22, measure); const double cell_height = total_height(cell_lines[r][col]) + cell_pad * 2.0; row_height = std::max(row_height, cell_height); } diff --git a/src/table_layout.cpp b/src/table_layout.cpp index 9ad32c2..f8fee1d 100644 --- a/src/table_layout.cpp +++ b/src/table_layout.cpp @@ -1,6 +1,6 @@ #include -#include "utf8_decode.h" +#include "text_layout.h" #include #include @@ -9,31 +9,18 @@ namespace mark2haru { namespace { -Pdf_font font_for(Inline_style style) -{ - switch (style) { - case Inline_style::BOLD: return Pdf_font::BOLD; - case Inline_style::ITALIC: return Pdf_font::ITALIC; - case Inline_style::BOLD_ITALIC: return Pdf_font::BOLD_ITALIC; - case Inline_style::CODE: return Pdf_font::MONO; - case Inline_style::NORMAL: - default: return Pdf_font::REGULAR; - } -} - -struct Token -{ - std::string text; - Inline_style style = Inline_style::NORMAL; - bool newline = false; -}; +using text_layout::font_for; +using text_layout::total_height; -struct Line +// Builds a measurer callable that the wrap engine in text_layout.h can use, +// bridging the Measurement_context member function into the generic +// `(font, text, size_pt) -> width_pt` interface. +auto metrics_measurer(const Measurement_context& metrics) { - std::vector> - spans; - double height_pt = 0.0; -}; + return [&metrics](Pdf_font font, const std::string& text, double size_pt) { + return metrics.measure_text_width(font, text, size_pt); + }; +} std::vector table_cell_runs( const Table_row& row, @@ -61,145 +48,6 @@ std::vector table_cell_runs( return runs; } -std::vector tokenize_runs(const std::vector& runs) -{ - std::vector tokens; - for (const auto& run : runs) { - size_t start = 0; - while (start <= run.text.size()) { - const size_t nl = run.text.find('\n', start); - const std::string chunk = nl == std::string::npos - ? run.text.substr(start) - : run.text.substr(start, nl - start); - - size_t piece = 0; - while (piece <= chunk.size()) { - const size_t sp = chunk.find(' ', piece); - if (sp == std::string::npos) { - if (piece < chunk.size()) { - tokens.push_back({ chunk.substr(piece), run.style, false }); - } - break; - } - if (sp > piece) { - tokens.push_back({ chunk.substr(piece, sp - piece), run.style, false }); - } - tokens.push_back({ " ", run.style, false }); - piece = sp + 1; - } - - if (nl == std::string::npos) { - break; - } - tokens.push_back({ {}, run.style, true }); - start = nl + 1; - } - } - return tokens; -} - -std::vector wrap_runs( - const std::vector& runs, - double max_width_pt, - double size_pt, - double leading_pt, - const Measurement_context& metrics) -{ - std::vector lines; - Line current; - double current_width = 0.0; - - auto append_span = [&](const std::string& text, Inline_style style) { - if (text.empty()) { - return; - } - if (!current.spans.empty() && current.spans.back().second == style) { - current.spans.back().first += text; - return; - } - current.spans.emplace_back(text, style); - }; - - auto finish_line = [&]() { - current.height_pt = leading_pt; - lines.push_back(current); - current = Line{}; - current_width = 0.0; - }; - - auto add_word = [&](const std::string& word, Inline_style style) { - const Pdf_font font = font_for(style); - const double word_width = metrics.measure_text_width(font, word, size_pt); - if (current_width > 0.0 && current_width + word_width > max_width_pt) { - finish_line(); - } - if (word_width <= max_width_pt) { - append_span(word, style); - current_width += word_width; - return; - } - - std::string fragment; - double fragment_width = 0.0; - for (const auto& piece : utf8::split_pieces(word)) { - const double piece_width = metrics.measure_text_width(font, piece, size_pt); - const bool has_content = current_width > 0.0 || fragment_width > 0.0; - if (has_content && current_width + fragment_width + piece_width > max_width_pt) { - if (!fragment.empty()) { - append_span(fragment, style); - current_width += fragment_width; - fragment.clear(); - fragment_width = 0.0; - } - finish_line(); - } - fragment += piece; - fragment_width += piece_width; - } - if (!fragment.empty()) { - append_span(fragment, style); - current_width += fragment_width; - } - }; - - for (const auto& token : tokenize_runs(runs)) { - if (token.newline) { - finish_line(); - continue; - } - if (token.text == " ") { - const double space_width = - metrics.measure_text_width(font_for(token.style), token.text, size_pt); - if (!current.spans.empty() && current_width + space_width > max_width_pt) { - finish_line(); - } - else - if (!current.spans.empty()) { - append_span(token.text, token.style); - current_width += space_width; - } - continue; - } - if (!token.text.empty()) { - add_word(token.text, token.style); - } - } - - if (!current.spans.empty() || lines.empty()) { - finish_line(); - } - return lines; -} - -double lines_height(const std::vector& lines) -{ - double height = 0.0; - for (const auto& line : lines) { - height += line.height_pt; - } - return height; -} - double cell_min_width( const std::vector& runs, double size_pt, @@ -208,7 +56,7 @@ double cell_min_width( double max_word = 0.0; for (const auto& run : runs) { const Pdf_font font = font_for(run.style); - for (const auto& token : tokenize_runs({ run })) { + for (const auto& token : text_layout::tokenize_runs({ run })) { if (!token.text.empty() && token.text != " ") { max_word = std::max( max_word, @@ -250,17 +98,18 @@ double table_row_height( double row_height = style.text_leading_pt; const int header_rows = table.has_header ? 1 : 0; const auto& row = table.rows[row_index]; + const auto measure = metrics_measurer(metrics); for (int col = 0; col < columns.column_count; ++col) { const double content_width = columns.widths_pt[col] - 2.0 * style.cell_padding_pt; const auto runs = table_cell_runs(row, col, row_index < header_rows); row_height = std::max( row_height, - lines_height(wrap_runs( + total_height(text_layout::wrap_runs( runs, content_width, style.text_size_pt, style.text_leading_pt, - metrics)) + 2.0 * style.cell_padding_pt); + measure)) + 2.0 * style.cell_padding_pt); } return row_height; } @@ -286,7 +135,12 @@ int cell_line_count( { return static_cast( - wrap_runs(runs, content_width_pt, style.text_size_pt, style.text_leading_pt, metrics).size() + text_layout::wrap_runs( + runs, + content_width_pt, + style.text_size_pt, + style.text_leading_pt, + metrics_measurer(metrics)).size() ); } @@ -643,18 +497,19 @@ Table_row_layout layout_table_row( }); } - double x = left_pt; - const auto& row = table.rows[row_index]; + double x = left_pt; + const auto& row = table.rows[row_index]; + const auto measure = metrics_measurer(metrics); for (int col = 0; col < columns.column_count; ++col) { const double cell_x = x + style.cell_padding_pt; double cell_y = top_pt + style.cell_padding_pt; const auto runs = table_cell_runs(row, col, is_header); - for (const auto& line : wrap_runs( + for (const auto& line : text_layout::wrap_runs( runs, columns.widths_pt[col] - 2.0 * style.cell_padding_pt, style.text_size_pt, style.text_leading_pt, - metrics)) + measure)) { double text_x = cell_x; for (const auto& [text, inline_style] : line.spans) { diff --git a/src/text_layout.h b/src/text_layout.h new file mode 100644 index 0000000..86716ec --- /dev/null +++ b/src/text_layout.h @@ -0,0 +1,213 @@ +#pragma once + +#include +#include + +#include "utf8_decode.h" + +#include +#include +#include +#include + +namespace mark2haru { +namespace text_layout { + +// Maps the parser's inline-style enum to the writer's font slot. Used by +// every wrap- or measure-shaped helper to look up the right metrics for a +// given run of text. +inline Pdf_font font_for(Inline_style style) +{ + switch (style) { + case Inline_style::BOLD: return Pdf_font::BOLD; + case Inline_style::ITALIC: return Pdf_font::ITALIC; + case Inline_style::BOLD_ITALIC: return Pdf_font::BOLD_ITALIC; + case Inline_style::CODE: return Pdf_font::MONO; + case Inline_style::NORMAL: + default: return Pdf_font::REGULAR; + } +} + +struct Token +{ + std::string text; + Inline_style style = Inline_style::NORMAL; + bool newline = false; +}; + +struct Line +{ + std::vector> + spans; + double height_pt = 0.0; +}; + +// Splits each Inline_run on '\n' and ' '. Newlines become tokens with +// `newline=true` and empty text; spaces become single-space tokens; other +// runs of non-space, non-newline characters become word tokens. Style is +// carried through unchanged. +inline std::vector tokenize_runs(const std::vector& runs) +{ + std::vector tokens; + for (const auto& run : runs) { + std::size_t start = 0; + while (start <= run.text.size()) { + const std::size_t nl = run.text.find('\n', start); + const std::string chunk = nl == std::string::npos + ? run.text.substr(start) + : run.text.substr(start, nl - start); + + std::size_t piece = 0; + while (piece <= chunk.size()) { + const std::size_t sp = chunk.find(' ', piece); + if (sp == std::string::npos) { + if (piece < chunk.size()) { + tokens.push_back({ chunk.substr(piece), run.style, false }); + } + break; + } + if (sp > piece) { + tokens.push_back({ chunk.substr(piece, sp - piece), run.style, false }); + } + tokens.push_back({ " ", run.style, false }); + piece = sp + 1; + } + + if (nl == std::string::npos) { + break; + } + tokens.push_back({ {}, run.style, true }); + start = nl + 1; + } + } + return tokens; +} + +// Wraps a pre-tokenized sequence into lines that fit within `max_width_pt`. +// `line_height_pt` is the resolved per-line height (callers that think in +// terms of a leading factor pass `size_pt * factor`). `measure(font, text, +// size_pt) -> double` returns the rendered width of `text` in points. +// +// Consecutive spans of the same style on the same line are merged so the +// caller sees the fewest possible draw spans. +template +std::vector wrap_tokens( + const std::vector& tokens, + double max_width_pt, + double size_pt, + double line_height_pt, + MeasureFn&& measure) +{ + std::vector lines; + Line current; + double current_width = 0.0; + + auto append_span = [&](const std::string& text, Inline_style style) { + if (text.empty()) { + return; + } + if (!current.spans.empty() && current.spans.back().second == style) { + current.spans.back().first += text; + return; + } + current.spans.emplace_back(text, style); + }; + + auto finish_line = [&]() { + current.height_pt = line_height_pt; + lines.push_back(current); + current = Line{}; + current_width = 0.0; + }; + + auto add_word = [&](const std::string& word, Inline_style style) { + const Pdf_font font = font_for(style); + const double word_width = measure(font, word, size_pt); + if (current_width > 0.0 && current_width + word_width > max_width_pt) { + finish_line(); + } + if (word_width <= max_width_pt) { + append_span(word, style); + current_width += word_width; + return; + } + + // Word is wider than a full line: break it at code-point boundaries. + std::string fragment; + double fragment_width = 0.0; + for (const auto& piece : utf8::split_pieces(word)) { + const double piece_width = measure(font, piece, size_pt); + const bool has_content = current_width > 0.0 || fragment_width > 0.0; + if (has_content && current_width + fragment_width + piece_width > max_width_pt) { + if (!fragment.empty()) { + append_span(fragment, style); + current_width += fragment_width; + fragment.clear(); + fragment_width = 0.0; + } + finish_line(); + } + fragment += piece; + fragment_width += piece_width; + } + if (!fragment.empty()) { + append_span(fragment, style); + current_width += fragment_width; + } + }; + + for (const auto& token : tokens) { + if (token.newline) { + finish_line(); + continue; + } + if (token.text == " ") { + const double space_width = measure(font_for(token.style), token.text, size_pt); + if (!current.spans.empty() && current_width + space_width > max_width_pt) { + finish_line(); + } + else + if (!current.spans.empty()) { + append_span(token.text, token.style); + current_width += space_width; + } + continue; + } + if (!token.text.empty()) { + add_word(token.text, token.style); + } + } + + if (!current.spans.empty() || lines.empty()) { + finish_line(); + } + return lines; +} + +template +std::vector wrap_runs( + const std::vector& runs, + double max_width_pt, + double size_pt, + double line_height_pt, + MeasureFn&& measure) +{ + return wrap_tokens( + tokenize_runs(runs), + max_width_pt, + size_pt, + line_height_pt, + std::forward(measure)); +} + +inline double total_height(const std::vector& lines) +{ + double height = 0.0; + for (const auto& line : lines) { + height += line.height_pt; + } + return height; +} + +} // namespace text_layout +} // namespace mark2haru From dc83faf88b88270acced235473266cf9c75d99c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 19 May 2026 18:40:24 +0000 Subject: [PATCH 2/6] Unify big-endian integer reads behind read_be template png_image.cpp had free functions read_be16/read_be32, and ttf_font.cpp's Bytes_view had u16/u32 methods, each duplicating the same byte-shifted read. Replace both with a single constexpr template read_be in io_helpers.h that works for any unsigned 1/2/4/8-byte integer. The existing per-call-site wrappers (read_be16, read_be32, Bytes_view::u16, ::u32) stay as thin delegates so call sites read unchanged. --- src/io_helpers.h | 18 ++++++++++++++++++ src/png_image.cpp | 10 ++-------- src/ttf_font.cpp | 10 ++-------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/io_helpers.h b/src/io_helpers.h index 6001024..522e21d 100644 --- a/src/io_helpers.h +++ b/src/io_helpers.h @@ -7,6 +7,7 @@ #include #include #include +#include #include namespace mark2haru @@ -22,6 +23,23 @@ inline bool read_file_bytes(const std::filesystem::path& path, std::vector +constexpr T read_be(const std::uint8_t* p) noexcept +{ + static_assert(std::is_unsigned_v, "read_be: T must be unsigned"); + static_assert( + sizeof(T) == 1 || sizeof(T) == 2 || sizeof(T) == 4 || sizeof(T) == 8, + "read_be: only 8/16/32/64-bit widths are supported"); + T value = 0; + for (std::size_t i = 0; i < sizeof(T); ++i) { + value = static_cast((value << 8) | static_cast(p[i])); + } + return value; +} + // Splits on '\n', stripping a trailing '\r' so CRLF and LF inputs produce // the same line vector. Keeps a final empty line if the input ends with '\n'. inline std::vector split_lines(std::string_view text) diff --git a/src/png_image.cpp b/src/png_image.cpp index 2667daa..74672d5 100644 --- a/src/png_image.cpp +++ b/src/png_image.cpp @@ -24,18 +24,12 @@ constexpr std::size_t k_max_idat_bytes = 256u * 1024u * 1024u; // 256 MiB std::uint32_t read_be32(const std::vector& bytes, std::size_t offset) { - return - (static_cast(bytes[offset ]) << 24) | - (static_cast(bytes[offset + 1]) << 16) | - (static_cast(bytes[offset + 2]) << 8) | - static_cast(bytes[offset + 3]); + return read_be(&bytes[offset]); } std::uint16_t read_be16(const std::vector& bytes, std::size_t offset) { - return static_cast( - (static_cast(bytes[offset ]) << 8) | - static_cast(bytes[offset + 1]) ); + return read_be(&bytes[offset]); } // Adds two size_t values and reports overflow. Returns true on success; diff --git a/src/ttf_font.cpp b/src/ttf_font.cpp index 70c7e9a..e8c1894 100644 --- a/src/ttf_font.cpp +++ b/src/ttf_font.cpp @@ -33,9 +33,7 @@ class Bytes_view if (!has(off, 2)) { return 0; } - return static_cast( - (static_cast(m_data[off ]) << 8) | - static_cast(m_data[off + 1]) ); + return read_be(m_data + off); } std::int16_t i16(std::size_t off) const @@ -48,11 +46,7 @@ class Bytes_view if (!has(off, 4)) { return 0; } - return - (static_cast(m_data[off ]) << 24) | - (static_cast(m_data[off + 1]) << 16) | - (static_cast(m_data[off + 2]) << 8) | - static_cast(m_data[off + 3]); + return read_be(m_data + off); } std::string string_at(std::size_t off, std::size_t len) const From c540b1eaac58d1a12a9666398144f126ba4ca19c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 19 May 2026 18:41:34 +0000 Subject: [PATCH 3/6] Extract bisect template for table-layout midpoint searches table_layout.cpp had three open-coded 18-step midpoint refinements that differed only in the predicate (cell line count vs total table height). Collapse them behind one anonymous-namespace bisect(lo, hi) template and pass the per-site predicate as a lambda. Behavior is identical: same iteration count, same accept-hi/reject-lo semantics, byte-identical smoke-render output. --- src/table_layout.cpp | 75 +++++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 40 deletions(-) diff --git a/src/table_layout.cpp b/src/table_layout.cpp index f8fee1d..bd98e59 100644 --- a/src/table_layout.cpp +++ b/src/table_layout.cpp @@ -22,6 +22,25 @@ auto metrics_measurer(const Measurement_context& metrics) }; } +// Returns the smallest value in [lo, hi] for which `fits(value)` is true, +// approximated by 18 steps of midpoint refinement (~3.8e-6 of the initial +// range). `fits` is assumed monotone: false below the threshold, true at +// or above it; `fits(hi)` must already be true at the initial `hi`. +template +double bisect(double lo, double hi, Predicate fits) +{ + for (int step = 0; step < 18; ++step) { + const double mid = (lo + hi) * 0.5; + if (fits(mid)) { + hi = mid; + } + else { + lo = mid; + } + } + return hi; +} + std::vector table_cell_runs( const Table_row& row, int col, @@ -156,18 +175,9 @@ double min_content_width_for_line_count( return -1.0; } - double lo = current_width_pt; - double hi = max_width_pt; - for (int step = 0; step < 18; ++step) { - const double mid = (lo + hi) * 0.5; - if (cell_line_count(runs, mid, style, metrics) <= target_lines) { - hi = mid; - } - else { - lo = mid; - } - } - return hi; + return bisect(current_width_pt, max_width_pt, [&](double width) { + return cell_line_count(runs, width, style, metrics) <= target_lines; + }); } // Water-fill the room above min_widths. Every column gets at least its @@ -273,19 +283,13 @@ void tighten_columns( runs, current_content_width, style, metrics); if (current_lines <= 0) { continue; } if (cell_line_count(runs, lower_bound_content, style, metrics) <= current_lines) { continue; } - double lo = lower_bound_content; - double hi = current_content_width; - for (int step = 0; step < 18; ++step) { - const double mid = (lo + hi) * 0.5; - if (cell_line_count(runs, mid, style, metrics) <= current_lines) { - hi = mid; - } - else { - lo = mid; - } - } - if (hi > needed_content_width) { - needed_content_width = hi; + const double tightened = bisect( + lower_bound_content, current_content_width, + [&](double width) { + return cell_line_count(runs, width, style, metrics) <= current_lines; + }); + if (tightened > needed_content_width) { + needed_content_width = tightened; } } @@ -328,24 +332,15 @@ Table_columns optimize_columns( continue; } - double lo = 0.0; - double hi = slack; - for (int step = 0; step < 18; ++step) { - const double mid = (lo + hi) * 0.5; + const double extra = bisect(0.0, slack, [&](double delta) { Table_columns probe = columns; - probe.widths_pt[col] += mid; - if (table_total_height(table, probe, style, metrics) + k_height_epsilon_pt - < current_height) - { - hi = mid; - } - else { - lo = mid; - } - } + probe.widths_pt[col] += delta; + return table_total_height(table, probe, style, metrics) + k_height_epsilon_pt + < current_height; + }); Table_columns candidate = columns; - candidate.widths_pt[col] += hi; + candidate.widths_pt[col] += extra; const double candidate_height = table_total_height(table, candidate, style, metrics); if (candidate_is_better(candidate, candidate_height, best, best_height)) { best = std::move(candidate); From b79685284987aeb09227e16207e1b94ca75b4766 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 19 May 2026 18:45:09 +0000 Subject: [PATCH 4/6] Extract shared test fixture into tests/test_common.h test_png.cpp and test_pdf_writer.cpp both did the same argv[0] parent-dir derivation and Measurement_context bring-up with briefutil_default fonts. Promote those two patterns into inline helpers executable_dir(argc, argv) and load_default_metrics(exe_dir) in a new private header tests/test_common.h. test_png.cpp's path derivation is now normalized via fs::absolute, matching what test_pdf_writer.cpp already did; the font_context resolver is already cwd-aware so this is at worst a no-op. --- tests/test_common.h | 38 ++++++++++++++++++++++++++++++++++++++ tests/test_pdf_writer.cpp | 14 ++++---------- tests/test_png.cpp | 11 ++++------- 3 files changed, 46 insertions(+), 17 deletions(-) create mode 100644 tests/test_common.h diff --git a/tests/test_common.h b/tests/test_common.h new file mode 100644 index 0000000..6e12f4b --- /dev/null +++ b/tests/test_common.h @@ -0,0 +1,38 @@ +#pragma once + +#include + +#include +#include +#include + +namespace mark2haru_test { + +// Resolves the directory containing the current executable, which the +// bundled-font loader walks up from to find a `fonts/` directory. Falls back +// to the current working directory when argv[0] is missing. +inline std::filesystem::path executable_dir(int argc, char** argv) +{ + if (argc <= 0 || argv == nullptr || argv[0] == nullptr) { + return std::filesystem::current_path(); + } + return std::filesystem::absolute(argv[0]).parent_path(); +} + +// Loads the briefutil default font family rooted at `exe_dir`. Returns +// nullptr on failure after writing a diagnostic to stderr; the caller picks +// the exit code. +inline std::shared_ptr load_default_metrics( + const std::filesystem::path& exe_dir) +{ + auto metrics = std::make_shared( + mark2haru::Font_family_config::briefutil_default(), + exe_dir); + if (!metrics->loaded()) { + std::fprintf(stderr, "font load failed: %s\n", metrics->error().c_str()); + return nullptr; + } + return metrics; +} + +} // namespace mark2haru_test diff --git a/tests/test_pdf_writer.cpp b/tests/test_pdf_writer.cpp index 186c9b8..4805f70 100644 --- a/tests/test_pdf_writer.cpp +++ b/tests/test_pdf_writer.cpp @@ -1,13 +1,12 @@ -#include #include #include "miniz.h" +#include "test_common.h" #include #include #include #include -#include #include #include @@ -86,16 +85,11 @@ bool contains(const std::string& haystack, const std::string& needle) int main(int argc, char* argv[]) { - const fs::path exe_dir = argc > 0 - ? fs::absolute(argv[0]).parent_path() - : fs::current_path(); + const fs::path exe_dir = mark2haru_test::executable_dir(argc, argv); const fs::path out_path = exe_dir / "test_pdf_writer.pdf"; - auto metrics = std::make_shared( - mark2haru::Font_family_config::briefutil_default(), - exe_dir); - if (!metrics->loaded()) { - std::cerr << metrics->error() << "\n"; + auto metrics = mark2haru_test::load_default_metrics(exe_dir); + if (!metrics) { return 1; } diff --git a/tests/test_png.cpp b/tests/test_png.cpp index da29877..fd2206a 100644 --- a/tests/test_png.cpp +++ b/tests/test_png.cpp @@ -2,6 +2,7 @@ #include #include "miniz.h" +#include "test_common.h" #include #include @@ -9,7 +10,6 @@ #include #include #include -#include #include #include @@ -234,7 +234,7 @@ int main(int argc, char** argv) return 2; } - const fs::path exe_dir = fs::path(argv[0]).parent_path(); + const fs::path exe_dir = mark2haru_test::executable_dir(argc, argv); const fs::path temp_dir = fs::temp_directory_path() / "mark2haru_png_test"; fs::create_directories(temp_dir); @@ -270,11 +270,8 @@ int main(int argc, char** argv) if (!expect_png(indexed_path, 1, 1, 3, true, { 0x01, 0x23, 0x45 }, { 17 })) { return 7; } if (!expect_png(rgba16_path, 1, 1, 3, true, { 0x12, 0x56, 0x9A }, { 0xDE })) { return 8; } - auto metrics = std::make_shared( - mark2haru::Font_family_config::briefutil_default(), - exe_dir); - if (!metrics->loaded()) { - std::cerr << metrics->error() << "\n"; + auto metrics = mark2haru_test::load_default_metrics(exe_dir); + if (!metrics) { return 9; } From 3eecd3d5cb3f885ce8ac69a6e8355296355df978 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 19 May 2026 18:46:24 +0000 Subject: [PATCH 5/6] Collapse build_stream_object overloads into one Container template The two thin overloads for std::string and std::vector both just delegated to the raw-pointer build_stream_object. Replace them with a single template on Container (anything with .data()/.size()), which subsumes both call sites without changing behavior. The raw- pointer version remains the authoritative implementation. --- src/pdf_writer.cpp | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/pdf_writer.cpp b/src/pdf_writer.cpp index 9925086..9a3ea10 100644 --- a/src/pdf_writer.cpp +++ b/src/pdf_writer.cpp @@ -240,7 +240,11 @@ std::string build_stream_object( return out; } -std::string build_stream_object(const std::string& payload, const std::string& extra = {}) +// Catches anything with .data() and .size(): std::string, std::vector, +// std::array, std::span, etc. The reinterpret_cast is a no-op for unsigned-byte +// containers and a defined conversion for std::string (char -> unsigned char). +template +std::string build_stream_object(const Container& payload, const std::string& extra = {}) { return build_stream_object( reinterpret_cast(payload.data()), @@ -248,13 +252,6 @@ std::string build_stream_object(const std::string& payload, const std::string& e extra); } -std::string build_stream_object( - const std::vector& payload, - const std::string& extra = {}) -{ - return build_stream_object(payload.data(), payload.size(), extra); -} - // Allocates 1-based PDF object IDs on demand and lets callers fill in the // body any time before write(). Eliminates the manual "next_obj++" arithmetic // and the implicit invariant that body order matches reservation order. From fdffe42e86a5640137eb639225192ea7a293190b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 19 May 2026 19:33:59 +0000 Subject: [PATCH 6/6] ci: update consumer smoke test to current API name The downstream-consumer smoke test still referenced mark2haru::render_options_t, which was renamed to mark2haru::Render_options in 9aa1c0e (May 3) as part of the Varinomics API style pass. The workflow file was missed at the time. --- .github/workflows/build-and-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 6895b03..f44410c 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -59,7 +59,7 @@ jobs: cat > build/_consumer/consumer.cpp <<'EOF' #include int main() { - mark2haru::render_options_t options; + mark2haru::Render_options options; return options.page_width_pt > 0.0 ? 0 : 1; } EOF