From c0ec82fac15f2e3c9339b69d172dc75da7a31c70 Mon Sep 17 00:00:00 2001 From: kj16609 Date: Mon, 7 Apr 2025 16:00:23 -0400 Subject: [PATCH] Some bugfixing cleanup --- src/engine/assets/image/Sampler.cpp | 4 -- src/engine/assets/material/Material.cpp | 5 +- src/engine/camera/Camera.cpp | 12 ++++- src/engine/camera/Camera.hpp | 3 +- src/engine/descriptors/DescriptorSet.cpp | 46 ++++++++----------- src/engine/descriptors/DescriptorSet.hpp | 17 ++++--- .../descriptors/DescriptorSetLayout.cpp | 2 +- .../memory/buffers/BufferSuballocation.cpp | 27 ++++++----- .../buffers/UniqueFrameSuballocation.hpp | 8 ++-- src/engine/rendering/devices/Device.cpp | 10 ++++ src/shaders/culling.slang | 2 +- src/shaders/textured.slang | 2 +- 12 files changed, 74 insertions(+), 64 deletions(-) diff --git a/src/engine/assets/image/Sampler.cpp b/src/engine/assets/image/Sampler.cpp index 864a634..3708808 100644 --- a/src/engine/assets/image/Sampler.cpp +++ b/src/engine/assets/image/Sampler.cpp @@ -82,19 +82,15 @@ namespace fgl::engine default: throw std::runtime_error( "Failed to translate wrapping filter to vk address mode" ); case GL_REPEAT: - log::info( "eRepeat" ); return vk::SamplerAddressMode::eRepeat; case GL_MIRRORED_REPEAT: - log::info( "eMirroredRepeat" ); return vk::SamplerAddressMode::eMirroredRepeat; #ifdef GL_CLAMP_TO_BORDER case GL_CLAMP_TO_BORDER: - log::info( "eClampToBorder" ); return vk::SamplerAddressMode::eClampToBorder; #endif #ifdef GL_CLAMP_TO_EDGE case GL_CLAMP_TO_EDGE: - log::info( "eClampToEdge" ); return vk::SamplerAddressMode::eClampToEdge; #endif } diff --git a/src/engine/assets/material/Material.cpp b/src/engine/assets/material/Material.cpp index 69e2806..669a679 100644 --- a/src/engine/assets/material/Material.cpp +++ b/src/engine/assets/material/Material.cpp @@ -54,12 +54,13 @@ namespace fgl::engine Material::Material() : m_id( material_id_counter.getID() ) { - getDescriptorSet().bindArray( + auto& descriptor_set { getDescriptorSet() }; + descriptor_set.bindArray( 0, EngineContext::getInstance().getMaterialManager().getBufferSuballocation(), m_id, sizeof( DeviceMaterialData ) ); - getDescriptorSet().update(); + descriptor_set.update(); } bool Material::ready() const diff --git a/src/engine/camera/Camera.cpp b/src/engine/camera/Camera.cpp index eae18e1..2b70c82 100644 --- a/src/engine/camera/Camera.cpp +++ b/src/engine/camera/Camera.cpp @@ -345,11 +345,18 @@ namespace fgl::engine m_composite_swapchain( std::make_unique< CompositeSwapchain >( m_target_extent ) ), m_gbuffer_swapchain( std::make_unique< GBufferSwapchain >( m_target_extent ) ), m_camera_renderer( renderer ), - m_camera_frame_info( buffer ) + m_camera_frame_info( buffer ), + m_camera_info_descriptors( createCameraDescriptors() ) { FGL_ASSERT( renderer, "Camera renderer is null" ); this->setPerspectiveProjection( m_fov_y, aspectRatio(), constants::NEAR_PLANE, constants::FAR_PLANE ); this->setView( WorldCoordinate( constants::CENTER ), QuatRotation( 0.0f, 0.0f, 0.0f ) ); + } + + std::vector< std::unique_ptr< descriptors::DescriptorSet > > Camera::createCameraDescriptors() + { + std::vector< std::unique_ptr< descriptors::DescriptorSet > > sets {}; + sets.reserve( constants::MAX_FRAMES_IN_FLIGHT ); for ( std::uint8_t i = 0; i < constants::MAX_FRAMES_IN_FLIGHT; ++i ) { @@ -358,8 +365,9 @@ namespace fgl::engine set->update(); set->setName( std::format( "Camera {} descriptor set {}", m_camera_idx, i ) ); - m_camera_info_descriptors.emplace_back( std::move( set ) ); + sets.emplace_back( std::move( set ) ); } + return sets; } void Camera::setExtent( const vk::Extent2D extent ) diff --git a/src/engine/camera/Camera.hpp b/src/engine/camera/Camera.hpp index 089de5a..6cae17c 100644 --- a/src/engine/camera/Camera.hpp +++ b/src/engine/camera/Camera.hpp @@ -91,7 +91,8 @@ namespace fgl::engine PerFrameSuballocation< HostSingleT< CameraInfo > > m_camera_frame_info; // Camera info is expected at binding 0 - std::vector< std::unique_ptr< descriptors::DescriptorSet > > m_camera_info_descriptors {}; + std::vector< std::unique_ptr< descriptors::DescriptorSet > > createCameraDescriptors(); + std::vector< std::unique_ptr< descriptors::DescriptorSet > > m_camera_info_descriptors; std::string m_name { "Unnamed Camera" }; diff --git a/src/engine/descriptors/DescriptorSet.cpp b/src/engine/descriptors/DescriptorSet.cpp index 43e4f7d..2be00c9 100644 --- a/src/engine/descriptors/DescriptorSet.cpp +++ b/src/engine/descriptors/DescriptorSet.cpp @@ -26,32 +26,6 @@ namespace fgl::engine::descriptors m_infos.resize( m_binding_count ); } - DescriptorSet::DescriptorSet( DescriptorSet&& other ) noexcept : - m_set_idx( other.m_set_idx ), - m_infos( std::move( other.m_infos ) ), - m_descriptor_writes( std::move( other.m_descriptor_writes ) ), - m_resources( std::move( other.m_resources ) ), - m_set( std::move( other.m_set ) ), - m_binding_count( other.m_binding_count ) - { - other.m_set = VK_NULL_HANDLE; - other.m_binding_count = 0; - } - - DescriptorSet& DescriptorSet::operator=( DescriptorSet&& other ) noexcept - { - m_set_idx = other.m_set_idx; - m_infos = std::move( other.m_infos ); - m_descriptor_writes = std::move( other.m_descriptor_writes ); - m_resources = std::move( other.m_resources ); - m_set = std::move( other.m_set ); - other.m_set = VK_NULL_HANDLE; - m_binding_count = other.m_binding_count; - other.m_binding_count = 0; - - return *this; - } - void DescriptorSet::bindUniformBuffer( const std::uint32_t binding_idx, const memory::BufferSuballocation& buffer ) { assert( binding_idx < m_infos.size() && "Binding index out of range" ); @@ -168,9 +142,22 @@ namespace fgl::engine::descriptors void DescriptorSet::update() { Device::getInstance().device().updateDescriptorSets( m_descriptor_writes, {} ); + m_initalized = true; resetUpdate(); } + VkDescriptorSet DescriptorSet::operator*() const + { + return getVkDescriptorSet(); + } + + VkDescriptorSet DescriptorSet::getVkDescriptorSet() const + { + FGL_ASSERT( !hasUpdates(), "Descriptor set has updates but binding was attempted" ); + FGL_ASSERT( m_initalized, "Descriptor set has not been initialized" ); + return *m_set; + } + DescriptorSet::~DescriptorSet() {} @@ -183,6 +170,11 @@ namespace fgl::engine::descriptors m_infos.resize( m_binding_count ); } + bool DescriptorSet::hasUpdates() const + { + return !m_descriptor_writes.empty(); + } + void DescriptorSet:: bindAttachment( const std::uint32_t binding_idx, const ImageView& view, const vk::ImageLayout layout ) { @@ -209,7 +201,7 @@ namespace fgl::engine::descriptors vk::DebugUtilsObjectNameInfoEXT info {}; info.objectType = vk::ObjectType::eDescriptorSet; info.pObjectName = str.c_str(); - info.setObjectHandle( reinterpret_cast< std::uint64_t >( getVkDescriptorSet() ) ); + info.setObjectHandle( reinterpret_cast< std::uint64_t >( static_cast< VkDescriptorSet >( *m_set ) ) ); Device::getInstance().setDebugUtilsObjectName( info ); } diff --git a/src/engine/descriptors/DescriptorSet.hpp b/src/engine/descriptors/DescriptorSet.hpp index df96029..7ebb6a3 100644 --- a/src/engine/descriptors/DescriptorSet.hpp +++ b/src/engine/descriptors/DescriptorSet.hpp @@ -33,6 +33,7 @@ namespace fgl::engine::descriptors //TODO: Maybe redo this to not be a monostate variant? std::vector< std::variant< std::monostate, vk::DescriptorImageInfo, vk::DescriptorBufferInfo > > m_infos; std::vector< vk::WriteDescriptorSet > m_descriptor_writes; + bool m_initalized { false }; using Resource = std::variant< std::shared_ptr< ImageView >, std::shared_ptr< memory::BufferSuballocation > >; @@ -48,18 +49,14 @@ namespace fgl::engine::descriptors public: - [[nodiscard]] bool hasUpdates() const { return !m_descriptor_writes.empty(); } + [[nodiscard]] bool hasUpdates() const; //! Updates the descriptor set, updates all pending writes created by using bindImage(), bindUniformBuffer(), bindArray(), bindAttachment(), or bindTexture(). void update(); - VkDescriptorSet operator*() const - { - FGL_ASSERT( !hasUpdates(), "Descriptor set has updates but binding was attempted" ); - return *m_set; - } + [[nodiscard]] VkDescriptorSet operator*() const; - [[nodiscard]] VkDescriptorSet getVkDescriptorSet() const { return *m_set; } + [[nodiscard]] VkDescriptorSet getVkDescriptorSet() const; [[nodiscard]] DescriptorIDX setIDX() const { return m_set_idx; } @@ -69,9 +66,11 @@ namespace fgl::engine::descriptors FGL_DELETE_COPY( DescriptorSet ); + FGL_DELETE_MOVE( DescriptorSet ); + //Move - DescriptorSet( DescriptorSet&& other ) noexcept; - DescriptorSet& operator=( DescriptorSet&& other ) noexcept; + // DescriptorSet( DescriptorSet&& other ) noexcept; + // DescriptorSet& operator=( DescriptorSet&& other ) noexcept; ~DescriptorSet(); diff --git a/src/engine/descriptors/DescriptorSetLayout.cpp b/src/engine/descriptors/DescriptorSetLayout.cpp index f61ac40..3bb6193 100644 --- a/src/engine/descriptors/DescriptorSetLayout.cpp +++ b/src/engine/descriptors/DescriptorSetLayout.cpp @@ -15,7 +15,7 @@ namespace fgl::engine::descriptors m_set_idx( set_idx ), m_binding_count( descriptors.size() ) { - FGL_ASSERT( descriptors.size() > 0, "Must have more then 1 descriptor set" ); + FGL_ASSERT( !descriptors.empty(), "Must have more then 1 descriptor set" ); for ( const auto& descriptor_w : descriptors ) { diff --git a/src/engine/memory/buffers/BufferSuballocation.cpp b/src/engine/memory/buffers/BufferSuballocation.cpp index 02c6745..a1da8bd 100644 --- a/src/engine/memory/buffers/BufferSuballocation.cpp +++ b/src/engine/memory/buffers/BufferSuballocation.cpp @@ -13,6 +13,14 @@ namespace fgl::engine::memory { + BufferSuballocation::BufferSuballocation( std::shared_ptr< BufferSuballocationHandle > handle ) : + m_handle( std::move( handle ) ), + m_offset( m_handle->m_offset ), + m_byte_size( m_handle->m_size ) + { + if ( handle.use_count() > 30 ) throw std::runtime_error( "AAAAAAAAA" ); + } + BufferSuballocation& BufferSuballocation::operator=( BufferSuballocation&& other ) noexcept { m_handle = std::move( other.m_handle ); @@ -28,14 +36,6 @@ namespace fgl::engine::memory return *this; } - BufferSuballocation::BufferSuballocation( std::shared_ptr< BufferSuballocationHandle > handle ) : - m_handle( std::move( handle ) ), - m_offset( m_handle->m_offset ), - m_byte_size( m_handle->m_size ) - { - if ( handle.use_count() > 30 ) throw std::runtime_error( "AAAAAAAAA" ); - } - BufferSuballocation::BufferSuballocation( BufferSuballocation&& other ) noexcept : m_handle( std::move( other.m_handle ) ), m_offset( m_handle->m_offset ), @@ -100,12 +100,15 @@ namespace fgl::engine::memory assert( !std::isnan( m_offset ) ); assert( !std::isnan( m_byte_size ) ); - return vk::DescriptorBufferInfo( getVkBuffer(), m_offset + byte_offset, m_byte_size ); + FGL_ASSERT( byte_offset < m_byte_size, "Byte offset was greater then byte size!" ); + FGL_ASSERT( + m_offset + byte_offset < this->getBuffer().size(), + "Byte offset + buffer offset was greater then parent buffer size" ); + + return { getVkBuffer(), m_offset + byte_offset, m_byte_size }; } - BufferSuballocation::~BufferSuballocation() - { - } + BufferSuballocation::~BufferSuballocation() = default; SuballocationView BufferSuballocation::view( const vk::DeviceSize offset, const vk::DeviceSize size ) const { diff --git a/src/engine/memory/buffers/UniqueFrameSuballocation.hpp b/src/engine/memory/buffers/UniqueFrameSuballocation.hpp index c6994a7..3f2054e 100644 --- a/src/engine/memory/buffers/UniqueFrameSuballocation.hpp +++ b/src/engine/memory/buffers/UniqueFrameSuballocation.hpp @@ -18,11 +18,11 @@ namespace fgl::engine public: - PerFrameSuballocation( memory::Buffer& buffer ) : m_buffer( buffer ) + explicit PerFrameSuballocation( memory::Buffer& buffer ) : m_buffer( buffer ) { - constexpr auto frames_in_flight { constants::MAX_FRAMES_IN_FLIGHT }; - m_suballocations.reserve( frames_in_flight ); - for ( std::uint16_t i = 0; i < frames_in_flight; ++i ) + m_suballocations.reserve( constants::MAX_FRAMES_IN_FLIGHT ); + + for ( std::uint16_t i = 0; i < constants::MAX_FRAMES_IN_FLIGHT; ++i ) { static_assert( std::is_default_constructible_v< typename Suballocation::value_type >, diff --git a/src/engine/rendering/devices/Device.cpp b/src/engine/rendering/devices/Device.cpp index 9a02206..e43e1de 100644 --- a/src/engine/rendering/devices/Device.cpp +++ b/src/engine/rendering/devices/Device.cpp @@ -52,6 +52,13 @@ namespace fgl::engine throw std::runtime_error( "wideLines not supported by device" ); } +#ifndef NDEBUG + if ( available_features.robustBufferAccess != VK_TRUE ) + { + throw std::runtime_error( "robustBufferAccess not supported by device" ); + } +#endif + //Set enabled features vk::PhysicalDeviceFeatures deviceFeatures = {}; deviceFeatures.samplerAnisotropy = VK_TRUE; @@ -59,6 +66,9 @@ namespace fgl::engine deviceFeatures.tessellationShader = VK_TRUE; deviceFeatures.drawIndirectFirstInstance = VK_TRUE; deviceFeatures.wideLines = VK_TRUE; +#ifndef NDEBUG + deviceFeatures.robustBufferAccess = VK_TRUE; +#endif return deviceFeatures; } diff --git a/src/shaders/culling.slang b/src/shaders/culling.slang index 55f78b4..2242bcf 100644 --- a/src/shaders/culling.slang +++ b/src/shaders/culling.slang @@ -49,7 +49,7 @@ void computeMain( uint3 dispatch_id : SV_DispatchThreadID) // each instance of this represents a unique primitive to be rendered const uint instance_index = dispatch_id.x; // global thread - if ( instance_index > pc.draw_count ) return; + if (pc.draw_count == 0 || instance_index + 1 > pc.draw_count ) return; const PrimitiveInstanceInfo instance = primitive_instances[ instance_index ]; const PrimitiveRenderInfo primitive = primitives[ instance.render_info_id ]; diff --git a/src/shaders/textured.slang b/src/shaders/textured.slang index a543a3a..69ca7e9 100644 --- a/src/shaders/textured.slang +++ b/src/shaders/textured.slang @@ -14,7 +14,7 @@ struct CoarseVertex { } [ [ vk::binding( 0, 1 ) ] ] -ParameterBlock< CameraData > camera : CAMERA; +ConstantBuffer< CameraData > camera : CAMERA; [shader("vertex")] CoarseVertex vertexMain( ModelVertex in_vertex )