From d853db62ae27921c0c164f08c8259ed60fca11c5 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Mon, 28 Sep 2026 16:32:18 +0200 Subject: [PATCH] fix(text): carry a glyph's negative bearing and harden the bake's extents --- include/plot/render/renderer.hpp | 9 +- include/plot/text/true_type_font.hpp | 52 +++- src/text/true_type_font.cpp | 83 ++++-- tests/CMakeLists.txt | 10 + tests/test_true_type_font.cpp | 376 +++++++++++++++++++++++++-- 5 files changed, 470 insertions(+), 60 deletions(-) diff --git a/include/plot/render/renderer.hpp b/include/plot/render/renderer.hpp index a184f5b..6d17583 100644 --- a/include/plot/render/renderer.hpp +++ b/include/plot/render/renderer.hpp @@ -98,9 +98,12 @@ class Renderer { /// which is the same lifetime rule `Plot::set_text_backend()` states. void set_text_backend(const render::TextBackend* font) noexcept; - /// The backend the last frame was drawn with, or the embedded face when the - /// host installed none. The renderer's own wrapper while a frame is in flight, - /// so a caller reads this after `draw()` to see what the plot was handed. + /// The backend this renderer *borrows*: whatever `set_text_backend()` was given, + /// or the embedded face when nothing was installed. It is deliberately **not** + /// the `AxisFont` wrapper the renderer builds around it -- after a `draw()`, + /// `Plot::text_backend()` reads back that wrapper, because the wrapper is what + /// the plot measured with -- so a caller checking or re-pointing the face itself + /// asks here. const render::TextBackend* text_backend() const noexcept { return font_; } /// Starts a frame: records `fb_w` x `fb_h` for the projection, flips diff --git a/include/plot/text/true_type_font.hpp b/include/plot/text/true_type_font.hpp index 0d7f598..faf96ac 100644 --- a/include/plot/text/true_type_font.hpp +++ b/include/plot/text/true_type_font.hpp @@ -44,6 +44,18 @@ namespace plot::text { /// face's own, so a proportional run advances per glyph while every rect stays /// one size. /// +/// **No ink is trimmed away, in either direction.** The cell is as wide as the +/// widest advance, the widest ink *to the right* of the pen, **and** the deepest +/// ink to its left: an italic glyph reaches back over the pen that advances it +/// (half the italic faces on a stock Linux box do, `U+006A` being the usual +/// offender), so the cell sits a texel or two to the right of the pen and +/// `Glyph::xoff` is the matching non-positive bearing -- the pipeline places the +/// cell corner at `pen + xoff`, which is exactly what makes a negative bearing +/// expressible. Bounding ink on one side and clipping it on the other would be an +/// inconsistency, and a silent one: the bitmap `stb_truetype` hands over is +/// already trimmed to its ink, so a cell that could not hold it would drop the +/// columns it did not fit without any error to report. +/// /// **What is rejected, and how.** `from_file()` parses and bakes first and /// installs the result only on success, so a failure leaves the object exactly as /// it was -- a host can try a file, keep the font it already had, and read why @@ -53,11 +65,17 @@ namespace plot::text { /// is `bool` plus out-params, the same one every other fallible `plot` call uses, /// and no `Result` is introduced for a font. /// -/// **Trust.** stb_truetype's own header says it does no range checking of the -/// offsets in a font file, so a file crafted to be hostile can read memory. This -/// class parses the host's fonts, not untrusted input: a *random* file is -/// rejected, a *malicious* one is out of scope, and the same is true of -/// everything else in `third_party/`. +/// **Trust, and what this class does not check.** The file is assumed to be well +/// formed. What *is* checked here, before the vendored parser sees a byte, is that +/// the input is an sfnt (or a collection) whose table directory fits inside the +/// bytes read -- because that parser's own table scan reads `numTables` records +/// whatever the file's length is (its header says so in capitals, and warns +/// against untrusted files). Nothing *inside* those tables is validated by us: +/// stb_truetype does no range checking of the offsets it finds there, so a font +/// crafted to be hostile can still read memory, and a font this class accepts is +/// not a font that has been vetted. Untrusted fonts are out of scope: bake fonts +/// you or the user trust, and treat a font from the network the way you would +/// treat any other parser input from it. /// /// **Where it lives.** `plot_core` does not provide this class: it is built by /// the optional `plot_font_ttf` target, and this header is deliberately absent @@ -90,10 +108,12 @@ class TrueTypeFont final : public render::TextBackend { /// another file. /// /// False with `*st` filled in (and `*out` untouched) for a path that is not a - /// readable regular file, a file the parser cannot accept, a non-positive or - /// non-finite `px`, and a bake in which no requested codepoint produced ink -- - /// the last is the `tools/fonts/gen_atlas.py` rule, where a blank cell that - /// nobody notices is exactly the failure this feature exists to remove. + /// readable regular file, a file the parser cannot accept, a non-positive, + /// non-finite or absurdly large `px` (an em beyond a million pixels is refused + /// as out of range, like a coverage set whose atlas would not fit -- both before + /// anything is allocated), and a bake in which no requested codepoint produced + /// ink: a bake of nothing but blank cells and the fallback box is refused rather + /// than shipped, the rule `tools/fonts/gen_atlas.py` applies at authoring time. /// `*out` must not be null. It allocates, so a failure to allocate is reported /// as `ErrorCode::OutOfMemory` rather than thrown. static bool from_file(const char* utf8_path, f32 px, TrueTypeFont* out, Status* st = nullptr); @@ -140,11 +160,15 @@ class TrueTypeFont final : public render::TextBackend { f32 em_px() const override; /// The glyph for `codepoint`: its cell as a normalised rect (the same rect for - /// every glyph -- the fixed-cell contract), its bearing into the cell -- zero, - /// because the pen starts at the cell's left edge and the ink is placed inside - /// it -- and the face's advance. Never null once a face is loaded: a codepoint - /// outside the coverage maps to the fallback box, so a missing glyph is visible - /// rather than blank. Null for a backend with no face. + /// every glyph -- the fixed-cell contract), its bearing, and the face's advance. + /// The bearing is `xoff = -shift`, zero for a face whose ink stays right of + /// every pen, and negative when some covered glyph reaches back over it -- the + /// cell corner goes at `pen + xoff`, which is what keeps that overhang inside + /// the cell instead of clipped (`yoff` is always zero: the baseline is + /// `ascent()` below the cell's top, and no glyph's ink reaches above it + /// unaccounted for). Never null once a face is loaded: a codepoint outside the + /// coverage maps to the fallback box, so a missing glyph is visible rather than + /// blank. Null for a backend with no face. const render::Glyph* glyph(u32 codepoint) const override; /// `(cell_width() / atlas_width(), 1)` -- the atlas is a single row of cells. diff --git a/src/text/true_type_font.cpp b/src/text/true_type_font.cpp index 02def20..5d285f4 100644 --- a/src/text/true_type_font.cpp +++ b/src/text/true_type_font.cpp @@ -10,11 +10,15 @@ // because the text pipeline stages one rect origin per glyph and carries the // rect's size once per draw (see render::TextBackend::cell_uv). The cell is as // wide as the widest advance *or* the widest ink's right edge -- the pen must -// never leave the cell, and neither may a glyph's ink -- and as tall as the -// face's ascent-to-descent extent, extended when a baked glyph's own bitmap box -// needs more room than the face declares. The pen starts at the cell's left edge -// and the baseline sits `ascent()` below the cell's top, so the ink is blitted at -// the offset its bitmap box reports from the pen and every rect stays one size. +// never leave the cell, and neither may a glyph's ink, in *either* direction: the +// cell also steps right of the pen by the deepest backward overhang any covered +// glyph has, because an italic glyph's ink starts left of its pen, and a cell +// whose left edge was the pen would clip that overhang away without saying so. +// The cell is as tall as the face's ascent-to-descent extent, extended when a +// baked glyph's own bitmap box needs more room than the face declares, and the +// baseline sits `ascent()` below its top, so every glyph's ink is blitted at the +// offset its bitmap box reports from the pen, into a cell it cannot leave, and +// every rect stays one size. #include @@ -46,6 +50,21 @@ constexpr f32 kFallbackInnerY0 = 0.0f; constexpr f32 kFallbackInnerX1 = 480.0f; constexpr f32 kFallbackInnerY1 = 600.0f; +/// The largest texel extent and the largest em this backend will compute with. +/// +/// Both exist to keep a float-to-integer conversion inside its target's range -- +/// [conv.fpint] makes one outside it undefined, and `from_file()` accepts any +/// positive `px`, so without a bound a caller could hand the bake a number whose +/// floor or ceiling does not fit in the `int`/`u32` the cell is computed in. The +/// em ceiling is the tighter one on purpose: it also keeps the *vendored* +/// rasteriser's own pixel arithmetic inside `int`, since stb scales a glyph's +/// outline by the same factor and narrows it to pixels itself. A million-pixel em +/// is already a thousand times any legible bake, so nothing real is refused: an +/// over-sized request still gets a `Status`, from the atlas-size check rather than +/// from here (see `bake_face`). +constexpr f32 kMaxTexels = 1.0e9f; +constexpr f32 kMaxEmPx = 1.0e7f; + /// A codepoint the face can draw, with the advance the pen moves by: pass one of /// a bake, whose only job is to size the cell every one of them is drawn into. struct Drawable { @@ -153,9 +172,13 @@ u32 blit_cell(std::vector& atlas, const Bake& bake, u32 slot, const u8* bitm /// Draws the fallback box into `slot`'s cell: the outline between the outer and /// inner rectangles, roughly one texel thick at the bake's scale. It is geometry, /// not a rasterised glyph, so the box is there for a face with no usable -/// `.notdef` too. -void blit_fallback_box(std::vector& atlas, const Bake& bake, u32 slot, f32 scale) noexcept { - const auto to_x = [scale](f32 units) { return static_cast(std::lround(units * scale)); }; +/// `.notdef` too. It is placed in the same coordinates the glyphs are -- font +/// units from the pen, into a cell that sits `shift` texels right of it. +void blit_fallback_box(std::vector& atlas, const Bake& bake, u32 slot, f32 scale, + u32 shift) noexcept { + const auto to_x = [scale, shift](f32 units) { + return static_cast(std::lround(units * scale)) + static_cast(shift); + }; const auto to_y = [&bake, scale](f32 units) { return bake.baseline - static_cast(std::lround(units * scale)); }; @@ -212,6 +235,7 @@ bool bake_face(const stbtt_fontinfo& info, f32 scale, std::span cover f32 widest_advance = 0.0f; i32 highest = 0; i32 lowest = 0; + i32 leftmost = 0; i32 rightmost = 0; for (const u32 codepoint : requested) { if (codepoint > 0x10FFFFu) { @@ -233,6 +257,7 @@ bool bake_face(const stbtt_fontinfo& info, f32 scale, std::span cover widest_advance = std::max(widest_advance, advance); highest = std::min(highest, y0); lowest = std::max(lowest, y1); + leftmost = std::min(leftmost, x0); rightmost = std::max(rightmost, x1); drawable.push_back(Drawable{codepoint, advance}); } @@ -241,16 +266,26 @@ bool bake_face(const stbtt_fontinfo& info, f32 scale, std::span cover // generator's rule -- an overhanging glyph stays inside its own cell instead of // reaching into the next one), as tall as the face's ascent-to-descent extent, // and never so short that a baked bitmap box hangs out of it. Each extent is - // clamped before it is narrowed to texels: a float-to-unsigned conversion of a - // negative value is undefined, and a face whose metrics or ink are degenerate - // enough to produce one must land on a one-texel cell, not on undefined - // behaviour. + // clamped at both ends before it is narrowed to texels: a float-to-integer + // conversion outside the target type's range is undefined, and neither a + // degenerate face nor an absurd em size may reach that -- they land on a + // one-texel cell or on the atlas-size refusal below instead. const auto ceil_to_texels = [](f32 value) { - return static_cast(std::ceil(std::max(0.0f, value - 1e-4f))); + return static_cast(std::ceil(std::min(std::max(0.0f, value - 1e-4f), kMaxTexels))); }; - const u32 cell_w = - std::max(1u, ceil_to_texels(std::max(widest_advance, static_cast(rightmost)))); - i32 baseline = static_cast(std::floor(ascent_f)); + // The cell sits `shift` texels right of the pen, because the pen is where the + // *advance* starts and not where the ink does: an italic glyph reaches left of + // its pen (`U+006A` in half the italic faces on a stock Linux box), and a cell + // whose left edge was the pen would drop that overhang -- silently, since the + // trimmed bitmap stb returns is exactly what would be clipped. Widening the + // same way the right edge is widened is what makes the rect uniform *and* + // lossless; `-shift` is then the non-positive bearing `glyph()` reports, and + // the pipeline places the cell corner at the pen plus that. + const u32 shift = + leftmost < 0 ? static_cast(-static_cast(leftmost)) : 0u; + const u32 cell_w = std::max( + 1u, shift + ceil_to_texels(std::max(widest_advance, static_cast(rightmost)))); + i32 baseline = static_cast(std::floor(std::min(ascent_f, kMaxTexels))); baseline = std::max(baseline, -highest); // room for the tallest ink above the pen baseline = std::max(baseline, 0); const u32 below = static_cast(std::max(0, baseline + lowest)); @@ -288,7 +323,7 @@ bool bake_face(const stbtt_fontinfo& info, f32 scale, std::span cover const f32 dv = static_cast(cell_h) / static_cast(out->atlas_h); const auto glyph_for_slot = [&](u32 slot, f32 advance) { const f32 u0 = static_cast(slot * cell_w) / static_cast(out->atlas_w); - return render::Glyph{u0, 0.0f, u0 + du, dv, 0.0f, 0.0f, advance}; + return render::Glyph{u0, 0.0f, u0 + du, dv, -static_cast(shift), 0.0f, advance}; }; // Pass two: rasterise and blit. The offset `stbtt_GetCodepointBitmap` reports is @@ -304,8 +339,8 @@ bool bake_face(const stbtt_fontinfo& info, f32 scale, std::span cover u8* bitmap = stbtt_GetCodepointBitmap(&info, scale, scale, static_cast(glyph.codepoint), &width, &height, &x, &y); if (bitmap != nullptr) { - inked += blit_cell(out->atlas, *out, static_cast(i), bitmap, width, height, x, - baseline + y); + inked += blit_cell(out->atlas, *out, static_cast(i), bitmap, width, height, + x + static_cast(shift), baseline + y); stbtt_FreeBitmap(bitmap, nullptr); } out->codepoints.push_back(glyph.codepoint); @@ -321,7 +356,7 @@ bool bake_face(const stbtt_fontinfo& info, f32 scale, std::span cover // The fallback slot, last: the embedded face's box, with the widest advance so a // run of missing glyphs takes the room of a real one. const u32 fallback_slot = static_cast(drawable.size()); - blit_fallback_box(out->atlas, *out, fallback_slot, scale); + blit_fallback_box(out->atlas, *out, fallback_slot, scale, shift); out->fallback = glyph_for_slot(fallback_slot, widest_advance); return true; } @@ -380,6 +415,14 @@ bool TrueTypeFont::from_file(const char* utf8_path, f32 px, TrueTypeFont* out, S return fail(st, ErrorCode::InvalidArgument, "a font em size must be a positive, finite number of pixels"); } + if (px > kMaxEmPx) { + // Not merely pedantry: the rasteriser scales outlines by this number and + // narrows them to pixels itself, so an em past this bound would overflow + // arithmetic this backend does not control. A size this large is not a font + // size, and saying so beats undefined behaviour (see kMaxEmPx). + return fail(st, ErrorCode::InvalidArgument, + "the font em size is larger than this backend will rasterise"); + } std::error_code ec; const std::filesystem::path path{utf8_path}; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 57dc2cd..0883058 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -91,6 +91,16 @@ if(TARGET plot::font_ttf) target_compile_definitions(plot_tests PRIVATE PLOT_CORE_ARCHIVE="$" PLOT_CMAKE_COMMAND="${CMAKE_COMMAND}") + # The cases compare a bake against the rasteriser's own glyph bitmaps, which is + # the only reference that can tell a complete cell from a clipped one, and for + # that the test needs the vendored header's *declarations* (its implementation is + # already in the binary through plot::font_ttf). This is the optional target's + # own test, compiled only when that target exists, and it is the one target + # outside src/text that sees third_party/: the isolation this project promises -- + # plot_core's archive and every public header under include/plot/ -- is what + # plot_core_stays_free_of_stb_truetype keeps asserting, and it is unaffected by + # which include paths a test binary has. + target_include_directories(plot_tests PRIVATE "${CMAKE_SOURCE_DIR}/third_party") endif() # Same switches the library is built with: the cost tests read the counters diff --git a/tests/test_true_type_font.cpp b/tests/test_true_type_font.cpp index d7d3756..a0e21ce 100644 --- a/tests/test_true_type_font.cpp +++ b/tests/test_true_type_font.cpp @@ -24,6 +24,13 @@ #include #include +// Declarations only, for the reference the bake is measured against: the vendored +// rasteriser's own glyph bitmaps and boxes. Its implementation is already in this +// binary -- plot_tests links plot::font_ttf, which compiles it in -- so this is a +// call, not a second copy of third-party code (see tests/CMakeLists.txt for why +// this one target has third_party/ on its include path). +#include + #include #include #include @@ -45,6 +52,21 @@ namespace { /// equal terms. constexpr const char* kAdwaitaMono = "/usr/share/fonts/Adwaita/AdwaitaMono-Regular.ttf"; +/// Italic faces to look for, in the order the case prefers them: every one of +/// these leaves some glyph's ink *left* of its pen (an oblique or italic design +/// slants the outline over the advance that starts at the pen), which is the shape +/// the cell's left edge has to accommodate. Distro paths differ, so the list names +/// the layouts of both the Debian-style and the Arch-style font trees. +constexpr const char* kItalicCandidates[] = { + "/usr/share/fonts/TTF/DejaVuSerif-Italic.ttf", + "/usr/share/fonts/truetype/dejavu/DejaVuSerif-Italic.ttf", + "/usr/share/fonts/TTF/TypesetterSlab-Italic.ttf", + "/usr/share/fonts/gsfonts/NimbusRoman-Italic.otf", + "/usr/share/fonts/opentype/urw-base35/NimbusRoman-Italic.otf", + "/usr/share/fonts/liberation/LiberationSerif-Italic.ttf", + "/usr/share/fonts/TTF/LiberationSerif-Italic.ttf", +}; + /// The viewport the renderer case draws into. Smaller than the smoke suite's 640 /// x 360 because every assertion there scans pixels one `glReadPixels` at a time. constexpr plot::u32 kW = 360; @@ -92,6 +114,161 @@ plot::u32 slot_of(const plot::text::TrueTypeFont& font, const plot::render::Glyp return static_cast(std::lround(cells)); } +/// The vendored rasteriser, driven by the test the way the backend drives it, so +/// a bake can be compared against the glyphs it was *supposed* to produce. +/// +/// This is the only reference that can tell a complete cell from a clipped one: +/// the atlas is self-consistent either way, so a cell that quietly lost its +/// leftmost ink column looks exactly like a cell that never had one. Nothing about +/// the cell's size or placement is recomputed here -- that would only repeat the +/// arithmetic under test -- the reference is stb_truetype's own bitmap and box. +class ReferenceRasteriser { + public: + /// Reads `path` and parses it. False (with the case's skip reason printed by the + /// caller) when the file is missing or the parser refuses it. + bool open(const char* path) { + std::ifstream in(path, std::ios::binary); + if (!in) { + return false; + } + bytes_.assign(std::istreambuf_iterator(in), std::istreambuf_iterator()); + if (bytes_.empty()) { + return false; + } + const int offset = stbtt_GetFontOffsetForIndex(bytes_.data(), 0); + return offset >= 0 && stbtt_InitFont(&info_, bytes_.data(), offset) != 0; + } + + /// The scale the backend uses for `px` em, which is the rasteriser's own. + float scale(f32 px) const { return stbtt_ScaleForPixelHeight(&info_, px); } + + /// The ink bitmap and box of `codepoint` at `scale`: the bytes the cell must + /// hold, and where, in pixels from the pen. The caller frees it. + plot::u8* bitmap(u32 codepoint, float scale, plot::i32* width, plot::i32* height, plot::i32* x, plot::i32* y) const { + int w = 0; + int h = 0; + int bx = 0; + int by = 0; + plot::u8* pixels = stbtt_GetCodepointBitmap(&info_, scale, scale, static_cast(codepoint), &w, &h, + &bx, &by); + *width = w; + *height = h; + *x = bx; + *y = by; + return pixels; + } + + /// The ink box stb reports for `codepoint`: the *geometric* box, which is what + /// the bake sizes its cells from and is worth more than the bitmap here -- a + /// bitmap cannot say whether a codepoint has ink left of its pen. + void box(u32 codepoint, float scale, plot::i32* x0, plot::i32* y0, plot::i32* x1, plot::i32* y1) const { + int ix0 = 0; + int iy0 = 0; + int ix1 = 0; + int iy1 = 0; + stbtt_GetCodepointBitmapBox(&info_, static_cast(codepoint), scale, scale, &ix0, &iy0, + &ix1, &iy1); + *x0 = ix0; + *y0 = iy0; + *x1 = ix1; + *y1 = iy1; + } + + bool has_glyph(u32 codepoint) const { + return stbtt_FindGlyphIndex(&info_, static_cast(codepoint)) != 0; + } + + private: + std::vector bytes_; + stbtt_fontinfo info_{}; +}; + +/// False, with `why` filled in, unless `codepoint`'s cell holds *exactly* the +/// bitmap the rasteriser produced for it, placed where the glyph's bearing puts +/// the pen: bit for bit, texel for texel, with no texel of the bitmap left outside +/// the cell. A column dropped because the cell could not hold it (the negative +/// bearing an italic glyph has), ink placed at the wrong offset, or a bake that +/// lost its shape, all fail here, and none of them is visible to an assertion +/// about ink being *present*. +bool cell_holds_reference(const plot::text::TrueTypeFont& font, const ReferenceRasteriser& face, + float scale, plot::u32 codepoint, std::string* why) { + const plot::render::Glyph* glyph = font.glyph(codepoint); + if (glyph == nullptr) { + *why = "no glyph"; + return false; + } + plot::i32 width = 0; + plot::i32 height = 0; + plot::i32 ink_x = 0; + plot::i32 ink_y = 0; + plot::u8* bitmap = face.bitmap(codepoint, scale, &width, &height, &ink_x, &ink_y); + if (bitmap == nullptr) { + return true; // no ink to compare: a blank cell is the whole truth here + } + const plot::u8* atlas = font.atlas_pixels(); + const plot::u32 cell_w = font.cell_width(); + const plot::u32 cell_h = font.cell_height(); + const plot::u32 atlas_w = font.atlas_width(); + const plot::u32 slot = slot_of(font, *glyph); + // Where the cell's own coordinates put the pen: the bearing is the cell corner's + // offset from the pen, so the pen is that much *into* the cell. + const plot::i32 pen_x = static_cast(std::lround(-static_cast(glyph->xoff))); + const plot::i32 pen_y = static_cast(std::lround(static_cast(font.ascent()))); + bool ok = true; + for (plot::i32 row = 0; row < height && ok; ++row) { + for (plot::i32 column = 0; column < width; ++column) { + const plot::i32 cell_x = pen_x + ink_x + column; + const plot::i32 cell_y = pen_y + ink_y + row; + plot::u8 held = 0; + if (cell_x >= 0 && cell_y >= 0 && cell_x < static_cast(cell_w) && + cell_y < static_cast(cell_h)) { + held = atlas[static_cast(cell_y) * atlas_w + + static_cast(slot) * cell_w + static_cast(cell_x)]; + } + const plot::u8 want = + bitmap[static_cast(row) * static_cast(width) + + static_cast(column)]; + if (held == want) { + continue; + } + *why = "U+" + std::to_string(codepoint) + " bitmap texel (column " + std::to_string(column) + + ", row " + std::to_string(row) + ") is " + std::to_string(want) + + " but its cell holds " + std::to_string(held) + + (held == 0 && want != 0 ? " -- the ink was dropped" : " -- the ink is misplaced"); + ok = false; + break; + } + } + stbtt_FreeBitmap(bitmap, nullptr); + return ok; +} + +/// `cell_holds_reference`, as a case asserts it: the diagnostic the reference +/// produced is the failure message, because "which texel of which codepoint" is +/// the whole value of the check. +void expect_cell_holds_reference(const plot::text::TrueTypeFont& font, + const ReferenceRasteriser& face, float scale, + plot::u32 codepoint) { + std::string why; + if (!cell_holds_reference(font, face, scale, codepoint, &why)) { + throw ::plottest::Failure{why}; + } +} + +/// The first italic face from `kItalicCandidates` that is installed and parses, +/// with `face` opened on it. Null when none is: the case that wants one is about a +/// shape only an italic face has, and says so rather than passing on a face that +/// does not. +const char* open_an_italic_face(ReferenceRasteriser* face) { + for (const char* candidate : kItalicCandidates) { + std::error_code ec; + if (std::filesystem::is_regular_file(candidate, ec) && face->open(candidate)) { + return candidate; + } + } + return nullptr; +} + /// A cheap order-sensitive signature of the ink in `area`: two frames that drew /// the same thing hash the same, and a frame whose glyphs came from another atlas /// -- or from another cell of the same atlas -- does not. The position is folded @@ -242,6 +419,18 @@ PLOT_TEST(ttf_backend_bakes_the_requested_coverage) { } } + // And the cells hold the glyphs the rasteriser produced, whole: for every + // codepoint, the bitmap stb_truetype hands back -- itself trimmed to its ink, so + // it is the one artefact that knows what the cell must contain -- must be in the + // cell texel for texel, at the offset the glyph's bearing names. This is the + // assertion that catches ink a cell boundary dropped, which no assertion about + // ink being *present* can see. + ReferenceRasteriser face; + PLOT_CHECK(face.open(kAdwaitaMono)); + for (const plot::u32 codepoint : coverage) { + expect_cell_holds_reference(font, face, face.scale(px), codepoint); + } + // A codepoint outside the coverage is not blank: it draws the fallback box, and // it draws it as the same cell as every other glyph. const plot::render::Glyph* fallback = font.glyph(kUncovered); @@ -260,13 +449,62 @@ PLOT_TEST(ttf_backend_bakes_the_requested_coverage) { } // Every way a font can fail to arrive: a path that is not there, one that is a -// directory, a file that is not a font, an em size that is not positive, and a -// bake whose coverage came out blank. Each reports through a `Status`, each -// leaves the object it was handed exactly as it was -- the atlas, the cell, the -// revision and every glyph pointer -- and none of them takes the process with it. +// directory, a file that is not a font, an em size that is not positive or is +// absurd, and a bake whose coverage came out blank. Each reports through a +// `Status`, each leaves the object it was handed exactly as it was, and none of +// them takes the process with it. +// +// It has two halves on purpose. The rejections that need no face run against a +// default-constructed backend, whose untouched state is the fontless one -- so +// they are exercised on a machine with no system face at all, which is a machine +// this project supports. The stronger half -- a *loaded* font, with an atlas, a +// cell and glyph pointers that must survive a refused re-point -- needs a face, and +// says so by itself rather than taking the other half's coverage down with it. PLOT_TEST(ttf_failure_paths_report_status) { + // --- without a face: what "untouched" means for an object with nothing to lose + plot::text::TrueTypeFont fontless; + const auto still_fontless = [&] { + return fontless.glyph('A') == nullptr && fontless.glyph(plot::render::kInvalidCodepoint) == + nullptr && + fontless.atlas_pixels() == nullptr && fontless.atlas_width() == 0u && + fontless.atlas_height() == 0u && fontless.cell_width() == 0u && + fontless.cell_height() == 0u && fontless.glyph_count() == 0u && + fontless.revision() == 0u && fontless.em_px() == 0.0f && + fontless.measure("A") == 0.0f; + }; + PLOT_CHECK(still_fontless()); + const auto refuses = [&](const char* what, const char* path, plot::f32 px) { + plot::Status failure; + PLOT_CHECK(!plot::text::TrueTypeFont::from_file(path, px, &fontless, &failure)); + PLOT_CHECK(!static_cast(failure)); + PLOT_CHECK(failure.code != plot::ErrorCode::Ok); + PLOT_CHECK(failure.message != nullptr && failure.message[0] != '\0'); + PLOT_CHECK(still_fontless()); + std::printf("[fail-%s] %s\n", what, failure.message); + }; + + refuses("missing", "/nonexistent/plot/no-such-font.ttf", 20.0f); + refuses("directory", "/usr/share/fonts", 20.0f); + // A real text file from this repository: it exists, it reads, and it is not a + // font -- the case for a file the parser cannot accept. + refuses("not-a-font", PLOT_SOURCE_DIR "/CMakeLists.txt", 20.0f); + refuses("zero-px", kAdwaitaMono, 0.0f); + refuses("negative-px", kAdwaitaMono, -3.0f); + // Past the em ceiling the rasteriser's own pixel arithmetic would leave `int`, + // so it is refused as out of range before it is scaled by anything. + refuses("beyond-the-em-ceiling", kAdwaitaMono, 1.0e8f); + // Nowhere to put the font is the caller's own mistake, refused before the path is + // even looked at -- which is why a path that does not exist still serves here. + plot::Status no_destination; + PLOT_CHECK(!plot::text::TrueTypeFont::from_file("/nonexistent/plot/no-such-font.ttf", 20.0f, + nullptr, &no_destination)); + PLOT_CHECK(!static_cast(no_destination)); + PLOT_CHECK(still_fontless()); + + // --- with a face: the state that must survive is a real one if (!have_adwaita_mono()) { - skip("ttf_failure_paths_report_status", "no system Adwaita Mono to bake"); + skip("ttf_failure_paths_report_status", + "no system Adwaita Mono, so the loaded-state half did not run"); return; } plot::text::TrueTypeFont font; @@ -282,35 +520,127 @@ PLOT_TEST(ttf_failure_paths_report_status) { font.atlas_pixels() == atlas && font.cell_width() == cell_w && font.glyph('A') == glyph; }; - - const auto rejects = [&](const char* what, const char* path, plot::f32 px) { + const auto survives = [&](const char* what, const char* path, plot::f32 px) { plot::Status failure; PLOT_CHECK(!plot::text::TrueTypeFont::from_file(path, px, &font, &failure)); PLOT_CHECK(!static_cast(failure)); - PLOT_CHECK(failure.code != plot::ErrorCode::Ok); - PLOT_CHECK(failure.message != nullptr && failure.message[0] != '\0'); PLOT_CHECK(unchanged()); - std::printf("[fail-%s] %s\n", what, failure.message); + std::printf("[fail-with-a-face-%s] %s\n", what, failure.message); }; - rejects("missing", "/nonexistent/plot/no-such-font.ttf", 20.0f); - rejects("directory", "/usr/share/fonts", 20.0f); - // A real text file from this repository: it exists, it reads, and it is not a - // font -- the case for a file the parser cannot accept. - rejects("not-a-font", PLOT_SOURCE_DIR "/CMakeLists.txt", 20.0f); - rejects("zero-px", kAdwaitaMono, 0.0f); - rejects("negative-px", kAdwaitaMono, -3.0f); + survives("missing", "/nonexistent/plot/no-such-font.ttf", 20.0f); + survives("directory", "/usr/share/fonts", 20.0f); + survives("not-a-font", PLOT_SOURCE_DIR "/CMakeLists.txt", 20.0f); + survives("zero-px", kAdwaitaMono, 0.0f); + survives("negative-px", kAdwaitaMono, -3.0f); // Positive, but far below one texel of ink: the bake itself is what fails here, // which is the "a bake that produced no ink" path rather than a bad argument. - rejects("blank-bake", kAdwaitaMono, 0.1f); - - // And the one failure that is the caller's own: nowhere to put the font. - plot::Status no_destination; - PLOT_CHECK(!plot::text::TrueTypeFont::from_file(kAdwaitaMono, 20.0f, nullptr, &no_destination)); - PLOT_CHECK(!static_cast(no_destination)); + survives("blank-bake", kAdwaitaMono, 0.1f); + PLOT_CHECK(!plot::text::TrueTypeFont::from_file(kAdwaitaMono, 20.0f, nullptr, &st)); PLOT_CHECK(unchanged()); } +// A face whose ink reaches *left* of the pen is the case a cell that started at +// the pen cannot hold: an italic or oblique design slants its outlines over the +// advance, so some glyphs' bitmap boxes begin at a negative x, and a bake that +// placed such a bitmap at the pen would drop the columns that fell outside -- +// silently, because the bitmap the rasteriser hands over is already trimmed to its +// ink, so a lost column looks exactly like a column that was never there. +// +// Two things are asserted, and both fail against a bake without the left widening: +// every cell holds its glyph whole (bit for bit, against the rasteriser's own +// bitmap), and the bearings say so -- a negative `xoff` is how the pipeline knows +// to place the cell corner right of the pen. Which codepoints count as "left of +// the pen" is decided by the *rasteriser's* boxes, not by the bake's bearings: +// asking the code under test whether there was ink to clip would let a bake that +// clipped it also claim there was nothing to clip. +PLOT_TEST(ttf_negative_bearing_glyphs_keep_their_ink) { + constexpr const char* kCase = "ttf_negative_bearing_glyphs_keep_their_ink"; + ReferenceRasteriser face; + const char* path = open_an_italic_face(&face); + if (path == nullptr) { + skip(kCase, "no italic face from the candidate list is installed"); + return; + } + const plot::f32 px = 20.0f; + plot::text::TrueTypeFont font; + plot::Status st; + PLOT_CHECK(plot::text::TrueTypeFont::from_file(path, px, &font, &st)); + const float scale = face.scale(px); + + // Every covered codepoint, whole -- the same reference check the other cases + // make, on a face where a cell boundary has something to drop. + for (const plot::u32 codepoint : plot::text::TrueTypeFont::default_coverage()) { + expect_cell_holds_reference(font, face, scale, codepoint); + } + + plot::u32 left_of_the_pen = 0; + for (const plot::u32 codepoint : plot::text::TrueTypeFont::default_coverage()) { + if (!face.has_glyph(codepoint)) { + continue; + } + plot::i32 x0 = 0; + plot::i32 y0 = 0; + plot::i32 x1 = 0; + plot::i32 y1 = 0; + face.box(codepoint, scale, &x0, &y0, &x1, &y1); + if (x0 >= 0) { + continue; + } + ++left_of_the_pen; + const plot::render::Glyph* glyph = font.glyph(codepoint); + PLOT_CHECK(glyph != nullptr); + PLOT_CHECK(glyph->xoff < 0.0f); // the cell corner is right of the pen + // ... by at least the overhang the rasteriser reports, so no ink is left outside. + PLOT_CHECK(static_cast(std::lround(-static_cast(glyph->xoff))) >= -x0); + // And the cell still reaches past the advance, not into it: the pen's own step + // must stay inside the cell the rect declares. + PLOT_CHECK(static_cast(font.cell_width()) + glyph->xoff >= glyph->advance); + } + if (left_of_the_pen == 0u) { + skip(kCase, "this italic face draws no covered glyph left of its pen"); + return; + } + std::printf("[italic] %s: %u covered codepoints have ink left of the pen\n", path, + left_of_the_pen); +} + +// The two bounds on a bake's size, both of which refuse *before* allocating: an +// atlas that would not fit is a bad argument rather than an allocation failure, +// and an em past what this backend will rasterise is refused rather than narrowed +// into an integer it does not fit (see kMaxEmPx in src/text/true_type_font.cpp). +// The `InvalidArgument` code is what pins the order -- a bake that got as far as +// allocating would report `OutOfMemory` instead. +PLOT_TEST(ttf_refuses_a_bake_it_cannot_hold) { + constexpr const char* kCase = "ttf_refuses_a_bake_it_cannot_hold"; + if (!have_adwaita_mono()) { + skip(kCase, "no system Adwaita Mono to bake"); + return; + } + plot::text::TrueTypeFont font; + plot::Status st; + // A million-pixel em is a legal size and an impossible atlas: 101 cells of about + // 5.1e5 x 1e6 texels. Refused as an argument, not as 5.1e13 bytes that failed to + // allocate. + PLOT_CHECK(!plot::text::TrueTypeFont::from_file(kAdwaitaMono, 1.0e6f, &font, &st)); + PLOT_CHECK(st.code == plot::ErrorCode::InvalidArgument); + PLOT_CHECK(!static_cast(st)); + PLOT_CHECK(font.atlas_pixels() == nullptr); + PLOT_CHECK(font.glyph_count() == 0u); + std::printf("[bounds-1000000px] %s\n", st.message); + // Past the em ceiling, before any of that arithmetic runs. + PLOT_CHECK(!plot::text::TrueTypeFont::from_file(kAdwaitaMono, 1.0e8f, &font, &st)); + PLOT_CHECK(st.code == plot::ErrorCode::InvalidArgument); + std::printf("[bounds-100000000px] %s\n", st.message); + // And neither bound is over-eager: the sizes a plot really draws at still bake, + // including one well past the embedded face's 13 px and the 20 px this suite + // bakes elsewhere. + PLOT_CHECK(plot::text::TrueTypeFont::from_file(kAdwaitaMono, 96.0f, &font, &st)); + PLOT_CHECK(font.cell_width() > 0u); + PLOT_CHECK(font.glyph_count() == 101u); + PLOT_CHECK(font.atlas_pixels() != nullptr); +} + // A renderer draws with the backend it was given, and re-uploads when that // backend re-bakes. The same plot is drawn four times: with the embedded face, // with the TrueType bake at 20 px (a size the embedded bake does not have), with