Skip to content

Commit

Permalink
[Impeller] If validations are enabled but not found, still create the…
Browse files Browse the repository at this point in the history
… VK context. (flutter#45674)

Fixes flutter/flutter#131714
  • Loading branch information
chinmaygarde authored Sep 12, 2023
1 parent 4519acd commit 9a32784
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 19 deletions.
29 changes: 16 additions & 13 deletions impeller/renderer/backend/vulkan/capabilities_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,7 @@ namespace impeller {

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.";
}
CapabilitiesVK::CapabilitiesVK(bool enable_validations) {
auto extensions = vk::enumerateInstanceExtensionProperties();
auto layers = vk::enumerateInstanceLayerProperties();

Expand All @@ -42,6 +38,17 @@ CapabilitiesVK::CapabilitiesVK(bool enable_validations)
}
}

validations_enabled_ =
enable_validations && HasLayer("VK_LAYER_KHRONOS_validation");
if (enable_validations && !validations_enabled_) {
FML_LOG(ERROR)
<< "Requested Impeller context creation with validations but the "
"validation layers could not be found. Expect no Vulkan validation "
"checks!";
}
if (validations_enabled_) {
FML_LOG(INFO) << "Vulkan validations are enabled.";
}
is_valid_ = true;
}

Expand All @@ -52,19 +59,15 @@ bool CapabilitiesVK::IsValid() const {
}

bool CapabilitiesVK::AreValidationsEnabled() const {
return enable_validations_;
return validations_enabled_;
}

std::optional<std::vector<std::string>> CapabilitiesVK::GetEnabledLayers()
const {
std::vector<std::string> required;

if (enable_validations_) {
if (!HasLayer("VK_LAYER_KHRONOS_validation")) {
VALIDATION_LOG
<< "Requested validations but the validation layer was not found.";
return std::nullopt;
}
if (validations_enabled_) {
// The presence of this layer is already checked in the ctor.
required.push_back("VK_LAYER_KHRONOS_validation");
}

Expand Down Expand Up @@ -131,7 +134,7 @@ CapabilitiesVK::GetEnabledInstanceExtensions() const {
return std::nullopt;
}

if (enable_validations_) {
if (validations_enabled_) {
if (!HasExtension("VK_EXT_debug_utils")) {
VALIDATION_LOG << "Requested validations but could not find the "
"VK_EXT_debug_utils extension.";
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 @@ -100,7 +100,7 @@ class CapabilitiesVK final : public Capabilities,
PixelFormat GetDefaultDepthStencilFormat() const override;

private:
const bool enable_validations_;
bool validations_enabled_ = false;
std::map<std::string, std::set<std::string>> exts_;
std::set<OptionalDeviceExtensionVK> optional_device_extensions_;
mutable PixelFormat default_color_format_ = PixelFormat::kUnknown;
Expand Down
8 changes: 8 additions & 0 deletions impeller/renderer/backend/vulkan/context_vk_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,5 +83,13 @@ TEST(ContextVKTest, DeletePipelineLibraryAfterContext) {
"vkDestroyDevice") != functions->end());
}

TEST(ContextVKTest, CanCreateContextInAbsenceOfValidationLayers) {
// The mocked methods don't report the presence of a validation layer but we
// explicitly ask for validation. Context creation should continue anyway.
auto context = CreateMockVulkanContext(
[](auto& settings) { settings.enable_validation = true; });
ASSERT_NE(context, nullptr);
}

} // namespace testing
} // namespace impeller
8 changes: 6 additions & 2 deletions impeller/renderer/backend/vulkan/test/mock_vulkan.cc
Original file line number Diff line number Diff line change
Expand Up @@ -513,10 +513,14 @@ PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance,

} // namespace

std::shared_ptr<ContextVK> CreateMockVulkanContext(void) {
ContextVK::Settings settings;
std::shared_ptr<ContextVK> CreateMockVulkanContext(
const std::function<void(ContextVK::Settings&)>& settings_callback) {
auto message_loop = fml::ConcurrentMessageLoop::Create();
ContextVK::Settings settings;
settings.proc_address_callback = GetMockVulkanProcAddress;
if (settings_callback) {
settings_callback(settings);
}
return ContextVK::Create(std::move(settings));
}

Expand Down
14 changes: 13 additions & 1 deletion impeller/renderer/backend/vulkan/test/mock_vulkan.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#pragma once

#include <functional>
#include <string>
#include <vector>

Expand All @@ -15,7 +16,18 @@ namespace testing {
std::shared_ptr<std::vector<std::string>> GetMockVulkanFunctions(
VkDevice device);

std::shared_ptr<ContextVK> CreateMockVulkanContext(void);
//------------------------------------------------------------------------------
/// @brief Create a Vulkan context with Vulkan functions mocked. The caller
/// is given a chance to tinker on the settings right before a
/// context is created.
///
/// @param[in] settings_callback The settings callback
///
/// @return A context if one can be created.
///
std::shared_ptr<ContextVK> CreateMockVulkanContext(
const std::function<void(ContextVK::Settings&)>& settings_callback =
nullptr);

} // namespace testing
} // namespace impeller
7 changes: 5 additions & 2 deletions shell/platform/android/android_context_vulkan_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,17 @@ static std::shared_ptr<impeller::Context> CreateImpellerContext(
settings.cache_directory = fml::paths::GetCachesDirectory();
settings.enable_validation = enable_vulkan_validation;

if (settings.enable_validation) {
auto context = impeller::ContextVK::Create(std::move(settings));

if (context && impeller::CapabilitiesVK::Cast(*context->GetCapabilities())
.AreValidationsEnabled()) {
FML_LOG(ERROR) << "Using the Impeller rendering backend (Vulkan with "
"Validation Layers).";
} else {
FML_LOG(ERROR) << "Using the Impeller rendering backend (Vulkan).";
}

return impeller::ContextVK::Create(std::move(settings));
return context;
}

AndroidContextVulkanImpeller::AndroidContextVulkanImpeller(
Expand Down

0 comments on commit 9a32784

Please sign in to comment.