From ea8fccc76d8c3f8f3860ffda87fb8a044b59aa65 Mon Sep 17 00:00:00 2001 From: Michael Migliore Date: Fri, 3 Jan 2025 14:55:36 +0100 Subject: [PATCH 1/5] Refactor image::compare --- application/F3DStarter.cxx | 2 +- examples/libf3d/cpp/render-image/check.cxx | 3 +- examples/libf3d/python/img-cmp/img-cmp.py | 4 +- library/public/image.h | 11 ++---- library/src/image.cxx | 43 +++++----------------- library/testing/TestSDKHelpers.h | 5 ++- library/testing/TestSDKImage.cxx | 41 ++++++--------------- python/testing/test_image_compare.py | 2 +- python/testing/test_scene.py | 4 +- 9 files changed, 34 insertions(+), 81 deletions(-) diff --git a/application/F3DStarter.cxx b/application/F3DStarter.cxx index a8739c5409..f2ea681015 100644 --- a/application/F3DStarter.cxx +++ b/application/F3DStarter.cxx @@ -1025,7 +1025,7 @@ int F3DStarter::Start(int argc, char** argv) f3d::image diff; double error; const double& threshold = this->Internals->AppOptions.RefThreshold; - if (!img.compare(ref, threshold, error)) + if (img.compare(ref) > threshold) { if (output.empty()) { diff --git a/examples/libf3d/cpp/render-image/check.cxx b/examples/libf3d/cpp/render-image/check.cxx index 1fe8284f81..c5f72d85f5 100644 --- a/examples/libf3d/cpp/render-image/check.cxx +++ b/examples/libf3d/cpp/render-image/check.cxx @@ -13,6 +13,5 @@ int main(int argc, char** argv) f3d::image img1(argv[2]); // Compare them - double error; - return img0.compare(img1, 0.05, error) ? EXIT_SUCCESS : EXIT_FAILURE; + return img0.compare(img1) <= 0.05 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/examples/libf3d/python/img-cmp/img-cmp.py b/examples/libf3d/python/img-cmp/img-cmp.py index 093abdcb2a..58b82fb5d2 100644 --- a/examples/libf3d/python/img-cmp/img-cmp.py +++ b/examples/libf3d/python/img-cmp/img-cmp.py @@ -11,9 +11,7 @@ diff = f3d.Image() error = 0.0 - result = img_0.compare(img_1, 0.05, diff, error) - - if result: + if img_0.compare(img_1) <= 0.05: print("Images are identical") else: print("Images are different") diff --git a/library/public/image.h b/library/public/image.h index 1165c5a504..dddaab8b82 100644 --- a/library/public/image.h +++ b/library/public/image.h @@ -135,23 +135,20 @@ class F3D_EXPORT image ///@} /** - * Compare current image to a reference using the provided threshold. - * If the comparison fails, ie. error is higher than the threshold, - * this outputs the resulting diff and error and return false, - * return true otherwise. + * Compare current image to a reference. * The error is minimum between Minkownski and Wasserstein distance * on a SSIM computation, as specified in VTK. * Please note, due to possible arithmetic imprecision in the SSIM computation - * using a threshold of zero may return false with identical images. + * a non-zero value can be returned with identical images. * Depending on the VTK version, another comparison algorithm may be used. - * Threshold should be in range [0, 1[, this returns false otherwise. + * Error value meaning is described below: * 1e-14: Pixel perfect comparison. * 0.04: Visually indistinguishable. * 0.1: Small visible difference. * 0.5: Comparable images. * 1.0: Different type, size or number of components */ - bool compare(const image& reference, double threshold, double& error) const; + double compare(const image& reference) const; /** * Save an image to the provided file path, used as is, in the specified format. diff --git a/library/src/image.cxx b/library/src/image.cxx index c0a68d199d..9fc2bfc4cc 100644 --- a/library/src/image.cxx +++ b/library/src/image.cxx @@ -344,39 +344,28 @@ void* image::getContent() const } //---------------------------------------------------------------------------- -bool image::compare(const image& reference, double threshold, double& error) const +double image::compare(const image& reference) const { - // Sanity check for threshold - if (threshold < 0 || threshold >= 1) - { - error = 1; - return false; - } - ChannelType type = this->getChannelType(); if (type != reference.getChannelType()) { - error = 1; - return false; + return 1.0; } unsigned int count = this->getChannelCount(); if (count != reference.getChannelCount()) { - error = 1; - return false; + return 1.0; } if (this->getWidth() != reference.getWidth() || this->getHeight() != reference.getHeight()) { - error = 1; - return false; + return 1.0; } if (this->getWidth() == 0 && this->getHeight() == 0) { - error = 0; - return true; + return 0.0; } #if VTK_VERSION_NUMBER >= VTK_VERSION_CHECK(9, 3, 20240729) @@ -406,39 +395,27 @@ bool image::compare(const image& reference, double threshold, double& error) con // Thanks to the checks above, this is always true assert(scalars != nullptr); - double unused; + double error, unused; vtkImageSSIM::ComputeErrorMetrics(scalars, error, unused); - return error <= threshold; + return error; #else - threshold *= 1000; - vtkNew imDiff; imDiff->SetThreshold(0); imDiff->SetInputData(this->Internals->Image); imDiff->SetImageData(reference.Internals->Image); imDiff->UpdateInformation(); - error = imDiff->GetThresholdedError(); - - if (error <= threshold) - { - imDiff->Update(); - error = imDiff->GetThresholdedError(); - } - - bool ret = error <= threshold; - error /= 1000; - return ret; + double error = imDiff->GetThresholdedError(); + return error / 1000.0; #endif } //---------------------------------------------------------------------------- bool image::operator==(const image& reference) const { - double error; // XXX: We do not use 0 because even with identical images, rounding error, arithmetic imprecision // or architecture issue may cause the value to not be 0. See: // https://develop.openfoam.com/Development/openfoam/-/issues/2958 - return this->compare(reference, 1e-14, error); + return this->compare(reference) <= 1e-14; } //---------------------------------------------------------------------------- diff --git a/library/testing/TestSDKHelpers.h b/library/testing/TestSDKHelpers.h index f156bb7b87..09980dd224 100644 --- a/library/testing/TestSDKHelpers.h +++ b/library/testing/TestSDKHelpers.h @@ -35,9 +35,10 @@ static bool RenderTest(const f3d::image& img, const std::string& baselinePath, } f3d::image reference(baseline); - double error; - if (!img.compare(reference, threshold, error)) + double error = img.compare(reference); + + if (error > threshold) { std::cerr << "Current rendering difference with reference image " << baseline << " : " << error << " is higher than the threshold of " << threshold << std::endl; diff --git a/library/testing/TestSDKImage.cxx b/library/testing/TestSDKImage.cxx index 6cbc7554b2..7b022de275 100644 --- a/library/testing/TestSDKImage.cxx +++ b/library/testing/TestSDKImage.cxx @@ -139,10 +139,7 @@ int TestSDKImage(int argc, char* argv[]) f3d::image baseline(testingDir + "/baselines/TestSDKImage.png"); if (generated != baseline) { - double error; - generated.compare(baseline, 0, error); - - std::cerr << "Generated image is different from the png baseline: " << error << std::endl; + std::cerr << "Generated image is different from the png baseline: " << generated.compare(baseline) << std::endl; return EXIT_FAILURE; } @@ -151,10 +148,7 @@ int TestSDKImage(int argc, char* argv[]) f3d::image baselineTIF(testingDir + "/baselines/TestSDKImage.tif"); if (generated != baselineTIF) { - double error; - generated.compare(baselineTIF, 0, error); - - std::cerr << "Generated image is different from the tif baseline: " << error << std::endl; + std::cerr << "Generated image is different from the tif baseline: " << generated.compare(baselineTIF) << std::endl; return EXIT_FAILURE; }*/ @@ -171,10 +165,7 @@ int TestSDKImage(int argc, char* argv[]) f3d::image baseline16(testingDir + "/baselines/TestSDKImage16.png"); if (generated16 != baseline16) { - double error; - generated16.compare(baseline16, 0, error); - - std::cerr << "generated short image is different from the baseline: " << error << std::endl; + std::cerr << "generated short image is different from the baseline: " << generated16.compare(baseline16) << std::endl; return EXIT_FAILURE; } @@ -183,10 +174,7 @@ int TestSDKImage(int argc, char* argv[]) f3d::image baseline16TIF(testingDir + "/baselines/TestSDKImage16.tif"); if (generated16 != baseline16TIF) { - double error; - generated16.compare(baseline16TIF, 0, error); - - std::cerr << "generated short image is different from the TIF baseline: " << error << std::endl; + std::cerr << "generated short image is different from the TIF baseline: " << generated16.compare(baseline16TIF) << std::endl; return EXIT_FAILURE; }*/ @@ -204,10 +192,7 @@ int TestSDKImage(int argc, char* argv[]) if (generated32 != baseline32) { - double error; - generated32.compare(baseline32, 0, error); - - std::cerr << "generated float image is different from the baseline: " << error << std::endl; + std::cerr << "generated float image is different from the baseline: " << generated32.compare(baseline32) << std::endl; return EXIT_FAILURE; } #endif // F3D_SSIM_COMPARE @@ -300,22 +285,18 @@ int TestSDKImage(int argc, char* argv[]) } // Test image::compare dedicated code paths - double error; - test("compare images with different channel types", - !generated.compare(generated16, 0, error) && error == 1.); + test("compare images with different channel types", generated.compare(generated16) == 1.); f3d::image generatedCount(width, height, channels + 1); - test("compare images with different channel count", - !generated.compare(generatedCount, 0, error) && error == 1.); + test("compare images with different channel count", generated.compare(generatedCount) == 1.); f3d::image generatedSize(width + 1, height, channels); - test("compare images with different size", - !generated.compare(generatedSize, 0, error) && error == 1.); + test("compare images with different size", generated.compare(generatedSize) == 1.); f3d::image empty(0, 0, 0); - test("compare empty images", empty.compare(empty, 0, error) && error == 0.); - test("compare with negative threshold", !empty.compare(empty, -1, error) && error == 1.); - test("compare with threshold == 1", !empty.compare(empty, 1, error) && error == 1.); + test("compare empty images", empty.compare(empty) == 0.); + test("compare with negative threshold", empty.compare(empty) == 1.); + test("compare with threshold == 1", empty.compare(empty) == 1.); return test.result(); } diff --git a/python/testing/test_image_compare.py b/python/testing/test_image_compare.py index e7fa2cf87b..f3d85e0209 100644 --- a/python/testing/test_image_compare.py +++ b/python/testing/test_image_compare.py @@ -26,4 +26,4 @@ def test_compare_with_file(): error = 0.0 - assert img.compare(f3d.Image(reference), 0.05, error) + assert img.compare(f3d.Image(reference)) <= 0.05 diff --git a/python/testing/test_scene.py b/python/testing/test_scene.py index f28e6d0f72..cd69f22670 100644 --- a/python/testing/test_scene.py +++ b/python/testing/test_scene.py @@ -26,7 +26,7 @@ def test_scene_memory(): error = 0.0 - assert img.compare(f3d.Image(reference), 0.05, error) + assert img.compare(f3d.Image(reference)) < 0.05 def test_scene(): @@ -54,4 +54,4 @@ def test_scene(): error = 0.0 - assert img.compare(f3d.Image(reference), 0.05, error) + assert img.compare(f3d.Image(reference)) < 0.05 From b76a66710655c8bb473219648c82d9079feb5af1 Mon Sep 17 00:00:00 2001 From: Michael Migliore Date: Fri, 3 Jan 2025 14:57:57 +0100 Subject: [PATCH 2/5] format --- library/testing/TestSDKImage.cxx | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/library/testing/TestSDKImage.cxx b/library/testing/TestSDKImage.cxx index 7b022de275..85efc9a61d 100644 --- a/library/testing/TestSDKImage.cxx +++ b/library/testing/TestSDKImage.cxx @@ -139,7 +139,8 @@ int TestSDKImage(int argc, char* argv[]) f3d::image baseline(testingDir + "/baselines/TestSDKImage.png"); if (generated != baseline) { - std::cerr << "Generated image is different from the png baseline: " << generated.compare(baseline) << std::endl; + std::cerr << "Generated image is different from the png baseline: " + << generated.compare(baseline) << std::endl; return EXIT_FAILURE; } @@ -148,7 +149,8 @@ int TestSDKImage(int argc, char* argv[]) f3d::image baselineTIF(testingDir + "/baselines/TestSDKImage.tif"); if (generated != baselineTIF) { - std::cerr << "Generated image is different from the tif baseline: " << generated.compare(baselineTIF) << std::endl; + std::cerr << "Generated image is different from the tif baseline: " + << generated.compare(baselineTIF) << std::endl; return EXIT_FAILURE; }*/ @@ -165,7 +167,8 @@ int TestSDKImage(int argc, char* argv[]) f3d::image baseline16(testingDir + "/baselines/TestSDKImage16.png"); if (generated16 != baseline16) { - std::cerr << "generated short image is different from the baseline: " << generated16.compare(baseline16) << std::endl; + std::cerr << "generated short image is different from the baseline: " + << generated16.compare(baseline16) << std::endl; return EXIT_FAILURE; } @@ -174,7 +177,8 @@ int TestSDKImage(int argc, char* argv[]) f3d::image baseline16TIF(testingDir + "/baselines/TestSDKImage16.tif"); if (generated16 != baseline16TIF) { - std::cerr << "generated short image is different from the TIF baseline: " << generated16.compare(baseline16TIF) << std::endl; + std::cerr << "generated short image is different from the TIF baseline: " + << generated16.compare(baseline16TIF) << std::endl; return EXIT_FAILURE; }*/ @@ -192,7 +196,8 @@ int TestSDKImage(int argc, char* argv[]) if (generated32 != baseline32) { - std::cerr << "generated float image is different from the baseline: " << generated32.compare(baseline32) << std::endl; + std::cerr << "generated float image is different from the baseline: " + << generated32.compare(baseline32) << std::endl; return EXIT_FAILURE; } #endif // F3D_SSIM_COMPARE From 551c28a60c3d73117bcccb6df2ac3e98155bff13 Mon Sep 17 00:00:00 2001 From: Michael Migliore Date: Fri, 3 Jan 2025 15:10:19 +0100 Subject: [PATCH 3/5] fix --- application/F3DStarter.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/application/F3DStarter.cxx b/application/F3DStarter.cxx index f2ea681015..5ef2b704b6 100644 --- a/application/F3DStarter.cxx +++ b/application/F3DStarter.cxx @@ -1023,9 +1023,9 @@ int F3DStarter::Start(int argc, char** argv) f3d::image img = window.renderToImage(this->Internals->AppOptions.NoBackground); f3d::image ref(reference); f3d::image diff; - double error; + double error = img.compare(ref); const double& threshold = this->Internals->AppOptions.RefThreshold; - if (img.compare(ref) > threshold) + if (error > threshold) { if (output.empty()) { From 550572e5634b6c9c62dc3f3f171a1bef21142b7b Mon Sep 17 00:00:00 2001 From: Michael Migliore Date: Fri, 3 Jan 2025 16:11:23 +0100 Subject: [PATCH 4/5] fix --- library/testing/TestSDKImage.cxx | 2 -- 1 file changed, 2 deletions(-) diff --git a/library/testing/TestSDKImage.cxx b/library/testing/TestSDKImage.cxx index 85efc9a61d..f7b7d7f558 100644 --- a/library/testing/TestSDKImage.cxx +++ b/library/testing/TestSDKImage.cxx @@ -300,8 +300,6 @@ int TestSDKImage(int argc, char* argv[]) f3d::image empty(0, 0, 0); test("compare empty images", empty.compare(empty) == 0.); - test("compare with negative threshold", empty.compare(empty) == 1.); - test("compare with threshold == 1", empty.compare(empty) == 1.); return test.result(); } From 5450f0b0a1fe26a458f895f53bba988386e5c461 Mon Sep 17 00:00:00 2001 From: Michael Migliore Date: Fri, 3 Jan 2025 16:49:00 +0100 Subject: [PATCH 5/5] fix --- library/src/image.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/src/image.cxx b/library/src/image.cxx index 9fc2bfc4cc..121b95d6c5 100644 --- a/library/src/image.cxx +++ b/library/src/image.cxx @@ -403,7 +403,7 @@ double image::compare(const image& reference) const imDiff->SetThreshold(0); imDiff->SetInputData(this->Internals->Image); imDiff->SetImageData(reference.Internals->Image); - imDiff->UpdateInformation(); + imDiff->Update(); double error = imDiff->GetThresholdedError(); return error / 1000.0; #endif