Review fix pass 1 on the task 7 work; four findings, all closed.
1. The Qt demo's region sat across the middle of the window, which is exactly
where its own scripted input check pans: after the check's box zoom the press
opened DragMode::MoveRegion, the x range never moved, and the check printed
'input check pan FAIL' -- under an exit 0, because the verdict was printed and
not returned. The region now rides the left sixth (as the GLFW demo's does),
both checks pass, and the verdict is wired to the exit status. The same audit
found two more printed-only verdicts -- the GTK demo's scripted-resize layout
check and its screenshot write (and Qt's) -- now wired too. What stays
printed-only is the screenshot line of the GLFW host and the ImGui widget,
where the dump happens inside the adapter's frame loop.
2. A follow-ing link group was invisible to the input layer: a member's own axis
must be XFollow::Off (link_x() refuses a Live member), so Controller's
x_zoom_anchor() extrapolated a coordinate from the pointer instead of using
the live edge and a wheel over a row's axis strip walked the window back by
19% of its width (behind 0.000000 -> 0.285809 s). The group now publishes its
own edge to every member (Axis::live_owner + Plot::set_x_live_owner, written
by Figure::push_link, cleared when the group stops following or the row
unlinks), and x_zoom_anchor() reads it. Public-header addition, called out.
Measured after: behind 0.012992 s and the window's right edge bit-identical to
the edge the group published at the gesture.
3. The multiplot summary printed x_link_count() -- a number of groups -- under
'rows in it'. It now reports the members and the group count separately, and
every row line carries its own group id.
4. Host::controller_at() promised to clamp a pixel in a row gap to the nearest
row and only clamped below the last one, so a gap pixel went to row 0. It now
selects by distance, and tests/test_glfw_host.cpp pins it with a 40 px gap --
the first coverage of the host's figure routing, which needs no window.
New cases: tests/test_controller.cpp x2 (the anchor and the hint's lifecycle),
tests/test_glfw_host.cpp x3 (routing, the gap, the single-plot fallback): 229
cases, 0 failed, ASan clean. All five demos exit 0, the three self-checking
modes exit 0, and the amended assertions were confirmed failing against the
pre-fix behaviour.
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.
The linux-release preset inherited PLOT_BUILD_TESTS=ON, so the release build
compiled the pyramid cover/merge and ring upload/wait/slot counters into
libplot_core.a and cmake --install shipped them. Set PLOT_BUILD_TESTS=OFF in the
preset; the suite keeps running in linux-debug and linux-asan, which are
unchanged.
Also pin the two header properties the plan calls Global Constraints and that
nothing guarded: every header under include/plot/ compiles standalone, and
include/plot/plot.hpp includes every public one. tests/CMakeLists.txt globs the
headers at configure time (CONFIGURE_DEPENDS) and generates one TU per header
plus an umbrella TU that constructs Plot, Renderer, data::StreamSeries and
input::Controller; a ctest entry builds the header TUs and runs the umbrella,
and cmake/plot_umbrella_check.cmake fails configure and ctest when a public
header is missing from the umbrella.
The factor both the viewport and every forwarded input coordinate use came from
`gtk_widget_get_scale_factor()`, which GTK documents as "the next higher integer
value" under fractional scaling. It is now derived by `DeviceMapping::from_sizes()`
from the widget's logical allocation and the device size `GtkGLArea::resize`
reports -- the viewport that signal's default handler sets with
`glViewport(0, 0, w, h)` -- so the viewport, the renderer and input are one value
by construction. The public `int scale_factor()` became `f32 device_scale()`.
`kSurfacePixelsPerNotch` is now 100.0 and documented as a *calibration* with a
stated assumption, not a unit conversion attributed to axis_value120/Qt.
`::unrealize` releases the renderer unconditionally, after making the context
current: the error it used to check says nothing about whether GL names exist
(creation failure is the path that returns before a renderer is built), and
skipping the release left the deletes to the wrapper's destructor, with no
context.
The drag anchor caches its press point and offsets in logical pixels and scales
the difference by the factor in force now, so a scale change mid-drag cannot
become a phantom pan.
The demo stops its producer in `on_teardown`, before destroying the window whose
renderer the stream feeds.
The persistent-mapped instance ring had three slots and recorded a fence per
upload, so a frame recycled a slot whose fence was recorded a few draws earlier
*in that same frame*: from the fourth of the demos' nine uploads on, each one
waited on the frame's own work. It is free on a driver that retires draws
promptly (the GLFW host, 1.07 ms/frame) and ~4 ms of the Qt widget's 7.11 ms
under llvmpipe, and it grows with the trace and cursor counts this library
exists for.
* The ring grows to cover a frame's upload count, the frame's write cursor is
the ring index (so a frame writes 0, 1, 2, ... and never a slot twice), one
fence is recorded after the frame's last draw, and the only wait is at the
start of the next frame. A wait that times out, or a slot written with no
fence ever recorded behind it, drops the store to the orphan path -- the
treatment a refused fence already had -- so nothing is ever written into
storage the GPU may still be reading.
* New test-only counters (PLOT_RING_STATS, the PLOT_PYRAMID_COUNT_* pattern:
free functions, no member, so no consumer's layout changes) pin a frame's
upload and wait counts. The new tests assert the frame's slot offsets are
distinct, that the ring grew past its floor to cover the frame, and that a
frame waits at most once per store -- read from the counters and from
glClientWaitSync itself.
* qt6: print the unresolved entry-point names before the load-failure branch,
which used to return first and leave the one failure this widget can have
reporting nothing but a generic message.
* qt6: the cached pointer anchor is now in logical pixels and a move's delta is
the logical difference scaled by the DPR in effect, so a scale change between
two move events (the window dragged onto another monitor mid-drag) is no
longer a cross-scale jump applied as a pan. The anchor moved into a
toolkit-free unit (adapters/qt6/qt_pointer.hpp) with its own test.
GLFW demo headless: 1.07 -> 0.73 ms/frame. Qt widget under xvfb/llvmpipe:
7.1 -> 3.8 ms at dpr 1.0, 6.7 -> 3.6 ms at 1.25, with both upload paths still
rendering and the demo's scripted pan/box-zoom checks passing at both scales.
The widget drew a region derived from the child's content rect alone, so a child
the hosting window cut off cleared and painted over the host's pixels; it also
scaled ImGui's (-FLT_MAX, -FLT_MAX) no-pointer sentinel and differenced it against
the last real position, turning a drag out of the window into a ~1e35 pan, and it
forgot the host's button levels across a re-bind, handing the next plot a press
that never happened.
The region is now the content rect intersected with the child's clip rect (a
no-op when nothing clips it), an invalid position is no position update, and the
button levels survive a re-bind. The controller sanitises non-finite coordinates
and deltas for any host, and the widget's input translation now has a committed
test on the headless EGL context.
Frame cost was O(capacity x levels) once the ring saturated: acquire() saw the
window base move (it always moves once the ring wraps), cleared the pyramid and
re-appended all 1,048,576 retained samples, and append() walked every live level
with no early stop -- 13,647,936 bucket merges and 73.8 ms per frame at a
1M-sample window, scaling with the window, which is what capped the 1 MS/s demo
at 32-58 ms/frame while it was still filling.
Index the pyramid in absolute bucket space instead. Level l bucket k covers the
samples [k * (kBucket << l), (k + 1) * (kBucket << l)), so a bucket's span never
depends on where the window starts and the window can slide over the aggregate
instead of forcing it to be rebuilt. Each level keeps its complete buckets in a
deque plus the absolute base of its first entry; append() folds a sample into the
single in-progress level-0 bucket and, only when that bucket completes, cascades
upward, stopping at the first level that gains no new entry; a new drop_before()
slides the window by discarding leading buckets and remembers where the window
begins. acquire() is now: drop to the new base, then append exactly what was
produced -- 256 merges and 0.034 ms per frame for 8192 new samples, independent
of the window, and memory bounded by the window (32,767 entries after 50M samples
through a 1M ring, the same as after 10M). A producer that laps the consumer by
more than capacity() between two acquires restarts the window rather than
splicing a gap.
Interface: `level(l) -> std::span<const MinMax>` cannot survive a ring and is
replaced by level_base/level_size/bucket in absolute bucket coordinates, plus
window_base/window_end/drop_before. The *_buckets queries stay in window
coordinates (index 0 = oldest retained sample), so Series keeps its window
semantics for callers; its exactness split now aligns in absolute space
(window_base() + the ceil/floor) because the bucket grid is absolute. The sums
move from per-level prefix sums to per-bucket ones -- the only representation a
sliding front can keep exact -- and StreamSeries::y_bounds() becomes
reduce(0, size()), exact in O(log n + kBucket) per frame instead of a scan of the
window. The pyramid owns the published window now, so there is no second copy of
base/end to keep in step, and gather() emits only buckets that are both complete
and wholly retained.
Tests: test-only merge counters (mirroring PLOT_PYRAMID_COUNT_COVERS, absent from
every non-test build) pin an acquire proportional to production and independent
of capacity, an append cost amortised over 16 live levels, and bounded memory;
brute-force checks pin absolute bucket aggregates across slides, sliding-window
reduce/sum/sumsq over unaligned ranges, and the level-1 gather mapping. The demo
now draws 946 samples/px at 1.03 ms/frame (967 fps) where the saturated window
cost 54.96 ms/frame (18 fps).
cover_buckets chose one level per pass, so an odd first whole bucket (half of all
measurement ranges) never reached a level above 0 and folded every remaining
bucket at level 0 -- 15624 entries for a 1e6-sample pyramid. Pick the largest
aligned, in-range, materialised level at each cursor instead; the new cost test
counts folded entries through a PLOT_PYRAMID_COUNT_COVERS-only hook (absent from
normal builds) and fails on the old body.
Ring slots are now relaxed std::atomic: overwrite-on-full means the producer
reuses the slot of a snapshot's oldest sample while the consumer reads it, which
was a data race, not just a stale value. copy_from loads explicitly instead of
std::copy; push stays lock-free. A new producer/consumer case is the one TSan
flags without this.
Also extends the wrapped-window test with x0/bounds/pyramid/sum/sumsq/gather
checks, and corrects the StreamSeries threading note to the sample-count bound.
Martino Ferrari