From 1b153b2ec33a47c334cf556aa3c5acc4b897d06d Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Mon, 28 Sep 2026 16:13:56 +0200 Subject: [PATCH] fix(text): refuse an atlas that would overflow its own arithmetic, and clamp the cell before narrowing it --- include/plot/text/true_type_font.hpp | 5 ++++ src/text/true_type_font.cpp | 34 +++++++++++++++++++++------- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/include/plot/text/true_type_font.hpp b/include/plot/text/true_type_font.hpp index 0e66831..0d7f598 100644 --- a/include/plot/text/true_type_font.hpp +++ b/include/plot/text/true_type_font.hpp @@ -104,6 +104,11 @@ class TrueTypeFont final : public render::TextBackend { /// cell: the draw then falls back to the box, which is visible, where a blank /// cell would silently render nothing. /// + /// A re-bake *replaces* the glyph table, so the pointers `glyph()` handed out + /// before it must not be held across it -- the same rule as `from_file()` on + /// this object, and the reason `set_text_backend()` is how a swap is meant to + /// be announced rather than a long-lived pointer kept somewhere. + /// /// A re-bake that produces no ink leaves the previous atlas in place -- there /// is no return value to report through, and losing a working font to a bad /// argument is worse than ignoring it. On a backend with no face this does diff --git a/src/text/true_type_font.cpp b/src/text/true_type_font.cpp index 65304cb..02def20 100644 --- a/src/text/true_type_font.cpp +++ b/src/text/true_type_font.cpp @@ -240,22 +240,40 @@ bool bake_face(const stbtt_fontinfo& info, f32 scale, std::span cover // The cell: wide enough for the widest advance *and* the widest ink (the // 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. - const u32 cell_w = std::max( - 1u, - static_cast(std::ceil(std::max(widest_advance, static_cast(rightmost)) - 1e-4f))); + // 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. + const auto ceil_to_texels = [](f32 value) { + return static_cast(std::ceil(std::max(0.0f, value - 1e-4f))); + }; + 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)); baseline = std::max(baseline, -highest); // room for the tallest ink above the pen baseline = std::max(baseline, 0); - const u32 extent = static_cast(std::ceil(ascent_f + descent_f - 1e-4f)); const u32 below = static_cast(std::max(0, baseline + lowest)); - const u32 cell_h = std::max(1u, std::max(extent, below)); + const u32 cell_h = std::max(1u, std::max(ceil_to_texels(ascent_f + descent_f), below)); + + // The atlas' width is where a caller's coverage list becomes a buffer size, so + // the product is formed in 64 bits and checked before it is narrowed: a set + // large enough to wrap it would otherwise allocate one buffer and index another, + // which is the one arithmetic error in this file that a host could reach by + // asking for too much rather than by handing over a broken font. + const u32 slots = static_cast(drawable.size()) + 1u; // the fallback box is the last slot + const u64 atlas_w = static_cast(cell_w) * static_cast(slots); + constexpr u64 kMaxAtlasTexels = 1ull << 30; // a gibibyte of R8 coverage + if (atlas_w > static_cast(0xFFFFFFFFu) || + atlas_w * static_cast(cell_h) > kMaxAtlasTexels) { + return fail(st, ErrorCode::InvalidArgument, + "the requested coverage needs an atlas larger than this backend will hold"); + } out->cell_w = cell_w; out->cell_h = cell_h; out->baseline = baseline; - const u32 slots = static_cast(drawable.size()) + 1u; // the fallback box is the last slot - out->atlas_w = cell_w * slots; + out->atlas_w = static_cast(atlas_w); out->atlas_h = cell_h; out->atlas.assign(static_cast(out->atlas_w) * out->atlas_h, 0u); out->codepoints.clear();