Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulkan: Some surface code cleanup #13164

Merged
merged 6 commits into from
Jul 18, 2020
Merged

Vulkan: Some surface code cleanup #13164

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
697c3b8
Cleanup log output on swapchain/surface recreation.
hrydgard Jul 18, 2020
49d9cae
SDL: Avoid destroying the surface on resize.
hrydgard Jul 18, 2020
e48cb91
Buildfix
hrydgard Jul 18, 2020
1897eed
More buildfixes
hrydgard Jul 18, 2020
05ba88d
Windows/Vulkan: Don't destroy the surface during resize, no need.
hrydgard Jul 18, 2020
21f9925
Eliminate an unnecessary global g_Vulkan.
hrydgard Jul 18, 2020
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
91 changes: 55 additions & 36 deletions Common/Vulkan/VulkanContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,26 +318,18 @@ bool VulkanContext::MemoryTypeFromProperties(uint32_t typeBits, VkFlags requirem
return false;
}

bool VulkanContext::InitObjects() {
if (!InitQueue()) {
return false;
}

if (!InitSwapchain()) {
// Destroy queue?
return false;
void VulkanContext::DestroySwapchain() {
if (swapchain_ != VK_NULL_HANDLE) {
vkDestroySwapchainKHR(device_, swapchain_, nullptr);
swapchain_ = VK_NULL_HANDLE;
}
return true;
}

void VulkanContext::DestroyObjects() {
ILOG("VulkanContext::DestroyObjects (including swapchain)");
if (swapchain_ != VK_NULL_HANDLE)
vkDestroySwapchainKHR(device_, swapchain_, nullptr);
swapchain_ = VK_NULL_HANDLE;

vkDestroySurfaceKHR(instance_, surface_, nullptr);
surface_ = VK_NULL_HANDLE;
void VulkanContext::DestroySurface() {
if (surface_ != VK_NULL_HANDLE) {
vkDestroySurfaceKHR(instance_, surface_, nullptr);
surface_ = VK_NULL_HANDLE;
}
}

VkResult VulkanContext::GetInstanceLayerExtensionList(const char *layerName, std::vector<VkExtensionProperties> &extensions) {
Expand Down Expand Up @@ -716,7 +708,10 @@ VkResult VulkanContext::ReinitSurface() {
surface_ = VK_NULL_HANDLE;
}

ILOG("Creating Vulkan surface for window");
ILOG("Creating Vulkan surface for window (%p %p)", winsysData1_, winsysData2_);

VkResult retval = VK_SUCCESS;

switch (winsys_) {
#ifdef _WIN32
case WINDOWSYSTEM_WIN32:
Expand All @@ -725,7 +720,8 @@ VkResult VulkanContext::ReinitSurface() {
win32.flags = 0;
win32.hwnd = (HWND)winsysData2_;
win32.hinstance = (HINSTANCE)winsysData1_;
return vkCreateWin32SurfaceKHR(instance_, &win32, nullptr, &surface_);
retval = vkCreateWin32SurfaceKHR(instance_, &win32, nullptr, &surface_);
break;
}
#endif
#if defined(__ANDROID__)
Expand All @@ -735,7 +731,8 @@ VkResult VulkanContext::ReinitSurface() {
VkAndroidSurfaceCreateInfoKHR android{ VK_STRUCTURE_TYPE_ANDROID_SURFACE_CREATE_INFO_KHR };
android.flags = 0;
android.window = wnd;
return vkCreateAndroidSurfaceKHR(instance_, &android, nullptr, &surface_);
retval = vkCreateAndroidSurfaceKHR(instance_, &android, nullptr, &surface_);
break;
}
#endif
#if defined(VK_USE_PLATFORM_METAL_EXT)
Expand All @@ -745,7 +742,8 @@ VkResult VulkanContext::ReinitSurface() {
metal.flags = 0;
metal.pLayer = winsysData1_;
metal.pNext = winsysData2_;
return vkCreateMetalSurfaceEXT(instance_, &metal, nullptr, &surface_);
retval = vkCreateMetalSurfaceEXT(instance_, &metal, nullptr, &surface_);
break;
}
#endif
#if defined(VK_USE_PLATFORM_XLIB_KHR)
Expand All @@ -755,7 +753,8 @@ VkResult VulkanContext::ReinitSurface() {
xlib.flags = 0;
xlib.dpy = (Display *)winsysData1_;
xlib.window = (Window)winsysData2_;
return vkCreateXlibSurfaceKHR(instance_, &xlib, nullptr, &surface_);
retval = vkCreateXlibSurfaceKHR(instance_, &xlib, nullptr, &surface_);
break;
}
#endif
#if defined(VK_USE_PLATFORM_XCB_KHR)
Expand All @@ -765,7 +764,8 @@ VkResult VulkanContext::ReinitSurface() {
xcb.flags = 0;
xcb.connection = (Connection *)winsysData1_;
xcb.window = (Window)(uintptr_t)winsysData2_;
return vkCreateXcbSurfaceKHR(instance_, &xcb, nullptr, &surface_);
retval = vkCreateXcbSurfaceKHR(instance_, &xcb, nullptr, &surface_);
break;
}
#endif
#if defined(VK_USE_PLATFORM_WAYLAND_KHR)
Expand All @@ -775,17 +775,28 @@ VkResult VulkanContext::ReinitSurface() {
wayland.flags = 0;
wayland.display = (wl_display *)winsysData1_;
wayland.surface = (wl_surface *)winsysData2_;
return vkCreateWaylandSurfaceKHR(instance_, &wayland, nullptr, &surface_);
retval = vkCreateWaylandSurfaceKHR(instance_, &wayland, nullptr, &surface_);
break;
}
#endif

default:
_assert_msg_(G3D, false, "Vulkan support for chosen window system not implemented");
return VK_ERROR_INITIALIZATION_FAILED;
}

if (retval != VK_SUCCESS) {
return retval;
}

if (!ChooseQueue()) {
return VK_ERROR_INITIALIZATION_FAILED;
}

return VK_SUCCESS;
}

bool VulkanContext::InitQueue() {
bool VulkanContext::ChooseQueue() {
// Iterate over each queue to learn whether it supports presenting:
VkBool32 *supportsPresent = new VkBool32[queue_count];
for (uint32_t i = 0; i < queue_count; i++) {
Expand Down Expand Up @@ -869,7 +880,6 @@ bool VulkanContext::InitQueue() {
}

vkGetDeviceQueue(device_, graphics_queue_family_index_, 0, &gfx_queue_);
ILOG("gfx_queue_: %p", gfx_queue_);
return true;
}

Expand Down Expand Up @@ -906,21 +916,27 @@ bool VulkanContext::InitSwapchain() {
res = vkGetPhysicalDeviceSurfacePresentModesKHR(physical_devices_[physical_device_], surface_, &presentModeCount, presentModes);
assert(res == VK_SUCCESS);

ILOG("surfCapabilities_.currentExtent: %dx%d", surfCapabilities_.currentExtent.width, surfCapabilities_.currentExtent.height);
ILOG("surfCapabilities_.minImageExtent: %dx%d", surfCapabilities_.minImageExtent.width, surfCapabilities_.minImageExtent.height);
ILOG("surfCapabilities_.maxImageExtent: %dx%d", surfCapabilities_.maxImageExtent.width, surfCapabilities_.maxImageExtent.height);

swapChainExtent_.width = clamp(surfCapabilities_.currentExtent.width, surfCapabilities_.minImageExtent.width, surfCapabilities_.maxImageExtent.width);
swapChainExtent_.height = clamp(surfCapabilities_.currentExtent.height, surfCapabilities_.minImageExtent.height, surfCapabilities_.maxImageExtent.height);

ILOG("swapChainExtent: %dx%d", swapChainExtent_.width, swapChainExtent_.height);
ILOG("surfCapabilities_.current: %dx%d min: %dx%d max: %dx%d computed: %dx%d",
surfCapabilities_.currentExtent.width, surfCapabilities_.currentExtent.height,
surfCapabilities_.minImageExtent.width, surfCapabilities_.minImageExtent.height,
surfCapabilities_.maxImageExtent.width, surfCapabilities_.maxImageExtent.height,
swapChainExtent_.width, swapChainExtent_.height);

// TODO: Find a better way to specify the prioritized present mode while being able
// to fall back in a sensible way.
VkPresentModeKHR swapchainPresentMode = VK_PRESENT_MODE_MAX_ENUM_KHR;
std::string modes = "";
for (size_t i = 0; i < presentModeCount; i++) {
ILOG("Supported present mode: %d (%s)", presentModes[i], PresentModeString(presentModes[i]));
modes += PresentModeString(presentModes[i]);
if (i != presentModeCount - 1) {
modes += ", ";
}
}
ILOG("Supported present modes: %s", modes.c_str());
for (size_t i = 0; i < presentModeCount; i++) {
bool match = false;
match = match || ((flags_ & VULKAN_FLAG_PRESENT_MAILBOX) && presentModes[i] == VK_PRESENT_MODE_MAILBOX_KHR);
Expand All @@ -940,28 +956,28 @@ bool VulkanContext::InitSwapchain() {
// HACK
swapchainPresentMode = VK_PRESENT_MODE_FIFO_KHR;
#endif
ILOG("Chosen present mode: %d (%s)", swapchainPresentMode, PresentModeString(swapchainPresentMode));
delete[] presentModes;
// Determine the number of VkImage's to use in the swap chain (we desire to
// own only 1 image at a time, besides the images being displayed and
// queued for display):
uint32_t desiredNumberOfSwapChainImages = surfCapabilities_.minImageCount + 1;
ILOG("numSwapChainImages: %d", desiredNumberOfSwapChainImages);
if ((surfCapabilities_.maxImageCount > 0) &&
(desiredNumberOfSwapChainImages > surfCapabilities_.maxImageCount))
{
// Application must settle for fewer images than desired:
desiredNumberOfSwapChainImages = surfCapabilities_.maxImageCount;
}

ILOG("Chosen present mode: %d (%s). numSwapChainImages: %d/%d",
swapchainPresentMode, PresentModeString(swapchainPresentMode),
desiredNumberOfSwapChainImages, surfCapabilities_.maxImageCount);

// We mostly follow the practices from
// https://arm-software.github.io/vulkan_best_practice_for_mobile_developers/samples/surface_rotation/surface_rotation_tutorial.html
//
VkSurfaceTransformFlagBitsKHR preTransform;
std::string supportedTransforms = surface_transforms_to_string(surfCapabilities_.supportedTransforms);
std::string currentTransform = surface_transforms_to_string(surfCapabilities_.currentTransform);
ILOG("Supported transforms: %s", supportedTransforms.c_str());
ILOG("Current transform: %s", currentTransform.c_str());
g_display_rotation = DisplayRotation::ROTATE_0;
g_display_rot_matrix.setIdentity();
bool swapChainExtentSwap = false;
Expand Down Expand Up @@ -993,8 +1009,10 @@ bool VulkanContext::InitSwapchain() {
// Let the OS rotate the image (potentially slow on many Android devices)
preTransform = VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR;
}

std::string preTransformStr = surface_transforms_to_string(preTransform);
ILOG("Chosen pretransform transform: %s", preTransformStr.c_str());

ILOG("Transform supported: %s current: %s chosen: %s", supportedTransforms.c_str(), currentTransform.c_str(), preTransformStr.c_str());

if (physicalDeviceProperties_[physical_device_].properties.vendorID == VULKAN_VENDOR_IMGTEC) {
ILOG("Applying PowerVR hack (rounding off the width!)");
Expand All @@ -1017,6 +1035,7 @@ bool VulkanContext::InitSwapchain() {
swap_chain_info.oldSwapchain = VK_NULL_HANDLE;
swap_chain_info.clipped = true;
swap_chain_info.imageUsage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT;

// Don't ask for TRANSFER_DST for the swapchain image, we don't use that.
// if (surfCapabilities_.supportedUsageFlags & VK_IMAGE_USAGE_TRANSFER_DST_BIT)
// swap_chain_info.imageUsage |= VK_IMAGE_USAGE_TRANSFER_DST_BIT;
Expand Down
9 changes: 5 additions & 4 deletions Common/Vulkan/VulkanContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,11 @@ class VulkanContext {
VkResult InitSurface(WindowSystem winsys, void *data1, void *data2);
VkResult ReinitSurface();

bool InitQueue();
bool InitObjects();
bool InitSwapchain();

// Also destroys the surface.
void DestroyObjects();
void DestroySwapchain();
void DestroySurface();

void DestroyDevice();

void PerformPendingDeletes();
Expand Down Expand Up @@ -273,6 +272,8 @@ class VulkanContext {
void GetImageMemoryRequirements(VkImage image, VkMemoryRequirements *mem_reqs, bool *dedicatedAllocation);

private:
bool ChooseQueue();

VkResult InitDebugUtilsCallback();

// A layer can expose extensions, keep track of those extensions here.
Expand Down
4 changes: 2 additions & 2 deletions SDL/SDLMain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,11 @@ static void InitSDLAudioDevice(const std::string &name = "") {
}
}
if (audioDev <= 0) {
ILOG("SDL: Trying a different device");
ILOG("SDL: Trying a different audio device");
audioDev = SDL_OpenAudioDevice(nullptr, 0, &fmt, &g_retFmt, SDL_AUDIO_ALLOW_FREQUENCY_CHANGE);
}
if (audioDev <= 0) {
ELOG("Failed to open audio: %s", SDL_GetError());
ELOG("Failed to open audio device: %s", SDL_GetError());
} else {
if (g_retFmt.samples != fmt.samples) // Notify, but still use it
ELOG("Output audio samples: %d (requested: %d)", g_retFmt.samples, fmt.samples);
Expand Down
25 changes: 17 additions & 8 deletions SDL/SDLVulkanGraphicsContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ static const bool g_Validate = true;
static const bool g_Validate = false;
#endif

static uint32_t FlagsFromConfig() {
uint32_t flags = 0;
flags = g_Config.bVSync ? VULKAN_FLAG_PRESENT_FIFO : VULKAN_FLAG_PRESENT_MAILBOX;
if (g_Validate) {
flags |= VULKAN_FLAG_VALIDATE;
}
return flags;
}

bool SDLVulkanGraphicsContext::Init(SDL_Window *&window, int x, int y, int mode, std::string *error_message) {
window = SDL_CreateWindow("Initializing Vulkan...", x, y, pixel_xres, pixel_yres, mode);
if (!window) {
Expand All @@ -41,10 +50,7 @@ bool SDLVulkanGraphicsContext::Init(SDL_Window *&window, int x, int y, int mode,
}

vulkan_ = new VulkanContext();
int vulkanFlags = VULKAN_FLAG_PRESENT_MAILBOX;
if (g_Validate) {
vulkanFlags |= VULKAN_FLAG_VALIDATE;
}
int vulkanFlags = FlagsFromConfig();

VulkanContext::CreateInfo info{};
info.app_name = "PPSSPP";
Expand Down Expand Up @@ -102,7 +108,7 @@ bool SDLVulkanGraphicsContext::Init(SDL_Window *&window, int x, int y, int mode,
break;
}

if (!vulkan_->InitObjects()) {
if (!vulkan_->InitSwapchain()) {
*error_message = vulkan_->InitError();
Shutdown();
return false;
Expand All @@ -126,7 +132,8 @@ void SDLVulkanGraphicsContext::Shutdown() {
delete draw_;
draw_ = nullptr;
vulkan_->WaitUntilQueueIdle();
vulkan_->DestroyObjects();
vulkan_->DestroySwapchain();
vulkan_->DestroySurface();
vulkan_->DestroyDevice();
vulkan_->DestroyInstance();
delete vulkan_;
Expand All @@ -136,9 +143,11 @@ void SDLVulkanGraphicsContext::Shutdown() {

void SDLVulkanGraphicsContext::Resize() {
draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, vulkan_->GetBackbufferWidth(), vulkan_->GetBackbufferHeight());
vulkan_->DestroyObjects();
vulkan_->DestroySwapchain();
vulkan_->DestroySurface();
vulkan_->UpdateFlags(FlagsFromConfig());
vulkan_->ReinitSurface();
vulkan_->InitObjects();
vulkan_->InitSwapchain();
draw_->HandleEvent(Draw::Event::GOT_BACKBUFFER, vulkan_->GetBackbufferWidth(), vulkan_->GetBackbufferHeight());
}

Expand Down
3 changes: 2 additions & 1 deletion UI/GameSettingsScreen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,13 +350,14 @@ void GameSettingsScreen::CreateViews() {
}
#endif

#ifdef _WIN32
#if !PPSSPP_PLATFORM(ANDROID)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's Vulkan only though, right? Worried this may cause confusion for people on OpenGL, i.e. Raspberry pi, etc.

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

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

Fully aware of this - which I will fix in a followup by simply adding support for toggling VSync on these platforms too.

CheckBox *vSync = graphicsSettings->Add(new CheckBox(&g_Config.bVSync, gr->T("VSync")));
vSync->OnClick.Add([=](EventParams &e) {
NativeResized();
return UI::EVENT_CONTINUE;
});
#endif

CheckBox *frameDuplication = graphicsSettings->Add(new CheckBox(&g_Config.bRenderDuplicateFrames, gr->T("Render duplicate frames to 60hz")));
frameDuplication->SetEnabledFunc([] {
return g_Config.iRenderingMode != FB_NON_BUFFERED_MODE || (g_Config.bSoftwareRendering && g_Config.iFrameSkip != 0);
Expand Down
8 changes: 3 additions & 5 deletions Windows/GPU/WindowsVulkanContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,10 @@ void WindowsVulkanContext::Shutdown() {

void WindowsVulkanContext::Resize() {
draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight());
g_Vulkan->DestroyObjects();

g_Vulkan->DestroySwapchain();
g_Vulkan->UpdateFlags(FlagsFromConfig());
g_Vulkan->ReinitSurface();

g_Vulkan->InitObjects();
g_Vulkan_->ReinitSurface();
g_Vulkan_->InitSwapchain();
draw_->HandleEvent(Draw::Event::GOT_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight());
}

Expand Down
11 changes: 7 additions & 4 deletions android/jni/AndroidVulkanContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ bool AndroidVulkanContext::InitFromRenderThread(ANativeWindow *wnd, int desiredB

ILOG("AndroidVulkanContext::Init completed, %s", success ? "successfully" : "but failed");
if (!success) {
g_Vulkan->DestroyObjects();
g_Vulkan->DestroySwapchain();
g_Vulkan->DestroySurface();
g_Vulkan->DestroyDevice();
g_Vulkan->DestroyInstance();
}
Expand All @@ -130,7 +131,8 @@ void AndroidVulkanContext::ShutdownFromRenderThread() {
draw_ = nullptr;
g_Vulkan->WaitUntilQueueIdle();
g_Vulkan->PerformPendingDeletes();
g_Vulkan->DestroyObjects(); // Also destroys the surface, a bit asymmetric
g_Vulkan->DestroySwapchain();
g_Vulkan->DestroySurface();
ILOG("Done with ShutdownFromRenderThread");
}

Expand All @@ -150,11 +152,12 @@ void AndroidVulkanContext::Resize() {
ILOG("AndroidVulkanContext::Resize begin (oldsize: %dx%d)", g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight());

draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight());
g_Vulkan->DestroyObjects();
g_Vulkan->DestroySwapchain();
g_Vulkan->DestroySurface();

g_Vulkan->UpdateFlags(FlagsFromConfig());
g_Vulkan->ReinitSurface();
g_Vulkan->InitObjects();
g_Vulkan->InitSwapchain();
draw_->HandleEvent(Draw::Event::GOT_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight());
ILOG("AndroidVulkanContext::Resize end (final size: %dx%d)", g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight());
}
Expand Down