Skip to content

Commit

Permalink
layers: Fix PresentMode check being hidden
Browse files Browse the repository at this point in the history
  • Loading branch information
@spencer-lunarg
spencer-lunarg committed Jun 27, 2024
1 parent 61ee97b commit 8cca8f4
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 45 deletions.
94 changes: 50 additions & 44 deletions layers/core_checks/cc_wsi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,61 +68,50 @@ static VkImageCreateInfo GetSwapchainImpliedImageCreateInfo(const VkSwapchainCre
return result;
}

// Validate VkSwapchainPresentModesCreateInfoEXT data
bool CoreChecks::ValidateSwapchainPresentModesCreateInfo(VkPresentModeKHR present_mode, const Location &create_info_loc,
const VkSwapchainCreateInfoKHR &create_info,
const std::vector<VkPresentModeKHR> &present_modes,
const vvl::Surface *surface_state) const {
bool skip = false;
std::vector<VkPresentModeKHR> present_modes{};
if (surface_state) {
present_modes = surface_state->GetPresentModes(physical_device, create_info_loc, this);
} else if (IsExtEnabled(instance_extensions.vk_google_surfaceless_query)) {
present_modes = physical_device_state->surfaceless_query_state.present_modes;
}

if (std::find(present_modes.begin(), present_modes.end(), present_mode) == present_modes.end()) {
skip |= LogError("VUID-VkSwapchainCreateInfoKHR-presentMode-01281", device, create_info_loc,
"called with a non-supported presentMode (%s).", string_VkPresentModeKHR(present_mode));
}

// Validate VkSwapchainPresentModesCreateInfoEXT data
auto swapchain_present_modes_ci = vku::FindStructInPNextChain<VkSwapchainPresentModesCreateInfoEXT>(create_info.pNext);
if (swapchain_present_modes_ci) {
ASSERT_AND_RETURN_SKIP(surface_state);
if (!swapchain_present_modes_ci) {
return skip;
}
ASSERT_AND_RETURN_SKIP(surface_state);

bool found_swapchain_modes_ci_present_mode = false;
const std::vector<VkPresentModeKHR> compatible_present_modes =
surface_state->GetCompatibleModes(physical_device, present_mode);
for (uint32_t i = 0; i < swapchain_present_modes_ci->presentModeCount; i++) {
VkPresentModeKHR swapchain_present_mode = swapchain_present_modes_ci->pPresentModes[i];

if (std::find(present_modes.begin(), present_modes.end(), swapchain_present_mode) == present_modes.end()) {
if (LogError("VUID-VkSwapchainPresentModesCreateInfoEXT-None-07762", device,
create_info_loc.pNext(Struct::VkSwapchainPresentModesCreateInfoEXT, Field::pPresentModes, i),
"%s is a non-supported presentMode.", string_VkPresentModeKHR(swapchain_present_mode))) {
skip |= true;
}
}
bool found_swapchain_modes_ci_present_mode = false;
const std::vector<VkPresentModeKHR> compatible_present_modes = surface_state->GetCompatibleModes(physical_device, present_mode);
for (uint32_t i = 0; i < swapchain_present_modes_ci->presentModeCount; i++) {
VkPresentModeKHR swapchain_present_mode = swapchain_present_modes_ci->pPresentModes[i];

if (std::find(compatible_present_modes.begin(), compatible_present_modes.end(), swapchain_present_mode) ==
compatible_present_modes.end()) {
if (LogError("VUID-VkSwapchainPresentModesCreateInfoEXT-pPresentModes-07763", device,
create_info_loc.pNext(Struct::VkSwapchainPresentModesCreateInfoEXT, Field::pPresentModes, i),
"%s is a non-compatible presentMode.", string_VkPresentModeKHR(swapchain_present_mode))) {
skip |= true;
}
if (std::find(present_modes.begin(), present_modes.end(), swapchain_present_mode) == present_modes.end()) {
if (LogError("VUID-VkSwapchainPresentModesCreateInfoEXT-None-07762", device,
create_info_loc.pNext(Struct::VkSwapchainPresentModesCreateInfoEXT, Field::pPresentModes, i),
"%s is a non-supported presentMode.", string_VkPresentModeKHR(swapchain_present_mode))) {
skip |= true;
}

const bool has_present_mode = (swapchain_present_modes_ci->pPresentModes[i] == present_mode);
found_swapchain_modes_ci_present_mode |= has_present_mode;
}
if (!found_swapchain_modes_ci_present_mode) {
if (LogError("VUID-VkSwapchainPresentModesCreateInfoEXT-presentMode-07764", device, create_info_loc,
"was called with a present mode (%s) that was not included in the set of present modes specified in "
"the vkSwapchainPresentModesCreateInfoEXT structure included in its pNext chain.",
string_VkPresentModeKHR(present_mode))) {

if (std::find(compatible_present_modes.begin(), compatible_present_modes.end(), swapchain_present_mode) ==
compatible_present_modes.end()) {
if (LogError("VUID-VkSwapchainPresentModesCreateInfoEXT-pPresentModes-07763", device,
create_info_loc.pNext(Struct::VkSwapchainPresentModesCreateInfoEXT, Field::pPresentModes, i),
"%s is a non-compatible presentMode.", string_VkPresentModeKHR(swapchain_present_mode))) {
skip |= true;
}
}

const bool has_present_mode = (swapchain_present_modes_ci->pPresentModes[i] == present_mode);
found_swapchain_modes_ci_present_mode |= has_present_mode;
}
if (!found_swapchain_modes_ci_present_mode) {
if (LogError("VUID-VkSwapchainPresentModesCreateInfoEXT-presentMode-07764", device, create_info_loc,
"was called with a present mode (%s) that was not included in the set of present modes specified in "
"the vkSwapchainPresentModesCreateInfoEXT structure included in its pNext chain.",
string_VkPresentModeKHR(present_mode))) {
skip |= true;
}
}
return skip;
}
Expand Down Expand Up @@ -527,6 +516,23 @@ bool CoreChecks::ValidateCreateSwapchain(const VkSwapchainCreateInfoKHR &create_
}
}

std::vector<VkPresentModeKHR> present_modes{};
if (surface_state) {
present_modes = surface_state->GetPresentModes(physical_device, create_info_loc, this);
} else if (IsExtEnabled(instance_extensions.vk_google_surfaceless_query)) {
present_modes = physical_device_state->surfaceless_query_state.present_modes;
}

if (std::find(present_modes.begin(), present_modes.end(), present_mode) == present_modes.end()) {
std::stringstream ss;
for (auto mode : present_modes) {
ss << string_VkPresentModeKHR(mode) << " ";
}
skip |= LogError("VUID-VkSwapchainCreateInfoKHR-presentMode-01281", device, create_info_loc.dot(Field::presentMode),
"(%s) is not supported (the following are supported %s).", string_VkPresentModeKHR(present_mode),
ss.str().c_str());
}

if (!IsExtEnabled(device_extensions.vk_ext_swapchain_maintenance1)) {
// Validate pCreateInfo->imageExtent against VkSurfaceCapabilitiesKHR::{current|min|max}ImageExtent:
if (!IsExtentInsideBounds(create_info.imageExtent, surface_caps.minImageExtent, surface_caps.maxImageExtent)) {
Expand All @@ -549,7 +555,7 @@ bool CoreChecks::ValidateCreateSwapchain(const VkSwapchainCreateInfoKHR &create_
}
}
} else {
skip |= ValidateSwapchainPresentModesCreateInfo(present_mode, create_info_loc, create_info, surface_state);
skip |= ValidateSwapchainPresentModesCreateInfo(present_mode, create_info_loc, create_info, present_modes, surface_state);
skip |= ValidateSwapchainPresentScalingCreateInfo(present_mode, create_info_loc, surface_caps, create_info, surface_state);
}
// Validate state for shared presentable case
Expand Down
1 change: 1 addition & 0 deletions layers/core_checks/core_validation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ class CoreChecks : public ValidationStateTracker {
VkSharingMode sharing_mode) const;
bool ValidateSwapchainPresentModesCreateInfo(VkPresentModeKHR present_mode, const Location& create_info_loc,
const VkSwapchainCreateInfoKHR& create_info,
const std::vector<VkPresentModeKHR>& present_modes,
const vvl::Surface* surface_state) const;
bool ValidateSwapchainPresentScalingCreateInfo(VkPresentModeKHR present_mode, const Location& create_info_loc,
const VkSurfaceCapabilitiesKHR& capabilities,
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/best_practices.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,9 @@ TEST_F(VkBestPracticesLayerTest, SwapchainCreationTest) {
// Warning is thrown any time the present mode is not VK_PRESENT_MODE_FIFO_KHR, but only with ARM BP
m_errorMonitor->SetAllowedFailureMsg("BestPractices-Arm-vkCreateSwapchainKHR-swapchain-presentmode-not-fifo");

// present mode might not be supported
m_errorMonitor->SetAllowedFailureMsg("VUID-VkSwapchainCreateInfoKHR-presentMode-01281");

m_errorMonitor->SetAllowedFailureMsg("VUID-VkSwapchainCreateInfoKHR-presentMode-02839");
vk::CreateSwapchainKHR(device(), &swapchain_create_info, nullptr, &m_swapchain);
m_errorMonitor->VerifyFound();
Expand Down
49 changes: 48 additions & 1 deletion tests/unit/wsi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3468,4 +3468,51 @@ TEST_F(NegativeWsi, SwapchainPresentModeInfoImplicit) {
m_errorMonitor->SetDesiredError("VUID-VkSwapchainPresentModeInfoEXT-pPresentModes-parameter");
vk::QueuePresentKHR(m_default_queue->handle(), &present);
m_errorMonitor->VerifyFound();
}
}

TEST_F(NegativeWsi, NonSupportedPresentMode) {
TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8204");
AddSurfaceExtension();
RETURN_IF_SKIP(Init());
RETURN_IF_SKIP(InitSurface());

uint32_t count;
vk::GetPhysicalDeviceSurfacePresentModesKHR(gpu(), m_surface, &count, nullptr);
std::vector<VkPresentModeKHR> present_modes(count);
vk::GetPhysicalDeviceSurfacePresentModesKHR(gpu(), m_surface, &count, present_modes.data());
for (auto present_mode : present_modes) {
if (present_mode == VK_PRESENT_MODE_IMMEDIATE_KHR) {
GTEST_SKIP() << "Need no support for VK_PRESENT_MODE_IMMEDIATE_KHR";
}
}

VkBool32 supported;
vk::GetPhysicalDeviceSurfaceSupportKHR(gpu(), m_device->graphics_queue_node_index_, m_surface, &supported);
if (!supported) {
GTEST_SKIP() << "Surface not supported.";
}

SurfaceInformation info = GetSwapchainInfo(m_surface);
InitSwapchainInfo();

VkSwapchainCreateInfoKHR swapchain_create_info = vku::InitStructHelper();
swapchain_create_info.surface = m_surface;
swapchain_create_info.minImageCount = info.surface_capabilities.minImageCount;
swapchain_create_info.imageFormat = info.surface_formats[0].format;
swapchain_create_info.imageColorSpace = info.surface_formats[0].colorSpace;
swapchain_create_info.imageExtent = {info.surface_capabilities.minImageExtent.width,
info.surface_capabilities.minImageExtent.height};
swapchain_create_info.imageArrayLayers = 1;
swapchain_create_info.imageUsage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT;
swapchain_create_info.imageSharingMode = VK_SHARING_MODE_EXCLUSIVE;
swapchain_create_info.preTransform = VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR;
swapchain_create_info.compositeAlpha = info.surface_composite_alpha;
swapchain_create_info.presentMode = VK_PRESENT_MODE_IMMEDIATE_KHR;
swapchain_create_info.clipped = VK_FALSE;
swapchain_create_info.oldSwapchain = VK_NULL_HANDLE;

VkSwapchainKHR swapchain;
m_errorMonitor->SetDesiredError("VUID-VkSwapchainCreateInfoKHR-presentMode-01281");
vk::CreateSwapchainKHR(device(), &swapchain_create_info, nullptr, &swapchain);
m_errorMonitor->VerifyFound();
}

0 comments on commit 8cca8f4

Please sign in to comment.