Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] Allow toggling vulkan validation using a command line test flag. #40728

Merged
merged 3 commits into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion impeller/golden_tests/metal_screenshoter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@

MetalScreenshoter::MetalScreenshoter() {
FML_CHECK(::glfwInit() == GLFW_TRUE);
playground_ = PlaygroundImpl::Create(PlaygroundBackend::kMetal);
playground_ =
PlaygroundImpl::Create(PlaygroundBackend::kMetal, PlaygroundSwitches{});
aiks_context_.reset(new AiksContext(playground_->GetContext()));
}

Expand Down
5 changes: 3 additions & 2 deletions impeller/playground/backend/gles/playground_impl_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ void PlaygroundImplGLES::DestroyWindowHandle(WindowHandle handle) {
::glfwDestroyWindow(reinterpret_cast<GLFWwindow*>(handle));
}

PlaygroundImplGLES::PlaygroundImplGLES()
: handle_(nullptr, &DestroyWindowHandle),
PlaygroundImplGLES::PlaygroundImplGLES(PlaygroundSwitches switches)
: PlaygroundImpl(switches),
handle_(nullptr, &DestroyWindowHandle),
worker_(std::shared_ptr<ReactorWorker>(new ReactorWorker())) {
::glfwDefaultWindowHints();

Expand Down
2 changes: 1 addition & 1 deletion impeller/playground/backend/gles/playground_impl_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace impeller {

class PlaygroundImplGLES final : public PlaygroundImpl {
public:
PlaygroundImplGLES();
explicit PlaygroundImplGLES(PlaygroundSwitches switches);

~PlaygroundImplGLES();

Expand Down
2 changes: 1 addition & 1 deletion impeller/playground/backend/metal/playground_impl_mtl.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace impeller {

class PlaygroundImplMTL final : public PlaygroundImpl {
public:
PlaygroundImplMTL();
explicit PlaygroundImplMTL(PlaygroundSwitches switches);

~PlaygroundImplMTL();

Expand Down
6 changes: 4 additions & 2 deletions impeller/playground/backend/metal/playground_impl_mtl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,10 @@
::glfwDestroyWindow(reinterpret_cast<GLFWwindow*>(handle));
}

PlaygroundImplMTL::PlaygroundImplMTL()
: handle_(nullptr, &DestroyWindowHandle), data_(std::make_unique<Data>()) {
PlaygroundImplMTL::PlaygroundImplMTL(PlaygroundSwitches switches)
: PlaygroundImpl(switches),
handle_(nullptr, &DestroyWindowHandle),
data_(std::make_unique<Data>()) {
::glfwDefaultWindowHints();
::glfwWindowHint(GLFW_CLIENT_API, GLFW_NO_API);
::glfwWindowHint(GLFW_VISIBLE, GLFW_FALSE);
Expand Down
6 changes: 4 additions & 2 deletions impeller/playground/backend/vulkan/playground_impl_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ void PlaygroundImplVK::DestroyWindowHandle(WindowHandle handle) {
::glfwDestroyWindow(reinterpret_cast<GLFWwindow*>(handle));
}

PlaygroundImplVK::PlaygroundImplVK()
: concurrent_loop_(fml::ConcurrentMessageLoop::Create()),
PlaygroundImplVK::PlaygroundImplVK(PlaygroundSwitches switches)
: PlaygroundImpl(switches),
concurrent_loop_(fml::ConcurrentMessageLoop::Create()),
handle_(nullptr, &DestroyWindowHandle) {
if (!::glfwVulkanSupported()) {
VALIDATION_LOG << "Attempted to initialize a Vulkan playground on a system "
Expand All @@ -76,6 +77,7 @@ PlaygroundImplVK::PlaygroundImplVK()
context_settings.shader_libraries_data = ShaderLibraryMappingsForPlayground();
context_settings.cache_directory = fml::paths::GetCachesDirectory();
context_settings.worker_task_runner = concurrent_loop_->GetTaskRunner();
context_settings.enable_validation = switches_.enable_vulkan_validation;

auto context = ContextVK::Create(std::move(context_settings));

Expand Down
2 changes: 1 addition & 1 deletion impeller/playground/backend/vulkan/playground_impl_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace impeller {

class PlaygroundImplVK final : public PlaygroundImpl {
public:
PlaygroundImplVK();
explicit PlaygroundImplVK(PlaygroundSwitches switches);

~PlaygroundImplVK();

Expand Down
4 changes: 3 additions & 1 deletion impeller/playground/compute_playground_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@

#include "flutter/fml/time/time_point.h"

#include "flutter/testing/test_args.h"
#include "impeller/playground/compute_playground_test.h"

namespace impeller {

ComputePlaygroundTest::ComputePlaygroundTest() = default;
ComputePlaygroundTest::ComputePlaygroundTest()
: Playground(PlaygroundSwitches{flutter::testing::GetArgsForProcess()}) {}

ComputePlaygroundTest::~ComputePlaygroundTest() = default;

Expand Down
7 changes: 4 additions & 3 deletions impeller/playground/playground.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ struct Playground::GLFWInitializer {
}
};

Playground::Playground()
: glfw_initializer_(std::make_unique<GLFWInitializer>()) {}
Playground::Playground(PlaygroundSwitches switches)
: switches_(switches),
glfw_initializer_(std::make_unique<GLFWInitializer>()) {}

Playground::~Playground() = default;

Expand Down Expand Up @@ -109,7 +110,7 @@ bool Playground::SupportsBackend(PlaygroundBackend backend) {
void Playground::SetupContext(PlaygroundBackend backend) {
FML_CHECK(SupportsBackend(backend));

impl_ = PlaygroundImpl::Create(backend);
impl_ = PlaygroundImpl::Create(backend, switches_);
if (!impl_) {
return;
}
Expand Down
5 changes: 4 additions & 1 deletion impeller/playground/playground.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "impeller/geometry/point.h"
#include "impeller/image/compressed_image.h"
#include "impeller/image/decompressed_image.h"
#include "impeller/playground/switches.h"
#include "impeller/renderer/renderer.h"
#include "impeller/renderer/texture.h"
#include "impeller/runtime_stage/runtime_stage.h"
Expand All @@ -33,7 +34,7 @@ class Playground {
public:
using SinglePassCallback = std::function<bool(RenderPass& pass)>;

explicit Playground();
explicit Playground(PlaygroundSwitches switches);

virtual ~Playground();

Expand Down Expand Up @@ -92,6 +93,8 @@ class Playground {
virtual std::string GetWindowTitle() const = 0;

protected:
const PlaygroundSwitches switches_;

virtual bool ShouldKeepRendering() const;

private:
Expand Down
12 changes: 7 additions & 5 deletions impeller/playground/playground_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,20 @@
namespace impeller {

std::unique_ptr<PlaygroundImpl> PlaygroundImpl::Create(
PlaygroundBackend backend) {
PlaygroundBackend backend,
PlaygroundSwitches switches) {
switch (backend) {
#if IMPELLER_ENABLE_METAL
case PlaygroundBackend::kMetal:
return std::make_unique<PlaygroundImplMTL>();
return std::make_unique<PlaygroundImplMTL>(switches);
#endif // IMPELLER_ENABLE_METAL
#if IMPELLER_ENABLE_OPENGLES
case PlaygroundBackend::kOpenGLES:
return std::make_unique<PlaygroundImplGLES>();
return std::make_unique<PlaygroundImplGLES>(switches);
#endif // IMPELLER_ENABLE_OPENGLES
#if IMPELLER_ENABLE_VULKAN
case PlaygroundBackend::kVulkan:
return std::make_unique<PlaygroundImplVK>();
return std::make_unique<PlaygroundImplVK>(switches);
#endif // IMPELLER_ENABLE_VULKAN
default:
FML_CHECK(false) << "Attempted to create playground with backend that "
Expand All @@ -44,7 +45,8 @@ std::unique_ptr<PlaygroundImpl> PlaygroundImpl::Create(
FML_UNREACHABLE();
}

PlaygroundImpl::PlaygroundImpl() = default;
PlaygroundImpl::PlaygroundImpl(PlaygroundSwitches switches)
: switches_(switches) {}

PlaygroundImpl::~PlaygroundImpl() = default;

Expand Down
8 changes: 6 additions & 2 deletions impeller/playground/playground_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@

#include "flutter/fml/macros.h"
#include "impeller/playground/playground.h"
#include "impeller/playground/switches.h"
#include "impeller/renderer/context.h"
#include "impeller/renderer/surface.h"

namespace impeller {

class PlaygroundImpl {
public:
static std::unique_ptr<PlaygroundImpl> Create(PlaygroundBackend backend);
static std::unique_ptr<PlaygroundImpl> Create(PlaygroundBackend backend,
PlaygroundSwitches switches);

virtual ~PlaygroundImpl();

Expand All @@ -31,7 +33,9 @@ class PlaygroundImpl {
Vector2 GetContentScale() const;

protected:
PlaygroundImpl();
const PlaygroundSwitches switches_;

explicit PlaygroundImpl(PlaygroundSwitches switches);

private:
FML_DISALLOW_COPY_AND_ASSIGN(PlaygroundImpl);
Expand Down
2 changes: 1 addition & 1 deletion impeller/playground/playground_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
namespace impeller {

PlaygroundTest::PlaygroundTest()
: switches_(flutter::testing::GetArgsForProcess()) {}
: Playground(PlaygroundSwitches{flutter::testing::GetArgsForProcess()}) {}

PlaygroundTest::~PlaygroundTest() = default;

Expand Down
2 changes: 0 additions & 2 deletions impeller/playground/playground_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ class PlaygroundTest : public Playground,
std::string GetWindowTitle() const override;

private:
const PlaygroundSwitches switches_;

// |Playground|
bool ShouldKeepRendering() const;

Expand Down
3 changes: 3 additions & 0 deletions impeller/playground/switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@

namespace impeller {

PlaygroundSwitches::PlaygroundSwitches() = default;

PlaygroundSwitches::PlaygroundSwitches(const fml::CommandLine& args) {
std::string timeout_str;
if (args.GetOptionValue("playground_timeout_ms", &timeout_str)) {
timeout = std::chrono::milliseconds(atoi(timeout_str.c_str()));
}
enable_vulkan_validation = args.HasOption("enable_vulkan_validation");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: It's possible to test this flag flip in a reasonable way via switches_unittests.cc these days

}

} // namespace impeller
3 changes: 3 additions & 0 deletions impeller/playground/switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ struct PlaygroundSwitches {
// specified in the timeout. If the timeout is zero, exactly one frame will be
// rendered in the playground.
std::optional<std::chrono::milliseconds> timeout;
bool enable_vulkan_validation = false;

PlaygroundSwitches();

explicit PlaygroundSwitches(const fml::CommandLine& args);
};
Expand Down
3 changes: 3 additions & 0 deletions impeller/renderer/backend/vulkan/capabilities_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ static constexpr const char* kInstanceLayer = "ImpellerInstance";

CapabilitiesVK::CapabilitiesVK(bool enable_validations)
: enable_validations_(enable_validations) {
if (enable_validations_) {
FML_LOG(INFO) << "Vulkan validations are enabled.";
}
auto extensions = vk::enumerateInstanceExtensionProperties();
auto layers = vk::enumerateInstanceLayerProperties();

Expand Down
2 changes: 1 addition & 1 deletion impeller/renderer/backend/vulkan/capabilities_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class ContextVK;
class CapabilitiesVK final : public Capabilities,
public BackendCast<CapabilitiesVK, Capabilities> {
public:
explicit CapabilitiesVK(bool enable_validations = false);
explicit CapabilitiesVK(bool enable_validations);

~CapabilitiesVK();

Expand Down
3 changes: 2 additions & 1 deletion impeller/renderer/backend/vulkan/context_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ void ContextVK::Setup(Settings settings) {
auto& dispatcher = VULKAN_HPP_DEFAULT_DISPATCHER;
dispatcher.init(settings.proc_address_callback);

auto caps = std::shared_ptr<CapabilitiesVK>(new CapabilitiesVK());
auto caps = std::shared_ptr<CapabilitiesVK>(
new CapabilitiesVK(settings.enable_validation));

if (!caps->IsValid()) {
VALIDATION_LOG << "Could not determine device capabilities.";
Expand Down
1 change: 1 addition & 0 deletions impeller/renderer/backend/vulkan/context_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class ContextVK final : public Context, public BackendCast<ContextVK, Context> {
std::vector<std::shared_ptr<fml::Mapping>> shader_libraries_data;
fml::UniqueFD cache_directory;
std::shared_ptr<fml::ConcurrentTaskRunner> worker_task_runner;
bool enable_validation = false;

Settings() = default;

Expand Down