From d6d5695417dbd0b667466c8e1fbd8e95911849e1 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 Mar 2023 14:28:28 -0700 Subject: [PATCH 01/26] Added golden image tests to impeller --- BUILD.gn | 5 +- impeller/golden_tests/BUILD.gn | 36 +++++++++++ impeller/golden_tests/golden_tests.cc | 70 +++++++++++++++++++++ impeller/golden_tests/main.cc | 55 ++++++++++++++++ impeller/golden_tests/metal_screenshot.h | 38 +++++++++++ impeller/golden_tests/metal_screenshot.mm | 57 +++++++++++++++++ impeller/golden_tests/metal_screenshoter.h | 29 +++++++++ impeller/golden_tests/metal_screenshoter.mm | 48 ++++++++++++++ impeller/golden_tests/skia_gold_client.cc | 44 +++++++++++++ impeller/golden_tests/skia_gold_client.h | 31 +++++++++ impeller/golden_tests/working_directory.cc | 35 +++++++++++ impeller/golden_tests/working_directory.h | 33 ++++++++++ 12 files changed, 480 insertions(+), 1 deletion(-) create mode 100644 impeller/golden_tests/BUILD.gn create mode 100644 impeller/golden_tests/golden_tests.cc create mode 100644 impeller/golden_tests/main.cc create mode 100644 impeller/golden_tests/metal_screenshot.h create mode 100644 impeller/golden_tests/metal_screenshot.mm create mode 100644 impeller/golden_tests/metal_screenshoter.h create mode 100644 impeller/golden_tests/metal_screenshoter.mm create mode 100644 impeller/golden_tests/skia_gold_client.cc create mode 100644 impeller/golden_tests/skia_gold_client.h create mode 100644 impeller/golden_tests/working_directory.cc create mode 100644 impeller/golden_tests/working_directory.h diff --git a/BUILD.gn b/BUILD.gn index 0b7cca847deb4..1ad4751ed5a29 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -177,7 +177,10 @@ group("unittests") { } if (is_mac || is_linux || is_win) { - public_deps += [ "//flutter/impeller:impeller_unittests" ] + public_deps += [ + "//flutter/impeller:impeller_unittests", + "//flutter/impeller/golden_tests:impeller_golden_tests", + ] } if (is_mac) { diff --git a/impeller/golden_tests/BUILD.gn b/impeller/golden_tests/BUILD.gn new file mode 100644 index 0000000000000..e5abb8a349b33 --- /dev/null +++ b/impeller/golden_tests/BUILD.gn @@ -0,0 +1,36 @@ +# Copyright 2013 The Flutter Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import("//flutter/impeller/tools/impeller.gni") + +test_fixtures("impeller_golden_tests_fixtures") { + fixtures = [] +} + +impeller_component("impeller_golden_tests") { + target_type = "executable" + + testonly = true + + sources = [ + "golden_tests.cc", + "main.cc", + "metal_screenshot.h", + "metal_screenshot.mm", + "metal_screenshoter.h", + "metal_screenshoter.mm", + "skia_gold_client.cc", + "skia_gold_client.h", + "working_directory.cc", + "working_directory.h", + ] + + deps = [ + ":impeller_golden_tests_fixtures", + "//flutter/impeller/aiks", + "//flutter/impeller/playground", + "//flutter/impeller/renderer/backend/metal:metal", + "//third_party/googletest:gtest", + ] +} diff --git a/impeller/golden_tests/golden_tests.cc b/impeller/golden_tests/golden_tests.cc new file mode 100644 index 0000000000000..d602eb852849c --- /dev/null +++ b/impeller/golden_tests/golden_tests.cc @@ -0,0 +1,70 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "gtest/gtest.h" + +#include + +#include "impeller/aiks/canvas.h" +#include "impeller/entity/contents/conical_gradient_contents.h" +#include "impeller/geometry/path_builder.h" +#include "impeller/golden_tests/metal_screenshot.h" +#include "impeller/golden_tests/metal_screenshoter.h" +#include "impeller/golden_tests/working_directory.h" + +namespace impeller { +namespace testing { + +namespace { +std::string GetGoldenFilename() { + std::string suite_name = + ::testing::UnitTest::GetInstance()->current_test_suite()->name(); + std::string test_name = + ::testing::UnitTest::GetInstance()->current_test_info()->name(); + std::stringstream ss; + ss << "impeller_" << suite_name << "_" << test_name << ".png"; + return ss.str(); +} + +bool SaveScreenshot(std::unique_ptr screenshot) { + if (!screenshot || !screenshot->GetBytes()) { + return false; + } + return screenshot->WriteToPNG( + WorkingDirectory::Instance()->GetFilenamePath(GetGoldenFilename())); +} +} // namespace + +class GoldenTests : public ::testing::Test { + public: + GoldenTests() : screenshoter_(new MetalScreenshoter()) {} + + MetalScreenshoter& Screenshoter() { return *screenshoter_; } + + private: + std::unique_ptr screenshoter_; +}; + +TEST_F(GoldenTests, ConicalGradient) { + Canvas canvas; + Paint paint; + paint.color_source_type = Paint::ColorSourceType::kConicalGradient; + paint.color_source = []() { + auto result = std::make_shared(); + result->SetCenterAndRadius(Point(125, 125), 125); + result->SetColors({Color(1.0, 0.0, 0.0, 1.0), Color(0.0, 0.0, 1.0, 1.0)}); + result->SetStops({0, 1}); + result->SetFocus(Point(180, 180), 0); + result->SetTileMode(Entity::TileMode::kClamp); + return result; + }; + paint.stroke_width = 0.0; + paint.style = Paint::Style::kFill; + canvas.DrawRect(Rect(10, 10, 250, 250), paint); + Picture picture = canvas.EndRecordingAsPicture(); + auto screenshot = Screenshoter().MakeScreenshot(std::move(picture)); + ASSERT_TRUE(SaveScreenshot(std::move(screenshot))); +} +} // namespace testing +} // namespace impeller diff --git a/impeller/golden_tests/main.cc b/impeller/golden_tests/main.cc new file mode 100644 index 0000000000000..2eee18739112a --- /dev/null +++ b/impeller/golden_tests/main.cc @@ -0,0 +1,55 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include "flutter/fml/backtrace.h" +#include "flutter/fml/build_config.h" +#include "flutter/fml/command_line.h" +#include "flutter/fml/logging.h" +#include "flutter/impeller/golden_tests/working_directory.h" +#include "gtest/gtest.h" + +namespace { +void print_usage() { + std::cout << "usage: impeller_golden_tests --working_dir=" + << std::endl + << std::endl; + std::cout << "flags:" << std::endl; + std::cout << " working_dir: Where the golden images will be generated and " + "uploaded to Skia Gold from." + << std::endl; +} +} // namespace + +int main(int argc, char** argv) { + fml::InstallCrashHandler(); + testing::InitGoogleTest(&argc, argv); + fml::CommandLine cmd = fml::CommandLineFromPlatformOrArgcArgv(argc, argv); + + std::optional working_dir; + for (const auto& option : cmd.options()) { + if (option.name == "working_dir") { + wordexp_t wordexp_result; + int code = wordexp(option.value.c_str(), &wordexp_result, 0); + FML_CHECK(code == 0); + FML_CHECK(wordexp_result.we_wordc != 0); + working_dir = wordexp_result.we_wordv[0]; + wordfree(&wordexp_result); + } + } + if (!working_dir) { + std::cout << "required argument \"working_dir\" is missing." << std::endl + << std::endl; + print_usage(); + return 1; + } + + impeller::testing::WorkingDirectory::Instance()->SetPath(working_dir.value()); + std::cout << "working directory: " + << impeller::testing::WorkingDirectory::Instance()->GetPath() + << std::endl; + + return RUN_ALL_TESTS(); +} diff --git a/impeller/golden_tests/metal_screenshot.h b/impeller/golden_tests/metal_screenshot.h new file mode 100644 index 0000000000000..f7049623f3232 --- /dev/null +++ b/impeller/golden_tests/metal_screenshot.h @@ -0,0 +1,38 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include +#include +#include + +#include "flutter/fml/macros.h" + +namespace impeller { +namespace testing { + +class MetalScreenshoter; + +class MetalScreenshot { + public: + ~MetalScreenshot(); + + const UInt8* GetBytes() const; + + size_t GetHeight() const; + + size_t GetWidth() const; + + bool WriteToPNG(const std::string& path) const; + + private: + friend class MetalScreenshoter; + MetalScreenshot(CGImageRef cgImage); + FML_DISALLOW_COPY_AND_ASSIGN(MetalScreenshot); + CGImageRef cgImage_; + CFDataRef pixel_data_; +}; +} // namespace testing +} // namespace impeller diff --git a/impeller/golden_tests/metal_screenshot.mm b/impeller/golden_tests/metal_screenshot.mm new file mode 100644 index 0000000000000..deee2716a1622 --- /dev/null +++ b/impeller/golden_tests/metal_screenshot.mm @@ -0,0 +1,57 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "impeller/golden_tests/metal_screenshot.h" + +namespace impeller { +namespace testing { + +extern char* golden_test_working_directory; + +MetalScreenshot::MetalScreenshot(CGImageRef cgImage) : cgImage_(cgImage) { + CGDataProviderRef data_provider = CGImageGetDataProvider(cgImage); + pixel_data_ = CGDataProviderCopyData(data_provider); +} + +MetalScreenshot::~MetalScreenshot() { + CFRelease(pixel_data_); + CFRelease(cgImage_); +} + +const UInt8* MetalScreenshot::GetBytes() const { + return CFDataGetBytePtr(pixel_data_); +} + +size_t MetalScreenshot::GetHeight() const { + return CGImageGetHeight(cgImage_); +} + +size_t MetalScreenshot::GetWidth() const { + return CGImageGetWidth(cgImage_); +} + +bool MetalScreenshot::WriteToPNG(const std::string& path) const { + bool result = false; + NSURL* output_url = + [NSURL fileURLWithPath:[NSString stringWithUTF8String:path.c_str()]]; + CGImageDestinationRef destination = CGImageDestinationCreateWithURL( + (__bridge CFURLRef)output_url, kUTTypePNG, 1, nullptr); + if (destination != nullptr) { + CGImageDestinationAddImage( + destination, cgImage_, (__bridge CFDictionaryRef) @{ + (__bridge NSString*)kCGImagePropertyOrientation : + @(kCGImagePropertyOrientationDownMirrored), + }); + + if (CGImageDestinationFinalize(destination)) { + result = true; + } + + CFRelease(destination); + } + return result; +} + +} // namespace testing +} // namespace impeller diff --git a/impeller/golden_tests/metal_screenshoter.h b/impeller/golden_tests/metal_screenshoter.h new file mode 100644 index 0000000000000..d179a7d16b834 --- /dev/null +++ b/impeller/golden_tests/metal_screenshoter.h @@ -0,0 +1,29 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include "flutter/fml/macros.h" +#include "flutter/impeller/aiks/picture.h" +#include "flutter/impeller/golden_tests/metal_screenshot.h" +#include "flutter/impeller/playground/playground_impl.h" + +namespace impeller { +namespace testing { + +class MetalScreenshoter { + public: + MetalScreenshoter(); + + std::unique_ptr MakeScreenshot(Picture&& picture, + const ISize& size = {300, + 300}); + + private: + std::unique_ptr playground_; + std::unique_ptr aiks_context_; +}; + +} // namespace testing +} // namespace impeller diff --git a/impeller/golden_tests/metal_screenshoter.mm b/impeller/golden_tests/metal_screenshoter.mm new file mode 100644 index 0000000000000..7237ddab997f8 --- /dev/null +++ b/impeller/golden_tests/metal_screenshoter.mm @@ -0,0 +1,48 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/impeller/golden_tests/metal_screenshoter.h" + +#include +#include "impeller/renderer/backend/metal/texture_mtl.h" +#define GLFW_INCLUDE_NONE +#include "third_party/glfw/include/GLFW/glfw3.h" + +namespace impeller { +namespace testing { + +MetalScreenshoter::MetalScreenshoter() { + FML_CHECK(::glfwInit() == GLFW_TRUE); + playground_ = PlaygroundImpl::Create(PlaygroundBackend::kMetal); + aiks_context_.reset(new AiksContext(playground_->GetContext())); +} + +std::unique_ptr MetalScreenshoter::MakeScreenshot( + Picture&& picture, + const ISize& size) { + std::shared_ptr image = picture.ToImage(*aiks_context_, size); + std::shared_ptr texture = image->GetTexture(); + id metal_texture = + std::static_pointer_cast(texture)->GetMTLTexture(); + + if (metal_texture.pixelFormat != MTLPixelFormatBGRA8Unorm) { + return {}; + } + + CIImage* ciImage = [[CIImage alloc] initWithMTLTexture:metal_texture + options:@{}]; + if (!ciImage) { + return {}; + } + + CIContext* context = [CIContext contextWithOptions:nil]; + + CGImageRef cgImage = [context createCGImage:ciImage + fromRect:[ciImage extent]]; + + return std::unique_ptr(new MetalScreenshot(cgImage)); +} + +} // namespace testing +} // namespace impeller diff --git a/impeller/golden_tests/skia_gold_client.cc b/impeller/golden_tests/skia_gold_client.cc new file mode 100644 index 0000000000000..c75edb7ca2e0e --- /dev/null +++ b/impeller/golden_tests/skia_gold_client.cc @@ -0,0 +1,44 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "impeller/golden_tests/skia_gold_client.h" + +#include +#include +#include + +namespace flutter { +namespace testing { +const char* GetSourcePath(); +const char* GetFixturesPath(); +const char* GetTestingAssetsPath(); +} // namespace testing +} // namespace flutter + +namespace impeller { +namespace testing { +struct SkiaGoldClientImpl { + std::string gold_ctl; + bool is_luci = false; + bool did_auth = false; +}; + +SkiaGoldClient::SkiaGoldClient() : impl_(new SkiaGoldClientImpl()) { + (void)flutter::testing::GetSourcePath(); +} + +SkiaGoldClient::~SkiaGoldClient() {} + +bool SkiaGoldClient::Auth() { + impl_->did_auth = true; + return true; +} + +void SkiaGoldClient::Compare(const std::string& name, + const std::string& png_path) { + assert(impl_->did_auth); +} + +} // namespace testing +} // namespace impeller diff --git a/impeller/golden_tests/skia_gold_client.h b/impeller/golden_tests/skia_gold_client.h new file mode 100644 index 0000000000000..298dcc72877c1 --- /dev/null +++ b/impeller/golden_tests/skia_gold_client.h @@ -0,0 +1,31 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include +#include +#include + +namespace impeller { +namespace testing { + +struct SkiaGoldClientImpl; + +class SkiaGoldClient { + public: + SkiaGoldClient(); + + virtual ~SkiaGoldClient(); + + bool Auth(); + + void Compare(const std::string& name, const std::string& png_path); + + private: + std::unique_ptr impl_; +}; + +} // namespace testing +} // namespace impeller diff --git a/impeller/golden_tests/working_directory.cc b/impeller/golden_tests/working_directory.cc new file mode 100644 index 0000000000000..42bf049e1bf5b --- /dev/null +++ b/impeller/golden_tests/working_directory.cc @@ -0,0 +1,35 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "impeller/golden_tests/working_directory.h" + +#include "flutter/fml/paths.h" + +namespace impeller { +namespace testing { + +WorkingDirectory* WorkingDirectory::instance_ = nullptr; + +WorkingDirectory::WorkingDirectory() {} + +WorkingDirectory* WorkingDirectory::Instance() { + if (!instance_) { + instance_ = new WorkingDirectory(); + } + return instance_; +} + +std::string WorkingDirectory::GetFilenamePath( + const std::string& filename) const { + return fml::paths::JoinPaths({path_, filename}); +} + +void WorkingDirectory::SetPath(const std::string& path) { + FML_CHECK(did_set_ == false); + path_ = path; + did_set_ = true; +} + +} // namespace testing +} // namespace impeller \ No newline at end of file diff --git a/impeller/golden_tests/working_directory.h b/impeller/golden_tests/working_directory.h new file mode 100644 index 0000000000000..7f9f4c449be9a --- /dev/null +++ b/impeller/golden_tests/working_directory.h @@ -0,0 +1,33 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include + +#include "flutter/fml/macros.h" + +namespace impeller { +namespace testing { + +class WorkingDirectory { + public: + static WorkingDirectory* Instance(); + + std::string GetFilenamePath(const std::string& filename) const; + + void SetPath(const std::string& path); + + const std::string& GetPath() const { return path_; } + + private: + FML_DISALLOW_COPY_AND_ASSIGN(WorkingDirectory); + WorkingDirectory(); + static WorkingDirectory* instance_; + std::string path_; + bool did_set_ = false; +}; + +} // namespace testing +} // namespace impeller From b17819eae4ed5b529321eb0842b64df259885885 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 20 Mar 2023 13:57:38 -0700 Subject: [PATCH 02/26] started writing a digest --- impeller/golden_tests/BUILD.gn | 4 +- impeller/golden_tests/golden_digest.cc | 58 +++++++++++++++++++++ impeller/golden_tests/golden_digest.h | 41 +++++++++++++++ impeller/golden_tests/golden_tests.cc | 15 ++++-- impeller/golden_tests/main.cc | 8 ++- impeller/golden_tests/metal_screenshot.mm | 4 +- impeller/golden_tests/metal_screenshoter.mm | 16 +++--- impeller/golden_tests/skia_gold_client.cc | 44 ---------------- impeller/golden_tests/skia_gold_client.h | 31 ----------- 9 files changed, 131 insertions(+), 90 deletions(-) create mode 100644 impeller/golden_tests/golden_digest.cc create mode 100644 impeller/golden_tests/golden_digest.h delete mode 100644 impeller/golden_tests/skia_gold_client.cc delete mode 100644 impeller/golden_tests/skia_gold_client.h diff --git a/impeller/golden_tests/BUILD.gn b/impeller/golden_tests/BUILD.gn index e5abb8a349b33..478c1479de4ef 100644 --- a/impeller/golden_tests/BUILD.gn +++ b/impeller/golden_tests/BUILD.gn @@ -14,14 +14,14 @@ impeller_component("impeller_golden_tests") { testonly = true sources = [ + "golden_digest.cc", + "golden_digest.h", "golden_tests.cc", "main.cc", "metal_screenshot.h", "metal_screenshot.mm", "metal_screenshoter.h", "metal_screenshoter.mm", - "skia_gold_client.cc", - "skia_gold_client.h", "working_directory.cc", "working_directory.h", ] diff --git a/impeller/golden_tests/golden_digest.cc b/impeller/golden_tests/golden_digest.cc new file mode 100644 index 0000000000000..65f4e8bf93a02 --- /dev/null +++ b/impeller/golden_tests/golden_digest.cc @@ -0,0 +1,58 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "impeller/golden_tests/golden_digest.h" + +#include + +namespace impeller { +namespace testing { + +GoldenDigest* GoldenDigest::instance_ = nullptr; + +GoldenDigest* GoldenDigest::Instance() { + if (!instance_) { + instance_ = new GoldenDigest(); + } + return instance_; +} + +GoldenDigest::GoldenDigest() {} + +void GoldenDigest::AddImage(const std::string& test_name, + const std::string& filename, + int32_t width, + int32_t height) { + entries_.push_back({test_name, filename, width, height}); +} + +bool GoldenDigest::Write(WorkingDirectory* working_directory) { + std::ofstream fout; + fout.open(working_directory->GetFilenamePath("digest.json")); + if (!fout.good()) { + return false; + } + + fout << "[" << std::endl; + bool is_first = true; + for (const auto& entry : entries_) { + if (!is_first) { + fout << "," << std::endl; + is_first = false; + } + fout << " { " + << "\"testName\" : \"" << entry.test_name << "\", " + << "\"fileName\" : \"" << entry.filename << "\", " + << "\"width\" : \"" << entry.width << "\", " + << "\"height\" : \"" << entry.height << "\" " + << "}"; + } + fout << std::endl << "]" << std::endl; + + fout.close(); + return true; +} + +} // namespace testing +} // namespace impeller diff --git a/impeller/golden_tests/golden_digest.h b/impeller/golden_tests/golden_digest.h new file mode 100644 index 0000000000000..e43cb02ab9d58 --- /dev/null +++ b/impeller/golden_tests/golden_digest.h @@ -0,0 +1,41 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include +#include + +#include "flutter/fml/macros.h" +#include "flutter/impeller/golden_tests/working_directory.h" + +namespace impeller { +namespace testing { + +class GoldenDigest { + public: + static GoldenDigest* Instance(); + + void AddImage(const std::string& test_name, + const std::string& filename, + int32_t width, + int32_t height); + + bool Write(WorkingDirectory* working_directory); + + private: + FML_DISALLOW_COPY_AND_ASSIGN(GoldenDigest); + GoldenDigest(); + struct Entry { + std::string test_name; + std::string filename; + int32_t width; + int32_t height; + }; + + static GoldenDigest* instance_; + std::vector entries_; +}; +} // namespace testing +} // namespace impeller diff --git a/impeller/golden_tests/golden_tests.cc b/impeller/golden_tests/golden_tests.cc index d602eb852849c..3bcb76e279720 100644 --- a/impeller/golden_tests/golden_tests.cc +++ b/impeller/golden_tests/golden_tests.cc @@ -9,6 +9,7 @@ #include "impeller/aiks/canvas.h" #include "impeller/entity/contents/conical_gradient_contents.h" #include "impeller/geometry/path_builder.h" +#include "impeller/golden_tests/golden_digest.h" #include "impeller/golden_tests/metal_screenshot.h" #include "impeller/golden_tests/metal_screenshoter.h" #include "impeller/golden_tests/working_directory.h" @@ -17,22 +18,30 @@ namespace impeller { namespace testing { namespace { -std::string GetGoldenFilename() { +std::string GetTestName() { std::string suite_name = ::testing::UnitTest::GetInstance()->current_test_suite()->name(); std::string test_name = ::testing::UnitTest::GetInstance()->current_test_info()->name(); std::stringstream ss; - ss << "impeller_" << suite_name << "_" << test_name << ".png"; + ss << "impeller_" << suite_name << "_" << test_name; return ss.str(); } +std::string GetGoldenFilename() { + return GetTestName() + ".png"; +} + bool SaveScreenshot(std::unique_ptr screenshot) { if (!screenshot || !screenshot->GetBytes()) { return false; } + std::string test_name = GetTestName(); + std::string filename = GetGoldenFilename(); + GoldenDigest::Instance()->AddImage( + test_name, filename, screenshot->GetWidth(), screenshot->GetHeight()); return screenshot->WriteToPNG( - WorkingDirectory::Instance()->GetFilenamePath(GetGoldenFilename())); + WorkingDirectory::Instance()->GetFilenamePath(filename)); } } // namespace diff --git a/impeller/golden_tests/main.cc b/impeller/golden_tests/main.cc index 2eee18739112a..e1ed2a8c4eccc 100644 --- a/impeller/golden_tests/main.cc +++ b/impeller/golden_tests/main.cc @@ -8,6 +8,7 @@ #include "flutter/fml/build_config.h" #include "flutter/fml/command_line.h" #include "flutter/fml/logging.h" +#include "flutter/impeller/golden_tests/golden_digest.h" #include "flutter/impeller/golden_tests/working_directory.h" #include "gtest/gtest.h" @@ -51,5 +52,10 @@ int main(int argc, char** argv) { << impeller::testing::WorkingDirectory::Instance()->GetPath() << std::endl; - return RUN_ALL_TESTS(); + int return_code = RUN_ALL_TESTS(); + if (0 == return_code) { + impeller::testing::GoldenDigest::Instance()->Write( + impeller::testing::WorkingDirectory::Instance()); + } + return return_code; } diff --git a/impeller/golden_tests/metal_screenshot.mm b/impeller/golden_tests/metal_screenshot.mm index deee2716a1622..4fb5069d3bc8e 100644 --- a/impeller/golden_tests/metal_screenshot.mm +++ b/impeller/golden_tests/metal_screenshot.mm @@ -7,8 +7,6 @@ namespace impeller { namespace testing { -extern char* golden_test_working_directory; - MetalScreenshot::MetalScreenshot(CGImageRef cgImage) : cgImage_(cgImage) { CGDataProviderRef data_provider = CGImageGetDataProvider(cgImage); pixel_data_ = CGDataProviderCopyData(data_provider); @@ -16,7 +14,7 @@ MetalScreenshot::~MetalScreenshot() { CFRelease(pixel_data_); - CFRelease(cgImage_); + CGImageRelease(cgImage_); } const UInt8* MetalScreenshot::GetBytes() const { diff --git a/impeller/golden_tests/metal_screenshoter.mm b/impeller/golden_tests/metal_screenshoter.mm index 7237ddab997f8..e5100a5b9d7c3 100644 --- a/impeller/golden_tests/metal_screenshoter.mm +++ b/impeller/golden_tests/metal_screenshoter.mm @@ -5,6 +5,7 @@ #include "flutter/impeller/golden_tests/metal_screenshoter.h" #include +#include "impeller/renderer/backend/metal/context_mtl.h" #include "impeller/renderer/backend/metal/texture_mtl.h" #define GLFW_INCLUDE_NONE #include "third_party/glfw/include/GLFW/glfw3.h" @@ -32,14 +33,17 @@ CIImage* ciImage = [[CIImage alloc] initWithMTLTexture:metal_texture options:@{}]; - if (!ciImage) { - return {}; - } + FML_CHECK(ciImage); - CIContext* context = [CIContext contextWithOptions:nil]; + std::shared_ptr context = playground_->GetContext(); + std::shared_ptr context_mtl = + std::static_pointer_cast(context); + CIContext* cicontext = + [CIContext contextWithMTLDevice:context_mtl->GetMTLDevice()]; + FML_CHECK(context); - CGImageRef cgImage = [context createCGImage:ciImage - fromRect:[ciImage extent]]; + CGImageRef cgImage = [cicontext createCGImage:ciImage + fromRect:[ciImage extent]]; return std::unique_ptr(new MetalScreenshot(cgImage)); } diff --git a/impeller/golden_tests/skia_gold_client.cc b/impeller/golden_tests/skia_gold_client.cc deleted file mode 100644 index c75edb7ca2e0e..0000000000000 --- a/impeller/golden_tests/skia_gold_client.cc +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "impeller/golden_tests/skia_gold_client.h" - -#include -#include -#include - -namespace flutter { -namespace testing { -const char* GetSourcePath(); -const char* GetFixturesPath(); -const char* GetTestingAssetsPath(); -} // namespace testing -} // namespace flutter - -namespace impeller { -namespace testing { -struct SkiaGoldClientImpl { - std::string gold_ctl; - bool is_luci = false; - bool did_auth = false; -}; - -SkiaGoldClient::SkiaGoldClient() : impl_(new SkiaGoldClientImpl()) { - (void)flutter::testing::GetSourcePath(); -} - -SkiaGoldClient::~SkiaGoldClient() {} - -bool SkiaGoldClient::Auth() { - impl_->did_auth = true; - return true; -} - -void SkiaGoldClient::Compare(const std::string& name, - const std::string& png_path) { - assert(impl_->did_auth); -} - -} // namespace testing -} // namespace impeller diff --git a/impeller/golden_tests/skia_gold_client.h b/impeller/golden_tests/skia_gold_client.h deleted file mode 100644 index 298dcc72877c1..0000000000000 --- a/impeller/golden_tests/skia_gold_client.h +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#pragma once - -#include -#include -#include - -namespace impeller { -namespace testing { - -struct SkiaGoldClientImpl; - -class SkiaGoldClient { - public: - SkiaGoldClient(); - - virtual ~SkiaGoldClient(); - - bool Auth(); - - void Compare(const std::string& name, const std::string& png_path); - - private: - std::unique_ptr impl_; -}; - -} // namespace testing -} // namespace impeller From 345d019f4d82cb56e0c0bb2c237e77d5fd7f8274 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 20 Mar 2023 14:45:41 -0700 Subject: [PATCH 03/26] added docstrings --- impeller/golden_tests/golden_digest.h | 4 ++++ impeller/golden_tests/metal_screenshot.h | 1 + impeller/golden_tests/metal_screenshoter.h | 1 + impeller/golden_tests/working_directory.h | 2 ++ 4 files changed, 8 insertions(+) diff --git a/impeller/golden_tests/golden_digest.h b/impeller/golden_tests/golden_digest.h index e43cb02ab9d58..011607ee599ae 100644 --- a/impeller/golden_tests/golden_digest.h +++ b/impeller/golden_tests/golden_digest.h @@ -13,6 +13,7 @@ namespace impeller { namespace testing { +/// Manages a global variable for tracking instances of golden images. class GoldenDigest { public: static GoldenDigest* Instance(); @@ -22,6 +23,9 @@ class GoldenDigest { int32_t width, int32_t height); + /// Writes a "digest.json" file to `working_directory`. + /// + /// Returns `true` on success. bool Write(WorkingDirectory* working_directory); private: diff --git a/impeller/golden_tests/metal_screenshot.h b/impeller/golden_tests/metal_screenshot.h index f7049623f3232..0d2df7fc44309 100644 --- a/impeller/golden_tests/metal_screenshot.h +++ b/impeller/golden_tests/metal_screenshot.h @@ -15,6 +15,7 @@ namespace testing { class MetalScreenshoter; +/// A screenshot that was produced from `MetalScreenshoter`. class MetalScreenshot { public: ~MetalScreenshot(); diff --git a/impeller/golden_tests/metal_screenshoter.h b/impeller/golden_tests/metal_screenshoter.h index d179a7d16b834..673f7f7091f0f 100644 --- a/impeller/golden_tests/metal_screenshoter.h +++ b/impeller/golden_tests/metal_screenshoter.h @@ -12,6 +12,7 @@ namespace impeller { namespace testing { +/// Converts `Picture`'s to `MetalScreenshot`'s with the playground backend. class MetalScreenshoter { public: MetalScreenshoter(); diff --git a/impeller/golden_tests/working_directory.h b/impeller/golden_tests/working_directory.h index 7f9f4c449be9a..b3a3d74317fa6 100644 --- a/impeller/golden_tests/working_directory.h +++ b/impeller/golden_tests/working_directory.h @@ -11,6 +11,8 @@ namespace impeller { namespace testing { +/// Keeps track of the global variable for the specified working +/// directory. class WorkingDirectory { public: static WorkingDirectory* Instance(); From 93725bad51b0dce2508c49a3347add22ef0ecd6a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 20 Mar 2023 15:17:50 -0700 Subject: [PATCH 04/26] added harvester --- impeller/golden_tests/golden_digest.cc | 6 +- impeller/golden_tests_harvester/.gitignore | 3 + impeller/golden_tests_harvester/CHANGELOG.md | 3 + impeller/golden_tests_harvester/README.md | 10 +++ .../analysis_options.yaml | 1 + .../bin/golden_tests_harvester.dart | 87 +++++++++++++++++++ .../lib/golden_tests_harvester.dart | 41 +++++++++ impeller/golden_tests_harvester/pubspec.yaml | 40 +++++++++ .../test/golden_tests_harvester_test.dart | 5 ++ 9 files changed, 193 insertions(+), 3 deletions(-) create mode 100644 impeller/golden_tests_harvester/.gitignore create mode 100644 impeller/golden_tests_harvester/CHANGELOG.md create mode 100644 impeller/golden_tests_harvester/README.md create mode 100644 impeller/golden_tests_harvester/analysis_options.yaml create mode 100644 impeller/golden_tests_harvester/bin/golden_tests_harvester.dart create mode 100644 impeller/golden_tests_harvester/lib/golden_tests_harvester.dart create mode 100644 impeller/golden_tests_harvester/pubspec.yaml create mode 100644 impeller/golden_tests_harvester/test/golden_tests_harvester_test.dart diff --git a/impeller/golden_tests/golden_digest.cc b/impeller/golden_tests/golden_digest.cc index 65f4e8bf93a02..3de06521b9dbb 100644 --- a/impeller/golden_tests/golden_digest.cc +++ b/impeller/golden_tests/golden_digest.cc @@ -43,9 +43,9 @@ bool GoldenDigest::Write(WorkingDirectory* working_directory) { } fout << " { " << "\"testName\" : \"" << entry.test_name << "\", " - << "\"fileName\" : \"" << entry.filename << "\", " - << "\"width\" : \"" << entry.width << "\", " - << "\"height\" : \"" << entry.height << "\" " + << "\"filename\" : \"" << entry.filename << "\", " + << "\"width\" : " << entry.width << ", " + << "\"height\" : " << entry.height << " " << "}"; } fout << std::endl << "]" << std::endl; diff --git a/impeller/golden_tests_harvester/.gitignore b/impeller/golden_tests_harvester/.gitignore new file mode 100644 index 0000000000000..3a85790408401 --- /dev/null +++ b/impeller/golden_tests_harvester/.gitignore @@ -0,0 +1,3 @@ +# https://dart.dev/guides/libraries/private-files +# Created by `dart pub` +.dart_tool/ diff --git a/impeller/golden_tests_harvester/CHANGELOG.md b/impeller/golden_tests_harvester/CHANGELOG.md new file mode 100644 index 0000000000000..effe43c82c8a7 --- /dev/null +++ b/impeller/golden_tests_harvester/CHANGELOG.md @@ -0,0 +1,3 @@ +## 1.0.0 + +- Initial version. diff --git a/impeller/golden_tests_harvester/README.md b/impeller/golden_tests_harvester/README.md new file mode 100644 index 0000000000000..59c589bfd07f1 --- /dev/null +++ b/impeller/golden_tests_harvester/README.md @@ -0,0 +1,10 @@ +# Golden Tests Harvester + +Reaps the output of impeller's golden image tests and sends it to Skia gold. + +## Usage + +``` +./out/host_debug_unopt_arm64/impeller_golden_tests --working_dir=~/Desktop/temp +dart run ~/Desktop/temp +``` diff --git a/impeller/golden_tests_harvester/analysis_options.yaml b/impeller/golden_tests_harvester/analysis_options.yaml new file mode 100644 index 0000000000000..f04c6cf0f30d4 --- /dev/null +++ b/impeller/golden_tests_harvester/analysis_options.yaml @@ -0,0 +1 @@ +include: ../../analysis_options.yaml diff --git a/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart b/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart new file mode 100644 index 0000000000000..44121b0185e64 --- /dev/null +++ b/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart @@ -0,0 +1,87 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io'; + +import 'package:golden_tests_harvester/golden_tests_harvester.dart'; +import 'package:process/src/interface/process_manager.dart'; +import 'package:skia_gold_client/skia_gold_client.dart'; + +const String _kLuciEnvName = 'LUCI_CONTEXT'; + +bool get isLuciEnv => Platform.environment.containsKey(_kLuciEnvName); + +/// Fake SkiaGoldClient that is used if the harvester is run outside of Luci. +class FakeSkiaGoldClient implements SkiaGoldClient { + FakeSkiaGoldClient(this._workingDirectory); + + final Directory _workingDirectory; + + @override + Future addImg(String testName, File goldenFile, + {double differentPixelsRate = 0.01, + int pixelColorDelta = 0, + required int screenshotSize}) async { + print('addImg $testName ${goldenFile.path} $screenshotSize'); + } + + @override + Future auth() async { + print('auth'); + } + + @override + String cleanTestName(String fileName) { + throw UnimplementedError(); + } + + @override + Map? get dimensions => throw UnimplementedError(); + + @override + List getCIArguments() { + throw UnimplementedError(); + } + + @override + Future getExpectationForTest(String testName) { + throw UnimplementedError(); + } + + @override + Future> getImageBytes(String imageHash) { + throw UnimplementedError(); + } + + @override + String getTraceID(String testName) { + throw UnimplementedError(); + } + + @override + HttpClient get httpClient => throw UnimplementedError(); + + @override + ProcessManager get process => throw UnimplementedError(); + + @override + Directory get workDirectory => _workingDirectory; +} + +void _printUsage() { + print('dart run ./bin/golden_tests_harvester.dart '); +} + +Future main(List arguments) async { + if (arguments.length != 1) { + return _printUsage(); + } + + final Directory workDirectory = Directory(arguments[0]); + final SkiaGoldClient skiaGoldClient = isLuciEnv + ? SkiaGoldClient(workDirectory) + : FakeSkiaGoldClient(workDirectory); + + await harvest(skiaGoldClient, workDirectory); +} diff --git a/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart b/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart new file mode 100644 index 0000000000000..2aab11c8c668b --- /dev/null +++ b/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart @@ -0,0 +1,41 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:convert'; +import 'dart:io'; + +import 'package:path/path.dart' as p; +import 'package:skia_gold_client/skia_gold_client.dart'; + +/// Reads the digest inside of [workDirectory], sending tests to +/// [skiaGoldClient]. +Future harvest(SkiaGoldClient skiaGoldClient, Directory workDirectory) async { + await skiaGoldClient.auth(); + + final File digest = File(p.join(workDirectory.path, 'digest.json')); + if (!digest.existsSync()) { + print('Error: digest.json does not exist in ${workDirectory.path}.'); + return; + } + final Object? decoded = jsonDecode(digest.readAsStringSync()); + final List entries = (decoded as List?)!; + final List> pendingComparisons = >[]; + for (final Object? entry in entries) { + final Map map = (entry as Map?)!; + final String testName = (map['testName'] as String?)!; + final String filename = (map['filename'] as String?)!; + final int width = (map['width'] as int?)!; + final int height = (map['height'] as int?)!; + final File goldenImage = File(p.join(workDirectory.path, filename)); + final Future future = skiaGoldClient + .addImg(testName, goldenImage, screenshotSize: width * height) + .catchError((dynamic err) { + print('skia gold comparison failed: $err'); + throw Exception('Failed comparison: $testName'); + }); + pendingComparisons.add(future); + } + + await Future.wait(pendingComparisons); +} diff --git a/impeller/golden_tests_harvester/pubspec.yaml b/impeller/golden_tests_harvester/pubspec.yaml new file mode 100644 index 0000000000000..016c1b46e9bc2 --- /dev/null +++ b/impeller/golden_tests_harvester/pubspec.yaml @@ -0,0 +1,40 @@ +# Copyright 2013 The Flutter Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +name: golden_tests_harvester +publish_to: none +environment: + sdk: '>=3.0.0-0 <4.0.0' + +# Do not add any dependencies that require more than what is provided in +# //third_party/dart/pkg, //third_party/dart/third_party/pkg, or +# //third_party/pkg. In particular, package:test is not usable here. + +# If you do add packages here, make sure you can run `pub get --offline`, and +# check the .packages and .package_config to make sure all the paths are +# relative to this directory into //third_party/dart, or //third_party/pkg +dependencies: + crypto: any + path: any + process: any + skia_gold_client: + path: ../../testing/skia_gold_client + +dependency_overrides: + collection: + path: ../../../third_party/dart/third_party/pkg/collection + crypto: + path: ../../../third_party/dart/third_party/pkg/crypto + file: + path: ../../../third_party/pkg/file/packages/file + meta: + path: ../../../third_party/dart/pkg/meta + path: + path: ../../../third_party/dart/third_party/pkg/path + platform: + path: ../../../third_party/pkg/platform + process: + path: ../../../third_party/pkg/process + typed_data: + path: ../../../third_party/dart/third_party/pkg/typed_data \ No newline at end of file diff --git a/impeller/golden_tests_harvester/test/golden_tests_harvester_test.dart b/impeller/golden_tests_harvester/test/golden_tests_harvester_test.dart new file mode 100644 index 0000000000000..f9b0dd79fed95 --- /dev/null +++ b/impeller/golden_tests_harvester/test/golden_tests_harvester_test.dart @@ -0,0 +1,5 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +void main() {} From aca9c66c384c9174fb07ccc1a6c718459c402197 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 20 Mar 2023 15:56:24 -0700 Subject: [PATCH 05/26] updated readme --- impeller/golden_tests_harvester/README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/impeller/golden_tests_harvester/README.md b/impeller/golden_tests_harvester/README.md index 59c589bfd07f1..9920093f3ecb7 100644 --- a/impeller/golden_tests_harvester/README.md +++ b/impeller/golden_tests_harvester/README.md @@ -4,7 +4,11 @@ Reaps the output of impeller's golden image tests and sends it to Skia gold. ## Usage -``` +```sh +cd $SRC ./out/host_debug_unopt_arm64/impeller_golden_tests --working_dir=~/Desktop/temp -dart run ~/Desktop/temp +cd flutter/impeller/golden_tests_harvester +dart run ./bin/golden_tests_harvester.dart ~/Desktop/temp ``` + +See also [golden_tests](../golden_tests/). From 1ec9eb396e3ba037f22480b62fef08ae1a0f5297 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 20 Mar 2023 16:02:45 -0700 Subject: [PATCH 06/26] added logger wrapper --- impeller/golden_tests_harvester/CHANGELOG.md | 3 --- .../bin/golden_tests_harvester.dart | 7 ++++--- .../lib/golden_tests_harvester.dart | 12 ++++++++--- .../golden_tests_harvester/lib/logger.dart | 20 +++++++++++++++++++ 4 files changed, 33 insertions(+), 9 deletions(-) delete mode 100644 impeller/golden_tests_harvester/CHANGELOG.md create mode 100644 impeller/golden_tests_harvester/lib/logger.dart diff --git a/impeller/golden_tests_harvester/CHANGELOG.md b/impeller/golden_tests_harvester/CHANGELOG.md deleted file mode 100644 index effe43c82c8a7..0000000000000 --- a/impeller/golden_tests_harvester/CHANGELOG.md +++ /dev/null @@ -1,3 +0,0 @@ -## 1.0.0 - -- Initial version. diff --git a/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart b/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart index 44121b0185e64..7aa1f84d128be 100644 --- a/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart +++ b/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart @@ -23,12 +23,12 @@ class FakeSkiaGoldClient implements SkiaGoldClient { {double differentPixelsRate = 0.01, int pixelColorDelta = 0, required int screenshotSize}) async { - print('addImg $testName ${goldenFile.path} $screenshotSize'); + Logger.instance.log('addImg $testName ${goldenFile.path} $screenshotSize'); } @override Future auth() async { - print('auth'); + Logger.instance.log('auth'); } @override @@ -70,7 +70,8 @@ class FakeSkiaGoldClient implements SkiaGoldClient { } void _printUsage() { - print('dart run ./bin/golden_tests_harvester.dart '); + Logger.instance + .log('dart run ./bin/golden_tests_harvester.dart '); } Future main(List arguments) async { diff --git a/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart b/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart index 2aab11c8c668b..5edf96b137bfb 100644 --- a/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart +++ b/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart @@ -8,14 +8,20 @@ import 'dart:io'; import 'package:path/path.dart' as p; import 'package:skia_gold_client/skia_gold_client.dart'; +import 'logger.dart'; + +export 'logger.dart'; + /// Reads the digest inside of [workDirectory], sending tests to /// [skiaGoldClient]. -Future harvest(SkiaGoldClient skiaGoldClient, Directory workDirectory) async { +Future harvest( + SkiaGoldClient skiaGoldClient, Directory workDirectory) async { await skiaGoldClient.auth(); final File digest = File(p.join(workDirectory.path, 'digest.json')); if (!digest.existsSync()) { - print('Error: digest.json does not exist in ${workDirectory.path}.'); + Logger.instance + .log('Error: digest.json does not exist in ${workDirectory.path}.'); return; } final Object? decoded = jsonDecode(digest.readAsStringSync()); @@ -31,7 +37,7 @@ Future harvest(SkiaGoldClient skiaGoldClient, Directory workDirectory) asy final Future future = skiaGoldClient .addImg(testName, goldenImage, screenshotSize: width * height) .catchError((dynamic err) { - print('skia gold comparison failed: $err'); + Logger.instance.log('skia gold comparison failed: $err'); throw Exception('Failed comparison: $testName'); }); pendingComparisons.add(future); diff --git a/impeller/golden_tests_harvester/lib/logger.dart b/impeller/golden_tests_harvester/lib/logger.dart new file mode 100644 index 0000000000000..b9f5818b37692 --- /dev/null +++ b/impeller/golden_tests_harvester/lib/logger.dart @@ -0,0 +1,20 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/// Logs messages to the appropriate console. +class Logger { + Logger._(); + + /// Singleton accessor for the [Logger]. + static Logger get instance { + _instance ??= Logger._(); + return _instance!; + } + + static Logger? _instance; + + /// Log [message] to the console. + // ignore: avoid_print + void log(String message) => print(message); +} From d079bd23b602412c179a467526a07fbf5cfd4500 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 20 Mar 2023 16:15:16 -0700 Subject: [PATCH 07/26] made the tests only run on mac --- impeller/golden_tests/BUILD.gn | 53 ++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/impeller/golden_tests/BUILD.gn b/impeller/golden_tests/BUILD.gn index 478c1479de4ef..c723725729dbf 100644 --- a/impeller/golden_tests/BUILD.gn +++ b/impeller/golden_tests/BUILD.gn @@ -2,35 +2,38 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import("//flutter/common/config.gni") import("//flutter/impeller/tools/impeller.gni") -test_fixtures("impeller_golden_tests_fixtures") { - fixtures = [] -} +if (is_mac) { + test_fixtures("impeller_golden_tests_fixtures") { + fixtures = [] + } -impeller_component("impeller_golden_tests") { - target_type = "executable" + impeller_component("impeller_golden_tests") { + target_type = "executable" - testonly = true + testonly = true - sources = [ - "golden_digest.cc", - "golden_digest.h", - "golden_tests.cc", - "main.cc", - "metal_screenshot.h", - "metal_screenshot.mm", - "metal_screenshoter.h", - "metal_screenshoter.mm", - "working_directory.cc", - "working_directory.h", - ] + sources = [ + "golden_digest.cc", + "golden_digest.h", + "golden_tests.cc", + "main.cc", + "metal_screenshot.h", + "metal_screenshot.mm", + "metal_screenshoter.h", + "metal_screenshoter.mm", + "working_directory.cc", + "working_directory.h", + ] - deps = [ - ":impeller_golden_tests_fixtures", - "//flutter/impeller/aiks", - "//flutter/impeller/playground", - "//flutter/impeller/renderer/backend/metal:metal", - "//third_party/googletest:gtest", - ] + deps = [ + ":impeller_golden_tests_fixtures", + "//flutter/impeller/aiks", + "//flutter/impeller/playground", + "//flutter/impeller/renderer/backend/metal:metal", + "//third_party/googletest:gtest", + ] + } } From 3141ca1260551035f5ca458e344b5a86072f262c Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 22 Mar 2023 10:13:27 -0700 Subject: [PATCH 08/26] updated the run_tests.py script --- testing/run_tests.py | 57 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 3 deletions(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 23be9205b1d45..7f52a7760394a 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -9,15 +9,17 @@ """ import argparse -import glob +import csv import errno +import glob import multiprocessing import os +from pathlib import Path import re import subprocess import sys +import tempfile as tempfile import time -import csv import xvfb SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) @@ -972,6 +974,45 @@ def run_engine_tasks_in_parallel(tasks): raise Exception() +class DirectoryChange(): + """ + A scoped change in the CWD. + """ + old_cwd: str = "" + new_cwd: str = "" + + def __init__(self, new_cwd: str): + self.new_cwd = new_cwd + + def __enter__(self): + self.old_cwd = os.getcwd() + os.chdir(self.new_cwd) + + def __exit__(self, type, value, traceback): + os.chdir(self.old_cwd) + + +def run_impeller_golden_tests(build_dir: str): + """ + Executes the impeller golden image tests from in the `variant` build. + """ + tests_path: str = os.path.join(build_dir, "impeller_golden_tests") + if not os.path.exists(tests_path): + raise Exception( + "Cannot find the \"impeller_golden_tests\" executable in \"%s\". You may need to build it." + % (variant_path) + ) + harvester_path: Path = Path(SCRIPT_DIR).parent.joinpath("impeller").joinpath( + "golden_tests_harvester" + ) + with tempfile.TemporaryDirectory(prefix="impeller_golden") as temp_dir: + run_cmd([tests_path, "--working_dir=%s" % temp_dir]) + with DirectoryChange(harvester_path): + bin_path = Path(".").joinpath("bin" + ).joinpath("golden_tests_harvester.dart") + run_cmd(["dart", "run", str(bin_path), temp_dir]) + + def main(): parser = argparse.ArgumentParser( description=""" @@ -980,7 +1021,14 @@ def main(): """ ) all_types = [ - 'engine', 'dart', 'benchmarks', 'java', 'android', 'objc', 'font-subset' + 'engine', + 'dart', + 'benchmarks', + 'java', + 'android', + 'objc', + 'font-subset', + 'impeller-golden', ] parser.add_argument( @@ -1171,6 +1219,9 @@ def main(): 'font-subset' in types) and args.variant not in variants_to_skip: run_cmd(['python3', 'test.py'], cwd=FONT_SUBSET_DIR) + if ('impeller-golden' in types): + run_impeller_golden_tests(build_dir) + if __name__ == '__main__': sys.exit(main()) From f8c4431eabba466a9ca2456103d3fcf607cab183 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 22 Mar 2023 10:31:53 -0700 Subject: [PATCH 09/26] added readme --- impeller/golden_tests/README.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 impeller/golden_tests/README.md diff --git a/impeller/golden_tests/README.md b/impeller/golden_tests/README.md new file mode 100644 index 0000000000000..857349f44ed0d --- /dev/null +++ b/impeller/golden_tests/README.md @@ -0,0 +1,20 @@ +# Impeller Golden Tests + +This is the executable that will generate the golden image results that can then +be sent to Skia Gold vial the +[golden_tests_harvester]("../golden_tests_harvester"). + +Running these tests should happen from +[//flutter/testing/run_tests.py](../../testing/run_tests.py). That will do all +the steps to generate the golden images and transmit them to Skia Gold. If you +run the tests locally it will not actually upload anything. That only happens if +the script is executed from LUCI. + +Example invocation: + +```sh +./run_tests.py --variant="host_debug_unopt_arm64" --type="impeller-golden" +``` + +Currently these tests are only supported on macOS and only test the Metal +backend to Impeller. From 0a6757f5f256115552344fc4d6920db78059bf1a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 22 Mar 2023 10:36:59 -0700 Subject: [PATCH 10/26] made sure that the golden image tests are only compiled for mac --- BUILD.gn | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 1ad4751ed5a29..d00ac7c9e234e 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -177,16 +177,13 @@ group("unittests") { } if (is_mac || is_linux || is_win) { - public_deps += [ - "//flutter/impeller:impeller_unittests", - "//flutter/impeller/golden_tests:impeller_golden_tests", - ] + public_deps += [ "//flutter/impeller:impeller_unittests" ] } if (is_mac) { - public_deps += - [ "//flutter/shell/platform/darwin:flutter_channels_unittests" ] public_deps += [ + "//flutter/impeller/golden_tests:impeller_golden_tests", + "//flutter/shell/platform/darwin:flutter_channels_unittests", "//flutter/third_party/spring_animation:spring_animation_unittests", ] } From b881ff550142b074bd348183c441c513af6c1f88 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 22 Mar 2023 10:51:48 -0700 Subject: [PATCH 11/26] updated licenses --- ci/licenses_golden/licenses_flutter | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index bec4b4a6e7e75..eab793f77ceb4 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1315,6 +1315,19 @@ ORIGIN: ../../../flutter/impeller/geometry/type_traits.cc + ../../../flutter/LIC ORIGIN: ../../../flutter/impeller/geometry/type_traits.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/geometry/vector.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/geometry/vector.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests/golden_digest.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests/golden_digest.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests/golden_tests.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests/main.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests/metal_screenshot.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests/metal_screenshot.mm + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests/metal_screenshoter.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests/metal_screenshoter.mm + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests/working_directory.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests/working_directory.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/golden_tests_harvester/lib/logger.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/image/backends/skia/compressed_image_skia.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/image/backends/skia/compressed_image_skia.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/image/compressed_image.cc + ../../../flutter/LICENSE @@ -3853,6 +3866,19 @@ FILE: ../../../flutter/impeller/geometry/type_traits.cc FILE: ../../../flutter/impeller/geometry/type_traits.h FILE: ../../../flutter/impeller/geometry/vector.cc FILE: ../../../flutter/impeller/geometry/vector.h +FILE: ../../../flutter/impeller/golden_tests/golden_digest.cc +FILE: ../../../flutter/impeller/golden_tests/golden_digest.h +FILE: ../../../flutter/impeller/golden_tests/golden_tests.cc +FILE: ../../../flutter/impeller/golden_tests/main.cc +FILE: ../../../flutter/impeller/golden_tests/metal_screenshot.h +FILE: ../../../flutter/impeller/golden_tests/metal_screenshot.mm +FILE: ../../../flutter/impeller/golden_tests/metal_screenshoter.h +FILE: ../../../flutter/impeller/golden_tests/metal_screenshoter.mm +FILE: ../../../flutter/impeller/golden_tests/working_directory.cc +FILE: ../../../flutter/impeller/golden_tests/working_directory.h +FILE: ../../../flutter/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart +FILE: ../../../flutter/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart +FILE: ../../../flutter/impeller/golden_tests_harvester/lib/logger.dart FILE: ../../../flutter/impeller/image/backends/skia/compressed_image_skia.cc FILE: ../../../flutter/impeller/image/backends/skia/compressed_image_skia.h FILE: ../../../flutter/impeller/image/compressed_image.cc From 28ef8ff4697a90608d1b67c15a9dc0c0ba595d10 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 22 Mar 2023 10:56:04 -0700 Subject: [PATCH 12/26] lint fixes for run_tests.py --- testing/run_tests.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 7f52a7760394a..1eaafbc10c3e0 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -18,7 +18,7 @@ import re import subprocess import sys -import tempfile as tempfile +import tempfile import time import xvfb @@ -978,8 +978,8 @@ class DirectoryChange(): """ A scoped change in the CWD. """ - old_cwd: str = "" - new_cwd: str = "" + old_cwd: str = '' + new_cwd: str = '' def __init__(self, new_cwd: str): self.new_cwd = new_cwd @@ -988,7 +988,7 @@ def __enter__(self): self.old_cwd = os.getcwd() os.chdir(self.new_cwd) - def __exit__(self, type, value, traceback): + def __exit__(self, exception_type, exception_value, exception_traceback): os.chdir(self.old_cwd) @@ -996,21 +996,21 @@ def run_impeller_golden_tests(build_dir: str): """ Executes the impeller golden image tests from in the `variant` build. """ - tests_path: str = os.path.join(build_dir, "impeller_golden_tests") + tests_path: str = os.path.join(build_dir, 'impeller_golden_tests') if not os.path.exists(tests_path): raise Exception( - "Cannot find the \"impeller_golden_tests\" executable in \"%s\". You may need to build it." - % (variant_path) + 'Cannot find the "impeller_golden_tests" executable in "%s". You may need to build it.' + % (build_dir) ) - harvester_path: Path = Path(SCRIPT_DIR).parent.joinpath("impeller").joinpath( - "golden_tests_harvester" + harvester_path: Path = Path(SCRIPT_DIR).parent.joinpath('impeller').joinpath( + 'golden_tests_harvester' ) - with tempfile.TemporaryDirectory(prefix="impeller_golden") as temp_dir: - run_cmd([tests_path, "--working_dir=%s" % temp_dir]) + with tempfile.TemporaryDirectory(prefix='impeller_golden') as temp_dir: + run_cmd([tests_path, '--working_dir=%s' % temp_dir]) with DirectoryChange(harvester_path): - bin_path = Path(".").joinpath("bin" - ).joinpath("golden_tests_harvester.dart") - run_cmd(["dart", "run", str(bin_path), temp_dir]) + bin_path = Path(".").joinpath('bin' + ).joinpath('golden_tests_harvester.dart') + run_cmd(['dart', 'run', str(bin_path), temp_dir]) def main(): @@ -1219,7 +1219,7 @@ def main(): 'font-subset' in types) and args.variant not in variants_to_skip: run_cmd(['python3', 'test.py'], cwd=FONT_SUBSET_DIR) - if ('impeller-golden' in types): + if 'impeller-golden' in types: run_impeller_golden_tests(build_dir) From e9e796ae3ea008ed1f85817610bf593102e05b52 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 22 Mar 2023 11:19:25 -0700 Subject: [PATCH 13/26] updated licenses again --- ci/licenses_golden/excluded_files | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index ac858e2a3fca7..ea6915d8143f5 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -135,6 +135,12 @@ ../../../flutter/impeller/geometry/README.md ../../../flutter/impeller/geometry/geometry_unittests.cc ../../../flutter/impeller/geometry/geometry_unittests.h +../../../flutter/impeller/golden_tests/README.md +../../../flutter/impeller/golden_tests_harvester/.gitignore +../../../flutter/impeller/golden_tests_harvester/README.md +../../../flutter/impeller/golden_tests_harvester/analysis_options.yaml +../../../flutter/impeller/golden_tests_harvester/pubspec.yaml +../../../flutter/impeller/golden_tests_harvester/test ../../../flutter/impeller/image/README.md ../../../flutter/impeller/playground ../../../flutter/impeller/renderer/compute_subgroup_unittests.cc From 9d902bb13f365e0ebe36078aa85e4e0a4400a5dc Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 22 Mar 2023 11:25:31 -0700 Subject: [PATCH 14/26] more lints --- impeller/golden_tests/working_directory.cc | 2 +- impeller/golden_tests_harvester/pubspec.yaml | 2 +- testing/run_tests.py | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/impeller/golden_tests/working_directory.cc b/impeller/golden_tests/working_directory.cc index 42bf049e1bf5b..ac9e608cf98ad 100644 --- a/impeller/golden_tests/working_directory.cc +++ b/impeller/golden_tests/working_directory.cc @@ -32,4 +32,4 @@ void WorkingDirectory::SetPath(const std::string& path) { } } // namespace testing -} // namespace impeller \ No newline at end of file +} // namespace impeller diff --git a/impeller/golden_tests_harvester/pubspec.yaml b/impeller/golden_tests_harvester/pubspec.yaml index 016c1b46e9bc2..04e93ab85e450 100644 --- a/impeller/golden_tests_harvester/pubspec.yaml +++ b/impeller/golden_tests_harvester/pubspec.yaml @@ -37,4 +37,4 @@ dependency_overrides: process: path: ../../../third_party/pkg/process typed_data: - path: ../../../third_party/dart/third_party/pkg/typed_data \ No newline at end of file + path: ../../../third_party/dart/third_party/pkg/typed_data diff --git a/testing/run_tests.py b/testing/run_tests.py index 1eaafbc10c3e0..356c5c2477222 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -14,7 +14,7 @@ import glob import multiprocessing import os -from pathlib import Path +import platform import re import subprocess import sys @@ -22,6 +22,8 @@ import time import xvfb +from pathlib import Path + SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) BUILDROOT_DIR = os.path.abspath( os.path.join(os.path.realpath(__file__), '..', '..', '..') @@ -1008,7 +1010,7 @@ def run_impeller_golden_tests(build_dir: str): with tempfile.TemporaryDirectory(prefix='impeller_golden') as temp_dir: run_cmd([tests_path, '--working_dir=%s' % temp_dir]) with DirectoryChange(harvester_path): - bin_path = Path(".").joinpath('bin' + bin_path = Path('.').joinpath('bin' ).joinpath('golden_tests_harvester.dart') run_cmd(['dart', 'run', str(bin_path), temp_dir]) From e87078c12dd7b8b7977df7c0f73bda5f37bbf086 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 22 Mar 2023 11:33:54 -0700 Subject: [PATCH 15/26] unused import --- testing/run_tests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 356c5c2477222..828cce92e19c7 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -14,7 +14,6 @@ import glob import multiprocessing import os -import platform import re import subprocess import sys From 83c93fe836b2b3d3a796b9dae62ecdf4573f82c5 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 22 Mar 2023 11:48:06 -0700 Subject: [PATCH 16/26] sorted imports --- testing/run_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 828cce92e19c7..26af325110706 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -8,6 +8,8 @@ A top level harness to run all unit-tests in a specific engine build. """ +from pathlib import Path + import argparse import csv import errno @@ -21,8 +23,6 @@ import time import xvfb -from pathlib import Path - SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) BUILDROOT_DIR = os.path.abspath( os.path.join(os.path.realpath(__file__), '..', '..', '..') From 5735ac3fbd6864d453215be358e28f6f0075e1f6 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 22 Mar 2023 13:12:05 -0700 Subject: [PATCH 17/26] turned on the golden tests by default --- testing/run_tests.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 26af325110706..549a1d5a56704 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -16,6 +16,7 @@ import glob import multiprocessing import os +import platform import re import subprocess import sys @@ -1220,7 +1221,8 @@ def main(): 'font-subset' in types) and args.variant not in variants_to_skip: run_cmd(['python3', 'test.py'], cwd=FONT_SUBSET_DIR) - if 'impeller-golden' in types: + if 'impeller-golden' in types or ('engine' in types and + platform.system() == 'Darwin'): run_impeller_golden_tests(build_dir) From 1f863c92338c747f83cd22cd68255ed301b7e317 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 22 Mar 2023 15:08:08 -0700 Subject: [PATCH 18/26] fixed remote execution --- testing/run_tests.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 549a1d5a56704..977d11e612f39 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -1010,6 +1010,7 @@ def run_impeller_golden_tests(build_dir: str): with tempfile.TemporaryDirectory(prefix='impeller_golden') as temp_dir: run_cmd([tests_path, '--working_dir=%s' % temp_dir]) with DirectoryChange(harvester_path): + run_cmd(['dart', 'pub', 'get']) bin_path = Path('.').joinpath('bin' ).joinpath('golden_tests_harvester.dart') run_cmd(['dart', 'run', str(bin_path), temp_dir]) @@ -1222,7 +1223,8 @@ def main(): run_cmd(['python3', 'test.py'], cwd=FONT_SUBSET_DIR) if 'impeller-golden' in types or ('engine' in types and - platform.system() == 'Darwin'): + platform.system() == 'Darwin' and + args.variant == 'host_debug_unopt'): run_impeller_golden_tests(build_dir) From 3f00b5df471f7b2a10be22ab408cf2b4ea53090a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 22 Mar 2023 15:58:16 -0700 Subject: [PATCH 19/26] turned the tests off for the engine runs, going to specify it in the recipe since we have to change it anyways --- testing/run_tests.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 977d11e612f39..98509be2b153a 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -16,7 +16,6 @@ import glob import multiprocessing import os -import platform import re import subprocess import sys @@ -1222,9 +1221,7 @@ def main(): 'font-subset' in types) and args.variant not in variants_to_skip: run_cmd(['python3', 'test.py'], cwd=FONT_SUBSET_DIR) - if 'impeller-golden' in types or ('engine' in types and - platform.system() == 'Darwin' and - args.variant == 'host_debug_unopt'): + if 'impeller-golden' in types: run_impeller_golden_tests(build_dir) From 7d0058be7fe65a65071539db165bcd5612977b7a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 23 Mar 2023 10:29:06 -0700 Subject: [PATCH 20/26] added goldctl dependency to "Mac host engine" --- .ci.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.ci.yaml b/.ci.yaml index 9e5b32cf6e8b4..08008888276eb 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -363,6 +363,10 @@ targets: gclient_variables: >- {"download_emsdk": true} add_recipes_cq: "true" + dependencies: >- + [ + {"dependency": "goldctl", "version": "git_revision:3a77d0b12c697a840ca0c7705208e8622dc94603"} + ] build_host: "true" timeout: 75 From d0c5781f7e95c278c7806ae5dfe12fdf6b87970b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 23 Mar 2023 13:23:03 -0700 Subject: [PATCH 21/26] tried switching to engine_v2 to run the golden tests --- .ci.yaml | 8 ++++---- ci/builders/mac_host_engine.json | 18 ++++++++++++++++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/.ci.yaml b/.ci.yaml index 08008888276eb..f9a936f0e898a 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -363,10 +363,6 @@ targets: gclient_variables: >- {"download_emsdk": true} add_recipes_cq: "true" - dependencies: >- - [ - {"dependency": "goldctl", "version": "git_revision:3a77d0b12c697a840ca0c7705208e8622dc94603"} - ] build_host: "true" timeout: 75 @@ -384,6 +380,10 @@ targets: properties: release_build: "true" config_name: mac_host_engine + dependencies: >- + [ + {"dependency": "goldctl", "version": "git_revision:3a77d0b12c697a840ca0c7705208e8622dc94603"} + ] $flutter/osx_sdk : >- { "sdk_version": "14a5294e" } diff --git a/ci/builders/mac_host_engine.json b/ci/builders/mac_host_engine.json index a5957064929d0..01723e9c5b378 100644 --- a/ci/builders/mac_host_engine.json +++ b/ci/builders/mac_host_engine.json @@ -127,10 +127,24 @@ "flutter/shell/platform/darwin/macos:zip_macos_flutter_framework", "flutter/build/archives:archive_gen_snapshot", "flutter/build/archives:artifacts", - "flutter/tools/font-subset" + "flutter/tools/font-subset", + "flutter/impeller/golden_tests:impeller_golden_tests" ] }, - "tests": [] + "tests": [ + { + "language": "python3", + "name": "Impeller golden Tests for host_release", + "parameters": [ + "--variant", + "host_release", + "--type", + "impeller-golden" + ], + "script": "flutter/testing/run_tests.py", + "type": "local" + } + ] }, { "archives": [ From 386ddb48d444c3bbe143e97d06f82330322db863 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 23 Mar 2023 15:17:55 -0700 Subject: [PATCH 22/26] tried adding the dependency to the drone --- ci/builders/mac_host_engine.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ci/builders/mac_host_engine.json b/ci/builders/mac_host_engine.json index 01723e9c5b378..c0bb6e152ccb5 100644 --- a/ci/builders/mac_host_engine.json +++ b/ci/builders/mac_host_engine.json @@ -110,6 +110,9 @@ "os=Mac-12", "cpu=x86" ], + "dependencies": [ + {"dependency": "goldctl", "version": "git_revision:3a77d0b12c697a840ca0c7705208e8622dc94603"} + ], "gclient_custom_vars": { "download_android_deps": false }, From bfa220031b37b916702d9d13e796bcbf6ec09175 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 24 Mar 2023 10:51:07 -0700 Subject: [PATCH 23/26] tried flipping the image in ciimage since it wasn't working when writing out the image. --- impeller/golden_tests/metal_screenshot.mm | 7 ++----- impeller/golden_tests/metal_screenshoter.mm | 5 ++++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/impeller/golden_tests/metal_screenshot.mm b/impeller/golden_tests/metal_screenshot.mm index 4fb5069d3bc8e..db274a1de81e2 100644 --- a/impeller/golden_tests/metal_screenshot.mm +++ b/impeller/golden_tests/metal_screenshot.mm @@ -36,11 +36,8 @@ CGImageDestinationRef destination = CGImageDestinationCreateWithURL( (__bridge CFURLRef)output_url, kUTTypePNG, 1, nullptr); if (destination != nullptr) { - CGImageDestinationAddImage( - destination, cgImage_, (__bridge CFDictionaryRef) @{ - (__bridge NSString*)kCGImagePropertyOrientation : - @(kCGImagePropertyOrientationDownMirrored), - }); + CGImageDestinationAddImage(destination, cgImage_, + (__bridge CFDictionaryRef) @{}); if (CGImageDestinationFinalize(destination)) { result = true; diff --git a/impeller/golden_tests/metal_screenshoter.mm b/impeller/golden_tests/metal_screenshoter.mm index e5100a5b9d7c3..cd1a8bbd536ea 100644 --- a/impeller/golden_tests/metal_screenshoter.mm +++ b/impeller/golden_tests/metal_screenshoter.mm @@ -42,7 +42,10 @@ [CIContext contextWithMTLDevice:context_mtl->GetMTLDevice()]; FML_CHECK(context); - CGImageRef cgImage = [cicontext createCGImage:ciImage + CIImage* flipped = [ciImage + imageByApplyingOrientation:kCGImagePropertyOrientationDownMirrored]; + + CGImageRef cgImage = [cicontext createCGImage:flipped fromRect:[ciImage extent]]; return std::unique_ptr(new MetalScreenshot(cgImage)); From 8267da78413e7da19b58c64f7bb9731d96d5ee00 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 24 Mar 2023 10:57:16 -0700 Subject: [PATCH 24/26] switched to sending the filename to skia gold client, since the code seems to want that. --- .../golden_tests_harvester/lib/golden_tests_harvester.dart | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart b/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart index 5edf96b137bfb..85fb43b60cafe 100644 --- a/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart +++ b/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart @@ -29,16 +29,15 @@ Future harvest( final List> pendingComparisons = >[]; for (final Object? entry in entries) { final Map map = (entry as Map?)!; - final String testName = (map['testName'] as String?)!; final String filename = (map['filename'] as String?)!; final int width = (map['width'] as int?)!; final int height = (map['height'] as int?)!; final File goldenImage = File(p.join(workDirectory.path, filename)); final Future future = skiaGoldClient - .addImg(testName, goldenImage, screenshotSize: width * height) + .addImg(filename, goldenImage, screenshotSize: width * height) .catchError((dynamic err) { Logger.instance.log('skia gold comparison failed: $err'); - throw Exception('Failed comparison: $testName'); + throw Exception('Failed comparison: $filename'); }); pendingComparisons.add(future); } From 79cc6fdb730c30e63f9c2fd0520941f8da8c6b66 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 24 Mar 2023 13:52:46 -0700 Subject: [PATCH 25/26] zach feedback 1 --- ci/builders/mac_host_engine.json | 6 +++--- tools/pub_get_offline.py | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ci/builders/mac_host_engine.json b/ci/builders/mac_host_engine.json index c0bb6e152ccb5..a5e72ade6b8e2 100644 --- a/ci/builders/mac_host_engine.json +++ b/ci/builders/mac_host_engine.json @@ -127,11 +127,11 @@ "ninja": { "config": "host_release", "targets": [ - "flutter/shell/platform/darwin/macos:zip_macos_flutter_framework", "flutter/build/archives:archive_gen_snapshot", "flutter/build/archives:artifacts", - "flutter/tools/font-subset", - "flutter/impeller/golden_tests:impeller_golden_tests" + "flutter/impeller/golden_tests:impeller_golden_tests", + "flutter/shell/platform/darwin/macos:zip_macos_flutter_framework", + "flutter/tools/font-subset" ] }, "tests": [ diff --git a/tools/pub_get_offline.py b/tools/pub_get_offline.py index e1807ebb0cc1a..36790b281f57f 100644 --- a/tools/pub_get_offline.py +++ b/tools/pub_get_offline.py @@ -21,6 +21,7 @@ ALL_PACKAGES = [ os.path.join(ENGINE_DIR, "ci"), + os.path.join(ENGINE_DIR, "impeller", "golden_tests_harvester"), os.path.join(ENGINE_DIR, "flutter_frontend_server"), os.path.join(ENGINE_DIR, "shell", "vmservice"), os.path.join(ENGINE_DIR, "testing", "benchmark"), From 02b9f031d450f4fc96184558acd283aacd39b5bc Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 24 Mar 2023 15:32:02 -0700 Subject: [PATCH 26/26] updated licenses golden --- ci/licenses_golden/excluded_files | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index ea6915d8143f5..adee33e61a433 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -136,9 +136,11 @@ ../../../flutter/impeller/geometry/geometry_unittests.cc ../../../flutter/impeller/geometry/geometry_unittests.h ../../../flutter/impeller/golden_tests/README.md +../../../flutter/impeller/golden_tests_harvester/.dart_tool ../../../flutter/impeller/golden_tests_harvester/.gitignore ../../../flutter/impeller/golden_tests_harvester/README.md ../../../flutter/impeller/golden_tests_harvester/analysis_options.yaml +../../../flutter/impeller/golden_tests_harvester/pubspec.lock ../../../flutter/impeller/golden_tests_harvester/pubspec.yaml ../../../flutter/impeller/golden_tests_harvester/test ../../../flutter/impeller/image/README.md