diff --git a/CMakeLists.txt b/CMakeLists.txt index 6af4c1e..cc6eedc 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 ) @@ -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_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/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/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..f258f77 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 @@ -71,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) { @@ -262,14 +280,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.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 fafa699..ca63ab9 100644 --- a/include/vnm_plot/qt/plot_widget.h +++ b/include/vnm_plot/qt/plot_widget.h @@ -45,6 +45,23 @@ 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. +// - 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 @@ -67,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(); @@ -148,9 +165,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 --- @@ -204,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. @@ -256,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 59a7d48..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, @@ -169,6 +165,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..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; } @@ -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/src/qt/plot_renderer.cpp b/src/qt/plot_renderer.cpp index 8f1627a..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; } @@ -966,9 +968,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 +1520,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 +1702,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 diff --git a/include/vnm_plot/qt/plot_renderer.h b/src/qt/plot_renderer.h similarity index 68% rename from include/vnm_plot/qt/plot_renderer.h rename to src/qt/plot_renderer.h index 80eb8f4..de4531b 100644 --- a/include/vnm_plot/qt/plot_renderer.h +++ b/src/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/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..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 @@ -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 && @@ -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 @@ -375,6 +359,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; @@ -388,34 +375,40 @@ 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)) { + return; + } if (m_time_axis) { m_time_axis->set_available_t_range(t_min, t_max); return; } { 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(); @@ -426,52 +419,43 @@ 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) { - 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) { - 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; + if (t_avail_ok) { + clamp_t_range_to_available( + view.t_available_range->first, + view.t_available_range->second); t_changed = true; } } - 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 +595,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; @@ -1208,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; } @@ -1413,8 +1400,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 +1417,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); } 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..e560847 --- /dev/null +++ b/tests/test_asset_loader.cpp @@ -0,0 +1,202 @@ +// vnm_plot Asset_loader tests +// Verifies logging wiring, embedded asset lookup, override-directory fallback, +// and the shader_sources helper. + +#include "test_macros.h" + +#include + +#include +#include +#include +#include +#include +#include + +namespace plot = vnm::plot; + +namespace { + +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_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 new file mode 100644 index 0000000..11c5a26 --- /dev/null +++ b/tests/test_concurrent_series.cpp @@ -0,0 +1,227 @@ +// 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 "test_macros.h" + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace plot = vnm::plot; + +namespace { + +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..a429ac5 --- /dev/null +++ b/tests/test_core_algo.cpp @@ -0,0 +1,228 @@ +// 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 "test_macros.h" + +#include +#include + +#include +#include +#include +#include +#include +#include + +namespace plot = vnm::plot; + +namespace { + +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; +} 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,