Tidy up RDTreeView expansion handling

qrenderdoc/Widgets/Extended/RDTreeView.cpp

@@ -223,58 +223,32 @@ QAbstractItemDelegate *RDTreeView::itemDelegate() const
   return m_userDelegate;
 }
 
-void RDTreeView::saveInternalExpansion(uint key, int keyColumn, int role)
-{
-  RDTreeViewExpansionState &state = m_Expansions[key];
-
-  saveExpansionExternal(state, key, keyColumn, role);
-}
-
-bool RDTreeView::hasInternalExpansion(uint key)
-{
-  return m_Expansions.contains(key);
-}
-
-void RDTreeView::applyInternalExpansion(uint key, int keyColumn, int role)
-{
-  RDTreeViewExpansionState &state = m_Expansions[key];
-
-  applyExternalExpansion(state, key, keyColumn, role);
-}
-
 void RDTreeView::rowsAboutToBeRemoved(const QModelIndex &parent, int start, int end)
 {
   m_currentHoverIndex = QModelIndex();
 }
 
-void RDTreeView::clearInternalExpansions()
-{
-  m_Expansions.clear();
-}
-
-void RDTreeView::saveExpansionExternal(RDTreeViewExpansionState &state, uint key, int keyColumn,
-                                       int role)
+void RDTreeView::saveExpansion(RDTreeViewExpansionState &state, const ExpansionKeyGen &keygen)
 {
   state.clear();
 
   for(int i = 0; i < model()->rowCount(); i++)
-    saveExpansion(state, model()->index(i, keyColumn), key, keyColumn, role);
+    saveExpansionFromRow(state, model()->index(i, 0), 0, keygen);
 }
 
-void RDTreeView::applyExternalExpansion(const RDTreeViewExpansionState &state, uint key,
-                                        int keyColumn, int role)
+void RDTreeView::applyExpansion(const RDTreeViewExpansionState &state, const ExpansionKeyGen &keygen)
 {
   for(int i = 0; i < model()->rowCount(); i++)
-    applyExpansion(state, model()->index(i, keyColumn), key, keyColumn, role);
+    applyExpansionToRow(state, model()->index(i, 0), 0, keygen);
 }
 
-void RDTreeView::saveExpansion(RDTreeViewExpansionState &state, QModelIndex idx, uint seed,
-                               int keyColumn, int role)
+void RDTreeView::saveExpansionFromRow(RDTreeViewExpansionState &state, QModelIndex idx, uint seed,
+                                      const ExpansionKeyGen &keygen)
 {
   if(!idx.isValid())
     return;
 
-  uint key = qHash(model()->data(idx, role).toString(), seed);
+  uint key = keygen(idx, seed);
   if(isExpanded(idx))
   {
     state.insert(key);
@@ -283,24 +257,24 @@ void RDTreeView::saveExpansion(RDTreeViewExpansionState &state, QModelIndex idx,
     // branches. Technically we're losing information here but it allows us to skip a full expensive
     // search
     for(int i = 0; i < model()->rowCount(idx); i++)
-      saveExpansion(state, model()->index(i, keyColumn, idx), key, keyColumn, role);
+      saveExpansionFromRow(state, model()->index(i, 0, idx), seed, keygen);
   }
 }
 
-void RDTreeView::applyExpansion(const RDTreeViewExpansionState &state, QModelIndex idx, uint seed,
-                                int keyColumn, int role)
+void RDTreeView::applyExpansionToRow(const RDTreeViewExpansionState &state, QModelIndex idx,
+                                     uint seed, const ExpansionKeyGen &keygen)
 {
   if(!idx.isValid())
     return;
 
-  uint key = qHash(model()->data(idx, role).toString(), seed);
+  uint key = keygen(idx, seed);
   if(state.contains(key))
   {
     expand(idx);
 
     // same as above - only recurse when we have a parent that's expanded.
     for(int i = 0; i < model()->rowCount(idx); i++)
-      applyExpansion(state, model()->index(i, keyColumn, idx), key, keyColumn, role);
+      applyExpansionToRow(state, model()->index(i, 0, idx), seed, keygen);
   }
 }
 

qrenderdoc/Widgets/Extended/RDTreeView.h

@@ -67,6 +67,8 @@ protected:
   void sendListenerEvent(QMouseEvent *e);
 };
 
+typedef std::function<uint(QModelIndex, uint)> ExpansionKeyGen;
+
 class RDTreeView : public QTreeView
 {
   Q_OBJECT
@@ -88,39 +90,34 @@ public:
   void setItemDelegate(QAbstractItemDelegate *delegate);
   QAbstractItemDelegate *itemDelegate() const;
 
-  void saveInternalExpansion(uint key, int keyColumn, int role = Qt::DisplayRole);
-  bool hasInternalExpansion(uint key);
-  void applyInternalExpansion(uint key, int keyColumn, int role = Qt::DisplayRole);
-  void clearInternalExpansions();
+  // state is the storage to save the expansion state into
+  // keygen is a function that will take the index of a row and a previous hash, and return the hash
+  //   for that row.
+  void saveExpansion(RDTreeViewExpansionState &state, const ExpansionKeyGen &keygen);
+  void applyExpansion(const RDTreeViewExpansionState &state, const ExpansionKeyGen &keygen);
 
-  void saveExpansionExternal(RDTreeViewExpansionState &state, uint key, int keyColumn,
-                             int role = Qt::DisplayRole);
-  void applyExternalExpansion(const RDTreeViewExpansionState &state, uint key, int keyColumn,
-                              int role = Qt::DisplayRole);
-
-  // convenience overloads taking a string as a key that just hashes the string
-  void saveInternalExpansion(QString keystring, int keyColumn, int role = Qt::DisplayRole)
+  // convenience overloads for the simple case of using a single column's data as hash
+  void saveExpansion(RDTreeViewExpansionState &state, int keyColumn, int keyRole = Qt::DisplayRole)
   {
-    saveInternalExpansion(qHash(keystring), keyColumn, role);
+    saveExpansion(state, [keyColumn, keyRole](QModelIndex idx, uint seed) {
+      return qHash(idx.sibling(idx.row(), keyColumn).data(keyRole).toString(), seed);
+    });
+  }
+  void applyExpansion(const RDTreeViewExpansionState &state, int keyColumn,
+                      int keyRole = Qt::DisplayRole)
+  {
+    applyExpansion(state, [keyColumn, keyRole](QModelIndex idx, uint seed) {
+      return qHash(idx.sibling(idx.row(), keyColumn).data(keyRole).toString(), seed);
+    });
   }
 
-  bool hasInternalExpansion(QString keystring) { return hasInternalExpansion(qHash(keystring)); }
-  void applyInternalExpansion(QString keystring, int keyColumn, int role = Qt::DisplayRole)
+  // Internally tracked expansions
+  RDTreeViewExpansionState &getInternalExpansion(uint expansionID)
   {
-    applyInternalExpansion(qHash(keystring), keyColumn, role);
+    return m_Expansions[expansionID];
   }
-
-  void saveExpansionExternal(RDTreeViewExpansionState &state, QString keystring, int keyColumn,
-                             int role = Qt::DisplayRole)
-  {
-    saveExpansionExternal(state, qHash(keystring), keyColumn, role);
-  }
-  void applyExternalExpansion(const RDTreeViewExpansionState &state, QString keystring,
-                              int keyColumn, int role = Qt::DisplayRole)
-  {
-    applyExternalExpansion(state, qHash(keystring), keyColumn, role);
-  }
-
+  bool hasInternalExpansion(uint expansionID) { return m_Expansions.contains(expansionID); }
+  void clearInternalExpansions() { m_Expansions.clear(); }
 signals:
   void leave(QEvent *e);
   void keyPress(QKeyEvent *e);
@@ -148,10 +145,10 @@ private:
 
   QMap<uint, RDTreeViewExpansionState> m_Expansions;
 
-  void saveExpansion(RDTreeViewExpansionState &state, QModelIndex idx, uint seed, int keyColumn,
-                     int role);
-  void applyExpansion(const RDTreeViewExpansionState &state, QModelIndex idx, uint seed,
-                      int keyColumn, int role);
+  void saveExpansionFromRow(RDTreeViewExpansionState &state, QModelIndex idx, uint seed,
+                            const ExpansionKeyGen &keygen);
+  void applyExpansionToRow(const RDTreeViewExpansionState &state, QModelIndex idx, uint seed,
+                           const ExpansionKeyGen &keygen);
 
   QAbstractItemDelegate *m_userDelegate = NULL;
   RDTreeViewDelegate *m_delegate;

qrenderdoc/Windows/APIInspector.cpp

@@ -70,14 +70,14 @@ void APIInspector::OnCaptureClosed()
 
 void APIInspector::OnSelectedEventChanged(uint32_t eventId)
 {
-  ui->apiEvents->saveInternalExpansion(m_EventID, 0);
+  ui->apiEvents->saveExpansion(ui->apiEvents->getInternalExpansion(m_EventID), 0);
 
   ui->apiEvents->clearSelection();
 
   fillAPIView();
 
   m_EventID = eventId;
-  ui->apiEvents->applyInternalExpansion(m_EventID, 0);
+  ui->apiEvents->applyExpansion(ui->apiEvents->getInternalExpansion(m_EventID), 0);
 }
 
 void APIInspector::addCallstack(rdcarray<rdcstr> calls)

qrenderdoc/Windows/ConstantBufferPreviewer.cpp

@@ -112,15 +112,16 @@ void ConstantBufferPreviewer::OnEventChanged(uint32_t eventId)
 
   updateLabels();
 
-  if(m_formatOverride.empty())
-  {
-    // stage, slot, and array index are all invariant across a given ConstantBufferPreviewer
-    // instance. We only need to use the actual bound shader as a key.
-    ui->variables->saveInternalExpansion(ToQStr(prevShader), 0);
-  }
-
   if(reflection == NULL || m_slot >= reflection->constantBlocks.size())
   {
+    // save expansion before clearing
+    if(m_formatOverride.empty())
+    {
+      RDTreeViewExpansionState &prevShaderExpansionState =
+          ui->variables->getInternalExpansion(qHash(ToQStr(prevShader)));
+      ui->variables->saveExpansion(prevShaderExpansionState, 0);
+    }
+
     setVariables({});
     return;
   }
@@ -132,30 +133,30 @@ void ConstantBufferPreviewer::OnEventChanged(uint32_t eventId)
       rdcarray<ShaderVariable> vars = applyFormatOverride(data);
       GUIInvoke::call(this, [this, vars, wasEmpty] {
         RDTreeViewExpansionState state;
-        ui->variables->saveExpansionExternal(state, 0, 0);
+        ui->variables->saveExpansion(state, 0);
         setVariables(vars);
         if(wasEmpty)
         {
           for(int i = 0; i < 3; i++)
             ui->variables->resizeColumnToContents(i);
         }
-        ui->variables->applyExternalExpansion(state, 0, 0);
+        ui->variables->applyExpansion(state, 0);
       });
     });
   }
   else
   {
-    m_Ctx.Replay().AsyncInvoke([this, entryPoint, offs, wasEmpty](IReplayController *r) {
+    m_Ctx.Replay().AsyncInvoke([this, prevShader, entryPoint, offs, wasEmpty](IReplayController *r) {
       rdcarray<ShaderVariable> vars = r->GetCBufferVariableContents(
           m_shader, entryPoint.toUtf8().data(), m_slot, m_cbuffer, offs);
-      GUIInvoke::call(this, [this, vars, wasEmpty] {
+      GUIInvoke::call(this, [this, prevShader, vars, wasEmpty] {
 
-        // save this state to reapply if we don't already have an internal expansion for the new
-        // shader, since this means two shaders with the same or similar constants will preserve
-        // expansion across selections.
-        RDTreeViewExpansionState state;
-        if(!ui->variables->hasInternalExpansion(ToQStr(m_shader)))
-          ui->variables->saveExpansionExternal(state, 0, 0);
+        RDTreeViewExpansionState &prevShaderExpansionState =
+            ui->variables->getInternalExpansion(qHash(ToQStr(prevShader)));
+
+        // stage, slot, and array index are all invariant across a given ConstantBufferPreviewer
+        // instance. We only need to use the actual bound shader as a key.
+        ui->variables->saveExpansion(prevShaderExpansionState, 0);
 
         setVariables(vars);
         if(wasEmpty)
@@ -164,10 +165,14 @@ void ConstantBufferPreviewer::OnEventChanged(uint32_t eventId)
             ui->variables->resizeColumnToContents(i);
         }
 
-        if(ui->variables->hasInternalExpansion(ToQStr(m_shader)))
-          ui->variables->applyInternalExpansion(ToQStr(m_shader), 0);
+        // if we have saved expansion state for the new shader, apply it, otherwise apply the
+        // previous one to get any overlap (e.g. two different shaders with very similar or
+        // identical constants)
+        if(ui->variables->hasInternalExpansion(qHash(ToQStr(m_shader))))
+          ui->variables->applyExpansion(
+              ui->variables->getInternalExpansion(qHash(ToQStr(m_shader))), 0);
         else
-          ui->variables->applyExternalExpansion(state, 0, 0);
+          ui->variables->applyExpansion(prevShaderExpansionState, 0);
       });
     });
   }

qrenderdoc/Windows/ResourceInspector.cpp

@@ -202,7 +202,7 @@ void ResourceInspector::Inspect(ResourceId id)
     return;
 
   if(m_Resource != ResourceId())
-    ui->initChunks->saveInternalExpansion(ToQStr(m_Resource), 0);
+    ui->initChunks->saveExpansion(ui->initChunks->getInternalExpansion(qHash(ToQStr(m_Resource))), 0);
 
   m_Resource = id;
 
@@ -330,7 +330,8 @@ void ResourceInspector::Inspect(ResourceId id)
   ui->initChunks->setUpdatesEnabled(true);
 
   if(m_Resource != ResourceId())
-    ui->initChunks->applyInternalExpansion(ToQStr(m_Resource), 0);
+    ui->initChunks->applyExpansion(ui->initChunks->getInternalExpansion(qHash(ToQStr(m_Resource))),
+                                   0);
 }
 
 void ResourceInspector::OnCaptureLoaded()

qrenderdoc/Windows/ShaderViewer.cpp

@@ -1953,10 +1953,8 @@ void ShaderViewer::updateDebugging()
 
   if(m_Trace->hasLocals)
   {
-    const QString stateprefix = lit("!!@");
-
     RDTreeViewExpansionState expansion;
-    ui->locals->saveExpansionExternal(expansion, stateprefix, 0);
+    ui->locals->saveExpansion(expansion, 0);
 
     ui->locals->clear();
 
@@ -2097,7 +2095,7 @@ void ShaderViewer::updateDebugging()
     while(fakeroot.childCount() > 0)
       ui->locals->addTopLevelItem(fakeroot.takeChild(0));
 
-    ui->locals->applyExternalExpansion(expansion, stateprefix, 0);
+    ui->locals->applyExpansion(expansion, 0);
   }
 
   if(ui->registers->topLevelItemCount() == 0)