refactor(render): the atlas is a declared fixed-cell grid, and staging checks it

The text pipeline stages a glyph's rect *origin* per instance and carries the
rect's *size* once per draw, so a backend whose glyphs have differing rect sizes
-- a TrueType packing trimmed to ink, which is Task 8's shape -- would have drawn
every glyph at whichever size it happened to be told, with nothing downstream
able to tell. Make that constraint structural rather than documented:

- TextBackend::cell_uv() declares the one cell size, and the contract (an atlas
  is a fixed-cell grid; a proportional face draws its glyphs *into* a uniform
  cell instead of trimming each rect to its ink, and why the pipeline needs it)
  is stated on the atlas accessors.
- glyph_is_cell() is the check; add_text()/set_cell() declare the cell from the
  backend, and the staging path asserts it for every glyph in a debug build.
  A release build draws on with the cell it was told, as before.
- Renderer::flush_text() takes u_glyph_uv from the backend, not from the batch,
  so the draw's one rect size can never come from whichever glyph was staged
  first.

tests/test_gl_context.cpp

@@ -384,7 +384,7 @@ PLOT_TEST(text_pipeline_draws_a_glyph) {
   program.bind();
   program.set_viewport({static_cast<f32>(ctx.width), static_cast<f32>(ctx.height)});
   program.set_px_range({static_cast<f32>(BitmapFont::kGlyphPx), static_cast<f32>(BitmapFont::kGlyphPx)});
-  program.set_glyph_uv(batch.glyph_cell_uv());
+  program.set_glyph_uv(font.cell_uv());
   program.set_texture_size(atlas.texel_size());
   program.set_color(plot::Color::hex(0xFF00FF));
   atlas.bind();

tests/test_primitives.cpp

@@ -328,3 +328,159 @@ PLOT_TEST(text_batch_advances_along_its_basis) {
   PLOT_CHECK_NEAR(b.data()[4], 0.6, 1e-5);
   PLOT_CHECK_NEAR(b.data()[5], 0.8, 1e-5);
 }
+
+/// A backend whose atlas is *not* a fixed-cell grid: it declares the cell its
+/// pipeline needs and then publishes one glyph trimmed to its ink, which is what
+/// a naive TrueType packing produces (`stbtt_GetCodepointBitmap` hands back an ink
+/// box, not a cell). The pipeline cannot draw that glyph correctly -- a staged
+/// instance carries the rect's origin, and the draw carries one size for the whole
+/// run -- so the staging path asserts `glyph_is_cell()` for every glyph it stages.
+class TrimmedCellBackend final : public plot::render::TextBackend {
+ public:
+  plot::f32 measure(const char*) const override { return 8.0f; }
+  plot::f32 height() const override { return 8.0f; }
+  plot::f32 ascent() const override { return 8.0f; }
+  plot::Vec2 cell_uv() const override { return plot::Vec2{0.5f, 0.25f}; }
+  plot::u32 revision() const override { return 1u; }  // baked once, never re-baked
+  const plot::render::Glyph* glyph(plot::u32 codepoint) const override {
+    // 'i' is trimmed to half a cell wide, as an ink box would be.
+    return codepoint == static_cast<plot::u32>('i') ? &trimmed_ : &cell_;
+  }
+  const plot::u8* atlas_pixels() const override { return atlas_; }
+  plot::u32 atlas_width() const override { return 8u; }
+  plot::u32 atlas_height() const override { return 4u; }
+
+ private:
+  // Two cells of a 2x2 grid of 0.5 x 0.25, one of them trimmed to the ink.
+  const plot::render::Glyph cell_{0.0f, 0.0f, 0.5f, 0.25f, 0.0f, 0.0f, 4.0f};
+  const plot::render::Glyph trimmed_{0.5f, 0.0f, 0.6875f, 0.25f, 0.0f, 0.0f, 4.0f};
+  const plot::u8 atlas_[8 * 4] = {};
+};
+
+// The fixed-cell contract, and the check that enforces it. A backend that trims
+// each atlas rect to its ink -- the shape a TrueType packing produces, and the
+// reason the contract is stated on `TextBackend::cell_uv()` -- publishes glyphs
+// the pipeline would draw at the wrong size: the instance carries the rect's
+// *origin* only, so a 3/8-cell rect would be read as a half cell, past its own
+// rows.
+//
+// This drives the predicate the staging path asserts, rather than staging the
+// offending glyph, because `add_text()` with this backend aborts the test binary
+// in a debug build -- which is the point of the assertion.
+PLOT_TEST(a_trimmed_glyph_is_not_the_atlas_cell) {
+  TrimmedCellBackend backend;
+  const plot::Vec2 cell = backend.cell_uv();
+  PLOT_CHECK_NEAR(cell.x, 0.5, 1e-6);
+  PLOT_CHECK_NEAR(cell.y, 0.25, 1e-6);
+
+  // The glyph that occupies a whole cell passes ...
+  PLOT_CHECK(plot::render::glyph_is_cell(*backend.glyph(static_cast<plot::u32>('A')), cell));
+  // ... and the one trimmed to its ink does not: this is what the staging path
+  // asserts in a debug build.
+  PLOT_CHECK(!plot::render::glyph_is_cell(*backend.glyph(static_cast<plot::u32>('i')), cell));
+
+  // The check is per axis and per glyph, not a single "is it square" test: a rect
+  // that matches on one axis and not the other is still not the cell.
+  plot::render::Glyph wide = *backend.glyph(static_cast<plot::u32>('A'));
+  wide.u1 = wide.u0 + 0.75f;
+  PLOT_CHECK(!plot::render::glyph_is_cell(wide, cell));
+  plot::render::Glyph tall = *backend.glyph(static_cast<plot::u32>('A'));
+  tall.v1 = tall.v0 + 0.5f;
+  PLOT_CHECK(!plot::render::glyph_is_cell(tall, cell));
+
+  // Rounding a correct cell through the atlas arithmetic twice is not a failure:
+  // a rect is a subtraction of two rounded fractions, and the tolerance is two
+  // orders of magnitude above that slop and below one texel.
+  plot::render::Glyph rounded = *backend.glyph(static_cast<plot::u32>('A'));
+  rounded.u1 = rounded.u0 + cell.x + 1e-7f;
+  PLOT_CHECK(plot::render::glyph_is_cell(rounded, cell));
+
+  // A real font's grid holds for every glyph it publishes, not just the first.
+  plot::render::BitmapFont font;
+  for (plot::u32 codepoint = 32u; codepoint < 127u; ++codepoint) {
+    const plot::render::Glyph* g = font.glyph(codepoint);
+    PLOT_CHECK(g != nullptr);
+    PLOT_CHECK(plot::render::glyph_is_cell(*g, font.cell_uv()));
+  }
+  PLOT_CHECK_NEAR(font.cell_uv().x, static_cast<plot::f32>(plot::render::BitmapFont::kGlyphPx) /
+                                       static_cast<plot::f32>(plot::render::BitmapFont::kAtlasWidth),
+                  1e-9);
+  PLOT_CHECK_NEAR(font.cell_uv().y, static_cast<plot::f32>(plot::render::BitmapFont::kGlyphPx) /
+                                       static_cast<plot::f32>(plot::render::BitmapFont::kAtlasHeight),
+                  1e-9);
+}
+
+/// A backend whose cell is nothing like the bitmap font's -- half by a quarter of
+/// a small atlas -- so anything a batch reports that equals it came from the
+/// backend and not from a glyph of some other font.
+class HalfCellBackend final : public plot::render::TextBackend {
+ public:
+  plot::f32 measure(const char*) const override { return 4.0f; }
+  plot::f32 height() const override { return 8.0f; }
+  plot::f32 ascent() const override { return 6.0f; }
+  plot::Vec2 cell_uv() const override { return plot::Vec2{0.5f, 0.25f}; }
+  plot::u32 revision() const override { return 1u; }
+  const plot::render::Glyph* glyph(plot::u32) const override { return &cell_; }
+  const plot::u8* atlas_pixels() const override { return atlas_; }
+  plot::u32 atlas_width() const override { return 8u; }
+  plot::u32 atlas_height() const override { return 4u; }
+
+ private:
+  // One cell, at the second slot of a 2x2 grid. Its rect is the declared cell to
+  // within the atlas tolerance (5e-7) but not bit for bit -- which is what any
+  // backend's rounded arithmetic looks like, and what makes "the backend's
+  // number" and "this glyph's rect size" two distinguishable values.
+  const plot::render::Glyph cell_{0.5f, 0.0f, 1.0000005f, 0.2500005f, 0.0f, 0.0f, 4.0f};
+  const plot::u8 atlas_[8 * 4] = {};
+};
+
+// The draw's one rect size is the *backend's* `cell_uv()`, and it is known before
+// a glyph is staged: a batch that took it from whatever glyph it happened to
+// stage first could not answer the first of these, and would take another font's
+// grid the moment a caller staged two backends into one batch. `add_text()`
+// declares it from the backend it is handed, and `set_cell()` is how a caller
+// staging glyphs one at a time -- the renderer's clipped walk -- says it.
+PLOT_TEST(text_batch_takes_its_cell_from_the_backend) {
+  HalfCellBackend backend;
+  plot::render::TextBatch b;
+
+  b.begin();
+  PLOT_CHECK_NEAR(b.glyph_cell_uv().x, 0.0, 1e-6);   // nothing declared, nothing staged
+  PLOT_CHECK_NEAR(b.glyph_cell_uv().y, 0.0, 1e-6);
+  b.set_cell(backend.cell_uv());
+  PLOT_CHECK_NEAR(b.glyph_cell_uv().x, 0.5, 1e-6);   // declared, still nothing staged
+  PLOT_CHECK_NEAR(b.glyph_cell_uv().y, 0.25, 1e-6);
+
+  // A staged run keeps it -- and keeps the *backend's* number, not the staged
+  // glyph's: this backend's rects are 5e-7 off its declared cell (inside the
+  // tolerance, so it is a legal grid), and an implementation that recorded
+  // whatever glyph it staged first would land off these by exactly that.
+  b.begin();
+  const plot::Vec2 end = b.add_text("AB", {0.0f, 0.0f}, 1.0f, backend);
+  PLOT_CHECK(b.glyph_count() == 2);
+  PLOT_CHECK_NEAR(b.glyph_cell_uv().x, backend.cell_uv().x, 1e-12);
+  PLOT_CHECK_NEAR(b.glyph_cell_uv().y, backend.cell_uv().y, 1e-12);
+  PLOT_CHECK(backend.cell_uv().x != backend.glyph(0u)->u1 - backend.glyph(0u)->u0);
+  PLOT_CHECK(plot::render::glyph_is_cell(*backend.glyph(0u), b.glyph_cell_uv()));
+  PLOT_CHECK_NEAR(end.x, 2.0f * backend.glyph(static_cast<plot::u32>('A'))->advance, 1e-6);
+  // The instances carry the cell's *origin* in the atlas; the size rides along
+  // once, above, which is the whole reason a glyph fits in six floats.
+  PLOT_CHECK_NEAR(b.data()[2], backend.glyph(0u)->u0, 1e-6);
+  PLOT_CHECK_NEAR(b.data()[3], backend.glyph(0u)->v0, 1e-6);
+  PLOT_CHECK_NEAR(b.data()[4], 1.0, 1e-6);           // unrotated basis
+
+  // And the real font's grid, staged the same way, reports the font's cell -- the
+  // two never mix silently, and the batch's value tracks the backend it was given.
+  plot::render::BitmapFont font;
+  b.begin();
+  b.add_text("A", {0.0f, 0.0f}, 1.0f, font);
+  PLOT_CHECK_NEAR(b.glyph_cell_uv().x, font.cell_uv().x, 1e-9);
+  PLOT_CHECK_NEAR(b.glyph_cell_uv().y, font.cell_uv().y, 1e-9);
+  PLOT_CHECK(b.glyph_cell_uv().x != backend.cell_uv().x);
+
+  // begin() forgets the declaration along with the instances, so a frame cannot
+  // inherit the previous one's cell.
+  b.begin();
+  PLOT_CHECK_NEAR(b.glyph_cell_uv().x, 0.0, 1e-6);
+  PLOT_CHECK_NEAR(b.glyph_cell_uv().y, 0.0, 1e-6);
+}

tests/test_render_smoke.cpp

@@ -15,7 +15,11 @@
 
 #include <atomic>
 #include <cmath>
+#include <csignal>
 #include <cstdlib>
+#include <cstdio>
+#include <sys/wait.h>
+#include <unistd.h>
 #include <vector>
 
 namespace {
@@ -129,6 +133,7 @@ class OnePxPerStringBackend final : public plot::render::TextBackend {
   plot::f32 height() const override { return 1.0f; }
   plot::f32 ascent() const override { return 1.0f; }
   const plot::render::Glyph* glyph(plot::u32) const override { return nullptr; }
+  plot::Vec2 cell_uv() const override { return plot::Vec2{}; }  // no atlas, no cell
   const plot::u8* atlas_pixels() const override { return nullptr; }
   plot::u32 atlas_width() const override { return 0u; }
   plot::u32 atlas_height() const override { return 0u; }
@@ -161,6 +166,14 @@ class BakeableBackend final : public plot::render::TextBackend {
     ++revision_;
   }
 
+  /// Trims the published glyph's rect to an ink box, which is what breaks the
+  /// fixed-cell grid (see `TextBackend::cell_uv`): the rect is no longer the cell
+  /// `cell_uv()` declares, so the staging path's check fails. Used by the case
+  /// that pins that check.
+  void trim_glyph_rect(plot::f32 width) {
+    glyph_.u1 = glyph_.u0 + width;
+  }
+
   /// Publishes no atlas at all (the "no text" a backend is allowed to be), and
   /// bumps the revision -- which is how a backend that *starts* publishing one
   /// announces it.
@@ -178,6 +191,7 @@ class BakeableBackend final : public plot::render::TextBackend {
   plot::f32 height() const override { return static_cast<plot::f32>(size_); }
   plot::f32 ascent() const override { return static_cast<plot::f32>(size_); }
   const plot::render::Glyph* glyph(plot::u32) const override { return &glyph_; }
+  plot::Vec2 cell_uv() const override { return pixels_.empty() ? plot::Vec2{} : plot::Vec2{1.0f, 1.0f}; }
   const plot::u8* atlas_pixels() const override { return pixels_.empty() ? nullptr : pixels_.data(); }
   plot::u32 atlas_width() const override { return size_; }
   plot::u32 atlas_height() const override { return size_; }
@@ -252,9 +266,9 @@ InkBox draw_text_run(plottest::HeadlessContext& ctx, plot::render::TextAtlas& at
   plot::gl::glBlendFunc(plot::gl::GL_SRC_ALPHA, plot::gl::GL_ONE_MINUS_SRC_ALPHA);
   program.bind();
   program.set_viewport({static_cast<plot::f32>(ctx.width), static_cast<plot::f32>(ctx.height)});
-  // The same two numbers the renderer's flush computes: the staged cell, and that
-  // cell's size on screen at this scale.
-  const plot::Vec2 cell = batch.glyph_cell_uv();
+  // The same two numbers the renderer's flush computes: the backend's cell, and
+  // that cell's size on screen at this scale.
+  const plot::Vec2 cell = backend.cell_uv();
   const plot::Vec2 texels = atlas.texel_size();
   program.set_glyph_uv(cell);
   program.set_px_range({cell.x * texels.x * px, cell.y * texels.y * px});
@@ -1409,3 +1423,36 @@ PLOT_TEST(a_backend_without_an_atlas_uploads_nothing) {
   PLOT_CHECK(plot::gl::glGetError() == plot::gl::GL_NO_ERROR);
   ctx.destroy();
 }
+
+// The fixed-cell check itself, in the build where it is fatal. A backend whose
+// glyph rects are ink boxes (see `TextBackend::cell_uv`) would draw every glyph at
+// the size the draw was told, reading past its own rows of the atlas -- so the
+// staging path asserts `glyph_is_cell()` and this case pins that it does, not
+// merely that the predicate can fail.
+//
+// The assertion's death is the observable, and the only protocol for observing a
+// death is a child process: staging this glyph in-process would take the suite
+// with it. The child allocates nothing before it dies -- the check comes before
+// the first `push_back` -- and its own `assert` line lands on stderr above this
+// case's line, which is the evidence for a human reading the log.
+PLOT_TEST(staging_a_glyph_that_is_not_the_cell_aborts_in_a_debug_build) {
+#ifndef NDEBUG
+  BakeableBackend backend;
+  backend.trim_glyph_rect(0.5f);  // half a cell: no longer the declared cell
+  PLOT_CHECK(!plot::render::glyph_is_cell(*backend.glyph(0u), backend.cell_uv()));
+
+  std::fflush(nullptr);
+  const pid_t child = ::fork();
+  PLOT_CHECK(child >= 0);
+  if (child == 0) {
+    plot::render::TextBatch batch;
+    batch.begin();
+    batch.add_text("X", {0.0f, 0.0f}, 1.0f, backend);  // dies here, in debug
+    ::_exit(0);  // reached only when the check did not fire
+  }
+  int status = 0;
+  PLOT_CHECK(::waitpid(child, &status, 0) == child);
+  PLOT_CHECK(WIFSIGNALED(status));          // it did not return
+  PLOT_CHECK(WTERMSIG(status) == SIGABRT);  // and it was the assertion
+#endif
+}