Scarfski/filament
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

vk: clean-up VulkanContext usage (#8945)

Browse Source 
- Make sure that we don't allocate a VulkanContext on the function
   stack since this was passed erroroneously to other components
   and stored as a pointer
 - Use const& to refer to VulkanContext across components for
   consistency.
 - Remove unused VkPhysicalDeviceProtectedMemoryProperties struct.
This commit is contained in:
Powei Feng
2025-07-14 19:29:55 +00:00
committed by GitHub
parent 1422c8cba9
commit f64117fa26
5 changed files with 37 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
filament/backend/src/vulkan/VulkanCommands.cpp
+10 -10
View File
@@ -86,8 +86,8 @@ bool VulkanGroupMarkers::empty() const noexcept {
}
#endif // FVK_DEBUG_GROUP_MARKERS
VulkanCommandBuffer::VulkanCommandBuffer(VulkanContext* context, VkDevice device, VkQueue queue,
VkCommandPool pool, bool isProtected)
VulkanCommandBuffer::VulkanCommandBuffer(VulkanContext const& context, VkDevice device,
VkQueue queue, VkCommandPool pool, bool isProtected)
: mContext(context),
mMarkerCount(0),
isProtected(isProtected),
@@ -120,14 +120,14 @@ void VulkanCommandBuffer::reset() noexcept {
}
void VulkanCommandBuffer::pushMarker(char const* marker) noexcept {
if (mContext->isDebugUtilsSupported()) {
if (mContext.isDebugUtilsSupported()) {
VkDebugUtilsLabelEXT labelInfo = {
.sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_LABEL_EXT,
.pLabelName = marker,
.color = {0, 1, 0, 1},
};
vkCmdBeginDebugUtilsLabelEXT(mBuffer, &labelInfo);
} else if (mContext->isDebugMarkersSupported()) {
} else if (mContext.isDebugMarkersSupported()) {
VkDebugMarkerMarkerInfoEXT markerInfo = {
.sType = VK_STRUCTURE_TYPE_DEBUG_MARKER_MARKER_INFO_EXT,
.pMarkerName = marker,
@@ -140,23 +140,23 @@ void VulkanCommandBuffer::pushMarker(char const* marker) noexcept {
void VulkanCommandBuffer::popMarker() noexcept{
assert_invariant(mMarkerCount > 0);
if (mContext->isDebugUtilsSupported()) {
if (mContext.isDebugUtilsSupported()) {
vkCmdEndDebugUtilsLabelEXT(mBuffer);
} else if (mContext->isDebugMarkersSupported()) {
} else if (mContext.isDebugMarkersSupported()) {
vkCmdDebugMarkerEndEXT(mBuffer);
}
mMarkerCount--;
}
void VulkanCommandBuffer::insertEvent(char const* marker) noexcept {
if (mContext->isDebugUtilsSupported()) {
if (mContext.isDebugUtilsSupported()) {
VkDebugUtilsLabelEXT labelInfo = {
.sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_LABEL_EXT,
.pLabelName = marker,
.color = {1, 1, 0, 1},
};
vkCmdInsertDebugUtilsLabelEXT(mBuffer, &labelInfo);
} else if (mContext->isDebugMarkersSupported()) {
} else if (mContext.isDebugMarkersSupported()) {
VkDebugMarkerMarkerInfoEXT markerInfo = {
.sType = VK_STRUCTURE_TYPE_DEBUG_MARKER_MARKER_INFO_EXT,
.pMarkerName = marker,
@@ -232,7 +232,7 @@ VkSemaphore VulkanCommandBuffer::submit() {
return mSubmission;
}
CommandBufferPool::CommandBufferPool(VulkanContext* context, VkDevice device, VkQueue queue,
CommandBufferPool::CommandBufferPool(VulkanContext const& context, VkDevice device, VkQueue queue,
uint8_t queueFamilyIndex, bool isProtected)
: mDevice(device),
mRecording(INVALID) {
@@ -384,7 +384,7 @@ void CommandBufferPool::insertEvent(char const* marker) {
#endif // FVK_DEBUG_GROUP_MARKERS
VulkanCommands::VulkanCommands(VkDevice device, VkQueue queue, uint32_t queueFamilyIndex,
VkQueue protectedQueue, uint32_t protectedQueueFamilyIndex, VulkanContext* context)
VkQueue protectedQueue, uint32_t protectedQueueFamilyIndex, VulkanContext const& context)
: mDevice(device),
mProtectedQueue(protectedQueue),
mProtectedQueueFamilyIndex(protectedQueueFamilyIndex),
filament/backend/src/vulkan/VulkanCommands.h
+7 -7
View File
@@ -23,6 +23,7 @@
#include "VulkanAsyncHandles.h"
#include "VulkanConstants.h"
#include "VulkanContext.h"
#include "vulkan/memory/ResourcePointer.h"
#include "vulkan/utils/StaticVector.h"
@@ -41,8 +42,6 @@ namespace filament::backend {
using namespace fvkmemory;
struct VulkanContext;
#if FVK_ENABLED(FVK_DEBUG_GROUP_MARKERS)
class VulkanGroupMarkers {
public:
@@ -64,7 +63,7 @@ private:
// DriverApi fence object and should not be destroyed until both the DriverApi object is freed and
// we're done waiting on the most recent submission of the given command buffer.
struct VulkanCommandBuffer {
VulkanCommandBuffer(VulkanContext* mContext,
VulkanCommandBuffer(VulkanContext const& mContext,
VkDevice device, VkQueue queue, VkCommandPool pool, bool isProtected);
VulkanCommandBuffer(VulkanCommandBuffer const&) = delete;
@@ -110,7 +109,7 @@ struct VulkanCommandBuffer {
}
private:
VulkanContext* mContext;
VulkanContext const& mContext;
uint8_t mMarkerCount;
bool const isProtected;
VkDevice mDevice;
@@ -127,7 +126,7 @@ struct CommandBufferPool {
using ActiveBuffers = utils::bitset64;
static constexpr int8_t INVALID = -1;
CommandBufferPool(VulkanContext* context, VkDevice device, VkQueue queue,
CommandBufferPool(VulkanContext const& context, VkDevice device, VkQueue queue,
uint8_t queueFamilyIndex, bool isProtected);
~CommandBufferPool();
@@ -195,7 +194,8 @@ private:
class VulkanCommands {
public:
VulkanCommands(VkDevice device, VkQueue queue, uint32_t queueFamilyIndex,
VkQueue protectedQueue, uint32_t protectedQueueFamilyIndex, VulkanContext* context);
VkQueue protectedQueue, uint32_t protectedQueueFamilyIndex,
VulkanContext const& context);
void terminate();
@@ -246,7 +246,7 @@ private:
VkQueue const mProtectedQueue;
// For defered initialization if/when we need protected content
uint32_t const mProtectedQueueFamilyIndex;
VulkanContext* mContext;
VulkanContext const& mContext;
std::unique_ptr<CommandBufferPool> mPool;
std::unique_ptr<CommandBufferPool> mProtectedPool;
filament/backend/src/vulkan/VulkanDriver.cpp
+10 -8
View File
@@ -143,8 +143,10 @@ VKAPI_ATTR VkBool32 VKAPI_CALL debugUtilsCallback(VkDebugUtilsMessageSeverityFla
using DebugUtils = VulkanDriver::DebugUtils;
DebugUtils* DebugUtils::mSingleton = nullptr;
DebugUtils::DebugUtils(VkInstance instance, VkDevice device, VulkanContext const* context)
: mInstance(instance), mDevice(device), mEnabled(context->isDebugUtilsSupported()) {
DebugUtils::DebugUtils(VkInstance instance, VkDevice device, VulkanContext const& context)
: mInstance(instance),
mDevice(device),
mEnabled(context.isDebugUtilsSupported()) {
#if FVK_ENABLED(FVK_DEBUG_VALIDATION)
// Also initialize debug utils messenger here
@@ -199,7 +201,7 @@ Dispatcher VulkanDriver::getDispatcher() const noexcept {
return ConcreteDispatcher<VulkanDriver>::make();
}
VulkanDriver::VulkanDriver(VulkanPlatform* platform, VulkanContext const& context,
VulkanDriver::VulkanDriver(VulkanPlatform* platform, VulkanContext& context,
Platform::DriverConfig const& driverConfig)
: mPlatform(platform),
mResourceManager(driverConfig.handleArenaSize, driverConfig.disableHandleUseAfterFreeCheck,
@@ -209,11 +211,11 @@ VulkanDriver::VulkanDriver(VulkanPlatform* platform, VulkanContext const& contex
mContext(context),
mCommands(mPlatform->getDevice(), mPlatform->getGraphicsQueue(),
mPlatform->getGraphicsQueueFamilyIndex(), mPlatform->getProtectedGraphicsQueue(),
mPlatform->getProtectedGraphicsQueueFamilyIndex(), &mContext),
mPlatform->getProtectedGraphicsQueueFamilyIndex(), mContext),
mPipelineLayoutCache(mPlatform->getDevice()),
mPipelineCache(mPlatform->getDevice()),
mStagePool(mAllocator, &mResourceManager, &mCommands, &mContext.getPhysicalDeviceLimits()),
mBufferCache(context, mResourceManager, mAllocator),
mBufferCache(mContext, mResourceManager, mAllocator),
mFramebufferCache(mPlatform->getDevice()),
mYcbcrConversionCache(mPlatform->getDevice()),
mSamplerCache(mPlatform->getDevice()),
@@ -229,13 +231,13 @@ VulkanDriver::VulkanDriver(VulkanPlatform* platform, VulkanContext const& contex
#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS)
DebugUtils::mSingleton =
new DebugUtils(mPlatform->getInstance(), mPlatform->getDevice(), &context);
new DebugUtils(mPlatform->getInstance(), mPlatform->getDevice(), &mContext);
#endif
#if FVK_ENABLED(FVK_DEBUG_VALIDATION)
UTILS_UNUSED const PFN_vkCreateDebugReportCallbackEXT createDebugReportCallback
= vkCreateDebugReportCallbackEXT;
if (!context.isDebugUtilsSupported() && createDebugReportCallback) {
if (!mContext.isDebugUtilsSupported() && createDebugReportCallback) {
VkDebugReportCallbackCreateInfoEXT const cbinfo = {
.sType = VK_STRUCTURE_TYPE_DEBUG_REPORT_CALLBACK_CREATE_INFO_EXT,
.flags = VK_DEBUG_REPORT_WARNING_BIT_EXT | VK_DEBUG_REPORT_ERROR_BIT_EXT,
@@ -253,7 +255,7 @@ VulkanDriver::VulkanDriver(VulkanPlatform* platform, VulkanContext const& contex
VulkanDriver::~VulkanDriver() noexcept = default;
UTILS_NOINLINE
Driver* VulkanDriver::create(VulkanPlatform* platform, VulkanContext const& context,
Driver* VulkanDriver::create(VulkanPlatform* platform, VulkanContext& context,
Platform::DriverConfig const& driverConfig) {
#if 0
// this is useful for development, but too verbose even for debug builds
filament/backend/src/vulkan/VulkanDriver.h
+4 -4
View File
@@ -56,7 +56,7 @@ constexpr uint8_t MAX_RENDERTARGET_ATTACHMENT_TEXTURES =
class VulkanDriver final : public DriverBase {
public:
static Driver* create(VulkanPlatform* platform, VulkanContext const& context,
static Driver* create(VulkanPlatform* platform, VulkanContext& context,
Platform::DriverConfig const& driverConfig);
#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS)
@@ -69,7 +69,7 @@ public:
private:
static DebugUtils* get();
DebugUtils(VkInstance instance, VkDevice device, VulkanContext const* context);
DebugUtils(VkInstance instance, VkDevice device, VulkanContext const& context);
~DebugUtils();
VkInstance const mInstance;
@@ -92,7 +92,7 @@ private:
void debugCommandBegin(CommandStream* cmds, bool synchronous,
const char* methodName) noexcept override;
VulkanDriver(VulkanPlatform* platform, VulkanContext const& context,
VulkanDriver(VulkanPlatform* platform, VulkanContext& context,
Platform::DriverConfig const& driverConfig);
~VulkanDriver() noexcept override;
@@ -134,7 +134,7 @@ private:
VmaAllocator mAllocator = VK_NULL_HANDLE;
VkDebugReportCallbackEXT mDebugCallback = VK_NULL_HANDLE;
VulkanContext mContext = {};
VulkanContext& mContext;
VulkanCommands mCommands;
VulkanPipelineLayoutCache mPipelineLayoutCache;
filament/backend/src/vulkan/platform/VulkanPlatform.cpp
+6 -9
View File
@@ -740,7 +740,7 @@ Driver* VulkanPlatform::createDriver(void* sharedContext,
mImpl->mSharedContext = true;
}
VulkanContext context;
VulkanContext& context = mImpl->mContext;
ExtensionSet instExts;
// If using a shared context, we do not assume any extensions.
if (!mImpl->mSharedContext) {
@@ -787,12 +787,11 @@ Driver* VulkanPlatform::createDriver(void* sharedContext,
VkPhysicalDeviceProtectedMemoryFeatures queryProtectedMemoryFeatures = {
.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROTECTED_MEMORY_FEATURES,
};
VkPhysicalDeviceProtectedMemoryProperties protectedMemoryProperties = {
.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROTECTED_MEMORY_PROPERTIES,
};
// Note that we're chaining a struct on the stack into a struct in context, which will then be
// passed to the driver. However, this should be ok since the use of
// queryProtectedMemoryFeatures is only in this function.
chainStruct(&context.mPhysicalDeviceFeatures, &queryProtectedMemoryFeatures);
chainStruct(&context.mPhysicalDeviceFeatures, &context.mPhysicalDeviceVk11Features);
chainStruct(&context.mPhysicalDeviceProperties, &protectedMemoryProperties);
// Initialize the following fields: physicalDeviceProperties, memoryProperties,
// physicalDeviceFeatures, graphicsQueueFamilyIndex.
@@ -923,10 +922,8 @@ Driver* VulkanPlatform::createDriver(void* sharedContext,
printDepthFormats(mImpl->mPhysicalDevice);
#endif
// Keep a copy of context for swapchains.
mImpl->mContext = context;
return VulkanDriver::create(this, mImpl->mContext, driverConfig);
// Note that `context` is an alias of mImpl->mContext.
return VulkanDriver::create(this, context, driverConfig);
}
// This needs to be explictly written for