Skip to content

Commit

Permalink
vk: refactor texture default layout (#8183)
Browse files Browse the repository at this point in the history 
Previously, default layout is based on usage, but this actually
has two paths (Filament's TextureUsage and the computed
VkTextureUsage) that do not always agree.  We simplify so that
default layout is stored in the texture itself.

Also remove some unnecessary code that is no longer necessary.
In particular, we shouldn't be doing a flush and wait for the
transition to complete before updating a sampler descriptor.
We just need to make sure the layout before it is accessed is
correctly given in the update struct.
  • Loading branch information
@poweifeng
poweifeng authored Oct 10, 2024
1 parent b1b2d70 commit 875b295
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 102 deletions.
8 changes: 4 additions & 4 deletions filament/backend/src/vulkan/VulkanBlitter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ inline void blitFast(VulkanCommandBuffer* commands, VkImageAspectFlags aspect, V
1, blitRegions, filter);

if (oldSrcLayout == VulkanLayout::UNDEFINED) {
oldSrcLayout = imgutil::getDefaultLayout(src.texture->usage);
oldSrcLayout = src.texture->getDefaultLayout();
}
if (oldDstLayout == VulkanLayout::UNDEFINED) {
oldDstLayout = imgutil::getDefaultLayout(dst.texture->usage);
oldDstLayout = dst.texture->getDefaultLayout();
}
src.texture->transitionLayout(commands, srcRange, oldSrcLayout);
dst.texture->transitionLayout(commands, dstRange, oldDstLayout);
Expand Down Expand Up @@ -109,10 +109,10 @@ inline void resolveFast(VulkanCommandBuffer* commands, VkImageAspectFlags aspect
1, resolveRegions);

if (oldSrcLayout == VulkanLayout::UNDEFINED) {
oldSrcLayout = imgutil::getDefaultLayout(src.texture->usage);
oldSrcLayout = src.texture->getDefaultLayout();
}
if (oldDstLayout == VulkanLayout::UNDEFINED) {
oldDstLayout = imgutil::getDefaultLayout(dst.texture->usage);
oldDstLayout = dst.texture->getDefaultLayout();
}
src.texture->transitionLayout(commands, srcRange, oldSrcLayout);
dst.texture->transitionLayout(commands, dstRange, oldDstLayout);
Expand Down
10 changes: 0 additions & 10 deletions filament/backend/src/vulkan/VulkanDriver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,14 +404,6 @@ void VulkanDriver::updateDescriptorSetTexture(
SamplerParams params) {
VulkanDescriptorSet* set = mResourceAllocator.handle_cast<VulkanDescriptorSet*>(dsh);
VulkanTexture* texture = mResourceAllocator.handle_cast<VulkanTexture*>(th);

// We need to make sure the initial layout transition has been completed before we can write
// the sampler descriptor. We flush and wait until the transition has been completed.
if (!texture->transitionReady()) {
mCommands.flush();
mCommands.wait();
}

VkSampler const vksampler = mSamplerCache.getSampler(params);
mDescriptorSetManager.updateSampler(set, binding, texture, vksampler);
}
Expand Down Expand Up @@ -1360,8 +1352,6 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP

VulkanTexture* texture = attachment.texture;
if (texture->samples == 1) {
mRenderPassFboInfo.hasColorResolve = true;

auto const& range = attachment.getSubresourceRange();
attachment.texture->transitionLayout(&commands,
range, VulkanLayout::COLOR_ATTACHMENT);
Expand Down
1 change: 0 additions & 1 deletion filament/backend/src/vulkan/VulkanDriver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ class VulkanDriver final : public DriverBase {
struct {
using AttachmentArray = CappedArray<VulkanAttachment, MAX_RENDERTARGET_ATTACHMENT_TEXTURES>;
AttachmentArray attachments;
bool hasColorResolve = false;
} mRenderPassFboInfo = {};

bool const mIsSRGBSwapChainSupported;
Expand Down
30 changes: 0 additions & 30 deletions filament/backend/src/vulkan/VulkanImageUtility.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,36 +76,6 @@ inline VkImageViewType getViewType(SamplerType target) {
}
}

inline VulkanLayout getDefaultLayout(TextureUsage usage) {
if (any(usage & TextureUsage::DEPTH_ATTACHMENT)) {
if (any(usage & TextureUsage::SAMPLEABLE)) {
return VulkanLayout::DEPTH_SAMPLER;
} else {
return VulkanLayout::DEPTH_ATTACHMENT;
}
}

if (any(usage & TextureUsage::COLOR_ATTACHMENT)) {
return VulkanLayout::COLOR_ATTACHMENT;
}
// Finally, the layout for an immutable texture is optimal read-only.
return VulkanLayout::READ_ONLY;
}

inline VulkanLayout getDefaultLayout(VkImageUsageFlags vkusage) {
TextureUsage usage{};
if (vkusage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) {
usage = usage | TextureUsage::DEPTH_ATTACHMENT;
}
if (vkusage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) {
usage = usage | TextureUsage::COLOR_ATTACHMENT;
}
if (vkusage & VK_IMAGE_USAGE_SAMPLED_BIT) {
usage = usage | TextureUsage::SAMPLEABLE;
}
return getDefaultLayout(usage);
}

constexpr inline VkImageLayout getVkLayout(VulkanLayout layout) {
switch (layout) {
case VulkanLayout::UNDEFINED:
Expand Down
4 changes: 2 additions & 2 deletions filament/backend/src/vulkan/VulkanReadPixels.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ void VulkanReadPixels::run(VulkanRenderTarget* srcTarget, uint32_t const x, uint

VulkanAttachment const srcAttachment = srcTarget->getColor(0);
VkImageSubresourceRange const srcRange = srcAttachment.getSubresourceRange();
srcTexture->transitionLayout(cmdbuffer, {}, srcRange, VulkanLayout::TRANSFER_SRC);
srcTexture->transitionLayout(cmdbuffer, srcRange, VulkanLayout::TRANSFER_SRC);

VkImageCopy const imageCopyRegion = {
.srcSubresource = {
Expand Down Expand Up @@ -268,7 +268,7 @@ void VulkanReadPixels::run(VulkanRenderTarget* srcTarget, uint32_t const x, uint
imgutil::getVkLayout(VulkanLayout::TRANSFER_DST), 1, &imageCopyRegion);

// Restore the source image layout.
srcTexture->transitionLayout(cmdbuffer, {}, srcRange, VulkanLayout::COLOR_ATTACHMENT);
srcTexture->transitionLayout(cmdbuffer, srcRange, srcTexture->getDefaultLayout());

vkEndCommandBuffer(cmdbuffer);

Expand Down
114 changes: 74 additions & 40 deletions filament/backend/src/vulkan/VulkanTexture.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,25 +113,52 @@ VkComponentMapping composeSwizzle(VkComponentMapping const& prev, VkComponentMap
};
}

} // anonymous namespace
inline VulkanLayout getDefaultLayoutImpl(TextureUsage usage) {
if (any(usage & TextureUsage::DEPTH_ATTACHMENT)) {
if (any(usage & TextureUsage::SAMPLEABLE)) {
return VulkanLayout::DEPTH_SAMPLER;
} else {
return VulkanLayout::DEPTH_ATTACHMENT;
}
}

VulkanTextureState::VulkanTextureState(
VkDevice device, VmaAllocator allocator, VulkanCommands* commands,
VulkanStagePool& stagePool,
VkFormat format, VkImageViewType viewType, uint8_t levels, uint8_t layerCount)
: VulkanResource(VulkanResourceType::HEAP_ALLOCATED),
mVkFormat(format),
mViewType(viewType),
mFullViewRange {
filament::backend::getImageAspect(format), 0, levels, 0, layerCount
},
mStagePool(stagePool),
mDevice(device),
mAllocator(allocator),
mCommands(commands),
mIsTransientAttachment(false) {
if (any(usage & TextureUsage::COLOR_ATTACHMENT)) {
return VulkanLayout::COLOR_ATTACHMENT;
}
// Finally, the layout for an immutable texture is optimal read-only.
return VulkanLayout::READ_ONLY;
}

inline VulkanLayout getDefaultLayoutImpl(VkImageUsageFlags vkusage) {
TextureUsage usage{};
if (vkusage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) {
usage = usage | TextureUsage::DEPTH_ATTACHMENT;
}
if (vkusage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) {
usage = usage | TextureUsage::COLOR_ATTACHMENT;
}
if (vkusage & VK_IMAGE_USAGE_SAMPLED_BIT) {
usage = usage | TextureUsage::SAMPLEABLE;
}
return getDefaultLayoutImpl(usage);
}

} // anonymous namespace

VulkanTextureState::VulkanTextureState(VkDevice device, VmaAllocator allocator,
VulkanCommands* commands, VulkanStagePool& stagePool, VkFormat format,
VkImageViewType viewType, uint8_t levels, uint8_t layerCount, VulkanLayout defaultLayout)
: VulkanResource(VulkanResourceType::HEAP_ALLOCATED),
mVkFormat(format),
mViewType(viewType),
mFullViewRange{filament::backend::getImageAspect(format), 0, levels, 0, layerCount},
mDefaultLayout(defaultLayout),
mStagePool(stagePool),
mDevice(device),
mAllocator(allocator),
mCommands(commands),
mIsTransientAttachment(false) {}

VulkanTextureState* VulkanTexture::getSharedState() {
VulkanTextureState* state = mAllocator->handle_cast<VulkanTextureState*>(mState);
return state;
Expand All @@ -142,6 +169,7 @@ VulkanTextureState const* VulkanTexture::getSharedState() const {
return state;
}

// Constructor for internally passed VkImage
VulkanTexture::VulkanTexture(
VkDevice device, VmaAllocator allocator, VulkanCommands* commands,
VulkanResourceAllocator* handleAllocator,
Expand All @@ -154,12 +182,14 @@ VulkanTexture::VulkanTexture(
mAllocator(handleAllocator),
mState(handleAllocator->initHandle<VulkanTextureState>(
device, allocator, commands, stagePool,
format, imgutil::getViewType(SamplerType::SAMPLER_2D), 1, 1)) {
format, imgutil::getViewType(SamplerType::SAMPLER_2D), 1, 1,
getDefaultLayoutImpl(tusage))) {
auto* const state = getSharedState();
state->mTextureImage = image;
mPrimaryViewRange = state->mFullViewRange;
}

// Constructor for user facing texture
VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice,
VulkanContext const& context, VmaAllocator allocator, VulkanCommands* commands,
VulkanResourceAllocator* handleAllocator, SamplerType target, uint8_t levels,
Expand All @@ -171,7 +201,7 @@ VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice,
mAllocator(handleAllocator),
mState(handleAllocator->initHandle<VulkanTextureState>(device, allocator, commands, stagePool,
backend::getVkFormat(tformat), imgutil::getViewType(target), levels,
getLayerCount(target, depth))) {
getLayerCount(target, depth), VulkanLayout::UNDEFINED)) {
auto* const state = getSharedState();

// Create an appropriately-sized device-only VkImage, but do not fill it yet.
Expand Down Expand Up @@ -326,18 +356,20 @@ VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice,

VulkanCommandBuffer& commandsBuf = state->mCommands->get();
commandsBuf.acquire(this);
transitionLayout(&commandsBuf, mPrimaryViewRange, imgutil::getDefaultLayout(imageInfo.usage));

auto const defaultLayout = state->mDefaultLayout = getDefaultLayoutImpl(imageInfo.usage);
transitionLayout(&commandsBuf, mPrimaryViewRange, defaultLayout);
}

// Constructor for creating a texture view
VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice,
VulkanContext const& context, VmaAllocator allocator, VulkanCommands* commands,
VulkanResourceAllocator* handleAllocator,
VulkanTexture const* src, uint8_t baseLevel, uint8_t levelCount)
: HwTexture(src->target, src->levels, src->samples, src->width, src->height, src->depth,
src->format, src->usage),
VulkanResource(VulkanResourceType::TEXTURE),
mAllocator(handleAllocator)
{
VulkanResourceAllocator* handleAllocator, VulkanTexture const* src, uint8_t baseLevel,
uint8_t levelCount)
: HwTexture(src->target, src->levels, src->samples, src->width, src->height, src->depth,
src->format, src->usage),
VulkanResource(VulkanResourceType::TEXTURE),
mAllocator(handleAllocator) {
mState = src->mState;
auto* state = getSharedState();

Expand All @@ -347,14 +379,15 @@ VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice,
mPrimaryViewRange.levelCount = levelCount;
}

// Constructor for creating a texture view with swizzle
VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice,
VulkanContext const& context, VmaAllocator allocator, VulkanCommands* commands,
VulkanResourceAllocator* handleAllocator,
VulkanTexture const* src, VkComponentMapping swizzle)
: HwTexture(src->target, src->levels, src->samples, src->width, src->height, src->depth,
src->format, src->usage),
VulkanResource(VulkanResourceType::TEXTURE),
mAllocator(handleAllocator) {
VulkanResourceAllocator* handleAllocator, VulkanTexture const* src,
VkComponentMapping swizzle)
: HwTexture(src->target, src->levels, src->samples, src->width, src->height, src->depth,
src->format, src->usage),
VulkanResource(VulkanResourceType::TEXTURE),
mAllocator(handleAllocator) {
mState = src->mState;
auto* state = getSharedState();
state->refs++;
Expand Down Expand Up @@ -457,7 +490,7 @@ void VulkanTexture::updateImage(const PixelBufferDescriptor& data, uint32_t widt
VkImageLayout const newVkLayout = imgutil::getVkLayout(newLayout);

if (nextLayout == VulkanLayout::UNDEFINED) {
nextLayout = imgutil::getDefaultLayout(this->usage);
nextLayout = getDefaultLayout();
}

transitionLayout(&commands, transitionRange, newLayout);
Expand Down Expand Up @@ -508,6 +541,11 @@ void VulkanTexture::updateImageWithBlit(const PixelBufferDescriptor& hostData, u
transitionLayout(&commands, range, oldLayout);
}

VulkanLayout VulkanTexture::getDefaultLayout() const {
auto* const state = getSharedState();
return state->mDefaultLayout;
}

VkImageView VulkanTexture::getAttachmentView(VkImageSubresourceRange range) {
range.levelCount = 1;
range.layerCount = 1;
Expand Down Expand Up @@ -553,13 +591,11 @@ VkImageAspectFlags VulkanTexture::getImageAspect() const {
}

bool VulkanTexture::transitionLayout(VulkanCommandBuffer* commands,
const VkImageSubresourceRange& range, VulkanLayout newLayout) {
return transitionLayout(commands->buffer(), commands->fence, range, newLayout);
VkImageSubresourceRange const& range, VulkanLayout newLayout) {
return transitionLayout(commands->buffer(), range, newLayout);
}

bool VulkanTexture::transitionLayout(
VkCommandBuffer cmdbuf, std::shared_ptr<VulkanCmdFence> fence,
const VkImageSubresourceRange& range,
bool VulkanTexture::transitionLayout(VkCommandBuffer cmdbuf, VkImageSubresourceRange const& range,
VulkanLayout newLayout) {
auto* const state = getSharedState();
VulkanLayout const oldLayout = getLayout(range.baseArrayLayer, range.baseMipLevel);
Expand Down Expand Up @@ -619,8 +655,6 @@ bool VulkanTexture::transitionLayout(
setLayout(range, newLayout);

if (hasTransitions) {
state->mTransitionFence = fence;

#if FVK_ENABLED(FVK_DEBUG_LAYOUT_TRANSITION)
FVK_LOGD << "transition texture=" << state->mTextureImage << " (" << range.baseArrayLayer
<< "," << range.baseMipLevel << ")" << " count=(" << range.layerCount << ","
Expand Down
24 changes: 10 additions & 14 deletions filament/backend/src/vulkan/VulkanTexture.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ class VulkanResourceAllocator;

struct VulkanTextureState : public VulkanResource {
VulkanTextureState(VkDevice device, VmaAllocator allocator, VulkanCommands* commands,
VulkanStagePool& stagePool,
VkFormat format, VkImageViewType viewType, uint8_t levels, uint8_t layerCount);
VulkanStagePool& stagePool, VkFormat format, VkImageViewType viewType, uint8_t levels,
uint8_t layerCount, VulkanLayout defaultLayout);

struct ImageViewKey {
VkImageSubresourceRange range; // 4 * 5 bytes
Expand Down Expand Up @@ -67,8 +67,8 @@ struct VulkanTextureState : public VulkanResource {
VkFormat const mVkFormat;
VkImageViewType const mViewType;
VkImageSubresourceRange const mFullViewRange;

VkImage mTextureImage = VK_NULL_HANDLE;
VulkanLayout mDefaultLayout;

// Track the image layout of each subresource using a sparse range map.
utils::RangeMap<uint32_t, VulkanLayout> mSubresourceLayouts;
Expand All @@ -78,7 +78,6 @@ struct VulkanTextureState : public VulkanResource {
VkDevice mDevice;
VmaAllocator mAllocator;
VulkanCommands* mCommands;
std::shared_ptr<VulkanCmdFence> mTransitionFence;
bool mIsTransientAttachment;
};

Expand Down Expand Up @@ -135,6 +134,10 @@ struct VulkanTexture : public HwTexture, VulkanResource {
return getLayout(mPrimaryViewRange.baseArrayLayer, mPrimaryViewRange.baseMipLevel);
}

// Returns the layout for the intended use of this texture (and not the expected layout at the
// time of the call.
VulkanLayout getDefaultLayout() const;

// Gets or creates a cached VkImageView for a single subresource that can be used as a render
// target attachment. Unlike the primary image view, this always has type VK_IMAGE_VIEW_TYPE_2D
// and the identity swizzle.
Expand Down Expand Up @@ -176,11 +179,11 @@ struct VulkanTexture : public HwTexture, VulkanResource {
return state->mIsTransientAttachment;
}

bool transitionLayout(VulkanCommandBuffer* commands, const VkImageSubresourceRange& range,
bool transitionLayout(VulkanCommandBuffer* commands, VkImageSubresourceRange const& range,
VulkanLayout newLayout);

bool transitionLayout(VkCommandBuffer cmdbuf, std::shared_ptr<VulkanCmdFence> fence,
VkImageSubresourceRange const& range, VulkanLayout newLayout);
bool transitionLayout(VkCommandBuffer cmdbuf, VkImageSubresourceRange const& range,
VulkanLayout newLayout);

void attachmentToSamplerBarrier(VulkanCommandBuffer* commands,
VkImageSubresourceRange const& range);
Expand All @@ -196,13 +199,6 @@ struct VulkanTexture : public HwTexture, VulkanResource {
// manually (outside of calls to transitionLayout).
void setLayout(VkImageSubresourceRange const& range, VulkanLayout newLayout);

bool transitionReady() {
VulkanTextureState* state = getSharedState();
auto res = !state->mTransitionFence || state->mTransitionFence->getStatus() == VK_SUCCESS;
state->mTransitionFence.reset();
return res;
}

#if FVK_ENABLED(FVK_DEBUG_TEXTURE)
void print() const;
#endif
Expand Down
2 changes: 1 addition & 1 deletion filament/backend/src/vulkan/caching/VulkanDescriptorSetManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ void VulkanDescriptorSetManager::updateSampler(VulkanDescriptorSet* set, uint8_t
} else {
info.imageView = texture->getViewForType(range, expectedType);
}
info.imageLayout = imgutil::getVkLayout(texture->getPrimaryImageLayout());
info.imageLayout = imgutil::getVkLayout(texture->getDefaultLayout());
VkWriteDescriptorSet const descriptorWrite = {
.sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET,
.pNext = nullptr,
Expand Down

0 comments on commit 875b295

Please sign in to comment.