Skip to content

Commit

Permalink
vk: Fix unsupported depth blitting (#7789)
Browse files Browse the repository at this point in the history
* vk: Fix unsupported depth blitting

On certain hardware (pixel 4 for example), blitting of depth texture
is not supported as an "optimalTilingFeature".  In these cases, we'd
would need to do a shader-based blit. We
 - Add the shader blit in PostProcessingManager
 - Add a driver API to check for support for blitting depthStencil
   attachments.
 - Fix some debugging ifdefs in vk backend.

The validation fixed is:
`[ VUID-vkCmdBlitImage-dstImage-02000 ] Object 0: handle = 0xb400007c300701d0, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0xf2039b0000000771, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x86bc2a78 | In vkCmdBlitImage, VkFormatFeatureFlags (0x1c601) does not support required feature VK_FORMAT_FEATURE_2_BLIT_DST_BIT for format 126 used by VkImage 0xf2039b0000000771[] with tiling VK_IMAGE_TILING_OPTIMAL. The Vulkan spec states: The format features of dstImage must contain VK_FORMAT_FEATURE_BLIT_DST_BIT`
  • Loading branch information
poweifeng committed Apr 24, 2024
1 parent 99eac62 commit c8335fa
Show file tree
Hide file tree
Showing 15 changed files with 203 additions and 65 deletions.
1 change: 1 addition & 0 deletions filament/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ set(PRIVATE_HDRS
set(MATERIAL_SRCS
src/materials/antiAliasing/fxaa.mat
src/materials/antiAliasing/taa.mat
src/materials/blitDepth.mat
src/materials/blitLow.mat
src/materials/blitArray.mat
src/materials/bloom/bloomDownsample.mat
Expand Down
1 change: 1 addition & 0 deletions filament/backend/include/private/backend/DriverAPI.inc
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ DECL_DRIVER_API_SYNCHRONOUS_0(bool, isProtectedContentSupported)
DECL_DRIVER_API_SYNCHRONOUS_N(bool, isStereoSupported, backend::StereoscopicType, stereoscopicType)
DECL_DRIVER_API_SYNCHRONOUS_0(bool, isParallelShaderCompileSupported)
DECL_DRIVER_API_SYNCHRONOUS_0(bool, isDepthStencilResolveSupported)
DECL_DRIVER_API_SYNCHRONOUS_N(bool, isDepthStencilBlitSupported, backend::TextureFormat, format)
DECL_DRIVER_API_SYNCHRONOUS_0(bool, isProtectedTexturesSupported)
DECL_DRIVER_API_SYNCHRONOUS_0(uint8_t, getMaxDrawBuffers)
DECL_DRIVER_API_SYNCHRONOUS_0(size_t, getMaxUniformBufferSize)
Expand Down
4 changes: 4 additions & 0 deletions filament/backend/src/metal/MetalDriver.mm
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,10 @@
return false;
}

bool MetalDriver::isDepthStencilBlitSupported(TextureFormat format) {
return true;
}

bool MetalDriver::isProtectedTexturesSupported() {
return false;
}
Expand Down
4 changes: 4 additions & 0 deletions filament/backend/src/noop/NoopDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ bool NoopDriver::isDepthStencilResolveSupported() {
return true;
}

bool NoopDriver::isDepthStencilBlitSupported(TextureFormat format) {
return true;
}

bool NoopDriver::isProtectedTexturesSupported() {
return true;
}
Expand Down
4 changes: 4 additions & 0 deletions filament/backend/src/opengl/OpenGLDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2023,6 +2023,10 @@ bool OpenGLDriver::isDepthStencilResolveSupported() {
return true;
}

bool OpenGLDriver::isDepthStencilBlitSupported(TextureFormat format) {
return true;
}

bool OpenGLDriver::isProtectedTexturesSupported() {
return getContext().ext.EXT_protected_textures;
}
Expand Down
4 changes: 0 additions & 4 deletions filament/backend/src/vulkan/VulkanBlitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@

#include <smolv.h>

#include "generated/vkshaders/vkshaders.h"

using namespace bluevk;
using namespace utils;

Expand All @@ -40,7 +38,6 @@ namespace {
inline void blitFast(const VkCommandBuffer cmdbuffer, VkImageAspectFlags aspect, VkFilter filter,
VulkanAttachment src, VulkanAttachment dst,
const VkOffset3D srcRect[2], const VkOffset3D dstRect[2]) {

if constexpr (FVK_ENABLED(FVK_DEBUG_BLITTER)) {
utils::slog.d << "Fast blit from=" << src.texture->getVkImage() << ",level=" << (int) src.level
<< " layout=" << src.getLayout()
Expand Down Expand Up @@ -93,7 +90,6 @@ inline void blitFast(const VkCommandBuffer cmdbuffer, VkImageAspectFlags aspect,

inline void resolveFast(const VkCommandBuffer cmdbuffer, VkImageAspectFlags aspect,
VulkanAttachment src, VulkanAttachment dst) {

if constexpr (FVK_ENABLED(FVK_DEBUG_BLITTER)) {
utils::slog.d << "Fast blit from=" << src.texture->getVkImage() << ",level=" << (int) src.level
<< " layout=" << src.getLayout()
Expand Down
4 changes: 2 additions & 2 deletions filament/backend/src/vulkan/VulkanConstants.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
// granualarity of a renderpass. You can enable this along with FVK_DEBUG_DEBUG_UTILS to take
// advantage of vkCmdBegin/EndDebugUtilsLabelEXT. You can also just enable this with
// FVK_DEBUG_PRINT_GROUP_MARKERS to print the current marker to stdout.
#define FVK_DEBUG_GROUP_MARKERS 0x00000002
#define FVK_DEBUG_GROUP_MARKERS 0x00000002

#define FVK_DEBUG_TEXTURE 0x00000004
#define FVK_DEBUG_LAYOUT_TRANSITION 0x00000008
Expand Down Expand Up @@ -112,7 +112,7 @@ static_assert(FVK_ENABLED(FVK_DEBUG_VALIDATION));
// end dependcy checks

// Shorthand for combination of enabled debug flags
#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) || FVK_ENABLED(FVK_DEBUG_TEXTURE)
#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) && FVK_ENABLED(FVK_DEBUG_TEXTURE)
#define FVK_ENABLED_DEBUG_SAMPLER_NAME 1
#else
#define FVK_ENABLED_DEBUG_SAMPLER_NAME 0
Expand Down
11 changes: 8 additions & 3 deletions filament/backend/src/vulkan/VulkanContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,12 @@ struct VulkanContext {
return (uint32_t) VK_MAX_MEMORY_TYPES;
}

inline VkFormatList const& getAttachmentDepthFormats() const {
return mDepthFormats;
inline VkFormatList const& getAttachmentDepthStencilFormats() const {
return mDepthStencilFormats;
}

inline VkFormatList const& getBlittableDepthStencilFormats() const {
return mBlittableDepthStencilFormats;
}

inline VkPhysicalDeviceLimits const& getPhysicalDeviceLimits() const noexcept {
Expand Down Expand Up @@ -131,7 +135,8 @@ struct VulkanContext {
bool mDebugMarkersSupported = false;
bool mDebugUtilsSupported = false;

VkFormatList mDepthFormats;
VkFormatList mDepthStencilFormats;
VkFormatList mBlittableDepthStencilFormats;

// For convenience so that VulkanPlatform can initialize the private fields.
friend class VulkanPlatform;
Expand Down
7 changes: 6 additions & 1 deletion filament/backend/src/vulkan/VulkanDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,11 @@ bool VulkanDriver::isDepthStencilResolveSupported() {
return false;
}

bool VulkanDriver::isDepthStencilBlitSupported(TextureFormat format) {
auto const& formats = mContext.getBlittableDepthStencilFormats();
return std::find(formats.begin(), formats.end(), getVkFormat(format)) != formats.end();
}

bool VulkanDriver::isProtectedTexturesSupported() {
return false;
}
Expand Down Expand Up @@ -1807,7 +1812,7 @@ void VulkanDriver::bindPipeline(PipelineState pipelineState) {
// This fallback path is very flaky because the dummy texture might not have
// matching characteristics. (e.g. if the missing texture is a 3D texture)
if (UTILS_UNLIKELY(texture->getPrimaryImageLayout() == VulkanLayout::UNDEFINED)) {
#if FVK_ENABLED(FVK_DEBUG_TEXTURE)
#if FVK_ENABLED(FVK_DEBUG_TEXTURE) && FVK_ENABLED_DEBUG_SAMPLER_NAME
utils::slog.w << "Uninitialized texture bound to '" << bindingToName[binding] << "'";
utils::slog.w << " in material '" << program->name.c_str() << "'";
utils::slog.w << " at binding point " << +binding << utils::io::endl;
Expand Down
2 changes: 1 addition & 1 deletion filament/backend/src/vulkan/VulkanImageUtility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ bool operator<(const VkImageSubresourceRange& a, const VkImageSubresourceRange&
return false;
}

#if FVK_ENABLED(FVK_DEBUG_LAYOUT_TRANSITION | FVK_DEBUG_TEXTURE)
#if FVK_ENABLED(FVK_DEBUG_LAYOUT_TRANSITION) || FVK_ENABLED(FVK_DEBUG_TEXTURE)
#define CASE(VALUE) \
case filament::backend::VulkanLayout::VALUE: { \
out << #VALUE; \
Expand Down
34 changes: 29 additions & 5 deletions filament/backend/src/vulkan/platform/VulkanPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,13 +344,13 @@ std::tuple<ExtensionSet, ExtensionSet> pruneExtensions(VkPhysicalDevice device,
ExtensionSet newInstExts = instExts;
ExtensionSet newDeviceExts = deviceExts;

#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS)
#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS)
// debugUtils and debugMarkers extensions are used mutually exclusively.
if (newInstExts.find(VK_EXT_DEBUG_UTILS_EXTENSION_NAME) != newInstExts.end()
&& newDeviceExts.find(VK_EXT_DEBUG_MARKER_EXTENSION_NAME) != newDeviceExts.end()) {
newDeviceExts.erase(VK_EXT_DEBUG_MARKER_EXTENSION_NAME);
}
#endif
#endif

#if FVK_ENABLED(FVK_DEBUG_VALIDATION)
// debugMarker must also request debugReport the instance extension. So check if that's present.
Expand Down Expand Up @@ -500,7 +500,7 @@ VkPhysicalDevice selectPhysicalDevice(VkInstance instance,
return device;
}

VkFormatList findAttachmentDepthFormats(VkPhysicalDevice device) {
VkFormatList findAttachmentDepthStencilFormats(VkPhysicalDevice device) {
VkFormatFeatureFlags const features = VK_FORMAT_FEATURE_DEPTH_STENCIL_ATTACHMENT_BIT;

// The ordering here indicates the preference of choosing depth+stencil format.
Expand All @@ -524,6 +524,28 @@ VkFormatList findAttachmentDepthFormats(VkPhysicalDevice device) {
return ret;
}

VkFormatList findBlittableDepthStencilFormats(VkPhysicalDevice device) {
std::vector<VkFormat> selectedFormats;
VkFormatFeatureFlags const required = VK_FORMAT_FEATURE_DEPTH_STENCIL_ATTACHMENT_BIT |
VK_FORMAT_FEATURE_BLIT_SRC_BIT | VK_FORMAT_FEATURE_BLIT_DST_BIT;
for (VkFormat format = (VkFormat) 1;;) {
if (isVkDepthFormat(format)) {
VkFormatProperties props;
vkGetPhysicalDeviceFormatProperties(device, format, &props);
if ((props.optimalTilingFeatures & required) == required) {
selectedFormats.push_back(format);
}
}
format = (VkFormat) (1 + (int) format);
if (format == VK_FORMAT_ASTC_12x12_SRGB_BLOCK) {
break;
}
}
VkFormatList ret(selectedFormats.size());
std::copy(selectedFormats.begin(), selectedFormats.end(), ret.begin());
return ret;
}

}// anonymous namespace

using SwapChainPtr = VulkanPlatform::SwapChainPtr;
Expand Down Expand Up @@ -669,9 +691,11 @@ Driver* VulkanPlatform::createDriver(void* sharedContext,
"Debug utils should not be enabled in release build.");
#endif

context.mDepthFormats = findAttachmentDepthFormats(mImpl->mPhysicalDevice);
context.mDepthStencilFormats = findAttachmentDepthStencilFormats(mImpl->mPhysicalDevice);
context.mBlittableDepthStencilFormats =
findBlittableDepthStencilFormats(mImpl->mPhysicalDevice);

assert_invariant(context.mDepthFormats.size() > 0);
assert_invariant(context.mDepthStencilFormats.size() > 0);

#if FVK_ENABLED(FVK_DEBUG_VALIDATION)
printDepthFormats(mImpl->mPhysicalDevice);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ VkResult VulkanPlatformSurfaceSwapChain::create() {
mSwapChainBundle.colors = enumerate(vkGetSwapchainImagesKHR, mDevice, mSwapchain);
mSwapChainBundle.colorFormat = surfaceFormat.format;
mSwapChainBundle.depthFormat =
selectDepthFormat(mContext.getAttachmentDepthFormats(), mHasStencil);
selectDepthFormat(mContext.getAttachmentDepthStencilFormats(), mHasStencil);
mSwapChainBundle.depth = createImage(mSwapChainBundle.extent, mSwapChainBundle.depthFormat);

slog.i << "vkCreateSwapchain"
Expand Down Expand Up @@ -330,7 +330,7 @@ VulkanPlatformHeadlessSwapChain::VulkanPlatformHeadlessSwapChain(VulkanContext c

bool const hasStencil = (flags & backend::SWAP_CHAIN_HAS_STENCIL_BUFFER) != 0;
mSwapChainBundle.depthFormat =
selectDepthFormat(mContext.getAttachmentDepthFormats(), hasStencil);
selectDepthFormat(mContext.getAttachmentDepthStencilFormats(), hasStencil);
mSwapChainBundle.depth = createImage(extent, mSwapChainBundle.depthFormat);
}

Expand Down
Loading

0 comments on commit c8335fa

Please sign in to comment.