fix(render): drop x title rows the chrome clamp cut short

src/render/renderer.cpp

@@ -586,22 +586,38 @@ void Renderer::draw_titles(const Plot& plot, const PlotLayout& layout) {
   // than the strip -- which the layout caps, because a title may shrink the plot
   // but never erase it -- stops at the strip's padding instead of spilling over the
   // labels or the data.
+  //
+  // A line is staged only if the cell it occupies *fits* the strip, and that is a
+  // check this function owns: a run's clip cuts along the run (`walk_text_clipped()`),
+  // the layout clamps the strip against the viewport's chrome allowance without
+  // knowing what a line is (`Plot::layout()`), and the cell's other dimension -- a
+  // row's height, a column's width -- is one text line. Rows are staged top down and
+  // columns left to right, so the lines a clamped strip cut short are the trailing
+  // ones: keep the leading lines that fit and stage no more. A dropped title is
+  // chrome that had no room, which is the same bargain the label strips already make
+  // -- never drawn outside its strip, and never inside `axes`.
   const f32 pad = style_.grid_label_pad;
   const f32 line_h = font_backend_.height();
 
   // X titles: the strip's rows top down, one per titled axis, each read left to
-  // right on the row it owns.
+  // right on the row it owns. A row must fit the strip's *height* -- with this basis
+  // the cell is [pen.y, pen.y + line_h] -- and the x strip is the bottom-most, so a
+  // row that does not fit leaves the target entirely (nothing is over it) and is
+  // clipped rather than overprinted. It is dropped all the same: a title is drawn in
+  // the strip the layout reserved, or not at all, and this is the dimension the
+  // layout's clamp is applied to.
   for (u32 i = 0, row = 0; i < plot.x_axis_count(); ++i) {
     const Axis& axis = plot.x_axis(i);
     if (axis.label.empty()) continue;
     const Rect& strip = layout.x_title_area;
+    const f32 y = strip.y + pad + static_cast<f32>(row) * line_h;
+    if (y + line_h > strip.bottom()) break;
     const f32 budget = strip.w - 2.0f * pad;
     const f32 width = clipped_text_width(axis.label.c_str(), budget, kTextRight);
     // Centred on the strip: the tick labels have their own rows above it, so the
     // title needs no alignment of its own.
-    stage_text_clipped({strip.x + 0.5f * (strip.w - width),
+    stage_text_clipped({strip.x + 0.5f * (strip.w - width), y}, axis.label.c_str(), budget,
-                        strip.y + pad + static_cast<f32>(row) * line_h},
+                       kTextRight);
-                       axis.label.c_str(), budget, kTextRight);
     ++row;
   }
 
@@ -622,21 +638,11 @@ void Renderer::draw_titles(const Plot& plot, const PlotLayout& layout) {
     const Axis& axis = plot.y_axis(i);
     if (axis.label.empty()) continue;
     const Rect& strip = layout.y_title_area;
-    // A column must *fit* before it is staged, and its own cell is what has to fit:
+    // A column must fit the strip's *width* -- with this basis the cell is
-    // with this basis the cell occupies [pen.x, pen.x + line_h], so the strip's
+    // [pen.x, pen.x + line_h] -- and a column that does not fit is unlike an x row,
-    // width -- the one dimension the run never advances along -- is the whole of a
+    // which would leave the target and be clipped: this one is drawn beside the
-    // column's cross extent, and nothing else in this path can see it. `walk_text_clipped()`
+    // columns that do fit, over the tick labels the next strip holds, and past `axes`
-    // cuts along the run and would happily stage a column that is wider than the
+    // once the columns reach it (a 64 px viewport does both). See the rule above.
-    // strip it belongs to, drawing its cells over whatever is beside them.
-    //
-    // The layout clamps the strip against the viewport's chrome allowance (see
-    // `Plot::layout()`), so a viewport narrow enough to spend that allowance on the
-    // tick labels leaves room for fewer columns than there are titled axes, or for
-    // none at all. Columns are laid out left to right, so the ones the clamp cut
-    // short are the trailing ones: keep the leading columns that fit and stage no
-    // more. A dropped title is chrome that had no room, which is the same bargain
-    // the label strip already makes -- never drawn outside its strip, and never
-    // inside `axes`.
     const f32 x = strip.x + pad + static_cast<f32>(column) * line_h;
     if (x + line_h > strip.right()) break;
     const f32 budget = strip.h - 2.0f * pad;

tests/test_render_smoke.cpp

@@ -377,13 +377,41 @@ RectInk scan_rect_ink(plottest::HeadlessContext& ctx, const plot::Rect& area, pl
   return ink;
 }
 
-/// One band of inked columns: `[first, last]`, both inclusive, in framebuffer x.
+/// One band of inked lines: `[first, last]`, both inclusive, in the coordinate the
+/// band runs across -- framebuffer x for `ink_column_bands()`, framebuffer y for
+/// `ink_row_bands()`.
 struct InkBand {
   plot::u32 first = kNoColumn;
   plot::u32 last = 0;
   plot::u32 width() const noexcept { return first == kNoColumn ? 0u : last - first + 1u; }
 };
 
+/// The bands of inked lines `text`-coloured ink forms in `area`, top to bottom: one
+/// band per drawn text line that a gap of blank lines separates from its neighbours,
+/// which is how a case counts the rows a strip holds (an x title's rows, a y strip's
+/// stacked labels). Two lines drawn over each other make one band.
+std::vector<InkBand> ink_row_bands(plottest::HeadlessContext& ctx, const plot::Rect& area,
+                                   plot::Color text) {
+  std::vector<InkBand> bands;
+  for (plot::u32 y = static_cast<plot::u32>(area.y);
+       y < static_cast<plot::u32>(area.bottom()); ++y) {
+    bool has = false;
+    for (plot::u32 x = static_cast<plot::u32>(area.x);
+         x < static_cast<plot::u32>(area.right()) && !has; ++x) {
+      plot::u8 px[4] = {};
+      PLOT_CHECK(ctx.read_pixel(x, ctx.height - 1u - y, px));
+      has = text_ink(px, text);
+    }
+    if (!has) continue;
+    if (bands.empty() || bands.back().last + 1u != y) {
+      bands.push_back(InkBand{y, y});
+    } else {
+      bands.back().last = y;
+    }
+  }
+  return bands;
+}
+
 /// The bands of inked columns `text`-coloured ink forms in `area`, left to right:
 /// one band per drawn run that a gap of blank columns separates from its
 /// neighbours. Two runs drawn over each other make one band; two separated by
@@ -1547,6 +1575,120 @@ PLOT_TEST(narrow_viewport_keeps_only_the_y_title_columns_that_fit) {
   ctx.destroy();
 }
 
+// The clamp reaches the x title from the vertical side: `x_title_height` is capped at
+// the viewport's allowance minus the tick label strip's height, so a viewport short
+// enough leaves the strip too short for a single row. A row staged anyway is not wholly
+// clipped -- its cell starts inside the strip, so the top of its glyphs lands in the
+// frame, a title cut off mid-stroke -- and neither the run's own clip (which cuts along
+// x) nor the layout's clamp can see that. So this is the one titled axis where nothing
+// fits, and nothing may be staged.
+PLOT_TEST(short_viewport_draws_no_x_title_it_cannot_fit) {
+  // 80 px tall: the label strip takes 23.69 px of the 40 px allowance, leaving a
+  // 16.31 px strip against the 19.69 px a row needs -- no room for a row, but 12.31 px
+  // of a staged row's cell would still be inside the target.
+  constexpr plot::u32 kShortW = 320;
+  constexpr plot::u32 kShortH = 80;
+  plottest::HeadlessContext ctx(kShortW, kShortH);
+  plot::Status st;
+  PLOT_CHECK(ctx.create(&st));
+  PLOT_CHECK(plot::gl::load(nullptr));
+  plot::Renderer r;
+  PLOT_CHECK(r.init(&st));
+
+  plot::Plot p;
+  p.set_viewport({0, 0, static_cast<plot::f32>(kShortW), static_cast<plot::f32>(kShortH)});
+  p.add_x_axis({"seconds", {0.0, 1.0}, false, plot::AxisPlace::Bottom, false});
+  p.add_y_axis({"v", {-1.0, 1.0}, false, plot::AxisPlace::Left, false});
+  r.begin_frame(kShortW, kShortH,
+                {0, 0, static_cast<plot::f32>(kShortW), static_cast<plot::f32>(kShortH)});
+  r.draw(p);
+  r.end_frame();
+  PLOT_CHECK(!r.last_frame_had_gl_error());
+  dump_if_requested(r, "x_title_clamped.ppm");
+
+  const plot::PlotLayout layout = p.layout();
+  const plot::Color ink = r.style().axis_text;
+  const plot::f32 line = r.axis_font_px();
+  const plot::f32 pad = r.style().grid_label_pad;
+  const plot::Rect& strip = layout.x_title_area;
+
+  // The clamp bit the way this case needs it to: not one row fits.
+  PLOT_CHECK(p.titled_x_axis_count() == 1u);
+  const plot::u32 fits = static_cast<plot::u32>(std::max(0.0f, (strip.h - pad) / line));
+  PLOT_CHECK(fits == 0u);
+  // ... and the plot kept its floor in a viewport this short.
+  PLOT_CHECK(layout.axes.h >= 0.5f * static_cast<plot::f32>(kShortH));
+  PLOT_CHECK(layout.axes.w > 0.0f);
+
+  // Nothing was staged for it: no title ink in the strip at all (a staged row's ink
+  // would land here, cut off by the viewport's bottom edge), and none in the data
+  // area either.
+  PLOT_CHECK(!scan_rect_ink(ctx, strip, ink).any());
+  PLOT_CHECK(!rect_has_exact(ctx, layout.axes, ink));
+  ctx.destroy();
+}
+
+// The same clamp one row short instead of absent: three titled x axes ask for three
+// rows and the strip holds one, so the rows it cut short are the trailing ones and must
+// be dropped rather than squeezed past its end. Each is partly visible otherwise -- its
+// cell starts inside the strip, and the rest of it is cut by the target's bottom edge,
+// which is exactly where an x strip ends -- so the strip's row bands are what says how
+// many were staged: one per row that fits, each inside its own row's cell.
+PLOT_TEST(short_viewport_keeps_only_the_x_title_rows_that_fit) {
+  // 112 px tall: the label strip takes 23.69 px of the 56 px allowance, leaving a
+  // 32.31 px strip against the 19.69 px a row needs and the 35.38 px two rows need, so
+  // one row fits with 12.6 px of the strip left below it -- enough for the row that
+  // must be dropped to show its glyph tops before the target's bottom edge cuts it.
+  constexpr plot::u32 kShortW = 320;
+  constexpr plot::u32 kShortH = 112;
+  plottest::HeadlessContext ctx(kShortW, kShortH);
+  plot::Status st;
+  PLOT_CHECK(ctx.create(&st));
+  PLOT_CHECK(plot::gl::load(nullptr));
+  plot::Renderer r;
+  PLOT_CHECK(r.init(&st));
+
+  plot::Plot p;
+  p.set_viewport({0, 0, static_cast<plot::f32>(kShortW), static_cast<plot::f32>(kShortH)});
+  p.add_x_axis({"seconds", {0.0, 1.0}, false, plot::AxisPlace::Bottom, false});
+  p.add_x_axis({"samples", {0.0, 1.0}, false, plot::AxisPlace::Top, false});
+  p.add_x_axis({"ticks", {0.0, 1.0}, false, plot::AxisPlace::Bottom, false});
+  p.add_y_axis({"v", {-1.0, 1.0}, false, plot::AxisPlace::Left, false});
+  r.begin_frame(kShortW, kShortH,
+                {0, 0, static_cast<plot::f32>(kShortW), static_cast<plot::f32>(kShortH)});
+  r.draw(p);
+  r.end_frame();
+  PLOT_CHECK(!r.last_frame_had_gl_error());
+  dump_if_requested(r, "x_title_rows_clamped.ppm");
+
+  const plot::PlotLayout layout = p.layout();
+  const plot::Color ink = r.style().axis_text;
+  const plot::f32 line = r.axis_font_px();
+  const plot::f32 pad = r.style().grid_label_pad;
+  const plot::Rect& strip = layout.x_title_area;
+
+  // A row occupies the strip's top padding plus one line, so this is how many of the
+  // three fit -- the rule the renderer applies, derived here rather than assumed.
+  PLOT_CHECK(p.titled_x_axis_count() == 3u);
+  const plot::u32 fits = static_cast<plot::u32>(std::max(0.0f, (strip.h - pad) / line));
+  PLOT_CHECK(fits >= 1u);
+  PLOT_CHECK(fits < 3u);
+  PLOT_CHECK(layout.axes.h >= 0.5f * static_cast<plot::f32>(kShortH));
+  PLOT_CHECK(layout.axes.w > 0.0f);
+
+  // One band of inked rows per row that fits, each inside its own row's cell (band i
+  // starts at the padding plus i lines and is one line tall).
+  const std::vector<InkBand> rows = ink_row_bands(ctx, strip, ink);
+  PLOT_CHECK(rows.size() == fits);
+  for (plot::u32 i = 0; i < static_cast<plot::u32>(rows.size()); ++i) {
+    PLOT_CHECK(rows[i].first >=
+               static_cast<plot::u32>(strip.y + pad + static_cast<plot::f32>(i) * line));
+    PLOT_CHECK(rows[i].last <=
+               static_cast<plot::u32>(strip.y + pad + static_cast<plot::f32>(i + 1u) * line));
+  }
+  ctx.destroy();
+}
+
 // Finding 4 of the review: `mark_drawn()` dropped a failed fence's status, so the
 // slot stayed unsynchronised and the next upload memcpy'd into storage the GPU may
 // still have been reading -- exactly the tearing the ring exists to prevent. A