From fbb743377a8a188681359eb645278f66e26b5ace Mon Sep 17 00:00:00 2001 From: John Stiles Date: Mon, 13 Dec 2021 11:11:14 -0500 Subject: [PATCH] Add method to get the suffix for a trace's slot. We already had this logic in SkVMDebugTrace::dump; it just needed to be factored out so that it could be accessed directly. Change-Id: Idd6c92d23ab4dddc60fdc3c7b1693a0d89b1c992 Bug: skia:12666 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/483517 Reviewed-by: Brian Osman Commit-Queue: John Stiles Auto-Submit: John Stiles --- src/sksl/tracing/SkVMDebugTrace.cpp | 36 +++++++++------ src/sksl/tracing/SkVMDebugTrace.h | 3 ++ tests/SkVMDebugTracePlayerTest.cpp | 71 ++++++++++++++--------------- tests/SkVMDebugTraceTest.cpp | 46 +++++++++++++++++++ 4 files changed, 105 insertions(+), 51 deletions(-) diff --git a/src/sksl/tracing/SkVMDebugTrace.cpp b/src/sksl/tracing/SkVMDebugTrace.cpp index ac35636fc9..5cb62db74b 100644 --- a/src/sksl/tracing/SkVMDebugTrace.cpp +++ b/src/sksl/tracing/SkVMDebugTrace.cpp @@ -14,6 +14,26 @@ namespace SkSL { +std::string SkVMDebugTrace::getSlotComponentSuffix(int slotIndex) const { + const SkSL::SkVMSlotInfo& slot = fSlotInfo[slotIndex]; + + if (slot.rows > 1) { + return "[" + std::to_string(slot.componentIndex / slot.rows) + + "][" + std::to_string(slot.componentIndex % slot.rows) + + "]"; + } + if (slot.columns > 1) { + switch (slot.componentIndex) { + case 0: return ".x"; break; + case 1: return ".y"; break; + case 2: return ".z"; break; + case 3: return ".w"; break; + default: return "[???]"; break; + } + } + return {}; +} + void SkVMDebugTrace::setTraceCoord(const SkIPoint& coord) { fTraceCoord = coord; } @@ -89,21 +109,7 @@ void SkVMDebugTrace::dump(SkWStream* o) const { const SkVMSlotInfo& slot = fSlotInfo[data0]; o->writeText(indent.c_str()); o->writeText(slot.name.c_str()); - if (slot.rows > 1) { - o->writeText("["); - o->writeDecAsText(slot.componentIndex / slot.rows); - o->writeText("]["); - o->writeDecAsText(slot.componentIndex % slot.rows); - o->writeText("]"); - } else if (slot.columns > 1) { - switch (slot.componentIndex) { - case 0: o->writeText(".x"); break; - case 1: o->writeText(".y"); break; - case 2: o->writeText(".z"); break; - case 3: o->writeText(".w"); break; - default: o->writeText("[???]"); break; - } - } + o->writeText(this->getSlotComponentSuffix(data0).c_str()); o->writeText(" = "); switch (slot.numberKind) { case SkSL::Type::NumberKind::kSigned: diff --git a/src/sksl/tracing/SkVMDebugTrace.h b/src/sksl/tracing/SkVMDebugTrace.h index 3351d5cf1a..f4ae412915 100644 --- a/src/sksl/tracing/SkVMDebugTrace.h +++ b/src/sksl/tracing/SkVMDebugTrace.h @@ -69,6 +69,9 @@ public: /** Generates a human-readable dump of the debug trace. */ void dump(SkWStream* o) const override; + /** Returns a slot's component as a variable-name suffix, e.g. ".x" or "[2][2]". */ + std::string getSlotComponentSuffix(int slotIndex) const; + /** The device-coordinate pixel to trace (controlled by setTraceCoord) */ SkIPoint fTraceCoord = {}; diff --git a/tests/SkVMDebugTracePlayerTest.cpp b/tests/SkVMDebugTracePlayerTest.cpp index d66562d316..55ae4bb47e 100644 --- a/tests/SkVMDebugTracePlayerTest.cpp +++ b/tests/SkVMDebugTracePlayerTest.cpp @@ -71,8 +71,7 @@ static std::string make_vars_string( const SkSL::SkVMSlotInfo& slot = trace.fSlotInfo[var.fSlotIndex]; text += slot.name; - text += ":"; - text += std::to_string(slot.componentIndex); + text += trace.getSlotComponentSuffix(var.fSlotIndex); text += " = "; switch (slot.numberKind) { @@ -151,7 +150,7 @@ int main() { // Line 2 REPORTER_ASSERT(r, player.traceHasCompleted()); REPORTER_ASSERT(r, player.getCurrentLine() == -1); REPORTER_ASSERT(r, player.getCallStack().empty()); - REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result:0 = 4"); + REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result = 4"); } DEF_TEST(SkSLTracePlayerReset, r) { @@ -228,7 +227,7 @@ int main() { // Line 8 REPORTER_ASSERT(r, player.traceHasCompleted()); REPORTER_ASSERT(r, player.getCurrentLine() == -1); REPORTER_ASSERT(r, player.getCallStack().empty()); - REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result:0 = 4"); + REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result = 4"); // Watch the stack grow and shrink as single-step. player.reset(trace); @@ -250,17 +249,17 @@ int main() { // Line 8 player.step(); REPORTER_ASSERT(r, make_stack_string(*trace, player) == "int main() -> int fnA()"); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "[fnB].result:0 = 4"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "[fnB].result = 4"); REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == ""); player.step(); REPORTER_ASSERT(r, make_stack_string(*trace, player) == "int main()"); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "[fnA].result:0 = 4"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "[fnA].result = 4"); REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == ""); player.step(); REPORTER_ASSERT(r, player.traceHasCompleted()); - REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result:0 = 4"); + REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result = 4"); } DEF_TEST(SkSLTracePlayerVariables, r) { @@ -291,12 +290,12 @@ int main() { // Line 6 REPORTER_ASSERT(r, player.getCurrentLine() == 8); REPORTER_ASSERT(r, make_stack_string(*trace, player) == "int main()"); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a:0 = 123"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a = 123"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 9); REPORTER_ASSERT(r, make_stack_string(*trace, player) == "int main()"); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a:0 = 123, b:0 = true"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a = 123, b = true"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 3); @@ -306,38 +305,38 @@ int main() { // Line 6 REPORTER_ASSERT(r, player.getCurrentLine() == 4); REPORTER_ASSERT(r, make_stack_string(*trace, player) == "int main() -> void func()"); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "z:0 = 456"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "z = 456"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 9); REPORTER_ASSERT(r, make_stack_string(*trace, player) == "int main()"); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a:0 = 123, b:0 = true"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a = 123, b = true"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 10); REPORTER_ASSERT(r, make_stack_string(*trace, player) == "int main()"); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a:0 = 123, b:0 = true"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a = 123, b = true"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 11); REPORTER_ASSERT(r, make_stack_string(*trace, player) == "int main()"); REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == - "a:0 = 123, b:0 = true, c:0 = 0.000000, c:1 = 0.500000, " - "c:2 = 1.000000, c:3 = -1.000000"); + "a = 123, b = true, c.x = 0.000000, c.y = 0.500000, c.z = 1.000000, " + "c.w = -1.000000"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 12); REPORTER_ASSERT(r, make_stack_string(*trace, player) == "int main()"); REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == - "a:0 = 123, b:0 = true, c:0 = 0.000000, c:1 = 0.500000, c:2 = " - "1.000000, c:3 = -1.000000, d:0 = 2.000000, d:1 = 0.000000, d:2 = " - "0.000000, d:3 = 0.000000, d:4 = 2.000000, d:5 = 0.000000, d:6 = " - "0.000000, d:7 = 0.000000, d:8 = 2.000000"); + "a = 123, b = true, c.x = 0.000000, c.y = 0.500000, c.z = 1.000000, " + "c.w = -1.000000, d[0][0] = 2.000000, d[0][1] = 0.000000, d[0][2] = 0.000000, " + "d[1][0] = 0.000000, d[1][1] = 2.000000, d[1][2] = 0.000000, " + "d[2][0] = 0.000000, d[2][1] = 0.000000, d[2][2] = 2.000000"); player.step(); REPORTER_ASSERT(r, player.traceHasCompleted()); REPORTER_ASSERT(r, make_stack_string(*trace, player) == ""); - REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result:0 = 123"); + REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result = 123"); } DEF_TEST(SkSLTracePlayerIfStatement, r) { @@ -368,29 +367,29 @@ int main() { // Line 2 player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 4); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val:0 = 0"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 0"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 5); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val:0 = 0"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 0"); player.step(); // We skip over the false-branch. REPORTER_ASSERT(r, player.getCurrentLine() == 9); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val:0 = 1"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 1"); player.step(); // We skip over the true-branch. REPORTER_ASSERT(r, player.getCurrentLine() == 12); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val:0 = 1"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 1"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 14); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val:0 = 4"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 4"); player.step(); REPORTER_ASSERT(r, player.traceHasCompleted()); - REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result:0 = 4"); + REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result = 4"); } DEF_TEST(SkSLTracePlayerForLoop, r) { @@ -414,31 +413,31 @@ int main() { // Line 2 player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 4); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val:0 = 0"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 0"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 5); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val:0 = 0, x:0 = 1"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 0, x = 1"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 4); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val:0 = 1, x:0 = 1"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 1, x = 1"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 5); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val:0 = 1, x:0 = 2"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 1, x = 2"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 4); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val:0 = 2, x:0 = 2"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 2, x = 2"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 7); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val:0 = 2, x:0 = 2"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 2, x = 2"); player.step(); REPORTER_ASSERT(r, player.traceHasCompleted()); - REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result:0 = 2"); + REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result = 2"); } DEF_TEST(SkSLTracePlayerStepOut, r) { @@ -472,20 +471,20 @@ int main() { // Line 9 REPORTER_ASSERT(r, player.getCurrentLine() == 4); REPORTER_ASSERT(r, make_stack_string(*trace, player) == "int main() -> int fn()"); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a:0 = 11"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a = 11"); player.step(); REPORTER_ASSERT(r, player.getCurrentLine() == 5); REPORTER_ASSERT(r, make_stack_string(*trace, player) == "int main() -> int fn()"); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a:0 = 11, b:0 = 22"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a = 11, b = 22"); player.stepOut(); // We should now be back inside main(), right where we left off. REPORTER_ASSERT(r, player.getCurrentLine() == 10); REPORTER_ASSERT(r, make_stack_string(*trace, player) == "int main()"); - REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "[fn].result:0 = 44"); + REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "[fn].result = 44"); player.stepOut(); REPORTER_ASSERT(r, player.traceHasCompleted()); - REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result:0 = 44"); + REPORTER_ASSERT(r, make_global_vars_string(*trace, player) == "[main].result = 44"); } diff --git a/tests/SkVMDebugTraceTest.cpp b/tests/SkVMDebugTraceTest.cpp index 3eae99c560..948e1817ed 100644 --- a/tests/SkVMDebugTraceTest.cpp +++ b/tests/SkVMDebugTraceTest.cpp @@ -126,3 +126,49 @@ DEF_TEST(SkVMDebugTraceRead, r) { REPORTER_ASSERT(r, i.fTraceInfo[3].data[0] == 20); REPORTER_ASSERT(r, i.fTraceInfo[3].data[1] == 0); } + +DEF_TEST(SkVMDebugTraceGetSlotComponentSuffix, r) { + // SkVMSlotInfo fields: + // - name + // - columns + // - rows + // - componentIndex + // - numberKind + // - line + // - fnReturnValue + + SkSL::SkVMDebugTrace i; + i.fSlotInfo = {{"s", 1, 1, 0, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"v", 4, 1, 0, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"v", 4, 1, 1, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"v", 4, 1, 2, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"v", 4, 1, 3, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 0, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 1, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 2, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 3, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 4, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 5, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 6, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 7, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 8, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 9, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 10, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 11, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 12, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 13, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 14, SkSL::Type::NumberKind::kFloat, 0, -1}, + {"m", 4, 4, 15, SkSL::Type::NumberKind::kFloat, 0, -1}}; + + const std::string kExpected[] = {"", + ".x", ".y", ".z", ".w", + "[0][0]", "[0][1]", "[0][2]", "[0][3]", + "[1][0]", "[1][1]", "[1][2]", "[1][3]", + "[2][0]", "[2][1]", "[2][2]", "[2][3]", + "[3][0]", "[3][1]", "[3][2]", "[3][3]"}; + + REPORTER_ASSERT(r, i.fSlotInfo.size() == SK_ARRAY_COUNT(kExpected)); + for (size_t index = 0; index < SK_ARRAY_COUNT(kExpected); ++index) { + REPORTER_ASSERT(r, kExpected[index] == i.getSlotComponentSuffix(index)); + } +}