diff --git a/libs/viewer/include/viewer/Automation.h b/libs/viewer/include/viewer/Automation.h index 51559ca651..c765e646c6 100644 --- a/libs/viewer/include/viewer/Automation.h +++ b/libs/viewer/include/viewer/Automation.h @@ -22,38 +22,32 @@ namespace filament { namespace viewer { -// Immutable list of settings permutations that are created and owned by AutomationList. -struct AutomationSpec { - char const* name; - size_t count; - Settings const* settings; -}; - -// List of automation specs constructed from a JSON string. +// Immutable list of Settings objects generated from a JSON spec. // -// Each top-level item in the JSON is an object with "name", "base" and "permute". +// Each top-level item in the JSON spec is an object with "name", "base" and "permute". // The "base" object specifies a single set of changes to apply to default settings. // The optional "permute" object specifies a cross product of changes to apply to the base. // See the unit test for an example. class AutomationList { public: - // Parses a JSON spec, then generates a set of Settings lists. + // Parses a JSON spec, then generates a list of Settings objects. // Returns null on failure (see utils log for warnings and errors). // Clients should release memory using "delete". - static AutomationList* generate(const char* jsonChunk, size_t size); + static AutomationList* generate(const char* jsonSpec, size_t size); - // Generates a default list of cases using an embedded JSON spec. - static AutomationList* generateDefault(); + // Generates a list of Settings objects using an embedded JSON spec. + static AutomationList* generateDefaultTestCases(); - // Returns the number of generated Settings lists. + // Returns the number of generated Settings objects. size_t size() const; - // Returns a view onto a generated Settings list. - AutomationSpec get(size_t index) const; + // Gets a generated Settings object and copies it out. + // Returns false if the given index is out of bounds. + bool get(size_t index, Settings* out) const; - // Returns the total number of Settings objects. - size_t totalCount() const; + // Returns the name of the JSON group for a given Settings object. + char const* getName(size_t index) const; // Frees all Settings objects and name strings. ~AutomationList(); diff --git a/libs/viewer/src/Automation.cpp b/libs/viewer/src/Automation.cpp index 145eeca8cf..c16adcf8a0 100644 --- a/libs/viewer/src/Automation.cpp +++ b/libs/viewer/src/Automation.cpp @@ -46,9 +46,6 @@ static const char* DEFAULT_AUTOMATION = R"TXT([ }, { "name": "viewopts", - "base": { - "view.postProcessingEnabled": true - } "permute": { "view.dithering": ["NONE", "TEMPORAL"], "view.sampleCount": [1, 4], @@ -61,13 +58,19 @@ static const char* DEFAULT_AUTOMATION = R"TXT([ ] )TXT"; -struct SpecImpl { +struct Case { + Settings settings; + char const* name; +}; + +struct CaseGroup { std::string name; std::vector cases; }; struct AutomationList::Impl { - std::vector specs; + std::vector cases; + std::vector names; }; static int parse(jsmntok_t const* tokens, int i, const char* jsonChunk, std::string* val) { @@ -135,7 +138,7 @@ static int parsePermutationsSpec(jsmntok_t const* tokens, int i, const char* jso } static int parseAutomationSpec(jsmntok_t const* tokens, int i, const char* jsonChunk, - SpecImpl* out) { + CaseGroup* out) { CHECK_TOKTYPE(tokens[i], JSMN_OBJECT); int size = tokens[i++].size; Settings base; @@ -162,6 +165,13 @@ static int parseAutomationSpec(jsmntok_t const* tokens, int i, const char* jsonC } } + // Leave early if there are no permutations. + if (permute.empty()) { + out->cases.resize(1); + out->cases[0] = base; + return i; + } + // Determine the number of permutations. size_t caseCount = 1; size_t propIndex = 0; @@ -176,50 +186,45 @@ static int parseAutomationSpec(jsmntok_t const* tokens, int i, const char* jsonC iters[propIndex++] = prop.begin(); } out->cases.resize(caseCount); - if (VERBOSE) { slog.i << " Case count: " << caseCount << io::endl; } size_t caseIndex = 0; while (true) { - - // Append a copy of the current Settings object to the case list. if (VERBOSE) { - slog.i << " Appending case " << caseIndex << io::endl; - } - out->cases[caseIndex++] = base; - - // Leave early if there are no permutations. - if (iters.empty()) { - return i; + slog.i << " Generating test case " << caseIndex << io::endl; } - // Use a basic counting algorithm to generate the next test case. + // Use a basic counting algorithm to select the next combination of property values. // Bump the first digit, if it rolls back to 0 then bump the next digit, etc. - // In this case, the "digit" is an iterator into a vector of JSON strings. - propIndex = 0; - for (auto& iter : iters) { - const auto& prop = permute[propIndex++]; - if (++iter != prop.end()) { - break; - } - iter = prop.begin(); + // In this case, each "digit" is an iterator into a vector of JSON strings. + if (caseIndex > 0) { + propIndex = 0; + for (auto& iter : iters) { + const auto& prop = permute[propIndex++]; + if (++iter != prop.end()) { + break; + } + iter = prop.begin(); - // Check if all permutations have been generated. - if (propIndex == permute.size()) { - assert(caseIndex == out->cases.size()); - return i; + // Check if all permutations have been generated. + if (propIndex == permute.size()) { + assert(caseIndex == caseCount); + return i; + } } } - // Apply changes to the settings object. + // Copy the base settings object, then apply changes. + Settings& testCase = out->cases[caseIndex++]; + testCase = base; for (const auto& iter : iters) { const std::string& jsonString = *iter; if (VERBOSE) { - slog.i << " Applying " << jsonString.c_str() << io::endl; + slog.i << " Applying " << jsonString.c_str() << io::endl; } - if (!readJson(jsonString.c_str(), jsonString.size(), &base)) { + if (!readJson(jsonString.c_str(), jsonString.size(), &testCase)) { return -1; } } @@ -228,7 +233,7 @@ static int parseAutomationSpec(jsmntok_t const* tokens, int i, const char* jsonC return i; } -static int parse(jsmntok_t const* tokens, int i, const char* jsonChunk, vector* out) { +static int parse(jsmntok_t const* tokens, int i, const char* jsonChunk, vector* out) { CHECK_TOKTYPE(tokens[i], JSMN_ARRAY); int size = tokens[i++].size; out->resize(size); @@ -257,40 +262,63 @@ AutomationList* AutomationList::generate(const char* jsonChunk, size_t size) { return nullptr; } - AutomationList::Impl* impl = new AutomationList::Impl(); - int i = parse(tokens, 0, jsonChunk, &impl->specs); + vector groups; + int i = parse(tokens, 0, jsonChunk, &groups); free(tokens); if (i < 0) { - delete impl; return nullptr; } + AutomationList::Impl* impl = new AutomationList::Impl(); + + // Compute the flattened number of Settings objects. + size_t total = 0; + for (const auto& group : groups) { + total += group.cases.size(); + } + + impl->names.resize(groups.size()); + impl->cases.resize(total); + + // Flatten the groups. + size_t caseIndex = 0, groupIndex = 0; + for (const auto& group : groups) { + impl->names[groupIndex] = group.name; + for (const auto& settings : group.cases) { + impl->cases[caseIndex].name = impl->names[groupIndex].c_str(); + impl->cases[caseIndex].settings = settings; + ++caseIndex; + } + ++groupIndex; + } + return new AutomationList(impl); } -AutomationList* AutomationList::generateDefault() { +AutomationList* AutomationList::generateDefaultTestCases() { return generate(DEFAULT_AUTOMATION, strlen(DEFAULT_AUTOMATION)); } -AutomationSpec AutomationList::get(size_t index) const { - const SpecImpl& spec = mImpl->specs[index]; - return { - .name = spec.name.c_str(), - .count = spec.cases.size(), - .settings = spec.cases.data() - }; -} - -size_t AutomationList::totalCount() const { - size_t count = 0; - for (const auto& spec : mImpl->specs) { - count += spec.cases.size(); +bool AutomationList::get(size_t index, Settings* out) const { + if (index >= mImpl->cases.size()) { + return false; } - return count; + if (out == nullptr) { + return true; + } + *out = mImpl->cases.at(index).settings; + return true; } -size_t AutomationList::size() const { return mImpl->specs.size(); } +char const* AutomationList::getName(size_t index) const { + if (index >= mImpl->cases.size()) { + return nullptr; + } + return mImpl->cases.at(index).name; +} + +size_t AutomationList::size() const { return mImpl->cases.size(); } AutomationList::AutomationList(Impl* impl) : mImpl(impl) {} AutomationList::~AutomationList() { delete mImpl; } diff --git a/libs/viewer/tests/test_settings.cpp b/libs/viewer/tests/test_settings.cpp index d415d20444..70d3babde0 100644 --- a/libs/viewer/tests/test_settings.cpp +++ b/libs/viewer/tests/test_settings.cpp @@ -143,12 +143,23 @@ TEST_F(ViewSettingsTest, JsonTestDefaults) { } TEST_F(ViewSettingsTest, AutomationSpec) { - AutomationList* specs = AutomationList::generateDefault(); + AutomationList* specs = AutomationList::generateDefaultTestCases(); ASSERT_TRUE(specs); - ASSERT_EQ(specs->size(), 2); - ASSERT_EQ(specs->get(0).count, 1); - ASSERT_EQ(specs->get(1).count, 1 << 6); - ASSERT_EQ(specs->totalCount(), 1 + (1 << 6)); + ASSERT_EQ(specs->size(), 65); + + Settings settings; + + ASSERT_TRUE(specs->get(0, &settings)); + ASSERT_FALSE(settings.view.postProcessingEnabled); + ASSERT_EQ(settings.view.dithering, Dithering::TEMPORAL); + + ASSERT_TRUE(specs->get(1, &settings)); + ASSERT_TRUE(settings.view.postProcessingEnabled); + ASSERT_EQ(settings.view.dithering, Dithering::NONE); + + ASSERT_TRUE(specs->get(64, &settings)); + ASSERT_FALSE(specs->get(65, &settings)); + delete specs; }