From 26ade98411c1d76540695f15378ff7f6b5388b1a Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Thu, 14 Aug 2014 19:21:55 +0200 Subject: [PATCH] Pica/citra-qt: Replace command list view and command list debugging code with something more sophisticated. --- src/citra_qt/debugger/graphics_cmdlists.cpp | 144 +++++++------------- src/citra_qt/debugger/graphics_cmdlists.hxx | 42 ++---- src/citra_qt/main.cpp | 4 +- src/core/hle/service/gsp.cpp | 5 - src/video_core/command_processor.cpp | 2 + src/video_core/debug_utils/debug_utils.cpp | 55 ++++++++ src/video_core/debug_utils/debug_utils.h | 21 +++ src/video_core/gpu_debugger.h | 63 --------- 8 files changed, 142 insertions(+), 194 deletions(-) diff --git a/src/citra_qt/debugger/graphics_cmdlists.cpp b/src/citra_qt/debugger/graphics_cmdlists.cpp index e98560a19..71dd166cd 100644 --- a/src/citra_qt/debugger/graphics_cmdlists.cpp +++ b/src/citra_qt/debugger/graphics_cmdlists.cpp @@ -2,53 +2,21 @@ // Licensed under GPLv2 // Refer to the license.txt file included. -#include "graphics_cmdlists.hxx" +#include +#include +#include #include -extern GraphicsDebugger g_debugger; +#include "graphics_cmdlists.hxx" -GPUCommandListModel::GPUCommandListModel(QObject* parent) : QAbstractItemModel(parent) +GPUCommandListModel::GPUCommandListModel(QObject* parent) : QAbstractListModel(parent) { - root_item = new TreeItem(TreeItem::ROOT, 0, NULL, this); - connect(this, SIGNAL(CommandListCalled()), this, SLOT(OnCommandListCalledInternal()), Qt::UniqueConnection); -} - -QModelIndex GPUCommandListModel::index(int row, int column, const QModelIndex& parent) const -{ - TreeItem* item; - - if (!parent.isValid()) { - item = root_item; - } else { - item = (TreeItem*)parent.internalPointer(); - } - - return createIndex(row, column, item->children[row]); -} - -QModelIndex GPUCommandListModel::parent(const QModelIndex& child) const -{ - if (!child.isValid()) - return QModelIndex(); - - TreeItem* item = (TreeItem*)child.internalPointer(); - - if (item->parent == NULL) - return QModelIndex(); - - return createIndex(item->parent->index, 0, item->parent); } int GPUCommandListModel::rowCount(const QModelIndex& parent) const { - TreeItem* item; - if (!parent.isValid()) { - item = root_item; - } else { - item = (TreeItem*)parent.internalPointer(); - } - return item->children.size(); + return pica_trace.writes.size(); } int GPUCommandListModel::columnCount(const QModelIndex& parent) const @@ -61,79 +29,67 @@ QVariant GPUCommandListModel::data(const QModelIndex& index, int role) const if (!index.isValid()) return QVariant(); - const TreeItem* item = (const TreeItem*)index.internalPointer(); + const auto& writes = pica_trace.writes; + const Pica::CommandProcessor::CommandHeader cmd{writes[index.row()].Id()}; + const u32 val{writes[index.row()].Value()}; - if (item->type == TreeItem::COMMAND_LIST) - { - const GraphicsDebugger::PicaCommandList& cmdlist = command_lists[item->index].second; - u32 address = command_lists[item->index].first; - - if (role == Qt::DisplayRole && index.column() == 0) - { - return QVariant(QString("0x%1 bytes at 0x%2").arg(cmdlist.size(), 0, 16).arg(address, 8, 16, QLatin1Char('0'))); + if (role == Qt::DisplayRole) { + QString content; + if (index.column() == 0) { + content = QString::fromLatin1(Pica::Regs::GetCommandName(cmd.cmd_id).c_str()); + content.append(" "); + } else if (index.column() == 1) { + content.append(QString("%1 ").arg(cmd.hex, 8, 16, QLatin1Char('0'))); + content.append(QString("%1 ").arg(val, 8, 16, QLatin1Char('0'))); } - } - else - { - // index refers to a specific command - const GraphicsDebugger::PicaCommandList& cmdlist = command_lists[item->parent->index].second; - const GraphicsDebugger::PicaCommand& cmd = cmdlist[item->index]; - const Pica::CommandProcessor::CommandHeader& header = cmd.GetHeader(); - if (role == Qt::DisplayRole) { - QString content; - if (index.column() == 0) { - content = QString::fromLatin1(Pica::Regs::GetCommandName(header.cmd_id).c_str()); - content.append(" "); - } else if (index.column() == 1) { - for (int j = 0; j < cmd.size(); ++j) - content.append(QString("%1 ").arg(cmd[j], 8, 16, QLatin1Char('0'))); - } - - return QVariant(content); - } + return QVariant(content); } return QVariant(); } -void GPUCommandListModel::OnCommandListCalled(const GraphicsDebugger::PicaCommandList& lst, bool is_new) -{ - emit CommandListCalled(); -} - - -void GPUCommandListModel::OnCommandListCalledInternal() +void GPUCommandListModel::OnPicaTraceFinished(const Pica::DebugUtils::PicaTrace& trace) { beginResetModel(); - command_lists = GetDebugger()->GetCommandLists(); - - // delete root item and rebuild tree - delete root_item; - root_item = new TreeItem(TreeItem::ROOT, 0, NULL, this); - - for (int command_list_idx = 0; command_list_idx < command_lists.size(); ++command_list_idx) { - TreeItem* command_list_item = new TreeItem(TreeItem::COMMAND_LIST, command_list_idx, root_item, root_item); - root_item->children.push_back(command_list_item); - - const GraphicsDebugger::PicaCommandList& command_list = command_lists[command_list_idx].second; - for (int command_idx = 0; command_idx < command_list.size(); ++command_idx) { - TreeItem* command_item = new TreeItem(TreeItem::COMMAND, command_idx, command_list_item, command_list_item); - command_list_item->children.push_back(command_item); - } - } + pica_trace = trace; endResetModel(); } + GPUCommandListWidget::GPUCommandListWidget(QWidget* parent) : QDockWidget(tr("Pica Command List"), parent) { GPUCommandListModel* model = new GPUCommandListModel(this); - g_debugger.RegisterObserver(model); - QTreeView* tree_widget = new QTreeView; - tree_widget->setModel(model); - tree_widget->setFont(QFont("monospace")); - setWidget(tree_widget); + QWidget* main_widget = new QWidget; + + QTreeView* list_widget = new QTreeView; + list_widget->setModel(model); + list_widget->setFont(QFont("monospace")); + list_widget->setRootIsDecorated(false); + + QPushButton* toggle_tracing = new QPushButton(tr("Start Tracing")); + + connect(toggle_tracing, SIGNAL(clicked()), this, SLOT(OnToggleTracing())); + connect(this, SIGNAL(TracingFinished(const Pica::DebugUtils::PicaTrace&)), + model, SLOT(OnPicaTraceFinished(const Pica::DebugUtils::PicaTrace&))); + + QVBoxLayout* main_layout = new QVBoxLayout; + main_layout->addWidget(list_widget); + main_layout->addWidget(toggle_tracing); + main_widget->setLayout(main_layout); + + setWidget(main_widget); +} + +void GPUCommandListWidget::OnToggleTracing() +{ + if (!Pica::DebugUtils::IsPicaTracing()) { + Pica::DebugUtils::StartPicaTracing(); + } else { + pica_trace = Pica::DebugUtils::FinishPicaTracing(); + emit TracingFinished(*pica_trace); + } } diff --git a/src/citra_qt/debugger/graphics_cmdlists.hxx b/src/citra_qt/debugger/graphics_cmdlists.hxx index b4e6e3c8a..479ef0326 100644 --- a/src/citra_qt/debugger/graphics_cmdlists.hxx +++ b/src/citra_qt/debugger/graphics_cmdlists.hxx @@ -4,53 +4,28 @@ #pragma once -#include +#include #include #include "video_core/gpu_debugger.h" +#include "video_core/debug_utils/debug_utils.h" -// TODO: Rename class, since it's not actually a list model anymore... -class GPUCommandListModel : public QAbstractItemModel, public GraphicsDebugger::DebuggerObserver +class GPUCommandListModel : public QAbstractListModel { Q_OBJECT public: GPUCommandListModel(QObject* parent); - QModelIndex index(int row, int column, const QModelIndex& parent = QModelIndex()) const; - QModelIndex parent(const QModelIndex& child) const; int columnCount(const QModelIndex& parent = QModelIndex()) const; int rowCount(const QModelIndex& parent = QModelIndex()) const override; QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; -public: - void OnCommandListCalled(const GraphicsDebugger::PicaCommandList& lst, bool is_new) override; - public slots: - void OnCommandListCalledInternal(); - -signals: - void CommandListCalled(); + void OnPicaTraceFinished(const Pica::DebugUtils::PicaTrace& trace); private: - struct TreeItem : public QObject - { - enum Type { - ROOT, - COMMAND_LIST, - COMMAND - }; - - TreeItem(Type type, int index, TreeItem* item_parent, QObject* parent) : QObject(parent), type(type), index(index), parent(item_parent) {} - - Type type; - int index; - std::vector children; - TreeItem* parent; - }; - - std::vector> command_lists; - TreeItem* root_item; + Pica::DebugUtils::PicaTrace pica_trace; }; class GPUCommandListWidget : public QDockWidget @@ -60,5 +35,12 @@ class GPUCommandListWidget : public QDockWidget public: GPUCommandListWidget(QWidget* parent = 0); +public slots: + void OnToggleTracing(); + +signals: + void TracingFinished(const Pica::DebugUtils::PicaTrace&); + private: + std::unique_ptr pica_trace; }; diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index 9a16cf92d..a6b87f781 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -52,11 +52,11 @@ GMainWindow::GMainWindow() graphicsWidget = new GPUCommandStreamWidget(this); addDockWidget(Qt::RightDockWidgetArea, graphicsWidget); - callstackWidget->hide(); + graphicsWidget ->hide(); graphicsCommandsWidget = new GPUCommandListWidget(this); addDockWidget(Qt::RightDockWidgetArea, graphicsCommandsWidget); - callstackWidget->hide(); + graphicsCommandsWidget->hide(); QMenu* debug_menu = ui.menu_View->addMenu(tr("Debugging")); debug_menu->addAction(disasmWidget->toggleViewAction()); diff --git a/src/core/hle/service/gsp.cpp b/src/core/hle/service/gsp.cpp index 027ba5a37..46c5a8ddd 100644 --- a/src/core/hle/service/gsp.cpp +++ b/src/core/hle/service/gsp.cpp @@ -230,11 +230,6 @@ void ExecuteCommand(const Command& command, u32 thread_id) { // TODO: Not sure if we are supposed to always write this .. seems to trigger processing though WriteGPURegister(GPU_REG_INDEX(command_processor_config.trigger), 1); - // TODO: Move this to GPU - // TODO: Not sure what units the size is measured in - g_debugger.CommandListCalled(params.address, - (u32*)Memory::GetPointer(params.address), - params.size); SignalInterrupt(InterruptId::P3D); break; } diff --git a/src/video_core/command_processor.cpp b/src/video_core/command_processor.cpp index f7a412bc1..76fdb4e4a 100644 --- a/src/video_core/command_processor.cpp +++ b/src/video_core/command_processor.cpp @@ -33,6 +33,8 @@ static inline void WritePicaReg(u32 id, u32 value, u32 mask) { u32 old_value = registers[id]; registers[id] = (old_value & ~mask) | (value & mask); + DebugUtils::OnPicaRegWrite(id, registers[id]); + switch(id) { // It seems like these trigger vertex rendering case PICA_REG_INDEX(trigger_draw): diff --git a/src/video_core/debug_utils/debug_utils.cpp b/src/video_core/debug_utils/debug_utils.cpp index f41249eac..1bbc0330c 100644 --- a/src/video_core/debug_utils/debug_utils.cpp +++ b/src/video_core/debug_utils/debug_utils.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include "video_core/pica.h" @@ -260,6 +261,60 @@ void DumpShader(const u32* binary_data, u32 binary_size, const u32* swizzle_data } } +static std::unique_ptr pica_trace; +static std::mutex pica_trace_mutex; +static int is_pica_tracing = false; + +void StartPicaTracing() +{ + if (is_pica_tracing) { + ERROR_LOG(GPU, "StartPicaTracing called even though tracing already running!"); + return; + } + + pica_trace_mutex.lock(); + pica_trace = std::unique_ptr(new PicaTrace); + + is_pica_tracing = true; + pica_trace_mutex.unlock(); +} + +bool IsPicaTracing() +{ + return is_pica_tracing; +} + +void OnPicaRegWrite(u32 id, u32 value) +{ + // Double check for is_pica_tracing to avoid pointless locking overhead + if (!is_pica_tracing) + return; + + std::unique_lock lock(pica_trace_mutex); + + if (!is_pica_tracing) + return; + + pica_trace->writes.push_back({id, value}); +} + +std::unique_ptr FinishPicaTracing() +{ + if (!is_pica_tracing) { + ERROR_LOG(GPU, "FinishPicaTracing called even though tracing already running!"); + return {}; + } + + // signalize that no further tracing should be performed + is_pica_tracing = false; + + // Wait until running tracing is finished + pica_trace_mutex.lock(); + std::unique_ptr ret(std::move(pica_trace)); + pica_trace_mutex.unlock(); + return std::move(ret); +} + } // namespace } // namespace diff --git a/src/video_core/debug_utils/debug_utils.h b/src/video_core/debug_utils/debug_utils.h index bd7a0a89b..023500066 100644 --- a/src/video_core/debug_utils/debug_utils.h +++ b/src/video_core/debug_utils/debug_utils.h @@ -5,6 +5,7 @@ #pragma once #include +#include #include #include "video_core/pica.h" @@ -38,6 +39,26 @@ private: void DumpShader(const u32* binary_data, u32 binary_size, const u32* swizzle_data, u32 swizzle_size, u32 main_offset, const Regs::VSOutputAttributes* output_attributes); + +// Utility class to log Pica commands. +struct PicaTrace { + struct Write : public std::pair { + Write(u32 id, u32 value) : std::pair(id, value) {} + + u32& Id() { return first; } + const u32& Id() const { return first; } + + u32& Value() { return second; } + const u32& Value() const { return second; } + }; + std::vector writes; +}; + +void StartPicaTracing(); +bool IsPicaTracing(); +void OnPicaRegWrite(u32 id, u32 value); +std::unique_ptr FinishPicaTracing(); + } // namespace } // namespace diff --git a/src/video_core/gpu_debugger.h b/src/video_core/gpu_debugger.h index 2ba873457..5a81fcfcb 100644 --- a/src/video_core/gpu_debugger.h +++ b/src/video_core/gpu_debugger.h @@ -18,19 +18,6 @@ class GraphicsDebugger { public: - // A few utility structs used to expose data - // A vector of commands represented by their raw byte sequence - struct PicaCommand : public std::vector - { - const Pica::CommandProcessor::CommandHeader& GetHeader() const - { - const u32& val = at(1); - return *(Pica::CommandProcessor::CommandHeader*)&val; - } - }; - - typedef std::vector PicaCommandList; - // Base class for all objects which need to be notified about GPU events class DebuggerObserver { @@ -55,16 +42,6 @@ public: ERROR_LOG(GSP, "Received command: id=%x", (int)cmd.id.Value()); } - /** - * @param lst command list which triggered this call - * @param is_new true if the command list was called for the first time - * @todo figure out how to make sure called functions don't keep references around beyond their life time - */ - virtual void OnCommandListCalled(const PicaCommandList& lst, bool is_new) - { - ERROR_LOG(GSP, "Command list called: %d", (int)is_new); - } - protected: const GraphicsDebugger* GetDebugger() const { @@ -93,49 +70,12 @@ public: } ); } - void CommandListCalled(u32 address, u32* command_list, u32 size_in_words) - { - if (observers.empty()) - return; - - PicaCommandList cmdlist; - for (u32* parse_pointer = command_list; parse_pointer < command_list + size_in_words;) - { - const Pica::CommandProcessor::CommandHeader& header = *(Pica::CommandProcessor::CommandHeader*)(&parse_pointer[1]); - - cmdlist.push_back(PicaCommand()); - auto& cmd = cmdlist.back(); - - size_t size = 2 + header.extra_data_length; - size = (size + 1) / 2 * 2; // align to 8 bytes - cmd.reserve(size); - std::copy(parse_pointer, parse_pointer + size, std::back_inserter(cmd)); - - parse_pointer += size; - } - - auto obj = std::pair(address, cmdlist); - auto it = std::find(command_lists.begin(), command_lists.end(), obj); - bool is_new = (it == command_lists.end()); - if (is_new) - command_lists.push_back(obj); - - ForEachObserver([&](DebuggerObserver* observer) { - observer->OnCommandListCalled(obj.second, is_new); - } ); - } - const GSP_GPU::Command& ReadGXCommandHistory(int index) const { // TODO: Is this thread-safe? return gx_command_history[index]; } - const std::vector>& GetCommandLists() const - { - return command_lists; - } - void RegisterObserver(DebuggerObserver* observer) { // TODO: Check for duplicates @@ -158,7 +98,4 @@ private: std::vector observers; std::vector gx_command_history; - - // vector of pairs of command lists and their storage address - std::vector> command_lists; };