From 00d30bc712c521151ee68cdd5b3b172e64806477 Mon Sep 17 00:00:00 2001 From: spencer-lunarg Date: Mon, 25 Nov 2024 10:48:06 -0500 Subject: [PATCH 1/2] layers: Remove unused UpdateDrawStates args --- layers/state_tracker/cmd_buffer_state.cpp | 2 +- layers/state_tracker/descriptor_sets.cpp | 4 ++-- layers/state_tracker/descriptor_sets.h | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/layers/state_tracker/cmd_buffer_state.cpp b/layers/state_tracker/cmd_buffer_state.cpp index 5df408aa9c9..6f6e17f3eb4 100644 --- a/layers/state_tracker/cmd_buffer_state.cpp +++ b/layers/state_tracker/cmd_buffer_state.cpp @@ -1242,7 +1242,7 @@ void CommandBuffer::UpdatePipelineState(Func command, const VkPipelineBindPoint } // Bind this set and its active descriptor resources to the command buffer - descriptor_set->UpdateDrawState(&dev_data, this, command, pipe, set_binding_pair.second); + descriptor_set->UpdateDrawStates(&dev_data, this, set_binding_pair.second); set_info.validated_set = descriptor_set.get(); set_info.validated_set_change_count = descriptor_set->GetChangeCount(); diff --git a/layers/state_tracker/descriptor_sets.cpp b/layers/state_tracker/descriptor_sets.cpp index 8add97bfae1..42c538b1325 100644 --- a/layers/state_tracker/descriptor_sets.cpp +++ b/layers/state_tracker/descriptor_sets.cpp @@ -687,8 +687,8 @@ void vvl::DescriptorSet::PerformCopyUpdate(const VkCopyDescriptorSet &update, co // TODO: Modify the UpdateDrawState virtural functions to *only* set initial layout and not change layouts // Prereq: This should be called for a set that has been confirmed to be active for the given cb_state, meaning it's going // to be used in a draw by the given cb_state -void vvl::DescriptorSet::UpdateDrawState(ValidationStateTracker *device_data, vvl::CommandBuffer *cb_state, vvl::Func command, - const vvl::Pipeline *pipe, const BindingVariableMap &binding_req_map) { +void vvl::DescriptorSet::UpdateDrawStates(ValidationStateTracker *device_data, vvl::CommandBuffer *cb_state, + const BindingVariableMap &binding_req_map) { // Descriptor UpdateDrawState only call image layout validation callbacks. If it is disabled, skip the entire loop. if (device_data->disabled[image_layout_validation]) { return; diff --git a/layers/state_tracker/descriptor_sets.h b/layers/state_tracker/descriptor_sets.h index 3bd986d8a15..b7a4542502b 100644 --- a/layers/state_tracker/descriptor_sets.h +++ b/layers/state_tracker/descriptor_sets.h @@ -828,8 +828,7 @@ class DescriptorSet : public StateObject { VkDescriptorSet VkHandle() const { return handle_.Cast(); }; // Bind given cmd_buffer to this descriptor set and // update CB image layout map with image/imagesampler descriptor image layouts - void UpdateDrawState(ValidationStateTracker *, vvl::CommandBuffer *cb_state, vvl::Func command, const vvl::Pipeline *, - const BindingVariableMap &); + void UpdateDrawStates(ValidationStateTracker *, vvl::CommandBuffer *cb_state, const BindingVariableMap &); // For a particular binding, get the global index const IndexRange GetGlobalIndexRangeFromBinding(const uint32_t binding, bool actual_length = false) const { From 25d446781f22560c4bf3c9c06f214eceebf69822 Mon Sep 17 00:00:00 2001 From: spencer-lunarg Date: Mon, 25 Nov 2024 11:28:15 -0500 Subject: [PATCH 2/2] layers: Remove callback for image layout --- layers/core_checks/cc_device.cpp | 8 -------- layers/drawdispatch/descriptor_validator.cpp | 4 ++-- layers/gpu/core/gpuav_setup.cpp | 8 -------- layers/state_tracker/cmd_buffer_state.cpp | 2 +- layers/state_tracker/descriptor_sets.cpp | 10 +++++----- layers/state_tracker/descriptor_sets.h | 6 +++--- layers/state_tracker/state_tracker.h | 14 -------------- 7 files changed, 11 insertions(+), 41 deletions(-) diff --git a/layers/core_checks/cc_device.cpp b/layers/core_checks/cc_device.cpp index c71780228ef..42e63e1f999 100644 --- a/layers/core_checks/cc_device.cpp +++ b/layers/core_checks/cc_device.cpp @@ -343,14 +343,6 @@ void CoreChecks::PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const L // The state tracker sets up the device state (also if extension and/or features are enabled) StateTracker::PostCreateDevice(pCreateInfo, loc); - // Add the callback hooks for the functions that are either broadly or deeply used and that the ValidationStateTracker refactor - // would be messier without. - // TODO: Find a good way to do this hooklessly. - SetSetImageViewInitialLayoutCallback( - [](vvl::CommandBuffer *cb_state, const vvl::ImageView &iv_state, VkImageLayout layout) -> void { - cb_state->SetImageViewInitialLayout(iv_state, layout); - }); - AdjustValidatorOptions(device_extensions, enabled_features, spirv_val_options, &spirv_val_option_hash); // Allocate shader validation cache diff --git a/layers/drawdispatch/descriptor_validator.cpp b/layers/drawdispatch/descriptor_validator.cpp index b7ba393f097..4bf101a5274 100644 --- a/layers/drawdispatch/descriptor_validator.cpp +++ b/layers/drawdispatch/descriptor_validator.cpp @@ -160,7 +160,7 @@ bool DescriptorValidator::ValidateBindingDynamic(const DescriptorBindingInfo &bi auto &imgs_binding = static_cast(binding); for (auto index : indices) { auto &descriptor = imgs_binding.descriptors[index]; - descriptor.UpdateDrawState(&dev_state, &cb_state); + descriptor.UpdateDrawState(&dev_state, cb_state); } skip |= ValidateDescriptorsDynamic(binding_info, imgs_binding, indices); break; @@ -169,7 +169,7 @@ bool DescriptorValidator::ValidateBindingDynamic(const DescriptorBindingInfo &bi auto &img_binding = static_cast(binding); for (auto index : indices) { auto &descriptor = img_binding.descriptors[index]; - descriptor.UpdateDrawState(&dev_state, &cb_state); + descriptor.UpdateDrawState(&dev_state, cb_state); } skip |= ValidateDescriptorsDynamic(binding_info, img_binding, indices); break; diff --git a/layers/gpu/core/gpuav_setup.cpp b/layers/gpu/core/gpuav_setup.cpp index 3b030995409..62e59627a6a 100644 --- a/layers/gpu/core/gpuav_setup.cpp +++ b/layers/gpu/core/gpuav_setup.cpp @@ -384,14 +384,6 @@ void Validator::PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Lo return; } - // Add the callback hooks for the functions that are either broadly or deeply used and that the ValidationStateTracker refactor - // would be messier without. - // TODO: Find a good way to do this hooklessly. - SetSetImageViewInitialLayoutCallback( - [](vvl::CommandBuffer *cb_state, const vvl::ImageView &iv_state, VkImageLayout layout) -> void { - cb_state->SetImageViewInitialLayout(iv_state, layout); - }); - // Set up a stub implementation of the descriptor heap in case we abort. desc_heap_.emplace(*this, 0, loc); diff --git a/layers/state_tracker/cmd_buffer_state.cpp b/layers/state_tracker/cmd_buffer_state.cpp index 6f6e17f3eb4..897d3a0c917 100644 --- a/layers/state_tracker/cmd_buffer_state.cpp +++ b/layers/state_tracker/cmd_buffer_state.cpp @@ -1242,7 +1242,7 @@ void CommandBuffer::UpdatePipelineState(Func command, const VkPipelineBindPoint } // Bind this set and its active descriptor resources to the command buffer - descriptor_set->UpdateDrawStates(&dev_data, this, set_binding_pair.second); + descriptor_set->UpdateDrawStates(&dev_data, *this, set_binding_pair.second); set_info.validated_set = descriptor_set.get(); set_info.validated_set_change_count = descriptor_set->GetChangeCount(); diff --git a/layers/state_tracker/descriptor_sets.cpp b/layers/state_tracker/descriptor_sets.cpp index 42c538b1325..b4d96a2eea0 100644 --- a/layers/state_tracker/descriptor_sets.cpp +++ b/layers/state_tracker/descriptor_sets.cpp @@ -687,7 +687,7 @@ void vvl::DescriptorSet::PerformCopyUpdate(const VkCopyDescriptorSet &update, co // TODO: Modify the UpdateDrawState virtural functions to *only* set initial layout and not change layouts // Prereq: This should be called for a set that has been confirmed to be active for the given cb_state, meaning it's going // to be used in a draw by the given cb_state -void vvl::DescriptorSet::UpdateDrawStates(ValidationStateTracker *device_data, vvl::CommandBuffer *cb_state, +void vvl::DescriptorSet::UpdateDrawStates(ValidationStateTracker *device_data, vvl::CommandBuffer &cb_state, const BindingVariableMap &binding_req_map) { // Descriptor UpdateDrawState only call image layout validation callbacks. If it is disabled, skip the entire loop. if (device_data->disabled[image_layout_validation]) { @@ -873,11 +873,11 @@ void vvl::ImageDescriptor::CopyUpdate(DescriptorSet &set_state, const Validation UpdateKnownValidView(is_bindless); } -void vvl::ImageDescriptor::UpdateDrawState(ValidationStateTracker *dev_data, vvl::CommandBuffer *cb_state) { +void vvl::ImageDescriptor::UpdateDrawState(ValidationStateTracker *dev_data, vvl::CommandBuffer &cb_state) { // Add binding for image auto iv_state = GetImageViewState(); if (iv_state) { - dev_data->CallSetImageViewInitialLayoutCallback(cb_state, *iv_state, image_layout_); + cb_state.SetImageViewInitialLayout(*iv_state, image_layout_); } } @@ -1301,11 +1301,11 @@ VkDeviceSize vvl::MutableDescriptor::GetEffectiveRange() const { } } -void vvl::MutableDescriptor::UpdateDrawState(ValidationStateTracker *dev_data, vvl::CommandBuffer *cb_state) { +void vvl::MutableDescriptor::UpdateDrawState(ValidationStateTracker *dev_data, vvl::CommandBuffer &cb_state) { auto active_class = DescriptorTypeToClass(active_descriptor_type_); if (active_class == DescriptorClass::Image || active_class == DescriptorClass::ImageSampler) { if (image_view_state_) { - dev_data->CallSetImageViewInitialLayoutCallback(cb_state, *image_view_state_, image_layout_); + cb_state.SetImageViewInitialLayout(*image_view_state_, image_layout_); } } } diff --git a/layers/state_tracker/descriptor_sets.h b/layers/state_tracker/descriptor_sets.h index b7a4542502b..b4c3df31316 100644 --- a/layers/state_tracker/descriptor_sets.h +++ b/layers/state_tracker/descriptor_sets.h @@ -449,7 +449,7 @@ class ImageDescriptor : public Descriptor { bool is_bindless) override; void CopyUpdate(DescriptorSet &set_state, const ValidationStateTracker &dev_data, const Descriptor &, bool is_bindless, VkDescriptorType type) override; - void UpdateDrawState(ValidationStateTracker *, vvl::CommandBuffer *cb_state); + void UpdateDrawState(ValidationStateTracker *, vvl::CommandBuffer &cb_state); VkImageView GetImageView() const; const vvl::ImageView *GetImageViewState() const { return image_view_state_.get(); } vvl::ImageView *GetImageViewState() { return image_view_state_.get(); } @@ -614,7 +614,7 @@ class MutableDescriptor : public Descriptor { return acc_khr != VK_NULL_HANDLE; } - void UpdateDrawState(ValidationStateTracker *, vvl::CommandBuffer *cb_state); + void UpdateDrawState(ValidationStateTracker *, vvl::CommandBuffer &cb_state); bool AddParent(StateObject *state_object) override; void RemoveParent(StateObject *state_object) override; @@ -828,7 +828,7 @@ class DescriptorSet : public StateObject { VkDescriptorSet VkHandle() const { return handle_.Cast(); }; // Bind given cmd_buffer to this descriptor set and // update CB image layout map with image/imagesampler descriptor image layouts - void UpdateDrawStates(ValidationStateTracker *, vvl::CommandBuffer *cb_state, const BindingVariableMap &); + void UpdateDrawStates(ValidationStateTracker *, vvl::CommandBuffer &cb_state, const BindingVariableMap &); // For a particular binding, get the global index const IndexRange GetGlobalIndexRangeFromBinding(const uint32_t binding, bool actual_length = false) const { diff --git a/layers/state_tracker/state_tracker.h b/layers/state_tracker/state_tracker.h index 7d14c536c6d..a04aa17ccae 100644 --- a/layers/state_tracker/state_tracker.h +++ b/layers/state_tracker/state_tracker.h @@ -437,18 +437,6 @@ class ValidationStateTracker : public ValidationObject { return {written_count, buffer_address_map_.size()}; } - using SetImageViewInitialLayoutCallback = std::function; - template - void SetSetImageViewInitialLayoutCallback(Fn&& fn) { - set_image_view_initial_layout_callback.reset(new SetImageViewInitialLayoutCallback(std::forward(fn))); - } - - void CallSetImageViewInitialLayoutCallback(vvl::CommandBuffer* cb_state, const vvl::ImageView& iv_state, VkImageLayout layout) { - if (set_image_view_initial_layout_callback) { - (*set_image_view_initial_layout_callback)(cb_state, iv_state, layout); - } - } - VkDeviceSize AllocFakeMemory(VkDeviceSize size) { return fake_memory.Alloc(size); } void FreeFakeMemory(VkDeviceSize address) { fake_memory.Free(address); } @@ -1788,8 +1776,6 @@ class ValidationStateTracker : public ValidationObject { // Link for derived device objects back to their parent instance object ValidationStateTracker* instance_state; - std::unique_ptr set_image_view_initial_layout_callback; - DeviceFeatures enabled_features = {}; // Device specific data VkPhysicalDeviceMemoryProperties phys_dev_mem_props = {};