Skip to content

Commit

Permalink
[Impeller] attempt to get validation errors from CI unittests. (flutt…
Browse files Browse the repository at this point in the history
…er#51341)

Enables Vulkan validation layers for macOS and linux builds, enables macOS validation for macOS builds.

Fixes flutter/flutter#144967

There may still be remaining issues with macOS validation, I need to verify I can reproduce flutter/flutter#143324 . I plan to do that separately
  • Loading branch information
jonahwilliams authored Mar 13, 2024
1 parent ed1413c commit 67e6328
Show file tree
Hide file tree
Showing 17 changed files with 171 additions and 161 deletions.
3 changes: 2 additions & 1 deletion ci/builders/linux_unopt.json
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@
"--target-dir",
"host_debug_unopt_impeller_tests",
"--rbe",
"--no-goma"
"--no-goma",
"--use-glfw-swiftshader"
],
"name": "host_debug_unopt_impeller_tests",
"ninja": {
Expand Down
5 changes: 3 additions & 2 deletions ci/builders/mac_unopt.json
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@
"arm64",
"--rbe",
"--no-goma",
"--xcode-symlinks"
"--xcode-symlinks",
"--use-glfw-swiftshader"
],
"name": "host_debug_unopt_arm64",
"ninja": {
Expand All @@ -157,7 +158,7 @@
"--variant",
"host_debug_unopt_arm64",
"--type",
"dart,dart-host,engine",
"dart,dart-host,engine,impeller-golden",
"--engine-capture-core-dump"
]
}
Expand Down
4 changes: 0 additions & 4 deletions impeller/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,15 @@ fml::StatusOr<Scalar> CalculateSigmaForBlurRadius(

class GaussianBlurFilterContentsTest : public EntityPlayground {
public:
std::shared_ptr<Texture> MakeTexture(const TextureDescriptor& desc) {
return GetContentContext()
->GetContext()
->GetResourceAllocator()
->CreateTexture(desc);
/// Create a texture that has been cleared to transparent black.
std::shared_ptr<Texture> MakeTexture(ISize size) {
auto render_target = GetContentContext()->MakeSubpass(
"Clear Subpass", size,
[](const ContentContext&, RenderPass&) { return true; });
if (render_target.ok()) {
return render_target.value().GetRenderTargetTexture();
}
return nullptr;
}
};
INSTANTIATE_PLAYGROUND_SUITE(GaussianBlurFilterContentsTest);
Expand Down Expand Up @@ -109,6 +113,7 @@ TEST(GaussianBlurFilterContentsTest, CoverageSimple) {
Entity entity;
std::optional<Rect> coverage =
contents.GetFilterCoverage(inputs, entity, /*effect_transform=*/Matrix());

ASSERT_EQ(coverage, Rect::MakeLTRB(10, 10, 110, 110));
}

Expand All @@ -125,45 +130,35 @@ TEST(GaussianBlurFilterContentsTest, CoverageWithSigma) {
Entity entity;
std::optional<Rect> coverage =
contents.GetFilterCoverage(inputs, entity, /*effect_transform=*/Matrix());

EXPECT_TRUE(coverage.has_value());
if (coverage.has_value()) {
EXPECT_RECT_NEAR(coverage.value(), Rect::MakeLTRB(99, 99, 201, 201));
}
}

TEST_P(GaussianBlurFilterContentsTest, CoverageWithTexture) {
TextureDescriptor desc = {
.storage_mode = StorageMode::kDevicePrivate,
.format = PixelFormat::kB8G8R8A8UNormInt,
.size = ISize(100, 100),
};
fml::StatusOr<Scalar> sigma_radius_1 =
CalculateSigmaForBlurRadius(1.0, Matrix());
ASSERT_TRUE(sigma_radius_1.ok());
GaussianBlurFilterContents contents(
/*sigma_X=*/sigma_radius_1.value(),
/*sigma_y=*/sigma_radius_1.value(), Entity::TileMode::kDecal,
FilterContents::BlurStyle::kNormal, /*mask_geometry=*/nullptr);
std::shared_ptr<Texture> texture =
GetContentContext()->GetContext()->GetResourceAllocator()->CreateTexture(
desc);
std::shared_ptr<Texture> texture = MakeTexture(ISize(100, 100));
FilterInput::Vector inputs = {FilterInput::Make(texture)};
Entity entity;
entity.SetTransform(Matrix::MakeTranslation({100, 100, 0}));
std::optional<Rect> coverage =
contents.GetFilterCoverage(inputs, entity, /*effect_transform=*/Matrix());

EXPECT_TRUE(coverage.has_value());
if (coverage.has_value()) {
EXPECT_RECT_NEAR(coverage.value(), Rect::MakeLTRB(99, 99, 201, 201));
}
}

TEST_P(GaussianBlurFilterContentsTest, CoverageWithEffectTransform) {
TextureDescriptor desc = {
.storage_mode = StorageMode::kDevicePrivate,
.format = PixelFormat::kB8G8R8A8UNormInt,
.size = ISize(100, 100),
};
Matrix effect_transform = Matrix::MakeScale({2.0, 2.0, 1.0});
fml::StatusOr<Scalar> sigma_radius_1 =
CalculateSigmaForBlurRadius(1.0, effect_transform);
Expand All @@ -172,9 +167,7 @@ TEST_P(GaussianBlurFilterContentsTest, CoverageWithEffectTransform) {
/*sigma_x=*/sigma_radius_1.value(),
/*sigma_y=*/sigma_radius_1.value(), Entity::TileMode::kDecal,
FilterContents::BlurStyle::kNormal, /*mask_geometry=*/nullptr);
std::shared_ptr<Texture> texture =
GetContentContext()->GetContext()->GetResourceAllocator()->CreateTexture(
desc);
std::shared_ptr<Texture> texture = MakeTexture(ISize(100, 100));
FilterInput::Vector inputs = {FilterInput::Make(texture)};
Entity entity;
entity.SetTransform(Matrix::MakeTranslation({100, 100, 0}));
Expand Down Expand Up @@ -218,12 +211,7 @@ TEST(GaussianBlurFilterContentsTest, CalculateSigmaValues) {
}

TEST_P(GaussianBlurFilterContentsTest, RenderCoverageMatchesGetCoverage) {
TextureDescriptor desc = {
.storage_mode = StorageMode::kDevicePrivate,
.format = PixelFormat::kB8G8R8A8UNormInt,
.size = ISize(100, 100),
};
std::shared_ptr<Texture> texture = MakeTexture(desc);
std::shared_ptr<Texture> texture = MakeTexture(ISize(100, 100));
fml::StatusOr<Scalar> sigma_radius_1 =
CalculateSigmaForBlurRadius(1.0, Matrix());
ASSERT_TRUE(sigma_radius_1.ok());
Expand Down Expand Up @@ -254,12 +242,7 @@ TEST_P(GaussianBlurFilterContentsTest, RenderCoverageMatchesGetCoverage) {

TEST_P(GaussianBlurFilterContentsTest,
RenderCoverageMatchesGetCoverageTranslate) {
TextureDescriptor desc = {
.storage_mode = StorageMode::kDevicePrivate,
.format = PixelFormat::kB8G8R8A8UNormInt,
.size = ISize(100, 100),
};
std::shared_ptr<Texture> texture = MakeTexture(desc);
std::shared_ptr<Texture> texture = MakeTexture(ISize(100, 100));
fml::StatusOr<Scalar> sigma_radius_1 =
CalculateSigmaForBlurRadius(1.0, Matrix());
ASSERT_TRUE(sigma_radius_1.ok());
Expand Down Expand Up @@ -292,12 +275,7 @@ TEST_P(GaussianBlurFilterContentsTest,

TEST_P(GaussianBlurFilterContentsTest,
RenderCoverageMatchesGetCoverageRotated) {
TextureDescriptor desc = {
.storage_mode = StorageMode::kDevicePrivate,
.format = PixelFormat::kB8G8R8A8UNormInt,
.size = ISize(400, 300),
};
std::shared_ptr<Texture> texture = MakeTexture(desc);
std::shared_ptr<Texture> texture = MakeTexture(ISize(400, 300));
fml::StatusOr<Scalar> sigma_radius_1 =
CalculateSigmaForBlurRadius(1.0, Matrix());
auto contents = std::make_unique<GaussianBlurFilterContents>(
Expand Down Expand Up @@ -329,12 +307,7 @@ TEST_P(GaussianBlurFilterContentsTest,
}

TEST_P(GaussianBlurFilterContentsTest, CalculateUVsSimple) {
TextureDescriptor desc = {
.storage_mode = StorageMode::kDevicePrivate,
.format = PixelFormat::kB8G8R8A8UNormInt,
.size = ISize(100, 100),
};
std::shared_ptr<Texture> texture = MakeTexture(desc);
std::shared_ptr<Texture> texture = MakeTexture(ISize(100, 100));
auto filter_input = FilterInput::Make(texture);
Entity entity;
Quad uvs = GaussianBlurFilterContents::CalculateUVs(
Expand All @@ -347,13 +320,7 @@ TEST_P(GaussianBlurFilterContentsTest, CalculateUVsSimple) {
}

TEST_P(GaussianBlurFilterContentsTest, TextureContentsWithDestinationRect) {
TextureDescriptor desc = {
.storage_mode = StorageMode::kDevicePrivate,
.format = PixelFormat::kB8G8R8A8UNormInt,
.size = ISize(100, 100),
};

std::shared_ptr<Texture> texture = MakeTexture(desc);
std::shared_ptr<Texture> texture = MakeTexture(ISize(100, 100));
auto texture_contents = std::make_shared<TextureContents>();
texture_contents->SetSourceRect(Rect::MakeSize(texture->GetSize()));
texture_contents->SetTexture(texture);
Expand Down Expand Up @@ -388,13 +355,7 @@ TEST_P(GaussianBlurFilterContentsTest, TextureContentsWithDestinationRect) {

TEST_P(GaussianBlurFilterContentsTest,
TextureContentsWithDestinationRectScaled) {
TextureDescriptor desc = {
.storage_mode = StorageMode::kDevicePrivate,
.format = PixelFormat::kB8G8R8A8UNormInt,
.size = ISize(100, 100),
};

std::shared_ptr<Texture> texture = MakeTexture(desc);
std::shared_ptr<Texture> texture = MakeTexture(ISize(100, 100));
auto texture_contents = std::make_shared<TextureContents>();
texture_contents->SetSourceRect(Rect::MakeSize(texture->GetSize()));
texture_contents->SetTexture(texture);
Expand Down Expand Up @@ -429,14 +390,8 @@ TEST_P(GaussianBlurFilterContentsTest,
}

TEST_P(GaussianBlurFilterContentsTest, TextureContentsWithEffectTransform) {
TextureDescriptor desc = {
.storage_mode = StorageMode::kDevicePrivate,
.format = PixelFormat::kB8G8R8A8UNormInt,
.size = ISize(100, 100),
};

Matrix effect_transform = Matrix::MakeScale({2.0, 2.0, 1.0});
std::shared_ptr<Texture> texture = MakeTexture(desc);
std::shared_ptr<Texture> texture = MakeTexture(ISize(100, 100));
auto texture_contents = std::make_shared<TextureContents>();
texture_contents->SetSourceRect(Rect::MakeSize(texture->GetSize()));
texture_contents->SetTexture(texture);
Expand Down
23 changes: 12 additions & 11 deletions impeller/entity/entity_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2167,7 +2167,6 @@ TEST_P(EntityTest, RuntimeEffect) {

auto contents = std::make_shared<RuntimeEffectContents>();
contents->SetGeometry(Geometry::MakeCover());

contents->SetRuntimeStage(runtime_stage);

struct FragUniforms {
Expand Down Expand Up @@ -2643,13 +2642,15 @@ TEST_P(EntityTest, AdvancedBlendCoverageHintIsNotResetByEntityPass) {
}

TEST_P(EntityTest, SpecializationConstantsAreAppliedToVariants) {
auto content_context =
ContentContext(GetContext(), TypographerContextSkia::Make());
auto content_context = GetContentContext();

auto default_color_burn = content_context.GetBlendColorBurnPipeline(
{.has_depth_stencil_attachments = false});
auto alt_color_burn = content_context.GetBlendColorBurnPipeline(
{.has_depth_stencil_attachments = true});
auto default_color_burn = content_context->GetBlendColorBurnPipeline({
.color_attachment_pixel_format = PixelFormat::kR8G8B8A8UNormInt,
.has_depth_stencil_attachments = false,
});
auto alt_color_burn = content_context->GetBlendColorBurnPipeline(
{.color_attachment_pixel_format = PixelFormat::kR8G8B8A8UNormInt,
.has_depth_stencil_attachments = true});

ASSERT_NE(default_color_burn, alt_color_burn);
ASSERT_EQ(default_color_burn->GetDescriptor().GetSpecializationConstants(),
Expand All @@ -2663,10 +2664,10 @@ TEST_P(EntityTest, SpecializationConstantsAreAppliedToVariants) {
}

TEST_P(EntityTest, DecalSpecializationAppliedToMorphologyFilter) {
auto content_context =
ContentContext(GetContext(), TypographerContextSkia::Make());

auto default_color_burn = content_context.GetMorphologyFilterPipeline({});
auto content_context = GetContentContext();
auto default_color_burn = content_context->GetMorphologyFilterPipeline({
.color_attachment_pixel_format = PixelFormat::kR8G8B8A8UNormInt,
});

auto decal_supported = static_cast<Scalar>(
GetContext()->GetCapabilities()->SupportsDecalSamplerAddressMode());
Expand Down
9 changes: 6 additions & 3 deletions impeller/entity/geometry/point_field_geometry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "impeller/entity/geometry/point_field_geometry.h"

#include "impeller/geometry/color.h"
#include "impeller/renderer/command_buffer.h"

namespace impeller {
Expand Down Expand Up @@ -157,7 +158,8 @@ GeometryResult PointFieldGeometry::GetPositionBufferGPU(
DefaultUniformAlignment());

BufferView geometry_buffer =
host_buffer.Emplace(nullptr, total * sizeof(Point), alignof(Point));
host_buffer.Emplace(nullptr, total * sizeof(Point),
std::max(DefaultUniformAlignment(), alignof(Point)));

BufferView output;
{
Expand Down Expand Up @@ -185,8 +187,9 @@ GeometryResult PointFieldGeometry::GetPositionBufferGPU(
}

if (texture_coverage.has_value() && effect_transform.has_value()) {
BufferView geometry_uv_buffer =
host_buffer.Emplace(nullptr, total * sizeof(Vector4), alignof(Vector4));
BufferView geometry_uv_buffer = host_buffer.Emplace(
nullptr, total * sizeof(Vector4),
std::max(DefaultUniformAlignment(), alignof(Vector4)));

using UV = UvComputeShader;

Expand Down
1 change: 0 additions & 1 deletion impeller/golden_tests/golden_playground_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include <memory>

#include "flutter/fml/macros.h"
#include "flutter/impeller/aiks/aiks_context.h"
#include "flutter/impeller/playground/playground.h"
#include "flutter/impeller/renderer/render_target.h"
Expand Down
2 changes: 0 additions & 2 deletions impeller/golden_tests/golden_playground_test_mac.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ static const std::vector<std::string> kSkipTests = {
"impeller_Play_AiksTest_CanRenderClippedRuntimeEffects_Vulkan",
};

static const std::vector<std::string> kVulkanDenyValidationTests = {};

namespace {
std::string GetTestName() {
std::string suite_name =
Expand Down
1 change: 1 addition & 0 deletions impeller/playground/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ impeller_component("playground") {
"image:image_skia_backend",
"imgui:imgui_impeller_backend",
"//flutter/fml",
"//flutter/testing:testing_lib",
"//flutter/third_party/glfw",
"//flutter/third_party/imgui:imgui_glfw",
]
Expand Down
29 changes: 17 additions & 12 deletions impeller/playground/backend/vulkan/playground_impl_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,7 @@ void PlaygroundImplVK::DestroyWindowHandle(WindowHandle handle) {

PlaygroundImplVK::PlaygroundImplVK(PlaygroundSwitches switches)
: PlaygroundImpl(switches), handle_(nullptr, &DestroyWindowHandle) {
if (!::glfwVulkanSupported()) {
#ifdef TARGET_OS_MAC
VALIDATION_LOG << "Attempted to initialize a Vulkan playground on macOS "
"where Vulkan cannot be found. It can be installed via "
"MoltenVK and make sure to install it globally so "
"dlopen can find it.";
#else
VALIDATION_LOG << "Attempted to initialize a Vulkan playground on a system "
"that does not support Vulkan.";
#endif
return;
}
FML_CHECK(IsVulkanDriverPresent());

InitGlobalVulkanInstance();

Expand Down Expand Up @@ -224,4 +213,20 @@ fml::Status PlaygroundImplVK::SetCapabilities(
"PlaygroundImplVK doesn't support setting the capabilities.");
}

bool PlaygroundImplVK::IsVulkanDriverPresent() {
if (::glfwVulkanSupported()) {
return true;
}
#ifdef TARGET_OS_MAC
FML_LOG(ERROR) << "Attempting to initialize a Vulkan playground on macOS "
"where Vulkan cannot be found. It can be installed via "
"MoltenVK and make sure to install it globally so "
"dlopen can find it.";
#else // TARGET_OS_MAC
FML_LOG(ERROR) << "Attempting to initialize a Vulkan playground on a system "
"that does not support Vulkan.";
#endif // TARGET_OS_MAC
return false;
}

} // namespace impeller
3 changes: 2 additions & 1 deletion impeller/playground/backend/vulkan/playground_impl_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@
#ifndef FLUTTER_IMPELLER_PLAYGROUND_BACKEND_VULKAN_PLAYGROUND_IMPL_VK_H_
#define FLUTTER_IMPELLER_PLAYGROUND_BACKEND_VULKAN_PLAYGROUND_IMPL_VK_H_

#include "flutter/fml/macros.h"
#include "impeller/playground/playground_impl.h"
#include "impeller/renderer/backend/vulkan/vk.h"

namespace impeller {

class PlaygroundImplVK final : public PlaygroundImpl {
public:
static bool IsVulkanDriverPresent();

explicit PlaygroundImplVK(PlaygroundSwitches switches);

~PlaygroundImplVK();
Expand Down
Loading

0 comments on commit 67e6328

Please sign in to comment.