From c677193dc6e691493081fe87476a04e1674093a5 Mon Sep 17 00:00:00 2001 From: Jens Steinhauser Date: Wed, 2 Sep 2015 11:25:37 +0200 Subject: [PATCH] View: Change the type of tick_period to pv::util::Timestamp This makes the tick position/label calculation in the ruler more precise, avoiding rounded values for zero which would yield to "+0" or "-0" labels. --- pv/view/ruler.cpp | 9 ++- pv/view/ruler.hpp | 12 +++- pv/view/view.cpp | 20 +++--- pv/view/view.hpp | 6 +- test/CMakeLists.txt | 1 + test/test.cpp | 6 ++ test/test.hpp | 28 ++++++++ test/util.cpp | 6 +- test/view/ruler.cpp | 172 ++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 239 insertions(+), 21 deletions(-) create mode 100644 test/test.hpp create mode 100644 test/view/ruler.cpp diff --git a/pv/view/ruler.cpp b/pv/view/ruler.cpp index 6f8e89a..5f7b1f6 100644 --- a/pv/view/ruler.cpp +++ b/pv/view/ruler.cpp @@ -156,7 +156,7 @@ void Ruler::paintEvent(QPaintEvent*) } Ruler::TickPositions Ruler::calculate_tick_positions( - const double major_period, + const pv::util::Timestamp& major_period, const pv::util::Timestamp& offset, const double scale, const int width, @@ -164,7 +164,8 @@ Ruler::TickPositions Ruler::calculate_tick_positions( { TickPositions tp; - const double minor_period = major_period / MinorTickSubdivision; + const double minor_period = + (major_period / MinorTickSubdivision).convert_to(); const pv::util::Timestamp first_major_division = floor(offset / major_period); const pv::util::Timestamp first_minor_division = ceil(offset / minor_period); const pv::util::Timestamp t0 = first_major_division * major_period; @@ -175,10 +176,12 @@ Ruler::TickPositions Ruler::calculate_tick_positions( double x; do { - const pv::util::Timestamp t = t0 + division * minor_period; + pv::util::Timestamp t = t0 + division * minor_period; x = ((t - offset) / scale).convert_to(); if (division % MinorTickSubdivision == 0) { + // Recalculate 't' without using 'minor_period' which is of type double. + t = t0 + division / MinorTickSubdivision * major_period; tp.major.emplace_back(x, format_function(t)); } else { tp.minor.emplace_back(x); diff --git a/pv/view/ruler.hpp b/pv/view/ruler.hpp index d01b151..f717c37 100644 --- a/pv/view/ruler.hpp +++ b/pv/view/ruler.hpp @@ -29,6 +29,12 @@ #include "marginwidget.hpp" #include +namespace RulerTest { +class tick_position_test_0; +class tick_position_test_1; +class tick_position_test_2; +} + namespace pv { namespace view { @@ -39,6 +45,10 @@ class Ruler : public MarginWidget { Q_OBJECT + friend class RulerTest::tick_position_test_0; + friend class RulerTest::tick_position_test_1; + friend class RulerTest::tick_position_test_2; + private: /// Height of the ruler in multipes of the text height @@ -116,7 +126,7 @@ private: * tick positions. */ static TickPositions calculate_tick_positions( - const double major_period, + const pv::util::Timestamp& major_period, const pv::util::Timestamp& offset, const double scale, const int width, diff --git a/pv/view/view.cpp b/pv/view/view.cpp index fd8781d..e00917f 100644 --- a/pv/view/view.cpp +++ b/pv/view/view.cpp @@ -103,7 +103,7 @@ View::View(Session &session, QWidget *parent) : updating_scroll_(false), sticky_scrolling_(false), // Default setting is set in MainWindow::setup_ui() always_zoom_to_fit_(false), - tick_period_(0.0), + tick_period_(0), tick_prefix_(pv::util::SIPrefix::yocto), tick_precision_(0), time_unit_(util::TimeUnit::Time), @@ -275,12 +275,12 @@ void View::set_tick_precision(unsigned tick_precision) } } -double View::tick_period() const +const pv::util::Timestamp& View::tick_period() const { return tick_period_; } -void View::set_tick_period(double tick_period) +void View::set_tick_period(const pv::util::Timestamp& tick_period) { if (tick_period_ != tick_period) { tick_period_ = tick_period; @@ -561,7 +561,8 @@ void View::calculate_tick_spacing() const double min_period = scale_ * min_width; const int order = (int)floorf(log10f(min_period)); - const double order_decimal = pow(10.0, order); + const pv::util::Timestamp order_decimal = + pow(pv::util::Timestamp(10), order); // Allow for a margin of error so that a scale unit of 1 can be used. // Otherwise, for a SU of 1 the tick period will almost always be below @@ -572,7 +573,8 @@ void View::calculate_tick_spacing() unsigned int unit = 0; do { - tp_with_margin = order_decimal * (ScaleUnits[unit++] + tp_margin); + tp_with_margin = order_decimal.convert_to() * + (ScaleUnits[unit++] + tp_margin); } while (tp_with_margin < min_period && unit < countof(ScaleUnits)); set_tick_period(order_decimal * ScaleUnits[unit - 1]); @@ -581,9 +583,10 @@ void View::calculate_tick_spacing() // Precision is the number of fractional digits required, not // taking the prefix into account (and it must never be negative) - set_tick_precision(std::max((int)ceil(log10f(1 / tick_period_)), 0)); + set_tick_precision(std::max( + ceil(log10(1 / tick_period_)).convert_to(), 0)); - tick_period_width = tick_period_ / scale_; + tick_period_width = (tick_period_ / scale_).convert_to(); const QString label_text = format_time(max_time, tick_prefix_, time_unit_, tick_precision_); @@ -593,7 +596,6 @@ void View::calculate_tick_spacing() MinValueSpacing; min_width += SpacingIncrement; - } while (tick_period_width < label_width); } @@ -609,7 +611,7 @@ void View::update_scroll() get_scroll_layout(length, offset); length = max(length - areaSize.width(), 0.0); - int major_tick_distance = tick_period_ / scale_; + int major_tick_distance = (tick_period_ / scale_).convert_to(); horizontalScrollBar()->setPageStep(areaSize.width() / 2); horizontalScrollBar()->setSingleStep(major_tick_distance); diff --git a/pv/view/view.hpp b/pv/view/view.hpp index 90a9b87..5f81d44 100644 --- a/pv/view/view.hpp +++ b/pv/view/view.hpp @@ -133,7 +133,7 @@ public: /** * Returns period of the graticule time markings. */ - double tick_period() const; + const pv::util::Timestamp& tick_period() const; /** * Returns the unit of time currently used. @@ -343,7 +343,7 @@ private Q_SLOTS: * Sets the 'tick_period_' member and emits the 'tick_period_changed' * signal if needed. */ - void set_tick_period(double tick_period); + void set_tick_period(const pv::util::Timestamp& tick_period); /** * Sets the 'time_unit' member and emits the 'time_unit_changed' @@ -369,7 +369,7 @@ private: bool always_zoom_to_fit_; QTimer delayed_view_updater_; - double tick_period_; + pv::util::Timestamp tick_period_; pv::util::SIPrefix tick_prefix_; unsigned int tick_precision_; util::TimeUnit time_unit_; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index f2262b4..65182e1 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -73,6 +73,7 @@ set(pulseview_TEST_SOURCES ${PROJECT_SOURCE_DIR}/pv/widgets/wellarray.cpp data/analogsegment.cpp data/logicsegment.cpp + view/ruler.cpp test.cpp util.cpp ) diff --git a/test/test.cpp b/test/test.cpp index 04c9981..2d67f29 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -20,3 +20,9 @@ #define BOOST_TEST_MAIN #include +#include "test/test.hpp" + +std::ostream& operator<<(std::ostream& stream, const QString& str) +{ + return stream << str.toUtf8().data(); +} diff --git a/test/test.hpp b/test/test.hpp new file mode 100644 index 0000000..1400370 --- /dev/null +++ b/test/test.hpp @@ -0,0 +1,28 @@ +/* + * This file is part of the PulseView project. + * + * Copyright (C) 2015 Jens Steinhauser + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#ifndef PULSEVIEW_TEST_TEST_HPP +#define PULSEVIEW_TEST_TEST_HPP + +#include + +std::ostream& operator<<(std::ostream& stream, const QString& str); + +#endif diff --git a/test/util.cpp b/test/util.cpp index 70550ae..17c9155 100644 --- a/test/util.cpp +++ b/test/util.cpp @@ -21,6 +21,7 @@ #include #include "pv/util.hpp" +#include "test/test.hpp" using namespace pv::util; using ts = pv::util::Timestamp; @@ -39,11 +40,6 @@ namespace { pv::util::TimeUnit Time = pv::util::TimeUnit::Time; } -std::ostream& operator<<(std::ostream& stream, const QString& str) -{ - return stream << str.toUtf8().data(); -} - BOOST_AUTO_TEST_SUITE(UtilTest) BOOST_AUTO_TEST_CASE(exponent_test) diff --git a/test/view/ruler.cpp b/test/view/ruler.cpp new file mode 100644 index 0000000..400f845 --- /dev/null +++ b/test/view/ruler.cpp @@ -0,0 +1,172 @@ +/* + * This file is part of the PulseView project. + * + * Copyright (C) 2015 Jens Steinhauser + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include +#include + +#include "pv/view/ruler.hpp" +#include "test/test.hpp" + +using namespace pv::view; + +namespace { + QString format(const pv::util::Timestamp& t) + { + return pv::util::format_si_value(t, "s", pv::util::SIPrefix::none, 6); + } + + const double e = 0.0001; +}; + +BOOST_AUTO_TEST_SUITE(RulerTest) + +BOOST_AUTO_TEST_CASE(tick_position_test_0) +{ + const pv::util::Timestamp major_period("0.1"); + const pv::util::Timestamp offset("0"); + const double scale(0.001); + const int width(500); + + const Ruler::TickPositions ts = Ruler::calculate_tick_positions( + major_period, offset, scale, width, format); + + BOOST_REQUIRE_EQUAL(ts.major.size(), 6); + + BOOST_CHECK_CLOSE(ts.major[0].first, 0, e); + BOOST_CHECK_CLOSE(ts.major[1].first, 100, e); + BOOST_CHECK_CLOSE(ts.major[2].first, 200, e); + BOOST_CHECK_CLOSE(ts.major[3].first, 300, e); + BOOST_CHECK_CLOSE(ts.major[4].first, 400, e); + BOOST_CHECK_CLOSE(ts.major[5].first, 500, e); + + BOOST_CHECK_EQUAL(ts.major[0].second, "0.000000 s"); + BOOST_CHECK_EQUAL(ts.major[1].second, "+0.100000 s"); + BOOST_CHECK_EQUAL(ts.major[2].second, "+0.200000 s"); + BOOST_CHECK_EQUAL(ts.major[3].second, "+0.300000 s"); + BOOST_CHECK_EQUAL(ts.major[4].second, "+0.400000 s"); + BOOST_CHECK_EQUAL(ts.major[5].second, "+0.500000 s"); + + BOOST_REQUIRE_EQUAL(ts.minor.size(), 16); + + BOOST_CHECK_CLOSE(ts.minor[ 0], -25, e); + BOOST_CHECK_CLOSE(ts.minor[ 1], 25, e); + BOOST_CHECK_CLOSE(ts.minor[ 2], 50, e); + BOOST_CHECK_CLOSE(ts.minor[ 3], 75, e); + BOOST_CHECK_CLOSE(ts.minor[ 4], 125, e); + BOOST_CHECK_CLOSE(ts.minor[ 5], 150, e); + BOOST_CHECK_CLOSE(ts.minor[ 6], 175, e); + BOOST_CHECK_CLOSE(ts.minor[ 7], 225, e); + BOOST_CHECK_CLOSE(ts.minor[ 8], 250, e); + BOOST_CHECK_CLOSE(ts.minor[ 9], 275, e); + BOOST_CHECK_CLOSE(ts.minor[10], 325, e); + BOOST_CHECK_CLOSE(ts.minor[11], 350, e); + BOOST_CHECK_CLOSE(ts.minor[12], 375, e); + BOOST_CHECK_CLOSE(ts.minor[13], 425, e); + BOOST_CHECK_CLOSE(ts.minor[14], 450, e); + BOOST_CHECK_CLOSE(ts.minor[15], 475, e); +} + +BOOST_AUTO_TEST_CASE(tick_position_test_1) +{ + const pv::util::Timestamp major_period("0.1"); + const pv::util::Timestamp offset("-0.463"); + const double scale(0.001); + const int width(500); + + const Ruler::TickPositions ts = Ruler::calculate_tick_positions( + major_period, offset, scale, width, format); + + BOOST_REQUIRE_EQUAL(ts.major.size(), 5); + + BOOST_CHECK_CLOSE(ts.major[0].first, 63, e); + BOOST_CHECK_CLOSE(ts.major[1].first, 163, e); + BOOST_CHECK_CLOSE(ts.major[2].first, 263, e); + BOOST_CHECK_CLOSE(ts.major[3].first, 363, e); + BOOST_CHECK_CLOSE(ts.major[4].first, 463, e); + + BOOST_CHECK_EQUAL(ts.major[0].second, "-0.400000 s"); + BOOST_CHECK_EQUAL(ts.major[1].second, "-0.300000 s"); + BOOST_CHECK_EQUAL(ts.major[2].second, "-0.200000 s"); + BOOST_CHECK_EQUAL(ts.major[3].second, "-0.100000 s"); + BOOST_CHECK_EQUAL(ts.major[4].second, "0.000000 s"); + + BOOST_REQUIRE_EQUAL(ts.minor.size(), 17); + BOOST_CHECK_CLOSE(ts.minor[ 0], -12, e); + BOOST_CHECK_CLOSE(ts.minor[ 1], 13, e); + BOOST_CHECK_CLOSE(ts.minor[ 2], 38, e); + BOOST_CHECK_CLOSE(ts.minor[ 3], 88, e); + BOOST_CHECK_CLOSE(ts.minor[ 4], 113, e); + BOOST_CHECK_CLOSE(ts.minor[ 5], 138, e); + BOOST_CHECK_CLOSE(ts.minor[ 6], 188, e); + BOOST_CHECK_CLOSE(ts.minor[ 7], 213, e); + BOOST_CHECK_CLOSE(ts.minor[ 8], 238, e); + BOOST_CHECK_CLOSE(ts.minor[ 9], 288, e); + BOOST_CHECK_CLOSE(ts.minor[10], 313, e); + BOOST_CHECK_CLOSE(ts.minor[11], 338, e); + BOOST_CHECK_CLOSE(ts.minor[12], 388, e); + BOOST_CHECK_CLOSE(ts.minor[13], 413, e); + BOOST_CHECK_CLOSE(ts.minor[14], 438, e); + BOOST_CHECK_CLOSE(ts.minor[15], 488, e); + BOOST_CHECK_CLOSE(ts.minor[16], 513, e); +} + +BOOST_AUTO_TEST_CASE(tick_position_test_2) +{ + const pv::util::Timestamp major_period("20"); + const pv::util::Timestamp offset("8"); + const double scale(0.129746); + const int width(580); + + const Ruler::TickPositions ts = Ruler::calculate_tick_positions( + major_period, offset, scale, width, format); + + const double mp = 5; + const int off = 8; + + BOOST_REQUIRE_EQUAL(ts.major.size(), 4); + + BOOST_CHECK_CLOSE(ts.major[0].first, ( 4 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.major[1].first, ( 8 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.major[2].first, (12 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.major[3].first, (16 * mp - off) / scale, e); + + BOOST_CHECK_EQUAL(ts.major[0].second, "+20.000000 s"); + BOOST_CHECK_EQUAL(ts.major[1].second, "+40.000000 s"); + BOOST_CHECK_EQUAL(ts.major[2].second, "+60.000000 s"); + BOOST_CHECK_EQUAL(ts.major[3].second, "+80.000000 s"); + + BOOST_REQUIRE_EQUAL(ts.minor.size(), 13); + + BOOST_CHECK_CLOSE(ts.minor[ 0], ( 1 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.minor[ 1], ( 2 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.minor[ 2], ( 3 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.minor[ 3], ( 5 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.minor[ 4], ( 6 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.minor[ 5], ( 7 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.minor[ 6], ( 9 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.minor[ 7], (10 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.minor[ 8], (11 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.minor[ 9], (13 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.minor[10], (14 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.minor[11], (15 * mp - off) / scale, e); + BOOST_CHECK_CLOSE(ts.minor[12], (17 * mp - off) / scale, e); +} + +BOOST_AUTO_TEST_SUITE_END() -- 2.30.2