fix(plot,render,gl): live streaming measurements, handed-back GL state

Final whole-branch review fixes.

1 (Important). Plot::measurement() cached on (series pointer, sample count)
only, so a saturated StreamSeries -- whose size() stays pinned while x0()
advances every acquire() -- froze its readout on the first saturated frame
until a cursor moved. The cache key now carries the series' x0() (new
MeasureSlot::last_x0), so a sliding window recomputes and a static series
still does not: asserted with a new macro-gated counter hook
(plot/detail/measure_count.hpp, PLOT_COUNT_MEASURE_RECOMPUTES, wired like the
pyramid and ring counters, absent from the release library). New case
saturated_stream_measurement_tracks_the_sliding_window: saturated ring,
stationary cursors, ramp in absolute x -- zero recomputes for repeated reads
with nothing moving, one per acquire while it slides.

2-3, 9. Spec: 4.3 described a GL_LINE_STRIP path that does not exist (there is
one rasteriser -- instanced anti-aliased quads -- and dense_threshold_spp
selects the gather level, not a primitive); 8 named a PLOT_GL_CHECK() macro
that exists nowhere, and stated no exception policy. The table, the field's
comment and 8 now describe the real gather-level threshold, the real
glGetError() drain plus the stores' drop-to-orphan fallbacks, and the real
policy: library failures return Status/bool, allocating entry points propagate
std::bad_alloc.

4. The frame handed back neither GL_UNPACK_ALIGNMENT (init() sets 1 for the
atlas) nor the clear colour (queried once with a new glGetFloatv entry).
end_frame() restores both and renderer.hpp lists them in the residue
contract.

5. draw() kept a host TextBackend for layout while drawing with its own font.
The renderer now installs its font unconditionally -- strips and labels measure
and draw with one backend -- with a debug-build diagnostic if a different one
was set, and Plot::set_text_backend() is documented as the no-renderer path.
New case renderer_measures_the_labels_it_draws asserts layout, measuring and
the drawn label's ink agree.

6-10. draw()'s style overwrite stated at the call site; the load-sensitive
wall-clock bound in test_measure.cpp replaced by a counter assertion (same
interval, same fold count over a 512x length ratio); dump_frame_ppm drops
noexcept and says why it may allocate; error.txt ignored (not this project's,
not generated here) rather than deleted.

Verified: debug and release build clean (-Wall -Wextra -Wpedantic -Wconversion
-Wshadow), 4/4 ctest entries pass, 130 cases pass (was 127), umbrella and
header self-containment checks pass, and no counter symbol in the release
archive.
This commit is contained in:
Martino Ferrari committed 2026-09-28 03:42:34 +02:00
1 parent f363523195
commit a08b184481
15 files changed
+478 -54

No files matched your search

+15 -4
View File
@@ -1,4 +1,5 @@
#include <plot/plot/plot.hpp>
#include <plot/plot/detail/measure_count.hpp>
#include <algorithm>
#include <cmath>
@@ -271,11 +272,20 @@ const Measurement& Plot::measurement(u32 id) const {
const data::Series* series = nullptr;
if (slot.trace < traces_.size()) series = traces_[slot.trace].series;
const u64 samples = series != nullptr ? series->size() : 0;
// A saturated ring keeps `samples` constant while its window slides: x0() is
// what moves, and with it the sample each cursor snaps to and the interval they
// measure. Caching on (pointer, count) alone would freeze the readout at the
// first saturated frame until a cursor moved.
const f64 x0 = series != nullptr ? series->x0() : 0.0;
// Recompute only when someone moved a cursor, replaced the trace's series, or
// grew it. Everything else is a cache hit, which is what keeps a steady frame
// from re-evaluating every measure.
if (slot.dirty || series != slot.last_series || samples != slot.last_size) {
// Recompute only when someone moved a cursor, replaced the trace's series, grew
// it, or slid the window under it (x0 above). Everything else is a cache hit,
// which is what keeps a steady frame from re-evaluating every measure.
if (slot.dirty || series != slot.last_series || samples != slot.last_size ||
x0 != slot.last_x0) {
#ifdef PLOT_COUNT_MEASURE_RECOMPUTES
detail::measure_recomputes().fetch_add(1, std::memory_order_relaxed);
#endif
slot.result = Measurement{};
if (series != nullptr && samples > 0 && slot.cursor_a < cursors_.size() &&
slot.cursor_b < cursors_.size()) {
@@ -287,6 +297,7 @@ const Measurement& Plot::measurement(u32 id) const {
}
slot.last_series = series;
slot.last_size = samples;
slot.last_x0 = x0;
slot.dirty = false;
}
return slot.result;
+53 -7
View File
@@ -76,9 +76,21 @@ f32 walk_text_clipped(Vec2 pen, const char* utf8, f32 budget_px, const render::B
return used;
}
} // namespace
#ifndef NDEBUG
/// A host that installed its own `TextBackend` is told its metrics are gone: the
/// renderer draws every label with its artwork's font, so laying the strips out
/// with a different one puts the labels wherever that font's metrics happen to
/// fall. A diagnostic rather than an abort -- a host's frame must still draw --
/// and debug-only, like `plot::gl::load()`'s unresolved-entry-point notice.
void warn_replaced_text_backend(const render::TextBackend* previous) noexcept {
std::fprintf(stderr,
"plot::Renderer::draw: replacing the Plot's TextBackend (%p) with the renderer's "
"own bitmap font; a plot drawn by a Renderer measures the labels it draws.\n",
static_cast<const void*>(previous));
}
#endif
// --------------------------------------------------------------- AxisFont
} // namespace
void Renderer::AxisFont::reset(const render::BitmapFont& font, f32 requested_px) noexcept {
font_ = &font;
@@ -125,6 +137,12 @@ bool Renderer::init(Status* st) noexcept {
font_backend_.reset(font_, style_.axis_font_px);
// The atlas upload needs GL_UNPACK_ALIGNMENT 1 (single-channel rows have no
// padding), which is the only global state `init()` changes; the host's value is
// read first so `end_frame()` can hand it back rather than leave the host's own
// uploads to the alignment this renderer wanted.
gl::glGetIntegerv(gl::GL_UNPACK_ALIGNMENT, &host_unpack_alignment_);
if (!atlas_.create_r8(render::BitmapFont::kAtlasWidth, render::BitmapFont::kAtlasHeight, st)) {
return false;
}
@@ -182,6 +200,19 @@ void Renderer::begin_frame(u32 fb_w, u32 fb_h, Rect viewport_px) noexcept {
// already painted keeps everything outside the rect. The scissor box is in
// framebuffer coordinates and is not affected by glViewport, which is why the
// clear happens before the viewport is set.
//
// The host's clear colour is queried once, before the first frame sets its own:
// GL keeps no history for it, so this is the only moment the value can be
// recovered for `end_frame()` to hand back. A host that changes its own clear
// colour *after* the first frame therefore gets the first frame's value restored
// -- the renderer cannot tell its own writes from the host's -- which is still
// the host's colour, and stated as such in end_frame()'s contract.
if (!host_clear_saved_) {
f32 rgba[4] = {0.0f, 0.0f, 0.0f, 0.0f};
gl::glGetFloatv(gl::GL_COLOR_CLEAR_VALUE, rgba);
host_clear_ = Color{rgba[0], rgba[1], rgba[2], rgba[3]};
host_clear_saved_ = true;
}
gl::glEnable(gl::GL_SCISSOR_TEST);
gl::glScissor(vx, vy, vw, vh);
const Color& background = style_.background;
@@ -206,9 +237,18 @@ void Renderer::draw(Plot& plot) {
// measure with the size the text pipeline draws with, and re-read the style
// each frame so a changed axis_font_px takes effect on the next one.
font_backend_.reset(font_, style_.axis_font_px);
if (plot.text_backend() == nullptr) {
plot.set_text_backend(&font_backend_);
#ifndef NDEBUG
// The renderer owns the text of the frames it draws: every label goes through
// the text pipeline in `font_`, so the plot is handed this font's metrics
// whatever a host installed. A host backend is therefore replaced on the first
// draw -- said out loud in a debug build, since its disappearance is otherwise
// silent -- and a Plot nobody draws keeps it (see Plot::set_text_backend()).
if (const render::TextBackend* previous = plot.text_backend();
previous != nullptr && previous != &font_backend_) {
warn_replaced_text_backend(previous);
}
#endif
plot.set_text_backend(&font_backend_);
// One style, not two: the plot reserves its strips and picks its tick density
// from the values the renderer draws with, so the label pad is never one number
// for the layout and another for the text placed inside it.
@@ -245,8 +285,10 @@ void Renderer::end_frame() noexcept {
gl_error_ = true;
}
// Leave the host's context as close to neutral as a frame can: nothing bound to
// a pipeline stage, and off every state bit the frame turned on. What remains
// is the viewport (the caller's to set for its next pass), the pixel-pack
// a pipeline stage, off every state bit the frame turned on, and the two values
// the frame *changed* rather than merely used handed back -- the unpack
// alignment `init()` set for the atlas, and the host's clear colour. What
// remains is the viewport (the caller's to set for its next pass), the pixel-pack
// alignment a PPM dump may have set, and the vertex buffers' own contents.
gl::glBindVertexArray(0);
gl::glUseProgram(0);
@@ -254,6 +296,10 @@ void Renderer::end_frame() noexcept {
gl::glActiveTexture(gl::GL_TEXTURE0);
gl::glBindTexture(gl::GL_TEXTURE_2D, 0);
gl::glDisable(gl::GL_BLEND);
gl::glPixelStorei(gl::GL_UNPACK_ALIGNMENT, host_unpack_alignment_);
if (host_clear_saved_) {
gl::glClearColor(host_clear_.r, host_clear_.g, host_clear_.b, host_clear_.a);
}
// Drain, do not peek: the first error hides every later one, and a frame that
// produced several must be reported as having produced one.
while (gl::glGetError() != gl::GL_NO_ERROR) {
@@ -647,7 +693,7 @@ void Renderer::draw_annotations(const Plot& plot) {
// -------------------------------------------------------------------- dump
bool Renderer::dump_frame_ppm(const char* path, Status* st) const noexcept {
bool Renderer::dump_frame_ppm(const char* path, Status* st) const {
if (path == nullptr || path[0] == '\0') {
return fail(st, ErrorCode::InvalidArgument, "PPM path is empty");
}