From bee28aaf0d8427a0b0e85e43686640096a28d266 Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Tue, 14 Sep 2021 21:13:16 +0530 Subject: [PATCH] Validation Layer Filter + Fix `Texture`, GPU & `PresentationEngine` bugs This commit implements a filter by type for any validation layer output, this allows filtering out any logs which may be unnecessary and additionally triggering a breakpoint as required. An issue concerning the `NDEBUG` flag never being set was fixed, it's now supplied as a release compiler flag. The issue can manifest itself by always relying on a validation layer even though it shouldn't on release, this is why the validation layer was mistakenly disabled entirely previously by using `#ifndef` rather than `#ifdef`. An issue with the initial layout of a texture being supplied as neither `VK_IMAGE_LAYOUT_UNDEFINED` or `VK_IMAGE_LAYOUT_PREINITIALIZED` was fixed, these cases are now handled by transitioning to those layouts after creating the image rather than supplying it within `initialLayout`. Another issue was fixed regarding not maintaining a transformation after a surface has been destroyed and recreated existed and manifested itself when the user would go out of the app and come back in, they would see the surface having an identity transformation rather than the desired one. --- .idea/misc.xml | 2 +- .idea/scopes/SkylineNative.xml | 4 +- app/CMakeLists.txt | 7 +-- app/src/main/cpp/skyline/gpu.cpp | 28 ++++++++- .../cpp/skyline/gpu/presentation_engine.cpp | 5 ++ .../main/cpp/skyline/gpu/texture/texture.cpp | 58 +++++++++++++++---- .../main/cpp/skyline/gpu/texture/texture.h | 18 +++++- app/src/main/cpp/skyline/nce.cpp | 2 +- 8 files changed, 102 insertions(+), 22 deletions(-) diff --git a/.idea/misc.xml b/.idea/misc.xml index 7689fac6..a9a1b1da 100644 --- a/.idea/misc.xml +++ b/.idea/misc.xml @@ -60,7 +60,7 @@ - + diff --git a/.idea/scopes/SkylineNative.xml b/.idea/scopes/SkylineNative.xml index fe4f5eaf..a2de2ab7 100644 --- a/.idea/scopes/SkylineNative.xml +++ b/.idea/scopes/SkylineNative.xml @@ -1,3 +1,3 @@ - - + + \ No newline at end of file diff --git a/app/CMakeLists.txt b/app/CMakeLists.txt index f735619e..0944fe18 100644 --- a/app/CMakeLists.txt +++ b/app/CMakeLists.txt @@ -10,11 +10,8 @@ set(CMAKE_CXX_STANDARD_REQUIRED TRUE) set(source_DIR ${CMAKE_SOURCE_DIR}/src/main/cpp) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-strict-aliasing") -set(CMAKE_CXX_FLAGS_RELEASE "-Ofast -flto=full -fno-stack-protector -Wno-unused-command-line-argument") -if (uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE") - add_compile_definitions(NDEBUG) - set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE) -endif () +set(CMAKE_CXX_FLAGS_RELEASE "-Ofast -flto=full -fno-stack-protector -Wno-unused-command-line-argument -DNDEBUG") +set(CMAKE_INTERPROCEDURAL_OPTIMIZATION_RELEASE TRUE) # {fmt} add_subdirectory("libraries/fmt") diff --git a/app/src/main/cpp/skyline/gpu.cpp b/app/src/main/cpp/skyline/gpu.cpp index 27e75966..3d2bb2c8 100644 --- a/app/src/main/cpp/skyline/gpu.cpp +++ b/app/src/main/cpp/skyline/gpu.cpp @@ -13,7 +13,7 @@ namespace skyline::gpu { .apiVersion = VkApiVersion, }; - #ifndef NDEBUG + #ifdef NDEBUG constexpr std::array requiredLayers{}; #else constexpr std::array requiredLayers{ @@ -82,7 +82,33 @@ namespace skyline::gpu { Logger::LogLevel::Error, // VK_DEBUG_REPORT_ERROR_BIT_EXT Logger::LogLevel::Debug, // VK_DEBUG_REPORT_DEBUG_BIT_EXT }; + + #define IGNORE_VALIDATION(string) \ + case util::Hash(string): \ + if(string == type) \ + return VK_FALSE; \ + else \ + break + + #define DEBUG_VALIDATION(string) \ + case util::Hash(string): \ + if(string == type) \ + __builtin_debugtrap(); \ + break + + std::string_view type(message); + auto first{type.find_first_of('[') + 2}; + type = type.substr(first, type.find_first_of(']', first) - 4); + + switch (util::Hash(type)) { + IGNORE_VALIDATION("UNASSIGNED-CoreValidation-SwapchainPreTransform"); // We handle transformation via Android APIs directly + IGNORE_VALIDATION("UNASSIGNED-GeneralParameterPerfWarn-SuboptimalSwapchain"); // Same as SwapchainPreTransform + } + + #undef IGNORE_TYPE + logger->Write(severityLookup.at(std::countr_zero(static_cast(flags))), util::Format("Vk{}:{}[0x{:X}]:I{}:L{}: {}", layerPrefix, vk::to_string(vk::DebugReportObjectTypeEXT(objectType)), object, messageCode, location, message)); + return VK_FALSE; } diff --git a/app/src/main/cpp/skyline/gpu/presentation_engine.cpp b/app/src/main/cpp/skyline/gpu/presentation_engine.cpp index fb06ff22..97fa753e 100644 --- a/app/src/main/cpp/skyline/gpu/presentation_engine.cpp +++ b/app/src/main/cpp/skyline/gpu/presentation_engine.cpp @@ -190,6 +190,9 @@ namespace skyline::gpu { if (windowScalingMode != NativeWindowScalingMode::ScaleToWindow && (result = window->perform(window, NATIVE_WINDOW_SET_SCALING_MODE, static_cast(windowScalingMode)))) throw exception("Setting the layer scaling mode to '{}' failed with {}", ToString(windowScalingMode), result); + if (windowTransform != NativeWindowTransform::Identity && (result = window->perform(window, NATIVE_WINDOW_SET_BUFFERS_TRANSFORM, static_cast(windowTransform)))) + throw exception("Setting the buffer transform to '{}' failed with {}", ToString(windowTransform), result); + if ((result = window->perform(window, NATIVE_WINDOW_ENABLE_FRAME_TIMESTAMPS, true))) throw exception("Enabling frame timestamps failed with {}", result); @@ -226,6 +229,8 @@ namespace skyline::gpu { windowTransform = transform; } + gpu.vkDevice.resetFences(*acquireFence); + std::pair nextImage; while (nextImage = vkSwapchain->acquireNextImage(std::numeric_limits::max(), {}, *acquireFence), nextImage.first != vk::Result::eSuccess) [[unlikely]] { if (nextImage.first == vk::Result::eSuboptimalKHR) diff --git a/app/src/main/cpp/skyline/gpu/texture/texture.cpp b/app/src/main/cpp/skyline/gpu/texture/texture.cpp index 202898ab..5c7b9df9 100644 --- a/app/src/main/cpp/skyline/gpu/texture/texture.cpp +++ b/app/src/main/cpp/skyline/gpu/texture/texture.cpp @@ -7,19 +7,38 @@ #include "texture.h" namespace skyline::gpu { - Texture::Texture(GPU &gpu, BackingType &&backing, GuestTexture guest, texture::Dimensions dimensions, texture::Format format, vk::ImageLayout layout, vk::ImageTiling tiling, u32 mipLevels, u32 layerCount, vk::SampleCountFlagBits sampleCount) : gpu(gpu), backing(std::move(backing)), layout(layout), guest(std::move(guest)), dimensions(dimensions), format(format), tiling(tiling), mipLevels(mipLevels), layerCount(layerCount), sampleCount(sampleCount) { + Texture::Texture(GPU &gpu, BackingType &&backing, GuestTexture guest, texture::Dimensions dimensions, texture::Format format, vk::ImageLayout layout, vk::ImageTiling tiling, u32 mipLevels, u32 layerCount, vk::SampleCountFlagBits sampleCount) + : gpu(gpu), + backing(std::move(backing)), + layout(layout), + guest(std::move(guest)), + dimensions(dimensions), + format(format), + tiling(tiling), + mipLevels(mipLevels), + layerCount(layerCount), + sampleCount(sampleCount) { if (GetBacking()) SynchronizeHost(); } - Texture::Texture(GPU &gpu, BackingType &&backing, texture::Dimensions dimensions, texture::Format format, vk::ImageLayout layout, vk::ImageTiling tiling, u32 mipLevels, u32 layerCount, vk::SampleCountFlagBits sampleCount) : gpu(gpu), backing(std::move(backing)), dimensions(dimensions), format(format), layout(layout), tiling(tiling), mipLevels(mipLevels), layerCount(layerCount), sampleCount(sampleCount) {} + Texture::Texture(GPU &gpu, BackingType &&backing, texture::Dimensions dimensions, texture::Format format, vk::ImageLayout layout, vk::ImageTiling tiling, u32 mipLevels, u32 layerCount, vk::SampleCountFlagBits sampleCount) + : gpu(gpu), + backing(std::move(backing)), + dimensions(dimensions), + format(format), + layout(layout), + tiling(tiling), + mipLevels(mipLevels), + layerCount(layerCount), + sampleCount(sampleCount) {} Texture::Texture(GPU &pGpu, GuestTexture pGuest) : gpu(pGpu), guest(std::move(pGuest)), dimensions(guest->dimensions), format(guest->format), - layout(vk::ImageLayout::eGeneral), + layout(vk::ImageLayout::eUndefined), tiling((guest->tileConfig.mode == texture::TileMode::Block) ? vk::ImageTiling::eOptimal : vk::ImageTiling::eLinear), mipLevels(1), layerCount(guest->layerCount), @@ -39,9 +58,18 @@ namespace skyline::gpu { .initialLayout = layout, }; backing = tiling != vk::ImageTiling::eLinear ? gpu.memory.AllocateImage(imageCreateInfo) : gpu.memory.AllocateMappedImage(imageCreateInfo); + TransitionLayout(vk::ImageLayout::eGeneral); } - Texture::Texture(GPU &gpu, texture::Dimensions dimensions, texture::Format format, vk::ImageLayout initialLayout, vk::ImageUsageFlags usage, vk::ImageTiling tiling, u32 mipLevels, u32 layerCount, vk::SampleCountFlagBits sampleCount) : gpu(gpu), dimensions(dimensions), format(format), layout(initialLayout), tiling(tiling), mipLevels(mipLevels), layerCount(layerCount), sampleCount(sampleCount) { + Texture::Texture(GPU &gpu, texture::Dimensions dimensions, texture::Format format, vk::ImageLayout initialLayout, vk::ImageUsageFlags usage, vk::ImageTiling tiling, u32 mipLevels, u32 layerCount, vk::SampleCountFlagBits sampleCount) + : gpu(gpu), + dimensions(dimensions), + format(format), + layout(initialLayout == vk::ImageLayout::ePreinitialized ? vk::ImageLayout::ePreinitialized : vk::ImageLayout::eUndefined), + tiling(tiling), + mipLevels(mipLevels), + layerCount(layerCount), + sampleCount(sampleCount) { vk::ImageCreateInfo imageCreateInfo{ .imageType = dimensions.GetType(), .format = *format, @@ -54,9 +82,11 @@ namespace skyline::gpu { .sharingMode = vk::SharingMode::eExclusive, .queueFamilyIndexCount = 1, .pQueueFamilyIndices = &gpu.vkQueueFamilyIndex, - .initialLayout = initialLayout, + .initialLayout = layout, }; backing = tiling != vk::ImageTiling::eLinear ? gpu.memory.AllocateImage(imageCreateInfo) : gpu.memory.AllocateMappedImage(imageCreateInfo); + if (initialLayout != layout) + TransitionLayout(initialLayout); } bool Texture::WaitOnBacking() { @@ -354,7 +384,6 @@ namespace skyline::gpu { TextureView::TextureView(std::shared_ptr backing, vk::ImageViewType type, vk::ImageSubresourceRange range, texture::Format format, vk::ComponentMapping mapping) : backing(std::move(backing)), type(type), format(format), mapping(mapping), range(range) {} vk::ImageView TextureView::GetView() { - /* if (view) return **view; @@ -369,14 +398,21 @@ namespace skyline::gpu { } }()}; - return *view.emplace(backing->gpu.vkDevice, vk::ImageViewCreateInfo{ + vk::ImageViewCreateInfo createInfo{ .image = backing->GetBacking(), - .viewType = vk::ImageViewType::eCube, + .viewType = viewType, .format = format ? *format : *backing->format, .components = mapping, .subresourceRange = range, - }); - */ - throw exception("TODO: TextureView::GetView"); + }; + + auto &views{backing->views}; + auto iterator{std::find_if(views.begin(), views.end(), [&](const std::pair &item) { + return item.first == createInfo; + })}; + if (iterator != views.end()) + return *iterator->second; + + return *views.emplace_back(createInfo, vk::raii::ImageView(backing->gpu.vkDevice, createInfo)).second; } } diff --git a/app/src/main/cpp/skyline/gpu/texture/texture.h b/app/src/main/cpp/skyline/gpu/texture/texture.h index 60c233ff..0140cbc0 100644 --- a/app/src/main/cpp/skyline/gpu/texture/texture.h +++ b/app/src/main/cpp/skyline/gpu/texture/texture.h @@ -115,6 +115,7 @@ namespace skyline::gpu { /** * @brief A wrapper around a pointer to underlying format metadata to prevent redundant copies + * @note The equality operators **do not** compare equality for pointers but for the underlying formats while considering nullability */ class Format { private: @@ -133,6 +134,14 @@ namespace skyline::gpu { return *base; } + constexpr bool operator==(const Format &format) const { + return base && format.base ? (*base) == (*format.base) : base == format.base; + } + + constexpr bool operator!=(const Format &format) const { + return base && format.base ? (*base) != (*format.base) : base != format.base; + } + constexpr operator bool() const { return base; } @@ -281,6 +290,10 @@ namespace skyline::gpu { * @return A Vulkan Image View that corresponds to the properties of this view */ vk::ImageView GetView(); + + bool operator==(const TextureView &rhs) { + return backing == rhs.backing && type == rhs.type && format == rhs.format && mapping == rhs.mapping && range == rhs.range; + } }; /** @@ -294,11 +307,14 @@ namespace skyline::gpu { std::condition_variable backingCondition; //!< Signalled when a valid backing has been swapped in using BackingType = std::variant; BackingType backing; //!< The Vulkan image that backs this texture, it is nullable - std::shared_ptr cycle; //!< A fence cycle for when any host operation mutating the texture has completed, it must be waited on prior to any mutations to the backing + + std::vector> views; //!< VkImageView(s) that have been constructed from this Texture, utilized for caching friend TextureManager; + friend TextureView; public: + std::shared_ptr cycle; //!< A fence cycle for when any host operation mutating the texture has completed, it must be waited on prior to any mutations to the backing std::optional guest; texture::Dimensions dimensions; texture::Format format; diff --git a/app/src/main/cpp/skyline/nce.cpp b/app/src/main/cpp/skyline/nce.cpp index a0d9ed85..e18abc6e 100644 --- a/app/src/main/cpp/skyline/nce.cpp +++ b/app/src/main/cpp/skyline/nce.cpp @@ -171,9 +171,9 @@ namespace skyline::nce { /* Replace Skyline TLS with host TLS */ *patch++ = 0xD53BD041; // MRS X1, TPIDR_EL0 *patch++ = 0xF9415022; // LDR X2, [X1, #0x2A0] (ThreadContext::hostTpidrEl0) + *patch++ = 0xD51BD042; // MSR TPIDR_EL0, X2 /* Replace guest stack with host stack */ - *patch++ = 0xD51BD042; // MSR TPIDR_EL0, X2 *patch++ = 0x910003E2; // MOV X2, SP *patch++ = 0xF9415423; // LDR X3, [X1, #0x2A8] (ThreadContext::hostSp) *patch++ = 0x9100007F; // MOV SP, X3