From 85ec207086ac0dbe2a51deb22fa9c328cfbc7875 Mon Sep 17 00:00:00 2001 From: kj16609 Date: Tue, 2 Jul 2024 02:01:28 -0400 Subject: [PATCH] Docs update and cleanup --- Doxyfile | 7 +- src/engine/assets/AssetManager.hpp | 117 ++++++++++----------- src/engine/assets/TransferManager.cpp | 5 +- src/engine/assets/TransferManager.hpp | 54 ++++++---- src/engine/buffers/vector/BufferVector.cpp | 2 +- src/engine/buffers/vector/DeviceVector.hpp | 2 +- src/engine/gui/core.cpp | 2 - src/engine/image/ImageView.cpp | 2 +- src/engine/image/ImageView.hpp | 2 +- src/engine/image/Sampler.cpp | 20 ++-- src/engine/image/Sampler.hpp | 15 ++- src/engine/rendering/SwapChain.cpp | 2 - src/engine/systems/GuiSystem.cpp | 2 - src/engine/texture/Texture.cpp | 25 +++-- src/engine/texture/Texture.hpp | 16 ++- 15 files changed, 155 insertions(+), 118 deletions(-) diff --git a/Doxyfile b/Doxyfile index c6c0f02..05ba231 100644 --- a/Doxyfile +++ b/Doxyfile @@ -68,7 +68,7 @@ PROJECT_LOGO = ./docs/libfgl_custom/TitorV2.png # entered, it will be relative to the location where doxygen was started. If # left blank the current directory will be used. -OUTPUT_DIRECTORY = ./docs +OUTPUT_DIRECTORY = ./build/docs # If the CREATE_SUBDIRS tag is set to YES then doxygen will create up to 4096 # sub-directories (in 2 levels) under the output directory of each output format @@ -945,7 +945,6 @@ WARN_LOGFILE = INPUT = ./src/engine \ ./src/core \ - ./src/shaders \ ./README.md # This tag can be used to specify the character encoding of the source files @@ -2354,7 +2353,7 @@ ENABLE_PREPROCESSING = YES # The default value is: NO. # This tag requires that the tag ENABLE_PREPROCESSING is set to YES. -MACRO_EXPANSION = NO +MACRO_EXPANSION = YES # If the EXPAND_ONLY_PREDEF and MACRO_EXPANSION tags are both set to YES then # the macro expansion is limited to the macros specified with the PREDEFINED and @@ -2479,7 +2478,7 @@ HIDE_UNDOC_RELATIONS = YES # set to NO # The default value is: NO. -HAVE_DOT = NO +HAVE_DOT = YES # The DOT_NUM_THREADS specifies the number of dot invocations doxygen is allowed # to run in parallel. When set to 0 doxygen will base this on the number of diff --git a/src/engine/assets/AssetManager.hpp b/src/engine/assets/AssetManager.hpp index 470c967..084e85e 100644 --- a/src/engine/assets/AssetManager.hpp +++ b/src/engine/assets/AssetManager.hpp @@ -5,12 +5,9 @@ #pragma once #include -#include -#include -#include #include -#include +#include namespace fgl::engine { @@ -18,6 +15,10 @@ namespace fgl::engine template < typename T > class AssetStore; + /** + * @brief Interface class for Assets. Allows for usage in AssetStore + * @tparam T + */ template < typename T > struct AssetInterface { @@ -27,82 +28,74 @@ namespace fgl::engine virtual ~AssetInterface() = default; }; + //! Concept for ensuring that the args given can extract a key + /** + * @anchor CanExtractKeyAnchor + */ + template < typename T, typename... TArgs > + concept can_extract_key = requires( TArgs&&... args ) { + { + T::extractKey( args... ) + } -> std::same_as< typename T::UIDKeyT >; + }; + + /** + * @brief Object store to keep track of assets. + * @tparam T Type to store T must define a `extractKey` method in T with the same parameters as the construction + * @copybrief + * T must: + * - Define UIDKeyT type inside of T. + * - Define and implement extractKey functions for each constructor used in load. (See can_extract_key) + */ template < typename T > class AssetStore { static_assert( - std::is_base_of_v< AssetInterface< T >, T >, "AssetStore: T must inherit from AssetInterface" ); + std::is_base_of_v< AssetInterface< T >, T >, "AssetStore: T must inherit from AssetInterface" ); - //! Items that are actively in use. - //std::unordered_map< TKey, std::weak_ptr< T > > active_map {}; + //! Key type given by T + using KeyT = typename T::UIDKeyT; - //! Assets needing to be staged - //TODO: ASYNC Ring buffer queue - std::queue< std::shared_ptr< T > > to_stage {}; - //TODO: Add tracy monitor to mutex - std::mutex queue_mtx {}; - - //! Assets currently being staged. - std::vector< std::shared_ptr< T > > processing {}; + std::unordered_map< KeyT, std::weak_ptr< T > > active_map {}; + std::mutex map_mtx; public: - //TODO: Come up with a better design the the loading function. - /// We should have a way to prevent a asset from being loaded multiple times. + /** + * @brief Extracts the key supplied by the arguments (can_extract_key). Locates object in the map, If not in the map it is constructed with the arguments. + * @tparam T_Args Types to construct with + * @param args given args to construct T with + * @returns shared pointer to T + */ template < typename... T_Args > - //TODO: This genuinely seems like a GCC Bug. Perhaps try to find a workaround later or report as such. - //requires std::constructible_from< T, T_Args... > + requires can_extract_key< T, T_Args... > std::shared_ptr< T > load( T_Args&&... args ) { ZoneScoped; - std::lock_guard guard { queue_mtx }; + const auto key { T::extractKey( args... ) }; - T* ptr { new T( std::forward< T_Args >( args )... ) }; - std::shared_ptr< T > s_ptr { ptr }; + std::lock_guard guard { map_mtx }; - to_stage.push( s_ptr ); + if ( auto itter = active_map.find( key ); itter != active_map.end() ) + { + // We've found the item in the map. We can now check if it's still active + + if ( std::weak_ptr< T >& item = itter->second; !item.expired() ) + { + return item.lock(); + } + + //Item was expired. Remove it from the map and continue + active_map.erase( itter ); + } + + std::shared_ptr< T > s_ptr { new T( std::forward< T_Args >( args )... ) }; + + // Add the weak pointer to the map so we can find it later. + active_map.insert( std::make_pair( key, s_ptr ) ); return s_ptr; } - - //! Returns true if all items to be staged were submitted to the queue - //! Returns false if more items remain - bool stage( vk::raii::CommandBuffer& buffer ) - { - ZoneScoped; - std::lock_guard guard { queue_mtx }; - //! Number of items to process during a stage step - constexpr std::size_t max_count { 16 }; - - for ( std::size_t i = 0; i < max_count; ++i ) - { - if ( to_stage.empty() ) break; - - processing.emplace_back( to_stage.front() ); - to_stage.pop(); - } - - if ( processing.size() == 0 ) return true; - - for ( const auto& ptr : processing ) - { - ptr->stage( buffer ); - } - - return to_stage.empty(); - } - - void confirmStaged() - { - for ( const auto& ptr : processing ) - { - ptr->dropStaging(); - ptr->setReady(); - } - - //TODO: Map this into a weak ptr in order to prevent duplication. - processing.clear(); - } }; } // namespace fgl::engine diff --git a/src/engine/assets/TransferManager.cpp b/src/engine/assets/TransferManager.cpp index 3347458..d6c87a6 100644 --- a/src/engine/assets/TransferManager.cpp +++ b/src/engine/assets/TransferManager.cpp @@ -233,9 +233,6 @@ namespace fgl::engine copy_regions.clear(); } - void TransferManager::prepareStaging() - {} - void TransferManager::createInstance( Device& device, std::uint64_t buffer_size ) { log::info( @@ -250,7 +247,7 @@ namespace fgl::engine return *global_transfer_manager; } - void TransferManager::copyToBuffer( BufferVector& source, BufferVector& target ) + void TransferManager::copyToVector( BufferVector& source, BufferVector& target ) { TransferData transfer_data { source.getHandle(), target.getHandle() }; diff --git a/src/engine/assets/TransferManager.hpp b/src/engine/assets/TransferManager.hpp index ff79c79..13816ef 100644 --- a/src/engine/assets/TransferManager.hpp +++ b/src/engine/assets/TransferManager.hpp @@ -27,7 +27,7 @@ namespace fgl::engine class Image; class BufferSuballocation; - // + //! using CopyRegionKey = std::pair< vk::Buffer, vk::Buffer >; struct BufferHasher @@ -53,8 +53,10 @@ namespace fgl::engine using CopyRegionMap = std::unordered_map< CopyRegionKey, std::vector< vk::BufferCopy >, CopyRegionKeyHasher >; + //! Data store for staging operations class TransferData { + //! Type of transfer this data represents enum TransferType { IMAGE_FROM_RAW, @@ -70,12 +72,21 @@ namespace fgl::engine using SourceData = std::variant< RawData, TransferBufferHandle, TransferImageHandle >; using TargetData = std::variant< TransferBufferHandle, TransferImageHandle >; + //! Source data. Data type depends on m_type SourceData m_source; + + //! Target data. Data type depends on m_type TargetData m_target; + //! Performs copy of raw data to the staging buffer + bool convertRawToBuffer( Buffer& staging_buffer ); + bool performImageStage( vk::raii::CommandBuffer& cmd_buffer, std::uint32_t transfer_idx, std::uint32_t graphics_idx ); + //! Same as @ref performImageStage Performs extra step of copying data to a staging buffer + /** @note After calling this function m_type will be `IMAGE_FROM_BUFFER` + */ bool performRawImageStage( vk::raii::CommandBuffer& buffer, Buffer& staging_buffer, @@ -84,10 +95,11 @@ namespace fgl::engine bool performBufferStage( CopyRegionMap& copy_regions ); + //! Same as @ref performBufferStage Performs extra step of copying data to a staging buffer + /** @note After calling this function m_type will be `BUFFER_FROM_BUFFER` + */ bool performRawBufferStage( Buffer& staging_buffer, CopyRegionMap& copy_regions ); - bool convertRawToBuffer( Buffer& ); - friend class TransferManager; public: @@ -100,6 +112,7 @@ namespace fgl::engine TransferData( TransferData&& other ) = default; TransferData& operator=( TransferData&& ) = default; + //! BUFFER_FROM_BUFFER TransferData( const std::shared_ptr< BufferSuballocationHandle >& source, const std::shared_ptr< BufferSuballocationHandle >& target ) : @@ -113,6 +126,7 @@ namespace fgl::engine markBad(); } + //! BUFFER_FROM_RAW TransferData( std::vector< std::byte >&& source, const std::shared_ptr< BufferSuballocationHandle >& target ) : m_type( BUFFER_FROM_RAW ), m_source( std::forward< std::vector< std::byte > >( source ) ), @@ -125,6 +139,7 @@ namespace fgl::engine markBad(); } + //! IMAGE_FROM_BUFFER TransferData( const std::shared_ptr< BufferSuballocationHandle >& source, const std::shared_ptr< ImageHandle >& target ) : m_type( IMAGE_FROM_BUFFER ), @@ -137,6 +152,7 @@ namespace fgl::engine markBad(); } + //! IMAGE_FROM_RAW TransferData( std::vector< std::byte >&& source, const std::shared_ptr< ImageHandle >& target ) : m_type( IMAGE_FROM_RAW ), m_source( std::forward< std::vector< std::byte > >( source ) ), @@ -163,15 +179,16 @@ namespace fgl::engine void markGood(); }; + //! Manages transfers from HOST (CPU) to DEVICE (GPU) class TransferManager { //TODO: Ring Buffer + //! Queue of data needing to be transfered and submitted. std::queue< TransferData > queue {}; + //! Data actively in flight (Submitted to the DEVICE transfer queue) std::vector< TransferData > processing {}; - // std::thread transfer_thread; - //! Buffer used for any raw -> buffer transfers std::unique_ptr< Buffer > staging_buffer {}; @@ -215,16 +232,15 @@ namespace fgl::engine vk::raii::Semaphore& getFinishedSem() { return transfer_semaphore; } + //! Takes ownership of memory regions from the graphics queue via memory barriers. void takeOwnership( vk::raii::CommandBuffer& buffer ); + //! Records the barriers required for transfering queue ownership void recordOwnershipTransferDst( vk::raii::CommandBuffer& command_buffer ); //! Drops the processed items void dump(); - //! Prepares the staging buffer. Filling it as much as possible - void prepareStaging(); - static void createInstance( Device& device, std::uint64_t buffer_size ); static TransferManager& getInstance(); @@ -232,6 +248,7 @@ namespace fgl::engine FGL_DELETE_ALL_Ro5( TransferManager ); + //! Resizes the staging buffer. void resizeBuffer( const std::uint64_t size ) { staging_buffer = std::make_unique< Buffer >( @@ -240,20 +257,21 @@ namespace fgl::engine vk::MemoryPropertyFlagBits::eHostVisible | vk::MemoryPropertyFlagBits::eHostCoherent ); } - //! Queues an buffer to be transfered - template < typename BufferT > - requires is_device_vector< BufferT > - void copyToBuffer( std::vector< std::byte >&& data, BufferT& buffer ) + //! Queues a buffer to be transfered + template < typename DeviceVectorT > + requires is_device_vector< DeviceVectorT > + void copyToVector( std::vector< std::byte >&& data, DeviceVectorT& device_vector ) { assert( data.size() > 0 ); - TransferData transfer_data { std::forward< std::vector< std::byte > >( data ), buffer.m_handle }; + TransferData transfer_data { std::forward< std::vector< std::byte > >( data ), device_vector.m_handle }; queue.emplace( std::move( transfer_data ) ); } - template < typename T, typename BufferT > - requires is_device_vector< BufferT > - void copyToBuffer( const std::vector< T >& data, BufferT& buffer ) + //! Queues a data copy from a STL vector to a device vector + template < typename T, typename DeviceVectorT > + requires is_device_vector< DeviceVectorT > + void copyToVector( const std::vector< T >& data, DeviceVectorT& device_vector ) { assert( data.size() > 0 ); std::vector< std::byte > punned_data {}; @@ -261,10 +279,10 @@ namespace fgl::engine std::memcpy( punned_data.data(), data.data(), sizeof( T ) * data.size() ); - copyToBuffer( std::move( punned_data ), buffer ); + copyToVector( std::move( punned_data ), device_vector ); } - void copyToBuffer( BufferVector& source, BufferVector& target ); + void copyToVector( BufferVector& source, BufferVector& target ); void copyToImage( std::vector< std::byte >&& data, Image& image ); diff --git a/src/engine/buffers/vector/BufferVector.cpp b/src/engine/buffers/vector/BufferVector.cpp index 4eb9011..0bb3b90 100644 --- a/src/engine/buffers/vector/BufferVector.cpp +++ b/src/engine/buffers/vector/BufferVector.cpp @@ -46,7 +46,7 @@ namespace fgl::engine BufferVector other { this->getBuffer(), count, m_stride }; - TransferManager::getInstance().copyToBuffer( *this, other ); + TransferManager::getInstance().copyToVector( *this, other ); *this = std::move( other ); } diff --git a/src/engine/buffers/vector/DeviceVector.hpp b/src/engine/buffers/vector/DeviceVector.hpp index 256ad1e..3498ad4 100644 --- a/src/engine/buffers/vector/DeviceVector.hpp +++ b/src/engine/buffers/vector/DeviceVector.hpp @@ -32,7 +32,7 @@ namespace fgl::engine DeviceVector( Buffer& buffer, const std::vector< T >& data ) : DeviceVector( buffer, static_cast< std::uint32_t >( data.size() ) ) { - TransferManager::getInstance().copyToBuffer( data, *this ); + TransferManager::getInstance().copyToVector( data, *this ); } }; diff --git a/src/engine/gui/core.cpp b/src/engine/gui/core.cpp index 3893edd..e4efa3d 100644 --- a/src/engine/gui/core.cpp +++ b/src/engine/gui/core.cpp @@ -10,8 +10,6 @@ #pragma GCC diagnostic ignored "-Wconversion" #include #include -#include - #include #pragma GCC diagnostic pop diff --git a/src/engine/image/ImageView.cpp b/src/engine/image/ImageView.cpp index af43ba7..f793cb5 100644 --- a/src/engine/image/ImageView.cpp +++ b/src/engine/image/ImageView.cpp @@ -62,7 +62,7 @@ namespace fgl::engine return m_resource->extent(); } - void ImageView::setName( const std::string str ) + void ImageView::setName( const std::string& str ) { m_resource->setName( str ); } diff --git a/src/engine/image/ImageView.hpp b/src/engine/image/ImageView.hpp index a0e2c54..8912b9d 100644 --- a/src/engine/image/ImageView.hpp +++ b/src/engine/image/ImageView.hpp @@ -24,7 +24,7 @@ namespace fgl::engine public: - void setName( const std::string str ); + void setName( const std::string& str ); //! Returns true if the resource has been staged bool ready(); diff --git a/src/engine/image/Sampler.cpp b/src/engine/image/Sampler.cpp index d6d8562..66c813c 100644 --- a/src/engine/image/Sampler.cpp +++ b/src/engine/image/Sampler.cpp @@ -45,7 +45,6 @@ namespace fgl::engine const vk::SamplerAddressMode sampler_wrap_u, const vk::SamplerAddressMode sampler_wrap_v, const vk::SamplerAddressMode sampler_wrap_w ) : - valid( true ), m_sampler( createSampler( mag_filter, min_filter, mipmap_mode, sampler_wrap_u, sampler_wrap_v, sampler_wrap_w ) ) {} @@ -105,17 +104,26 @@ namespace fgl::engine gl::wrappingToVk( wrapt ) ) {} - Sampler::Sampler( Sampler&& other ) : valid( other.valid ), m_sampler( std::move( other.m_sampler ) ) + Sampler::Sampler( Sampler&& other ) : m_sampler( std::move( other.m_sampler ) ) { - other.valid = false; + other.m_sampler = VK_NULL_HANDLE; } Sampler& Sampler::operator=( Sampler&& other ) { - this->valid = other.valid; - other.valid = false; - if ( valid ) m_sampler = std::move( other.m_sampler ); + m_sampler = std::move( other.m_sampler ); + other.m_sampler = VK_NULL_HANDLE; return *this; } + void Sampler::setName( const std::string str ) + { + vk::DebugUtilsObjectNameInfoEXT info {}; + info.objectType = vk::ObjectType::eSampler; + info.pObjectName = str.c_str(); + info.setObjectHandle( reinterpret_cast< std::uint64_t >( static_cast< VkSampler >( *m_sampler ) ) ); + + Device::getInstance().setDebugUtilsObjectName( info ); + } + } // namespace fgl::engine diff --git a/src/engine/image/Sampler.hpp b/src/engine/image/Sampler.hpp index 44d3c4e..bd60eb1 100644 --- a/src/engine/image/Sampler.hpp +++ b/src/engine/image/Sampler.hpp @@ -8,17 +8,19 @@ #include #include "engine/FGL_DEFINES.hpp" +#include "engine/logging/logging.hpp" namespace fgl::engine { class Sampler { - bool valid; - vk::raii::Sampler m_sampler; + vk::raii::Sampler m_sampler { VK_NULL_HANDLE }; public: + FGL_DELETE_COPY( Sampler ) + Sampler() : Sampler( vk::Filter::eLinear, @@ -56,12 +58,17 @@ namespace fgl::engine VkSampler operator*() { return *m_sampler; } - Sampler( const Sampler& ) = delete; - Sampler& operator=( const Sampler& ) = delete; Sampler( Sampler&& other ); Sampler& operator=( Sampler&& ); + ~Sampler() + { + if ( *m_sampler != VK_NULL_HANDLE ) log::debug( "Sampler destroyed" ); + } + vk::raii::Sampler& getVkSampler() { return m_sampler; } + + void setName( const std::string str ); }; } // namespace fgl::engine \ No newline at end of file diff --git a/src/engine/rendering/SwapChain.cpp b/src/engine/rendering/SwapChain.cpp index 8711127..c2c9f0b 100644 --- a/src/engine/rendering/SwapChain.cpp +++ b/src/engine/rendering/SwapChain.cpp @@ -197,8 +197,6 @@ namespace fgl::engine auto& image { m_swap_chain_images[ i ] }; image.setName( "SwapChainImage: " + std::to_string( i ) ); - - auto texture { std::make_unique< Texture >( image ) }; } colorAttachment.linkImages( m_swap_chain_images ); diff --git a/src/engine/systems/GuiSystem.cpp b/src/engine/systems/GuiSystem.cpp index 3019bb8..42d9399 100644 --- a/src/engine/systems/GuiSystem.cpp +++ b/src/engine/systems/GuiSystem.cpp @@ -5,9 +5,7 @@ #include "GuiSystem.hpp" #include "engine/FrameInfo.hpp" -#include "engine/debug_defines.hpp" #include "engine/gui/core.hpp" -#include "engine/rendering/Device.hpp" namespace fgl::engine { diff --git a/src/engine/texture/Texture.cpp b/src/engine/texture/Texture.cpp index 243a5b4..9099f7c 100644 --- a/src/engine/texture/Texture.cpp +++ b/src/engine/texture/Texture.cpp @@ -89,8 +89,6 @@ namespace fgl::engine extent = getExtent(); } - const ImVec2 imgui_size { static_cast< float >( extent.width ), static_cast< float >( extent.height ) }; - if ( !ready() ) { //TODO: Render placeholder @@ -98,6 +96,10 @@ namespace fgl::engine return ImGui::Button( "No texture :(" ); } + assert( *m_image_view->getSampler() != VK_NULL_HANDLE ); + + const ImVec2 imgui_size { static_cast< float >( extent.width ), static_cast< float >( extent.height ) }; + return ImGui::ImageButton( static_cast< ImTextureID >( getImGuiDescriptorSet() ), imgui_size ); } @@ -130,19 +132,18 @@ namespace fgl::engine Texture::Texture( const std::filesystem::path& path, const vk::Format format ) : Texture( loadTexture( path, format ) ) - {} + { + setName( path.filename() ); + } Texture::Texture( const std::filesystem::path& path ) : Texture( loadTexture( path ) ) - {} + { + setName( path.filename() ); + } Texture::~Texture() { - //TODO: Implement deffered destruction if ( m_imgui_set != VK_NULL_HANDLE ) ImGui_ImplVulkan_RemoveTexture( m_imgui_set ); - if ( m_image_view.use_count() == 1 ) - { - log::info( "Destroying texture {}", getID() ); - } } vk::DescriptorImageInfo Texture::getDescriptor() const @@ -204,6 +205,7 @@ namespace fgl::engine bool Texture::ready() const { + assert( m_image_view ); return this->m_image_view->ready(); } @@ -212,6 +214,11 @@ namespace fgl::engine return m_texture_id; } + void Texture::setName( const std::string& str ) + { + this->getImageView().setName( str ); + } + DescriptorSet& Texture::getTextureDescriptorSet() { static std::unique_ptr< DescriptorSet > set { nullptr }; diff --git a/src/engine/texture/Texture.hpp b/src/engine/texture/Texture.hpp index ffeb712..8c98c7d 100644 --- a/src/engine/texture/Texture.hpp +++ b/src/engine/texture/Texture.hpp @@ -35,28 +35,41 @@ namespace fgl::engine friend class TransferManager; + //! Key used for the global map keeping track of Textures + using UIDKeyT = std::filesystem::path; + //TODO: Implement reusing texture ids TextureID m_texture_id; - std::shared_ptr< ImageView > m_image_view {}; + std::shared_ptr< ImageView > m_image_view; vk::Extent2D m_extent; + //! Descriptor set used for displaying the texture in ImGui vk::DescriptorSet m_imgui_set { VK_NULL_HANDLE }; [[nodiscard]] Texture( std::tuple< std::vector< std::byte >, int, int, vk::Format > ); + //! Construct texture with a specific extent and data [[nodiscard]] Texture( std::vector< std::byte >&& data, const int x, const int y, const vk::Format texture_format ); + //! Construct texture with a specific extent and data [[nodiscard]] Texture( std::vector< std::byte >&& data, const vk::Extent2D extent, const vk::Format texture_format ); + //! Construct with a specific format [[nodiscard]] Texture( const std::filesystem::path& path, const vk::Format format ); + + //! Construct with no format [[nodiscard]] Texture( const std::filesystem::path& path ); public: + inline static UIDKeyT extractKey( const std::filesystem::path& path ) { return path; } + + inline static UIDKeyT extractKey( const std::filesystem::path& path, const vk::Format format ) { return path; } + Texture() = delete; ~Texture(); @@ -72,6 +85,7 @@ namespace fgl::engine bool ready() const; [[nodiscard]] TextureID getID() const; + void setName( const std::string& str ); [[nodiscard]] vk::DescriptorImageInfo getDescriptor() const; [[nodiscard]] vk::DescriptorSet& getImGuiDescriptorSet();