Sub-allocated descriptor sets #95
Conversation
| include(common RESULT_VARIABLE RES) | ||
| if(NOT RES) | ||
| message(FATAL_ERROR "common.cmake not found. Should be in {repo_root}/cmake directory") | ||
| endif() | ||
|
|
||
| nbl_create_executable_project("" "" "" "" "${NBL_EXECUTABLE_PROJECT_CREATION_PCH_TARGET}") | ||
|
|
||
| if(NBL_EMBED_BUILTIN_RESOURCES) | ||
| set(_BR_TARGET_ ${EXECUTABLE_NAME}_builtinResourceData) | ||
| set(RESOURCE_DIR "app_resources") | ||
|
|
||
| get_filename_component(_SEARCH_DIRECTORIES_ "${CMAKE_CURRENT_SOURCE_DIR}" ABSOLUTE) | ||
| get_filename_component(_OUTPUT_DIRECTORY_SOURCE_ "${CMAKE_CURRENT_BINARY_DIR}/src" ABSOLUTE) | ||
| get_filename_component(_OUTPUT_DIRECTORY_HEADER_ "${CMAKE_CURRENT_BINARY_DIR}/include" ABSOLUTE) | ||
|
|
||
| file(GLOB_RECURSE BUILTIN_RESOURCE_FILES RELATIVE "${CMAKE_CURRENT_SOURCE_DIR}/${RESOURCE_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}/${RESOURCE_DIR}/*") | ||
| foreach(RES_FILE ${BUILTIN_RESOURCE_FILES}) | ||
| LIST_BUILTIN_RESOURCE(RESOURCES_TO_EMBED "${RES_FILE}") | ||
| endforeach() | ||
|
|
||
| ADD_CUSTOM_BUILTIN_RESOURCES(${_BR_TARGET_} RESOURCES_TO_EMBED "${_SEARCH_DIRECTORIES_}" "${RESOURCE_DIR}" "nbl::this_example::builtin" "${_OUTPUT_DIRECTORY_HEADER_}" "${_OUTPUT_DIRECTORY_SOURCE_}") | ||
|
|
||
| LINK_BUILTIN_RESOURCES_TO_TARGET(${EXECUTABLE_NAME} ${_BR_TARGET_}) | ||
| endif() |
There was a problem hiding this comment.
I'd make this example 1x or 2x, using 2x for basic utility/extension tests now
There was a problem hiding this comment.
number 27 is up for grabs
| video::IGPUDescriptorSetLayout::SBinding bindings[1]; | ||
| { | ||
| bindings[0].binding = 0; | ||
| bindings[0].count = 65536u; | ||
| bindings[0].createFlags = core::bitflag(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_AFTER_BIND_BIT) | ||
| | IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_UNUSED_WHILE_PENDING_BIT | ||
| | IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_PARTIALLY_BOUND_BIT; | ||
| bindings[0].type = asset::IDescriptor::E_TYPE::ET_STORAGE_IMAGE; | ||
| bindings[0].stageFlags = asset::IShader::E_SHADER_STAGE::ESS_COMPUTE; | ||
| } |
There was a problem hiding this comment.
try to use const and designated initializers if you can
| if (i % 2 == 0) bindings[i].type = asset::IDescriptor::E_TYPE::ET_STORAGE_IMAGE; | ||
| else if (i % 2 == 1) bindings[i].type = asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER; |
There was a problem hiding this comment.
you might want to try all descriptor types just to make the testing complete
There was a problem hiding this comment.
enable acceleration structure if available, and even test that if its there
(can be done as separate PR, no clue if the AS refactor works without stupid typos yet)
| smart_refctd_ptr<IGPUImageView> m_descriptorImages[MaxDescriptors]; | ||
| smart_refctd_ptr<IGPUBuffer> m_descriptorBuffers[MaxDescriptors]; |
There was a problem hiding this comment.
declaring them like this might keep them alive via shared ownership, an it might hide lifetime tracking issues.
There was a problem hiding this comment.
alternative is to keep a container<pair<binding,offset>> of everything you haven't freed (stil occcupant) where the container can be a vector or a set
then you draw a random number of elements from that to multi_deallocate
| indices.reserve(elementCount); | ||
| for (uint32_t i = 0; i < elementCount; i++) | ||
| indices.push_back(i); | ||
|
|
||
| bool response = writeDescriptors(elementCount, indices.data(), values.data()); | ||
| assert(response); |
There was a problem hiding this comment.
I'd rather actually create the resources here and rely on the set to keep them alive
There was a problem hiding this comment.
this function does create them
There was a problem hiding this comment.
this function does create them
yes I know, what I mean I'd want you to create them there, because right now you have the m_descriptor.. keeping them alive
| // COMMAND RECORDING | ||
| // Here we would hipothetically use the descriptors created above |
There was a problem hiding this comment.
an idea, actually bind the descriptor set, launch an empty compute dispatch of a single workgroup that doesn't do anything
might find some bugs in IGPUCommandBuffer and friends
|
|
||
| const ISemaphore::SWaitInfo futureWait = {m_timeline.get(),m_iteration}; | ||
| m_poolCache->releasePool(futureWait,poolIx); | ||
| m_subAllocDescriptorSet->multi_deallocate(AllocatedBinding, elementCount, values.data(), futureWait); |
There was a problem hiding this comment.
don't deallocate everything you allocated every frame
Adds an example for sub-allocated descriptor sets.