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.

diff --git a/pv/view/ruler.cpp b/pv/view/ruler.cpp
index 6f8e89a5b69833ad3449167f0599dc87b87794a4..5f7b1f67faade83e7d3acf388e8230d26a7d07fa 100644 (file)
--- 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<double>();
        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<double>();
 
                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 d01b151243aeb2ea3c9a32ab8a3077cf20a76677..f717c3785674c2b814838b915a18d18741ad6fed 100644 (file)
--- a/pv/view/ruler.hpp
+++ b/pv/view/ruler.hpp
@@ -29,6 +29,12 @@
 #include "marginwidget.hpp"
 #include <pv/util.hpp>
 
+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 fd8781d642f06fff22f1ef2250e7fa6cfd05a6c6..e00917f64fa4d49086890ce333d924ed76ad77fc 100644 (file)
--- 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<double>() *
+                               (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<int>(), 0));
 
-               tick_period_width = tick_period_ / scale_;
+               tick_period_width = (tick_period_ / scale_).convert_to<double>();
 
                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<int>();
 
        horizontalScrollBar()->setPageStep(areaSize.width() / 2);
        horizontalScrollBar()->setSingleStep(major_tick_distance);

diff --git a/pv/view/view.hpp b/pv/view/view.hpp
index 90a9b877cb04b164d5bedd8d19f1c244fd1cc848..5f81d443c8f53e08353e91d13b9534b7a6805dd7 100644 (file)
--- 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 f2262b4a71c7bdfed60a0b595b922c32fe76858b..65182e11254dd8db0dbba02d37252d52eb5aa3b7 100644 (file)
--- 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 04c99819aa0d6691e917d43d11d0de10a829612c..2d67f2935e2bf48e3cc4b9c83208ec374bac168d 100644 (file)
--- a/test/test.cpp
+++ b/test/test.cpp
@@ -20,3 +20,9 @@
 
 #define BOOST_TEST_MAIN
 #include <boost/test/unit_test.hpp>
+#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 (file)
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 <jens.steinhauser@gmail.com>
+ *
+ * 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 <QString>
+
+std::ostream& operator<<(std::ostream& stream, const QString& str);
+
+#endif

diff --git a/test/util.cpp b/test/util.cpp
index 70550ae660c34115b6bc334ef0a01fdc4b20a36d..17c9155586f5ad9aa6da7cc0d855cdb26f1510cf 100644 (file)
--- a/test/util.cpp
+++ b/test/util.cpp
@@ -21,6 +21,7 @@
 #include <boost/test/unit_test.hpp>
 
 #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 (file)
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 <jens.steinhauser@gmail.com>
+ *
+ * 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 <boost/test/unit_test.hpp>
+#include <boost/test/floating_point_comparison.hpp>
+
+#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()