From fc86525863df9eb5a2a01c948fa08f30e6cf616d Mon Sep 17 00:00:00 2001 From: Olivier Prat Date: Wed, 4 Apr 2018 12:00:06 +0200 Subject: [PATCH 1/5] Cleaned up a bit shadow map clear --- libraries/render-utils/src/RenderShadowTask.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libraries/render-utils/src/RenderShadowTask.cpp b/libraries/render-utils/src/RenderShadowTask.cpp index 69c5b3c689..faa5889307 100644 --- a/libraries/render-utils/src/RenderShadowTask.cpp +++ b/libraries/render-utils/src/RenderShadowTask.cpp @@ -149,9 +149,7 @@ void RenderShadowMap::run(const render::RenderContextPointer& renderContext, con batch.setStateScissorRect(viewport); batch.setFramebuffer(fbo); - batch.clearFramebuffer( - gpu::Framebuffer::BUFFER_COLOR0 | gpu::Framebuffer::BUFFER_DEPTH, - vec4(vec3(1.0, 1.0, 1.0), 0.0), 1.0, 0, true); + batch.clearDepthFramebuffer(1.0, false); glm::mat4 projMat; Transform viewMat; From 573f399023eafd625212284f827ea3207f006f76 Mon Sep 17 00:00:00 2001 From: Olivier Prat Date: Fri, 6 Apr 2018 14:45:16 +0200 Subject: [PATCH 2/5] Fixed incorrect shadow frustum far clip computation due to not taking into account shadow receivers --- .../render-utils/src/RenderShadowTask.cpp | 27 ++++++++++-------- libraries/render-utils/src/RenderShadowTask.h | 2 +- libraries/render/src/render/CullTask.cpp | 28 +++++++++++++------ libraries/render/src/render/CullTask.h | 2 +- 4 files changed, 38 insertions(+), 21 deletions(-) diff --git a/libraries/render-utils/src/RenderShadowTask.cpp b/libraries/render-utils/src/RenderShadowTask.cpp index faa5889307..fbb4bba263 100644 --- a/libraries/render-utils/src/RenderShadowTask.cpp +++ b/libraries/render-utils/src/RenderShadowTask.cpp @@ -230,12 +230,11 @@ void RenderShadowTask::build(JobModel& task, const render::Varying& input, rende const auto queryResolution = setupOutput.getN(2); // Fetch and cull the items from the scene - // Enable models to not cast shadows (otherwise, models will always cast shadows) - static const auto shadowCasterFilter = ItemFilter::Builder::visibleWorldItems().withTypeShape().withOpaque().withoutLayered().withTagBits(tagBits, tagMask).withShadowCaster(); + static const auto shadowCasterReceiverFilter = ItemFilter::Builder::visibleWorldItems().withTypeShape().withOpaque().withoutLayered().withTagBits(tagBits, tagMask); - const auto fetchInput = FetchSpatialTree::Inputs(shadowCasterFilter, queryResolution).asVarying(); + const auto fetchInput = FetchSpatialTree::Inputs(shadowCasterReceiverFilter, queryResolution).asVarying(); const auto shadowSelection = task.addJob("FetchShadowTree", fetchInput); - const auto selectionInputs = FetchSpatialSelection::Inputs(shadowSelection, shadowCasterFilter).asVarying(); + const auto selectionInputs = FetchSpatialSelection::Inputs(shadowSelection, shadowCasterReceiverFilter).asVarying(); const auto shadowItems = task.addJob("FetchShadowSelection", selectionInputs); // Cull objects that are not visible in camera view. Hopefully the cull functor only performs LOD culling, not @@ -259,21 +258,22 @@ void RenderShadowTask::build(JobModel& task, const render::Varying& input, rende char jobName[64]; sprintf(jobName, "ShadowCascadeSetup%d", i); const auto cascadeSetupOutput = task.addJob(jobName, i, _cullFunctor, tagBits, tagMask); - const auto shadowFilter = cascadeSetupOutput.getN(0); + const auto shadowRenderFilter = cascadeSetupOutput.getN(0); + const auto shadowBoundsFilter = cascadeSetupOutput.getN(1); auto antiFrustum = render::Varying(ViewFrustumPointer()); - cascadeFrustums[i] = cascadeSetupOutput.getN(1); + cascadeFrustums[i] = cascadeSetupOutput.getN(2); if (i > 1) { antiFrustum = cascadeFrustums[i - 2]; } // CPU jobs: finer grained culling - const auto cullInputs = CullShapeBounds::Inputs(sortedShapes, shadowFilter, antiFrustum).asVarying(); + const auto cullInputs = CullShapeBounds::Inputs(sortedShapes, shadowRenderFilter, shadowBoundsFilter, antiFrustum).asVarying(); const auto culledShadowItemsAndBounds = task.addJob("CullShadowCascade", cullInputs, shadowCullFunctor, RenderDetails::SHADOW); // GPU jobs: Render to shadow map sprintf(jobName, "RenderShadowMap%d", i); task.addJob(jobName, culledShadowItemsAndBounds, shapePlumber, i); - task.addJob("ShadowCascadeTeardown", shadowFilter); + task.addJob("ShadowCascadeTeardown", shadowRenderFilter); } task.addJob("ShadowTeardown", setupOutput); @@ -404,7 +404,11 @@ void RenderShadowCascadeSetup::run(const render::RenderContextPointer& renderCon const auto globalShadow = lightStage->getCurrentKeyShadow(); if (globalShadow && _cascadeIndexgetCascadeCount()) { - output.edit0() = ItemFilter::Builder::visibleWorldItems().withTypeShape().withOpaque().withoutLayered().withTagBits(_tagBits, _tagMask).withShadowCaster(); + auto baseFilter = ItemFilter::Builder::visibleWorldItems().withTypeShape().withOpaque().withoutLayered().withTagBits(_tagBits, _tagMask); + // Second item filter is to filter items to keep in shadow frustum computation (here we need to keep shadow receivers) + output.edit1() = baseFilter; + // First item filter is to filter items to render in shadow map (so only keep casters) + output.edit0() = baseFilter.withShadowCaster(); // Set the keylight render args auto& cascade = globalShadow->getCascade(_cascadeIndex); @@ -417,10 +421,11 @@ void RenderShadowCascadeSetup::run(const render::RenderContextPointer& renderCon texelSize *= minTexelCount; _cullFunctor._minSquareSize = texelSize * texelSize; - output.edit1() = cascadeFrustum; + output.edit2() = cascadeFrustum; } else { output.edit0() = ItemFilter::Builder::nothing(); - output.edit1() = ViewFrustumPointer(); + output.edit1() = ItemFilter::Builder::nothing(); + output.edit2() = ViewFrustumPointer(); } } diff --git a/libraries/render-utils/src/RenderShadowTask.h b/libraries/render-utils/src/RenderShadowTask.h index 98b70c0c9f..19ffcb4234 100644 --- a/libraries/render-utils/src/RenderShadowTask.h +++ b/libraries/render-utils/src/RenderShadowTask.h @@ -118,7 +118,7 @@ private: class RenderShadowCascadeSetup { public: - using Outputs = render::VaryingSet2; + using Outputs = render::VaryingSet3; using JobModel = render::Job::ModelO; RenderShadowCascadeSetup(unsigned int cascadeIndex, RenderShadowTask::CullFunctor& cullFunctor, uint8_t tagBits = 0x00, uint8_t tagMask = 0x00) : diff --git a/libraries/render/src/render/CullTask.cpp b/libraries/render/src/render/CullTask.cpp index f04427540a..b5819f114f 100644 --- a/libraries/render/src/render/CullTask.cpp +++ b/libraries/render/src/render/CullTask.cpp @@ -368,17 +368,19 @@ void CullShapeBounds::run(const RenderContextPointer& renderContext, const Input RenderArgs* args = renderContext->args; const auto& inShapes = inputs.get0(); - const auto& filter = inputs.get1(); - const auto& antiFrustum = inputs.get2(); + const auto& cullFilter = inputs.get1(); + const auto& boundsFilter = inputs.get2(); + const auto& antiFrustum = inputs.get3(); auto& outShapes = outputs.edit0(); auto& outBounds = outputs.edit1(); outShapes.clear(); outBounds = AABox(); - if (!filter.selectsNothing()) { + if (!cullFilter.selectsNothing() || !boundsFilter.selectsNothing()) { auto& details = args->_details.edit(_detailType); Test test(_cullFunctor, args, details, antiFrustum); + auto scene = args->_scene; for (auto& inItems : inShapes) { auto key = inItems.first; @@ -393,16 +395,26 @@ void CullShapeBounds::run(const RenderContextPointer& renderContext, const Input if (antiFrustum == nullptr) { for (auto& item : inItems.second) { if (test.solidAngleTest(item.bound) && test.frustumTest(item.bound)) { - outItems->second.emplace_back(item); - outBounds += item.bound; + const auto shapeKey = scene->getItem(item.id).getKey(); + if (cullFilter.test(shapeKey)) { + outItems->second.emplace_back(item); + } + if (boundsFilter.test(shapeKey)) { + outBounds += item.bound; + } } } } else { for (auto& item : inItems.second) { if (test.solidAngleTest(item.bound) && test.frustumTest(item.bound) && test.antiFrustumTest(item.bound)) { - outItems->second.emplace_back(item); - outBounds += item.bound; - } + const auto shapeKey = scene->getItem(item.id).getKey(); + if (cullFilter.test(shapeKey)) { + outItems->second.emplace_back(item); + } + if (boundsFilter.test(shapeKey)) { + outBounds += item.bound; + } + } } } details._rendered += (int)outItems->second.size(); diff --git a/libraries/render/src/render/CullTask.h b/libraries/render/src/render/CullTask.h index 3c5a30de89..47abe8a960 100644 --- a/libraries/render/src/render/CullTask.h +++ b/libraries/render/src/render/CullTask.h @@ -110,7 +110,7 @@ namespace render { class CullShapeBounds { public: - using Inputs = render::VaryingSet3; + using Inputs = render::VaryingSet4; using Outputs = render::VaryingSet2; using JobModel = Job::ModelIO; From ee4b1af3aa4ff93eb1348fb5d8a924b49237a9e2 Mon Sep 17 00:00:00 2001 From: NissimHadar Date: Tue, 8 May 2018 16:51:58 -0700 Subject: [PATCH 3/5] Corrected missing images in test.md files --- tools/auto-tester/src/Test.cpp | 76 ---------------------------------- tools/auto-tester/src/Test.h | 4 -- 2 files changed, 80 deletions(-) diff --git a/tools/auto-tester/src/Test.cpp b/tools/auto-tester/src/Test.cpp index 99f9025fdd..bdf734897e 100644 --- a/tools/auto-tester/src/Test.cpp +++ b/tools/auto-tester/src/Test.cpp @@ -478,26 +478,6 @@ ExtractedText Test::getTestScriptLines(QString testFileName) { QString regexTestTitle(ws + functionPerformName + "\\(" + quotedString + "\\," + ws + ownPath + "\\," + ws + functionParameter + ws + "{" + ".*"); QRegularExpression lineContainingTitle = QRegularExpression(regexTestTitle); - // Assert platform checks that test is running on the correct OS - const QString functionAssertPlatform(ws + "autoTester" + ws + "\\." + ws + "assertPlatform"); - const QString regexAssertPlatform(ws + functionAssertPlatform + ws + "\\(" + ws + quotedString + ".*"); - const QRegularExpression lineAssertPlatform = QRegularExpression(regexAssertPlatform); - - // Assert display checks that test is running on the correct display - const QString functionAssertDisplay(ws + "autoTester" + ws + "\\." + ws + "assertDisplay"); - const QString regexAssertDisplay(ws + functionAssertDisplay + ws + "\\(" + ws + quotedString + ".*"); - const QRegularExpression lineAssertDisplay = QRegularExpression(regexAssertDisplay); - - // Assert CPU checks that test is running on the correct type of CPU - const QString functionAssertCPU(ws + "autoTester" + ws + "\\." + ws + "assertCPU"); - const QString regexAssertCPU(ws + functionAssertCPU + ws + "\\(" + ws + quotedString + ".*"); - const QRegularExpression lineAssertCPU = QRegularExpression(regexAssertCPU); - - // Assert GPU checks that test is running on the correct type of GPU - const QString functionAssertGPU(ws + "autoTester" + ws + "\\." + ws + "assertGPU"); - const QString regexAssertGPU(ws + functionAssertGPU + ws + "\\(" + ws + quotedString + ".*"); - const QRegularExpression lineAssertGPU = QRegularExpression(regexAssertGPU); - // Each step is either of the following forms: // autoTester.addStepSnapshot("Take snapshot"... // autoTester.addStep("Clean up after test"... @@ -514,18 +494,6 @@ ExtractedText Test::getTestScriptLines(QString testFileName) { if (lineContainingTitle.match(line).hasMatch()) { QStringList tokens = line.split('"'); relevantTextFromTest.title = tokens[1]; - } else if (lineAssertPlatform.match(line).hasMatch()) { - QStringList platforms = line.split('"'); - relevantTextFromTest.platform = platforms[1]; - } else if (lineAssertDisplay.match(line).hasMatch()) { - QStringList displays = line.split('"'); - relevantTextFromTest.display = displays[1]; - } else if (lineAssertCPU.match(line).hasMatch()) { - QStringList cpus = line.split('"'); - relevantTextFromTest.cpu = cpus[1]; - } else if (lineAssertGPU.match(line).hasMatch()) { - QStringList gpus = line.split('"'); - relevantTextFromTest.gpu = gpus[1]; } else if (lineStepSnapshot.match(line).hasMatch()) { QStringList tokens = line.split('"'); QString nameOfStep = tokens[1]; @@ -628,50 +596,6 @@ void Test::createMDFile(QString testDirectory) { stream << "## Preconditions" << "\n"; stream << "- In an empty region of a domain with editing rights." << "\n\n"; - // Platform - QStringList platforms = testScriptLines.platform.split(" ");; - stream << "## Platforms\n"; - stream << "Run the test on each of the following platforms\n"; - for (int i = 0; i < platforms.size(); ++i) { - // Note that the platforms parameter may include extra spaces, these appear as empty strings in the list - if (platforms[i] != QString()) { - stream << " - " << platforms[i] << "\n"; - } - } - - // Display - QStringList displays = testScriptLines.display.split(" "); - stream << "## Displays\n"; - stream << "Run the test on each of the following displays\n"; - for (int i = 0; i < displays.size(); ++i) { - // Note that the displays parameter may include extra spaces, these appear as empty strings in the list - if (displays[i] != QString()) { - stream << " - " << displays[i] << "\n"; - } - } - - // CPU - QStringList cpus = testScriptLines.cpu.split(" "); - stream << "## Processors\n"; - stream << "Run the test on each of the following processors\n"; - for (int i = 0; i < cpus.size(); ++i) { - // Note that the cpus parameter may include extra spaces, these appear as empty strings in the list - if (cpus[i] != QString()) { - stream << " - " << cpus[i] << "\n"; - } - } - - // GPU - QStringList gpus = testScriptLines.gpu.split(" "); - stream << "## Graphics Cards\n"; - stream << "Run the test on graphics cards from each of the following vendors\n"; - for (int i = 0; i < gpus.size(); ++i) { - // Note that the gpus parameter may include extra spaces, these appear as empty strings in the list - if (gpus[i] != QString()) { - stream << " - " << gpus[i] << "\n"; - } - } - stream << "## Steps\n"; stream << "Press space bar to advance step by step\n\n"; diff --git a/tools/auto-tester/src/Test.h b/tools/auto-tester/src/Test.h index e69459fef2..975d5dfdaf 100644 --- a/tools/auto-tester/src/Test.h +++ b/tools/auto-tester/src/Test.h @@ -30,10 +30,6 @@ using StepList = std::vector; class ExtractedText { public: QString title; - QString platform; - QString display; - QString cpu; - QString gpu; StepList stepList; }; From 32f2494416a64a210a2da668be9a87485018e029 Mon Sep 17 00:00:00 2001 From: NissimHadar Date: Wed, 9 May 2018 11:58:16 -0700 Subject: [PATCH 4/5] Stores previous user-selected folders. --- tools/auto-tester/src/Test.cpp | 128 +++++++++++++++++++++++---------- tools/auto-tester/src/Test.h | 7 +- 2 files changed, 96 insertions(+), 39 deletions(-) diff --git a/tools/auto-tester/src/Test.cpp b/tools/auto-tester/src/Test.cpp index bdf734897e..1ab934cf63 100644 --- a/tools/auto-tester/src/Test.cpp +++ b/tools/auto-tester/src/Test.cpp @@ -176,33 +176,38 @@ void Test::appendTestResultsToFile(QString testResultsFolderPath, TestFailure te void Test::startTestsEvaluation() { // Get list of JPEG images in folder, sorted by name - pathToTestResultsDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select folder containing the test images", ".", QFileDialog::ShowDirsOnly); - if (pathToTestResultsDirectory == "") { + QString previousSelection = snapshotDirectory; + + snapshotDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select folder containing the test images", + previousSelection, QFileDialog::ShowDirsOnly); + + // If user cancelled then restore previous selection and return + if (snapshotDirectory == "") { + snapshotDirectory = previousSelection; return; } // Quit if test results folder could not be created - if (!createTestResultsFolderPath(pathToTestResultsDirectory)) { + if (!createTestResultsFolderPath(snapshotDirectory)) { return; } // Before any processing - all images are converted to PNGs, as this is the format stored on GitHub - QStringList sortedSnapshotFilenames = createListOfAll_imagesInDirectory("jpg", pathToTestResultsDirectory); + QStringList sortedSnapshotFilenames = createListOfAll_imagesInDirectory("jpg", snapshotDirectory); foreach(QString filename, sortedSnapshotFilenames) { QStringList stringParts = filename.split("."); - copyJPGtoPNG( - pathToTestResultsDirectory + "/" + stringParts[0] + ".jpg", - pathToTestResultsDirectory + "/" + stringParts[0] + ".png" + copyJPGtoPNG(snapshotDirectory + "/" + stringParts[0] + ".jpg", + snapshotDirectory + "/" + stringParts[0] + ".png" ); - QFile::remove(pathToTestResultsDirectory + "/" + stringParts[0] + ".jpg"); + QFile::remove(snapshotDirectory + "/" + stringParts[0] + ".jpg"); } // Create two lists. The first is the test results, the second is the expected images // The expected images are represented as a URL to enable download from GitHub // Images that are in the wrong format are ignored. - QStringList sortedTestResultsFilenames = createListOfAll_imagesInDirectory("png", pathToTestResultsDirectory); + QStringList sortedTestResultsFilenames = createListOfAll_imagesInDirectory("png", snapshotDirectory); QStringList expectedImagesURLs; resultImagesFullFilenames.clear(); @@ -210,7 +215,7 @@ void Test::startTestsEvaluation() { expectedImagesFullFilenames.clear(); foreach(QString currentFilename, sortedTestResultsFilenames) { - QString fullCurrentFilename = pathToTestResultsDirectory + "/" + currentFilename; + QString fullCurrentFilename = snapshotDirectory + "/" + currentFilename; if (isInSnapshotFilenameFormat("png", currentFilename)) { resultImagesFullFilenames << fullCurrentFilename; @@ -230,11 +235,11 @@ void Test::startTestsEvaluation() { QString expectedImageFilename = currentFilename.replace("/", "_").replace(".", "_EI."); expectedImagesFilenames << expectedImageFilename; - expectedImagesFullFilenames << pathToTestResultsDirectory + "/" + expectedImageFilename; + expectedImagesFullFilenames << snapshotDirectory + "/" + expectedImageFilename; } } - autoTester->downloadImages(expectedImagesURLs, pathToTestResultsDirectory, expectedImagesFilenames); + autoTester->downloadImages(expectedImagesURLs, snapshotDirectory, expectedImagesFilenames); } void Test::finishTestsEvaluation(bool interactiveMode, QProgressBar* progressBar) { @@ -295,25 +300,39 @@ void Test::importTest(QTextStream& textStream, const QString& testPathname) { // This script will run all text.js scripts in every applicable sub-folder void Test::createRecursiveScript() { // Select folder to start recursing from - QString topLevelDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select folder that will contain the top level test script", ".", QFileDialog::ShowDirsOnly); - if (topLevelDirectory == "") { + QString previousSelection = testDirectory; + + testDirectory = + QFileDialog::getExistingDirectory(nullptr, "Please select folder that will contain the top level test script", + previousSelection, QFileDialog::ShowDirsOnly); + + // If user cancelled then restore previous selection and return + if (testDirectory == "") { + testDirectory = previousSelection; return; } - createRecursiveScript(topLevelDirectory, true); + createRecursiveScript(testDirectory, true); } // This method creates a `testRecursive.js` script in every sub-folder. void Test::createAllRecursiveScripts() { // Select folder to start recursing from - QString topLevelDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select the root folder for the recursive scripts", ".", QFileDialog::ShowDirsOnly); - if (topLevelDirectory == "") { + QString previousSelection = testDirectory; + + testDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select the root folder for the recursive scripts", + previousSelection, + QFileDialog::ShowDirsOnly); + + // If user cancelled then restore previous selection and return + if (testDirectory == "") { + testDirectory = previousSelection; return; } - createRecursiveScript(topLevelDirectory, false); + createRecursiveScript(testDirectory, false); - QDirIterator it(topLevelDirectory.toStdString().c_str(), QDirIterator::Subdirectories); + QDirIterator it(testDirectory.toStdString().c_str(), QDirIterator::Subdirectories); while (it.hasNext()) { QString directory = it.next(); @@ -416,29 +435,42 @@ void Test::createRecursiveScript(QString topLevelDirectory, bool interactiveMode void Test::createTest() { // Rename files sequentially, as ExpectedResult_00000.jpeg, ExpectedResult_00001.jpg and so on // Any existing expected result images will be deleted - QString imageSourceDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select folder containing the test images", ".", QFileDialog::ShowDirsOnly); - if (imageSourceDirectory == "") { + QString previousSelection = snapshotDirectory; + + snapshotDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select folder containing the test images", + previousSelection, + QFileDialog::ShowDirsOnly); + + // If user cancelled then restore previous selection and return + if (snapshotDirectory == "") { + snapshotDirectory = previousSelection; return; } - QString imageDestinationDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select folder to save the test images", ".", QFileDialog::ShowDirsOnly); - if (imageDestinationDirectory == "") { + previousSelection = testDirectory; + + QString testDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select folder to save the test images", + previousSelection, QFileDialog::ShowDirsOnly); + + // If user cancelled then restore previous selection and return + if (testDirectory == "") { + testDirectory = previousSelection; return; } - QStringList sortedImageFilenames = createListOfAll_imagesInDirectory("jpg", imageSourceDirectory); + QStringList sortedImageFilenames = createListOfAll_imagesInDirectory("jpg", snapshotDirectory); int i = 1; const int maxImages = pow(10, NUM_DIGITS); foreach (QString currentFilename, sortedImageFilenames) { - QString fullCurrentFilename = imageSourceDirectory + "/" + currentFilename; + QString fullCurrentFilename = snapshotDirectory + "/" + currentFilename; if (isInSnapshotFilenameFormat("jpg", currentFilename)) { if (i >= maxImages) { messageBox.critical(0, "Error", "More than " + QString::number(maxImages) + " images not supported"); exit(-1); } QString newFilename = "ExpectedImage_" + QString::number(i - 1).rightJustified(5, '0') + ".png"; - QString fullNewFileName = imageDestinationDirectory + "/" + newFilename; + QString fullNewFileName = testDirectory + "/" + newFilename; try { copyJPGtoPNG(fullCurrentFilename, fullNewFileName); @@ -486,7 +518,7 @@ ExtractedText Test::getTestScriptLines(QString testFileName) { const QRegularExpression lineStepSnapshot = QRegularExpression(regexStepSnapshot); const QString functionAddStepName(ws + "autoTester" + ws + "\\." + ws + "addStep"); - const QString regexStep(ws + functionAddStepName + ws + "\\(" + ws + quotedString + ws + "\\)" + ".*"); + const QString regexStep(ws + functionAddStepName + ws + "\\(" + ws + quotedString + ".*"); const QRegularExpression lineStep = QRegularExpression(regexStep); while (!line.isNull()) { @@ -522,29 +554,43 @@ ExtractedText Test::getTestScriptLines(QString testFileName) { // The folder selected must contain a script named "test.js", the file produced is named "test.md" void Test::createMDFile() { // Folder selection - QString testDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select folder containing the test", ".", QFileDialog::ShowDirsOnly); + QString previousSelection = testDirectory; + + testDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select folder containing the test", previousSelection, + QFileDialog::ShowDirsOnly); + + // If user cancelled then restore previous selection and return if (testDirectory == "") { + testDirectory = previousSelection; return; } createMDFile(testDirectory); + + messageBox.information(0, "Success", "MD file has been created"); } void Test::createAllMDFiles() { // Select folder to start recursing from - QString topLevelDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select the root folder for the MD files", ".", QFileDialog::ShowDirsOnly); - if (topLevelDirectory == "") { + QString previousSelection = testDirectory; + + testDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select the root folder for the MD files", + previousSelection, QFileDialog::ShowDirsOnly); + + // If user cancelled then restore previous selection and return + if (testDirectory == "") { + testDirectory = previousSelection; return; } // First test if top-level folder has a test.js file - const QString testPathname{ topLevelDirectory + "/" + TEST_FILENAME }; + const QString testPathname { testDirectory + "/" + TEST_FILENAME }; QFileInfo fileInfo(testPathname); if (fileInfo.exists()) { - createMDFile(topLevelDirectory); + createMDFile(testDirectory); } - QDirIterator it(topLevelDirectory.toStdString().c_str(), QDirIterator::Subdirectories); + QDirIterator it(testDirectory.toStdString().c_str(), QDirIterator::Subdirectories); while (it.hasNext()) { QString directory = it.next(); @@ -613,13 +659,19 @@ void Test::createMDFile(QString testDirectory) { } void Test::createTestsOutline() { - QString testsRootDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select the tests root folder", ".", QFileDialog::ShowDirsOnly); - if (testsRootDirectory == "") { + QString previousSelection = testDirectory; + + testDirectory = QFileDialog::getExistingDirectory(nullptr, "Please select the tests root folder", previousSelection, + QFileDialog::ShowDirsOnly); + + // If user cancelled then restore previous selection and return + if (testDirectory == "") { + testDirectory = previousSelection; return; } const QString testsOutlineFilename { "testsOutline.md" }; - QString mdFilename(testsRootDirectory + "/" + testsOutlineFilename); + QString mdFilename(testDirectory + "/" + testsOutlineFilename); QFile mdFile(mdFilename); if (!mdFile.open(QIODevice::WriteOnly)) { messageBox.critical(0, "Internal error: " + QString(__FILE__) + ":" + QString::number(__LINE__), "Failed to create file " + mdFilename); @@ -633,10 +685,10 @@ void Test::createTestsOutline() { stream << "Directories with an appended (*) have an automatic test\n\n"; // We need to know our current depth, as this isn't given by QDirIterator - int rootDepth { testsRootDirectory.count('/') }; + int rootDepth { testDirectory.count('/') }; // Each test is shown as the folder name linking to the matching GitHub URL, and the path to the associated test.md file - QDirIterator it(testsRootDirectory.toStdString().c_str(), QDirIterator::Subdirectories); + QDirIterator it(testDirectory.toStdString().c_str(), QDirIterator::Subdirectories); while (it.hasNext()) { QString directory = it.next(); diff --git a/tools/auto-tester/src/Test.h b/tools/auto-tester/src/Test.h index 975d5dfdaf..896fe24fcc 100644 --- a/tools/auto-tester/src/Test.h +++ b/tools/auto-tester/src/Test.h @@ -91,7 +91,12 @@ private: const int NUM_DIGITS { 5 }; const QString EXPECTED_IMAGE_PREFIX { "ExpectedImage_" }; - QString pathToTestResultsDirectory; + // We have to directories to work with. + // The first is the directory containing the test we are working with + // The second contains the snapshots taken for test runs that need to be evaluated + QString testDirectory; + QString snapshotDirectory; + QStringList expectedImagesFilenames; QStringList expectedImagesFullFilenames; QStringList resultImagesFullFilenames; From a9a783588bae2dca4f048fd257253f4cca9dfe71 Mon Sep 17 00:00:00 2001 From: NissimHadar Date: Mon, 14 May 2018 09:25:17 -0700 Subject: [PATCH 5/5] Fixed typo. --- tools/auto-tester/src/Test.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/auto-tester/src/Test.h b/tools/auto-tester/src/Test.h index c7ce5f8893..0fb957d309 100644 --- a/tools/auto-tester/src/Test.h +++ b/tools/auto-tester/src/Test.h @@ -89,7 +89,7 @@ private: const int NUM_DIGITS { 5 }; const QString EXPECTED_IMAGE_PREFIX { "ExpectedImage_" }; - // We have to directories to work with. + // We have two directories to work with. // The first is the directory containing the test we are working with // The second contains the snapshots taken for test runs that need to be evaluated QString testDirectory;