From 3d2fa8cd3ea0dbf350f2fee0c520bdc0d123a022 Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 27 Mar 2019 17:59:29 +0000 Subject: [PATCH] Optimise RichResourceText to special-case for single ResourceId text * Most cases don't have other text together with a ResourceId, so handle an isolated ResourceId specially and manually render it. * Further work - we could cache the name the same way as the RichResourceText does. So far it doesn't seem to appear on profiling. --- qrenderdoc/Code/QRDUtils.cpp | 214 +++++++++++++++++-- qrenderdoc/Code/QRDUtils.h | 44 ++-- qrenderdoc/Widgets/Extended/RDLabel.cpp | 6 +- qrenderdoc/Widgets/Extended/RDTableView.cpp | 2 +- qrenderdoc/Widgets/Extended/RDTreeWidget.cpp | 2 +- 5 files changed, 217 insertions(+), 51 deletions(-) diff --git a/qrenderdoc/Code/QRDUtils.cpp b/qrenderdoc/Code/QRDUtils.cpp index 561931929..76ebe63d7 100644 --- a/qrenderdoc/Code/QRDUtils.cpp +++ b/qrenderdoc/Code/QRDUtils.cpp @@ -69,6 +69,8 @@ std::string DoStringise(const ResourceId &el) return lit("ResourceId::%1").arg(num).toStdString(); } +// this is an opaque struct that contains the data to render, hit-test, etc for some text that +// contains links to resources. It will update and cache the names of the resources. struct RichResourceText { QVector fragments; @@ -165,13 +167,18 @@ struct RichResourceText for(i = 0; i < doc.blockCount(); i++) doc.findBlockByNumber(i).setUserState(fragmentIndexFromBlockIndex[i]); - qreal old = doc.textWidth(); doc.setTextWidth(-1); idealWidth = doc.idealWidth(); - doc.setTextWidth(old); + doc.setTextWidth(10000); } }; +// we use QSharedPointer to refer to the text since the lifetime management of these objects would +// get quite complicated. There's not necessarily an obvious QObject parent to assign to if the text +// is being initialised before being assigned to a widget and we want the most seamless interface we +// can get. +typedef QSharedPointer RichResourceTextPtr; + Q_DECLARE_METATYPE(RichResourceTextPtr); QString ResIdTextToString(RichResourceTextPtr ptr) @@ -179,28 +186,54 @@ QString ResIdTextToString(RichResourceTextPtr ptr) return ptr->text; } +QString ResIdToString(ResourceId ptr) +{ + return ToQStr(ptr); +} + void RegisterMetatypeConversions() { QMetaType::registerConverter(&ResIdTextToString); + QMetaType::registerConverter(&ResIdToString); } void RichResourceTextInitialise(QVariant &var) { + // we only upconvert from strings, any other type with a string representation is not expected to + // contain ResourceIds. In particular if the variant is already a ResourceId we can return. + if(GetVariantMetatype(var) != QMetaType::QString) + return; + + // we trim the string because that will happen naturally when rendering as HTML, and it makes it + // easier to detect strings where the only contents are ResourceId text. + QString text = var.toString().trimmed(); + + // do a simple string search first before using regular expressions + if(!text.contains(lit("ResourceId::"))) + return; + + // use regexp to split up into fragments of text and resourceid. The resourceid is then + // formatted on the fly in RichResourceText::cacheDocument static QRegularExpression re(lit("(ResourceId::)([0-9]*)")); - if(var.userType() == qMetaTypeId() || re.match(var.toString()).hasMatch()) + QRegularExpressionMatch match = re.match(text); + + if(match.hasMatch()) { - QString text; - if(var.userType() == qMetaTypeId()) - text = ToQStr(var.value()); - else - text = var.toString(); + // if the match is the whole string, this is just a plain ResourceId on its own, so make that + // the variant without being rich resource text, so we can process it faster. + if(match.capturedStart(0) == 0 && match.capturedLength(0) == text.length()) + { + qulonglong idnum = match.captured(2).toULongLong(); + ResourceId id; + memcpy(&id, &idnum, sizeof(id)); + + var = id; + return; + } RichResourceTextPtr linkedText(new RichResourceText); - // use regexp to split up into fragments of text and resourceid. The resourceid is then - // formatted on the fly in RichResourceText::cacheDocument - QRegularExpressionMatch match = re.match(text); while(match.hasMatch()) { qulonglong idnum = match.captured(2).toULongLong(); @@ -229,20 +262,78 @@ void RichResourceTextInitialise(QVariant &var) bool RichResourceTextCheck(const QVariant &var) { - return var.userType() == qMetaTypeId(); + return var.userType() == qMetaTypeId() || + var.userType() == qMetaTypeId(); } +// I'm not sure if this should come from the style or not - the QTextDocument handles this in +// the rich text case. +static const int RichResourceTextMargin = 2; + void RichResourceTextPaint(const QWidget *owner, QPainter *painter, QRect rect, QFont font, QPalette palette, bool mouseOver, QPoint mousePos, const QVariant &var) { + // special case handling for ResourceId + if(var.userType() == qMetaTypeId()) + { + QFont origfont = painter->font(); + QFont f = origfont; + f.setBold(true); + painter->setFont(f); + + static const int margin = RichResourceTextMargin; + + rect.adjust(margin, 0, -margin * 2, 0); + + QString name; + + ICaptureContext *ctxptr = getCaptureContext(owner); + + ResourceId id = var.value(); + + if(ctxptr) + name = ctxptr->GetResourceName(id); + else + name = ToQStr(id); + + painter->drawText(rect, Qt::AlignLeft | Qt::AlignVCenter, name); + + QRect textRect = + painter->fontMetrics().boundingRect(rect, Qt::AlignLeft | Qt::AlignVCenter, name); + + const QPixmap &px = Pixmaps::link(owner->devicePixelRatio()); + + rect.setTop(textRect.top()); + rect.setWidth(textRect.width() + margin + px.width()); + rect.setHeight(qMax(textRect.height(), px.height())); + + QPoint pos; + pos.setX(rect.right() - px.width() + 1); + pos.setY(rect.center().y() - px.height() / 2); + + painter->drawPixmap(pos, px, px.rect()); + + if(mouseOver && rect.contains(mousePos) && id != ResourceId()) + { + int underline_y = textRect.bottom() + margin; + + painter->setPen(QPen(palette.brush(QPalette::WindowText), 1.0)); + painter->drawLine(QPoint(rect.left(), underline_y), QPoint(rect.right(), underline_y)); + } + + painter->setFont(origfont); + + return; + } + RichResourceTextPtr linkedText = var.value(); linkedText->cacheDocument(owner); painter->translate(rect.left(), rect.top()); - linkedText->doc.setTextWidth(10000); - linkedText->doc.setDefaultFont(font); + if(font != linkedText->doc.defaultFont()) + linkedText->doc.setDefaultFont(font); // vertical align to the centre, if there's spare room. int diff = rect.height() - linkedText->doc.size().height(); @@ -298,8 +389,33 @@ void RichResourceTextPaint(const QWidget *owner, QPainter *painter, QRect rect, } } -int RichResourceTextWidthHint(const QWidget *owner, const QVariant &var) +int RichResourceTextWidthHint(const QWidget *owner, const QFont &font, const QVariant &var) { + // special case handling for ResourceId + if(var.userType() == qMetaTypeId()) + { + QFont f = font; + f.setBold(true); + + static const int margin = RichResourceTextMargin; + + QFontMetrics metrics(f); + + QString name; + + ICaptureContext *ctxptr = getCaptureContext(owner); + + if(ctxptr) + name = ctxptr->GetResourceName(var.value()); + else + name = ToQStr(var.value()); + + const QPixmap &px = Pixmaps::link(owner->devicePixelRatio()); + + int ret = margin + metrics.boundingRect(name).width() + margin + px.width() + margin; + return ret; + } + RichResourceTextPtr linkedText = var.value(); linkedText->cacheDocument(owner); @@ -308,23 +424,75 @@ int RichResourceTextWidthHint(const QWidget *owner, const QVariant &var) } bool RichResourceTextMouseEvent(const QWidget *owner, const QVariant &var, QRect rect, - QMouseEvent *event) + const QFont &font, QMouseEvent *event) { // only process clicks or moves if(event->type() != QEvent::MouseButtonRelease && event->type() != QEvent::MouseMove) return false; + // special case handling for ResourceId + if(var.userType() == qMetaTypeId()) + { + ResourceId id = var.value(); + + // empty resource ids are not clickable or hover-highlighted. + if(id == ResourceId()) + return false; + + QFont f = font; + f.setBold(true); + + static const int margin = RichResourceTextMargin; + + rect.adjust(margin, 0, -margin * 2, 0); + + QString name; + + ICaptureContext *ctxptr = getCaptureContext(owner); + + if(ctxptr) + name = ctxptr->GetResourceName(id); + else + name = ToQStr(id); + + QRect textRect = QFontMetrics(f).boundingRect(rect, Qt::AlignLeft | Qt::AlignVCenter, name); + + const QPixmap &px = Pixmaps::link(owner->devicePixelRatio()); + + rect.setTop(textRect.top()); + rect.setWidth(textRect.width() + margin + px.width()); + rect.setHeight(qMax(textRect.height(), px.height())); + + if(rect.contains(event->pos()) && id != ResourceId()) + { + if(event->type() == QEvent::MouseButtonRelease && ctxptr) + { + ICaptureContext &ctx = *(ICaptureContext *)ctxptr; + + if(!ctx.HasResourceInspector()) + ctx.ShowResourceInspector(); + + ctx.GetResourceInspector()->Inspect(id); + + ctx.RaiseDockWindow(ctx.GetResourceInspector()->Widget()); + } + + return true; + } + + return false; + } + RichResourceTextPtr linkedText = var.value(); linkedText->cacheDocument(owner); QAbstractTextDocumentLayout *layout = linkedText->doc.documentLayout(); - QPoint p = event->pos() - rect.topLeft(); - // vertical align to the centre, if there's spare room. int diff = rect.height() - linkedText->doc.size().height(); + QPoint p = event->pos() - rect.topLeft(); if(diff > 0) p -= QPoint(0, diff / 2); @@ -352,7 +520,7 @@ bool RichResourceTextMouseEvent(const QWidget *owner, const QVariant &var, QRect if(!ctx.HasResourceInspector()) ctx.ShowResourceInspector(); - ctx.GetResourceInspector()->Inspect(v.value()); + ctx.GetResourceInspector()->Inspect(res); ctx.RaiseDockWindow(ctx.GetResourceInspector()->Widget()); } @@ -428,7 +596,7 @@ QSize RichTextViewDelegate::sizeHint(const QStyleOptionViewItem &option, const Q QVariant v = index.data(); if(RichResourceTextCheck(v)) - return QSize(RichResourceTextWidthHint(m_widget, v), option.fontMetrics.height()); + return QSize(RichResourceTextWidthHint(m_widget, option.font, v), option.fontMetrics.height()); } return ForwardingDelegate::sizeHint(option, index); @@ -454,7 +622,7 @@ bool RichTextViewDelegate::editorEvent(QEvent *event, QAbstractItemModel *model, } // ignore the return value, we always consume clicks on this cell - RichResourceTextMouseEvent(m_widget, v, rect, (QMouseEvent *)event); + RichResourceTextMouseEvent(m_widget, v, rect, option.font, (QMouseEvent *)event); return true; } } @@ -462,7 +630,7 @@ bool RichTextViewDelegate::editorEvent(QEvent *event, QAbstractItemModel *model, return ForwardingDelegate::editorEvent(event, model, option, index); } -bool RichTextViewDelegate::linkHover(QMouseEvent *e, const QModelIndex &index) +bool RichTextViewDelegate::linkHover(QMouseEvent *e, const QFont &font, const QModelIndex &index) { if(index.isValid()) { @@ -482,7 +650,7 @@ bool RichTextViewDelegate::linkHover(QMouseEvent *e, const QModelIndex &index) 4); } - return RichResourceTextMouseEvent(m_widget, v, rect, e); + return RichResourceTextMouseEvent(m_widget, v, rect, font, e); } } diff --git a/qrenderdoc/Code/QRDUtils.h b/qrenderdoc/Code/QRDUtils.h index b7342418b..ee95fa56d 100644 --- a/qrenderdoc/Code/QRDUtils.h +++ b/qrenderdoc/Code/QRDUtils.h @@ -120,45 +120,43 @@ class RDTreeWidgetItem; void addStructuredObjects(RDTreeWidgetItem *parent, const StructuredObjectList &objs, bool parentIsArray); -// this is an opaque struct that contains the data to render, hit-test, etc for some text that -// contains links to resources. It will update and cache the names of the resources. -struct RichResourceText; - -// we use QSharedPointer to refer to the text since the lifetime management of these objects would -// get quite complicated. There's not necessarily an obvious QObject parent to assign to if the text -// is being initialised before being assigned to a widget and we want the most seamless interface we -// can get. -typedef QSharedPointer RichResourceTextPtr; - // this will check the variant, and if it contains a ResourceId directly or text with ResourceId -// identifiers then it will be converted into a RichResourceTextPtr in-place. The new QVariant will -// still convert to QString so it doesn't have to be special-cased. However it must be processed -// through one of the functions below (generally painting) to cache the rendered text. -// If the variant doesn't match the above conditions, it's unchanged. So it's safe to apply this +// identifiers then it will be converted into a RichResourceTextPtr or ResourceId in-place. The new +// QVariant will still convert to QString so it doesn't have to be special-cased. However it must be +// processed through one of the functions below (generally painting) to cache the rendered text. If +// the variant doesn't match the above conditions, it's unchanged. So it's safe to apply this // reasonably liberally. +// +// In this case the variant may not actually be a complex RichResourceText object, that's only used +// when there is text with ResourceId(s) inside it. If the text is just a ResourceId on its own +// (which is quite common) it will be stored as just a ResourceId but will still be painted & mouse +// handled the same way, and all of the below functions can be used on the variant either way. +// // NOTE: It is not possible to move a RichResourceText instance from one ICaptureContext to another // as the pointer is cached internally. Instead you should delete the old and re-initialise from // scratch. void RichResourceTextInitialise(QVariant &var); -// checks if a variant is rich resource text +// Checks if a variant is rich resource text and should be treated specially +// Particularly meaning we need mouse tracking on the widget to handle the on-hover highlighting +// and mouse clicks bool RichResourceTextCheck(const QVariant &var); -// paint the given variant containing rich text with the given parameters. +// Paint the given variant containing rich text with the given parameters. void RichResourceTextPaint(const QWidget *owner, QPainter *painter, QRect rect, QFont font, QPalette palette, bool mouseOver, QPoint mousePos, const QVariant &var); -// gives the width for a size hint for the rich text (since it might be larger than the original +// Gives the width for a size hint for the rich text (since it might be larger than the original // text) -int RichResourceTextWidthHint(const QWidget *owner, const QVariant &var); +int RichResourceTextWidthHint(const QWidget *owner, const QFont &font, const QVariant &var); -// handle a mouse event on some rich resource text. -// returns true if the event is processed - for mouse move events, this means that the mouse is over +// Handle a mouse event on some rich resource text. +// Returns true if the event is processed - for mouse move events, this means that the mouse is over // a resource link (which can be used to change the cursor to a pointing hand, for example). bool RichResourceTextMouseEvent(const QWidget *owner, const QVariant &var, QRect rect, - QMouseEvent *event); + const QFont &font, QMouseEvent *event); -// register runtime conversions for custom Qt metatypes +// Register runtime conversions for custom Qt metatypes void RegisterMetatypeConversions(); struct Formatter @@ -484,7 +482,7 @@ public: bool editorEvent(QEvent *event, QAbstractItemModel *model, const QStyleOptionViewItem &option, const QModelIndex &index) override; - bool linkHover(QMouseEvent *e, const QModelIndex &index); + bool linkHover(QMouseEvent *e, const QFont &font, const QModelIndex &index); private: QAbstractItemView *m_widget; diff --git a/qrenderdoc/Widgets/Extended/RDLabel.cpp b/qrenderdoc/Widgets/Extended/RDLabel.cpp index c3bb6ca08..1e9644b51 100644 --- a/qrenderdoc/Widgets/Extended/RDLabel.cpp +++ b/qrenderdoc/Widgets/Extended/RDLabel.cpp @@ -41,7 +41,7 @@ void RDLabel::modifySizeHint(QSize &sz) const sz.setWidth(sz.width() - contentsMargins().left() - contentsMargins().right()); if(m_variant.isValid()) - sz.setWidth(qMax(RichResourceTextWidthHint(this, m_variant) + contentsMargins().left() + + sz.setWidth(qMax(RichResourceTextWidthHint(this, font(), m_variant) + contentsMargins().left() + contentsMargins().right() + margin() * 2, sz.width())); } @@ -106,7 +106,7 @@ void RDLabel::mouseReleaseEvent(QMouseEvent *event) { if(m_variant.isValid()) { - RichResourceTextMouseEvent(this, m_variant, rect(), event); + RichResourceTextMouseEvent(this, m_variant, rect(), font(), event); return; } @@ -119,7 +119,7 @@ void RDLabel::mouseMoveEvent(QMouseEvent *event) if(m_variant.isValid()) { - bool hover = RichResourceTextMouseEvent(this, m_variant, rect(), event); + bool hover = RichResourceTextMouseEvent(this, m_variant, rect(), font(), event); if(hover) setCursor(QCursor(Qt::PointingHandCursor)); else diff --git a/qrenderdoc/Widgets/Extended/RDTableView.cpp b/qrenderdoc/Widgets/Extended/RDTableView.cpp index 5cfa32aeb..14a983920 100644 --- a/qrenderdoc/Widgets/Extended/RDTableView.cpp +++ b/qrenderdoc/Widgets/Extended/RDTableView.cpp @@ -342,7 +342,7 @@ void RDTableView::mouseMoveEvent(QMouseEvent *e) update(newHover); - if(m_delegate && m_delegate->linkHover(e, newHover)) + if(m_delegate && m_delegate->linkHover(e, font(), newHover)) { setCursor(QCursor(Qt::PointingHandCursor)); } diff --git a/qrenderdoc/Widgets/Extended/RDTreeWidget.cpp b/qrenderdoc/Widgets/Extended/RDTreeWidget.cpp index 9561ad116..a7159fcd0 100644 --- a/qrenderdoc/Widgets/Extended/RDTreeWidget.cpp +++ b/qrenderdoc/Widgets/Extended/RDTreeWidget.cpp @@ -767,7 +767,7 @@ void RDTreeWidget::mouseMoveEvent(QMouseEvent *e) { setCursor(QCursor(Qt::PointingHandCursor)); } - else if(m_delegate->linkHover(e, m_currentHoverIndex)) + else if(m_delegate->linkHover(e, font(), m_currentHoverIndex)) { m_model->itemChanged(m_model->itemForIndex(m_currentHoverIndex), {Qt::DecorationRole}); setCursor(QCursor(Qt::PointingHandCursor));