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

@@ -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<QVariant> 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<RichResourceText> 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<RichResourceTextPtr, QString>(&ResIdTextToString);
+  QMetaType::registerConverter<ResourceId, QString>(&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<ResourceId>() || re.match(var.toString()).hasMatch())
+  QRegularExpressionMatch match = re.match(text);
+
+  if(match.hasMatch())
   {
-    QString text;
-    if(var.userType() == qMetaTypeId<ResourceId>())
-      text = ToQStr(var.value<ResourceId>());
-    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<RichResourceTextPtr>();
+  return var.userType() == qMetaTypeId<RichResourceTextPtr>() ||
+         var.userType() == qMetaTypeId<ResourceId>();
 }
 
+// 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<ResourceId>())
+  {
+    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<ResourceId>();
+
+    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<RichResourceTextPtr>();
 
   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<ResourceId>())
+  {
+    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<ResourceId>());
+    else
+      name = ToQStr(var.value<ResourceId>());
+
+    const QPixmap &px = Pixmaps::link(owner->devicePixelRatio());
+
+    int ret = margin + metrics.boundingRect(name).width() + margin + px.width() + margin;
+    return ret;
+  }
+
   RichResourceTextPtr linkedText = var.value<RichResourceTextPtr>();
 
   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>())
+  {
+    ResourceId id = var.value<ResourceId>();
+
+    // 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<RichResourceTextPtr>();
 
   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<ResourceId>());
+          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);
     }
   }
 

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<RichResourceText> 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;

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

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));
   }

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));