diff --git a/tools/nitpick/src/AWSInterface.cpp b/tools/nitpick/src/AWSInterface.cpp index 4e83460b9e..6dcc255286 100644 --- a/tools/nitpick/src/AWSInterface.cpp +++ b/tools/nitpick/src/AWSInterface.cpp @@ -27,7 +27,10 @@ AWSInterface::AWSInterface(QObject* parent) : QObject(parent) { void AWSInterface::createWebPageFromResults(const QString& testResults, const QString& workingDirectory, QCheckBox* updateAWSCheckBox, - QLineEdit* urlLineEdit) { + QRadioButton* diffImageRadioButton, + QRadioButton* ssimImageRadionButton, + QLineEdit* urlLineEdit +) { _workingDirectory = workingDirectory; // Verify filename is in correct format @@ -52,6 +55,13 @@ void AWSInterface::createWebPageFromResults(const QString& testResults, QString zipFilenameWithoutExtension = zipFilename.split('.')[0]; extractTestFailuresFromZippedFolder(_workingDirectory + "/" + zipFilenameWithoutExtension); + + if (diffImageRadioButton->isChecked()) { + _comparisonImageFilename = "Difference Image.png"; + } else { + _comparisonImageFilename = "SSIM Image.png"; + } + createHTMLFile(); if (updateAWSCheckBox->isChecked()) { @@ -353,7 +363,7 @@ void AWSInterface::openTable(QTextStream& stream, const QString& testResult, con stream << "\t\t\t\t

Test

\n"; stream << "\t\t\t\t

Actual Image

\n"; stream << "\t\t\t\t

Expected Image

\n"; - stream << "\t\t\t\t

Difference Image

\n"; + stream << "\t\t\t\t

Comparison Image

\n"; stream << "\t\t\t\n"; } } @@ -378,12 +388,13 @@ void AWSInterface::createEntry(const int index, const QString& testResult, QText QString folder; bool differenceFileFound; + if (isFailure) { folder = FAILURES_FOLDER; - differenceFileFound = QFile::exists(_htmlFailuresFolder + "/" + resultName + "/Difference Image.png"); + differenceFileFound = QFile::exists(_htmlFailuresFolder + "/" + resultName + "/" + _comparisonImageFilename); } else { folder = SUCCESSES_FOLDER; - differenceFileFound = QFile::exists(_htmlSuccessesFolder + "/" + resultName + "/Difference Image.png"); + differenceFileFound = QFile::exists(_htmlSuccessesFolder + "/" + resultName + "/" + _comparisonImageFilename); } if (textResultsFileFound) { @@ -450,7 +461,7 @@ void AWSInterface::createEntry(const int index, const QString& testResult, QText stream << "\t\t\t\t\n"; if (differenceFileFound) { - stream << "\t\t\t\t\n"; + stream << "\t\t\t\t\n"; } else { stream << "\t\t\t\t

No Image Found

\n"; } @@ -512,12 +523,12 @@ void AWSInterface::updateAWS() { stream << "s3.Bucket('hifi-content').put_object(Bucket='" << AWS_BUCKET << "', Key='" << filename << "/" << "Expected Image.png" << "', Body=data)\n\n"; - if (QFile::exists(_htmlFailuresFolder + "/" + parts[parts.length() - 1] + "/Difference Image.png")) { + if (QFile::exists(_htmlFailuresFolder + "/" + parts[parts.length() - 1] + "/" + _comparisonImageFilename)) { stream << "data = open('" << _workingDirectory << "/" << filename << "/" - << "Difference Image.png" + << _comparisonImageFilename << "', 'rb')\n"; - stream << "s3.Bucket('hifi-content').put_object(Bucket='" << AWS_BUCKET << "', Key='" << filename << "/" << "Difference Image.png" << "', Body=data)\n\n"; + stream << "s3.Bucket('hifi-content').put_object(Bucket='" << AWS_BUCKET << "', Key='" << filename << "/" << _comparisonImageFilename << "', Body=data)\n\n"; } } } @@ -555,12 +566,12 @@ void AWSInterface::updateAWS() { stream << "s3.Bucket('hifi-content').put_object(Bucket='" << AWS_BUCKET << "', Key='" << filename << "/" << "Expected Image.png" << "', Body=data)\n\n"; - if (QFile::exists(_htmlSuccessesFolder + "/" + parts[parts.length() - 1] + "/Difference Image.png")) { + if (QFile::exists(_htmlSuccessesFolder + "/" + parts[parts.length() - 1] + "/" + _comparisonImageFilename)) { stream << "data = open('" << _workingDirectory << "/" << filename << "/" - << "Difference Image.png" + << _comparisonImageFilename << "', 'rb')\n"; - stream << "s3.Bucket('hifi-content').put_object(Bucket='" << AWS_BUCKET << "', Key='" << filename << "/" << "Difference Image.png" << "', Body=data)\n\n"; + stream << "s3.Bucket('hifi-content').put_object(Bucket='" << AWS_BUCKET << "', Key='" << filename << "/" << _comparisonImageFilename << "', Body=data)\n\n"; } } } diff --git a/tools/nitpick/src/AWSInterface.h b/tools/nitpick/src/AWSInterface.h index d95b8ecf2f..77d500fa7c 100644 --- a/tools/nitpick/src/AWSInterface.h +++ b/tools/nitpick/src/AWSInterface.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include "BusyWindow.h" @@ -28,6 +29,8 @@ public: void createWebPageFromResults(const QString& testResults, const QString& workingDirectory, QCheckBox* updateAWSCheckBox, + QRadioButton* diffImageRadioButton, + QRadioButton* ssimImageRadionButton, QLineEdit* urlLineEdit); void extractTestFailuresFromZippedFolder(const QString& folderName); @@ -67,6 +70,9 @@ private: QString AWS_BUCKET{ "hifi-qa" }; QLineEdit* _urlLineEdit; + + + QString _comparisonImageFilename; }; #endif // hifi_AWSInterface_h \ No newline at end of file diff --git a/tools/nitpick/src/ImageComparer.cpp b/tools/nitpick/src/ImageComparer.cpp index b9d556e544..7e3e6eaf63 100644 --- a/tools/nitpick/src/ImageComparer.cpp +++ b/tools/nitpick/src/ImageComparer.cpp @@ -12,17 +12,9 @@ #include -ImageComparer::ImageComparer() { - _ssimResults = new SSIMResults(); -} - -ImageComparer::~ImageComparer() { - delete _ssimResults; -} - // Computes SSIM - see https://en.wikipedia.org/wiki/Structural_similarity // The value is computed for the luminance component and the average value is returned -double ImageComparer::compareImages(const QImage& resultImage, const QImage& expectedImage) const { +void ImageComparer::compareImages(const QImage& resultImage, const QImage& expectedImage) { const int L = 255; // (2^number of bits per pixel) - 1 const double K1 { 0.01 }; @@ -47,8 +39,13 @@ double ImageComparer::compareImages(const QImage& resultImage, const QImage& exp double p[WIN_SIZE * WIN_SIZE]; double q[WIN_SIZE * WIN_SIZE]; + _ssimResults.results.clear(); + int windowCounter{ 0 }; double ssim{ 0.0 }; + double min { 1.0 }; + double max { -1.0 }; + while (x < expectedImage.width()) { int lastX = x + WIN_SIZE - 1; if (lastX > expectedImage.width() - 1) { @@ -104,8 +101,13 @@ double ImageComparer::compareImages(const QImage& resultImage, const QImage& exp double numerator = (2.0 * mP * mQ + c1) * (2.0 * sigPQ + c2); double denominator = (mP * mP + mQ * mQ + c1) * (sigsqP + sigsqQ + c2); - _ssimResults->results.push_back(numerator / denominator); - ssim += numerator / denominator; + double value { numerator / denominator }; + _ssimResults.results.push_back(value); + ssim += value; + + if (value < min) min = value; + if (value > max) max = value; + ++windowCounter; y += WIN_SIZE; @@ -115,12 +117,17 @@ double ImageComparer::compareImages(const QImage& resultImage, const QImage& exp y = 0; } - _ssimResults->width = (int)(expectedImage.width() / WIN_SIZE); - _ssimResults->height = (int)(expectedImage.height() / WIN_SIZE); - - return ssim / windowCounter; + _ssimResults.width = (int)(expectedImage.width() / WIN_SIZE); + _ssimResults.height = (int)(expectedImage.height() / WIN_SIZE); + _ssimResults.min = min; + _ssimResults.max = max; + _ssimResults.ssim = ssim / windowCounter; }; -SSIMResults* ImageComparer::getSSIMResults() { +double ImageComparer::getSSIMValue() { + return _ssimResults.ssim; +} + +SSIMResults ImageComparer::getSSIMResults() { return _ssimResults; } diff --git a/tools/nitpick/src/ImageComparer.h b/tools/nitpick/src/ImageComparer.h index 5bea8151ec..fc14dab94d 100644 --- a/tools/nitpick/src/ImageComparer.h +++ b/tools/nitpick/src/ImageComparer.h @@ -17,14 +17,13 @@ class ImageComparer { public: - ImageComparer(); - ~ImageComparer(); + void compareImages(const QImage& resultImage, const QImage& expectedImage); + double getSSIMValue(); - double compareImages(const QImage& resultImage, const QImage& expectedImage) const; - SSIMResults* getSSIMResults(); + SSIMResults getSSIMResults(); private: - SSIMResults* _ssimResults; + SSIMResults _ssimResults; }; #endif // hifi_ImageComparer_h diff --git a/tools/nitpick/src/MismatchWindow.cpp b/tools/nitpick/src/MismatchWindow.cpp index 7649929551..fd5df0dd4e 100644 --- a/tools/nitpick/src/MismatchWindow.cpp +++ b/tools/nitpick/src/MismatchWindow.cpp @@ -21,7 +21,7 @@ MismatchWindow::MismatchWindow(QWidget *parent) : QDialog(parent) { diffImage->setScaledContents(true); } -QPixmap MismatchWindow::computeDiffPixmap(QImage expectedImage, QImage resultImage) { +QPixmap MismatchWindow::computeDiffPixmap(const QImage& expectedImage, const QImage& resultImage) { // Create an empty difference image if the images differ in size if (expectedImage.height() != resultImage.height() || expectedImage.width() != resultImage.width()) { return QPixmap(); @@ -60,7 +60,7 @@ QPixmap MismatchWindow::computeDiffPixmap(QImage expectedImage, QImage resultIma return resultPixmap; } -void MismatchWindow::setTestResult(TestResult testResult) { +void MismatchWindow::setTestResult(const TestResult& testResult) { errorLabel->setText("Similarity: " + QString::number(testResult._error)); imagePath->setText("Path to test: " + testResult._pathname); @@ -100,34 +100,35 @@ QPixmap MismatchWindow::getComparisonImage() { return _diffPixmap; } -QPixmap MismatchWindow::getSSIMResultsImage(SSIMResults* ssimResults) { +QPixmap MismatchWindow::getSSIMResultsImage(const SSIMResults& ssimResults) { // This is an optimization, as QImage.setPixel() is embarrassingly slow const int ELEMENT_SIZE { 8 }; - unsigned char* buffer = new unsigned char[(ssimResults->height * ELEMENT_SIZE) * (ssimResults->width * ELEMENT_SIZE ) * 3]; + const int WIDTH{ ssimResults.width * ELEMENT_SIZE }; + const int HEIGHT{ ssimResults.height * ELEMENT_SIZE }; + + unsigned char* buffer = new unsigned char[WIDTH * HEIGHT * 3]; - // loop over each SSIM result (a double in [-1.0 .. 1.0] - int i { 0 }; - for (int y = 0; y < ssimResults->height; ++y) { - for (int x = 0; x < ssimResults->width; ++x) { - ////QRgb pixelP = expectedImage.pixel(QPoint(x, y)); - - ////// Convert to luminance - ////double p = R_Y * qRed(pixelP) + G_Y * qGreen(pixelP) + B_Y * qBlue(pixelP); - - ////// The intensity value is modified to increase the brightness of the displayed image - ////double absoluteDifference = fabs(p - q) / 255.0; - ////double modifiedDifference = sqrt(absoluteDifference); - - ////int difference = (int)(modifiedDifference * 255.0); - - ////buffer[3 * (x + y * expectedImage.width()) + 0] = difference; - ////buffer[3 * (x + y * expectedImage.width()) + 1] = difference; - ////buffer[3 * (x + y * expectedImage.width()) + 2] = difference; - - ++i; + // loop over each SSIM result + for (int y = 0; y < ssimResults.height; ++y) { + for (int x = 0; x < ssimResults.width; ++x) { + double scaledResult = (ssimResults.results[x * ssimResults.height + y] + 1.0) / (2.0); + //double scaledResult = (ssimResults.results[x * ssimResults.height + y] - ssimResults.min) / (ssimResults.max - ssimResults.min); + // Create a square + for (int yy = 0; yy < ELEMENT_SIZE; ++yy) { + for (int xx = 0; xx < ELEMENT_SIZE; ++xx) { + buffer[(xx + yy * WIDTH + x * ELEMENT_SIZE + y * WIDTH * ELEMENT_SIZE) * 3 + 0] = 255 * (1.0 - scaledResult); // R + buffer[(xx + yy * WIDTH + x * ELEMENT_SIZE + y * WIDTH * ELEMENT_SIZE) * 3 + 1] = 255 * scaledResult; // G + buffer[(xx + yy * WIDTH + x * ELEMENT_SIZE + y * WIDTH * ELEMENT_SIZE) * 3 + 2] = 0; // B + } + } } } - return QPixmap(); + QImage image(buffer, WIDTH, HEIGHT, QImage::Format_RGB888); + QPixmap pixmap = QPixmap::fromImage(image); + + delete[] buffer; + + return pixmap; } diff --git a/tools/nitpick/src/MismatchWindow.h b/tools/nitpick/src/MismatchWindow.h index d80e371ba0..116d35dfc5 100644 --- a/tools/nitpick/src/MismatchWindow.h +++ b/tools/nitpick/src/MismatchWindow.h @@ -20,14 +20,14 @@ class MismatchWindow : public QDialog, public Ui::MismatchWindow { public: MismatchWindow(QWidget *parent = Q_NULLPTR); - void setTestResult(TestResult testResult); + void setTestResult(const TestResult& testResult); UserResponse getUserResponse() { return _userResponse; } - QPixmap computeDiffPixmap(QImage expectedImage, QImage resultImage); + QPixmap computeDiffPixmap(const QImage& expectedImage, const QImage& resultImage); QPixmap getComparisonImage(); - QPixmap getSSIMResultsImage(SSIMResults* ssimResults); + QPixmap getSSIMResultsImage(const SSIMResults& ssimResults); private slots: void on_passTestButton_clicked(); diff --git a/tools/nitpick/src/Nitpick.cpp b/tools/nitpick/src/Nitpick.cpp index e17ea94634..c07a76fc58 100644 --- a/tools/nitpick/src/Nitpick.cpp +++ b/tools/nitpick/src/Nitpick.cpp @@ -148,10 +148,6 @@ void Nitpick::on_tabWidget_currentChanged(int index) { } } -void Nitpick::on_evaluateTestsPushbutton_clicked() { - _test->startTestsEvaluation(false, false); -} - void Nitpick::on_createRecursiveScriptPushbutton_clicked() { _test->createRecursiveScript(); } @@ -242,6 +238,10 @@ void Nitpick::on_showTaskbarPushbutton_clicked() { #endif } +void Nitpick::on_evaluateTestsPushbutton_clicked() { + _test->startTestsEvaluation(false, false); +} + void Nitpick::on_closePushbutton_clicked() { exit(0); } @@ -255,7 +255,7 @@ void Nitpick::on_createXMLScriptRadioButton_clicked() { } void Nitpick::on_createWebPagePushbutton_clicked() { - _test->createWebPage(_ui.updateAWSCheckBox, _ui.awsURLLineEdit); + _test->createWebPage(_ui.updateAWSCheckBox, _ui.diffImageRadioButton, _ui.ssimImageRadioButton, _ui.awsURLLineEdit); } void Nitpick::downloadFile(const QUrl& url) { diff --git a/tools/nitpick/src/Nitpick.h b/tools/nitpick/src/Nitpick.h index 80fef934d6..3095a14c05 100644 --- a/tools/nitpick/src/Nitpick.h +++ b/tools/nitpick/src/Nitpick.h @@ -56,7 +56,6 @@ private slots: void on_tabWidget_currentChanged(int index); - void on_evaluateTestsPushbutton_clicked(); void on_createRecursiveScriptPushbutton_clicked(); void on_createAllRecursiveScriptsPushbutton_clicked(); void on_createTestsPushbutton_clicked(); @@ -82,6 +81,8 @@ private slots: void on_hideTaskbarPushbutton_clicked(); void on_showTaskbarPushbutton_clicked(); + void on_evaluateTestsPushbutton_clicked(); + void on_createPythonScriptRadioButton_clicked(); void on_createXMLScriptRadioButton_clicked(); diff --git a/tools/nitpick/src/Test.cpp b/tools/nitpick/src/Test.cpp index a1f1bf92eb..7269fb3f02 100644 --- a/tools/nitpick/src/Test.cpp +++ b/tools/nitpick/src/Test.cpp @@ -89,25 +89,25 @@ int Test::compareImageLists() { QMessageBox::critical(0, "Internal error: " + QString(__FILE__) + ":" + QString::number(__LINE__), "Images are not the same size"); similarityIndex = -100.0; } else { - similarityIndex = _imageComparer.compareImages(resultImage, expectedImage); + _imageComparer.compareImages(resultImage, expectedImage); + similarityIndex = _imageComparer.getSSIMValue(); } TestResult testResult = TestResult{ (float)similarityIndex, _expectedImagesFullFilenames[i].left(_expectedImagesFullFilenames[i].lastIndexOf("/") + 1), // path to the test (including trailing /) QFileInfo(_expectedImagesFullFilenames[i].toStdString().c_str()).fileName(), // filename of expected image - QFileInfo(_resultImagesFullFilenames[i].toStdString().c_str()).fileName() // filename of result image + QFileInfo(_resultImagesFullFilenames[i].toStdString().c_str()).fileName(), // filename of result image + _imageComparer.getSSIMResults() // results of SSIM algoritm }; _mismatchWindow.setTestResult(testResult); - QPixmap ssimResultsPixMap = _mismatchWindow.getSSIMResultsImage(_imageComparer.getSSIMResults()); - if (similarityIndex < THRESHOLD) { ++numberOfFailures; if (!isInteractiveMode) { - appendTestResultsToFile(testResult, _mismatchWindow.getComparisonImage(), ssimResultsPixMap, true); + appendTestResultsToFile(testResult, _mismatchWindow.getComparisonImage(), _mismatchWindow.getSSIMResultsImage(testResult._ssimResults), true); } else { _mismatchWindow.exec(); @@ -115,7 +115,7 @@ int Test::compareImageLists() { case USER_RESPONSE_PASS: break; case USE_RESPONSE_FAIL: - appendTestResultsToFile(testResult, _mismatchWindow.getComparisonImage(), ssimResultsPixMap, true); + appendTestResultsToFile(testResult, _mismatchWindow.getComparisonImage(), _mismatchWindow.getSSIMResultsImage(testResult._ssimResults), true); break; case USER_RESPONSE_ABORT: keepOn = false; @@ -126,7 +126,7 @@ int Test::compareImageLists() { } } } else { - appendTestResultsToFile(testResult, _mismatchWindow.getComparisonImage(), ssimResultsPixMap, false); + appendTestResultsToFile(testResult, _mismatchWindow.getComparisonImage(), _mismatchWindow.getSSIMResultsImage(testResult._ssimResults), false); } _progressBar->setValue(i); @@ -218,15 +218,10 @@ void Test::appendTestResultsToFile(const TestResult& testResult, const QPixmap& exit(-1); } - // Create the SSIM results image - sourceFile = testResult._pathname + testResult._actualImageFilename; - destinationFile = resultFolderPath + "/" + "SSIM results.png"; - if (!QFile::copy(sourceFile, destinationFile)) { - QMessageBox::critical(0, "Internal error: " + QString(__FILE__) + ":" + QString::number(__LINE__), "Failed to copy " + sourceFile + " to " + destinationFile); - exit(-1); - } - comparisonImage.save(resultFolderPath + "/" + "Difference Image.png"); + + // Save the SSIM results image + ssimResultsImage.save(resultFolderPath + "/" + "SSIM Image.png"); } void::Test::appendTestResultsToFile(QString testResultFilename, bool hasFailed) { @@ -1105,7 +1100,12 @@ void Test::setTestRailCreateMode(TestRailCreateMode testRailCreateMode) { _testRailCreateMode = testRailCreateMode; } -void Test::createWebPage(QCheckBox* updateAWSCheckBox, QLineEdit* urlLineEdit) { +void Test::createWebPage( + QCheckBox* updateAWSCheckBox, + QRadioButton* diffImageRadioButton, + QRadioButton* ssimImageRadionButton, + QLineEdit* urlLineEdit +) { QString testResults = QFileDialog::getOpenFileName(nullptr, "Please select the zipped test results to update from", nullptr, "Zipped Test Results (TestResults--*.zip)"); if (testResults.isNull()) { @@ -1122,5 +1122,12 @@ void Test::createWebPage(QCheckBox* updateAWSCheckBox, QLineEdit* urlLineEdit) { _awsInterface = new AWSInterface; } - _awsInterface->createWebPageFromResults(testResults, workingDirectory, updateAWSCheckBox, urlLineEdit); + _awsInterface->createWebPageFromResults( + testResults, + workingDirectory, + updateAWSCheckBox, + diffImageRadioButton, + ssimImageRadionButton, + urlLineEdit + ); } \ No newline at end of file diff --git a/tools/nitpick/src/Test.h b/tools/nitpick/src/Test.h index 752dcbc5af..8753b9fcda 100644 --- a/tools/nitpick/src/Test.h +++ b/tools/nitpick/src/Test.h @@ -102,7 +102,11 @@ public: void setTestRailCreateMode(TestRailCreateMode testRailCreateMode); - void createWebPage(QCheckBox* updateAWSCheckBox, QLineEdit* urlLineEdit); + void createWebPage( + QCheckBox* updateAWSCheckBox, + QRadioButton* diffImageRadioButton, + QRadioButton* ssimImageRadionButton, + QLineEdit* urlLineEdit); private: QProgressBar* _progressBar; diff --git a/tools/nitpick/src/common.h b/tools/nitpick/src/common.h index ac776995b7..eb228ff2b3 100644 --- a/tools/nitpick/src/common.h +++ b/tools/nitpick/src/common.h @@ -13,19 +13,35 @@ #include #include +class SSIMResults { +public: + int width; + int height; + std::vector results; + double ssim; + + // Used for scaling + double min; + double max; +}; + class TestResult { public: - TestResult(float error, QString pathname, QString expectedImageFilename, QString actualImageFilename) : + TestResult(float error, const QString& pathname, const QString& expectedImageFilename, const QString& actualImageFilename, const SSIMResults& ssimResults) : _error(error), _pathname(pathname), _expectedImageFilename(expectedImageFilename), - _actualImageFilename(actualImageFilename) + _actualImageFilename(actualImageFilename), + _ssimResults(ssimResults) {} double _error; + QString _pathname; QString _expectedImageFilename; QString _actualImageFilename; + + SSIMResults _ssimResults; }; enum UserResponse { @@ -40,11 +56,4 @@ const double R_Y = 0.212655f; const double G_Y = 0.715158f; const double B_Y = 0.072187f; -class SSIMResults { -public: - int width; - int height; - std::vector results; -}; - #endif // hifi_common_h \ No newline at end of file diff --git a/tools/nitpick/ui/Nitpick.ui b/tools/nitpick/ui/Nitpick.ui index 47471522db..1857a2118f 100644 --- a/tools/nitpick/ui/Nitpick.ui +++ b/tools/nitpick/ui/Nitpick.ui @@ -43,7 +43,7 @@ - 0 + 5 @@ -760,7 +760,7 @@ 190 - 180 + 200 131 20 @@ -776,7 +776,7 @@ 330 - 170 + 190 181 51 @@ -889,8 +889,8 @@ - 270 - 30 + 370 + 20 160 51 @@ -922,6 +922,38 @@ + + + + 260 + 50 + 95 + 20 + + + + Diff Image + + + false + + + + + + 260 + 30 + 95 + 20 + + + + SSIM Image + + + true + + groupBox updateTestRailRunResultsPushbutton