From c413e5786abbfe18bd3bdcc2ad4a14763711554f Mon Sep 17 00:00:00 2001 From: baldurk Date: Fri, 19 Nov 2021 14:55:15 +0000 Subject: [PATCH] Add some unit test coverage of basic types --- renderdoc/api/replay/rdcarray.h | 77 +++-- renderdoc/driver/shaders/dxbc/dxbc_bytecode.h | 11 + .../driver/shaders/dxbc/dxbc_bytecode_ops.cpp | 22 ++ renderdoc/replay/basic_types_tests.cpp | 289 +++++++++++++++++- renderdoc/replay/capture_options.cpp | 49 +++ 5 files changed, 419 insertions(+), 29 deletions(-) diff --git a/renderdoc/api/replay/rdcarray.h b/renderdoc/api/replay/rdcarray.h index cb46f9dbf..3a2057515 100644 --- a/renderdoc/api/replay/rdcarray.h +++ b/renderdoc/api/replay/rdcarray.h @@ -45,22 +45,13 @@ struct ItemHelper new(first + i) T(); } - static bool equalRange(const T *a, const T *b, size_t count) + static int compRange(const T *a, const T *b, size_t count) { for(size_t i = 0; i < count; i++) if(!(a[i] == b[i])) - return false; + return a[i] < b[i] ? -1 : 1; - return true; - } - - static bool lessthanRange(const T *a, const T *b, size_t count) - { - for(size_t i = 0; i < count; i++) - if(!(a[i] == b[i])) - return a[i] < b[i]; - - return false; + return 0; } }; @@ -68,13 +59,9 @@ template struct ItemHelper { static void initRange(T *first, size_t itemCount) { memset(first, 0, itemCount * sizeof(T)); } - static bool equalRange(const T *a, const T *b, size_t count) + static int compRange(const T *a, const T *b, size_t count) { - return !memcmp(a, b, count * sizeof(T)); - } - static bool lessthanRange(const T *a, const T *b, size_t count) - { - return memcmp(a, b, count * sizeof(T)) < 0; + return memcmp(a, b, count * sizeof(T)); } }; @@ -184,14 +171,24 @@ public: const T &operator[](size_t i) const { return elems[i]; } bool operator==(const rdcarray &o) const { - return usedCount == o.usedCount && ItemHelper::equalRange(elems, o.elems, usedCount); + return usedCount == o.usedCount && ItemHelper::compRange(elems, o.elems, usedCount) == 0; } bool operator!=(const rdcarray &o) const { return !(*this == o); } bool operator<(const rdcarray &o) const { - if(usedCount != o.usedCount) - return usedCount < o.usedCount; - return ItemHelper::lessthanRange(elems, o.elems, usedCount); + // compare the subset of elements in both arrays + size_t c = usedCount; + if(o.usedCount < c) + c = o.usedCount; + + // compare the range + int comp = ItemHelper::compRange(elems, o.elems, usedCount); + // if it's not equal, we can return either true or false now + if(comp != 0) + return (comp < 0); + + // if they compared equal, the smaller array is less-than (if they're different sizes) + return usedCount < o.usedCount; } T *data() { return elems; } const T *data() const { return elems; } @@ -506,10 +503,38 @@ public: { // if we're inserting from within our range, save the index size_t idx = &el - begin(); + // do any potentially reallocating resize reserve(oldSize + 1); - // then move from the index in wherever elems now points - new(elems + offs) T(std::move(elems[idx])); + + // fast path where offs == size(), for push_back + if(offs == oldSize) + { + new(elems + offs) T(std::move(elems[idx])); + } + else + { + // we need to shuffle everything up by one + + // first pass, move construct elements and destruct as we go + const size_t moveCount = oldSize - offs; + for(size_t i = 0; i < moveCount; i++) + { + new(elems + oldSize - i) T(std::move(elems[oldSize - i - 1])); + ItemDestroyHelper::destroyRange(elems + oldSize - i - 1, 1); + } + + // if idx moved as a result of the insert, it will be coming from a different place + if(idx >= offs) + idx++; + + // then move construct the new value. + new(elems + offs) T(std::move(elems[idx])); + } + + // update new size + setUsedCount(usedCount + 1); + return; } @@ -830,12 +855,12 @@ public: const T &operator[](size_t i) const { return elems[i]; } bool operator==(const rdcfixedarray &o) const { - return ItemHelper::equalRange(elems, o.elems, N); + return ItemHelper::compRange(elems, o.elems, N) == 0; } bool operator!=(const rdcfixedarray &o) const { return !(*this == o); } bool operator<(const rdcfixedarray &o) const { - return ItemHelper::lessthanRange(elems, o.elems, N); + return ItemHelper::compRange(elems, o.elems, N) < 0; } T *data() { return elems; } const T *data() const { return elems; } diff --git a/renderdoc/driver/shaders/dxbc/dxbc_bytecode.h b/renderdoc/driver/shaders/dxbc/dxbc_bytecode.h index 622c748dc..16594f7d4 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_bytecode.h +++ b/renderdoc/driver/shaders/dxbc/dxbc_bytecode.h @@ -750,6 +750,7 @@ struct Operand } bool operator==(const Operand &o) const; + bool operator<(const Operand &o) const; // helper function that compares operands by their type and first index (for resources the logical // identifier - excluding register range on SM5.1) @@ -937,6 +938,16 @@ struct RegIndex return false; } + bool operator<(const RegIndex &o) const + { + if(absolute != o.absolute) + return absolute < o.absolute; + if(relative != o.relative) + return relative < o.relative; + if(index != o.index) + return index < o.index; + return operand < o.operand; + } rdcstr toString(const DXBC::Reflection *reflection, ToString flags) const; diff --git a/renderdoc/driver/shaders/dxbc/dxbc_bytecode_ops.cpp b/renderdoc/driver/shaders/dxbc/dxbc_bytecode_ops.cpp index fa894798b..2e51e1bf9 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_bytecode_ops.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_bytecode_ops.cpp @@ -101,6 +101,28 @@ bool Operand::operator==(const Operand &o) const return true; } +bool Operand::operator<(const Operand &o) const +{ + if(type != o.type) + return type < o.type; + if(numComponents != o.numComponents) + return numComponents < o.numComponents; + int c = memcmp(comps, o.comps, 4); + if(c != 0) + return c < 0; + if((flags & (FLAG_ABS | FLAG_NEG)) != (o.flags & (FLAG_ABS | FLAG_NEG))) + return (flags & (FLAG_ABS | FLAG_NEG)) < (o.flags & (FLAG_ABS | FLAG_NEG)); + + if(indices != o.indices) + return indices < o.indices; + + for(size_t i = 0; i < 4; i++) + if(values[i] != o.values[i]) + return values[i] < o.values[i]; + + return false; +} + bool Operand::sameResource(const Operand &o) const { if(type != o.type) diff --git a/renderdoc/replay/basic_types_tests.cpp b/renderdoc/replay/basic_types_tests.cpp index cb508091e..d88476b0e 100644 --- a/renderdoc/replay/basic_types_tests.cpp +++ b/renderdoc/replay/basic_types_tests.cpp @@ -47,6 +47,21 @@ static int32_t movedDestructor = 0; static int32_t copyAssignment = 0; static int32_t moveAssignment = 0; +struct NonTrivial +{ + NonTrivial(int v) : val(v) {} + int val = 6; + + bool operator==(const NonTrivial &o) const { return val == o.val; } + bool operator<(const NonTrivial &o) const { return val > o.val; } +}; + +template <> +rdcstr DoStringise(const NonTrivial &el) +{ + return "NonTrivial{" + DoStringise(el.val) + "}"; +} + struct ConstructorCounter { int value; @@ -108,6 +123,7 @@ TEST_CASE("Test array type", "[basictypes]") SECTION("Basic test") { rdcarray test; + const rdcarray &constTest = test; CHECK(test.size() == 0); CHECK(test.capacity() == 0); @@ -115,6 +131,12 @@ TEST_CASE("Test array type", "[basictypes]") CHECK(test.isEmpty()); CHECK(test.begin() == test.end()); + CHECK(constTest.size() == 0); + CHECK(constTest.capacity() == 0); + CHECK(constTest.empty()); + CHECK(constTest.isEmpty()); + CHECK(constTest.begin() == constTest.end()); + test.clear(); CHECK(test.size() == 0); @@ -134,12 +156,22 @@ TEST_CASE("Test array type", "[basictypes]") test.push_back(10); + CHECK(test.front() == 5); + CHECK(test.back() == 10); CHECK(test.size() == 2); CHECK(test.capacity() >= 2); CHECK_FALSE(test.empty()); CHECK_FALSE(test.isEmpty()); CHECK(test.begin() + 2 == test.end()); + CHECK(constTest.front() == 5); + CHECK(constTest.back() == 10); + CHECK(constTest.size() == 2); + CHECK(constTest.capacity() >= 2); + CHECK_FALSE(constTest.empty()); + CHECK_FALSE(constTest.isEmpty()); + CHECK(constTest.begin() + 2 == constTest.end()); + int sum = 0; for(int x : test) sum += x; @@ -154,7 +186,7 @@ TEST_CASE("Test array type", "[basictypes]") CHECK(test.isEmpty()); CHECK(test.begin() == test.end()); - test = {4, 1, 77, 0, 0, 8, 20, 934}; + test = {4, 1, 77, 6, 0, 8, 20, 934}; CHECK(test.size() == 8); CHECK(test.capacity() >= 8); @@ -166,7 +198,7 @@ TEST_CASE("Test array type", "[basictypes]") for(int x : test) sum += x; - CHECK(sum == 1044); + CHECK(sum == 1050); CHECK(test[2] == 77); @@ -181,8 +213,88 @@ TEST_CASE("Test array type", "[basictypes]") CHECK_FALSE(test.empty()); CHECK_FALSE(test.isEmpty()); CHECK(test.begin() + 8 == test.end()); + + int x = test.takeAt(2); + + CHECK(test.size() == 7); + CHECK(x == 10); + CHECK(test[2] == 6); + } + + SECTION("Comparison test with trivial type") + { + rdcarray test; + rdcarray test2; + + test = {1, 2, 3, 4, 5, 6}; + test2 = {1, 2, 3, 5, 6, 6}; + + CHECK(test < test2); + CHECK(test != test2); + CHECK_FALSE(test2 < test); + + test2[3]--; + test2[4]--; + + CHECK(test == test2); + + test.pop_back(); + + // smaller arrays are considered less-than if all elements are equal + CHECK(test < test2); + CHECK_FALSE(test2 < test); + CHECK_FALSE(test == test2); + + // if however one of the common prefix is greater, that's not true + test[0] += 10; + CHECK(test2 < test); + CHECK_FALSE(test < test2); + CHECK_FALSE(test == test2); }; + SECTION("Comparison test with non-trivial type") + { + rdcarray test; + rdcarray test2; + + test.push_back(NonTrivial(1)); + test.push_back(NonTrivial(2)); + test.push_back(NonTrivial(3)); + test.push_back(NonTrivial(4)); + test.push_back(NonTrivial(5)); + test.push_back(NonTrivial(6)); + + test2.push_back(NonTrivial(1)); + test2.push_back(NonTrivial(2)); + test2.push_back(NonTrivial(3)); + test2.push_back(NonTrivial(3)); + test2.push_back(NonTrivial(4)); + test2.push_back(NonTrivial(6)); + + // order is reversed because custom NonTrivial < is reversed + CHECK(test < test2); + CHECK(test != test2); + CHECK_FALSE(test2 < test); + + test2[3].val++; + test2[4].val++; + + CHECK(test == test2); + + test.pop_back(); + + // smaller arrays are considered less-than if all elements are equal + CHECK(test < test2); + CHECK_FALSE(test2 < test); + CHECK_FALSE(test == test2); + + // if however one of the common prefix is greater, that's not true + test[0].val -= 10; + CHECK(test2 < test); + CHECK_FALSE(test < test2); + CHECK_FALSE(test == test2); + } + SECTION("Test constructing/assigning from other types") { rdcarray test; @@ -373,6 +485,23 @@ TEST_CASE("Test array type", "[basictypes]") CHECK(vec[1] == 3); CHECK(vec[2] == 5); + // do some empty/invalid erases + vec.erase(2, 0); + + REQUIRE(vec.size() == 3); + REQUIRE(vec.capacity() >= 4); + CHECK(vec[0] == 6); + CHECK(vec[1] == 3); + CHECK(vec[2] == 5); + + vec.erase(200, 5); + + REQUIRE(vec.size() == 3); + REQUIRE(vec.capacity() >= 4); + CHECK(vec[0] == 6); + CHECK(vec[1] == 3); + CHECK(vec[2] == 5); + vec.insert(2, {0, 1}); REQUIRE(vec.size() == 5); @@ -913,6 +1042,30 @@ TEST_CASE("Test array type", "[basictypes]") CHECK(copyAssignment == 0); CHECK(moveAssignment == 0); + // insert an element at an invalid offset, this should do nothing + test.insert(100, &tmp, 1); + + CHECK(test.size() == 6); + CHECK(valueConstructor == 4); + CHECK(copyConstructor == 2); + CHECK(moveConstructor == 6); + CHECK(destructor == 6); + CHECK(movedDestructor == 6); + CHECK(copyAssignment == 0); + CHECK(moveAssignment == 0); + + // self-assignment should also do nothing + test = test; + + CHECK(test.size() == 6); + CHECK(valueConstructor == 4); + CHECK(copyConstructor == 2); + CHECK(moveConstructor == 6); + CHECK(destructor == 6); + CHECK(movedDestructor == 6); + CHECK(copyAssignment == 0); + CHECK(moveAssignment == 0); + // insert an element at 0, allowing it to move test.insert(4, ConstructorCounter(55)); @@ -936,6 +1089,82 @@ TEST_CASE("Test array type", "[basictypes]") CHECK(movedDestructor == 12); CHECK(copyAssignment == 0); CHECK(moveAssignment == 0); + + test[1].value = 10; + + CHECK(test[0].value == 9); + CHECK(test[1].value == 10); + CHECK(test[2].value == 5); + CHECK(test[3].value == 55); + CHECK(test[4].value == 25); + CHECK(test[5].value == 35); + + // this should move + test.push_back(std::move(test[0])); + + CHECK(test.back().value == 9); + CHECK(test.size() == 7); + CHECK(valueConstructor == 5); + CHECK(copyConstructor == 2); + CHECK(moveConstructor == 13); + CHECK(destructor == 13); + CHECK(movedDestructor == 12); + CHECK(copyAssignment == 0); + CHECK(moveAssignment == 0); + + // insert by moving at the end, this will only do the one move construct from [1] to [7] + test.insert(7, std::move(test[1])); + + CHECK(test[7].value == 10); + CHECK(test.size() == 8); + CHECK(valueConstructor == 5); + CHECK(copyConstructor == 2); + CHECK(moveConstructor == 14); + CHECK(destructor == 13); + CHECK(movedDestructor == 12); + CHECK(copyAssignment == 0); + CHECK(moveAssignment == 0); + + // insert with some moves required to shuffle up values, but the move itself still happens from + // the same element (lower to higher index) + test.insert(5, std::move(test[2])); + + CHECK(test[5].value == 5); + CHECK(test.size() == 9); + CHECK(valueConstructor == 5); + CHECK(copyConstructor == 2); + CHECK(moveConstructor == 18); + CHECK(destructor == 16); + CHECK(movedDestructor == 15); + CHECK(copyAssignment == 0); + CHECK(moveAssignment == 0); + + // insert with some moves required to shuffle up values, plus the move itself happens from one + // of the moved elements + test.insert(7, std::move(test[8])); + + CHECK(test[7].value == 10); + CHECK(test.size() == 10); + CHECK(valueConstructor == 5); + CHECK(copyConstructor == 2); + CHECK(moveConstructor == 21); + CHECK(destructor == 18); + CHECK(movedDestructor == 17); + CHECK(copyAssignment == 0); + CHECK(moveAssignment == 0); + + // invalid move insert + test.insert(100, std::move(test[8])); + + CHECK(test[7].value == 10); + CHECK(test.size() == 10); + CHECK(valueConstructor == 5); + CHECK(copyConstructor == 2); + CHECK(moveConstructor == 21); + CHECK(destructor == 18); + CHECK(movedDestructor == 17); + CHECK(copyAssignment == 0); + CHECK(moveAssignment == 0); }; }; @@ -2062,6 +2291,7 @@ TEST_CASE("Test rdcfixedarray type", "[basictypes][rdcfixedarray]") SECTION("Basic test") { rdcfixedarray test = {}; + const rdcfixedarray &constTest = test; CHECK(test.size() == 8); CHECK(test.byteSize() == 32); @@ -2075,10 +2305,29 @@ TEST_CASE("Test rdcfixedarray type", "[basictypes][rdcfixedarray]") CHECK(!test.contains(2)); CHECK(test.indexOf(8) == 5); CHECK(test.indexOf(9) == -1); - CHECK(test[0] == 4); CHECK(test[2] == 77); CHECK(test[4] == 0); + CHECK(test.front() == 4); + CHECK(test.back() == 934); + CHECK(test.data()[0] == 4); + CHECK(test.data()[2] == 77); + CHECK(test.begin() != test.end()); + CHECK(test.begin() + 8 == test.end()); + + CHECK(constTest.contains(1)); + CHECK(!constTest.contains(2)); + CHECK(constTest.indexOf(8) == 5); + CHECK(constTest.indexOf(9) == -1); + CHECK(constTest[0] == 4); + CHECK(constTest[2] == 77); + CHECK(constTest[4] == 0); + CHECK(constTest.front() == 4); + CHECK(constTest.back() == 934); + CHECK(constTest.data()[0] == 4); + CHECK(constTest.data()[2] == 77); + CHECK(constTest.begin() != constTest.end()); + CHECK(constTest.begin() + 8 == constTest.end()); int sum = 0; for(int x : test) @@ -2097,6 +2346,40 @@ TEST_CASE("Test rdcfixedarray type", "[basictypes][rdcfixedarray]") sum += x; CHECK(sum == 1045); + + rdcfixedarray test2 = {1, 2, 3, 4}; + + CHECK(test2[0] == 1); + CHECK(test2[1] == 2); + CHECK(test2[2] == 3); + CHECK(test2[3] == 4); + CHECK(test2[4] == 0); + CHECK(test2[5] == 0); + CHECK(test2[6] == 0); + CHECK(test2[7] == 0); + + int32_t arr[8] = {5, 6, 7, 8}; + + test2 = arr; + + CHECK(test2[0] == 5); + CHECK(test2[1] == 6); + CHECK(test2[2] == 7); + CHECK(test2[3] == 8); + CHECK(test2[4] == 0); + CHECK(test2[5] == 0); + CHECK(test2[6] == 0); + CHECK(test2[7] == 0); + + CHECK(test < test2); + CHECK_FALSE(test2 < test); + CHECK_FALSE(test == test2); + CHECK(test != test2); + + test2[0] = 3; + + CHECK(test2 < test); + CHECK_FALSE(test < test2); }; SECTION("Test of rdcfixedarray of ResourceId") diff --git a/renderdoc/replay/capture_options.cpp b/renderdoc/replay/capture_options.cpp index ff522697c..cd07d2b2d 100644 --- a/renderdoc/replay/capture_options.cpp +++ b/renderdoc/replay/capture_options.cpp @@ -185,3 +185,52 @@ CaptureOptions::CaptureOptions() captureAllCmdLists = false; debugOutputMute = true; } + +#if ENABLED(ENABLE_UNIT_TESTS) + +#undef None +#undef Always + +#include "catch/catch.hpp" + +TEST_CASE("Check CaptureOptions de/serialise to string", "[serialise]") +{ + CaptureOptions opts; + + bool *boolOpts[] = { + &opts.allowVSync, + &opts.allowFullscreen, + &opts.apiValidation, + &opts.captureCallstacks, + &opts.captureCallstacksOnlyActions, + &opts.verifyBufferAccess, + &opts.hookIntoChildren, + &opts.refAllResources, + &opts.captureAllCmdLists, + &opts.debugOutputMute, + }; + + for(uint32_t delay = 0; delay < 1000; delay++) + { + for(uint32_t variant = 0; variant < (1 << ARRAY_COUNT(boolOpts)); variant++) + { + opts.delayForDebugger = delay; + for(size_t o = 0; o < ARRAY_COUNT(boolOpts); o++) + { + *boolOpts[o] = (variant & (1 << o)) != 0; + } + + rdcstr s = opts.EncodeAsString(); + CaptureOptions decoded; + decoded.DecodeFromString(s); + + CHECK(memcmp(&opts, &decoded, sizeof(decoded)) == 0); + } + } + + // check that nothing explodes here + CaptureOptions a; + a.DecodeFromString(""); +} + +#endif