Skip to content

Commit

Permalink
[Impeller] flutter_tester --enable-impeller (flutter#46389)
Browse files Browse the repository at this point in the history
This patch does the following:

- Updates `flutter_tester` to set up an Impeller rendering context and surface if `--enable-impeller` is set to true, using the Vulkan backend with Swiftshader.
- Updates `run_tests.py` to run all tests except the smoke test (that one really has no rendering impact whatsoever) with and without `--enable-impeller`.
- Updates a few tests to work that were trivial:
  - A couple tests needed updated goldens for very minor rendering differences. Filed flutter/flutter#135684 to track using Skia gold for this instead.
  - Disabled SKP screenshotting if Impeller is enabled, and updated the test checking that to verify an error is thrown if an SKP is requested.
  - The Dart GPU based test now asserts that the gpu context is available if Impeller is enabled, and does not deadlock if run in a single threaded mode.
  - We were missing some trace events around `Canvas::SaveLayer` for Impeller as compared to Skia.
  - A couple other tests had strict checks about exception messages that are slightly different between Skia and Impeller.
- I've filed bugs for other tests that may require a little more work, and skipped them for now. For FragmentProgram on Vulkan I reused an existing bug.

This is part of my attempt to address flutter/flutter#135693, although @chinmaygarde and I had slightly different ideas about how to do this.

The goals here are:

- Run the Dart unit tests we already have with Impeller enabled.
- Enable running more of the framework tests (including gold tests) with Impeller enabled.
- Run all of these tests via public `dart:ui` API rather than mucking around in C++ internals in the engine.
  • Loading branch information
dnfield authored Oct 11, 2023
1 parent 5e788a8 commit 791e90a
Show file tree
Hide file tree
Showing 38 changed files with 478 additions and 66 deletions.
2 changes: 2 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -4043,6 +4043,7 @@ ORIGIN: ../../../flutter/vulkan/procs/vulkan_interface.cc + ../../../flutter/LIC
ORIGIN: ../../../flutter/vulkan/procs/vulkan_interface.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/vulkan/procs/vulkan_proc_table.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/vulkan/procs/vulkan_proc_table.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/vulkan/swiftshader_path.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/vulkan/vulkan_application.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/vulkan/vulkan_application.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/vulkan/vulkan_backbuffer.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -6828,6 +6829,7 @@ FILE: ../../../flutter/vulkan/procs/vulkan_interface.cc
FILE: ../../../flutter/vulkan/procs/vulkan_interface.h
FILE: ../../../flutter/vulkan/procs/vulkan_proc_table.cc
FILE: ../../../flutter/vulkan/procs/vulkan_proc_table.h
FILE: ../../../flutter/vulkan/swiftshader_path.h
FILE: ../../../flutter/vulkan/vulkan_application.cc
FILE: ../../../flutter/vulkan/vulkan_application.h
FILE: ../../../flutter/vulkan/vulkan_backbuffer.cc
Expand Down
4 changes: 4 additions & 0 deletions impeller/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ config("impeller_public_config") {
defines += [ "IMPELLER_ENABLE_VULKAN=1" ]
}

if (impeller_enable_vulkan_playgrounds) {
defines += [ "IMPELLER_ENABLE_VULKAN_PLAYGROUNDS=1" ]
}

if (impeller_trace_all_gl_calls) {
defines += [ "IMPELLER_TRACE_ALL_GL_CALLS" ]
}
Expand Down
2 changes: 2 additions & 0 deletions impeller/aiks/canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <utility>

#include "flutter/fml/logging.h"
#include "flutter/fml/trace_event.h"
#include "impeller/aiks/image_filter.h"
#include "impeller/aiks/paint_pass_delegate.h"
#include "impeller/entity/contents/atlas_contents.h"
Expand Down Expand Up @@ -536,6 +537,7 @@ size_t Canvas::GetClipDepth() const {
void Canvas::SaveLayer(const Paint& paint,
std::optional<Rect> bounds,
const std::shared_ptr<ImageFilter>& backdrop_filter) {
TRACE_EVENT0("flutter", "Canvas::saveLayer");
Save(true, paint.blend_mode, backdrop_filter);

auto& new_layer_pass = GetCurrentPass();
Expand Down
2 changes: 1 addition & 1 deletion impeller/base/validation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ void ImpellerValidationBreak(const char* message) {
} else {
FML_LOG(ERROR) << stream.str();
}
#endif // IMPELLER_DEBUG
#endif // IMPELLER_ENABLE_VALIDATION
}

} // namespace impeller
2 changes: 1 addition & 1 deletion impeller/playground/playground.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ bool Playground::SupportsBackend(PlaygroundBackend backend) {
return false;
#endif // IMPELLER_ENABLE_OPENGLES
case PlaygroundBackend::kVulkan:
#if IMPELLER_ENABLE_VULKAN
#if IMPELLER_ENABLE_VULKAN && IMPELLER_ENABLE_VULKAN_PLAYGROUNDS
return true;
#else // IMPELLER_ENABLE_VULKAN
return false;
Expand Down
1 change: 1 addition & 0 deletions impeller/renderer/backend/vulkan/allocator_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ class AllocatedTextureSourceVK final : public TextureSourceVK {
vk::Device device,
bool supports_memoryless_textures)
: TextureSourceVK(desc), resource_(std::move(resource_manager)) {
FML_DCHECK(desc.format != PixelFormat::kUnknown);
TRACE_EVENT0("impeller", "CreateDeviceTexture");
vk::ImageCreateInfo image_info;
image_info.flags = ToVKImageCreateFlags(desc.type);
Expand Down
5 changes: 5 additions & 0 deletions impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ TEST(BlitCommandVkTest, BlitCopyTextureToTextureCommandVK) {
auto encoder = std::make_unique<CommandEncoderFactoryVK>(context)->Create();
BlitCopyTextureToTextureCommandVK cmd;
cmd.source = context->GetResourceAllocator()->CreateTexture({
.format = PixelFormat::kR8G8B8A8UNormInt,
.size = ISize(100, 100),
});
cmd.destination = context->GetResourceAllocator()->CreateTexture({
.format = PixelFormat::kR8G8B8A8UNormInt,
.size = ISize(100, 100),
});
bool result = cmd.Encode(*encoder.get());
Expand All @@ -32,6 +34,7 @@ TEST(BlitCommandVkTest, BlitCopyTextureToBufferCommandVK) {
auto encoder = std::make_unique<CommandEncoderFactoryVK>(context)->Create();
BlitCopyTextureToBufferCommandVK cmd;
cmd.source = context->GetResourceAllocator()->CreateTexture({
.format = PixelFormat::kR8G8B8A8UNormInt,
.size = ISize(100, 100),
});
cmd.destination = context->GetResourceAllocator()->CreateBuffer({
Expand All @@ -48,6 +51,7 @@ TEST(BlitCommandVkTest, BlitCopyBufferToTextureCommandVK) {
auto encoder = std::make_unique<CommandEncoderFactoryVK>(context)->Create();
BlitCopyBufferToTextureCommandVK cmd;
cmd.destination = context->GetResourceAllocator()->CreateTexture({
.format = PixelFormat::kR8G8B8A8UNormInt,
.size = ISize(100, 100),
});
cmd.source = context->GetResourceAllocator()
Expand All @@ -66,6 +70,7 @@ TEST(BlitCommandVkTest, BlitGenerateMipmapCommandVK) {
auto encoder = std::make_unique<CommandEncoderFactoryVK>(context)->Create();
BlitGenerateMipmapCommandVK cmd;
cmd.texture = context->GetResourceAllocator()->CreateTexture({
.format = PixelFormat::kR8G8B8A8UNormInt,
.size = ISize(100, 100),
.mip_count = 2,
});
Expand Down
4 changes: 4 additions & 0 deletions impeller/renderer/backend/vulkan/surface_context_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ bool SurfaceContextVK::SetWindowSurface(vk::UniqueSurfaceKHR surface) {
VALIDATION_LOG << "Could not create swapchain.";
return false;
}
if (!swapchain->IsValid()) {
VALIDATION_LOG << "Could not create valid swapchain.";
return false;
}
swapchain_ = std::move(swapchain);
return true;
}
Expand Down
1 change: 1 addition & 0 deletions impeller/renderer/backend/vulkan/swapchain_impl_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ SwapchainImplVK::SwapchainImplVK(
vk::SwapchainKHR old_swapchain,
vk::SurfaceTransformFlagBitsKHR last_transform) {
if (!context) {
VALIDATION_LOG << "Cannot create a swapchain without a context.";
return;
}

Expand Down
2 changes: 2 additions & 0 deletions impeller/renderer/backend/vulkan/swapchain_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "impeller/renderer/backend/vulkan/swapchain_vk.h"

#include "flutter/fml/trace_event.h"
#include "impeller/base/validation.h"
#include "impeller/renderer/backend/vulkan/swapchain_impl_vk.h"

namespace impeller {
Expand All @@ -14,6 +15,7 @@ std::shared_ptr<SwapchainVK> SwapchainVK::Create(
vk::UniqueSurfaceKHR surface) {
auto impl = SwapchainImplVK::Create(context, std::move(surface));
if (!impl || !impl->IsValid()) {
VALIDATION_LOG << "Failed to create SwapchainVK implementation.";
return nullptr;
}
return std::shared_ptr<SwapchainVK>(new SwapchainVK(std::move(impl)));
Expand Down
9 changes: 8 additions & 1 deletion impeller/tools/impeller.gni
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,14 @@ declare_args() {
(is_linux || is_win || is_android) && target_os != "fuchsia"

# Whether the Vulkan backend is enabled.
impeller_enable_vulkan =
impeller_enable_vulkan = (is_linux || is_win || is_android ||
enable_unittests) && target_os != "fuchsia"

# Whether playgrounds should run with Vulkan.
#
# impeller_enable_vulkan may be true in build environments that run tests but
# do not have a Vulkan ICD present.
impeller_enable_vulkan_playgrounds =
(is_linux || is_win || is_android) && target_os != "fuchsia"

# Whether to use a prebuilt impellerc.
Expand Down
3 changes: 2 additions & 1 deletion lib/gpu/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ Dart_Handle InternalFlutterGpu_Context_InitializeDefault(Dart_Handle wrapper) {
// Grab the Impeller context from the IO manager.
std::promise<std::shared_ptr<impeller::Context>> context_promise;
auto impeller_context_future = context_promise.get_future();
dart_state->GetTaskRunners().GetIOTaskRunner()->PostTask(
fml::TaskRunner::RunNowOrPostTask(
dart_state->GetTaskRunners().GetIOTaskRunner(),
fml::MakeCopyable([promise = std::move(context_promise),
io_manager = dart_state->GetIOManager()]() mutable {
promise.set_value(io_manager ? io_manager->GetImpellerContext()
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added lib/ui/fixtures/impeller_heart_end.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 5 additions & 0 deletions shell/common/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1695,6 +1695,11 @@ bool Shell::OnServiceProtocolScreenshotSKP(
const ServiceProtocol::Handler::ServiceProtocolMap& params,
rapidjson::Document* response) {
FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread());
if (settings_.enable_impeller) {
ServiceProtocolFailureError(
response, "Cannot capture SKP screenshot with Impeller enabled.");
return false;
}
auto screenshot = rasterizer_->ScreenshotLastLayerTree(
Rasterizer::ScreenshotType::SkiaPicture, true);
if (screenshot.data) {
Expand Down
6 changes: 1 addition & 5 deletions shell/common/shell_test_platform_view_vulkan.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,8 @@

#if OS_FUCHSIA
#define VULKAN_SO_PATH "libvulkan.so"
#elif FML_OS_MACOSX
#define VULKAN_SO_PATH "libvk_swiftshader.dylib"
#elif FML_OS_WIN
#define VULKAN_SO_PATH "vk_swiftshader.dll"
#else
#define VULKAN_SO_PATH "libvk_swiftshader.so"
#include "flutter/vulkan/swiftshader_path.h"
#endif

namespace flutter {
Expand Down
4 changes: 4 additions & 0 deletions shell/gpu/gpu_surface_vulkan.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ sk_sp<SkSurface> GPUSurfaceVulkan::CreateSurfaceFromVulkanImage(
const VkImage image,
const VkFormat format,
const SkISize& size) {
#ifdef SK_VULKAN
GrVkImageInfo image_info = {
.fImage = image,
.fImageTiling = VK_IMAGE_TILING_OPTIMAL,
Expand All @@ -130,6 +131,9 @@ sk_sp<SkSurface> GPUSurfaceVulkan::CreateSurfaceFromVulkanImage(
SkColorSpace::MakeSRGB(), // color space
&surface_properties // surface properties
);
#else
return nullptr;
#endif // SK_VULKAN
}

SkColorType GPUSurfaceVulkan::ColorTypeFromFormat(const VkFormat format) {
Expand Down
5 changes: 5 additions & 0 deletions shell/gpu/gpu_surface_vulkan_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceVulkanImpeller::AcquireFrame(
auto& context_vk = impeller::SurfaceContextVK::Cast(*impeller_context_);
std::unique_ptr<impeller::Surface> surface = context_vk.AcquireNextSurface();

if (!surface) {
FML_LOG(ERROR) << "No surface available.";
return nullptr;
}

SurfaceFrame::SubmitCallback submit_callback =
fml::MakeCopyable([renderer = impeller_renderer_, //
aiks_context = aiks_context_, //
Expand Down
20 changes: 19 additions & 1 deletion shell/testing/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@
# found in the LICENSE file.

import("//build/fuchsia/sdk.gni")
import("//flutter/impeller/tools/impeller.gni")
import("//flutter/shell/gpu/gpu.gni")

shell_gpu_configuration("tester_gpu_configuration") {
enable_software = true
enable_gl = true
enable_vulkan = true
enable_metal = false
}

executable("testing") {
output_name = "flutter_tester"
Expand All @@ -13,8 +22,9 @@ executable("testing") {
]

sources = [ "tester_main.cc" ]
libs = []
if (is_win) {
libs = [
libs += [
"psapi.lib",
"user32.lib",
"FontSub.lib",
Expand All @@ -36,6 +46,14 @@ executable("testing") {
"//third_party/skia",
]

if (impeller_supports_rendering) {
deps += [
":tester_gpu_configuration",
"//flutter/impeller",
"//third_party/swiftshader",
]
}

metadata = {
entitlement_file_path = [ "flutter_tester" ]
}
Expand Down
Loading

0 comments on commit 791e90a

Please sign in to comment.