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.

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

Vk protected content command buffer #8241

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6218164
Adding the begin frame message for later xtrace post processing.
phoenixxxx Jun 4, 2024
e265850
Merge branch 'main' into main
phoenixxxx Jun 4, 2024
8671617
Merge branch 'main' into main
poweifeng Jun 5, 2024
d49554d
Merge branch 'main' into main
poweifeng Jun 5, 2024
4fccd95
Merge branch 'google:main' into main
phoenixxxx Jun 11, 2024
96cba96
Merge branch 'google:main' into main
phoenixxxx Jun 18, 2024
abe8a49
Merge branch 'google:main' into main
phoenixxxx Jun 27, 2024
c81cedd
Merge branch 'google:main' into main
phoenixxxx Sep 4, 2024
387942c
Merge branch 'google:main' into main
phoenixxxx Sep 21, 2024
0fdf1ca
Merge branch 'google:main' into main
phoenixxxx Oct 3, 2024
8bc4d3a
Merge branch 'google:main' into main
phoenixxxx Oct 9, 2024
9e92193
Merge branch 'google:main' into main
phoenixxxx Oct 18, 2024
f3afc25
Support for protected mode (part 1)
phoenixxxx Oct 24, 2024
0a2d9af
PR Feedback
phoenixxxx Oct 24, 2024
adc0197
Fixing CL based on feedback.
phoenixxxx Oct 25, 2024
b51a46d
Addressing more feedback from the PR
phoenixxxx Oct 26, 2024
01a2480
Merge branch 'google:main' into main
phoenixxxx Oct 26, 2024
58dc0e5
Merge branch 'main' into vk_protected_content_platform
phoenixxxx Oct 26, 2024
c42b76a
Adding structure types
phoenixxxx Oct 26, 2024
c2f3b1b
Start of the cmd buffer portion.
phoenixxxx Oct 29, 2024
48bb10a
Merge branch 'google:main' into main
phoenixxxx Oct 30, 2024
0478533
VK Comman protection
phoenixxxx Oct 30, 2024
dcdbbdb
Merge branch 'main' into vk_protected_content_command_buffer
phoenixxxx Oct 30, 2024
46b77ec
Fixing compiler error
phoenixxxx Oct 30, 2024
8797b0a
The scheduling of the protected cmd buffers
phoenixxxx Oct 30, 2024
b0d6799
Powei's VulkanCommand implementation.
phoenixxxx Oct 31, 2024
c0e0048
Missing code path for protection
phoenixxxx Oct 31, 2024
6d98715
Forgot to the previous commit.
phoenixxxx Oct 31, 2024
d8811f0
Android build error.
phoenixxxx Nov 1, 2024
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
757 changes: 404 additions & 353 deletions filament/backend/src/vulkan/VulkanCommands.cpp

Large diffs are not rendered by default.

409 changes: 220 additions & 189 deletions filament/backend/src/vulkan/VulkanCommands.h

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion filament/backend/src/vulkan/VulkanContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ void VulkanTimestamps::beginQuery(VulkanCommandBuffer const* commands,
vkCmdWriteTimestamp(cmdbuffer, VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, mPool, index);

// We stash this because getResult might come before the query is actually processed.
query->setFence(commands->fence);
query->setFence(commands->getFenceStatus());
}

void VulkanTimestamps::endQuery(VulkanCommandBuffer const* commands,
Expand Down
1 change: 1 addition & 0 deletions filament/backend/src/vulkan/VulkanContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class VulkanTimestamps {
};

struct VulkanRenderPass {
VulkanCommandBuffer* cmdBuffer;
VulkanRenderTarget* renderTarget;
VkRenderPass renderPass;
RenderPassParams params;
Expand Down
43 changes: 30 additions & 13 deletions filament/backend/src/vulkan/VulkanDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,11 @@ VulkanDriver::VulkanDriver(VulkanPlatform* platform, VulkanContext const& contex
mResourceAllocator(driverConfig.handleArenaSize, driverConfig.disableHandleUseAfterFreeCheck),
mResourceManager(&mResourceAllocator),
mThreadSafeResourceManager(&mResourceAllocator),
mCommands(mPlatform->getDevice(), mPlatform->getGraphicsQueue(),
mPlatform->getGraphicsQueueFamilyIndex(), &mContext, &mResourceAllocator),
mCommands(mPlatform->getDevice(), mPlatform->getGraphicsQueue(),
mPlatform->getGraphicsQueueFamilyIndex(),
mPlatform->getProtectedGraphicsQueue(),
mPlatform->getProtectedGraphicsQueueFamilyIndex(),
&mContext, &mResourceAllocator),
mPipelineLayoutCache(mPlatform->getDevice(), &mResourceAllocator),
mPipelineCache(mPlatform->getDevice(), mAllocator),
mStagePool(mAllocator, &mCommands),
Expand Down Expand Up @@ -690,8 +693,14 @@ void VulkanDriver::destroyRenderTarget(Handle<HwRenderTarget> rth) {
}

void VulkanDriver::createFenceR(Handle<HwFence> fh, int) {
VulkanCommandBuffer const& commandBuffer = mCommands.get();
mResourceAllocator.construct<VulkanFence>(fh, commandBuffer.fence);
VulkanCommandBuffer* cmdbuf = nullptr;
if (mCurrentRenderPass.cmdBuffer) {
cmdbuf = mCurrentRenderPass.cmdBuffer;
}
else {
cmdbuf = &mCommands.get();
}
mResourceAllocator.construct<VulkanFence>(fh, cmdbuf->getFenceStatus());
}

void VulkanDriver::createSwapChainR(Handle<HwSwapChain> sch, void* nativeWindow, uint64_t flags) {
Expand All @@ -703,6 +712,13 @@ void VulkanDriver::createSwapChainR(Handle<HwSwapChain> sch, void* nativeWindow,
auto swapChain = mResourceAllocator.construct<VulkanSwapChain>(sch, mPlatform, mContext,
mAllocator, &mCommands, &mResourceAllocator, mStagePool, nativeWindow, flags);
mResourceManager.acquire(swapChain);

if (flags & backend::SWAP_CHAIN_CONFIG_PROTECTED_CONTENT) {
if (!isProtectedContentSupported()) {
utils::slog.w << "protected swapchain requested, but Vulkan does not support it"
<< utils::io::endl;
}
}
}

void VulkanDriver::createSwapChainHeadlessR(Handle<HwSwapChain> sch, uint32_t width,
Expand Down Expand Up @@ -1257,6 +1273,7 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
sc->markFirstRenderPass();
}
}
mIsRenderPassProtected = rt->isProtected();

#if FVK_ENABLED(FVK_DEBUG_TEXTURE)
if (rt->hasDepth()) {
Expand All @@ -1269,7 +1286,7 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
// If that's the case, we need to change the layout of the texture to DEPTH_SAMPLER, which is a
// more general layout. Otherwise, we prefer the DEPTH_ATTACHMENT layout, which is optimal for
// the non-sampling case.
VulkanCommandBuffer& commands = mCommands.get();
VulkanCommandBuffer& commands = getCommandBuffer();
VkCommandBuffer const cmdbuffer = commands.buffer();

// Scissor is reset with each render pass
Expand Down Expand Up @@ -1390,7 +1407,7 @@ void VulkanDriver::endRenderPass(int) {
FVK_SYSTRACE_CONTEXT();
FVK_SYSTRACE_START("endRenderPass");

VulkanCommandBuffer& commands = mCommands.get();
VulkanCommandBuffer& commands = getCommandBuffer();
VkCommandBuffer cmdbuffer = commands.buffer();
vkCmdEndRenderPass(cmdbuffer);

Expand All @@ -1415,7 +1432,7 @@ void VulkanDriver::nextSubpass(int) {
assert_invariant(renderTarget);
assert_invariant(mCurrentRenderPass.params.subpassMask);

vkCmdNextSubpass(mCommands.get().buffer(), VK_SUBPASS_CONTENTS_INLINE);
vkCmdNextSubpass(getCommandBuffer().buffer(), VK_SUBPASS_CONTENTS_INLINE);

mPipelineCache.bindRenderPass(mCurrentRenderPass.renderPass,
++mCurrentRenderPass.currentSubpass);
Expand Down Expand Up @@ -1464,8 +1481,8 @@ void VulkanDriver::setPushConstant(backend::ShaderStage stage, uint8_t index,
backend::PushConstantVariant value) {
assert_invariant(mBoundPipeline.program && "Expect a program when writing to push constants");
VulkanCommands* commands = &mCommands;
mBoundPipeline.program->writePushConstant(commands, mBoundPipeline.pipelineLayout, stage, index,
value);
mBoundPipeline.program->writePushConstant(commands, (mIsContentProtected || mIsRenderPassProtected),
mBoundPipeline.pipelineLayout, stage, index, value);
}

void VulkanDriver::insertEventMarker(char const* string) {
Expand Down Expand Up @@ -1655,7 +1672,7 @@ void VulkanDriver::bindPipeline(PipelineState const& pipelineState) {
FVK_SYSTRACE_CONTEXT();
FVK_SYSTRACE_START("draw");

VulkanCommandBuffer* commands = &mCommands.get();
VulkanCommandBuffer* commands = &getCommandBuffer();
const VulkanVertexBufferInfo& vbi =
*mResourceAllocator.handle_cast<VulkanVertexBufferInfo*>(pipelineState.vertexBufferInfo);

Expand Down Expand Up @@ -1737,7 +1754,7 @@ void VulkanDriver::bindRenderPrimitive(Handle<HwRenderPrimitive> rph) {
FVK_SYSTRACE_CONTEXT();
FVK_SYSTRACE_START("bindRenderPrimitive");

VulkanCommandBuffer* commands = &mCommands.get();
VulkanCommandBuffer* commands = &getCommandBuffer();
VkCommandBuffer cmdbuffer = commands->buffer();
const VulkanRenderPrimitive& prim = *mResourceAllocator.handle_cast<VulkanRenderPrimitive*>(rph);
commands->acquire(prim.indexBuffer);
Expand Down Expand Up @@ -1778,7 +1795,7 @@ void VulkanDriver::draw2(uint32_t indexOffset, uint32_t indexCount, uint32_t ins
FVK_SYSTRACE_CONTEXT();
FVK_SYSTRACE_START("draw2");

VulkanCommandBuffer& commands = mCommands.get();
VulkanCommandBuffer& commands = getCommandBuffer();
VkCommandBuffer cmdbuffer = commands.buffer();

mDescriptorSetManager.commit(&commands, mBoundPipeline.pipelineLayout,
Expand Down Expand Up @@ -1809,7 +1826,7 @@ void VulkanDriver::dispatchCompute(Handle<HwProgram> program, math::uint3 workGr
}

void VulkanDriver::scissor(Viewport scissorBox) {
VulkanCommandBuffer& commands = mCommands.get();
VulkanCommandBuffer& commands = getCommandBuffer();
VkCommandBuffer cmdbuffer = commands.buffer();

// TODO: it's a common case that scissor() is called with (0, 0, maxint, maxint)
Expand Down
9 changes: 9 additions & 0 deletions filament/backend/src/vulkan/VulkanDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,15 @@ class VulkanDriver final : public DriverBase {

private:
void collectGarbage();
inline VulkanCommandBuffer& getCommandBuffer() {
Copy link
Contributor

Choose a reason for hiding this comment

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

let's remove this and also mIsRenderPassProtected and mIsContentProtected

// Check if protection encoding is needed:
if (mIsRenderPassProtected || mIsContentProtected)
return mCommands.getProtected();
else
return mCommands.get();
}
bool mIsRenderPassProtected = false;
bool mIsContentProtected = false;

VulkanPlatform* mPlatform = nullptr;
std::unique_ptr<VulkanTimestamps> mTimestamps;
Expand Down
17 changes: 14 additions & 3 deletions filament/backend/src/vulkan/VulkanHandles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,14 @@ PushConstantDescription::PushConstantDescription(backend::Program const& program
}
}

void PushConstantDescription::write(VulkanCommands* commands, VkPipelineLayout layout,
backend::ShaderStage stage, uint8_t index, backend::PushConstantVariant const& value) {
VulkanCommandBuffer* cmdbuf = &(commands->get());
void PushConstantDescription::write(VulkanCommands* commands, bool isProtected,
VkPipelineLayout layout, backend::ShaderStage stage, uint8_t index,
backend::PushConstantVariant const& value) {
VulkanCommandBuffer* cmdbuf;
if (isProtected)
cmdbuf = &(commands->getProtected());
else
cmdbuf = &(commands->get());
uint32_t binaryValue = 0;
UTILS_UNUSED_IN_RELEASE auto const& types = mTypes[(uint8_t) stage];
if (std::holds_alternative<bool>(value)) {
Expand Down Expand Up @@ -338,6 +343,8 @@ VulkanRenderTarget::VulkanRenderTarget(VkDevice device, VkPhysicalDevice physica
continue;
}

mProtected |= texture->getIsProtected();

attachments.push_back(attachment);
mInfo->colors.set(index);

Expand Down Expand Up @@ -384,6 +391,10 @@ VulkanRenderTarget::VulkanRenderTarget(VkDevice device, VkPhysicalDevice physica

if (depth.texture) {
auto depthTexture = depth.texture;

// largely unecessary but for completeness
mProtected |= depthTexture->getIsProtected();

mInfo->depthIndex = (uint8_t) attachments.size();
attachments.push_back(depth);
mResources.acquire(depthTexture);
Expand Down
13 changes: 8 additions & 5 deletions filament/backend/src/vulkan/VulkanHandles.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,9 @@ struct PushConstantDescription {

uint32_t getVkRangeCount() const noexcept { return mRangeCount; }

void write(VulkanCommands* commands, VkPipelineLayout layout, backend::ShaderStage stage,
uint8_t index, backend::PushConstantVariant const& value);
void write(VulkanCommands* commands, bool isProtected, VkPipelineLayout layout,
Copy link
Contributor

Choose a reason for hiding this comment

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

let's not change VulkanHandles in this PR

backend::ShaderStage stage, uint8_t index,
backend::PushConstantVariant const& value);

private:
static constexpr uint32_t ENTRY_SIZE = sizeof(uint32_t);
Expand Down Expand Up @@ -218,9 +219,11 @@ struct VulkanProgram : public HwProgram, VulkanResource {
return mInfo->pushConstantDescription.getVkRanges();
}

inline void writePushConstant(VulkanCommands* commands, VkPipelineLayout layout,
backend::ShaderStage stage, uint8_t index, backend::PushConstantVariant const& value) {
mInfo->pushConstantDescription.write(commands, layout, stage, index, value);
inline void writePushConstant(VulkanCommands* commands, bool isProtected,
VkPipelineLayout layout, backend::ShaderStage stage, uint8_t index,
backend::PushConstantVariant const& value) {
mInfo->pushConstantDescription.write(commands, isProtected, layout,
stage, index, value);
}

#if FVK_ENABLED_DEBUG_SAMPLER_NAME
Expand Down
5 changes: 0 additions & 5 deletions filament/backend/src/vulkan/VulkanPipelineCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,12 @@ void VulkanPipelineCache::bindPipeline(VulkanCommandBuffer* commands) {
VkCommandBuffer const cmdbuffer = commands->buffer();

PipelineCacheEntry* cacheEntry = getOrCreatePipeline();
// Check if the required pipeline is already bound.
if (cacheEntry->handle == commands->pipeline()) {
return;
}

// If an error occurred, allow higher levels to handle it gracefully.
assert_invariant(cacheEntry != nullptr && "Failed to create/find pipeline");

mBoundPipeline = mPipelineRequirements;
vkCmdBindPipeline(cmdbuffer, VK_PIPELINE_BIND_POINT_GRAPHICS, cacheEntry->handle);
commands->setPipeline(cacheEntry->handle);
}

VulkanPipelineCache::PipelineCacheEntry* VulkanPipelineCache::createPipeline() noexcept {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace filament::backend {
namespace {

std::tuple<VkImage, VkDeviceMemory> createImageAndMemory(VulkanContext const& context,
VkDevice device, VkExtent2D extent, VkFormat format) {
VkDevice device, VkExtent2D extent, VkFormat format, bool isProtected) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you do this in a separate PR?

bool const isDepth = isVkDepthFormat(format);
// Filament expects blit() to work with any texture, so we almost always set these usage flags
// (see copyFrame() and readPixels()).
Expand All @@ -38,6 +38,7 @@ std::tuple<VkImage, VkDeviceMemory> createImageAndMemory(VulkanContext const& co

VkImageCreateInfo imageInfo {
.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO,
.flags = isProtected ? VK_IMAGE_CREATE_PROTECTED_BIT : VkImageCreateFlags(0),
.imageType = VK_IMAGE_TYPE_2D,
.format = format,
.extent = {extent.width, extent.height, 1},
Expand All @@ -59,7 +60,8 @@ std::tuple<VkImage, VkDeviceMemory> createImageAndMemory(VulkanContext const& co
vkGetImageMemoryRequirements(device, image, &memReqs);

uint32_t memoryTypeIndex
= context.selectMemoryType(memReqs.memoryTypeBits, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT);
= context.selectMemoryType(memReqs.memoryTypeBits, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT |
(isProtected ? VK_MEMORY_PROPERTY_PROTECTED_BIT : 0U));

FILAMENT_CHECK_POSTCONDITION(memoryTypeIndex < VK_MAX_MEMORY_TYPES)
<< "VulkanPlatformSwapChainImpl: unable to find a memory type that meets requirements.";
Expand Down Expand Up @@ -110,8 +112,8 @@ void VulkanPlatformSwapChainImpl::destroy() {
mSwapChainBundle.colors.clear();
}

VkImage VulkanPlatformSwapChainImpl::createImage(VkExtent2D extent, VkFormat format) {
auto [image, memory] = createImageAndMemory(mContext, mDevice, extent, format);
VkImage VulkanPlatformSwapChainImpl::createImage(VkExtent2D extent, VkFormat format, bool isProtected) {
auto [image, memory] = createImageAndMemory(mContext, mDevice, extent, format, isProtected);
mMemory.insert({image, memory});
return image;
}
Expand Down Expand Up @@ -248,7 +250,7 @@ VkResult VulkanPlatformSurfaceSwapChain::create() {
mSwapChainBundle.colorFormat = surfaceFormat.format;
mSwapChainBundle.depthFormat =
selectDepthFormat(mContext.getAttachmentDepthStencilFormats(), mHasStencil);
mSwapChainBundle.depth = createImage(mSwapChainBundle.extent, mSwapChainBundle.depthFormat);
mSwapChainBundle.depth = createImage(mSwapChainBundle.extent, mSwapChainBundle.depthFormat, mIsProtected);
mSwapChainBundle.isProtected = mIsProtected;

FVK_LOGI << "vkCreateSwapchain"
Expand Down Expand Up @@ -356,13 +358,14 @@ VulkanPlatformHeadlessSwapChain::VulkanPlatformHeadlessSwapChain(VulkanContext c
images.reserve(HEADLESS_SWAPCHAIN_SIZE);
images.resize(HEADLESS_SWAPCHAIN_SIZE);
for (size_t i = 0; i < HEADLESS_SWAPCHAIN_SIZE; ++i) {
images[i] = createImage(extent, mSwapChainBundle.colorFormat);
assert_invariant(mSwapChainBundle.isProtected == false);
images[i] = createImage(extent, mSwapChainBundle.colorFormat, false);
}

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

VulkanPlatformHeadlessSwapChain::~VulkanPlatformHeadlessSwapChain() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ struct VulkanPlatformSwapChainImpl : public Platform::SwapChain {
// Non-virtual override-able method
void destroy();

VkImage createImage(VkExtent2D extent, VkFormat format);
VkImage createImage(VkExtent2D extent, VkFormat format, bool isProtected);

VulkanContext const& mContext;
VkDevice mDevice;
Expand Down
Loading