From 3c0e7763ab8b4f5b7847b8ee699eecad54e9a9c9 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Mon, 28 Sep 2026 01:38:09 +0200 Subject: [PATCH] fix(render): one fence per frame for the vertex ring; two qt6 adapter fixes 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. --- CMakeLists.txt | 9 +- adapters/qt6/qt_plot_widget.cpp | 45 +++-- adapters/qt6/qt_plot_widget.hpp | 28 +-- adapters/qt6/qt_pointer.hpp | 49 ++++++ include/plot/render/buffer.hpp | 88 ++++++---- include/plot/render/detail/ring_stats.hpp | 62 +++++++ include/plot/render/renderer.hpp | 12 +- include/plot/render/vertex_array.hpp | 16 +- src/render/buffer.cpp | 136 ++++++++++----- src/render/renderer.cpp | 29 ++-- tests/CMakeLists.txt | 10 +- tests/test_qt_pointer.cpp | 56 ++++++ tests/test_render_smoke.cpp | 199 ++++++++++++++++++++++ 13 files changed, 626 insertions(+), 113 deletions(-) create mode 100644 adapters/qt6/qt_pointer.hpp create mode 100644 include/plot/render/detail/ring_stats.hpp create mode 100644 tests/test_qt_pointer.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 2f03d1c..be7fb03 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -62,8 +62,15 @@ if(PLOT_BUILD_TESTS) # merges so a test can pin the query's O(log n) cost and the append path's # amortised O(1) cost, neither of which any answer reveals. Off in every # non-test build (see include/plot/data/detail/{cover,merge}_count.hpp). + # + # PLOT_RING_STATS is the same for the persistent-mapped instance ring: the + # properties a frame must hold there -- no slot written twice within a frame, + # and one fence wait at most, at the start of the frame -- are invisible in the + # pixels, so a test reads counters instead (see + # include/plot/render/detail/ring_stats.hpp). target_compile_definitions(plot_core PRIVATE PLOT_PYRAMID_COUNT_COVERS - PLOT_PYRAMID_COUNT_MERGES) + PLOT_PYRAMID_COUNT_MERGES + PLOT_RING_STATS) enable_testing() add_subdirectory(tests) diff --git a/adapters/qt6/qt_plot_widget.cpp b/adapters/qt6/qt_plot_widget.cpp index 0726286..7b3dbe5 100644 --- a/adapters/qt6/qt_plot_widget.cpp +++ b/adapters/qt6/qt_plot_widget.cpp @@ -1,4 +1,5 @@ #include +#include #include @@ -114,21 +115,26 @@ void PlotWidget::initializeGL() { renderer_.reset(); g_unresolved_count = 0; - if (!plot::gl::load(&resolve_via_qt)) { + const bool loaded = plot::gl::load(&resolve_via_qt); + // Printed before the failure branch, not after it: the names collected here are + // precisely what explains a failure -- which 3.3 entry point this context could + // not provide -- so a load that failed must report them too, or the one failure + // this widget can have says nothing but "missing a required entry point". + if (g_unresolved_count > 0) { + std::fprintf(stderr, + "[plot::qt6] QOpenGLContext::getProcAddress() did not resolve %u required" + " entry point(s); plot::gl::load() %s for:", + g_unresolved_count, loaded ? "used its dlsym fallback" : "failed"); + for (u32 i = 0; i < g_unresolved_count; ++i) std::fprintf(stderr, " %s", g_unresolved[i]); + std::fprintf(stderr, "\n"); + } + if (!loaded) { init_status_ = Status{ErrorCode::Unsupported, "plot::gl::load() failed: the current context is missing a required" " OpenGL 3.3 entry point"}; std::fprintf(stderr, "[plot::qt6] %s\n", init_status_.message); return; } - if (g_unresolved_count > 0) { - std::fprintf(stderr, - "[plot::qt6] QOpenGLContext::getProcAddress() did not resolve %u required" - " entry point(s); plot::gl::load() used its dlsym fallback for:", - g_unresolved_count); - for (u32 i = 0; i < g_unresolved_count; ++i) std::fprintf(stderr, " %s", g_unresolved[i]); - std::fprintf(stderr, "\n"); - } renderer_.emplace(); Status status; @@ -206,19 +212,24 @@ Vec2 PlotWidget::to_device(const QPointF& p) const noexcept { void PlotWidget::refresh_viewport() { controller_.set_viewport(device_viewport()); } void PlotWidget::forward_pointer(QMouseEvent* event, bool pressed, bool released) { - const Vec2 p = to_device(event->position()); + // The anchor stays logical (see PointerAnchor): a move that arrives after the + // window moved to a display with another scale factor must not measure the gap + // between a position taken at the old DPR and one taken at the new one. The + // delta is the logical difference scaled by the ratio in effect now, so the same + // gesture maps to the same device distance at either scale. + const QPointF position = event->position(); + const f64 dpr = static_cast(devicePixelRatioF()); + const Vec2 logical{static_cast(position.x()), static_cast(position.y())}; + const Vec2 p = to_device(position); + const Vec2 delta = anchor_.device_delta(logical, dpr); input::PointerEvent e; e.x = p.x; e.y = p.y; - if (have_pointer_) { - e.dx = p.x - last_x_; - e.dy = p.y - last_y_; - } + e.dx = delta.x; + e.dy = delta.y; // A press is also a pointer sample: the next move's delta starts here, or the // first motion of a drag would include the jump to the click. - last_x_ = p.x; - last_y_ = p.y; - have_pointer_ = true; + anchor_.set(logical); e.button = button_from(event->button()); e.mods = mods_from(event->modifiers()); e.down = any_plot_button(event->buttons()); diff --git a/adapters/qt6/qt_plot_widget.hpp b/adapters/qt6/qt_plot_widget.hpp index dc32cc3..e45c468 100644 --- a/adapters/qt6/qt_plot_widget.hpp +++ b/adapters/qt6/qt_plot_widget.hpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -80,14 +81,21 @@ namespace plot::qt { /// * the viewport handed to the plot is `round(width() * dpr)` x /// `round(height() * dpr)` (rounded, not truncated, so the last column of the /// FBO is inside the plot), and -/// * every pointer, wheel and key anchor is multiplied by the same -/// `devicePixelRatioF()` before it reaches the controller, deltas included. +/// * every pointer and wheel position is multiplied by the same +/// `devicePixelRatioF()` before it reaches the controller. /// /// `device_viewport()` and `to_device()` are the only two places that mapping /// exists, so the viewport and the input cannot disagree -- and they must not: a /// mismatch renders correctly but pans by the wrong amount, which is invisible on /// an unscaled display and wrong on every other one. /// +/// The *anchor* a move's delta is measured from is the one thing kept in logical +/// pixels: `devicePixelRatioF()` can change between two move events, and the delta +/// is then a logical difference scaled by the ratio in effect now, never the +/// difference of two device positions taken at different ratios (see +/// `PointerAnchor`). A drag that crosses onto a display with another scale factor +/// therefore pans by the distance the pointer moved, not by a cross-scale jump. +/// /// ## Input /// /// Qt reports levels and edges directly (`QMouseEvent` press/release are edges, @@ -174,9 +182,10 @@ class PlotWidget : public QOpenGLWidget, protected QOpenGLFunctions { /// The widget's rectangle in device pixels: the plot's viewport, and the size /// of the framebuffer the frame is drawn into. Rect device_viewport() const noexcept; - /// A Qt position (logical widget pixels) in device pixels. The one place the - /// input mapping exists, so every event is scaled by the same factor the - /// viewport is. + /// A Qt position (logical widget pixels) in device pixels. The one place a + /// pointer position is mapped, so everything the controller sees is scaled by + /// the same factor the viewport is -- a move's *delta*, whose ratio can change + /// between two events, is scaled by `PointerAnchor` instead. Vec2 to_device(const QPointF& p) const noexcept; /// Translates one mouse event and hands it to the controller. `pressed` and /// `released` are the transitions Qt reports on the event itself. @@ -191,11 +200,10 @@ class PlotWidget : public QOpenGLWidget, protected QOpenGLFunctions { FrameFn frame_; Status init_status_; - /// Last pointer position in device pixels: the deltas the controller pans with - /// are the movement since the previous event, which Qt does not report. - bool have_pointer_ = false; - f32 last_x_ = 0.0f; - f32 last_y_ = 0.0f; + /// The position a move's delta is measured from, in **logical** pixels: Qt + /// reports no delta of its own, and the ratio can change between two events + /// (see `PointerAnchor` for why the anchor must not be device pixels). + PointerAnchor anchor_; u64 frames_ = 0; f32 last_frame_ms_ = 0.0f; diff --git a/adapters/qt6/qt_pointer.hpp b/adapters/qt6/qt_pointer.hpp new file mode 100644 index 0000000..da99d58 --- /dev/null +++ b/adapters/qt6/qt_pointer.hpp @@ -0,0 +1,49 @@ +#pragma once + +#include +#include + +namespace plot::qt { + +/// The cached pointer position a move event's movement is measured from, in +/// **logical** widget pixels. +/// +/// Logical, not device, pixels on purpose. Qt reports every widget position in +/// logical pixels, and the device-pixel ratio can change between two move events +/// -- the window being dragged onto a monitor with another scale factor is the +/// case -- so an anchor cached in device pixels would compare a position computed +/// at the old ratio with one computed at the new one. At 1.0 -> 1.25 a logical +/// position of 500 would then look like a jump of 125 device pixels, and +/// `Controller::pan` applies a delta as a fraction of the range span (and clears +/// auto scale), so a scale change mid-drag would pan the plot by 125 pixels' worth +/// of data. Keeping the anchor logical and scaling the *difference* by the ratio +/// in effect now makes the same gesture map to the same device distance whatever +/// the window's scale is. +/// +/// A plain value with no toolkit dependency: the widget owns one, and this is the +/// part of the adapter's translation a test can drive directly (see +/// tests/test_qt_pointer.cpp). +struct PointerAnchor { + bool have = false; ///< false until the first sample, which has no movement + f32 x = 0.0f; ///< last logical position, widget pixels + f32 y = 0.0f; + + /// Movement from the cached position to `logical`, in device pixels at `dpr`. + /// Zero before the first sample, so the first motion of a drag never includes + /// the jump to where the drag began. + Vec2 device_delta(Vec2 logical, f64 dpr) const noexcept { + if (!have) return Vec2{0.0f, 0.0f}; + return Vec2{static_cast((static_cast(logical.x) - static_cast(x)) * dpr), + static_cast((static_cast(logical.y) - static_cast(y)) * dpr)}; + } + + /// Caches `logical` as the position the next movement is measured from. Always + /// logical pixels, whatever the ratio is: see the type's comment. + void set(Vec2 logical) noexcept { + x = logical.x; + y = logical.y; + have = true; + } +}; + +} // namespace plot::qt diff --git a/include/plot/render/buffer.hpp b/include/plot/render/buffer.hpp index 9761072..22d99e5 100644 --- a/include/plot/render/buffer.hpp +++ b/include/plot/render/buffer.hpp @@ -35,10 +35,12 @@ static_assert(sizeof(TextInstance) == 4 * sizeof(f32), "text instance must stay /// - `Orphan`: the GL 3.3 baseline. Every upload re-specifies the store with a /// null `glBufferData` (orphaning it, so the driver never has to wait for the /// draw that is still reading the previous one) and then refills it. -/// - `PersistentMapped`: GL 4.4 / ARB_buffer_storage. Three slots of immutable -/// storage are mapped once and written directly, and the fence recorded after -/// each slot's draw guards its reuse, so the render thread never waits on the -/// GPU. The vertex array's attribute pointers move with the slot. +/// - `PersistentMapped`: GL 4.4 / ARB_buffer_storage. Immutable storage is mapped +/// once and written directly, one slot per upload. A slot is never written +/// twice within a frame, one fence is recorded after the frame's last draw, and +/// the only wait is at the start of the next frame -- so the render thread +/// never synchronises with GPU work it has just issued. The vertex array's +/// attribute pointers move with the slot. enum class UploadMode { Orphan, PersistentMapped }; namespace detail { @@ -50,11 +52,13 @@ namespace detail { /// `UploadMode` is live. class InstanceStore { public: - /// Slots the persistent ring rotates through. Three is the smallest ring that - /// lets a frame's draw finish while the CPU fills two more. - static constexpr u32 kSlots = 3; - /// Upper bound on the wait for a slot's draw to retire: a driver that has - /// stopped making progress must not stall the render thread forever. + /// Slots the persistent ring starts with. The floor, not the ring's size: it + /// grows to cover the uploads a frame actually performs (see `upload()`), and + /// never shrinks, so a steady workload grows it at most a logarithmic number of + /// times over the life of the store. + static constexpr u32 kInitialSlots = 3; + /// Upper bound on the wait for the previous frame's draws to retire: a driver + /// that has stopped making progress must not stall the render thread forever. static constexpr u64 kFenceWaitNs = 1'000'000'000ull; /// Creates the vertex buffer in `mode`. Reports `ErrorCode::Unsupported` while @@ -64,19 +68,34 @@ class InstanceStore { /// store created that way on a machine without them would never draw. bool create(UploadMode mode, Status* st = nullptr) noexcept; + /// Starts a frame: waits, once, for the frame before it to retire the draws + /// that read the ring, so no slot about to be written can still be being read. + /// False when that wait timed out -- or when a slot was written without a fence + /// ever being recorded for it -- in which case the store has dropped to the + /// baseline path, which has no such hazard, and the caller reports it (see + /// `Renderer::begin_frame`). + /// + /// One wait covers every slot because no slot is written twice within a frame + /// (`upload()` grows the ring rather than wrapping it), so each slot this frame + /// touches was last written in the previous frame or earlier -- and the + /// previous frame's fence sits behind every draw that read one. + bool begin_frame() noexcept; + /// Uploads `count` instances of `instance_bytes`. `Orphan` mode orphans and - /// refills; `PersistentMapped` mode writes the next slot of the ring, waiting - /// out any draw still reading it. + /// refills; `PersistentMapped` mode writes the next slot of the ring, growing + /// the ring first when the frame has already written all of its slots. void upload(const void* data, u32 count, usize instance_bytes) noexcept; - /// Records that the draw the caller just issued read the current slot, so the - /// fence guarding that slot's reuse ends up in the command stream behind the - /// draw. True when the slot is guarded, and always in `Orphan` mode, where - /// nothing reads the store after the orphan. + /// Records the frame's single fence, behind every draw that read the ring, so + /// the next `begin_frame()` can wait for them. Called once per frame, after the + /// last draw that consumes the store. True in `Orphan` mode, where nothing + /// reads the store after the orphan, and true when the frame drew nothing from + /// the ring. /// - /// False when the driver refused the fence: the slot would stay unsynchronised, - /// so the store drops the mapped ring for the orphan path instead of rotating - /// through it (the caller reports the failure -- see `Renderer`). + /// False when the driver refused the fence: the slots written this frame would + /// stay unsynchronised, so the store drops the mapped ring for the orphan path + /// instead of rotating through it (the caller reports the failure -- see + /// `Renderer::end_frame`). bool mark_drawn() noexcept; /// Byte offset the current slot's instances start at: 0 in `Orphan` mode, @@ -101,24 +120,33 @@ class InstanceStore { bool persistent() const noexcept { return mode_ == UploadMode::PersistentMapped; } private: - /// Grows the mapped ring to `bytes` per slot. False when the driver refuses the - /// storage or the map, in which case the store has fallen back to a fresh - /// `Orphan` buffer and the frame still draws. - bool grow_persistent(usize bytes) noexcept; - /// Deletes every fence, unmaps and releases the GL buffer. `slot_bytes_` drops - /// to 0 so the next upload regrows. + /// (Re)allocates the mapped ring with `slots` slots of `bytes` each. False when + /// the driver refuses the storage or the map, in which case the store has fallen + /// back to a fresh `Orphan` buffer and the frame still draws. + bool grow_persistent(usize bytes, u32 slots) noexcept; + /// Deletes the frame fence, unmaps and releases the GL buffer. `slot_bytes_` + /// drops to 0 so the next upload regrows. void release_storage() noexcept; + /// Abandons the mapped ring for the baseline path and re-creates the empty + /// buffer that path needs. Used wherever the ring can no longer be guarded. + void drop_to_orphan() noexcept; gl::Buffer buffer_; UploadMode mode_ = UploadMode::Orphan; - u32 capacity_ = 0; ///< instances the orphan store can hold - usize slot_bytes_ = 0; ///< bytes per slot of the persistent ring - u32 slot_ = kSlots - 1u; ///< last slot written; an upload advances it first + u32 capacity_ = 0; ///< instances the orphan store can hold + usize slot_bytes_ = 0; ///< bytes per slot of the persistent ring + u32 slot_count_ = kInitialSlots; ///< slots the ring has; grows, never shrinks + u32 slot_ = 0; ///< slot the last upload wrote + /// Slots written since this frame began, and the index the next upload takes. + /// It is the ring index itself, which is what keeps a frame's slots distinct. + u32 frame_uploads_ = 0; /// Times an upload landed in a new slot. Kept apart from the buffer's own /// identity (see `epoch()`) so neither has to know about the other. u32 slot_epoch_ = 0; - u8* mapped_ = nullptr; ///< base of the mapped ring, null in `Orphan` mode - gl::Fence fences_[kSlots]; + u8* mapped_ = nullptr; ///< base of the mapped ring, null in `Orphan` mode + /// Guards every slot the frame just ended wrote. Recorded once, after its last + /// draw; waited on once, at the start of the next frame. + gl::Fence frame_fence_; }; } // namespace detail @@ -129,6 +157,7 @@ class LineBuffer { bool create(Status* st = nullptr, UploadMode mode = UploadMode::Orphan) noexcept { return store_.create(mode, st); } + bool begin_frame() noexcept { return store_.begin_frame(); } void upload(const f32* interleaved, u32 count) noexcept { store_.upload(interleaved, count, sizeof(LineInstance)); } @@ -148,6 +177,7 @@ class TextBuffer { bool create(Status* st = nullptr, UploadMode mode = UploadMode::Orphan) noexcept { return store_.create(mode, st); } + bool begin_frame() noexcept { return store_.begin_frame(); } void upload(const f32* interleaved, u32 count) noexcept { store_.upload(interleaved, count, sizeof(TextInstance)); } diff --git a/include/plot/render/detail/ring_stats.hpp b/include/plot/render/detail/ring_stats.hpp new file mode 100644 index 0000000..860d589 --- /dev/null +++ b/include/plot/render/detail/ring_stats.hpp @@ -0,0 +1,62 @@ +#pragma once + +#ifdef PLOT_RING_STATS + +#include + +#include + +namespace plot::render::detail { + +/// Test-only instrumentation for the persistent-mapped instance ring (see +/// `InstanceStore`). Every counter exists only when PLOT_RING_STATS is defined +/// -- the test build defines it, on the library and on the suite -- and none of +/// them is a data member of any class, so a consumer compiled without the macro +/// still lays `InstanceStore` out identically. No other build carries a counter +/// or an increment on the upload, frame or wait path. +/// +/// The properties they pin are invisible in the pixels a frame draws: a ring that +/// syncs mid-frame rasterises exactly what one that does not does, and a ring that +/// recycles a slot within a frame draws correctly until the GPU happens to be +/// slower than the CPU. So `InstanceStore` counts +/// +/// * `ring_uploads()` -- one per upload, i.e. a frame's upload count; +/// * `ring_waits()` -- one per fence wait. At most one per store per frame: +/// the wait is at the start of the frame, on the previous frame's fence; +/// * `ring_slots()` -- the largest ring any store has grown to, which must +/// cover the uploads a frame performs. +/// +/// They are process-global rather than per-store because a test that drives one +/// `InstanceStore` reads them directly; a test that drives the whole `Renderer` +/// reads the aggregate, where there are two stores and so at most two waits a +/// frame. +/// +/// Not part of any query surface and not meant for production use. +inline std::atomic& ring_uploads() { + static std::atomic count{0}; + return count; +} + +inline std::atomic& ring_waits() { + static std::atomic count{0}; + return count; +} + +inline std::atomic& ring_slots() { + static std::atomic count{0}; + return count; +} + +/// Records `slots` as the ring's largest size so far. A compare-exchange loop +/// rather than a store because two stores report into one counter. +inline void note_ring_slots(u64 slots) noexcept { + std::atomic& count = ring_slots(); + u64 largest = count.load(std::memory_order_relaxed); + while (largest < slots && + !count.compare_exchange_weak(largest, slots, std::memory_order_relaxed)) { + } +} + +} // namespace plot::render::detail + +#endif // PLOT_RING_STATS diff --git a/include/plot/render/renderer.hpp b/include/plot/render/renderer.hpp index 35b7406..557c4b6 100644 --- a/include/plot/render/renderer.hpp +++ b/include/plot/render/renderer.hpp @@ -80,6 +80,12 @@ class Renderer { /// `viewport_px` (top-left origin, like every `Rect` in this library) into GL's /// bottom-left origin once, clears that region to `style().background` and /// empties the staging batches. + /// + /// It is also where the instance stores wait -- once each -- for the previous + /// frame's draws to retire the ring they read, before this frame writes any + /// slot (see `InstanceStore::begin_frame`): a frame never synchronises with the + /// work it issued itself. A store that cannot retire that frame drops to the + /// baseline path, which this frame reports (see `last_frame_had_gl_error()`). void begin_frame(u32 fb_w, u32 fb_h, Rect viewport_px) noexcept; /// Draws `plot`, refreshing its layout, ticks and grid first. @@ -98,6 +104,10 @@ class Renderer { /// Ends the frame, draining `glGetError()` in a loop so a stale error cannot be /// attributed to a later frame (see `last_frame_had_gl_error()`). /// + /// It also records each instance store's single fence for the frame, behind + /// every draw that read the ring, which is what the next `begin_frame()` waits + /// on (see `InstanceStore::mark_drawn`). + /// /// The GL state it leaves behind, so a host can embed the plot without guessing: /// vertex array 0, program 0, `GL_ARRAY_BUFFER` 0, texture unit 0 active and /// nothing bound to `GL_TEXTURE_2D` on it, blending off, scissoring off, and the @@ -115,7 +125,7 @@ class Renderer { /// True when the last frame reported a GL failure: an error drained from /// `glGetError()` in `end_frame()`, or a fence the driver refused, which the - /// store reports when the draw that should have been guarded is issued (such a + /// store reports when the frame's fence is recorded or waited on (such a /// frame has already fallen back to the baseline path). Errors are drained in a /// loop, so a stale one cannot be attributed to a later frame. bool last_frame_had_gl_error() const noexcept { return gl_error_; } diff --git a/include/plot/render/vertex_array.hpp b/include/plot/render/vertex_array.hpp index 614edd8..336477a 100644 --- a/include/plot/render/vertex_array.hpp +++ b/include/plot/render/vertex_array.hpp @@ -30,11 +30,14 @@ class LineVertexArray { void upload(const f32* interleaved, u32 segments) noexcept; void bind() const noexcept { vao_.bind(); } - /// Records that the draw just issued read the instance buffer's current slot, - /// which is what lets the mapped path reuse it safely. True in the baseline - /// mode, so a caller always calls it the same way. False only when the driver - /// refused the fence: the buffer has then fallen back to the baseline path, and - /// `persistent()` says so from the next call on. + /// Waits, once, for the previous frame's draws to retire the ring before any + /// slot of this frame is written (see `InstanceStore::begin_frame`). False when + /// the store had to drop to the baseline path; the caller reports it. + bool begin_frame() noexcept { return buffer_.begin_frame(); } + /// Records the frame's single fence, behind every draw that read the ring (see + /// `InstanceStore::mark_drawn`); called once per frame, after its last draw. + /// False only when the driver refused the fence: the buffer has then fallen + /// back to the baseline path, and `persistent()` says so from the next call on. bool mark_drawn() noexcept { return buffer_.mark_drawn(); } /// True while this array's instance buffer is the mapped ring (see /// `UploadMode`); a failure that falls back clears it. @@ -62,6 +65,9 @@ class TextVertexArray { void upload(const f32* interleaved, u32 glyphs) noexcept; void bind() const noexcept { vao_.bind(); } + /// The line pipeline's frame pair, for the text store (see + /// `LineVertexArray::begin_frame` / `mark_drawn`). + bool begin_frame() noexcept { return buffer_.begin_frame(); } bool mark_drawn() noexcept { return buffer_.mark_drawn(); } bool persistent() const noexcept { return buffer_.persistent(); } diff --git a/src/render/buffer.cpp b/src/render/buffer.cpp index f702137..859761f 100644 --- a/src/render/buffer.cpp +++ b/src/render/buffer.cpp @@ -1,7 +1,9 @@ #include #include +#include +#include #include namespace plot::render { @@ -16,7 +18,9 @@ bool InstanceStore::create(UploadMode mode, Status* st) noexcept { } capacity_ = 0; slot_bytes_ = 0; - slot_ = kSlots - 1u; // the first upload takes slot 0 + slot_count_ = kInitialSlots; + slot_ = 0; + frame_uploads_ = 0; // The mapped path additionally needs its entry points, which are resolved // best-effort (see api.hpp): without them the store would never be able to // write anything, so it stays on the baseline path instead. @@ -34,16 +38,26 @@ void InstanceStore::release_storage() noexcept { buffer_.unmap(); mapped_ = nullptr; } - for (gl::Fence& fence : fences_) { - fence.reset(); - } + frame_fence_.reset(); buffer_.reset(); slot_bytes_ = 0; } -bool InstanceStore::grow_persistent(usize bytes) noexcept { - // Deleting the old buffer is safe while the GPU may still be reading it: GL - // keeps the storage alive until the last command that references it retires. +void InstanceStore::drop_to_orphan() noexcept { + // The hazard the ring exists to prevent -- writing storage the GPU may still be + // reading -- can no longer be guarded, and a frame that draws through the + // baseline path beats one that draws nothing. The old buffer is deleted while + // the GPU may still read it, which is safe: GL keeps its storage alive until the + // last command that references it retires. + release_storage(); + mode_ = UploadMode::Orphan; + capacity_ = 0; + frame_uploads_ = 0; + Status retry; + (void)buffer_.create(&retry); // a failed retry leaves nothing to draw with +} + +bool InstanceStore::grow_persistent(usize bytes, u32 slots) noexcept { release_storage(); Status st; if (!buffer_.create(&st)) { @@ -53,8 +67,9 @@ bool InstanceStore::grow_persistent(usize bytes) noexcept { } const gl::GLbitfield flags = gl::GL_MAP_WRITE_BIT | gl::GL_MAP_PERSISTENT_BIT | gl::GL_MAP_COHERENT_BIT; - buffer_.storage(kSlots * bytes, flags); - mapped_ = static_cast(buffer_.map_range(0, kSlots * bytes, flags)); + const usize total = usize(slots) * bytes; + buffer_.storage(total, flags); + mapped_ = static_cast(buffer_.map_range(0, total, flags)); if (mapped_ == nullptr) { // The driver refused the storage or the mapping. A frame that draws through // the baseline path beats one that draws nothing, so re-create the empty @@ -62,25 +77,72 @@ bool InstanceStore::grow_persistent(usize bytes) noexcept { // array's attribute pointers went out with the buffer they captured, and the // epoch reports it (`gl::Buffer::generation()`), which is what re-points them // at this buffer before the draw. - release_storage(); - mode_ = UploadMode::Orphan; - capacity_ = 0; - Status retry; - (void)buffer_.create(&retry); // a failed retry leaves nothing to draw with + drop_to_orphan(); return false; } slot_bytes_ = bytes; - slot_ = kSlots - 1u; // fresh storage: the first upload takes slot 0 + slot_count_ = slots; + slot_ = 0; + // Fresh storage: every one of its slots is unwritten, so the frame's slot + // cursor restarts at 0. The draws that read the old buffer are already in the + // command stream and keep it alive until they retire. + frame_uploads_ = 0; +#ifdef PLOT_RING_STATS + note_ring_slots(slot_count_); +#endif return true; } +bool InstanceStore::begin_frame() noexcept { + const u32 written = frame_uploads_; + frame_uploads_ = 0; + if (mode_ != UploadMode::PersistentMapped || mapped_ == nullptr) { + return true; // nothing the GPU reads, so no slot has to be waited for + } + if (written == 0) { + return true; // the frame before this one never read the ring + } + if (!frame_fence_) { + // Slots were written and no fence was ever recorded for them: the caller + // skipped mark_drawn(), so those slots are unguarded and writing any of them + // again could race a read still in flight. The ring is abandoned rather than + // reused, exactly as a refused fence's is. + drop_to_orphan(); + return false; + } +#ifdef PLOT_RING_STATS + ring_waits().fetch_add(1, std::memory_order_relaxed); +#endif + // The slots this frame is about to write were all last written a frame ago, and + // this one fence sits behind every draw that read them -- unless the driver has + // not retired it, in which case writing would race the GPU and the ring is + // abandoned for the baseline path. The non-blocking check is the common case. + const bool retired = frame_fence_.wait(0) || frame_fence_.wait(kFenceWaitNs); + frame_fence_.reset(); + if (retired) { + return true; + } + drop_to_orphan(); + return false; +} + void InstanceStore::upload(const void* data, u32 count, usize instance_bytes) noexcept { if (buffer_.id() == 0 || data == nullptr || count == 0 || instance_bytes == 0) { return; } const usize bytes = usize(count) * instance_bytes; - if (mode_ == UploadMode::PersistentMapped && bytes > slot_bytes_ && !grow_persistent(bytes)) { - // The fallback already replaced the store with an orphan buffer. + if (mode_ == UploadMode::PersistentMapped && + (bytes > slot_bytes_ || frame_uploads_ >= slot_count_)) { + // The frame has written every slot the ring has, or this instance payload is + // wider than a slot: grow rather than wrap. Covering the frame's upload count + // is what lets a single fence, recorded after its last draw, guard all of its + // slots -- and doubling makes the growth rare for a workload whose upload + // count has settled. + const usize want_bytes = std::max(bytes, slot_bytes_); + const u32 want_slots = frame_uploads_ >= slot_count_ + ? std::max(frame_uploads_ + 1u, slot_count_ * 2u) + : slot_count_; + (void)grow_persistent(want_bytes, want_slots); // a failure leaves the orphan path } if (mode_ == UploadMode::Orphan) { @@ -92,18 +154,16 @@ void InstanceStore::upload(const void* data, u32 count, usize instance_bytes) no return; } - // Advance the ring, then make sure the draw that last read this slot has - // retired. The non-blocking check is the common case; the bounded wait only - // costs anything when three frames' geometry is still in flight. - slot_ = (slot_ + 1u) % kSlots; - if (fences_[slot_]) { - if (!fences_[slot_].wait(0)) { - (void)fences_[slot_].wait(kFenceWaitNs); - } - fences_[slot_].reset(); - } + // The slot this frame has not written yet: the frame's write cursor is the ring + // index, so a frame writes 0, 1, 2, ... and never reaches a slot it already + // filled. That is the whole reason one fence at the end of the frame is enough. + slot_ = frame_uploads_; +#ifdef PLOT_RING_STATS + ring_uploads().fetch_add(1, std::memory_order_relaxed); +#endif // MAP_COHERENT storage needs no explicit flush, so this is the whole transfer. std::memcpy(mapped_ + usize(slot_) * slot_bytes_, data, bytes); + ++frame_uploads_; ++slot_epoch_; } @@ -111,22 +171,20 @@ bool InstanceStore::mark_drawn() noexcept { if (mode_ != UploadMode::PersistentMapped || mapped_ == nullptr) { return true; // nothing reads the store after the draw } + if (frame_uploads_ == 0) { + return true; // no draw read the ring this frame, so it needs no guard + } Status st; - fences_[slot_].reset(); - if (fences_[slot_].create(&st)) { + frame_fence_.reset(); + if (frame_fence_.create(&st)) { return true; } - // A slot whose fence the driver refused can never be reused safely: the next + // A frame whose fence the driver refused leaves the slots it wrote + // unsynchronised, and such a slot can never be written again safely: the next // upload would memcpy into storage the GPU may still be reading, which is the - // tearing the ring exists to prevent. So the ring is abandoned rather than - // rotated through -- the map failure's treatment, one step later -- and the - // fresh buffer the orphan path needs is created. Deleting the mapped buffer is - // safe while the draw still reads it: GL keeps the storage alive until then. - release_storage(); - mode_ = UploadMode::Orphan; - capacity_ = 0; - Status retry; - (void)buffer_.create(&retry); // a failed retry leaves nothing to draw with + // tearing the ring exists to prevent. So the ring is abandoned for the orphan + // path, which is what the map failure's treatment does one step earlier. + drop_to_orphan(); return false; } diff --git a/src/render/renderer.cpp b/src/render/renderer.cpp index 80484bb..ff8bcbf 100644 --- a/src/render/renderer.cpp +++ b/src/render/renderer.cpp @@ -163,6 +163,16 @@ void Renderer::begin_frame(u32 fb_w, u32 fb_h, Rect viewport_px) noexcept { return; } + // One wait per store, before any slot of this frame is written: the ring never + // synchronises with the work its own frame issued, only with the frame before + // it. A store that cannot retire that frame has dropped to the baseline path, + // which the frame reports. + const bool lines_retired = line_vao_.begin_frame(); + const bool glyphs_retired = text_vao_.begin_frame(); + if (!lines_retired || !glyphs_retired) { + gl_error_ = true; + } + const gl::GLint vx = static_cast(gl_viewport_.x); const gl::GLint vy = static_cast(gl_viewport_.y); const gl::GLsizei vw = static_cast(gl_viewport_.w); @@ -225,6 +235,15 @@ void Renderer::end_frame() noexcept { if (!ready_) { return; } + // The frame's single fence per store, behind every draw that read the ring: the + // next frame's begin_frame() waits for it, and until then nothing is written to + // a slot these draws may still be reading. Recording it here -- after the last + // draw, not after each upload -- is what keeps a frame from waiting on itself. + const bool lines_guarded = line_vao_.mark_drawn(); + const bool glyphs_guarded = text_vao_.mark_drawn(); + if (!lines_guarded || !glyphs_guarded) { + 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 @@ -270,13 +289,6 @@ void Renderer::flush_lines(const Mapping& map, Color color, f32 width) noexcept // One triangle strip per instance, four corners from gl_VertexID. gl::glDrawArraysInstanced(gl::GL_TRIANGLE_STRIP, 0, 4, static_cast(line_vao_.instance_count())); - // The draw is in the command stream now, so this is the point at which the - // mapped path can let a fence follow it. A driver that refused the fence has - // already dropped this store to the baseline path; the frame reports it here - // rather than rotating through a slot nothing guards. - if (!line_vao_.mark_drawn()) { - gl_error_ = true; - } stats_.staged_segments += segments; ++stats_.draw_calls; } @@ -299,9 +311,6 @@ void Renderer::flush_text() noexcept { text_vao_.bind(); gl::glDrawArraysInstanced(gl::GL_TRIANGLE_STRIP, 0, 4, static_cast(text_vao_.instance_count())); - if (!text_vao_.mark_drawn()) { - gl_error_ = true; - } ++stats_.draw_calls; } glyphs_.clear(); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 151b673..832c267 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -51,10 +51,18 @@ if(TARGET plot::imgui) target_link_libraries(plot_tests PRIVATE plot::imgui imgui) endif() +# The Qt adapter's pointer anchor is deliberately toolkit-free (see +# adapters/qt6/qt_pointer.hpp), so this one piece of that adapter's translation is +# tested without linking Qt -- and without `PLOT_BUILD_QT6`, since no Qt target has +# to exist for it to compile. +target_sources(plot_tests PRIVATE test_qt_pointer.cpp) +target_include_directories(plot_tests PRIVATE "${CMAKE_SOURCE_DIR}/adapters") + # Same switches the library is built with: the cost tests read the counters # behind them (see include/plot/data/detail/{cover,merge}_count.hpp). target_compile_definitions(plot_tests PRIVATE PLOT_PYRAMID_COUNT_COVERS - PLOT_PYRAMID_COUNT_MERGES) + PLOT_PYRAMID_COUNT_MERGES + PLOT_RING_STATS) plot_apply_warnings(plot_tests) add_test(NAME plot_tests COMMAND plot_tests) diff --git a/tests/test_qt_pointer.cpp b/tests/test_qt_pointer.cpp new file mode 100644 index 0000000..36d9018 --- /dev/null +++ b/tests/test_qt_pointer.cpp @@ -0,0 +1,56 @@ +// The Qt adapter's pointer translation, driven directly. The widget's cached +// pointer anchor is a plain value with no toolkit dependency (see +// adapters/qt6/qt_pointer.hpp) precisely so the one thing a screenshot cannot show +// -- which pixel space a drag delta is in -- is testable without a QApplication, a +// window or a display. +#include "harness.hpp" + +#include + +#include + +// The defect this pins: the anchor used to be cached in *device* pixels while the +// next event's delta was a raw difference of device positions, so a DPR change +// between two move events -- the window dragged onto a monitor with a different +// scale factor mid-drag -- mixed two pixel spaces. At 1.0 -> 1.25 a logical +// position of 500 produced a spurious 125 device-pixel delta, which +// `Controller::pan` applies as a fraction of the range span and which also clears +// auto scale: a scale change mid-drag would move the plot by 125 pixels' worth of +// data and silently disarm a live axis. +PLOT_TEST(pointer_anchor_survives_a_dpr_change_between_move_events) { + plot::qt::PointerAnchor anchor; + + // The press caches a logical position while the window is on the 1.0 display. + anchor.set({500.0f, 300.0f}); + + // The next move arrives after the window moved to the 1.25 display, at the same + // logical position: the pointer has not moved, so the delta must be zero -- not + // 500 * 1.25 - 500 * 1.0 = 125 device pixels, which is what comparing two device + // positions across the scale change reported. + const plot::Vec2 still = anchor.device_delta({500.0f, 300.0f}, 1.25); + PLOT_CHECK_NEAR(still.x, 0.0, 1e-6); + PLOT_CHECK_NEAR(still.y, 0.0, 1e-6); + + // A real movement is the logical delta scaled by the ratio in effect now, in both + // directions. + const plot::Vec2 moved = anchor.device_delta({503.0f, 295.0f}, 1.25); + PLOT_CHECK_NEAR(moved.x, 3.0 * 1.25, 1e-6); + PLOT_CHECK_NEAR(moved.y, -5.0 * 1.25, 1e-6); + + // And the anchor follows the sample it just reported: the following move is + // measured from there, at whatever ratio that event carries. + anchor.set({503.0f, 295.0f}); + const plot::Vec2 unchanged = anchor.device_delta({503.0f, 295.0f}, 1.0); + PLOT_CHECK_NEAR(unchanged.x, 0.0, 1e-6); + PLOT_CHECK_NEAR(unchanged.y, 0.0, 1e-6); +} + +// The first sample of a gesture has no anchor to measure from, so its delta is +// zero however far the position is from the origin: the first motion of a drag +// must never include the jump to where the drag began. +PLOT_TEST(pointer_anchor_reports_no_movement_before_its_first_sample) { + plot::qt::PointerAnchor anchor; + const plot::Vec2 delta = anchor.device_delta({500.0f, 300.0f}, 1.25); + PLOT_CHECK_NEAR(delta.x, 0.0, 1e-9); + PLOT_CHECK_NEAR(delta.y, 0.0, 1e-9); +} diff --git a/tests/test_render_smoke.cpp b/tests/test_render_smoke.cpp index 8918e04..ff76fd5 100644 --- a/tests/test_render_smoke.cpp +++ b/tests/test_render_smoke.cpp @@ -9,8 +9,11 @@ #include #include #include +#include +#include #include +#include #include #include #include @@ -151,6 +154,37 @@ class RefuseFenceSync { decltype(plot::gl::glFenceSync) saved_; }; +/// Counts the calls the ring's fence waits make through `glClientWaitSync` -- the +/// one entry point every wait goes through, whatever the store's own bookkeeping +/// says. The test-only counters in report how +/// often a store decides to wait; this reports what reached the driver, so the two +/// cannot agree by construction. +u64 g_fence_wait_calls = 0; +decltype(plot::gl::glClientWaitSync) g_fence_wait_saved = nullptr; + +plot::gl::GLenum count_fence_wait(plot::gl::GLsync sync, plot::gl::GLbitfield flags, + plot::gl::GLuint64 timeout) { + ++g_fence_wait_calls; + return g_fence_wait_saved(sync, flags, timeout); +} + +class CountFenceWaits { + public: + CountFenceWaits() noexcept : saved_(plot::gl::glClientWaitSync) { + g_fence_wait_saved = saved_; + plot::gl::glClientWaitSync = &count_fence_wait; + } + ~CountFenceWaits() { plot::gl::glClientWaitSync = saved_; } + CountFenceWaits(const CountFenceWaits&) = delete; + CountFenceWaits& operator=(const CountFenceWaits&) = delete; + + /// Calls made since the counter was installed. + u64 calls() const noexcept { return g_fence_wait_calls; } + + private: + decltype(plot::gl::glClientWaitSync) saved_; +}; + } // namespace PLOT_TEST(renders_visible_trace_across_viewport) { @@ -714,3 +748,168 @@ PLOT_TEST(refused_fence_drops_the_mapped_ring) { } ctx.destroy(); } + +/// The ring must absorb a frame that issues more uploads than it starts with +/// slots. The shape it replaced -- three slots and a fence per upload -- wrapped +/// within such a frame: from the fourth upload on, each one waited on a fence +/// recorded a few draws *earlier in the same frame*, i.e. an effective per-frame +/// GPU sync. That is free when the driver retires draws promptly (the GLFW host, +/// 1.07 ms a frame) and ~4 ms of the Qt widget's 7.11 ms under llvmpipe, which is +/// exactly the difference `PLOT_FORCE_GL33_UPLOAD=1` shows -- and it is a cliff +/// that grows with the trace and cursor counts this library exists for. +/// +/// The ring now grows to cover the frame's uploads, records one fence after the +/// frame's last draw, and waits once, at the start of the next frame, so a frame +/// never synchronises with work it issued itself. None of that is visible in the +/// pixels -- a syncing ring draws the same image -- so the test pins the three +/// properties directly: a frame writes distinct slots (the offsets it hands the +/// vertex array), the ring grows past its floor to cover them, and a frame +/// performs at most one wait. +PLOT_TEST(ring_grows_to_cover_a_frame_and_waits_once_at_its_start) { + plottest::HeadlessContext ctx(kW, kH); + plot::Status st; + PLOT_CHECK(ctx.create(&st)); + PLOT_CHECK(plot::gl::load(nullptr)); + + plot::render::detail::InstanceStore store; + PLOT_CHECK(store.create(plot::render::UploadMode::PersistentMapped, &st)); + if (!store.persistent()) { + ctx.destroy(); // no persistent-mapping path on this driver: nothing to pin + return; + } + + constexpr plot::u32 kUploadsPerFrame = 6; // more than the ring's three-slot floor + plot::f32 data[4 * kUploadsPerFrame]; + for (plot::u32 i = 0; i < 4 * kUploadsPerFrame; ++i) data[i] = static_cast(i); + + auto counter = [](const std::atomic& c) { + return c.load(std::memory_order_relaxed); + }; + plot::render::detail::ring_uploads().store(0, std::memory_order_relaxed); + plot::render::detail::ring_waits().store(0, std::memory_order_relaxed); + plot::render::detail::ring_slots().store(0, std::memory_order_relaxed); + + // The first frame has no fence outstanding, so it must not wait at all -- and it + // must have grown the ring past its floor, because six uploads do not fit in + // three slots. + PLOT_CHECK(store.begin_frame()); + for (plot::u32 i = 0; i < kUploadsPerFrame; ++i) { + store.upload(data, kUploadsPerFrame, sizeof(plot::render::LineInstance)); + } + PLOT_CHECK(store.mark_drawn()); + PLOT_CHECK(counter(plot::render::detail::ring_uploads()) == kUploadsPerFrame); + PLOT_CHECK(counter(plot::render::detail::ring_waits()) == 0); + PLOT_CHECK(counter(plot::render::detail::ring_slots()) > store.kInitialSlots); + PLOT_CHECK(counter(plot::render::detail::ring_slots()) >= kUploadsPerFrame); + + // The second frame, measured upload by upload. The ring covers it now, so the + // slot each upload lands in -- the offset the vertex array re-points at -- is + // readable, and the invariant is the offsets being distinct: a slot written + // twice in a frame would be one the GPU could still be reading the first copy + // from, because the only fence is the one at the end of the frame. + PLOT_CHECK(store.begin_frame()); + plot::usize offsets[kUploadsPerFrame] = {}; + for (plot::u32 i = 0; i < kUploadsPerFrame; ++i) { + store.upload(data, kUploadsPerFrame, sizeof(plot::render::LineInstance)); + offsets[i] = store.slot_offset(); + } + PLOT_CHECK(store.mark_drawn()); + for (plot::u32 i = 0; i < kUploadsPerFrame; ++i) { + for (plot::u32 j = i + 1u; j < kUploadsPerFrame; ++j) { + PLOT_CHECK(offsets[i] != offsets[j]); // no slot reused within the frame + } + } + // ... and this frame waited exactly once: on the fence the first one recorded, + // never on its own work. + PLOT_CHECK(counter(plot::render::detail::ring_uploads()) == 2u * kUploadsPerFrame); + PLOT_CHECK(counter(plot::render::detail::ring_waits()) == 1); + + // A third frame the same, so the wait is per frame and not per upload: six + // uploads a frame would be five waits a frame under the shape this replaced. + PLOT_CHECK(store.begin_frame()); + for (plot::u32 i = 0; i < kUploadsPerFrame; ++i) { + store.upload(data, kUploadsPerFrame, sizeof(plot::render::LineInstance)); + } + PLOT_CHECK(store.mark_drawn()); + PLOT_CHECK(counter(plot::render::detail::ring_waits()) == 2); + ctx.destroy(); +} + +/// The same property at the level the demos run at. A frame of this plot stages +/// what the demos stage -- two grid colour groups, the axis lines, tick labels and +/// titles, the trace, two cursors and the measurement band -- nine-ish uploads, or +/// three times the slots the ring used to have, so the old ring wrapped within the +/// frame several times over. The frame must absorb them with at most one wait per +/// store (two stores), and the upload counter must agree with the draw calls the +/// frame reports, so it is counting this frame's staging and nothing else. +/// +/// The wait is checked twice over: once from the store's own counter, and once at +/// `glClientWaitSync`, the call every wait is made through. A nine-upload frame +/// under the shape this replaced made six mid-frame wait sites, each costing the +/// non-blocking probe plus the bounded wait behind it. +PLOT_TEST(a_dense_frame_uploads_more_than_the_ring_floor_without_waiting) { + 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)); + if (!r.using_persistent_upload()) { + ctx.destroy(); // no persistent-mapping path on this driver: nothing to pin + return; + } + + plot::data::ArraySeries s(sine_samples(), 0.0, 1.0); + plot::Plot p; + p.set_viewport({0, 0, static_cast(kW), static_cast(kH)}); + const plot::u32 ax = p.add_x_axis({"t", s.x_bounds(), true, plot::AxisPlace::Bottom, false}); + const plot::u32 ay = p.add_y_axis({"v", {-1.0, 1.0}, true, plot::AxisPlace::Left, false}); + const plot::u32 tr = p.add_trace({&s, plot::Color::hex(0x40C0FF), 1.5f, ax, ay}); + p.set_x_range(ax, {0.0, 640.0}); + const plot::u32 ca = p.add_cursor(100.0, plot::Color::hex(0x00FF00)); + const plot::u32 cb = p.add_cursor(300.0, plot::Color::hex(0xFF00FF)); + PLOT_CHECK(p.add_measure(ca, cb, tr) == 0); + + auto counter = [](const std::atomic& c) { + return c.load(std::memory_order_relaxed); + }; + plot::render::detail::ring_uploads().store(0, std::memory_order_relaxed); + plot::render::detail::ring_waits().store(0, std::memory_order_relaxed); + plot::render::detail::ring_slots().store(0, std::memory_order_relaxed); + CountFenceWaits driver_waits; // installed after load(), restored with the test + + for (plot::u32 index = 0; index < 3u; ++index) { + const plot::u64 uploads_before = counter(plot::render::detail::ring_uploads()); + const plot::u64 waits_before = counter(plot::render::detail::ring_waits()); + const plot::u64 calls_before = driver_waits.calls(); + + 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::u64 uploads = counter(plot::render::detail::ring_uploads()) - uploads_before; + const plot::u64 waits = counter(plot::render::detail::ring_waits()) - waits_before; + // Every upload lands in a draw this frame reported, so the counter describes + // this frame's staging and no other store's. + PLOT_CHECK(uploads == r.stats().draw_calls); + // The scenario: more uploads than the ring's floor of slots. + PLOT_CHECK(uploads > plot::render::detail::InstanceStore::kInitialSlots); + // ... and not one of them waited: the only wait is the next frame's start, + // one per store. + PLOT_CHECK(waits <= 2); + if (index == 0) PLOT_CHECK(waits == 0); + // At the driver call too: one wait site per store, and a site costs at most + // two calls -- the non-blocking probe, then the bounded wait behind it. Six + // uploads past the old ring's three slots used to cost six sites on their own, + // inside the frame that made them. + const plot::u64 calls = driver_waits.calls() - calls_before; + PLOT_CHECK(calls <= 4u); + if (index == 0) PLOT_CHECK(calls == 0u); + } + // The ring really grew past its floor: what covers a nine-upload frame is not + // what three slots covered. + PLOT_CHECK(counter(plot::render::detail::ring_slots()) > + plot::render::detail::InstanceStore::kInitialSlots); + ctx.destroy(); +}