Style and architecture fixes

diff --git a/pv/views/trace/flag.cpp b/pv/views/trace/flag.cpp
index 97b21be232888dc909fc36b4df9c573005815f39..81af10868f46a070aaf8d8c45a56cce7359d622b 100644 (file)
--- a/pv/views/trace/flag.cpp
+++ b/pv/views/trace/flag.cpp
@@ -59,22 +59,28 @@ bool Flag::enabled() const
 
 QString Flag::get_text() const
 {
-       const shared_ptr<TimeItem> ref_item = view_.get_reference_time_item();
-       if (ref_item == nullptr || ref_item.get() == this) {
-               return text_;
-       } else {
-               return Ruler::format_time_with_distance(
+       QString s;
+
+       const shared_ptr<TimeItem> ref_item = view_.ruler()->get_reference_item();
+
+       if (!ref_item || (ref_item.get() == this))
+               s = text_;
+       else
+               s = Ruler::format_time_with_distance(
                        ref_item->time(), ref_item->delta(time_),
                        view_.tick_prefix(), view_.time_unit(), view_.tick_precision());
-       }
+
+       return s;
 }
 
 QRectF Flag::label_rect(const QRectF &rect) const
 {
-       const shared_ptr<TimeItem> ref_item = view_.get_reference_time_item();
-       if (ref_item == nullptr || ref_item.get() == this) {
-               return TimeMarker::label_rect(rect);
+       QRectF r;
+
+       const shared_ptr<TimeItem> ref_item = view_.ruler()->get_reference_item();
 
+       if (!ref_item || (ref_item.get() == this)) {
+               r = TimeMarker::label_rect(rect);
        } else {
                // TODO: Remove code duplication between here and cursor.cpp
                const float x = get_x();
@@ -85,17 +91,20 @@ QRectF Flag::label_rect(const QRectF &rect) const
                const QSizeF label_size(
                        text_size.width() + LabelPadding.width() * 2,
                        text_size.height() + LabelPadding.height() * 2);
-               const float top = rect.height() - label_size.height() -
-                       TimeMarker::ArrowSize - 0.5f;
+
                const float height = label_size.height();
+               const float top =
+                       rect.height() - label_size.height() - TimeMarker::ArrowSize - 0.5f;
 
                const pv::util::Timestamp& delta = ref_item->delta(time_);
 
                if (delta >= 0)
-                       return QRectF(x, top, label_size.width(), height);
+                       r = QRectF(x, top, label_size.width(), height);
                else
-                       return QRectF(x - label_size.width(), top, label_size.width(), height);
+                       r = QRectF(x - label_size.width(), top, label_size.width(), height);
        }
+
+       return r;
 }
 
 pv::widgets::Popup* Flag::create_popup(QWidget *parent)

diff --git a/pv/views/trace/ruler.cpp b/pv/views/trace/ruler.cpp
index 49cff2f14ca14633f982f266c10c3dfec29f35af..33746ed6f23814c888212b06c687afc071e3f7e2 100644 (file)
--- a/pv/views/trace/ruler.cpp
+++ b/pv/views/trace/ruler.cpp
@@ -19,7 +19,6 @@
 
 #include <extdef.h>
 
-#include <QApplication>
 #include <QFontMetrics>
 #include <QMenu>
 #include <QMouseEvent>
@@ -188,38 +187,45 @@ vector< shared_ptr<ViewItem> > Ruler::items()
 
 void Ruler::item_hover(const shared_ptr<ViewItem> &item, QPoint pos)
 {
-       Q_UNUSED(pos);
+       (void)pos;
+
        hover_item_ = dynamic_pointer_cast<TimeItem>(item);
 }
 
-shared_ptr<TimeItem> Ruler::get_reference_item()
+shared_ptr<TimeItem> Ruler::get_reference_item() const
 {
        if (mouse_modifiers_ & Qt::ShiftModifier)
                return nullptr;
 
-       if (hover_item_ != nullptr)
+       if (hover_item_)
                return hover_item_;
 
-       shared_ptr<TimeItem> found(nullptr);
+       shared_ptr<TimeItem> ref_item;
        const vector< shared_ptr<TimeItem> > items(view_.time_items());
+
        for (auto i = items.rbegin(); i != items.rend(); i++) {
                if ((*i)->enabled() && (*i)->selected()) {
-                       if (found == nullptr)
-                               found = *i;
-                       else
-                               return nullptr; // Return null if multiple items are selected
+                       if (!ref_item)
+                               ref_item = *i;
+                       else {
+                               // Return nothing if multiple items are selected
+                               ref_item.reset();
+                               break;
+                       }
                }
        }
 
-       return found;
+       return ref_item;
 }
 
 shared_ptr<ViewItem> Ruler::get_mouse_over_item(const QPoint &pt)
 {
        const vector< shared_ptr<TimeItem> > items(view_.time_items());
+
        for (auto i = items.rbegin(); i != items.rend(); i++)
                if ((*i)->enabled() && (*i)->label_rect(rect()).contains(pt))
                        return *i;
+
        return nullptr;
 }
 

diff --git a/pv/views/trace/ruler.hpp b/pv/views/trace/ruler.hpp
index 3a3ada528c7c32d1435e48e5ae19b1416bb94ad3..18a09d9baf5fce069f8ebeaf8b584a142058e36f 100644 (file)
--- a/pv/views/trace/ruler.hpp
+++ b/pv/views/trace/ruler.hpp
@@ -123,7 +123,7 @@ public:
        pv::util::Timestamp get_ruler_time_from_absolute_time(const pv::util::Timestamp& abs_time) const;
        pv::util::Timestamp get_absolute_time_from_ruler_time(const pv::util::Timestamp& ruler_time) const;
 
-       shared_ptr<TimeItem> get_reference_item();
+       shared_ptr<TimeItem> get_reference_item() const;
 
 protected:
        virtual void contextMenuEvent(QContextMenuEvent *event) override;

diff --git a/pv/views/trace/timeitem.cpp b/pv/views/trace/timeitem.cpp
index 47a7da3c503bbd616d5085a66b4ea5c8c103c74d..f2eb5957c45ce2d92f914afb2f7ba636957be916 100644 (file)
--- a/pv/views/trace/timeitem.cpp
+++ b/pv/views/trace/timeitem.cpp
@@ -40,6 +40,11 @@ void TimeItem::drag_by(const QPoint &delta)
                        view_.scale());
 }
 
+const pv::util::Timestamp TimeItem::delta(const pv::util::Timestamp& other) const
+{
+       return other - time();
+}
+
 } // namespace trace
 } // namespace views
 } // namespace pv

diff --git a/pv/views/trace/timeitem.hpp b/pv/views/trace/timeitem.hpp
index f53dda84ebc2efb019ab423aba02d6756961dad8..c9b322c7703c1d44287e9f14232e98697f1ca24d 100644 (file)
--- a/pv/views/trace/timeitem.hpp
+++ b/pv/views/trace/timeitem.hpp
@@ -53,7 +53,7 @@ public:
 
        virtual float get_x() const = 0;
 
-       virtual const pv::util::Timestamp delta(const pv::util::Timestamp& other) const = 0;
+       virtual const pv::util::Timestamp delta(const pv::util::Timestamp& other) const;
 
        /**
         * Drags the item to a delta relative to the drag point.

diff --git a/pv/views/trace/timemarker.cpp b/pv/views/trace/timemarker.cpp
index c547d63a7ecbcea528c6372d1f70782d44ae44d5..9cb33a7637e1d7a3d90550c5d90656d2b3fbedd6 100644 (file)
--- a/pv/views/trace/timemarker.cpp
+++ b/pv/views/trace/timemarker.cpp
@@ -77,11 +77,6 @@ float TimeMarker::get_x() const
        return roundf(((time_ - view_.offset()) / view_.scale()).convert_to<float>()) + 0.5f;
 }
 
-const pv::util::Timestamp TimeMarker::delta(const pv::util::Timestamp& other) const
-{
-       return other - time_;
-}
-
 QPoint TimeMarker::drag_point(const QRect &rect) const
 {
        (void)rect;

diff --git a/pv/views/trace/timemarker.hpp b/pv/views/trace/timemarker.hpp
index 327a498ffe964295d876eb6ea40d38ddb50557f4..99c56d34beedad769a7b4e988b456178210fa412 100644 (file)
--- a/pv/views/trace/timemarker.hpp
+++ b/pv/views/trace/timemarker.hpp
@@ -74,8 +74,6 @@ public:
 
        float get_x() const override;
 
-       virtual const pv::util::Timestamp delta(const pv::util::Timestamp& other) const override;
-
        /**
         * Gets the arrow-tip point of the time marker.
         * @param rect the rectangle of the ruler area.

diff --git a/pv/views/trace/triggermarker.cpp b/pv/views/trace/triggermarker.cpp
index ee1aa15adfb70025f03ba7c57978eeff0d0cba52..843ccd49b35475dc273e8d0ae3fbed9f5bd04d0c 100644 (file)
--- a/pv/views/trace/triggermarker.cpp
+++ b/pv/views/trace/triggermarker.cpp
@@ -52,6 +52,7 @@ bool TriggerMarker::is_draggable(QPoint pos) const
 void TriggerMarker::set_time(const pv::util::Timestamp& time)
 {
        time_ = time;
+
        view_.time_item_appearance_changed(true, true);
 }
 
@@ -65,11 +66,6 @@ float TriggerMarker::get_x() const
        return ((time_ - view_.offset()) / view_.scale()).convert_to<float>();
 }
 
-const pv::util::Timestamp TriggerMarker::delta(const pv::util::Timestamp& other) const
-{
-       return other - time_;
-}
-
 QPoint TriggerMarker::drag_point(const QRect &rect) const
 {
        (void)rect;

diff --git a/pv/views/trace/triggermarker.hpp b/pv/views/trace/triggermarker.hpp
index c81fd47065fa24a27e0521febe9ff909a1c22e1e..222d3fb9f78c76450d75f9dde25b32f6dc1240c0 100644 (file)
--- a/pv/views/trace/triggermarker.hpp
+++ b/pv/views/trace/triggermarker.hpp
@@ -71,8 +71,6 @@ public:
 
        float get_x() const override;
 
-       virtual const pv::util::Timestamp delta(const pv::util::Timestamp& other) const override;
-
        /**
         * Gets the arrow-tip point of the time marker.
         * @param rect the rectangle of the ruler area.

diff --git a/pv/views/trace/view.cpp b/pv/views/trace/view.cpp
index 22f23ccfbca737d00efa00c22ed07075560c9184..56461e4647effc796cad0895af355c72106ac2d7 100644 (file)
--- a/pv/views/trace/view.cpp
+++ b/pv/views/trace/view.cpp
@@ -129,7 +129,7 @@ View::View(Session &session, bool is_main_view, QWidget *parent) :
 
        // Note: Place defaults in View::reset_view_state(), not here
        splitter_(new QSplitter()),
-       header_was_shrunk_(false),      // The splitter remains unchanged after a reset, so this goes here
+       header_was_shrunk_(false),  // The splitter remains unchanged after a reset, so this goes here
        sticky_scrolling_(false)  // Default setting is set in MainWindow::setup_ui()
 {
        QVBoxLayout *root_layout = new QVBoxLayout(this);
@@ -168,8 +168,8 @@ View::View(Session &session, bool is_main_view, QWidget *parent) :
        splitter_->setHandleWidth(1);  // Don't show a visible rubber band
        splitter_->setCollapsible(0, false);  // Prevent the header from collapsing
        splitter_->setCollapsible(1, false);  // Prevent the traces from collapsing
-       splitter_->setStretchFactor(0, 0);      // Prevent the panes from being resized
-       splitter_->setStretchFactor(1, 1);      // when the entire view is resized
+       splitter_->setStretchFactor(0, 0);  // Prevent the panes from being resized
+       splitter_->setStretchFactor(1, 1);  // when the entire view is resized
        splitter_->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);
 
        viewport_->installEventFilter(this);
@@ -443,11 +443,6 @@ vector< shared_ptr<TimeItem> > View::time_items() const
        return items;
 }
 
-shared_ptr<TimeItem> View::get_reference_time_item()
-{
-       return ruler_->get_reference_item();
-}
-
 double View::scale() const
 {
        return scale_;
@@ -754,10 +749,10 @@ pair<Timestamp, Timestamp> View::get_time_extents() const
                        const Timestamp start_time = s->start_time();
                        left_time = left_time ?
                                min(*left_time, start_time) :
-                                                               start_time;
+                                               start_time;
                        right_time = right_time ?
                                max(*right_time, start_time + d->max_sample_count() / samplerate) :
-                                                                start_time + d->max_sample_count() / samplerate;
+                                                start_time + d->max_sample_count() / samplerate;
                }
        }
 
@@ -839,8 +834,8 @@ shared_ptr<CursorPair> View::cursors() const
 
 shared_ptr<Flag> View::add_flag(const Timestamp& time)
 {
-       shared_ptr<Flag> flag = make_shared<Flag>(
-                       *this, time, QString("%1").arg(next_flag_text_));
+       shared_ptr<Flag> flag =
+               make_shared<Flag>(*this, time, QString("%1").arg(next_flag_text_));
        flags_.push_back(flag);
 
        next_flag_text_ = (next_flag_text_ >= 'Z') ? 'A' :

diff --git a/pv/views/trace/view.hpp b/pv/views/trace/view.hpp
index ad28a829096039be703c02f627cd0c246fc98df0..e056805b4d798da23a2f2407ac3d5818e4e0c553 100644 (file)
--- a/pv/views/trace/view.hpp
+++ b/pv/views/trace/view.hpp
@@ -156,8 +156,6 @@ public:
         */
        vector< shared_ptr<TimeItem> > time_items() const;
 
-       shared_ptr<TimeItem> get_reference_time_item();
-
        /**
         * Returns the view time scale in seconds per pixel.
         */

diff --git a/pv/views/trace/viewwidget.cpp b/pv/views/trace/viewwidget.cpp
index d46182e449e67d5f3d0e7a71a063fb746a948144..e4de122d9e7ddd8bc6f0efc5763ac3b338fc63c2 100644 (file)
--- a/pv/views/trace/viewwidget.cpp
+++ b/pv/views/trace/viewwidget.cpp
@@ -287,7 +287,7 @@ void ViewWidget::mouseReleaseEvent(QMouseEvent *event)
 void ViewWidget::keyReleaseEvent(QKeyEvent *event)
 {
        // Update mouse_modifiers_ also if modifiers change, but pointer doesn't move
-       if (mouse_point_.x() >= 0 && mouse_point_.y() >= 0) // mouse is inside
+       if ((mouse_point_.x() >= 0) && (mouse_point_.y() >= 0)) // mouse is inside
                mouse_modifiers_ = event->modifiers();
        update();
 }
@@ -295,7 +295,7 @@ void ViewWidget::keyReleaseEvent(QKeyEvent *event)
 void ViewWidget::keyPressEvent(QKeyEvent *event)
 {
        // Update mouse_modifiers_ also if modifiers change, but pointer doesn't move
-       if (mouse_point_.x() >= 0 && mouse_point_.y() >= 0) // mouse is inside
+       if ((mouse_point_.x() >= 0) && (mouse_point_.y() >= 0)) // mouse is inside
                mouse_modifiers_ = event->modifiers();
        update();
 }

diff --git a/pv/views/trace/viewwidget.hpp b/pv/views/trace/viewwidget.hpp
index e7da1dbd25cbec17da10ae49e1aa8b30b5e20c97..427a8994e60cbeadf91995ce5fc6c3bfda76137e 100644 (file)
--- a/pv/views/trace/viewwidget.hpp
+++ b/pv/views/trace/viewwidget.hpp
@@ -147,10 +147,13 @@ Q_SIGNALS:
 protected:
        pv::views::trace::View &view_;
        QPoint mouse_point_;
-       Qt::KeyboardModifiers mouse_modifiers_;
        QPoint mouse_down_point_;
        pv::util::Timestamp mouse_down_offset_;
        shared_ptr<ViewItem> mouse_down_item_;
+
+       /// Keyboard modifiers that were active when mouse was last moved or clicked
+       Qt::KeyboardModifiers mouse_modifiers_;
+
        bool item_dragging_;
 };