From 0ee6e10c81b7c98b6518ce545907078d1f8f8802 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Sat, 3 Oct 2026 17:51:37 +0200 Subject: [PATCH] fix(render): a clamped ring front does not draw a segment across the panel A ring-backed series answers a *clamped* stamp for the oldest slots of its window -- the ones the producer has already overwritten -- because the published window is pinned while both rings slide under it, and the alternative (a sentinel) collapsed the whole index mapping. The consequence for a polyline is that the stamps do not increase at the *front*: the first points carry the newest stamp and everything after them is older. Drawn as they come, that is a segment from the newest sample back to the oldest -- a line across the whole panel, which a saturated ring shows as a flicker. The gathered points are now reduced to their longest strictly increasing suffix before anything is staged, so the unreadable front is left out and the real samples are untouched. A forward pass cannot express this: the bad points are at the front and everything behind them is legitimately smaller, so "drop anything that does not advance" would delete the trace -- walking backwards keeps each point strictly older than the one kept after it and stops at the first unreadable slot. One pass over the visible points, which the staging then walks anyway. Test: a_clamped_front_does_not_draw_a_segment_across_the_panel -- a series whose first two samples carry the newest stamp at -1 while the rest walk 0..50 at +1, the shape a saturated ChannelBuffer produces. Flat at +1 except for those two, so any ink below the middle of the axes is the bad segment: with the filter there is none (0 below, 1032 above in the columns the trace covers), and without it the diagonal crosses the panel. --- README.md | 4 ++ src/render/renderer.cpp | 34 +++++++++++++ tests/test_render_smoke.cpp | 96 +++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+) diff --git a/README.md b/README.md index f32c54d..0300b25 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,10 @@ run touched a GPU, so read it as an upper bound and a record of the software pat ## Features +- a trace whose series answers a *clamped* stamp for its oldest slots -- a ring whose published + window is pinned while it slides -- is drawn from its longest increasing *suffix*, so an unreadable + front cannot draw a segment from the newest sample back to the oldest (a line across the whole + panel); - a following group's window stays on its members' data: while the members retain at least a window's worth, the scroll offset is bounded by `retained - span`, so a pan or a zoom-in near an edge cannot leave the window's own span over data the scope never had (a window *wider* than the diff --git a/src/render/renderer.cpp b/src/render/renderer.cpp index c604950..7967c2b 100644 --- a/src/render/renderer.cpp +++ b/src/render/renderer.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -1065,6 +1066,39 @@ void Renderer::draw_traces(const Plot& plot, const PlotLayout& layout) { const bool log_x = plot.x_axis(trace.axis_x).log; const bool log_y = plot.y_axis(trace.axis_y).log; const Mapping map = mapping(plot, trace.axis_x, trace.axis_y, x_range.min); + // Keep the longest **increasing suffix** of the gathered points, and drop what precedes it. + // + // A ring-backed series may answer a *clamped* stamp for the oldest slots of its window -- the ones + // the producer has already overwritten. The clamp is deliberate (the alternative was a sentinel + // that collapsed the whole index mapping), but it makes the stamps *non-increasing at the front*: + // the first few points carry the newest stamp, and everything after them is older. A polyline + // through that draws a segment from the newest sample back to the oldest -- a line across the + // whole panel, which is what a saturated ring shows as a flicker. + // + // A forward filter cannot express this: the bad points are at the *front* and everything behind + // them is legitimately smaller, so dropping "anything that does not advance" would delete the + // trace. Walking backwards keeps each point that is strictly older than the one kept after it, + // which stops at the first unreadable slot and leaves the real samples alone. The cost is one + // pass over the visible points, which the staging below is about to walk anyway. + { + const usize count = gather_.size() / 2u; + usize kept = 0; + f64 next = std::numeric_limits::infinity(); + for (usize k = count; k > 0; --k) { + const usize i = k - 1u; + const f64 x = static_cast(gather_[2u * i]) + series.x0(); + if (!(x < next)) continue; // out of order with what follows: unreadable, and a NaN too + next = x; + gather_[2u * (count - 1u - kept)] = gather_[2u * i]; + gather_[2u * (count - 1u - kept) + 1u] = gather_[2u * i + 1u]; + ++kept; + } + if (kept != count) { + // The kept points were written back-to-front into the tail; move them down to the front. + std::memmove(gather_.data(), gather_.data() + 2u * (count - kept), 2u * kept * sizeof(f32)); + gather_.resize(2u * kept); + } + } const u32 points = static_cast(gather_.size() / 2u); // A trace's dash rides on the same per-draw uniform the crosshair uses, in diff --git a/tests/test_render_smoke.cpp b/tests/test_render_smoke.cpp index 3975780..f602c9a 100644 --- a/tests/test_render_smoke.cpp +++ b/tests/test_render_smoke.cpp @@ -1470,6 +1470,102 @@ PLOT_TEST(a_sparse_window_marks_its_samples) { ctx.destroy(); } +/// A ring-backed series whose oldest slots read a *clamped* stamp gets no segment across the panel. +/// +/// `ChannelBuffer`'s documented bargain: the published window is pinned while both rings slide under +/// it, so the oldest few slots of a saturated window point at time slots the producer has +/// overwritten and have to be clamped to the oldest *readable* one -- the alternative, a sentinel, +/// collapsed the whole index mapping. The effect on the polyline is that the stamps do not increase +/// at the *front*: the first points carry the newest stamp and everything behind them is older. Drawn +/// as they come, that is a segment from the newest sample back to the oldest: a line across the whole +/// panel, which is what "it connects the newest point with the oldest" looks like. +/// +/// The trace here is flat at +1 except for those two unreadable points, which carry -1. Any ink in the +/// lower half of the axes is therefore the bad segment, and nothing else in the frame is that colour. +PLOT_TEST(a_clamped_front_does_not_draw_a_segment_across_the_panel) { + plottest::HeadlessContext ctx(kW, kH); + plot::Status st; + PLOT_CHECK(ctx.create(&st)); + PLOT_CHECK(plot::gl::load(nullptr)); + plot::Renderer r; + PLOT_CHECK(r.init(&st)); + + /// Eight samples: indices 0 and 1 are the clamped front (newest stamp, low value), 2..7 walk 0..50 + /// at +1. + struct ClampedFront final : plot::data::Series { + ClampedFront() { + pyramid_.reserve(8u); + for (u64 i = 0; i < 8u; ++i) pyramid_.append(i < 2u ? -1.0f : 1.0f); + } + u64 size() const override { return 8u; } + f64 x0() const override { return 0.0; } + f64 dx() const override { return 1.0; } + plot::Range x_bounds() const override { return {0.0, 60.0}; } + plot::Range y_bounds() const override { return {-1.0, 1.0}; } + const plot::data::MinMaxPyramid& pyramid() const override { return pyramid_; } + f64 x_at(u64 i) const override { return i < 2u ? 60.0 : 10.0 * static_cast(i - 2u); } + bool neighbours(f64 x, u64& i0, u64& i1) const override { + if (x < 0.0 || x > 60.0) return false; + const u64 k = std::min(2u + static_cast(x / 10.0), 7u); + i0 = k; + i1 = k; + if (x_at(k) != x && k + 1u < 8u) i1 = k + 1u; + return true; + } + void gather(u32, u64 begin, u64 end, std::vector& out, plot::data::GatherMode) const override { + for (u64 i = begin; i < end; ++i) { + out.push_back(static_cast(x_at(i) - x0())); + out.push_back(i < 2u ? -1.0f : 1.0f); + } + } + + protected: + void copy_raw(u64 begin, u64 end, f32* out) const override { + for (u64 i = begin; i < end; ++i) out[i - begin] = i < 2u ? -1.0f : 1.0f; + } + + private: + plot::data::MinMaxPyramid pyramid_; + } series; + + plot::Plot p; + p.set_viewport({0, 0, static_cast(kW), static_cast(kH)}); + const plot::u32 ax = p.add_x_axis({"t", {-5.0, 55.0}, false, plot::AxisPlace::Bottom, false}); + const plot::u32 ay = p.add_y_axis({"v", {-2.0, 2.0}, false, plot::AxisPlace::Left, false}); + p.add_trace({&series, plot::Color::hex(0x00FF00), 1.5f, ax, ay}); + + r.begin_frame(kW, kH, {0, 0, static_cast(kW), static_cast(kH)}); + r.draw(p); + r.end_frame(); + PLOT_CHECK(!r.last_frame_had_gl_error()); + + const plot::Rect& a = p.layout().axes; + plot::u32 below = 0; + plot::u32 on_the_line = 0; + for (plot::u32 cx = static_cast(a.x) + 3u; cx + 3u < static_cast(a.x + a.w); + ++cx) { + for (plot::u32 row = static_cast(a.h / 2u) + 2u; + row + 2u < static_cast(a.h); ++row) { + plot::u8 px[4] = {}; + PLOT_CHECK(ctx.read_pixel(cx, ctx.height - 1u - (static_cast(a.y) + row), px)); + if (green_pixel(px)) ++below; + } + } + for (plot::u32 cx = static_cast(a.x) + 3u; cx + 3u < static_cast(a.x + a.w); + ++cx) { + for (plot::u32 row = static_cast(a.h / 2u) - 2u; + row > 1u; --row) { + plot::u8 px[4] = {}; + PLOT_CHECK(ctx.read_pixel(cx, ctx.height - 1u - (static_cast(a.y) + row), px)); + if (green_pixel(px)) ++on_the_line; + } + } + std::printf("[clamped-front] ink below the middle: %u, above: %u\n", below, on_the_line); + PLOT_CHECK(below == 0u); // the two clamped points' segments are nowhere + PLOT_CHECK(on_the_line > 100u); // and the real trace is still drawn + ctx.destroy(); +} + PLOT_TEST(grid_hairlines_land_on_whole_pixels) { plottest::HeadlessContext ctx(kW, kH); plot::Status st;