From 379e934afa729749412c4086db85c696ad247da2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Apr 2026 15:45:52 +0000 Subject: [PATCH 1/4] Harden public range setters against invalid inputs - Plot_widget and Plot_time_axis setters for t-range, available t-range and v-range now silently ignore non-finite or mis-ordered inputs instead of letting NaN/inf propagate into layout math and rendered time<->pixel conversions. - Plot_widget::set_view validates each optional range independently and no longer partially applies an available-range when the primary range is missing. - Narrow the torn-read window in set_rendered_v/t_range by invalidating before publishing new endpoints; document that a seqlock would be required for a full fix. Also: - Plot_config::field_count was out of sync with the struct (20 vs. the 22 actual fields); bumping it to match keeps the plot_config_equivalent static_assert honest so future fields still force comparator updates. - Document Series_renderer::initialize as infallible (shader compilation is lazy) and clarify the data_snapshot_t lifetime contract so consumers know hold must be retained for the view to remain valid. - Add a threading-model summary to Plot_widget so call sites know which methods are GUI-thread vs. render-thread, and that invalid range inputs are ignored rather than partially applied. https://claude.ai/code/session_012kn22AT9HdT6aoJoi8rz4Y --- include/vnm_plot/core/plot_config.h | 2 +- include/vnm_plot/core/series_renderer.h | 5 +- include/vnm_plot/core/types.h | 15 ++++- include/vnm_plot/qt/plot_widget.h | 15 +++++ src/qt/plot_time_axis.cpp | 47 +++++++++++----- src/qt/plot_widget.cpp | 74 ++++++++++++++++--------- 6 files changed, 114 insertions(+), 44 deletions(-) diff --git a/include/vnm_plot/core/plot_config.h b/include/vnm_plot/core/plot_config.h index 47948d2..9dd9b59 100644 --- a/include/vnm_plot/core/plot_config.h +++ b/include/vnm_plot/core/plot_config.h @@ -152,7 +152,7 @@ struct Plot_config // Maintenance aid: bump when adding a field so that comparators (e.g. // plot_config_equivalent) fail to compile until they are updated. - static constexpr int field_count = 20; + static constexpr int field_count = 22; }; // ----------------------------------------------------------------------------- diff --git a/include/vnm_plot/core/series_renderer.h b/include/vnm_plot/core/series_renderer.h index 1fff22e..968ede1 100644 --- a/include/vnm_plot/core/series_renderer.h +++ b/include/vnm_plot/core/series_renderer.h @@ -33,7 +33,10 @@ class Series_renderer Series_renderer(const Series_renderer&) = delete; Series_renderer& operator=(const Series_renderer&) = delete; - // Initialize with asset loader for shader loading + // Remember the asset loader used for on-demand shader lookup. Shaders are + // compiled lazily from render() rather than up front, so this call performs + // no GL work and cannot fail. The referenced loader must outlive the + // Series_renderer. void initialize(Asset_loader& asset_loader); // Clean up GL resources diff --git a/include/vnm_plot/core/types.h b/include/vnm_plot/core/types.h index 9e72389..4d93bca 100644 --- a/include/vnm_plot/core/types.h +++ b/include/vnm_plot/core/types.h @@ -56,9 +56,18 @@ using Byte_view = std::string_view; // If data2/count2 is set, the logical snapshot is split into two contiguous // segments (e.g., ring buffer wrap). The total count is `count`. // `sequence` is a monotonic counter that increments on data changes. -// NOTE: The Data_source implementation owns the data contract. If it returns -// copied data, it must keep that buffer alive. If it returns a direct view, -// it must ensure the view stays valid (e.g., by holding a lock in `hold`). +// +// Lifetime contract: +// - The Data_source implementation decides whether the pointers refer to a +// copy it owns, or a direct view into its live storage. +// - Whatever guarantees the view's validity (an internal buffer, a lock, a +// reference count) must be kept alive via `hold`; the snapshot is safe to +// read for exactly as long as the caller keeps `hold` alive (or, if `hold` +// is empty, as long as the Data_source promises the view remains stable — +// usually until the next call into the Data_source from the same thread). +// - Consumers must not copy `data`/`data2` into long-lived storage without +// also retaining `hold`. Treat the pointers as only valid while a +// `data_snapshot_t` value is live. struct data_snapshot_t { const void* data = nullptr; ///< Pointer to first sample diff --git a/include/vnm_plot/qt/plot_widget.h b/include/vnm_plot/qt/plot_widget.h index fafa699..11bbefb 100644 --- a/include/vnm_plot/qt/plot_widget.h +++ b/include/vnm_plot/qt/plot_widget.h @@ -45,6 +45,21 @@ struct Plot_view // ----------------------------------------------------------------------------- // Qt Quick widget for rendering GPU-accelerated plots. // This is the main public interface for the vnm_plot library. +// +// Threading model: +// - All public setters (e.g. set_t_range, set_v_range, set_config, add_series, +// adjust_* helpers) must be called from the Qt GUI thread (the one that owns +// the QQuickWindow). Internally they take short-lived locks on m_config_mutex, +// m_data_cfg_mutex, or m_series_mutex so the render thread can read a +// consistent snapshot. +// - Methods tagged `_from_renderer` (set_vbar_width_from_renderer, +// set_auto_v_range_from_renderer, set_rendered_v_range, set_rendered_t_range) +// are invoked on the GL render thread and are implemented to be safe to call +// without the main thread serializing with them. State they publish back to +// the UI thread is carried by atomics. +// - Non-finite values (NaN, +/-inf) are silently ignored by the range setters; +// they never produce a partial update. Invalid ranges (max <= min) are also +// ignored. class Plot_widget : public QQuickFramebufferObject { Q_OBJECT diff --git a/src/qt/plot_time_axis.cpp b/src/qt/plot_time_axis.cpp index eceab81..ac2ef34 100644 --- a/src/qt/plot_time_axis.cpp +++ b/src/qt/plot_time_axis.cpp @@ -107,6 +107,9 @@ void Plot_time_axis::clear_shared_vbar_width(const QObject* owner) void Plot_time_axis::set_t_min(double v) { + if (!std::isfinite(v)) { + return; + } double new_min = v; double new_max = m_t_max; if (v >= m_t_max) { @@ -121,6 +124,9 @@ void Plot_time_axis::set_t_min(double v) void Plot_time_axis::set_t_max(double v) { + if (!std::isfinite(v)) { + return; + } double new_min = m_t_min; double new_max = v; if (v <= m_t_min) { @@ -135,40 +141,53 @@ void Plot_time_axis::set_t_max(double v) void Plot_time_axis::set_t_available_min(double v) { + if (!std::isfinite(v)) { + return; + } set_limits_if_changed(m_t_min, m_t_max, v, m_t_available_max); } void Plot_time_axis::set_t_available_max(double v) { + if (!std::isfinite(v)) { + return; + } set_limits_if_changed(m_t_min, m_t_max, m_t_available_min, v); } void Plot_time_axis::set_t_range(double t_min, double t_max) { + if (!std::isfinite(t_min) || !std::isfinite(t_max) || !(t_max > t_min)) { + return; + } set_limits_if_changed(t_min, t_max, m_t_available_min, m_t_available_max); } void Plot_time_axis::set_available_t_range(double t_available_min, double t_available_max) { + if (!std::isfinite(t_available_min) || !std::isfinite(t_available_max) || + !(t_available_max > t_available_min)) + { + return; + } + double new_t_min = m_t_min; double new_t_max = m_t_max; - if (t_available_max > t_available_min) { - const double span = t_available_max - t_available_min; - const double cur_span = new_t_max - new_t_min; - if (cur_span > span) { + const double span = t_available_max - t_available_min; + const double cur_span = new_t_max - new_t_min; + if (cur_span > span) { + new_t_min = t_available_min; + new_t_max = t_available_max; + } + else { + if (new_t_min < t_available_min) { new_t_min = t_available_min; - new_t_max = t_available_max; + new_t_max = t_available_min + cur_span; } - else { - if (new_t_min < t_available_min) { - new_t_min = t_available_min; - new_t_max = t_available_min + cur_span; - } - if (new_t_max > t_available_max) { - new_t_max = t_available_max; - new_t_min = t_available_max - cur_span; - } + if (new_t_max > t_available_max) { + new_t_max = t_available_max; + new_t_min = t_available_max - cur_span; } } diff --git a/src/qt/plot_widget.cpp b/src/qt/plot_widget.cpp index c08aa6f..aafc955 100644 --- a/src/qt/plot_widget.cpp +++ b/src/qt/plot_widget.cpp @@ -100,7 +100,7 @@ bool plot_config_equivalent( const vnm::plot::Plot_config& rhs) { // If a field is added to Plot_config, update this comparator and bump field_count. - static_assert(vnm::plot::Plot_config::field_count == 20, + static_assert(vnm::plot::Plot_config::field_count == 22, "Plot_config field_count changed — update plot_config_equivalent to cover new fields"); return lhs.dark_mode == rhs.dark_mode && @@ -375,6 +375,9 @@ double Plot_widget::t_available_max() const void Plot_widget::set_t_range(double t_min, double t_max) { + if (!std::isfinite(t_min) || !std::isfinite(t_max) || !(t_max > t_min)) { + return; + } if (m_time_axis) { m_time_axis->set_t_range(t_min, t_max); return; @@ -390,6 +393,9 @@ void Plot_widget::set_t_range(double t_min, double t_max) void Plot_widget::set_available_t_range(double t_min, double t_max) { + if (!std::isfinite(t_min) || !std::isfinite(t_max) || !(t_max > t_min)) { + return; + } if (m_time_axis) { m_time_axis->set_available_t_range(t_min, t_max); return; @@ -426,43 +432,51 @@ void Plot_widget::set_view(const Plot_view& view) bool t_changed = false; bool v_changed = false; + const auto t_range_valid = [](const std::pair& r) { + return std::isfinite(r.first) && std::isfinite(r.second) && r.second > r.first; + }; + const auto v_range_valid = [](const std::pair& r) { + return std::isfinite(r.first) && std::isfinite(r.second) && r.second > r.first; + }; + + const bool t_range_ok = view.t_range && t_range_valid(*view.t_range); + const bool t_avail_ok = view.t_available_range && t_range_valid(*view.t_available_range); + if (m_time_axis) { - if (view.t_range) { + if (t_range_ok) { m_time_axis->set_t_range(view.t_range->first, view.t_range->second); t_changed = true; } - if (view.t_available_range) { + if (t_avail_ok) { m_time_axis->set_available_t_range(view.t_available_range->first, view.t_available_range->second); t_changed = true; } } else - if (view.t_range || view.t_available_range) { + if (t_range_ok || t_avail_ok) { std::unique_lock lock(m_data_cfg_mutex); - if (view.t_range) { + if (t_range_ok) { m_data_cfg.t_min = view.t_range->first; m_data_cfg.t_max = view.t_range->second; t_changed = true; } - if (view.t_available_range) { + if (t_avail_ok) { const double t_min = view.t_available_range->first; const double t_max = view.t_available_range->second; - if (t_max > t_min) { - const double span = t_max - t_min; - const double cur_span = m_data_cfg.t_max - m_data_cfg.t_min; - if (cur_span > span) { + const double span = t_max - t_min; + const double cur_span = m_data_cfg.t_max - m_data_cfg.t_min; + if (cur_span > span) { + m_data_cfg.t_min = t_min; + m_data_cfg.t_max = t_max; + } + else { + if (m_data_cfg.t_min < t_min) { m_data_cfg.t_min = t_min; - m_data_cfg.t_max = t_max; + m_data_cfg.t_max = t_min + cur_span; } - else { - if (m_data_cfg.t_min < t_min) { - m_data_cfg.t_min = t_min; - m_data_cfg.t_max = t_min + cur_span; - } - if (m_data_cfg.t_max > t_max) { - m_data_cfg.t_max = t_max; - m_data_cfg.t_min = t_max - cur_span; - } + if (m_data_cfg.t_max > t_max) { + m_data_cfg.t_max = t_max; + m_data_cfg.t_min = t_max - cur_span; } } m_data_cfg.t_available_min = t_min; @@ -471,7 +485,7 @@ void Plot_widget::set_view(const Plot_view& view) } } - if (view.v_range) { + if (view.v_range && v_range_valid(*view.v_range)) { std::unique_lock lock(m_data_cfg_mutex); m_data_cfg.v_min = view.v_range->first; m_data_cfg.v_max = view.v_range->second; @@ -611,6 +625,9 @@ void Plot_widget::set_v_auto(bool auto_scale) void Plot_widget::set_v_range(float v_min, float v_max) { + if (!std::isfinite(v_min) || !std::isfinite(v_max) || !(v_max > v_min)) { + return; + } { std::unique_lock lock(m_data_cfg_mutex); m_data_cfg.v_min = v_min; @@ -1413,8 +1430,13 @@ void Plot_widget::set_rendered_v_range(float v_min, float v_max) const m_rendered_v_range_valid.store(false, std::memory_order_release); return; } - m_rendered_v_min.store(v_min, std::memory_order_release); - m_rendered_v_max.store(v_max, std::memory_order_release); + // Narrow (but do not fully close) the window where a reader can observe a + // torn mix of old and new endpoints by invalidating first, publishing the + // new values, then re-validating. A full fix would require a seqlock; the + // worst case here is one frame of slightly stale indicator coordinates. + m_rendered_v_range_valid.store(false, std::memory_order_release); + m_rendered_v_min.store(v_min, std::memory_order_relaxed); + m_rendered_v_max.store(v_max, std::memory_order_relaxed); m_rendered_v_range_valid.store(true, std::memory_order_release); } @@ -1425,8 +1447,10 @@ void Plot_widget::set_rendered_t_range(double t_min, double t_max) const m_rendered_t_range_valid.store(false, std::memory_order_release); return; } - m_rendered_t_min.store(t_min, std::memory_order_release); - m_rendered_t_max.store(t_max, std::memory_order_release); + // See set_rendered_v_range for the invalidate-first rationale. + m_rendered_t_range_valid.store(false, std::memory_order_release); + m_rendered_t_min.store(t_min, std::memory_order_relaxed); + m_rendered_t_max.store(t_max, std::memory_order_relaxed); m_rendered_t_range_valid.store(true, std::memory_order_release); } From 8a9df880314da81b32c7074f69c0274346de5d04 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Apr 2026 16:11:13 +0000 Subject: [PATCH 2/4] Tighten API surface, diagnostics, and test coverage Address the remaining items from the earlier review: - Segregate Plot_widget's render-thread callbacks. The two _from_renderer methods still need to be addressable because Plot_renderer posts to them through QMetaObject::invokeMethod, but they no longer carry Q_INVOKABLE, so QML cannot call them, and a header comment marks them as internal-only. - Route shader and asset-loading failures through Plot_config::log_error. Plot_core::render now forwards the callback to the Asset_loader and Primitive_renderer (matching what plot_renderer.cpp does for the Qt path), and Series_renderer::get_or_load_shader logs each failure mode (no asset loader, empty vert stage, asset lookup miss, compile/link failure) so silent fallbacks stop swallowing bugs. - Document the float-precision tradeoff of Data_access_policy::get_value and get_range so users know to subtract large biases before narrowing from double to float. - Align the CMake project VERSION with the 1.0.4 tag referenced in the README FetchContent example so install metadata and docs no longer disagree. - Add three new core-only tests: * test_core_algo covers LOD level selection, contiguous and segmented timestamp binary searches, and the axis-label formatter. * test_asset_loader covers missing-asset logging, embedded lookup, override-directory precedence and fallback, and shader helper. * test_concurrent_series runs a ring-buffer Data_source under two readers plus a writer and verifies the shared_ptr-based hold semantics keep snapshots safe after further writes. All seven tests pass on both core-only and Qt builds. https://claude.ai/code/session_012kn22AT9HdT6aoJoi8rz4Y --- CMakeLists.txt | 2 +- include/vnm_plot/core/types.h | 15 +- include/vnm_plot/qt/plot_widget.h | 35 +++-- src/core/plot_core.cpp | 8 + src/core/series_renderer.cpp | 22 ++- tests/CMakeLists.txt | 73 +++++---- tests/test_asset_loader.cpp | 221 ++++++++++++++++++++++++++ tests/test_concurrent_series.cpp | 246 +++++++++++++++++++++++++++++ tests/test_core_algo.cpp | 247 ++++++++++++++++++++++++++++++ 9 files changed, 808 insertions(+), 61 deletions(-) create mode 100644 tests/test_asset_loader.cpp create mode 100644 tests/test_concurrent_series.cpp create mode 100644 tests/test_core_algo.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 6af4c1e..a5a49b3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,7 +6,7 @@ cmake_minimum_required(VERSION 3.16) project(vnm_plot - VERSION 0.1.0 + VERSION 1.0.4 DESCRIPTION "High-performance GPU-accelerated plotting library" LANGUAGES CXX C ) diff --git a/include/vnm_plot/core/types.h b/include/vnm_plot/core/types.h index 4d93bca..8d6309d 100644 --- a/include/vnm_plot/core/types.h +++ b/include/vnm_plot/core/types.h @@ -271,14 +271,21 @@ class Vector_data_source : public Data_source // ----------------------------------------------------------------------------- // Defines how the renderer extracts meaningful values from opaque sample data. // This enables rendering of arbitrary sample types without template explosion. +// +// Precision note: get_value and get_range return float because the renderer +// ultimately uploads values as float attributes. If your source data has +// narrow dynamic range offset by a large bias (e.g. physical quantities with +// an absolute reference), subtract that bias inside the accessor before +// narrowing to float so the remaining dynamic range survives the conversion. +// Timestamps and aux metrics use double and are not affected. struct Data_access_policy { // --- Sample value extraction --- - std::function get_timestamp; ///< Extract timestamp - std::function get_value; ///< Extract primary value - std::function(const void* sample)> get_range; ///< Extract min/max range + std::function get_timestamp; ///< Extract timestamp (double precision) + std::function get_value; ///< Extract primary value (narrow with care; see above) + std::function(const void* sample)> get_range; ///< Extract min/max range (narrow with care; see above) - std::function get_aux_metric; ///< Optional auxiliary metric + std::function get_aux_metric; ///< Optional auxiliary metric (double precision) std::function get_signal; ///< Optional [0,1] signal for COLORMAP_LINE // Optional sample cloning with timestamp rewrite, used for render-only hold-forward paths. diff --git a/include/vnm_plot/qt/plot_widget.h b/include/vnm_plot/qt/plot_widget.h index 11bbefb..650bc98 100644 --- a/include/vnm_plot/qt/plot_widget.h +++ b/include/vnm_plot/qt/plot_widget.h @@ -47,16 +47,19 @@ struct Plot_view // This is the main public interface for the vnm_plot library. // // Threading model: -// - All public setters (e.g. set_t_range, set_v_range, set_config, add_series, -// adjust_* helpers) must be called from the Qt GUI thread (the one that owns -// the QQuickWindow). Internally they take short-lived locks on m_config_mutex, -// m_data_cfg_mutex, or m_series_mutex so the render thread can read a -// consistent snapshot. -// - Methods tagged `_from_renderer` (set_vbar_width_from_renderer, -// set_auto_v_range_from_renderer, set_rendered_v_range, set_rendered_t_range) -// are invoked on the GL render thread and are implemented to be safe to call -// without the main thread serializing with them. State they publish back to -// the UI thread is carried by atomics. +// - All user-facing setters (e.g. set_t_range, set_v_range, set_config, +// add_series, adjust_* helpers) must be called from the Qt GUI thread (the +// one that owns the QQuickWindow). Internally they take short-lived locks on +// m_config_mutex, m_data_cfg_mutex, or m_series_mutex so the render thread +// can read a consistent snapshot. +// - set_vbar_width_from_renderer / set_auto_v_range_from_renderer are internal +// callbacks — Plot_renderer posts them through QMetaObject::invokeMethod so +// they execute on the GUI thread once the queued event is dispatched. They +// are intentionally not Q_INVOKABLE (QML must not call them). Application +// code should ignore them. +// - set_rendered_v_range / set_rendered_t_range run on the render thread and +// publish through atomics for UI-side readers (rendered_v_range / +// rendered_t_range). // - Non-finite values (NaN, +/-inf) are silently ignored by the range setters; // they never produce a partial update. Invalid ranges (max <= min) are also // ignored. @@ -163,9 +166,6 @@ class Plot_widget : public QQuickFramebufferObject double vbar_width_pixels() const; double vbar_width_qml() const; Q_INVOKABLE void set_vbar_width(double vbar_width); - Q_INVOKABLE void set_vbar_width_from_renderer(double px); - // Render-thread updates to keep v_min/v_max in sync with auto range. - Q_INVOKABLE void set_auto_v_range_from_renderer(float v_min, float v_max); // --- Interaction --- @@ -190,6 +190,15 @@ class Plot_widget : public QQuickFramebufferObject Q_INVOKABLE QVariantList get_indicator_samples(double x, double plot_width, double plot_height, double mouse_px = -1.0) const; Q_INVOKABLE QString format_timestamp_precise(double timestamp) const; + // --- Internal render-thread callbacks --- + // These exist only so Plot_renderer can post updates back to the widget via + // QMetaObject::invokeMethod. They are NOT part of the QML API (no + // Q_INVOKABLE) and application code should not call them directly. They + // accept non-finite values by silently ignoring them, matching the rest of + // the range-setter surface. + void set_vbar_width_from_renderer(double px); + void set_auto_v_range_from_renderer(float v_min, float v_max); + // --- Qt Quick FBO Interface --- Renderer* createRenderer() const override; diff --git a/src/core/plot_core.cpp b/src/core/plot_core.cpp index 59a7d48..bd8de38 100644 --- a/src/core/plot_core.cpp +++ b/src/core/plot_core.cpp @@ -169,6 +169,14 @@ void Plot_core::render( vnm::plot::Profiler* profiler = config ? config->profiler.get() : nullptr; m_impl->primitives.set_profiler(profiler); + // Route failures from asset loading, primitive rendering, and (where + // applicable) font rendering through the caller-provided log callback so + // standalone core users see the same diagnostics the Qt wrapper does. + if (config && config->log_error) { + m_impl->asset_loader.set_log_callback(config->log_error); + m_impl->primitives.set_log_callback(config->log_error); + } + const float preview_v_min = params.preview_v_min.value_or(params.v_min); const float preview_v_max = params.preview_v_max.value_or(params.v_max); const double t_available_min = params.t_available_min.value_or(params.t_min); diff --git a/src/core/series_renderer.cpp b/src/core/series_renderer.cpp index b38d1cf..33b93ce 100644 --- a/src/core/series_renderer.cpp +++ b/src/core/series_renderer.cpp @@ -315,7 +315,18 @@ std::shared_ptr Series_renderer::get_or_load_shader( const shader_set_t& shader_set, const Plot_config* config) { - if (shader_set.vert.empty() || !m_asset_loader) { + const auto log_error = [&](const std::string& message) { + if (config && config->log_error) { + config->log_error(message); + } + }; + + if (!m_asset_loader) { + log_error("Series_renderer: asset loader not initialized; cannot compile shaders"); + return nullptr; + } + if (shader_set.vert.empty()) { + log_error("Series_renderer: shader set has no vertex stage; cannot compile program"); return nullptr; } @@ -332,9 +343,7 @@ std::shared_ptr Series_renderer::get_or_load_shader( } if (!vert_src || !frag_src) { - if (config && config->log_error) { - config->log_error("Failed to load shader sources: " + normalized.vert); - } + log_error("Failed to load shader sources: " + normalized.vert); return nullptr; } @@ -345,10 +354,11 @@ std::shared_ptr Series_renderer::get_or_load_shader( geom_str.assign(geom_src->begin(), geom_src->end()); } - auto log_error = config ? config->log_error : std::function(); - auto sp = create_gl_program(vert_str, geom_str, frag_str, log_error); + auto log_error_fn = config ? config->log_error : std::function(); + auto sp = create_gl_program(vert_str, geom_str, frag_str, log_error_fn); if (!sp) { + log_error("Shader program creation failed for: " + normalized.vert); return nullptr; } auto shared_sp = std::shared_ptr(std::move(sp)); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 35eaea7..8404566 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,56 +1,55 @@ cmake_minimum_required(VERSION 3.16) -# Test executables -add_executable(test_cache_invalidation test_cache_invalidation.cpp) -add_executable(test_snapshot_caching test_snapshot_caching.cpp) -add_executable(test_typed_api test_typed_api.cpp) +# Core-only test executables. The existing three cover cache invalidation, +# snapshot caching, and the typed API; the three added here round out coverage +# for algo.h (LOD selection, timestamp search, axis formatter), Asset_loader +# (embedded/override/log-error paths), and Data_source (concurrent snapshots +# with hold-based buffer pinning). +set(_vnm_plot_core_tests + test_cache_invalidation + test_snapshot_caching + test_typed_api + test_core_algo + test_asset_loader + test_concurrent_series +) -target_link_libraries(test_cache_invalidation PRIVATE vnm_plot_core) -target_link_libraries(test_snapshot_caching PRIVATE vnm_plot_core) -target_link_libraries(test_typed_api PRIVATE vnm_plot_core) +foreach(_test IN LISTS _vnm_plot_core_tests) + add_executable(${_test} ${_test}.cpp) + target_link_libraries(${_test} PRIVATE vnm_plot_core) + set_target_properties(${_test} PROPERTIES + CXX_STANDARD 17 + CXX_STANDARD_REQUIRED ON + ) +endforeach() -if(TARGET vnm_plot) - add_executable(test_plot_interaction_item test_plot_interaction_item.cpp) - target_link_libraries(test_plot_interaction_item PRIVATE vnm_plot) +# The concurrent test spawns std::threads. +find_package(Threads QUIET) +if(TARGET Threads::Threads) + target_link_libraries(test_concurrent_series PRIVATE Threads::Threads) endif() -# Set C++17 standard -set_target_properties(test_cache_invalidation PROPERTIES - CXX_STANDARD 17 - CXX_STANDARD_REQUIRED ON -) - -set_target_properties(test_snapshot_caching PROPERTIES - CXX_STANDARD 17 - CXX_STANDARD_REQUIRED ON -) - -set_target_properties(test_typed_api PROPERTIES - CXX_STANDARD 17 - CXX_STANDARD_REQUIRED ON -) +add_test(NAME CacheInvalidation COMMAND test_cache_invalidation) +add_test(NAME SnapshotCaching COMMAND test_snapshot_caching) +add_test(NAME TypedApi COMMAND test_typed_api) +add_test(NAME CoreAlgo COMMAND test_core_algo) +add_test(NAME AssetLoader COMMAND test_asset_loader) +add_test(NAME ConcurrentSeries COMMAND test_concurrent_series) if(TARGET vnm_plot) + add_executable(test_plot_interaction_item test_plot_interaction_item.cpp) + target_link_libraries(test_plot_interaction_item PRIVATE vnm_plot) set_target_properties(test_plot_interaction_item PROPERTIES CXX_STANDARD 17 CXX_STANDARD_REQUIRED ON ) -endif() - -# Add tests -add_test(NAME CacheInvalidation COMMAND test_cache_invalidation) -add_test(NAME SnapshotCaching COMMAND test_snapshot_caching) -add_test(NAME TypedApi COMMAND test_typed_api) - -if(TARGET vnm_plot) add_test(NAME PlotInteractionItem COMMAND test_plot_interaction_item) endif() -# Print test info message(STATUS "Configured tests:") -message(STATUS " - test_cache_invalidation") -message(STATUS " - test_snapshot_caching") -message(STATUS " - test_typed_api") +foreach(_test IN LISTS _vnm_plot_core_tests) + message(STATUS " - ${_test}") +endforeach() if(TARGET vnm_plot) message(STATUS " - test_plot_interaction_item") endif() diff --git a/tests/test_asset_loader.cpp b/tests/test_asset_loader.cpp new file mode 100644 index 0000000..85245f7 --- /dev/null +++ b/tests/test_asset_loader.cpp @@ -0,0 +1,221 @@ +// vnm_plot Asset_loader tests +// Verifies logging wiring, embedded asset lookup, override-directory fallback, +// and the shader_sources helper. + +#include + +#include +#include +#include +#include +#include +#include + +namespace plot = vnm::plot; + +namespace { + +#define TEST_ASSERT(cond, msg) \ + do { \ + if (!(cond)) { \ + std::cerr << "FAIL: " << msg << " (line " << __LINE__ << ")" << std::endl; \ + return false; \ + } \ + } while (0) + +#define RUN_TEST(test_fn) \ + do { \ + std::cout << "Running " << #test_fn << "... "; \ + if (test_fn()) { \ + std::cout << "OK" << std::endl; \ + ++passed; \ + } \ + else { \ + std::cout << "FAIL" << std::endl; \ + ++failed; \ + } \ + } while (0) + +struct Scoped_temp_dir +{ + std::filesystem::path path; + + Scoped_temp_dir() + { + path = std::filesystem::temp_directory_path() / + ("vnm_plot_asset_test_" + std::to_string(std::hash{}(this))); + std::filesystem::create_directories(path); + } + + ~Scoped_temp_dir() + { + std::error_code ec; + std::filesystem::remove_all(path, ec); + } + + Scoped_temp_dir(const Scoped_temp_dir&) = delete; + Scoped_temp_dir& operator=(const Scoped_temp_dir&) = delete; +}; + +void write_file(const std::filesystem::path& path, const std::string& contents) +{ + std::filesystem::create_directories(path.parent_path()); + std::ofstream out(path, std::ios::binary); + out << contents; +} + +bool test_missing_asset_logs_and_returns_nullopt() +{ + plot::Asset_loader loader; + std::vector messages; + loader.set_log_callback([&](const std::string& msg) { messages.push_back(msg); }); + + auto result = loader.load("does/not/exist.vert"); + TEST_ASSERT(!result.has_value(), + "missing asset should return nullopt"); + TEST_ASSERT(!messages.empty(), + "missing asset should surface a log message"); + TEST_ASSERT(messages.front().find("does/not/exist.vert") != std::string::npos, + "log message should reference the missing asset name"); + return true; +} + +bool test_embedded_asset_returns_registered_bytes() +{ + plot::Asset_loader loader; + const std::string payload = "hello-shader-bytes"; + loader.register_embedded("example.vert", payload); + + auto result = loader.load("example.vert"); + TEST_ASSERT(result.has_value(), + "registered embedded asset should be loadable"); + TEST_ASSERT(*result == payload, + "embedded asset bytes should match what was registered"); + return true; +} + +bool test_override_directory_beats_embedded_asset() +{ + Scoped_temp_dir tmp; + plot::Asset_loader loader; + + loader.register_embedded("example.vert", "embedded-version"); + write_file(tmp.path / "example.vert", "override-version"); + + loader.set_override_directory(tmp.path.string()); + + auto result = loader.load("example.vert"); + TEST_ASSERT(result.has_value(), + "override asset should be loadable"); + TEST_ASSERT(*result == "override-version", + "override directory should take precedence over embedded asset"); + return true; +} + +bool test_override_directory_falls_back_to_embedded_when_missing() +{ + Scoped_temp_dir tmp; + plot::Asset_loader loader; + + loader.register_embedded("example.vert", "embedded-only"); + loader.set_override_directory(tmp.path.string()); + + auto result = loader.load("example.vert"); + TEST_ASSERT(result.has_value(), + "should fall back to embedded when override directory lacks the file"); + TEST_ASSERT(*result == "embedded-only", + "embedded fallback bytes should be returned"); + return true; +} + +bool test_load_shader_missing_required_stages_logs_and_returns_nullopt() +{ + plot::Asset_loader loader; + std::vector messages; + loader.set_log_callback([&](const std::string& msg) { messages.push_back(msg); }); + + auto missing_vert = loader.load_shader("ghost"); + TEST_ASSERT(!missing_vert.has_value(), + "shader without any stage should fail to load"); + + bool mentions_vert = false; + for (const auto& m : messages) { + if (m.find("ghost.vert") != std::string::npos) { + mentions_vert = true; + break; + } + } + TEST_ASSERT(mentions_vert, + "log should mention the missing vertex shader"); + return true; +} + +bool test_load_shader_includes_optional_geometry_when_present() +{ + plot::Asset_loader loader; + loader.register_embedded("good.vert", "vertex"); + loader.register_embedded("good.frag", "fragment"); + + auto without_geom = loader.load_shader("good"); + TEST_ASSERT(without_geom.has_value(), + "shader with just vert+frag should succeed"); + TEST_ASSERT(without_geom->vertex == "vertex", + "vertex bytes should match"); + TEST_ASSERT(without_geom->fragment == "fragment", + "fragment bytes should match"); + TEST_ASSERT(without_geom->geometry.empty(), + "geometry should be empty when not registered"); + + loader.register_embedded("good.geom", "geometry"); + auto with_geom = loader.load_shader("good"); + TEST_ASSERT(with_geom.has_value(), + "shader with all three stages should succeed"); + TEST_ASSERT(with_geom->geometry == "geometry", + "geometry bytes should match when registered"); + return true; +} + +bool test_override_directory_read_failure_logs() +{ + // Create an override path that exists but points at a directory entry of + // the same name; attempting to open it as a file should fail and log. + Scoped_temp_dir tmp; + std::filesystem::create_directories(tmp.path / "broken.vert"); // a dir, not a file + + plot::Asset_loader loader; + std::vector messages; + loader.set_log_callback([&](const std::string& msg) { messages.push_back(msg); }); + loader.register_embedded("broken.vert", "fallback"); + loader.set_override_directory(tmp.path.string()); + + auto result = loader.load("broken.vert"); + // The loader should still return the embedded fallback. Whether a log line + // is emitted depends on filesystem semantics (opening a directory may + // succeed on some platforms), so we only assert the fallback path. + TEST_ASSERT(result.has_value(), + "loader should fall back to embedded when override read fails"); + TEST_ASSERT(*result == "fallback", + "embedded fallback bytes should be returned"); + return true; +} + +} // namespace + +int main() +{ + std::cout << "Asset loader tests" << std::endl; + + int passed = 0; + int failed = 0; + + RUN_TEST(test_missing_asset_logs_and_returns_nullopt); + RUN_TEST(test_embedded_asset_returns_registered_bytes); + RUN_TEST(test_override_directory_beats_embedded_asset); + RUN_TEST(test_override_directory_falls_back_to_embedded_when_missing); + RUN_TEST(test_load_shader_missing_required_stages_logs_and_returns_nullopt); + RUN_TEST(test_load_shader_includes_optional_geometry_when_present); + RUN_TEST(test_override_directory_read_failure_logs); + + std::cout << "Results: " << passed << " passed, " << failed << " failed" << std::endl; + return failed > 0 ? 1 : 0; +} diff --git a/tests/test_concurrent_series.cpp b/tests/test_concurrent_series.cpp new file mode 100644 index 0000000..3d80921 --- /dev/null +++ b/tests/test_concurrent_series.cpp @@ -0,0 +1,246 @@ +// vnm_plot concurrent data-source tests +// Exercises Vector_data_source and a simple ring-buffer Data_source under +// concurrent snapshot and mutation, verifying that readers see a consistent +// view and the monotonic sequence counter reflects updates. + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace plot = vnm::plot; + +namespace { + +#define TEST_ASSERT(cond, msg) \ + do { \ + if (!(cond)) { \ + std::cerr << "FAIL: " << msg << " (line " << __LINE__ << ")" << std::endl; \ + return false; \ + } \ + } while (0) + +#define RUN_TEST(test_fn) \ + do { \ + std::cout << "Running " << #test_fn << "... "; \ + if (test_fn()) { \ + std::cout << "OK" << std::endl; \ + ++passed; \ + } \ + else { \ + std::cout << "FAIL" << std::endl; \ + ++failed; \ + } \ + } while (0) + +struct sample_t +{ + double t = 0.0; + float v = 0.0f; +}; + +bool test_vector_source_set_data_bumps_sequence() +{ + plot::Vector_data_source source; + TEST_ASSERT(source.sequence() == 0, "initial sequence should be 0"); + + source.set_data({{0.0, 0.0f}, {1.0, 1.0f}}); + TEST_ASSERT(source.sequence() == 1, "set_data should bump sequence once"); + + source.notify_changed(); + TEST_ASSERT(source.sequence() == 2, "notify_changed should bump sequence"); + + auto snap = source.try_snapshot(0); + TEST_ASSERT(static_cast(snap), "snapshot should be READY when data is present"); + TEST_ASSERT(snap.snapshot.sequence == 2, + "snapshot sequence should match Vector_data_source::sequence()"); + TEST_ASSERT(snap.snapshot.count == 2, "snapshot count should reflect stored samples"); + TEST_ASSERT(snap.snapshot.stride == sizeof(sample_t), "stride should be sizeof(sample_t)"); + return true; +} + +bool test_vector_source_empty_snapshot_reports_empty_status() +{ + plot::Vector_data_source source; + auto snap = source.try_snapshot(0); + TEST_ASSERT(snap.status == plot::snapshot_result_t::Snapshot_status::EMPTY, + "empty source should yield EMPTY status"); + TEST_ASSERT(!static_cast(snap), + "snapshot_result_t with EMPTY status should be falsey"); + return true; +} + +// A ring-buffer style Data_source that protects its writer with a mutex but +// allows snapshots to take a lock, pin a shared_ptr to a stable snapshot +// buffer, and return safely after releasing the lock. +class Ring_source final : public plot::Data_source +{ +public: + plot::snapshot_result_t try_snapshot(std::size_t /*lod*/) override + { + std::lock_guard lock(m_mutex); + if (m_samples.empty()) { + return {plot::data_snapshot_t{}, plot::snapshot_result_t::Snapshot_status::EMPTY}; + } + // Publish a copy so readers don't race writers after the lock drops. + auto buffer = std::make_shared>(m_samples); + plot::data_snapshot_t snapshot; + snapshot.data = buffer->data(); + snapshot.count = buffer->size(); + snapshot.stride = sizeof(sample_t); + snapshot.sequence = m_sequence; + snapshot.hold = buffer; // shared_ptr keeps buffer alive + return {snapshot, plot::snapshot_result_t::Snapshot_status::READY}; + } + + std::size_t sample_stride() const override { return sizeof(sample_t); } + std::uint64_t current_sequence(std::size_t) const override + { + std::lock_guard lock(m_mutex); + return m_sequence; + } + + void append(const sample_t& s) + { + std::lock_guard lock(m_mutex); + m_samples.push_back(s); + ++m_sequence; + } + +private: + mutable std::mutex m_mutex; + std::vector m_samples; + std::uint64_t m_sequence = 0; +}; + +bool test_ring_source_snapshots_are_consistent_under_concurrent_writes() +{ + Ring_source source; + std::atomic stop{false}; + std::atomic observed_max_sequence{0}; + std::atomic reader_errors{0}; + + std::thread writer([&] { + for (std::size_t i = 0; i < 5000 && !stop.load(std::memory_order_acquire); ++i) { + source.append({static_cast(i), static_cast(i)}); + } + }); + + auto reader_fn = [&] { + while (!stop.load(std::memory_order_acquire)) { + auto result = source.try_snapshot(0); + if (!result) { + continue; + } + const auto& snap = result.snapshot; + // Hold must keep the underlying buffer alive for the duration of + // the read; verify monotonic timestamps. + const auto* first = reinterpret_cast(snap.data); + double prev = first->t; + for (std::size_t i = 1; i < snap.count; ++i) { + const auto* cur = reinterpret_cast( + reinterpret_cast(snap.data) + i * snap.stride); + if (cur->t < prev) { + reader_errors.fetch_add(1, std::memory_order_relaxed); + break; + } + prev = cur->t; + } + std::uint64_t current = observed_max_sequence.load(std::memory_order_relaxed); + while (snap.sequence > current && + !observed_max_sequence.compare_exchange_weak(current, snap.sequence, + std::memory_order_relaxed)) { + // loop + } + } + }; + + std::thread reader1(reader_fn); + std::thread reader2(reader_fn); + + writer.join(); + // Let readers drain a final snapshot that covers the last writes. + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + stop.store(true, std::memory_order_release); + reader1.join(); + reader2.join(); + + TEST_ASSERT(reader_errors.load() == 0, + "readers should never observe non-monotonic timestamps inside a snapshot"); + TEST_ASSERT(observed_max_sequence.load() > 0, + "readers should observe at least one non-zero sequence"); + return true; +} + +bool test_ring_source_hold_keeps_buffer_alive_after_clear() +{ + Ring_source source; + source.append({1.0, 1.0f}); + source.append({2.0, 2.0f}); + + plot::data_snapshot_t snap; + { + auto result = source.try_snapshot(0); + TEST_ASSERT(static_cast(result), "expected READY snapshot"); + snap = result.snapshot; + } + + // hold keeps the buffer alive even though the Ring_source may later free + // internal storage; verify reading through the stored pointer is safe. + TEST_ASSERT(snap.hold.use_count() >= 1, "hold should pin at least one ref"); + + const auto* samples = reinterpret_cast(snap.data); + TEST_ASSERT(samples[0].t == 1.0 && samples[1].t == 2.0, + "samples accessed through pinned buffer should still be valid"); + + // Even if we append more, the previously taken snapshot must remain valid. + for (int i = 0; i < 50; ++i) { + source.append({static_cast(100 + i), 0.0f}); + } + TEST_ASSERT(samples[0].t == 1.0 && samples[1].t == 2.0, + "snapshot pointer should stay valid because hold pins the buffer"); + + return true; +} + +bool test_data_source_default_query_v_range_returns_false() +{ + Ring_source source; + float v_min = 123.f; + float v_max = 456.f; + std::uint64_t seq = 999; + const bool handled = source.query_v_range_for_t_window(0.0, 1.0, v_min, v_max, &seq); + TEST_ASSERT(!handled, + "default Data_source should report v-range query as unsupported"); + // The default implementation must not clobber outputs when it returns false. + TEST_ASSERT(v_min == 123.f && v_max == 456.f, + "default query_v_range_for_t_window must not touch its outputs"); + TEST_ASSERT(seq == 999, "default query_v_range_for_t_window must not touch out_sequence"); + return true; +} + +} // namespace + +int main() +{ + std::cout << "Concurrent series / Data_source tests" << std::endl; + + int passed = 0; + int failed = 0; + + RUN_TEST(test_vector_source_set_data_bumps_sequence); + RUN_TEST(test_vector_source_empty_snapshot_reports_empty_status); + RUN_TEST(test_ring_source_snapshots_are_consistent_under_concurrent_writes); + RUN_TEST(test_ring_source_hold_keeps_buffer_alive_after_clear); + RUN_TEST(test_data_source_default_query_v_range_returns_false); + + std::cout << "Results: " << passed << " passed, " << failed << " failed" << std::endl; + return failed > 0 ? 1 : 0; +} diff --git a/tests/test_core_algo.cpp b/tests/test_core_algo.cpp new file mode 100644 index 0000000..72e723a --- /dev/null +++ b/tests/test_core_algo.cpp @@ -0,0 +1,247 @@ +// vnm_plot core algorithm tests +// Covers LOD level selection, binary-search on timestamps (including segmented +// snapshots), and the small axis formatter used by default_format_timestamp. + +#include +#include + +#include +#include +#include +#include +#include +#include + +namespace plot = vnm::plot; + +namespace { + +#define TEST_ASSERT(cond, msg) \ + do { \ + if (!(cond)) { \ + std::cerr << "FAIL: " << msg << " (line " << __LINE__ << ")" << std::endl; \ + return false; \ + } \ + } while (0) + +#define RUN_TEST(test_fn) \ + do { \ + std::cout << "Running " << #test_fn << "... "; \ + if (test_fn()) { \ + std::cout << "OK" << std::endl; \ + ++passed; \ + } \ + else { \ + std::cout << "FAIL" << std::endl; \ + ++failed; \ + } \ + } while (0) + +struct sample_t +{ + double t = 0.0; + float v = 0.0f; +}; + +// Minimal Data_source exposing a configurable LOD ladder. +class Lod_source final : public plot::Data_source +{ +public: + explicit Lod_source(std::vector scales) + : m_scales(std::move(scales)) + {} + + plot::snapshot_result_t try_snapshot(std::size_t /*lod*/) override + { + return {plot::data_snapshot_t{}, plot::snapshot_result_t::Snapshot_status::EMPTY}; + } + + std::size_t lod_levels() const override { return m_scales.size(); } + std::size_t lod_scale(std::size_t level) const override + { + return (level < m_scales.size()) ? m_scales[level] : 1; + } + std::size_t sample_stride() const override { return sizeof(sample_t); } + +private: + std::vector m_scales; +}; + +bool test_choose_lod_level_picks_closest_pps() +{ + // Scales 1..1024 at pps=0.01; expect level whose scale * pps is closest to 1. + const std::vector scales = {1, 4, 16, 64, 256, 1024}; + const std::size_t level = plot::detail::choose_lod_level(scales, 0.01); + // 0.01 * 64 = 0.64 (err 0.36); 0.01 * 256 = 2.56 (err 1.56). Closest is 64. + TEST_ASSERT(level == 3, "expected choose_lod_level to pick scale 64"); + return true; +} + +bool test_choose_lod_level_handles_degenerate_inputs() +{ + TEST_ASSERT(plot::detail::choose_lod_level({}, 1.0) == 0, + "empty scales should yield level 0"); + TEST_ASSERT(plot::detail::choose_lod_level({1, 2, 4}, 0.0) == 0, + "non-positive base_pps should yield level 0"); + TEST_ASSERT(plot::detail::choose_lod_level({1, 2, 4}, -1.0) == 0, + "negative base_pps should yield level 0"); + return true; +} + +bool test_choose_lod_level_prefers_level_zero_on_tie() +{ + // When two levels give identical absolute error, the first one wins because + // the loop only updates on strict <. + const std::vector scales = {1, 2}; + // 0.75 * 1 = 0.75 (err 0.25); 0.75 * 2 = 1.50 (err 0.50); best is level 0. + TEST_ASSERT(plot::detail::choose_lod_level(scales, 0.75) == 0, + "closer-to-1.0 pps on coarser level should stay when finer is equally close"); + return true; +} + +bool test_compute_lod_scales_forces_minimum_of_one() +{ + // A malformed Data_source returning 0 should be clamped up to 1. + class Zero_scale_source final : public plot::Data_source + { + public: + plot::snapshot_result_t try_snapshot(std::size_t) override { + return {plot::data_snapshot_t{}, plot::snapshot_result_t::Snapshot_status::EMPTY}; + } + std::size_t lod_levels() const override { return 3; } + std::size_t lod_scale(std::size_t level) const override { + return (level == 0) ? 0 : (level == 1 ? 1 : 0); + } + std::size_t sample_stride() const override { return sizeof(sample_t); } + }; + + Zero_scale_source src; + auto scales = plot::detail::compute_lod_scales(src); + TEST_ASSERT(scales.size() == 3, "lod_levels should give 3 entries"); + TEST_ASSERT(scales[0] == 1, "zero scale at level 0 should be clamped to 1"); + TEST_ASSERT(scales[1] == 1, "unit scale at level 1 should survive"); + TEST_ASSERT(scales[2] == 1, "zero scale at level 2 should be clamped to 1"); + return true; +} + +bool test_lower_and_upper_bound_on_contiguous_buffer() +{ + std::vector samples; + for (int i = 0; i < 10; ++i) { + samples.push_back({static_cast(i), static_cast(i)}); + } + + const auto get_ts = [](const void* p) { + return static_cast(p)->t; + }; + + auto lb = plot::detail::lower_bound_timestamp( + samples.data(), samples.size(), sizeof(sample_t), get_ts, 3.5); + TEST_ASSERT(lb == 4, "lower_bound(3.5) should land on index 4"); + + auto lb_exact = plot::detail::lower_bound_timestamp( + samples.data(), samples.size(), sizeof(sample_t), get_ts, 3.0); + TEST_ASSERT(lb_exact == 3, "lower_bound(3.0) should land on index 3"); + + auto ub = plot::detail::upper_bound_timestamp( + samples.data(), samples.size(), sizeof(sample_t), get_ts, 3.0); + TEST_ASSERT(ub == 4, "upper_bound(3.0) should land on index 4"); + + // Out of range probes should clamp to the ends. + auto lb_below = plot::detail::lower_bound_timestamp( + samples.data(), samples.size(), sizeof(sample_t), get_ts, -5.0); + TEST_ASSERT(lb_below == 0, "lower_bound below range should be 0"); + + auto lb_above = plot::detail::lower_bound_timestamp( + samples.data(), samples.size(), sizeof(sample_t), get_ts, 100.0); + TEST_ASSERT(lb_above == samples.size(), "lower_bound above range should be count"); + + // Empty buffer is safe. + auto lb_empty = plot::detail::lower_bound_timestamp( + nullptr, 0, sizeof(sample_t), get_ts, 0.0); + TEST_ASSERT(lb_empty == 0, "lower_bound on empty buffer should be 0"); + + return true; +} + +bool test_bounds_on_segmented_snapshot() +{ + // Build a ring-buffer-style snapshot: first physical block holds newer + // samples, second holds older ones — but logical order (as the caller sees + // via snapshot.at) must be ascending. + std::vector tail; // older logical samples (first half) + std::vector head; // newer logical samples (second half) + for (int i = 0; i < 6; ++i) { + tail.push_back({static_cast(i), 0.0f}); + } + for (int i = 6; i < 10; ++i) { + head.push_back({static_cast(i), 0.0f}); + } + + plot::data_snapshot_t snap; + snap.data = tail.data(); + snap.count = 10; + snap.stride = sizeof(sample_t); + snap.data2 = head.data(); + snap.count2 = head.size(); + + const auto get_ts = [](const void* p) { + return static_cast(p)->t; + }; + + // Verify at() spans both segments monotonically. + TEST_ASSERT(get_ts(snap.at(0)) == 0.0, "segmented at(0) should be 0"); + TEST_ASSERT(get_ts(snap.at(5)) == 5.0, "segmented at(5) should be 5"); + TEST_ASSERT(get_ts(snap.at(6)) == 6.0, "segmented at(6) should cross into second segment"); + TEST_ASSERT(get_ts(snap.at(9)) == 9.0, "segmented at(9) should be 9"); + + auto lb = plot::detail::lower_bound_timestamp(snap, get_ts, 6.5); + TEST_ASSERT(lb == 7, "segmented lower_bound(6.5) should land on index 7"); + + auto ub = plot::detail::upper_bound_timestamp(snap, get_ts, 6.0); + TEST_ASSERT(ub == 7, "segmented upper_bound(6.0) should land on index 7"); + + // Empty snapshot. + plot::data_snapshot_t empty; + TEST_ASSERT(plot::detail::lower_bound_timestamp(empty, get_ts, 0.0) == 0, + "lower_bound on empty snapshot should be 0"); + + return true; +} + +bool test_format_axis_fixed_or_int() +{ + TEST_ASSERT(plot::format_axis_fixed_or_int(0.0, 0) == "0", + "zero as integer"); + TEST_ASSERT(plot::format_axis_fixed_or_int(3.0, 0) == "3", + "integer rounding"); + TEST_ASSERT(plot::format_axis_fixed_or_int(-3.49999, 0) == "-3", + "negative rounding towards zero at 0 digits"); + TEST_ASSERT(plot::format_axis_fixed_or_int(1.234, 2) == "1.23", + "fixed 2 digits"); + // Rounds to -0.00 but should print without sign. + TEST_ASSERT(plot::format_axis_fixed_or_int(-0.0001, 2) == "0.00", + "near-zero negative value should format as 0.00"); + return true; +} + +} // namespace + +int main() +{ + std::cout << "Core algorithm tests" << std::endl; + + int passed = 0; + int failed = 0; + + RUN_TEST(test_choose_lod_level_picks_closest_pps); + RUN_TEST(test_choose_lod_level_handles_degenerate_inputs); + RUN_TEST(test_choose_lod_level_prefers_level_zero_on_tie); + RUN_TEST(test_compute_lod_scales_forces_minimum_of_one); + RUN_TEST(test_lower_and_upper_bound_on_contiguous_buffer); + RUN_TEST(test_bounds_on_segmented_snapshot); + RUN_TEST(test_format_axis_fixed_or_int); + + std::cout << "Results: " << passed << " passed, " << failed << " failed" << std::endl; + return failed > 0 ? 1 : 0; +} From d75c00514f1e52ff4007059977133402ab36430c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Apr 2026 16:48:44 +0000 Subject: [PATCH 3/4] Make renderer callbacks truly private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously set_vbar_width_from_renderer and set_auto_v_range_from_renderer were public (without Q_INVOKABLE) because Plot_renderer's pimpl nested class (impl_t) could not take pointer-to-member expressions to private methods of Plot_widget — the friend class Plot_renderer declaration does not extend to nested types. Move the hooks into Plot_widget's private section and expose two static forwarders on Plot_renderer (post_vbar_width_from_renderer and post_auto_v_range_from_renderer). The forwarders form the pointer-to- member inside Plot_renderer's access context (where friendship applies) and pass it to QMetaObject::invokeMethod; impl_t calls those forwarders instead, which it can do because nested classes share their enclosing class's access rights. Net result: the methods no longer appear in the header's public section and can never be called by application code or QML, while the queued- invocation mechanism is preserved. https://claude.ai/code/session_012kn22AT9HdT6aoJoi8rz4Y --- include/vnm_plot/qt/plot_renderer.h | 8 ++++++ include/vnm_plot/qt/plot_widget.h | 44 ++++++++++++++--------------- src/qt/plot_renderer.cpp | 19 ++++++++++--- 3 files changed, 44 insertions(+), 27 deletions(-) diff --git a/include/vnm_plot/qt/plot_renderer.h b/include/vnm_plot/qt/plot_renderer.h index 80eb8f4..de4531b 100644 --- a/include/vnm_plot/qt/plot_renderer.h +++ b/include/vnm_plot/qt/plot_renderer.h @@ -37,6 +37,14 @@ class Plot_renderer : public QQuickFramebufferObject::Renderer private: struct impl_t; std::unique_ptr m_impl; + + // Forwarders for posting updates to the widget's private render-thread + // hooks. Plot_widget declares Plot_renderer (this class) as a friend, so + // taking the pointer-to-member of a private setter is legal here; the + // nested impl_t inherits that access through enclosing-class membership + // and calls these helpers instead of naming the private setters directly. + static void post_vbar_width_from_renderer(Plot_widget* widget, double px); + static void post_auto_v_range_from_renderer(Plot_widget* widget, float v_min, float v_max); }; } // namespace vnm::plot diff --git a/include/vnm_plot/qt/plot_widget.h b/include/vnm_plot/qt/plot_widget.h index 650bc98..23068a9 100644 --- a/include/vnm_plot/qt/plot_widget.h +++ b/include/vnm_plot/qt/plot_widget.h @@ -47,22 +47,21 @@ struct Plot_view // This is the main public interface for the vnm_plot library. // // Threading model: -// - All user-facing setters (e.g. set_t_range, set_v_range, set_config, -// add_series, adjust_* helpers) must be called from the Qt GUI thread (the -// one that owns the QQuickWindow). Internally they take short-lived locks on +// - All public setters (e.g. set_t_range, set_v_range, set_config, add_series, +// adjust_* helpers) must be called from the Qt GUI thread (the one that +// owns the QQuickWindow). Internally they take short-lived locks on // m_config_mutex, m_data_cfg_mutex, or m_series_mutex so the render thread // can read a consistent snapshot. -// - set_vbar_width_from_renderer / set_auto_v_range_from_renderer are internal -// callbacks — Plot_renderer posts them through QMetaObject::invokeMethod so -// they execute on the GUI thread once the queued event is dispatched. They -// are intentionally not Q_INVOKABLE (QML must not call them). Application -// code should ignore them. -// - set_rendered_v_range / set_rendered_t_range run on the render thread and -// publish through atomics for UI-side readers (rendered_v_range / -// rendered_t_range). -// - Non-finite values (NaN, +/-inf) are silently ignored by the range setters; -// they never produce a partial update. Invalid ranges (max <= min) are also -// ignored. +// - The render thread never calls public setters directly. Plot_renderer +// posts updates through static forwarders that reach the private +// set_*_from_renderer hooks via QMetaObject::invokeMethod, so the actual +// state mutation always runs on the GUI thread once the queued event is +// dispatched. set_rendered_v_range / set_rendered_t_range (also private) +// are the exception: they run on the render thread and publish through +// atomics for UI-side readers (rendered_v_range / rendered_t_range). +// - Non-finite values (NaN, +/-inf) are silently ignored by the range +// setters; they never produce a partial update. Invalid ranges (max <= min) +// are also ignored. class Plot_widget : public QQuickFramebufferObject { Q_OBJECT @@ -190,15 +189,6 @@ class Plot_widget : public QQuickFramebufferObject Q_INVOKABLE QVariantList get_indicator_samples(double x, double plot_width, double plot_height, double mouse_px = -1.0) const; Q_INVOKABLE QString format_timestamp_precise(double timestamp) const; - // --- Internal render-thread callbacks --- - // These exist only so Plot_renderer can post updates back to the widget via - // QMetaObject::invokeMethod. They are NOT part of the QML API (no - // Q_INVOKABLE) and application code should not call them directly. They - // accept non-finite values by silently ignoring them, matching the rest of - // the range-setter surface. - void set_vbar_width_from_renderer(double px); - void set_auto_v_range_from_renderer(float v_min, float v_max); - // --- Qt Quick FBO Interface --- Renderer* createRenderer() const override; @@ -228,6 +218,14 @@ class Plot_widget : public QQuickFramebufferObject private: friend class Plot_renderer; + // Render-thread callbacks. Plot_renderer is a friend of this class and + // exposes static forwarders (Plot_renderer::post_vbar_width_from_renderer + // and post_auto_v_range_from_renderer) so its nested pimpl can invoke + // these through QMetaObject::invokeMethod. They are intentionally not part + // of the public or QML-visible API; application code must not call them. + void set_vbar_width_from_renderer(double px); + void set_auto_v_range_from_renderer(float v_min, float v_max); + // Lock order (if ever needed concurrently): config -> data_cfg -> series. // Prefer holding only one lock at a time. diff --git a/src/qt/plot_renderer.cpp b/src/qt/plot_renderer.cpp index 8f1627a..ce29e2e 100644 --- a/src/qt/plot_renderer.cpp +++ b/src/qt/plot_renderer.cpp @@ -966,9 +966,8 @@ const frame_layout_result_t& Plot_renderer::impl_t::calculate_frame_layout( { // Notify widget to animate to new width if (owner) { - post_to_plot_widget( + Plot_renderer::post_vbar_width_from_renderer( const_cast(owner), - &Plot_widget::set_vbar_width_from_renderer, measured_vbar_width); } @@ -1519,9 +1518,8 @@ void Plot_renderer::render() if (std::abs(v0 - m_impl->snapshot.cfg.v_min) > k_auto_v_sync_eps || std::abs(v1 - m_impl->snapshot.cfg.v_max) > k_auto_v_sync_eps) { - post_to_plot_widget( + Plot_renderer::post_auto_v_range_from_renderer( const_cast(m_impl->owner), - &Plot_widget::set_auto_v_range_from_renderer, v0, v1); } } @@ -1702,4 +1700,17 @@ QOpenGLFramebufferObject* Plot_renderer::createFramebufferObject(const QSize& si return new QOpenGLFramebufferObject(size, format); } +void Plot_renderer::post_vbar_width_from_renderer(Plot_widget* widget, double px) +{ + post_to_plot_widget(widget, &Plot_widget::set_vbar_width_from_renderer, px); +} + +void Plot_renderer::post_auto_v_range_from_renderer( + Plot_widget* widget, + float v_min, + float v_max) +{ + post_to_plot_widget(widget, &Plot_widget::set_auto_v_range_from_renderer, v_min, v_max); +} + } // namespace vnm::plot From b36b45afe2097155c9da1e45ba2de889a6e3cde0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Apr 2026 17:38:02 +0000 Subject: [PATCH 4/4] Trim public surface, dedup setters, share test macros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Public-header changes: - Move plot_renderer.h out of include/vnm_plot/qt/ into src/qt/. The class is only constructed by Plot_widget::createRenderer() and returned as the QQuickFramebufferObject::Renderer base type, so users never had a reason to see it. - Drop the four sub-renderer accessors from Plot_core (primitives, series_renderer, chrome_renderer, text_renderer). Nothing under examples/, benchmark/, benchmark_headless/, or tests/ used them; they were leaky pass-throughs that exposed renderer internals. Only asset_loader() — needed for asset overrides — stays. - Stop core.h umbrella from pulling in default_shaders.h and vertex_layout.h. vertex_layout.h still reaches users transitively through access_policy.h; default_shaders.h is renderer-internal. - Sync vnm_plot.h k_version_* with the 1.0.4 CMake project version that the README also references. - Rename Q_PROPERTY timeAxis → time_axis to match every other property in Plot_widget; update PlotIndicator.qml, PlotView.qml, README. Dedup / simplification: - Extract update_config_field<>() so set_dark_mode / set_grid_visibility / set_preview_visibility / set_line_width_px share one lock+revision +signal+update body instead of repeating it four times. - Extract clamp_t_range_to_available() and use it from both set_available_t_range and set_view, removing one verbatim copy of the pan-clamping logic. - Use hash_mix_u64 consistently across hash_data_sources and hash_series_snapshot (the helper existed but only the third hasher was using it). - Add data_snapshot_t::is_valid() and replace the `!snap || count == 0 || stride == 0` check at every site (six call sites across core, qt, and benchmark). Test cleanup: - Move TEST_ASSERT and RUN_TEST into tests/test_macros.h and include it from all seven test binaries instead of duplicating the macros in each. All 7 tests still pass on the full Qt build. https://claude.ai/code/session_012kn22AT9HdT6aoJoi8rz4Y --- CMakeLists.txt | 3 +- README.md | 4 +- benchmark/include/benchmark_data_source.h | 2 +- include/vnm_plot/core.h | 10 +- include/vnm_plot/core/plot_core.h | 11 +- include/vnm_plot/core/types.h | 9 ++ include/vnm_plot/qt.h | 1 - include/vnm_plot/qt/plot_widget.h | 12 +- include/vnm_plot/vnm_plot.h | 11 +- qml/VnmPlot/PlotIndicator.qml | 2 +- qml/VnmPlot/PlotView.qml | 2 +- src/core/plot_core.cpp | 4 - src/core/series_renderer.cpp | 2 +- src/qt/plot_renderer.cpp | 38 +++--- {include/vnm_plot => src}/qt/plot_renderer.h | 0 src/qt/plot_widget.cpp | 130 +++++++------------ tests/test_asset_loader.cpp | 23 +--- tests/test_cache_invalidation.cpp | 23 +--- tests/test_concurrent_series.cpp | 23 +--- tests/test_core_algo.cpp | 23 +--- tests/test_macros.h | 29 +++++ tests/test_plot_interaction_item.cpp | 23 +--- tests/test_snapshot_caching.cpp | 22 +--- tests/test_typed_api.cpp | 23 +--- 24 files changed, 157 insertions(+), 273 deletions(-) rename {include/vnm_plot => src}/qt/plot_renderer.h (100%) create mode 100644 tests/test_macros.h diff --git a/CMakeLists.txt b/CMakeLists.txt index a5a49b3..cc6eedc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -331,10 +331,11 @@ set(VNM_PLOT_SOURCES set(VNM_PLOT_HEADERS # Public headers - include/vnm_plot/qt/plot_renderer.h include/vnm_plot/qt/plot_widget.h include/vnm_plot/qt/plot_interaction_item.h include/vnm_plot/qt/plot_time_axis.h + # Private internal header (lives next to its sources, not installed) + src/qt/plot_renderer.h ) # ----------------------------------------------------------------------------- diff --git a/README.md b/README.md index 858ab5e..6d2002b 100644 --- a/README.md +++ b/README.md @@ -100,8 +100,8 @@ import VnmPlot 1.0 PlotTimeAxis { id: sharedAxis } Column { - PlotView { timeAxis: sharedAxis } - PlotView { timeAxis: sharedAxis } + PlotView { time_axis: sharedAxis } + PlotView { time_axis: sharedAxis } } ``` diff --git a/benchmark/include/benchmark_data_source.h b/benchmark/include/benchmark_data_source.h index 6f41ee5..29c38c7 100644 --- a/benchmark/include/benchmark_data_source.h +++ b/benchmark/include/benchmark_data_source.h @@ -110,7 +110,7 @@ class Benchmark_data_source : public vnm::plot::Data_source { /// Update value range from current snapshot data void update_value_range(const vnm::plot::data_snapshot_t& snapshot) { - if (!snapshot || snapshot.count == 0 || snapshot.stride == 0) { + if (!snapshot.is_valid()) { m_value_min = 0.0f; m_value_max = 0.0f; m_range_valid = false; diff --git a/include/vnm_plot/core.h b/include/vnm_plot/core.h index 9be1145..ed56f2c 100644 --- a/include/vnm_plot/core.h +++ b/include/vnm_plot/core.h @@ -1,15 +1,19 @@ #pragma once // VNM Plot Library - Core Public Header +// +// Pulls in the user-facing core API. Renderer-internal headers +// (default_shaders.h, vertex_layout.h, gl_program.h, primitive_renderer.h, +// chrome_renderer.h, series_renderer.h, layout_calculator.h, range_cache.h, +// font_renderer.h, text_renderer.h) are reachable individually but are not +// surfaced through this umbrella because applications should not need them. -#include +#include // pulls vertex_layout.h transitively #include #include #include #include -#include #include #include #include #include #include -#include diff --git a/include/vnm_plot/core/plot_core.h b/include/vnm_plot/core/plot_core.h index 51a2cf2..16fcebd 100644 --- a/include/vnm_plot/core/plot_core.h +++ b/include/vnm_plot/core/plot_core.h @@ -12,10 +12,6 @@ namespace vnm::plot { class Asset_loader; -class Primitive_renderer; -class Series_renderer; -class Chrome_renderer; -class Text_renderer; struct Plot_config; class Plot_core @@ -57,11 +53,10 @@ class Plot_core bool initialize(const init_params_t& params); void cleanup_gl_resources(); + // Loader for application-supplied assets (custom shaders, fonts, etc.). + // Users register overrides through this; the sub-renderers themselves are + // intentionally not exposed. Asset_loader& asset_loader(); - Primitive_renderer& primitives(); - Series_renderer& series_renderer(); - Chrome_renderer& chrome_renderer(); - Text_renderer* text_renderer(); void render( const render_params_t& params, diff --git a/include/vnm_plot/core/types.h b/include/vnm_plot/core/types.h index 8d6309d..f258f77 100644 --- a/include/vnm_plot/core/types.h +++ b/include/vnm_plot/core/types.h @@ -80,6 +80,15 @@ struct data_snapshot_t explicit operator bool() const { return data != nullptr && count > 0; } + /// True iff the snapshot is non-empty AND has a usable stride. + /// Prefer this over the bool conversion when downstream code is going to + /// dereference samples; the bool conversion only checks `data` and + /// `count` and does not catch a zero-stride source. + bool is_valid() const noexcept + { + return data != nullptr && count > 0 && stride > 0; + } + const void* at(size_t index) const { if (index >= count || stride == 0) { diff --git a/include/vnm_plot/qt.h b/include/vnm_plot/qt.h index f543b36..fa9f153 100644 --- a/include/vnm_plot/qt.h +++ b/include/vnm_plot/qt.h @@ -2,6 +2,5 @@ // VNM Plot Library - Qt Public Header #include -#include #include #include diff --git a/include/vnm_plot/qt/plot_widget.h b/include/vnm_plot/qt/plot_widget.h index 23068a9..ca63ab9 100644 --- a/include/vnm_plot/qt/plot_widget.h +++ b/include/vnm_plot/qt/plot_widget.h @@ -84,7 +84,7 @@ class Plot_widget : public QQuickFramebufferObject Q_PROPERTY(double line_width_px READ line_width_px WRITE set_line_width_px NOTIFY line_width_px_changed) Q_PROPERTY(double vbar_width_px READ vbar_width_pixels NOTIFY vbar_width_changed) Q_PROPERTY(double vbar_width_qml READ vbar_width_qml NOTIFY vbar_width_changed) - Q_PROPERTY(Plot_time_axis* timeAxis READ time_axis WRITE set_time_axis NOTIFY time_axis_changed) + Q_PROPERTY(Plot_time_axis* time_axis READ time_axis WRITE set_time_axis NOTIFY time_axis_changed) public: Plot_widget(); @@ -278,6 +278,16 @@ class Plot_widget : public QQuickFramebufferObject std::pair current_v_range() const; data_config_t data_cfg_snapshot() const; + // Shared body of the QML property setters: locks m_config_mutex, no-ops + // when the value is unchanged, otherwise bumps m_config_revision, fires + // the supplied signal, and requests a repaint. + template + void update_config_field(Field& field, Value new_value, Signal signal); + + // Apply an available-range clamp to m_data_cfg in place. Caller must hold + // m_data_cfg_mutex. Used by both set_available_t_range and set_view. + void clamp_t_range_to_available(double t_avail_min, double t_avail_max); + bool consume_view_state_reset_request(); void set_rendered_v_range(float v_min, float v_max) const; void set_rendered_t_range(double t_min, double t_max) const; diff --git a/include/vnm_plot/vnm_plot.h b/include/vnm_plot/vnm_plot.h index ed6ddc1..e408da2 100644 --- a/include/vnm_plot/vnm_plot.h +++ b/include/vnm_plot/vnm_plot.h @@ -27,11 +27,12 @@ namespace vnm::plot { -// Library version -constexpr int k_version_major = 0; -constexpr int k_version_minor = 1; -constexpr int k_version_patch = 0; +// Library version. Kept in sync with the CMake project() VERSION; bump both +// together when releasing. +constexpr int k_version_major = 1; +constexpr int k_version_minor = 0; +constexpr int k_version_patch = 4; -constexpr const char* k_version_string = "0.1.0"; +constexpr const char* k_version_string = "1.0.4"; } // namespace vnm::plot diff --git a/qml/VnmPlot/PlotIndicator.qml b/qml/VnmPlot/PlotIndicator.qml index 144d90a..29b397b 100644 --- a/qml/VnmPlot/PlotIndicator.qml +++ b/qml/VnmPlot/PlotIndicator.qml @@ -15,7 +15,7 @@ Item { property string x_value_label: "x" property string y_value_label: "y" - readonly property var time_axis: plot_widget ? plot_widget.timeAxis : null + readonly property var time_axis: plot_widget ? plot_widget.time_axis : null readonly property real usable_width: width - plot_widget.vbar_width_qml readonly property real usable_height: height - plot_widget.reserved_height diff --git a/qml/VnmPlot/PlotView.qml b/qml/VnmPlot/PlotView.qml index aa64552..c08476e 100644 --- a/qml/VnmPlot/PlotView.qml +++ b/qml/VnmPlot/PlotView.qml @@ -16,7 +16,7 @@ Item { PlotWidget { id: plot anchors.fill: parent - timeAxis: root.time_axis + time_axis: root.time_axis } PlotIndicator { diff --git a/src/core/plot_core.cpp b/src/core/plot_core.cpp index bd8de38..2cf777d 100644 --- a/src/core/plot_core.cpp +++ b/src/core/plot_core.cpp @@ -150,10 +150,6 @@ void Plot_core::cleanup_gl_resources() } Asset_loader& Plot_core::asset_loader() { return m_impl->asset_loader; } -Primitive_renderer& Plot_core::primitives() { return m_impl->primitives; } -Series_renderer& Plot_core::series_renderer() { return m_impl->series; } -Chrome_renderer& Plot_core::chrome_renderer() { return m_impl->chrome; } -Text_renderer* Plot_core::text_renderer() { return m_impl->text.get(); } void Plot_core::render( const render_params_t& params, diff --git a/src/core/series_renderer.cpp b/src/core/series_renderer.cpp index 33b93ce..e08cdf8 100644 --- a/src/core/series_renderer.cpp +++ b/src/core/series_renderer.cpp @@ -108,7 +108,7 @@ bool compute_aux_metric_range( { out_used_data_source_range = false; - if (!access.get_aux_metric || !snapshot || snapshot.count == 0 || snapshot.stride == 0) { + if (!access.get_aux_metric || !snapshot.is_valid()) { return false; } diff --git a/src/qt/plot_renderer.cpp b/src/qt/plot_renderer.cpp index ce29e2e..028519f 100644 --- a/src/qt/plot_renderer.cpp +++ b/src/qt/plot_renderer.cpp @@ -1,4 +1,4 @@ -#include +#include "plot_renderer.h" #include #include #include @@ -173,6 +173,13 @@ bool spans_approx_equal(double a, double b) return diff <= scale * k_eps; } +// Boost-style 64-bit hash combiner; declared first so the helpers below can +// share it instead of inlining the same XOR/shift mix at every call site. +void hash_mix_u64(std::uint64_t& hash, std::uint64_t value) +{ + hash ^= value + 0x9e3779b97f4a7c15ULL + (hash << 6) + (hash >> 2); +} + std::uint64_t hash_data_sources(const std::map>& series_map) { std::uint64_t hash = 1469598103934665603ULL; @@ -200,14 +207,14 @@ std::uint64_t hash_data_sources(const std::map(id); - value ^= main_ptr + 0x9e3779b97f4a7c15ULL + (value << 6) + (value >> 2); + hash_mix_u64(value, main_ptr); if (has_preview) { - value ^= 0x0f0f0f0f0f0f0f0fULL + (value << 6) + (value >> 2); - value ^= preview_ptr + 0x9e3779b97f4a7c15ULL + (value << 6) + (value >> 2); - value ^= preview_layout_key + 0x9e3779b97f4a7c15ULL + (value << 6) + (value >> 2); - value ^= preview_style_bits + 0x9e3779b97f4a7c15ULL + (value << 6) + (value >> 2); + hash_mix_u64(value, 0x0f0f0f0f0f0f0f0fULL); + hash_mix_u64(value, preview_ptr); + hash_mix_u64(value, preview_layout_key); + hash_mix_u64(value, preview_style_bits); } - hash ^= value + 0x9e3779b97f4a7c15ULL + (hash << 6) + (hash >> 2); + hash_mix_u64(hash, value); } return hash; } @@ -222,17 +229,12 @@ std::uint64_t hash_series_snapshot(const std::map( reinterpret_cast(series.get())); std::uint64_t value = static_cast(id); - value ^= series_ptr + 0x9e3779b97f4a7c15ULL + (value << 6) + (value >> 2); - hash ^= value + 0x9e3779b97f4a7c15ULL + (hash << 6) + (hash >> 2); + hash_mix_u64(value, series_ptr); + hash_mix_u64(hash, value); } return hash; } -void hash_mix_u64(std::uint64_t& hash, std::uint64_t value) -{ - hash ^= value + 0x9e3779b97f4a7c15ULL + (hash << 6) + (hash >> 2); -} - std::uint64_t hash_series_sequences(const std::map>& series_map) { std::uint64_t hash = 1469598103934665603ULL; @@ -288,7 +290,7 @@ bool scan_snapshot_minmax( return false; } - if (!snapshot || snapshot.count == 0 || snapshot.stride == 0) { + if (!snapshot.is_valid()) { return false; } @@ -352,7 +354,7 @@ bool find_window_indices( start_idx = 0; end_idx = snapshot.count; - if (!access.get_timestamp || !snapshot || snapshot.count == 0 || snapshot.stride == 0) { + if (!access.get_timestamp || !snapshot.is_valid()) { return false; } if (!(t_max > t_min)) { @@ -402,7 +404,7 @@ bool get_lod_minmax( VNM_PLOT_PROFILE_SCOPE(profiler, "renderer.frame.range_calc.get_lod_minmax.snapshot"); snapshot = data_source.snapshot(level); } - if (!snapshot || snapshot.count == 0 || snapshot.stride == 0) { + if (!snapshot.is_valid()) { if (entry.valid) { out_min = entry.v_min; out_max = entry.v_max; @@ -641,7 +643,7 @@ std::pair compute_visible_v_range( std::size_t applied_level = desired_level; data_snapshot_t snapshot = view.source->snapshot(applied_level); - if (!snapshot || snapshot.count == 0 || snapshot.stride == 0) { + if (!snapshot.is_valid()) { snapshot = snapshot0; applied_level = 0; } diff --git a/include/vnm_plot/qt/plot_renderer.h b/src/qt/plot_renderer.h similarity index 100% rename from include/vnm_plot/qt/plot_renderer.h rename to src/qt/plot_renderer.h diff --git a/src/qt/plot_widget.cpp b/src/qt/plot_widget.cpp index aafc955..10cd382 100644 --- a/src/qt/plot_widget.cpp +++ b/src/qt/plot_widget.cpp @@ -1,5 +1,5 @@ #include -#include +#include "plot_renderer.h" #include #include #include @@ -271,20 +271,29 @@ bool Plot_widget::dark_mode() const return m_config.dark_mode; } -void Plot_widget::set_dark_mode(bool dark) +// Common path for the QML-property setters: take the unique lock, compare, and +// only bump the revision / fire the signal / request a repaint when something +// actually changes. Keeps the four trivial setters honest and consistent. +template +void Plot_widget::update_config_field(Field& field, Value new_value, Signal signal) { { std::unique_lock lock(m_config_mutex); - if (m_config.dark_mode == dark) { + if (field == new_value) { return; } - m_config.dark_mode = dark; + field = static_cast(new_value); m_config_revision.fetch_add(1, std::memory_order_relaxed); } - emit dark_mode_changed(); + (this->*signal)(); update(); } +void Plot_widget::set_dark_mode(bool dark) +{ + update_config_field(m_config.dark_mode, dark, &Plot_widget::dark_mode_changed); +} + double Plot_widget::grid_visibility() const { std::shared_lock lock(m_config_mutex); @@ -293,18 +302,10 @@ double Plot_widget::grid_visibility() const void Plot_widget::set_grid_visibility(double visibility) { - // Clamp to 0..1 - visibility = std::clamp(visibility, 0.0, 1.0); - { - std::unique_lock lock(m_config_mutex); - if (m_config.grid_visibility == visibility) { - return; - } - m_config.grid_visibility = visibility; - m_config_revision.fetch_add(1, std::memory_order_relaxed); - } - emit grid_visibility_changed(); - update(); + update_config_field( + m_config.grid_visibility, + std::clamp(visibility, 0.0, 1.0), + &Plot_widget::grid_visibility_changed); } double Plot_widget::preview_visibility() const @@ -315,18 +316,10 @@ double Plot_widget::preview_visibility() const void Plot_widget::set_preview_visibility(double visibility) { - // Clamp to 0..1 - visibility = std::clamp(visibility, 0.0, 1.0); - { - std::unique_lock lock(m_config_mutex); - if (m_config.preview_visibility == visibility) { - return; - } - m_config.preview_visibility = visibility; - m_config_revision.fetch_add(1, std::memory_order_relaxed); - } - emit preview_visibility_changed(); - update(); + update_config_field( + m_config.preview_visibility, + std::clamp(visibility, 0.0, 1.0), + &Plot_widget::preview_visibility_changed); } double Plot_widget::line_width_px() const @@ -337,16 +330,7 @@ double Plot_widget::line_width_px() const void Plot_widget::set_line_width_px(double width) { - { - std::unique_lock lock(m_config_mutex); - if (m_config.line_width_px == width) { - return; - } - m_config.line_width_px = width; - m_config_revision.fetch_add(1, std::memory_order_relaxed); - } - emit line_width_px_changed(); - update(); + update_config_field(m_config.line_width_px, width, &Plot_widget::line_width_px_changed); } double Plot_widget::t_min() const @@ -391,6 +375,28 @@ void Plot_widget::set_t_range(double t_min, double t_max) update(); } +void Plot_widget::clamp_t_range_to_available(double t_avail_min, double t_avail_max) +{ + const double span = t_avail_max - t_avail_min; + const double cur_span = m_data_cfg.t_max - m_data_cfg.t_min; + if (cur_span > span) { + m_data_cfg.t_min = t_avail_min; + m_data_cfg.t_max = t_avail_max; + } + else { + if (m_data_cfg.t_min < t_avail_min) { + m_data_cfg.t_min = t_avail_min; + m_data_cfg.t_max = t_avail_min + cur_span; + } + if (m_data_cfg.t_max > t_avail_max) { + m_data_cfg.t_max = t_avail_max; + m_data_cfg.t_min = t_avail_max - cur_span; + } + } + m_data_cfg.t_available_min = t_avail_min; + m_data_cfg.t_available_max = t_avail_max; +} + void Plot_widget::set_available_t_range(double t_min, double t_max) { if (!std::isfinite(t_min) || !std::isfinite(t_max) || !(t_max > t_min)) { @@ -402,26 +408,7 @@ void Plot_widget::set_available_t_range(double t_min, double t_max) } { std::unique_lock lock(m_data_cfg_mutex); - if (t_max > t_min) { - const double span = t_max - t_min; - const double cur_span = m_data_cfg.t_max - m_data_cfg.t_min; - if (cur_span > span) { - m_data_cfg.t_min = t_min; - m_data_cfg.t_max = t_max; - } - else { - if (m_data_cfg.t_min < t_min) { - m_data_cfg.t_min = t_min; - m_data_cfg.t_max = t_min + cur_span; - } - if (m_data_cfg.t_max > t_max) { - m_data_cfg.t_max = t_max; - m_data_cfg.t_min = t_max - cur_span; - } - } - } - m_data_cfg.t_available_min = t_min; - m_data_cfg.t_available_max = t_max; + clamp_t_range_to_available(t_min, t_max); } emit t_limits_changed(); update(); @@ -461,26 +448,9 @@ void Plot_widget::set_view(const Plot_view& view) t_changed = true; } if (t_avail_ok) { - const double t_min = view.t_available_range->first; - const double t_max = view.t_available_range->second; - const double span = t_max - t_min; - const double cur_span = m_data_cfg.t_max - m_data_cfg.t_min; - if (cur_span > span) { - m_data_cfg.t_min = t_min; - m_data_cfg.t_max = t_max; - } - else { - if (m_data_cfg.t_min < t_min) { - m_data_cfg.t_min = t_min; - m_data_cfg.t_max = t_min + cur_span; - } - if (m_data_cfg.t_max > t_max) { - m_data_cfg.t_max = t_max; - m_data_cfg.t_min = t_max - cur_span; - } - } - m_data_cfg.t_available_min = t_min; - m_data_cfg.t_available_max = t_max; + clamp_t_range_to_available( + view.t_available_range->first, + view.t_available_range->second); t_changed = true; } } @@ -1225,7 +1195,7 @@ QVariantList Plot_widget::get_indicator_samples( } auto snap = series->main_source()->snapshot(0); - if (!snap || snap.count == 0 || snap.stride == 0) { + if (!snap.is_valid()) { continue; } diff --git a/tests/test_asset_loader.cpp b/tests/test_asset_loader.cpp index 85245f7..e560847 100644 --- a/tests/test_asset_loader.cpp +++ b/tests/test_asset_loader.cpp @@ -2,6 +2,8 @@ // Verifies logging wiring, embedded asset lookup, override-directory fallback, // and the shader_sources helper. +#include "test_macros.h" + #include #include @@ -15,27 +17,6 @@ namespace plot = vnm::plot; namespace { -#define TEST_ASSERT(cond, msg) \ - do { \ - if (!(cond)) { \ - std::cerr << "FAIL: " << msg << " (line " << __LINE__ << ")" << std::endl; \ - return false; \ - } \ - } while (0) - -#define RUN_TEST(test_fn) \ - do { \ - std::cout << "Running " << #test_fn << "... "; \ - if (test_fn()) { \ - std::cout << "OK" << std::endl; \ - ++passed; \ - } \ - else { \ - std::cout << "FAIL" << std::endl; \ - ++failed; \ - } \ - } while (0) - struct Scoped_temp_dir { std::filesystem::path path; diff --git a/tests/test_cache_invalidation.cpp b/tests/test_cache_invalidation.cpp index b8fa930..8a48a61 100644 --- a/tests/test_cache_invalidation.cpp +++ b/tests/test_cache_invalidation.cpp @@ -1,5 +1,7 @@ // vnm_plot core cache tests +#include "test_macros.h" + #include #include #include @@ -184,27 +186,6 @@ void fill_lod_data(Lod_data_source& ds) } } -#define TEST_ASSERT(cond, msg) \ - do { \ - if (!(cond)) { \ - std::cerr << "FAIL: " << msg << " (line " << __LINE__ << ")" << std::endl; \ - return false; \ - } \ - } while (0) - -#define RUN_TEST(test_fn) \ - do { \ - std::cout << "Running " << #test_fn << "... "; \ - if (test_fn()) { \ - std::cout << "OK" << std::endl; \ - ++passed; \ - } \ - else { \ - std::cout << "FAIL" << std::endl; \ - ++failed; \ - } \ - } while (0) - bool test_lod0_sequence_fallback_calls_snapshot() { auto data_source = std::make_shared(); diff --git a/tests/test_concurrent_series.cpp b/tests/test_concurrent_series.cpp index 3d80921..11c5a26 100644 --- a/tests/test_concurrent_series.cpp +++ b/tests/test_concurrent_series.cpp @@ -3,6 +3,8 @@ // concurrent snapshot and mutation, verifying that readers see a consistent // view and the monotonic sequence counter reflects updates. +#include "test_macros.h" + #include #include @@ -19,27 +21,6 @@ namespace plot = vnm::plot; namespace { -#define TEST_ASSERT(cond, msg) \ - do { \ - if (!(cond)) { \ - std::cerr << "FAIL: " << msg << " (line " << __LINE__ << ")" << std::endl; \ - return false; \ - } \ - } while (0) - -#define RUN_TEST(test_fn) \ - do { \ - std::cout << "Running " << #test_fn << "... "; \ - if (test_fn()) { \ - std::cout << "OK" << std::endl; \ - ++passed; \ - } \ - else { \ - std::cout << "FAIL" << std::endl; \ - ++failed; \ - } \ - } while (0) - struct sample_t { double t = 0.0; diff --git a/tests/test_core_algo.cpp b/tests/test_core_algo.cpp index 72e723a..a429ac5 100644 --- a/tests/test_core_algo.cpp +++ b/tests/test_core_algo.cpp @@ -2,6 +2,8 @@ // Covers LOD level selection, binary-search on timestamps (including segmented // snapshots), and the small axis formatter used by default_format_timestamp. +#include "test_macros.h" + #include #include @@ -16,27 +18,6 @@ namespace plot = vnm::plot; namespace { -#define TEST_ASSERT(cond, msg) \ - do { \ - if (!(cond)) { \ - std::cerr << "FAIL: " << msg << " (line " << __LINE__ << ")" << std::endl; \ - return false; \ - } \ - } while (0) - -#define RUN_TEST(test_fn) \ - do { \ - std::cout << "Running " << #test_fn << "... "; \ - if (test_fn()) { \ - std::cout << "OK" << std::endl; \ - ++passed; \ - } \ - else { \ - std::cout << "FAIL" << std::endl; \ - ++failed; \ - } \ - } while (0) - struct sample_t { double t = 0.0; diff --git a/tests/test_macros.h b/tests/test_macros.h new file mode 100644 index 0000000..cf70e4b --- /dev/null +++ b/tests/test_macros.h @@ -0,0 +1,29 @@ +#pragma once + +// Shared test macros for the vnm_plot core test binaries. Each test file +// declares passed/failed locals at the top of main() and uses RUN_TEST to +// invoke individual `bool test_*()` functions; a failing TEST_ASSERT prints +// the offending expression and short-circuits its containing test. + +#include + +#define TEST_ASSERT(cond, msg) \ + do { \ + if (!(cond)) { \ + std::cerr << "FAIL: " << msg << " (line " << __LINE__ << ")" << std::endl; \ + return false; \ + } \ + } while (0) + +#define RUN_TEST(test_fn) \ + do { \ + std::cout << "Running " << #test_fn << "... "; \ + if (test_fn()) { \ + std::cout << "OK" << std::endl; \ + ++passed; \ + } \ + else { \ + std::cout << "FAIL" << std::endl; \ + ++failed; \ + } \ + } while (0) diff --git a/tests/test_plot_interaction_item.cpp b/tests/test_plot_interaction_item.cpp index 350f2bd..31da17a 100644 --- a/tests/test_plot_interaction_item.cpp +++ b/tests/test_plot_interaction_item.cpp @@ -1,5 +1,7 @@ // vnm_plot interaction math tests +#include "test_macros.h" + #include #include @@ -11,27 +13,6 @@ namespace plot = vnm::plot; namespace { -#define TEST_ASSERT(cond, msg) \ - do { \ - if (!(cond)) { \ - std::cerr << "FAIL: " << msg << " (line " << __LINE__ << ")" << std::endl; \ - return false; \ - } \ - } while (0) - -#define RUN_TEST(test_fn) \ - do { \ - std::cout << "Running " << #test_fn << "... "; \ - if (test_fn()) { \ - std::cout << "OK" << std::endl; \ - ++passed; \ - } \ - else { \ - std::cout << "FAIL" << std::endl; \ - ++failed; \ - } \ - } while (0) - struct zoom_state_t { double scale = 1.0; diff --git a/tests/test_snapshot_caching.cpp b/tests/test_snapshot_caching.cpp index 40c9bee..1982bbd 100644 --- a/tests/test_snapshot_caching.cpp +++ b/tests/test_snapshot_caching.cpp @@ -1,5 +1,7 @@ // vnm_plot core snapshot cache tests +#include "test_macros.h" + #include #define private public #include @@ -171,26 +173,6 @@ void fill_lod_samples(Two_level_source& source) } } -#define TEST_ASSERT(cond, msg) \ - do { \ - if (!(cond)) { \ - std::cerr << "FAIL: " << msg << " (line " << __LINE__ << ")" << std::endl; \ - return false; \ - } \ - } while (0) - -#define RUN_TEST(test_fn) \ - do { \ - std::cout << "Running " << #test_fn << "... "; \ - if (test_fn()) { \ - std::cout << "OK" << std::endl; \ - ++passed; \ - } \ - else { \ - std::cout << "FAIL" << std::endl; \ - ++failed; \ - } \ - } while (0) bool test_frame_scoped_cache_reuse() { diff --git a/tests/test_typed_api.cpp b/tests/test_typed_api.cpp index 8df33b6..c22aa5b 100644 --- a/tests/test_typed_api.cpp +++ b/tests/test_typed_api.cpp @@ -1,5 +1,7 @@ // vnm_plot typed API tests +#include "test_macros.h" + #include #include #include @@ -23,27 +25,6 @@ struct sample_t float v_max = 0.0f; }; -#define TEST_ASSERT(cond, msg) \ - do { \ - if (!(cond)) { \ - std::cerr << "FAIL: " << msg << " (line " << __LINE__ << ")" << std::endl; \ - return false; \ - } \ - } while (0) - -#define RUN_TEST(test_fn) \ - do { \ - std::cout << "Running " << #test_fn << "... "; \ - if (test_fn()) { \ - std::cout << "OK" << std::endl; \ - ++passed; \ - } \ - else { \ - std::cout << "FAIL" << std::endl; \ - ++failed; \ - } \ - } while (0) - bool test_vertex_attrib_type_for_mappings() { TEST_ASSERT(plot::detail::vertex_attrib_type_for() == plot::Vertex_attrib_type::FLOAT32,