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
jeremyg-lunarg committed Oct 20, 2021
1 parent cefc88b commit 7b634bd
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 74 deletions.
68 changes: 26 additions & 42 deletions layers/core_validation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13635,12 +13635,16 @@ 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);
auto supported = surface_state->GetQueueSupport(physical_device);
// should restrict search only to queue families of VkDeviceQueueCreateInfos, not whole phys. device
bool is_supported = false;
for (const auto &queue_entry : queueMap) {
auto qfi = queue_entry.second->queueFamilyIndex;
if (supported.size() > qfi && supported[qfi]) {
is_supported = true;
break;
}
}

if (!is_supported) {
if (LogError(
Expand Down Expand Up @@ -13822,17 +13826,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 @@ -13864,16 +13864,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 @@ -14168,14 +14160,14 @@ bool CoreChecks::PreCallValidateQueuePresentKHR(VkQueue queue, const VkPresentIn
// the application to have established support on any other platform.
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});
auto supported = surface_state->GetQueueSupport(physical_device);

if (support_it == surface_state->gpu_queue_support.end()) {
if (supported.empty() || supported.size() <= queue_state->queueFamilyIndex) {
skip |= LogError(
pPresentInfo->pSwapchains[i], kVUID_Core_DrawState_SwapchainUnsupportedQueue,
"vkQueuePresentKHR: Presenting pSwapchains[%u] image without calling vkGetPhysicalDeviceSurfaceSupportKHR",
i);
} else if (!support_it->second) {
} else if (!supported[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 @@ -14334,7 +14326,8 @@ bool CoreChecks::ValidateAcquireNextImage(VkDevice device, const CommandVersion

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 = "INVALID-vuid";
Expand Down Expand Up @@ -16431,26 +16424,17 @@ 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) {
auto supported = surface_state->GetQueueSupport(physicalDevice);
bool is_supported = false;
for (auto val : supported) {
if (val) {
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
85 changes: 77 additions & 8 deletions layers/image_state.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,21 +498,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 @@ -544,6 +537,8 @@ void SWAPCHAIN_NODE::AcquireImage(uint32_t image_index) {
void SWAPCHAIN_NODE::Destroy() {
for (auto &swapchain_image : images) {
if (swapchain_image.image_state) {
swapchain_image.image_state->bind_swapchain = nullptr;
RemoveParent(swapchain_image.image_state);
swapchain_image.image_state->Destroy();
dev_data->imageMap.erase(swapchain_image.image_state->image());
swapchain_image.image_state = nullptr;
Expand Down Expand Up @@ -578,3 +573,77 @@ 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);
gpu_queue_support_[phys_dev].resize(std::max(gpu_queue_support_[phys_dev].size(), static_cast<size_t>(qfi) + 1), false);
gpu_queue_support_[phys_dev][qfi] = supported;
}

std::vector<bool> SURFACE_STATE::GetQueueSupport(VkPhysicalDevice phys_dev) const {
std::vector<bool> result;
assert(phys_dev);
auto iter = gpu_queue_support_.find(phys_dev);
if (iter != gpu_queue_support_.end()) {
result = iter->second;
}
// NOTE: we don't provide a fallback query because the checking for VUID 01270
// needs to know if the application made the call or not and uses this empty result
// to do so.
return result;
}

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());
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);
assert(iter != capabilities_.end());
if (iter != capabilities_.end()) {
return iter->second;
}
VkSurfaceCapabilitiesKHR result;
DispatchGetPhysicalDeviceSurfaceCapabilitiesKHR(phys_dev, surface(), &result);
return result;
}
43 changes: 21 additions & 22 deletions layers/image_state.h
View file
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 All @@ -275,33 +274,21 @@ class SWAPCHAIN_NODE : public BASE_NODE {
void NotifyInvalidate(const LogObjectList &invalid_handles, bool unlink) override;
};

struct GpuQueue {
VkPhysicalDevice gpu;
uint32_t queue_family_index;
};

inline bool operator==(GpuQueue const &lhs, GpuQueue const &rhs) {
return (lhs.gpu == rhs.gpu && lhs.queue_family_index == rhs.queue_family_index);
}

namespace std {
template <>
struct hash<GpuQueue> {
size_t operator()(GpuQueue gq) const throw() {
return hash<uint64_t>()((uint64_t)(gq.gpu)) ^ hash<uint32_t>()(gq.queue_family_index);
}
};
} // namespace std

// State for VkSurfaceKHR objects.
// Parent -> child relationships in the object usage tree:
// 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:
layer_data::unordered_map<VkPhysicalDevice, std::vector<bool>> gpu_queue_support_;
layer_data::unordered_map<VkPhysicalDevice, std::vector<VkPresentModeKHR>> present_modes_;
layer_data::unordered_map<VkPhysicalDevice, std::vector<VkSurfaceFormatKHR>> formats_;
layer_data::unordered_map<VkPhysicalDevice, VkSurfaceCapabilitiesKHR> capabilities_;

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

~SURFACE_STATE() {
if (!Destroyed()) {
Expand All @@ -316,4 +303,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);
std::vector<bool> GetQueueSupport(VkPhysicalDevice phys_dev) 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 7b634bd

Please sign in to comment.