]> sigrok.org Git - pulseview.git/blobdiff - pv/views/trace/decodetrace.cpp
Session: Fix issue #67 by improving error handling
[pulseview.git] / pv / views / trace / decodetrace.cpp
index f4b9f3359ccbf35f41b845fb95842141c0e73afe..63384911d7f515b160249e94daa3c23b65fdd743 100644 (file)
@@ -17,9 +17,7 @@
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
-extern "C" {
 #include <libsigrokdecode/libsigrokdecode.h>
-}
 
 #include <limits>
 #include <mutex>
@@ -82,10 +80,6 @@ namespace pv {
 namespace views {
 namespace trace {
 
-#define DECODETRACE_COLOR_SATURATION (180) /* 0-255 */
-#define DECODETRACE_COLOR_VALUE (170) /* 0-255 */
-
-const QColor DecodeTrace::ErrorBgColor = QColor(0xEF, 0x29, 0x29);
 const QColor DecodeTrace::NoDecodeColor = QColor(0x88, 0x8A, 0x85);
 const QColor DecodeTrace::ExpandMarkerWarnColor = QColor(0xFF, 0xA5, 0x00); // QColorConstants::Svg::orange
 const QColor DecodeTrace::ExpandMarkerHiddenColor = QColor(0x69, 0x69, 0x69); // QColorConstants::Svg::dimgray
@@ -150,7 +144,6 @@ DecodeTrace::DecodeTrace(pv::Session &session,
        shared_ptr<data::SignalBase> signalbase, int index) :
        Trace(signalbase),
        session_(session),
-       max_visible_rows_(0),
        show_hidden_rows_(false),
        delete_mapper_(this),
        show_hide_mapper_(this),
@@ -165,7 +158,8 @@ DecodeTrace::DecodeTrace(pv::Session &session,
 
        // Determine shortest string we want to see displayed in full
        QFontMetrics m(QApplication::font());
-       min_useful_label_width_ = m.width("XX"); // e.g. two hex characters
+       // e.g. two hex characters
+       min_useful_label_width_ = util::text_width(m, "XX");
 
        default_row_height_ = (ViewItemPaintParams::text_height() * 6) / 4;
        annotation_height_ = (ViewItemPaintParams::text_height() * 5) / 4;
@@ -176,11 +170,14 @@ DecodeTrace::DecodeTrace(pv::Session &session,
        // Note: The offset equals the color of the first annotation
        QColor color;
        const int h = (120 + 160 * index) % 360;
-       const int s = DECODETRACE_COLOR_SATURATION;
-       const int v = DECODETRACE_COLOR_VALUE;
+       const int s = DECODE_COLOR_SATURATION;
+       const int v = DECODE_COLOR_VALUE;
        color.setHsv(h, s, v);
        base_->set_color(color);
 
+       connect(decode_signal_.get(), SIGNAL(color_changed(QColor)),
+               this, SLOT(on_color_changed(QColor)));
+
        connect(decode_signal_.get(), SIGNAL(new_annotations()),
                this, SLOT(on_new_annotations()));
        connect(decode_signal_.get(), SIGNAL(decode_reset()),
@@ -189,7 +186,16 @@ DecodeTrace::DecodeTrace(pv::Session &session,
                this, SLOT(on_decode_finished()));
        connect(decode_signal_.get(), SIGNAL(channels_updated()),
                this, SLOT(on_channels_updated()));
-
+#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
+       connect(&delete_mapper_, SIGNAL(mappedInt(int)),
+               this, SLOT(on_delete_decoder(int)));
+       connect(&show_hide_mapper_, SIGNAL(mappedInt(int)),
+               this, SLOT(on_show_hide_decoder(int)));
+       connect(&row_show_hide_mapper_, SIGNAL(mappedInt(int)),
+               this, SLOT(on_show_hide_row(int)));
+       connect(&class_show_hide_mapper_, SIGNAL(mappedObject(QObject*)),
+               this, SLOT(on_show_hide_class(QObject*)));
+#else
        connect(&delete_mapper_, SIGNAL(mapped(int)),
                this, SLOT(on_delete_decoder(int)));
        connect(&show_hide_mapper_, SIGNAL(mapped(int)),
@@ -198,6 +204,7 @@ DecodeTrace::DecodeTrace(pv::Session &session,
                this, SLOT(on_show_hide_row(int)));
        connect(&class_show_hide_mapper_, SIGNAL(mapped(QWidget*)),
                this, SLOT(on_show_hide_class(QWidget*)));
+#endif
 
        connect(&delayed_trace_updater_, SIGNAL(timeout()),
                this, SLOT(on_delayed_trace_update()));
@@ -242,10 +249,21 @@ shared_ptr<data::SignalBase> DecodeTrace::base() const
        return base_;
 }
 
+void DecodeTrace::set_owner(TraceTreeItemOwner *owner)
+{
+       Trace::set_owner(owner);
+
+       // The owner is set in trace::View::signals_changed(), which is a slot.
+       // So after this trace was added to the view, we won't have an owner
+       // that we need to initialize in update_rows(). Once we do, we call it
+       // from on_decode_reset().
+       on_decode_reset();
+}
+
 pair<int, int> DecodeTrace::v_extents() const
 {
        // Make an empty decode trace appear symmetrical
-       if (max_visible_rows_ == 0)
+       if (visible_rows_ == 0)
                return make_pair(-default_row_height_, default_row_height_);
 
        unsigned int height = 0;
@@ -265,6 +283,7 @@ void DecodeTrace::paint_back(QPainter &p, ViewItemPaintParams &pp)
 void DecodeTrace::paint_mid(QPainter &p, ViewItemPaintParams &pp)
 {
        lock_guard<mutex> lock(row_modification_mutex_);
+       unsigned int visible_rows;
 
 #if DECODETRACE_SHOW_RENDER_TIME
        render_time_.restart();
@@ -281,7 +300,7 @@ void DecodeTrace::paint_mid(QPainter &p, ViewItemPaintParams &pp)
        sample_range.second = min((int64_t)sample_range.second,
                decode_signal_->get_decoded_sample_count(current_segment_, false));
 
-       visible_rows_ = 0;
+       visible_rows = 0;
        int y = get_visual_y();
 
        for (DecodeTraceRow& r : rows_) {
@@ -311,23 +330,23 @@ void DecodeTrace::paint_mid(QPainter &p, ViewItemPaintParams &pp)
                if (r.currently_visible) {
                        draw_annotations(annotations, p, pp, y, r);
                        y += r.height;
-                       visible_rows_++;
+                       visible_rows++;
                }
        }
 
        draw_unresolved_period(p, pp.left(), pp.right());
 
-       if (visible_rows_ > max_visible_rows_) {
-               max_visible_rows_ = visible_rows_;
+       if (visible_rows != visible_rows_) {
+               visible_rows_ = visible_rows;
 
                // Call order is important, otherwise the lazy event handler won't work
                owner_->extents_changed(false, true);
                owner_->row_item_appearance_changed(false, true);
        }
 
-       const QString err = decode_signal_->error_message();
+       const QString err = base_->get_error_message();
        if (!err.isEmpty())
-               draw_error(p, err, pp);
+               paint_error(p, pp);
 
 #if DECODETRACE_SHOW_RENDER_TIME
        qDebug() << "Rendering" << base_->name() << "took" << render_time_.elapsed() << "ms";
@@ -601,9 +620,12 @@ void DecodeTrace::hover_point_changed(const QPoint &hp)
                r.expand_marker_highlighted = false;
 
        if (hover_row) {
-               int row_y = get_row_y(hover_row);
+               const pair<int, int> extents = v_extents();
+               const int trace_top = get_visual_y() + extents.first;
+               const int trace_btm = get_visual_y() + extents.second;
+
                if ((hp.x() > 0) && (hp.x() < (int)(ArrowSize + 3 + hover_row->title_width)) &&
-                       (hp.y() > (int)(row_y - ArrowSize)) && (hp.y() < (int)(row_y + ArrowSize))) {
+                       (hp.y() > trace_top) && (hp.y() < trace_btm)) {
 
                        hover_row->expand_marker_highlighted = true;
                        show_hidden_rows_ = true;
@@ -655,10 +677,19 @@ void DecodeTrace::mouse_left_press_event(const QMouseEvent* event)
                        continue;
 
                unsigned int y = get_row_y(&r);
-               if ((event->x() > 0) && (event->x() <= (int)(ArrowSize + 3 + r.title_width)) &&
-                       (event->y() > (int)(y - (default_row_height_ / 2))) &&
-                       (event->y() <= (int)(y + (default_row_height_ / 2)))) {
-
+               bool need_anim = true;
+#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
+               need_anim &= event->position().x() > 0;
+               need_anim &= event->position().x() <= (int)(ArrowSize + 3 + r.title_width);
+               need_anim &= event->position().y() > (int)(y - (default_row_height_ / 2));
+               need_anim &= event->position().y() <= (int)(y + (default_row_height_ / 2));
+#else
+               need_anim &= event->x() > 0;
+               need_anim &= event->x() <= (int)(ArrowSize + 3 + r.title_width);
+               need_anim &= event->y() > (int)(y - (default_row_height_ / 2));
+               need_anim &= event->y() <= (int)(y + (default_row_height_ / 2));
+#endif
+               if (need_anim) {
                        if (r.expanded) {
                                r.collapsing = true;
                                r.expanded = false;
@@ -686,7 +717,7 @@ void DecodeTrace::mouse_left_press_event(const QMouseEvent* event)
 void DecodeTrace::draw_annotations(deque<const Annotation*>& annotations,
                QPainter &p, const ViewItemPaintParams &pp, int y, const DecodeTraceRow& row)
 {
-       Annotation::Class block_class = 0;
+       uint32_t block_class = 0;
        bool block_class_uniform = true;
        qreal block_start = 0;
        int block_ann_count = 0;
@@ -778,8 +809,8 @@ void DecodeTrace::draw_annotation(const Annotation* a, QPainter &p,
        const double start = a->start_sample() / samples_per_pixel - pixels_offset;
        const double end = a->end_sample() / samples_per_pixel - pixels_offset;
 
-       p.setPen(row.ann_class_dark_color.at(a->ann_class_id()));
-       p.setBrush(row.ann_class_color.at(a->ann_class_id()));
+       p.setPen(a->dark_color());
+       p.setBrush(a->color());
 
        if ((start > (pp.right() + DrawPadding)) || (end < (pp.left() - DrawPadding)))
                return;
@@ -791,7 +822,7 @@ void DecodeTrace::draw_annotation(const Annotation* a, QPainter &p,
 }
 
 void DecodeTrace::draw_annotation_block(qreal start, qreal end,
-       Annotation::Class ann_class, bool use_ann_format, QPainter &p, int y,
+       uint32_t ann_class, bool use_ann_format, QPainter &p, int y,
        const DecodeTraceRow& row) const
 {
        const double top = y + .5 - annotation_height_ / 2;
@@ -802,8 +833,8 @@ void DecodeTrace::draw_annotation_block(qreal start, qreal end,
        // one format that all of these annotations have. Otherwise, we should use
        // a neutral color (i.e. gray)
        if (use_ann_format) {
-               p.setPen(row.ann_class_dark_color.at(ann_class));
-               p.setBrush(QBrush(row.ann_class_color.at(ann_class), Qt::Dense4Pattern));
+               p.setPen(row.decode_row->get_dark_class_color(ann_class));
+               p.setBrush(QBrush(row.decode_row->get_class_color(ann_class), Qt::Dense4Pattern));
        } else {
                p.setPen(QColor(Qt::darkGray));
                p.setBrush(QBrush(Qt::gray, Qt::Dense4Pattern));
@@ -893,28 +924,6 @@ void DecodeTrace::draw_range(const Annotation* a, QPainter &p,
                best_annotation, Qt::ElideRight, rect.width()));
 }
 
-void DecodeTrace::draw_error(QPainter &p, const QString &message,
-       const ViewItemPaintParams &pp)
-{
-       const int y = get_visual_y();
-
-       double samples_per_pixel, pixels_offset;
-       tie(pixels_offset, samples_per_pixel) = get_pixels_offset_samples_per_pixel();
-
-       p.setPen(ErrorBgColor.darker());
-       p.setBrush(ErrorBgColor);
-
-       const QRectF bounding_rect = QRectF(pp.left(), INT_MIN / 2 + y, pp.right(), INT_MAX);
-
-       const QRectF text_rect = p.boundingRect(bounding_rect, Qt::AlignCenter, message);
-       const qreal r = text_rect.height() / 4;
-
-       p.drawRoundedRect(text_rect.adjusted(-r, -r, r, r), r, r, Qt::AbsoluteSize);
-
-       p.setPen(Qt::black);
-       p.drawText(text_rect, message);
-}
-
 void DecodeTrace::draw_unresolved_period(QPainter &p, int left, int right) const
 {
        double samples_per_pixel, pixels_offset;
@@ -960,7 +969,7 @@ pair<double, double> DecodeTrace::get_pixels_offset_samples_per_pixel() const
        const double pixels_offset =
                ((view->offset() - decode_signal_->start_time()) / scale).convert_to<double>();
 
-       double samplerate = decode_signal_->samplerate();
+       double samplerate = decode_signal_->get_samplerate();
 
        // Show sample rate as 1Hz when it is unknown
        if (samplerate == 0.0)
@@ -984,34 +993,6 @@ pair<uint64_t, uint64_t> DecodeTrace::get_view_sample_range(
        return make_pair(start, end);
 }
 
-QColor DecodeTrace::get_row_color(int row_index) const
-{
-       // For each row color, use the base color hue and add an offset that's
-       // not a dividend of 360
-
-       QColor color;
-       const int h = (base_->color().toHsv().hue() + 20 * row_index) % 360;
-       const int s = DECODETRACE_COLOR_SATURATION;
-       const int v = DECODETRACE_COLOR_VALUE;
-       color.setHsl(h, s, v);
-
-       return color;
-}
-
-QColor DecodeTrace::get_annotation_color(QColor row_color, int annotation_index) const
-{
-       // For each row color, use the base color hue and add an offset that's
-       // not a dividend of 360 and not a multiple of the row offset
-
-       QColor color(row_color);
-       const int h = (color.toHsv().hue() + 55 * annotation_index) % 360;
-       const int s = DECODETRACE_COLOR_SATURATION;
-       const int v = DECODETRACE_COLOR_VALUE;
-       color.setHsl(h, s, v);
-
-       return color;
-}
-
 unsigned int DecodeTrace::get_row_y(const DecodeTraceRow* row) const
 {
        assert(row);
@@ -1068,8 +1049,17 @@ const QString DecodeTrace::get_annotation_at_point(const QPoint &point)
        decode_signal_->get_annotation_subset(annotations, r->decode_row,
                current_segment_, sample_range.first, sample_range.second);
 
-       return (annotations.empty()) ?
-               QString() : annotations[0]->annotations()->front();
+       const Annotation* a = annotations.empty() ? nullptr : annotations[0];
+
+       // Create a string of the format "CLASS: VALUE"
+       QString s;
+       if (a) {
+               if (!a->ann_class_description().isEmpty())
+                       s = a->ann_class_description() + ": ";
+               s += a->annotations()->front();
+       }
+
+       return s;
 }
 
 void DecodeTrace::create_decoder_form(int index, shared_ptr<Decoder> &dec,
@@ -1159,7 +1149,7 @@ QComboBox* DecodeTrace::create_channel_selector(QWidget *parent, const DecodeCha
 
        QComboBox *selector = new QComboBox(parent);
 
-       selector->addItem("-", qVariantFromValue((void*)nullptr));
+       selector->addItem("-", QVariant::fromValue((void*)nullptr));
 
        if (!ch->assigned_signal)
                selector->setCurrentIndex(0);
@@ -1167,10 +1157,9 @@ QComboBox* DecodeTrace::create_channel_selector(QWidget *parent, const DecodeCha
        for (const shared_ptr<data::SignalBase> &b : sig_list) {
                assert(b);
                if (b->logic_data() && b->enabled()) {
-                       selector->addItem(b->name(),
-                               qVariantFromValue((void*)b.get()));
+                       selector->addItem(b->name(), QVariant::fromValue(b));
 
-                       if (ch->assigned_signal == b.get())
+                       if (ch->assigned_signal == b)
                                selector->setCurrentIndex(selector->count() - 1);
                }
        }
@@ -1183,9 +1172,9 @@ QComboBox* DecodeTrace::create_channel_selector_init_state(QWidget *parent,
 {
        QComboBox *selector = new QComboBox(parent);
 
-       selector->addItem("0", qVariantFromValue((int)SRD_INITIAL_PIN_LOW));
-       selector->addItem("1", qVariantFromValue((int)SRD_INITIAL_PIN_HIGH));
-       selector->addItem("X", qVariantFromValue((int)SRD_INITIAL_PIN_SAME_AS_SAMPLE0));
+       selector->addItem("0", QVariant::fromValue((int)SRD_INITIAL_PIN_LOW));
+       selector->addItem("1", QVariant::fromValue((int)SRD_INITIAL_PIN_HIGH));
+       selector->addItem("X", QVariant::fromValue((int)SRD_INITIAL_PIN_SAME_AS_SAMPLE0));
 
        selector->setCurrentIndex(ch->initial_pin_state);
 
@@ -1279,6 +1268,19 @@ void DecodeTrace::initialize_row_widgets(DecodeTraceRow* r, unsigned int row_id)
        QPalette header_palette = owner_->view()->palette();
        QPalette selector_palette = owner_->view()->palette();
 
+#if QT_VERSION >= QT_VERSION_CHECK(5, 12, 0)
+       if (GlobalSettings::current_theme_is_dark()) {
+               header_palette.setColor(QPalette::Window,
+                       QColor(255, 255, 255, ExpansionAreaHeaderAlpha));
+               selector_palette.setColor(QPalette::Window,
+                       QColor(255, 255, 255, ExpansionAreaAlpha));
+       } else {
+               header_palette.setColor(QPalette::Window,
+                       QColor(0, 0, 0, ExpansionAreaHeaderAlpha));
+               selector_palette.setColor(QPalette::Window,
+                       QColor(0, 0, 0, ExpansionAreaAlpha));
+       }
+#else
        if (GlobalSettings::current_theme_is_dark()) {
                header_palette.setColor(QPalette::Background,
                        QColor(255, 255, 255, ExpansionAreaHeaderAlpha));
@@ -1290,6 +1292,7 @@ void DecodeTrace::initialize_row_widgets(DecodeTraceRow* r, unsigned int row_id)
                selector_palette.setColor(QPalette::Background,
                        QColor(0, 0, 0, ExpansionAreaAlpha));
        }
+#endif
 
        const int w = m.boundingRect(r->decode_row->title()).width() + RowTitleMargin;
        r->title_width = w;
@@ -1357,11 +1360,11 @@ void DecodeTrace::initialize_row_widgets(DecodeTraceRow* r, unsigned int row_id)
        for (const AnnotationClass* ann_class : ann_classes) {
                cb = new QCheckBox();
                cb->setText(tr(ann_class->description));
-               cb->setChecked(ann_class->visible);
+               cb->setChecked(ann_class->visible());
 
                int dim = ViewItemPaintParams::text_height() - 2;
                QPixmap pixmap(dim, dim);
-               pixmap.fill(r->ann_class_color[ann_class->id]);
+               pixmap.fill(r->decode_row->get_class_color(ann_class->id));
                cb->setIcon(pixmap);
 
                r->selector_container->layout()->addWidget(cb);
@@ -1378,6 +1381,9 @@ void DecodeTrace::initialize_row_widgets(DecodeTraceRow* r, unsigned int row_id)
 
 void DecodeTrace::update_rows()
 {
+       if (!owner_)
+               return;
+
        lock_guard<mutex> lock(row_modification_mutex_);
 
        for (DecodeTraceRow& r : rows_)
@@ -1394,6 +1400,7 @@ void DecodeTrace::update_rows()
                        // Row doesn't exist yet, create and append it
                        DecodeTraceRow nr;
                        nr.decode_row = decode_row;
+                       nr.decode_row->set_base_color(base_->color());
                        nr.height = default_row_height_;
                        nr.expanded_height = default_row_height_;
                        nr.currently_visible = false;
@@ -1407,16 +1414,6 @@ void DecodeTrace::update_rows()
                        nr.header_container = new QWidget(nr.container);
                        nr.selector_container = new QWidget(nr.container);
 
-                       nr.row_color = get_row_color(decode_row->index());
-
-                       vector<AnnotationClass*> ann_classes = decode_row->ann_classes();
-                       for (const AnnotationClass* ann_class : ann_classes) {
-                               nr.ann_class_color[ann_class->id] =
-                                       get_annotation_color(nr.row_color, ann_class->id);
-                               nr.ann_class_dark_color[ann_class->id] =
-                                       nr.ann_class_color[ann_class->id].darker();
-                       }
-
                        rows_.push_back(nr);
                        r = &rows_.back();
                        initialize_row_widgets(r, row_id);
@@ -1467,8 +1464,6 @@ void DecodeTrace::set_row_expanded(DecodeTraceRow* r)
 
        r->container->resize(owner_->view()->viewport()->width() - r->container->pos().x(),
                r->height - 2 * default_row_height_);
-
-       max_visible_rows_ = 0;
 }
 
 void DecodeTrace::set_row_collapsed(DecodeTraceRow* r)
@@ -1481,8 +1476,6 @@ void DecodeTrace::set_row_collapsed(DecodeTraceRow* r)
 
        r->container->resize(owner_->view()->viewport()->width() - r->container->pos().x(),
                r->height - 2 * default_row_height_);
-
-       max_visible_rows_ = 0;
 }
 
 void DecodeTrace::update_expanded_rows()
@@ -1512,10 +1505,17 @@ void DecodeTrace::on_setting_changed(const QString &key, const QVariant &value)
 {
        Trace::on_setting_changed(key, value);
 
-       if (key == GlobalSettings::Key_Dec_AlwaysShowAllRows) {
-               max_visible_rows_ = 0;
+       if (key == GlobalSettings::Key_Dec_AlwaysShowAllRows)
                always_show_all_rows_ = value.toBool();
-       }
+}
+
+void DecodeTrace::on_color_changed(const QColor &color)
+{
+       for (DecodeTraceRow& r : rows_)
+               r.decode_row->set_base_color(color);
+
+       if (owner_)
+               owner_->row_item_appearance_changed(false, true);
 }
 
 void DecodeTrace::on_new_annotations()
@@ -1532,7 +1532,6 @@ void DecodeTrace::on_delayed_trace_update()
 
 void DecodeTrace::on_decode_reset()
 {
-       max_visible_rows_ = 0;
        update_rows();
 
        if (owner_)
@@ -1563,8 +1562,8 @@ void DecodeTrace::on_channel_selected(int)
        QComboBox *cb = qobject_cast<QComboBox*>(QObject::sender());
 
        // Determine signal that was selected
-       const data::SignalBase *signal =
-               (data::SignalBase*)cb->itemData(cb->currentIndex()).value<void*>();
+       shared_ptr<data::SignalBase> signal =
+               cb->itemData(cb->currentIndex()).value<shared_ptr<data::SignalBase>>();
 
        // Determine decode channel ID this combo box is the channel selector for
        const uint16_t id = channel_id_map_.at(cb);
@@ -1604,8 +1603,6 @@ void DecodeTrace::on_delete_decoder(int index)
        decode_signal_->remove_decoder(index);
        update_rows();
 
-       // Force re-calculation of the trace height
-       max_visible_rows_ = 0;
        owner_->extents_changed(false, true);
 
        create_popup_form();
@@ -1618,11 +1615,8 @@ void DecodeTrace::on_show_hide_decoder(int index)
        assert(index < (int)decoder_forms_.size());
        decoder_forms_[index]->set_decoder_visible(state);
 
-       if (!state) {
-               // Force re-calculation of the trace height, see paint_mid()
-               max_visible_rows_ = 0;
+       if (!state)
                owner_->extents_changed(false, true);
-       }
 
        owner_->row_item_appearance_changed(false, true);
 }
@@ -1637,19 +1631,21 @@ void DecodeTrace::on_show_hide_row(int row_id)
        if (!rows_[row_id].decode_row->visible())
                set_row_collapsed(&rows_[row_id]);
 
-       // Force re-calculation of the trace height, see paint_mid()
-       max_visible_rows_ = 0;
        owner_->extents_changed(false, true);
        owner_->row_item_appearance_changed(false, true);
 }
 
+#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
+void DecodeTrace::on_show_hide_class(QObject* sender)
+#else
 void DecodeTrace::on_show_hide_class(QWidget* sender)
+#endif
 {
        void* ann_class_ptr = sender->property("ann_class_ptr").value<void*>();
        assert(ann_class_ptr);
        AnnotationClass* ann_class = (AnnotationClass*)ann_class_ptr;
 
-       ann_class->visible = !ann_class->visible;
+       ann_class->set_visible(!ann_class->visible());
 
        void* row_ptr = sender->property("decode_trace_row_ptr").value<void*>();
        assert(row_ptr);
@@ -1820,7 +1816,7 @@ void DecodeTrace::on_animation_timer()
                if (r.expanding) {
                        if (r.height < r.expanded_height) {
                                r.anim_height += height_delta / (float)AnimationDurationInTicks;
-                               r.height = r.anim_height;
+                               r.height = min((int)r.anim_height, (int)r.expanded_height);
                                r.anim_shape += ArrowSize / (float)AnimationDurationInTicks;
                                animation_finished = false;
                        } else
@@ -1830,7 +1826,7 @@ void DecodeTrace::on_animation_timer()
                if (r.collapsing) {
                        if (r.height > default_row_height_) {
                                r.anim_height -= height_delta / (float)AnimationDurationInTicks;
-                               r.height = r.anim_height;
+                               r.height = max((int)r.anim_height, (int)0);
                                r.anim_shape -= ArrowSize / (float)AnimationDurationInTicks;
                                animation_finished = false;
                        } else
@@ -1865,8 +1861,6 @@ void DecodeTrace::on_hide_hidden_rows()
        if (!any_highlighted) {
                show_hidden_rows_ = false;
 
-               // Force re-calculation of the trace height, see paint_mid()
-               max_visible_rows_ = 0;
                owner_->extents_changed(false, true);
                owner_->row_item_appearance_changed(false, true);
        }