Skip to content

Commit

Permalink
layers: Don't pre-query surface attributes during creation
Browse files Browse the repository at this point in the history
Some of the calls to get surface capabilities, formats or
presentation modes are very slow in some implementations.
Revert back to caching results made by the application, but
store them in SURFACE_STATE rather than PHYSICAL_DEVICE_STATE.
The original code assumed that the same results would apply
to all surfaces, which doesn't appear to be true.
  • Loading branch information
jeremyg-lunarg committed Oct 22, 2021
1 parent 62f00c3 commit 327afef
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 69 deletions.
82 changes: 27 additions & 55 deletions layers/core_validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13636,20 +13636,21 @@ bool CoreChecks::ValidateCreateSwapchain(const char *func_name, VkSwapchainCreat
// All physical devices and queue families are required to be able to present to any native window on Android; require the
// application to have established support on any other platform.
if (!instance_extensions.vk_khr_android_surface) {
auto support_predicate = [this](decltype(surface_state->gpu_queue_support)::value_type qs) -> bool {
// TODO: should restrict search only to queue families of VkDeviceQueueCreateInfos, not whole phys. device
return (qs.first.gpu == physical_device) && qs.second;
};
const auto &support = surface_state->gpu_queue_support;
bool is_supported = std::any_of(support.begin(), support.end(), support_predicate);
// restrict search only to queue families of VkDeviceQueueCreateInfos, not the whole physical device
bool is_supported = false;
for (const auto &queue_entry : queueMap) {
auto qfi = queue_entry.second->queueFamilyIndex;
if (surface_state->GetQueueSupport(physical_device, qfi)) {
is_supported = true;
break;
}
}

if (!is_supported) {
if (LogError(
device, "VUID-VkSwapchainCreateInfoKHR-surface-01270",
"%s: pCreateInfo->surface is not known at this time to be supported for presentation by this device. The "
"vkGetPhysicalDeviceSurfaceSupportKHR() must be called beforehand, and it must return VK_TRUE support with "
"this surface for at least one queue family of this device.",
func_name)) {
LogObjectList objs(device);
objs.add(surface_state->Handle());
if (LogError(objs, "VUID-VkSwapchainCreateInfoKHR-surface-01270",
"%s: pCreateInfo->surface is not supported for presentation by this device.", func_name)) {
return true;
}
}
Expand Down Expand Up @@ -13823,17 +13824,13 @@ bool CoreChecks::ValidateCreateSwapchain(const char *func_name, VkSwapchainCreat
}

// Validate pCreateInfo values with the results of vkGetPhysicalDeviceSurfaceFormatsKHR():
std::vector<VkSurfaceFormatKHR> surface_formats;
uint32_t surface_format_count = 0;
DispatchGetPhysicalDeviceSurfaceFormatsKHR(physical_device, pCreateInfo->surface, &surface_format_count, nullptr);
surface_formats.resize(surface_format_count);
DispatchGetPhysicalDeviceSurfaceFormatsKHR(physical_device, pCreateInfo->surface, &surface_format_count, &surface_formats[0]);
{
// Validate pCreateInfo->imageFormat against VkSurfaceFormatKHR::format:
bool found_format = false;
bool found_color_space = false;
bool found_match = false;
for (const auto &format : surface_formats) {
const auto formats = surface_state->GetFormats(physical_device);
for (const auto &format : formats) {
if (pCreateInfo->imageFormat == format.format) {
// Validate pCreateInfo->imageColorSpace against VkSurfaceFormatKHR::colorSpace:
found_format = true;
Expand Down Expand Up @@ -13865,16 +13862,8 @@ bool CoreChecks::ValidateCreateSwapchain(const char *func_name, VkSwapchainCreat
}
}

// Validate pCreateInfo values with the results of vkGetPhysicalDeviceSurfacePresentModesKHR():
std::vector<VkPresentModeKHR> present_modes;
uint32_t present_mode_count = 0;
DispatchGetPhysicalDeviceSurfacePresentModesKHR(physical_device_state->PhysDev(), pCreateInfo->surface, &present_mode_count,
nullptr);
present_modes.resize(present_mode_count);
DispatchGetPhysicalDeviceSurfacePresentModesKHR(physical_device_state->PhysDev(), pCreateInfo->surface, &present_mode_count,
&present_modes[0]);

// Validate pCreateInfo->presentMode against vkGetPhysicalDeviceSurfacePresentModesKHR():
auto present_modes = surface_state->GetPresentModes(physical_device);
bool found_match = std::find(present_modes.begin(), present_modes.end(), present_mode) != present_modes.end();
if (!found_match) {
if (LogError(device, "VUID-VkSwapchainCreateInfoKHR-presentMode-01281",
Expand Down Expand Up @@ -14165,18 +14154,10 @@ bool CoreChecks::PreCallValidateQueuePresentKHR(VkQueue queue, const VkPresentIn
}
}

// All physical devices and queue families are required to be able to present to any native window on Android; require
// the application to have established support on any other platform.
// All physical devices and queue families are required to be able to present to any native window on Android
if (!instance_extensions.vk_khr_android_surface) {
const auto surface_state = GetSurfaceState(swapchain_data->createInfo.surface);
auto support_it = surface_state->gpu_queue_support.find({physical_device, queue_state->queueFamilyIndex});

if (support_it == surface_state->gpu_queue_support.end()) {
skip |= LogError(
pPresentInfo->pSwapchains[i], kVUID_Core_DrawState_SwapchainUnsupportedQueue,
"vkQueuePresentKHR: Presenting pSwapchains[%u] image without calling vkGetPhysicalDeviceSurfaceSupportKHR",
i);
} else if (!support_it->second) {
if (!surface_state->GetQueueSupport(physical_device, queue_state->queueFamilyIndex)) {
skip |= LogError(
pPresentInfo->pSwapchains[i], "VUID-vkQueuePresentKHR-pSwapchains-01292",
"vkQueuePresentKHR: Presenting pSwapchains[%u] image on queue that cannot present to this surface.", i);
Expand Down Expand Up @@ -14335,7 +14316,8 @@ bool CoreChecks::ValidateAcquireNextImage(VkDevice device, const AcquireVersion

const uint32_t acquired_images = swapchain_data->acquired_images;
const uint32_t swapchain_image_count = static_cast<uint32_t>(swapchain_data->images.size());
const auto min_image_count = swapchain_data->surface_capabilities.minImageCount;
auto caps = swapchain_data->surface->GetCapabilities(physical_device);
const auto min_image_count = caps.minImageCount;
const bool too_many_already_acquired = acquired_images > swapchain_image_count - min_image_count;
if (timeout == UINT64_MAX && too_many_already_acquired) {
const char *vuid = version == ACQUIRE_VERSION_2 ? "VUID-vkAcquireNextImage2KHR-swapchain-01803"
Expand Down Expand Up @@ -16435,26 +16417,16 @@ bool CoreChecks::ValidatePhysicalDeviceSurfaceSupport(VkPhysicalDevice physicalD
bool skip = false;

const auto *pd_state = GetPhysicalDeviceState(physicalDevice);
if (pd_state) {
const auto *surface_state = GetSurfaceState(surface);
VkBool32 supported = VK_FALSE;
for (uint32_t i = 0; i < pd_state->queue_family_known_count; ++i) {
bool checked = false;
if (surface_state) {
const auto support_it = surface_state->gpu_queue_support.find({physicalDevice, i});
if (support_it != surface_state->gpu_queue_support.end()) {
supported = support_it->second;
checked = true;
}
}
if (!checked) {
DispatchGetPhysicalDeviceSurfaceSupportKHR(physicalDevice, i, surface, &supported);
}
if (supported) {
const auto *surface_state = GetSurfaceState(surface);
if (pd_state && surface_state) {
bool is_supported = false;
for (uint32_t i = 0; i < pd_state->queue_family_properties.size(); i++) {
if (surface_state->GetQueueSupport(physicalDevice, i)) {
is_supported = true;
break;
}
}
if (!supported) {
if (!is_supported) {
skip |= LogError(physicalDevice, vuid, "%s(): surface is not supported by the physicalDevice.", func_name);
}
}
Expand Down
84 changes: 76 additions & 8 deletions layers/image_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -499,21 +499,14 @@ static safe_VkImageCreateInfo GetImageCreateInfo(const VkSwapchainCreateInfoKHR
return safe_VkImageCreateInfo(&image_ci);
}

static VkSurfaceCapabilitiesKHR GetSurfaceCaps(VkPhysicalDevice physical_device, VkSurfaceKHR surface) {
VkSurfaceCapabilitiesKHR result{};
DispatchGetPhysicalDeviceSurfaceCapabilitiesKHR(physical_device, surface, &result);
return result;
}

SWAPCHAIN_NODE::SWAPCHAIN_NODE(ValidationStateTracker *dev_data_, const VkSwapchainCreateInfoKHR *pCreateInfo,
VkSwapchainKHR swapchain)
: BASE_NODE(swapchain, kVulkanObjectTypeSwapchainKHR),
createInfo(pCreateInfo),
shared_presentable(VK_PRESENT_MODE_SHARED_DEMAND_REFRESH_KHR == pCreateInfo->presentMode ||
VK_PRESENT_MODE_SHARED_CONTINUOUS_REFRESH_KHR == pCreateInfo->presentMode),
image_create_info(GetImageCreateInfo(pCreateInfo)),
dev_data(dev_data_),
surface_capabilities(GetSurfaceCaps(dev_data->physical_device, pCreateInfo->surface)) {}
dev_data(dev_data_) {}

void SWAPCHAIN_NODE::PresentImage(uint32_t image_index) {
if (image_index >= images.size()) return;
Expand Down Expand Up @@ -579,3 +572,78 @@ void SURFACE_STATE::RemoveParent(BASE_NODE *parent_node) {
}
BASE_NODE::RemoveParent(parent_node);
}

void SURFACE_STATE::SetQueueSupport(VkPhysicalDevice phys_dev, uint32_t qfi, bool supported) {
assert(phys_dev);
GpuQueue key{phys_dev, qfi};
gpu_queue_support_[key] = supported;
}

bool SURFACE_STATE::GetQueueSupport(VkPhysicalDevice phys_dev, uint32_t qfi) const {
assert(phys_dev);
GpuQueue key{phys_dev, qfi};
auto iter = gpu_queue_support_.find(key);
if (iter != gpu_queue_support_.end()) {
return iter->second;
}
VkBool32 supported = VK_FALSE;
DispatchGetPhysicalDeviceSurfaceSupportKHR(phys_dev, qfi, surface(), &supported);
gpu_queue_support_[key] = (supported == VK_TRUE);
return supported == VK_TRUE;
}

void SURFACE_STATE::SetPresentModes(VkPhysicalDevice phys_dev, std::vector<VkPresentModeKHR> &&modes) {
assert(phys_dev);
present_modes_[phys_dev] = std::move(modes);
}

std::vector<VkPresentModeKHR> SURFACE_STATE::GetPresentModes(VkPhysicalDevice phys_dev) const {
assert(phys_dev);
auto iter = present_modes_.find(phys_dev);
if (iter != present_modes_.end()) {
return iter->second;
}
std::vector<VkPresentModeKHR> result;
uint32_t count = 0;
DispatchGetPhysicalDeviceSurfacePresentModesKHR(phys_dev, surface(), &count, nullptr);
result.resize(count);
DispatchGetPhysicalDeviceSurfacePresentModesKHR(phys_dev, surface(), &count, result.data());
return result;
}

void SURFACE_STATE::SetFormats(VkPhysicalDevice phys_dev, std::vector<VkSurfaceFormatKHR> &&fmts) {
assert(phys_dev);
formats_[phys_dev] = std::move(fmts);
}

std::vector<VkSurfaceFormatKHR> SURFACE_STATE::GetFormats(VkPhysicalDevice phys_dev) const {
assert(phys_dev);
auto iter = formats_.find(phys_dev);
if (iter != formats_.end()) {
return iter->second;
}
std::vector<VkSurfaceFormatKHR> result;
uint32_t count = 0;
DispatchGetPhysicalDeviceSurfaceFormatsKHR(phys_dev, surface(), &count, nullptr);
result.resize(count);
DispatchGetPhysicalDeviceSurfaceFormatsKHR(phys_dev, surface(), &count, result.data());
formats_[phys_dev] = result;
return result;
}

void SURFACE_STATE::SetCapabilities(VkPhysicalDevice phys_dev, const VkSurfaceCapabilitiesKHR &caps) {
assert(phys_dev);
capabilities_[phys_dev] = caps;
}

VkSurfaceCapabilitiesKHR SURFACE_STATE::GetCapabilities(VkPhysicalDevice phys_dev) const {
assert(phys_dev);
auto iter = capabilities_.find(phys_dev);
if (iter != capabilities_.end()) {
return iter->second;
}
VkSurfaceCapabilitiesKHR result{};
DispatchGetPhysicalDeviceSurfaceCapabilitiesKHR(phys_dev, surface(), &result);
capabilities_[phys_dev] = result;
return result;
}
25 changes: 21 additions & 4 deletions layers/image_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ class SWAPCHAIN_NODE : public BASE_NODE {
const safe_VkImageCreateInfo image_create_info;
std::shared_ptr<SURFACE_STATE> surface;
ValidationStateTracker *dev_data;
const VkSurfaceCapabilitiesKHR surface_capabilities;
uint32_t acquired_images = 0;

SWAPCHAIN_NODE(ValidationStateTracker *dev_data, const VkSwapchainCreateInfoKHR *pCreateInfo, VkSwapchainKHR swapchain);
Expand Down Expand Up @@ -298,10 +297,16 @@ struct hash<GpuQueue> {
// SURFACE_STATE -> nothing
class SURFACE_STATE : public BASE_NODE {
public:
SWAPCHAIN_NODE *swapchain;
layer_data::unordered_map<GpuQueue, bool> gpu_queue_support;
SWAPCHAIN_NODE *swapchain{nullptr};

SURFACE_STATE(VkSurfaceKHR s) : BASE_NODE(s, kVulkanObjectTypeSurfaceKHR), swapchain(nullptr), gpu_queue_support() {}
private:
mutable layer_data::unordered_map<GpuQueue, bool> gpu_queue_support_;
mutable layer_data::unordered_map<VkPhysicalDevice, std::vector<VkPresentModeKHR>> present_modes_;
mutable layer_data::unordered_map<VkPhysicalDevice, std::vector<VkSurfaceFormatKHR>> formats_;
mutable layer_data::unordered_map<VkPhysicalDevice, VkSurfaceCapabilitiesKHR> capabilities_;

public:
SURFACE_STATE(VkSurfaceKHR s) : BASE_NODE(s, kVulkanObjectTypeSurfaceKHR) {}

~SURFACE_STATE() {
if (!Destroyed()) {
Expand All @@ -316,4 +321,16 @@ class SURFACE_STATE : public BASE_NODE {
VkImageCreateInfo GetImageCreateInfo() const;

void RemoveParent(BASE_NODE *parent_node) override;

void SetQueueSupport(VkPhysicalDevice phys_dev, uint32_t qfi, bool supported);
bool GetQueueSupport(VkPhysicalDevice phys_dev, uint32_t qfi) const;

void SetPresentModes(VkPhysicalDevice phys_dev, std::vector<VkPresentModeKHR> &&modes);
std::vector<VkPresentModeKHR> GetPresentModes(VkPhysicalDevice phys_dev) const;

void SetFormats(VkPhysicalDevice phys_dev, std::vector<VkSurfaceFormatKHR> &&fmts);
std::vector<VkSurfaceFormatKHR> GetFormats(VkPhysicalDevice phys_dev) const;

void SetCapabilities(VkPhysicalDevice phys_dev, const VkSurfaceCapabilitiesKHR &caps);
VkSurfaceCapabilitiesKHR GetCapabilities(VkPhysicalDevice phys_dev) const;
};
Loading

0 comments on commit 327afef

Please sign in to comment.