Extract text layout logic into reusable text_layout.h module - #4
Closed
imakris wants to merge 6 commits into
Closed
Conversation
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<MeasureFn> 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)
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<T> 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.
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<Predicate>(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.
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.
The two thin overloads for std::string and std::vector<uint8_t> 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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors text layout and measurement logic into a new
text_layout.hheader module to eliminate code duplication betweenrender.cppandtable_layout.cpp. This improves maintainability and allows both modules to share consistent text wrapping and measurement implementations.Key Changes
text_layout.hmodule: ExtractedToken,Line,font_for(),tokenize_runs(),wrap_tokens(),wrap_runs(), andtotal_height()into a reusable namespace that both rendering and table layout can usetable_layout.cpp:metrics_measurer()helper to bridgeMeasurement_contextinto the generic(font, text, size_pt) -> width_ptinterfacebisect()template for binary search refinement (used in column width optimization)text_layout::wrap_runs()render.cpp: Removed duplicate implementations and now usestext_layoutnamespace functions viausingdeclarationsrender.cppcallers: Changed from passingleadingfactor to passing pre-computedline_height_pt(e.g.,size_pt * leading→size_pt * leading)test_common.h: Extracted common test utilities (executable_dir(),load_default_metrics()) to reduce duplication in test filesio_helpers.h: Addedread_be<T>()template for reading big-endian integers, replacing inline bit-shift code inpng_image.cppandttf_font.cpppdf_writer.cpp: Templatedbuild_stream_object()to accept any container with.data()and.size()methodsNotable Implementation Details
wrap_runs()function now accepts a genericMeasureFncallable, allowing callers to provide custom measurement logic without coupling toMeasurement_contextbisect()template performs 18 iterations of midpoint refinement for ~3.8e-6 precision relative to the initial rangeinlinein the header to avoid ODR violations and allow template instantiation across translation unitshttps://claude.ai/code/session_013Q7H5C4AZ5XTpHwVa8UWmG