From 9f094349d415a37ec30fa2aa2bb6c979c03d6bc1 Mon Sep 17 00:00:00 2001 From: Soeren Apel Date: Sun, 30 Jun 2019 13:29:10 +0200 Subject: [PATCH] Style and architecture fixes --- pv/views/trace/flag.cpp | 35 ++++++++++++++++++++------------ pv/views/trace/ruler.cpp | 26 +++++++++++++++--------- pv/views/trace/ruler.hpp | 2 +- pv/views/trace/timeitem.cpp | 5 +++++ pv/views/trace/timeitem.hpp | 2 +- pv/views/trace/timemarker.cpp | 5 ----- pv/views/trace/timemarker.hpp | 2 -- pv/views/trace/triggermarker.cpp | 6 +----- pv/views/trace/triggermarker.hpp | 2 -- pv/views/trace/view.cpp | 19 +++++++---------- pv/views/trace/view.hpp | 2 -- pv/views/trace/viewwidget.cpp | 4 ++-- pv/views/trace/viewwidget.hpp | 5 ++++- 13 files changed, 59 insertions(+), 56 deletions(-) diff --git a/pv/views/trace/flag.cpp b/pv/views/trace/flag.cpp index 97b21be2..81af1086 100644 --- 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 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 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 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 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 49cff2f1..33746ed6 100644 --- a/pv/views/trace/ruler.cpp +++ b/pv/views/trace/ruler.cpp @@ -19,7 +19,6 @@ #include -#include #include #include #include @@ -188,38 +187,45 @@ vector< shared_ptr > Ruler::items() void Ruler::item_hover(const shared_ptr &item, QPoint pos) { - Q_UNUSED(pos); + (void)pos; + hover_item_ = dynamic_pointer_cast(item); } -shared_ptr Ruler::get_reference_item() +shared_ptr Ruler::get_reference_item() const { if (mouse_modifiers_ & Qt::ShiftModifier) return nullptr; - if (hover_item_ != nullptr) + if (hover_item_) return hover_item_; - shared_ptr found(nullptr); + shared_ptr ref_item; const vector< shared_ptr > 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 Ruler::get_mouse_over_item(const QPoint &pt) { const vector< shared_ptr > 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 3a3ada52..18a09d9b 100644 --- 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 get_reference_item(); + shared_ptr 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 47a7da3c..f2eb5957 100644 --- 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 f53dda84..c9b322c7 100644 --- 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 c547d63a..9cb33a76 100644 --- 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()) + 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 327a498f..99c56d34 100644 --- 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 ee1aa15a..843ccd49 100644 --- 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(); } -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 c81fd470..222d3fb9 100644 --- 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 22f23ccf..56461e46 100644 --- 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 > View::time_items() const return items; } -shared_ptr View::get_reference_time_item() -{ - return ruler_->get_reference_item(); -} - double View::scale() const { return scale_; @@ -754,10 +749,10 @@ pair 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 View::cursors() const shared_ptr View::add_flag(const Timestamp& time) { - shared_ptr flag = make_shared( - *this, time, QString("%1").arg(next_flag_text_)); + shared_ptr flag = + make_shared(*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 ad28a829..e056805b 100644 --- a/pv/views/trace/view.hpp +++ b/pv/views/trace/view.hpp @@ -156,8 +156,6 @@ public: */ vector< shared_ptr > time_items() const; - shared_ptr 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 d46182e4..e4de122d 100644 --- 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 e7da1dbd..427a8994 100644 --- 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 mouse_down_item_; + + /// Keyboard modifiers that were active when mouse was last moved or clicked + Qt::KeyboardModifiers mouse_modifiers_; + bool item_dragging_; }; -- 2.30.2