From 9431ee330aab4d1bb622bf9250a0cc57cfe99982 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Tue, 25 Aug 2015 04:52:37 -0300 Subject: [PATCH 01/12] Shader Disassembly: Cleanup code and improve output alignment --- .../debugger/graphics_vertex_shader.cpp | 145 ++++++++++-------- 1 file changed, 79 insertions(+), 66 deletions(-) diff --git a/src/citra_qt/debugger/graphics_vertex_shader.cpp b/src/citra_qt/debugger/graphics_vertex_shader.cpp index 1d9a00e89..eaa749f30 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.cpp +++ b/src/citra_qt/debugger/graphics_vertex_shader.cpp @@ -65,6 +65,19 @@ QVariant GraphicsVertexShaderModel::headerData(int section, Qt::Orientation orie return QVariant(); } +// e.g. "-c92[a0.x].xyzw" +static void print_input(std::ostringstream& output, const SourceRegister& input, + bool negate, const std::string& swizzle_mask, bool align = true, + const std::string& address_register_name = std::string()) { + if (align) + output << std::setw(4) << std::right; + output << ((negate ? "-" : "") + input.GetName()); + + if (!address_register_name.empty()) + output << '[' << address_register_name << ']'; + output << '.' << swizzle_mask; +}; + QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) const { switch (role) { case Qt::DisplayRole: @@ -81,43 +94,34 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con case 2: { - std::stringstream output; - output.flags(std::ios::hex); + std::ostringstream output; + output.flags(std::ios::hex | std::ios::uppercase); + + // To make the code aligning columns of assembly easier to keep track of, this function + // keeps track of the start of the start of the previous column, allowing alignment + // based on desired field widths. + int current_column = 0; + auto AlignToColumn = [&](int col_width) { + // Prints spaces to the output to pad previous column to size and advances the + // column marker. + current_column += col_width; + int to_add = std::max(1, current_column - (int)output.tellp()); + for (int i = 0; i < to_add; ++i) { + output << ' '; + } + }; Instruction instr = par->info.code[index.row()]; const SwizzlePattern& swizzle = par->info.swizzle_info[instr.common.operand_desc_id].pattern; // longest known instruction name: "setemit " - output << std::setw(8) << std::left << instr.opcode.Value().GetInfo().name; + int kOpcodeColumnWidth = 8; + // "rXX.xyzw " + int kOutputColumnWidth = 10; + // "-rXX.xyzw ", no attempt is made to align indexed inputs + int kInputOperandColumnWidth = 11; - // e.g. "-c92.xyzw" - static auto print_input = [](std::stringstream& output, const SourceRegister& input, - bool negate, const std::string& swizzle_mask) { - output << std::setw(4) << std::right << (negate ? "-" : "") + input.GetName(); - output << "." << swizzle_mask; - }; - - // e.g. "-c92[a0.x].xyzw" - static auto print_input_indexed = [](std::stringstream& output, const SourceRegister& input, - bool negate, const std::string& swizzle_mask, - const std::string& address_register_name) { - std::string relative_address; - if (!address_register_name.empty()) - relative_address = "[" + address_register_name + "]"; - - output << std::setw(10) << std::right << (negate ? "-" : "") + input.GetName() + relative_address; - output << "." << swizzle_mask; - }; - - // Use print_input or print_input_indexed depending on whether relative addressing is used or not. - static auto print_input_indexed_compact = [](std::stringstream& output, const SourceRegister& input, - bool negate, const std::string& swizzle_mask, - const std::string& address_register_name) { - if (address_register_name.empty()) - print_input(output, input, negate, swizzle_mask); - else - print_input_indexed(output, input, negate, swizzle_mask, address_register_name); - }; + output << instr.opcode.Value().GetInfo().name; switch (instr.opcode.Value().GetInfo().type) { case OpCode::Type::Trivial: @@ -130,53 +134,54 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con switch (instr.opcode.Value().EffectiveOpCode()) { case OpCode::Id::CMP: { + AlignToColumn(kOpcodeColumnWidth); + // NOTE: CMP always writes both cc components, so we do not consider the dest mask here. - output << std::setw(4) << std::right << "cc."; - output << "xy "; + output << " cc.xy"; + AlignToColumn(kOutputColumnWidth); SourceRegister src1 = instr.common.GetSrc1(false); SourceRegister src2 = instr.common.GetSrc2(false); - print_input_indexed_compact(output, src1, swizzle.negate_src1, swizzle.SelectorToString(false).substr(0,1), instr.common.AddressRegisterName()); - output << " " << instr.common.compare_op.ToString(instr.common.compare_op.x) << " "; - print_input(output, src2, swizzle.negate_src2, swizzle.SelectorToString(true).substr(0,1)); + output << ' '; + print_input(output, src1, swizzle.negate_src1, swizzle.SelectorToString(false).substr(0,1), false, instr.common.AddressRegisterName()); + output << ' ' << instr.common.compare_op.ToString(instr.common.compare_op.x) << ' '; + print_input(output, src2, swizzle.negate_src2, swizzle.SelectorToString(true).substr(0,1), false); output << ", "; - print_input_indexed_compact(output, src1, swizzle.negate_src1, swizzle.SelectorToString(false).substr(1,1), instr.common.AddressRegisterName()); - output << " " << instr.common.compare_op.ToString(instr.common.compare_op.y) << " "; - print_input(output, src2, swizzle.negate_src2, swizzle.SelectorToString(true).substr(1,1)); + print_input(output, src1, swizzle.negate_src1, swizzle.SelectorToString(false).substr(1,1), false, instr.common.AddressRegisterName()); + output << ' ' << instr.common.compare_op.ToString(instr.common.compare_op.y) << ' '; + print_input(output, src2, swizzle.negate_src2, swizzle.SelectorToString(true).substr(1,1), false); break; } default: { + AlignToColumn(kOpcodeColumnWidth); + bool src_is_inverted = 0 != (instr.opcode.Value().GetInfo().subtype & OpCode::Info::SrcInversed); if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::Dest) { // e.g. "r12.xy__" - output << std::setw(4) << std::right << instr.common.dest.Value().GetName() + "."; - output << swizzle.DestMaskToString(); + output << std::setw(3) << std::right << instr.common.dest.Value().GetName() << '.' << swizzle.DestMaskToString(); } else if (instr.opcode.Value().GetInfo().subtype == OpCode::Info::MOVA) { - output << std::setw(4) << std::right << "a0."; - output << swizzle.DestMaskToString(); - } else { - output << " "; + output << " a0." << swizzle.DestMaskToString(); } - output << " "; + AlignToColumn(kOutputColumnWidth); if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::Src1) { SourceRegister src1 = instr.common.GetSrc1(src_is_inverted); - print_input_indexed(output, src1, swizzle.negate_src1, swizzle.SelectorToString(false), instr.common.AddressRegisterName()); - } else { - output << " "; + print_input(output, src1, swizzle.negate_src1, swizzle.SelectorToString(false), true, instr.common.AddressRegisterName()); + AlignToColumn(kInputOperandColumnWidth); } // TODO: In some cases, the Address Register is used as an index for SRC2 instead of SRC1 if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::Src2) { SourceRegister src2 = instr.common.GetSrc2(src_is_inverted); print_input(output, src2, swizzle.negate_src2, swizzle.SelectorToString(true)); + AlignToColumn(kInputOperandColumnWidth); } break; } @@ -187,45 +192,53 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con case OpCode::Type::Conditional: { + output << ' '; + switch (instr.opcode.Value().EffectiveOpCode()) { case OpCode::Id::LOOP: output << "(unknown instruction format)"; break; default: - output << "if "; - if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::HasCondition) { - const char* ops[] = { - " || ", " && ", "", "" - }; - if (instr.flow_control.op != instr.flow_control.JustY) - output << ((!instr.flow_control.refx) ? "!" : " ") << "cc.x"; + output << '('; - output << ops[instr.flow_control.op]; + if (instr.flow_control.op != instr.flow_control.JustY) { + if (instr.flow_control.refx) output << '!'; + output << "cc.x"; + } - if (instr.flow_control.op != instr.flow_control.JustX) - output << ((!instr.flow_control.refy) ? "!" : " ") << "cc.y"; + if (instr.flow_control.op == instr.flow_control.Or) { + output << " || "; + } else if (instr.flow_control.op == instr.flow_control.And) { + output << " && "; + } - output << " "; + if (instr.flow_control.op != instr.flow_control.JustX) { + if (instr.flow_control.refy) output << '!'; + output << "cc.y"; + } + + output << ") "; } else if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::HasUniformIndex) { - output << "b" << instr.flow_control.bool_uniform_id << " "; + output << 'b' << instr.flow_control.bool_uniform_id << ' '; } u32 target_addr = instr.flow_control.dest_offset; u32 target_addr_else = instr.flow_control.dest_offset; if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::HasAlternative) { - output << "else jump to 0x" << std::setw(4) << std::right << std::setfill('0') << 4 * instr.flow_control.dest_offset << " "; + output << "else jump to 0x" << std::setw(4) << std::right << std::setfill('0') << (4 * instr.flow_control.dest_offset); } else if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::HasExplicitDest) { - output << "jump to 0x" << std::setw(4) << std::right << std::setfill('0') << 4 * instr.flow_control.dest_offset << " "; + output << "jump to 0x" << std::setw(4) << std::right << std::setfill('0') << (4 * instr.flow_control.dest_offset); } else { // TODO: Handle other cases + output << "(unknown destination)"; } if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::HasFinishPoint) { - output << "(return on " << std::setw(4) << std::right << std::setfill('0') - << 4 * instr.flow_control.dest_offset + 4 * instr.flow_control.num_instructions << ")"; + output << " (return on " << std::setw(4) << std::right << std::setfill('0') + << (4 * instr.flow_control.dest_offset + 4 * instr.flow_control.num_instructions) << ')'; } break; @@ -234,7 +247,7 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con } default: - output << "(unknown instruction format)"; + output << " (unknown instruction format)"; break; } From ecbad494d9f1e39b0e11fb58a38e868d2390c5c6 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Tue, 25 Aug 2015 05:44:59 -0300 Subject: [PATCH 02/12] Shader Debugger: Initialize input_vertex to prevent crashes If the first type of breakpoint to be hit wasn't "Vertex Loaded", the input_vertex would contain garbage, which would be passed to the shader interpreter and ocasionally cause crashes. --- src/citra_qt/debugger/graphics_vertex_shader.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/citra_qt/debugger/graphics_vertex_shader.cpp b/src/citra_qt/debugger/graphics_vertex_shader.cpp index eaa749f30..e5af76074 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.cpp +++ b/src/citra_qt/debugger/graphics_vertex_shader.cpp @@ -301,6 +301,13 @@ GraphicsVertexShaderWidget::GraphicsVertexShaderWidget(std::shared_ptr< Pica::De : BreakPointObserverDock(debug_context, "Pica Vertex Shader", parent) { setObjectName("PicaVertexShader"); + // Clear input vertex data so that it contains valid float values in case a debug shader + // execution happens before the first Vertex Loaded breakpoint. + // TODO: This makes a crash in the interpreter much less likely, but not impossible. The + // interpreter should guard against out-of-bounds accesses to ensure crashes in it aren't + // possible. + std::memset(&input_vertex, 0, sizeof(input_vertex)); + auto input_data_mapper = new QSignalMapper(this); // TODO: Support inputting data in hexadecimal raw format From 86d5461bcd0890ec8c039249424f26af636982d8 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Wed, 19 Aug 2015 04:39:24 -0300 Subject: [PATCH 03/12] Shader Disassembly: Introduce variables to hold common subexpressions --- .../debugger/graphics_vertex_shader.cpp | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/citra_qt/debugger/graphics_vertex_shader.cpp b/src/citra_qt/debugger/graphics_vertex_shader.cpp index e5af76074..ce81e7fae 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.cpp +++ b/src/citra_qt/debugger/graphics_vertex_shader.cpp @@ -111,8 +111,12 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con } }; - Instruction instr = par->info.code[index.row()]; - const SwizzlePattern& swizzle = par->info.swizzle_info[instr.common.operand_desc_id].pattern; + const Instruction instr = par->info.code[index.row()]; + const OpCode opcode = instr.opcode; + const OpCode::Info opcode_info = opcode.GetInfo(); + const u32 operand_desc_id = opcode_info.type == OpCode::Type::MultiplyAdd ? + instr.mad.operand_desc_id.Value() : instr.common.operand_desc_id.Value(); + const SwizzlePattern swizzle = par->info.swizzle_info[operand_desc_id].pattern; // longest known instruction name: "setemit " int kOpcodeColumnWidth = 8; @@ -121,9 +125,9 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con // "-rXX.xyzw ", no attempt is made to align indexed inputs int kInputOperandColumnWidth = 11; - output << instr.opcode.Value().GetInfo().name; + output << opcode_info.name; - switch (instr.opcode.Value().GetInfo().type) { + switch (opcode_info.type) { case OpCode::Type::Trivial: // Nothing to do here break; @@ -131,7 +135,7 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con case OpCode::Type::Arithmetic: { // Use custom code for special instructions - switch (instr.opcode.Value().EffectiveOpCode()) { + switch (opcode.EffectiveOpCode()) { case OpCode::Id::CMP: { AlignToColumn(kOpcodeColumnWidth); @@ -161,24 +165,24 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con { AlignToColumn(kOpcodeColumnWidth); - bool src_is_inverted = 0 != (instr.opcode.Value().GetInfo().subtype & OpCode::Info::SrcInversed); + bool src_is_inverted = 0 != (opcode_info.subtype & OpCode::Info::SrcInversed); - if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::Dest) { + if (opcode_info.subtype & OpCode::Info::Dest) { // e.g. "r12.xy__" output << std::setw(3) << std::right << instr.common.dest.Value().GetName() << '.' << swizzle.DestMaskToString(); - } else if (instr.opcode.Value().GetInfo().subtype == OpCode::Info::MOVA) { + } else if (opcode_info.subtype == OpCode::Info::MOVA) { output << " a0." << swizzle.DestMaskToString(); } AlignToColumn(kOutputColumnWidth); - if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::Src1) { + if (opcode_info.subtype & OpCode::Info::Src1) { SourceRegister src1 = instr.common.GetSrc1(src_is_inverted); print_input(output, src1, swizzle.negate_src1, swizzle.SelectorToString(false), true, instr.common.AddressRegisterName()); AlignToColumn(kInputOperandColumnWidth); } // TODO: In some cases, the Address Register is used as an index for SRC2 instead of SRC1 - if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::Src2) { + if (opcode_info.subtype & OpCode::Info::Src2) { SourceRegister src2 = instr.common.GetSrc2(src_is_inverted); print_input(output, src2, swizzle.negate_src2, swizzle.SelectorToString(true)); AlignToColumn(kInputOperandColumnWidth); @@ -194,13 +198,13 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con { output << ' '; - switch (instr.opcode.Value().EffectiveOpCode()) { + switch (opcode.EffectiveOpCode()) { case OpCode::Id::LOOP: output << "(unknown instruction format)"; break; default: - if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::HasCondition) { + if (opcode_info.subtype & OpCode::Info::HasCondition) { output << '('; if (instr.flow_control.op != instr.flow_control.JustY) { @@ -220,23 +224,23 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con } output << ") "; - } else if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::HasUniformIndex) { + } else if (opcode_info.subtype & OpCode::Info::HasUniformIndex) { output << 'b' << instr.flow_control.bool_uniform_id << ' '; } u32 target_addr = instr.flow_control.dest_offset; u32 target_addr_else = instr.flow_control.dest_offset; - if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::HasAlternative) { + if (opcode_info.subtype & OpCode::Info::HasAlternative) { output << "else jump to 0x" << std::setw(4) << std::right << std::setfill('0') << (4 * instr.flow_control.dest_offset); - } else if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::HasExplicitDest) { + } else if (opcode_info.subtype & OpCode::Info::HasExplicitDest) { output << "jump to 0x" << std::setw(4) << std::right << std::setfill('0') << (4 * instr.flow_control.dest_offset); } else { // TODO: Handle other cases output << "(unknown destination)"; } - if (instr.opcode.Value().GetInfo().subtype & OpCode::Info::HasFinishPoint) { + if (opcode_info.subtype & OpCode::Info::HasFinishPoint) { output << " (return on " << std::setw(4) << std::right << std::setfill('0') << (4 * instr.flow_control.dest_offset + 4 * instr.flow_control.num_instructions) << ')'; } From bc3f57efd0cf078c055cbf86623e6962f5f2e9a1 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Wed, 19 Aug 2015 04:39:58 -0300 Subject: [PATCH 04/12] Shader Disassembly: Implement support for MAD/MADI --- .../debugger/graphics_vertex_shader.cpp | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/citra_qt/debugger/graphics_vertex_shader.cpp b/src/citra_qt/debugger/graphics_vertex_shader.cpp index ce81e7fae..e0a78f657 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.cpp +++ b/src/citra_qt/debugger/graphics_vertex_shader.cpp @@ -65,6 +65,15 @@ QVariant GraphicsVertexShaderModel::headerData(int section, Qt::Orientation orie return QVariant(); } +static std::string SelectorToString(u32 selector) { + std::string ret; + for (int i = 0; i < 4; ++i) { + int component = (selector >> ((3 - i) * 2)) & 3; + ret += "xyzw"[component]; + } + return ret; +} + // e.g. "-c92[a0.x].xyzw" static void print_input(std::ostringstream& output, const SourceRegister& input, bool negate, const std::string& swizzle_mask, bool align = true, @@ -133,6 +142,7 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con break; case OpCode::Type::Arithmetic: + case OpCode::Type::MultiplyAdd: { // Use custom code for special instructions switch (opcode.EffectiveOpCode()) { @@ -161,6 +171,27 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con break; } + case OpCode::Id::MAD: + case OpCode::Id::MADI: + { + AlignToColumn(kOpcodeColumnWidth); + + bool src_is_inverted = 0 != (opcode_info.subtype & OpCode::Info::SrcInversed); + SourceRegister src1 = instr.mad.GetSrc1(src_is_inverted); + SourceRegister src2 = instr.mad.GetSrc2(src_is_inverted); + SourceRegister src3 = instr.mad.GetSrc3(src_is_inverted); + + output << std::setw(3) << std::right << instr.mad.dest.Value().GetName() << '.' << swizzle.DestMaskToString(); + AlignToColumn(kOutputColumnWidth); + print_input(output, src1, swizzle.negate_src1, SelectorToString(swizzle.src1_selector)); + AlignToColumn(kInputOperandColumnWidth); + print_input(output, src2, swizzle.negate_src2, SelectorToString(swizzle.src2_selector)); + AlignToColumn(kInputOperandColumnWidth); + print_input(output, src3, swizzle.negate_src3, SelectorToString(swizzle.src3_selector)); + AlignToColumn(kInputOperandColumnWidth); + break; + } + default: { AlignToColumn(kOpcodeColumnWidth); From 3194f40e96fcef87a2aeab95e52596d1bf3d973c Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Wed, 19 Aug 2015 04:47:45 -0300 Subject: [PATCH 05/12] Shader Disassembly: Fix disassembly of IFU/CALLU instructions --- src/citra_qt/debugger/graphics_vertex_shader.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/citra_qt/debugger/graphics_vertex_shader.cpp b/src/citra_qt/debugger/graphics_vertex_shader.cpp index e0a78f657..131b13952 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.cpp +++ b/src/citra_qt/debugger/graphics_vertex_shader.cpp @@ -226,6 +226,7 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con } case OpCode::Type::Conditional: + case OpCode::Type::UniformFlowControl: { output << ' '; From 8540e021768c8c148334b757cffdfc6db73ba54e Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Wed, 19 Aug 2015 04:52:44 -0300 Subject: [PATCH 06/12] Shader Disassembly: Fix printing of jump offsets --- src/citra_qt/debugger/graphics_vertex_shader.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/citra_qt/debugger/graphics_vertex_shader.cpp b/src/citra_qt/debugger/graphics_vertex_shader.cpp index 131b13952..59b63cd54 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.cpp +++ b/src/citra_qt/debugger/graphics_vertex_shader.cpp @@ -104,7 +104,7 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con case 2: { std::ostringstream output; - output.flags(std::ios::hex | std::ios::uppercase); + output.flags(std::ios::uppercase); // To make the code aligning columns of assembly easier to keep track of, this function // keeps track of the start of the start of the previous column, allowing alignment @@ -264,16 +264,16 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con u32 target_addr_else = instr.flow_control.dest_offset; if (opcode_info.subtype & OpCode::Info::HasAlternative) { - output << "else jump to 0x" << std::setw(4) << std::right << std::setfill('0') << (4 * instr.flow_control.dest_offset); + output << "else jump to 0x" << std::setw(4) << std::right << std::setfill('0') << std::hex << (4 * instr.flow_control.dest_offset); } else if (opcode_info.subtype & OpCode::Info::HasExplicitDest) { - output << "jump to 0x" << std::setw(4) << std::right << std::setfill('0') << (4 * instr.flow_control.dest_offset); + output << "jump to 0x" << std::setw(4) << std::right << std::setfill('0') << std::hex << (4 * instr.flow_control.dest_offset); } else { // TODO: Handle other cases output << "(unknown destination)"; } if (opcode_info.subtype & OpCode::Info::HasFinishPoint) { - output << " (return on " << std::setw(4) << std::right << std::setfill('0') + output << " (return on 0x" << std::setw(4) << std::right << std::setfill('0') << std::hex << (4 * instr.flow_control.dest_offset + 4 * instr.flow_control.num_instructions) << ')'; } From 2d195ba64e084ac382fb4ae463b724d4d8a1724f Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Wed, 19 Aug 2015 05:48:22 -0300 Subject: [PATCH 07/12] Shader Debugger: Improve space efficiency of the layout --- .../debugger/graphics_vertex_shader.cpp | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/citra_qt/debugger/graphics_vertex_shader.cpp b/src/citra_qt/debugger/graphics_vertex_shader.cpp index 59b63cd54..32c48cb6e 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.cpp +++ b/src/citra_qt/debugger/graphics_vertex_shader.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -395,6 +396,9 @@ GraphicsVertexShaderWidget::GraphicsVertexShaderWidget(std::shared_ptr< Pica::De // Create an HBoxLayout to store the widgets used to specify a particular attribute // and store it in a QWidget to allow for easy hiding and unhiding. auto row_layout = new QHBoxLayout; + // Remove unecessary padding between rows + row_layout->setContentsMargins(0, 0, 0, 0); + row_layout->addWidget(new QLabel(tr("Attribute %1").arg(i, 2))); for (unsigned comp = 0; comp < 4; ++comp) row_layout->addWidget(input_data[4 * i + comp]); @@ -414,20 +418,25 @@ GraphicsVertexShaderWidget::GraphicsVertexShaderWidget(std::shared_ptr< Pica::De input_data_group->setLayout(sub_layout); main_layout->addWidget(input_data_group); } - { - auto sub_layout = new QHBoxLayout; - sub_layout->addWidget(binary_list); - main_layout->addLayout(sub_layout); - } + + // Make program listing expand to fill available space in the dialog + binary_list->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::MinimumExpanding); + main_layout->addWidget(binary_list); + main_layout->addWidget(dump_shader); { - auto sub_layout = new QHBoxLayout; - sub_layout->addWidget(new QLabel(tr("Cycle Index:"))); - sub_layout->addWidget(cycle_index); + auto sub_layout = new QFormLayout; + sub_layout->addRow(tr("Cycle Index:"), cycle_index); + main_layout->addLayout(sub_layout); } + + // Set a minimum height so that the size of this label doesn't cause the rest of the bottom + // part of the UI to keep jumping up and down when cycling through instructions. + instruction_description->setMinimumHeight(instruction_description->fontMetrics().lineSpacing() * 6); + instruction_description->setAlignment(Qt::AlignLeft | Qt::AlignTop); main_layout->addWidget(instruction_description); - main_layout->addStretch(); + main_widget->setLayout(main_layout); setWidget(main_widget); From c1beb2ce2041c078674fb0cacafbccb4196cc3bc Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Wed, 19 Aug 2015 05:51:12 -0300 Subject: [PATCH 08/12] Shader Debugger: Fix freeze when double-clicking shader disassembly --- src/citra_qt/debugger/graphics_cmdlists.cpp | 2 +- src/citra_qt/debugger/graphics_vertex_shader.cpp | 10 +--------- src/citra_qt/debugger/graphics_vertex_shader.h | 6 ++---- 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/citra_qt/debugger/graphics_cmdlists.cpp b/src/citra_qt/debugger/graphics_cmdlists.cpp index 025434687..fa32d24b5 100644 --- a/src/citra_qt/debugger/graphics_cmdlists.cpp +++ b/src/citra_qt/debugger/graphics_cmdlists.cpp @@ -359,7 +359,7 @@ void GPUCommandListWidget::CopyAllToClipboard() { QClipboard* clipboard = QApplication::clipboard(); QString text; - QAbstractItemModel* model = static_cast(list_widget->model()); + QAbstractItemModel* model = static_cast(list_widget->model()); for (int row = 0; row < model->rowCount({}); ++row) { for (int col = 0; col < model->columnCount({}); ++col) { diff --git a/src/citra_qt/debugger/graphics_vertex_shader.cpp b/src/citra_qt/debugger/graphics_vertex_shader.cpp index 32c48cb6e..a94538874 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.cpp +++ b/src/citra_qt/debugger/graphics_vertex_shader.cpp @@ -27,18 +27,10 @@ using nihstro::Instruction; using nihstro::SourceRegister; using nihstro::SwizzlePattern; -GraphicsVertexShaderModel::GraphicsVertexShaderModel(GraphicsVertexShaderWidget* parent): QAbstractItemModel(parent), par(parent) { +GraphicsVertexShaderModel::GraphicsVertexShaderModel(GraphicsVertexShaderWidget* parent): QAbstractTableModel(parent), par(parent) { } -QModelIndex GraphicsVertexShaderModel::index(int row, int column, const QModelIndex& parent) const { - return createIndex(row, column); -} - -QModelIndex GraphicsVertexShaderModel::parent(const QModelIndex& child) const { - return QModelIndex(); -} - int GraphicsVertexShaderModel::columnCount(const QModelIndex& parent) const { return 3; } diff --git a/src/citra_qt/debugger/graphics_vertex_shader.h b/src/citra_qt/debugger/graphics_vertex_shader.h index 1b3f1f7ec..d4e93103f 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.h +++ b/src/citra_qt/debugger/graphics_vertex_shader.h @@ -4,7 +4,7 @@ #pragma once -#include +#include #include "graphics_breakpoint_observer.h" @@ -17,14 +17,12 @@ class QSpinBox; class GraphicsVertexShaderWidget; -class GraphicsVertexShaderModel : public QAbstractItemModel { +class GraphicsVertexShaderModel : public QAbstractTableModel { Q_OBJECT public: GraphicsVertexShaderModel(GraphicsVertexShaderWidget* parent); - QModelIndex index(int row, int column, const QModelIndex& parent = QModelIndex()) const override; - QModelIndex parent(const QModelIndex& child) const override; int columnCount(const QModelIndex& parent = QModelIndex()) const override; int rowCount(const QModelIndex& parent = QModelIndex()) const override; QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; From f77dfb3966f9f0aad09e54e77fff6ea2256e114d Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Tue, 25 Aug 2015 02:36:57 -0300 Subject: [PATCH 09/12] Shader Debugger: Fix only first vertex attribute being loaded --- src/citra_qt/debugger/graphics_vertex_shader.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/citra_qt/debugger/graphics_vertex_shader.cpp b/src/citra_qt/debugger/graphics_vertex_shader.cpp index a94538874..831ead51a 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.cpp +++ b/src/citra_qt/debugger/graphics_vertex_shader.cpp @@ -475,6 +475,7 @@ void GraphicsVertexShaderWidget::Reload(bool replace_vertex_data, void* vertex_d auto& shader_config = Pica::g_state.regs.vs; for (auto instr : shader_setup.program_code) info.code.push_back({instr}); + int num_attributes = Pica::g_state.regs.vertex_attributes.GetNumTotalAttributes(); for (auto pattern : shader_setup.swizzle_data) info.swizzle_info.push_back({pattern}); @@ -483,19 +484,18 @@ void GraphicsVertexShaderWidget::Reload(bool replace_vertex_data, void* vertex_d info.labels.insert({ entry_point, "main" }); // Generate debug information - debug_data = Pica::Shader::ProduceDebugInfo(input_vertex, 1, shader_config, shader_setup); + debug_data = Pica::Shader::ProduceDebugInfo(input_vertex, num_attributes, shader_config, shader_setup); // Reload widget state - - // Only show input attributes which are used as input to the shader - for (unsigned int attr = 0; attr < 16; ++attr) { - input_data_container[attr]->setVisible(false); - } - for (unsigned int attr = 0; attr < Pica::g_state.regs.vertex_attributes.GetNumTotalAttributes(); ++attr) { + for (unsigned int attr = 0; attr < num_attributes; ++attr) { unsigned source_attr = shader_config.input_register_map.GetRegisterForAttribute(attr); input_data_mapping[source_attr]->setText(QString("-> v%1").arg(attr)); input_data_container[source_attr]->setVisible(true); } + // Only show input attributes which are used as input to the shader + for (unsigned int attr = num_attributes; attr < 16; ++attr) { + input_data_container[attr]->setVisible(false); + } // Initialize debug info text for current cycle count cycle_index->setMaximum(debug_data.records.size() - 1); From 2c98275b5196fcd1c0c479a3a357a44fce324e07 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Tue, 25 Aug 2015 06:16:21 -0300 Subject: [PATCH 10/12] Shader Debugger: Remove useless signal --- src/citra_qt/debugger/graphics_vertex_shader.cpp | 7 ++----- src/citra_qt/debugger/graphics_vertex_shader.h | 5 ----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/citra_qt/debugger/graphics_vertex_shader.cpp b/src/citra_qt/debugger/graphics_vertex_shader.cpp index 831ead51a..64a3569d4 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.cpp +++ b/src/citra_qt/debugger/graphics_vertex_shader.cpp @@ -361,9 +361,6 @@ GraphicsVertexShaderWidget::GraphicsVertexShaderWidget(std::shared_ptr< Pica::De cycle_index = new QSpinBox; - connect(this, SIGNAL(SelectCommand(const QModelIndex&, QItemSelectionModel::SelectionFlags)), - binary_list->selectionModel(), SLOT(select(const QModelIndex&, QItemSelectionModel::SelectionFlags))); - connect(dump_shader, SIGNAL(clicked()), this, SLOT(DumpShader())); connect(cycle_index, SIGNAL(valueChanged(int)), this, SLOT(OnCycleIndexChanged(int))); @@ -550,7 +547,7 @@ void GraphicsVertexShaderWidget::OnCycleIndexChanged(int index) { instruction_description->setText(text); // Scroll to current instruction - const QModelIndex& instr_index = model->index(record.instruction_offset, 0); - emit SelectCommand(instr_index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + QModelIndex instr_index = model->index(record.instruction_offset, 0); + binary_list->selectionModel()->select(instr_index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); binary_list->scrollTo(instr_index, QAbstractItemView::EnsureVisible); } diff --git a/src/citra_qt/debugger/graphics_vertex_shader.h b/src/citra_qt/debugger/graphics_vertex_shader.h index d4e93103f..0bf1652fc 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.h +++ b/src/citra_qt/debugger/graphics_vertex_shader.h @@ -60,11 +60,6 @@ private slots: */ void Reload(bool replace_vertex_data = false, void* vertex_data = nullptr); - -signals: - // Call this to change the current command selection in the disassembly view - void SelectCommand(const QModelIndex&, QItemSelectionModel::SelectionFlags); - private: QLabel* instruction_description; QTreeView* binary_list; From 2bdf9ede9195ab99252094b6a04cab8eebdaafcc Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Tue, 25 Aug 2015 06:45:29 -0300 Subject: [PATCH 11/12] Shader Debugger: Highlight current instruction instead of focusing This avoid some annoying focus stealing in some situations, and looks nicer in general. --- .../debugger/graphics_vertex_shader.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/citra_qt/debugger/graphics_vertex_shader.cpp b/src/citra_qt/debugger/graphics_vertex_shader.cpp index 64a3569d4..f01be71cc 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.cpp +++ b/src/citra_qt/debugger/graphics_vertex_shader.cpp @@ -292,12 +292,23 @@ QVariant GraphicsVertexShaderModel::data(const QModelIndex& index, int role) con return GetMonospaceFont(); case Qt::BackgroundRole: - // Highlight instructions which have no debug data associated to them + { + // Highlight current instruction + int current_record_index = par->cycle_index->value(); + if (current_record_index < par->debug_data.records.size()) { + const auto& current_record = par->debug_data.records[current_record_index]; + if (index.row() == current_record.instruction_offset) { + return QColor(255, 255, 63); + } + } + + // Use a grey background for instructions which have no debug data associated to them for (const auto& record : par->debug_data.records) if (index.row() == record.instruction_offset) return QVariant(); - return QBrush(QColor(255, 255, 127)); + return QBrush(QColor(192, 192, 192)); + } // TODO: Draw arrows for each "reachable" instruction to visualize control flow @@ -546,8 +557,8 @@ void GraphicsVertexShaderWidget::OnCycleIndexChanged(int index) { instruction_description->setText(text); - // Scroll to current instruction + // Emit model update notification and scroll to current instruction QModelIndex instr_index = model->index(record.instruction_offset, 0); - binary_list->selectionModel()->select(instr_index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + emit model->dataChanged(instr_index, model->index(record.instruction_offset, model->columnCount())); binary_list->scrollTo(instr_index, QAbstractItemView::EnsureVisible); } From 2011f9a042bfb51124550462f64a2106e59e11df Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Tue, 25 Aug 2015 06:46:11 -0300 Subject: [PATCH 12/12] Shader Debugger: Allow editing of input vertex data --- src/citra_qt/debugger/graphics_vertex_shader.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/citra_qt/debugger/graphics_vertex_shader.cpp b/src/citra_qt/debugger/graphics_vertex_shader.cpp index f01be71cc..d48b3294b 100644 --- a/src/citra_qt/debugger/graphics_vertex_shader.cpp +++ b/src/citra_qt/debugger/graphics_vertex_shader.cpp @@ -518,6 +518,8 @@ void GraphicsVertexShaderWidget::OnResumed() { void GraphicsVertexShaderWidget::OnInputAttributeChanged(int index) { float value = input_data[index]->text().toFloat(); + input_vertex.attr[index / 4][index % 4] = Pica::float24::FromFloat32(value); + // Re-execute shader with updated value Reload(); }