layers: Dehandle the image layout maps
Change the keys in the CMD_BUFFER_STATE and CoreChecks image layout
maps to be IMAGE_STATE* rather than VkImage. Layout validation
requires the IMAGE_STATE object so this saves many lookups in cases
where the entire maps are processed. The CMD_BUFFER_STATE layout
map can be kept up to date by removing entries that are being
destroyed in the NotifyInvalidate() callback.
diff --git a/layers/buffer_validation.cpp b/layers/buffer_validation.cpp
index f89a2d3..e918cc2 100644
--- a/layers/buffer_validation.cpp
+++ b/layers/buffer_validation.cpp
@@ -334,7 +334,7 @@
uint32_t FullMipChainLevels(VkExtent2D extent) { return FullMipChainLevels(extent.height, extent.width); }
bool CoreChecks::FindLayouts(const IMAGE_STATE &image_state, std::vector<VkImageLayout> &layouts) const {
- const auto *layout_range_map = GetLayoutRangeMap(imageLayoutMap, image_state.image());
+ const auto *layout_range_map = GetLayoutRangeMap(imageLayoutMap, image_state);
if (!layout_range_map) return false;
// TODO: FindLayouts function should mutate into a ValidatePresentableLayout with the loop wrapping the LogError
// from the caller. You can then use decode to add the subresource of the range::begin to the error message.
@@ -569,7 +569,7 @@
// Cast pCB to const because we don't want to create entries that don't exist here (in case the key changes to something
// in common with the non-const version.)
// The lookup is expensive, so cache it.
- subresource_map = const_p_cb->GetImageSubresourceLayoutMap(image);
+ subresource_map = const_p_cb->GetImageSubresourceLayoutMap(*image_state);
has_queried_map = true;
}
@@ -871,13 +871,13 @@
if (img_barrier.oldLayout == VK_IMAGE_LAYOUT_UNDEFINED) {
// TODO: Set memory invalid which is in mem_tracker currently
} else if (!QueueFamilyIsExternal(img_barrier.srcQueueFamilyIndex)) {
- auto &write_subresource_map = layout_updates[img_barrier.image];
+ auto &write_subresource_map = layout_updates[image_state];
bool new_write = false;
if (!write_subresource_map) {
write_subresource_map.emplace(*image_state);
new_write = true;
}
- const auto ¤t_subresource_map = current_map.find(img_barrier.image);
+ const auto ¤t_subresource_map = current_map.find(image_state);
const auto &read_subresource_map = (new_write && current_subresource_map != current_map.end())
? (*current_subresource_map).second
: write_subresource_map;
@@ -1384,10 +1384,9 @@
if (disabled[image_layout_validation]) return false;
assert(cb_node);
assert(image_state);
- const auto image = image_state->image();
bool skip = false;
- const auto *subresource_map = cb_node->GetImageSubresourceLayoutMap(image);
+ const auto *subresource_map = cb_node->GetImageSubresourceLayoutMap(*image_state);
if (subresource_map) {
bool subres_skip = false;
LayoutUseCheckAndMessage layout_check(subresource_map, aspect_mask);
@@ -1396,12 +1395,13 @@
for (auto pos = subresource_map->Find(range); !(pos.AtEnd()) && !subres_skip; pos.IncrementInterval()) {
if (!layout_check.Check(pos->subresource, explicit_layout, pos->current_layout, pos->initial_layout)) {
*error = true;
- subres_skip |= LogError(cb_node->commandBuffer(), layout_mismatch_msg_code,
- "%s: Cannot use %s (layer=%u mip=%u) with specific layout %s that doesn't match the "
- "%s layout %s.",
- caller, report_data->FormatHandle(image).c_str(), pos->subresource.arrayLayer,
- pos->subresource.mipLevel, string_VkImageLayout(explicit_layout), layout_check.message,
- string_VkImageLayout(layout_check.layout));
+ subres_skip |=
+ LogError(cb_node->commandBuffer(), layout_mismatch_msg_code,
+ "%s: Cannot use %s (layer=%u mip=%u) with specific layout %s that doesn't match the "
+ "%s layout %s.",
+ caller, report_data->FormatHandle(image_state->Handle()).c_str(), pos->subresource.arrayLayer,
+ pos->subresource.mipLevel, string_VkImageLayout(explicit_layout), layout_check.message,
+ string_VkImageLayout(layout_check.layout));
}
}
skip |= subres_skip;
@@ -1414,7 +1414,8 @@
// LAYOUT_GENERAL is allowed, but may not be performance optimal, flag as perf warning.
skip |= LogPerformanceWarning(cb_node->commandBuffer(), kVUID_Core_DrawState_InvalidImageLayout,
"%s: For optimal performance %s layout should be %s instead of GENERAL.", caller,
- report_data->FormatHandle(image).c_str(), string_VkImageLayout(optimal_layout));
+ report_data->FormatHandle(image_state->Handle()).c_str(),
+ string_VkImageLayout(optimal_layout));
}
} else if (IsExtEnabled(device_extensions.vk_khr_shared_presentable_image)) {
if (image_state->shared_presentable) {
@@ -1429,7 +1430,7 @@
*error = true;
skip |= LogError(cb_node->commandBuffer(), layout_invalid_msg_code,
"%s: Layout for %s is %s but can only be %s or VK_IMAGE_LAYOUT_GENERAL.", caller,
- report_data->FormatHandle(image).c_str(), string_VkImageLayout(explicit_layout),
+ report_data->FormatHandle(image_state->Handle()).c_str(), string_VkImageLayout(explicit_layout),
string_VkImageLayout(optimal_layout));
}
}
@@ -2079,9 +2080,10 @@
void CoreChecks::PreCallRecordDestroyImage(VkDevice device, VkImage image, const VkAllocationCallbacks *pAllocator) {
// Clean up validation specific data
+ auto *image_state = Get<IMAGE_STATE>(image);
qfo_release_image_barrier_map.erase(image);
- imageLayoutMap.erase(image);
+ imageLayoutMap.erase(image_state);
// Clean up generic image state
StateTracker::PreCallRecordDestroyImage(device, image, pAllocator);
@@ -2148,7 +2150,7 @@
}
// Cast to const to prevent creation at validate time.
- const auto *subresource_map = cb_node->GetImageSubresourceLayoutMap(image_state->image());
+ const auto *subresource_map = cb_node->GetImageSubresourceLayoutMap(*image_state);
if (subresource_map) {
bool subres_skip = false;
LayoutUseCheckAndMessage layout_check(subresource_map);
@@ -4203,15 +4205,15 @@
GlobalImageLayoutRangeMap *GetLayoutRangeMap(GlobalImageLayoutMap &map, const IMAGE_STATE &image_state) {
// This approach allows for a single hash lookup or/create new
- auto &layout_map = map[image_state.image()];
+ auto &layout_map = map[&image_state];
if (!layout_map) {
layout_map.emplace(image_state.subresource_encoder.SubresourceCount());
}
return &layout_map;
}
-const GlobalImageLayoutRangeMap *GetLayoutRangeMap(const GlobalImageLayoutMap &map, VkImage image) {
- auto it = map.find(image);
+const GlobalImageLayoutRangeMap *GetLayoutRangeMap(const GlobalImageLayoutMap &map, const IMAGE_STATE &image_state) {
+ auto it = map.find(&image_state);
if (it != map.end()) {
return &it->second;
}
@@ -4246,16 +4248,14 @@
// Iterate over the layout maps for each referenced image
GlobalImageLayoutRangeMap empty_map(1);
for (const auto &layout_map_entry : pCB->image_layout_map) {
- const auto image = layout_map_entry.first;
- const auto *image_state = GetImageState(image);
- if (!image_state) continue; // Can't check layouts of a dead image
+ const auto *image_state = layout_map_entry.first;
const auto &subres_map = layout_map_entry.second;
const auto &layout_map = subres_map->GetLayoutMap();
// Validate the initial_uses for each subresource referenced
if (layout_map.empty()) continue;
auto *overlay_map = GetLayoutRangeMap(overlayLayoutMap, *image_state);
- const auto *global_map = GetLayoutRangeMap(globalImageLayoutMap, image);
+ const auto *global_map = GetLayoutRangeMap(globalImageLayoutMap, *image_state);
if (global_map == nullptr) {
global_map = &empty_map;
}
@@ -4297,7 +4297,7 @@
"%s command buffer %s expects %s (subresource: aspectMask 0x%X array layer %u, mip level %u) "
"to be in layout %s--instead, current layout is %s.",
loc.Message().c_str(), report_data->FormatHandle(pCB->commandBuffer()).c_str(),
- report_data->FormatHandle(image).c_str(), subresource.aspectMask, subresource.arrayLayer,
+ report_data->FormatHandle(image_state->Handle()).c_str(), subresource.aspectMask, subresource.arrayLayer,
subresource.mipLevel, string_VkImageLayout(initial_layout), string_VkImageLayout(image_layout));
}
}
@@ -4320,10 +4320,8 @@
void CoreChecks::UpdateCmdBufImageLayouts(CMD_BUFFER_STATE *pCB) {
for (const auto &layout_map_entry : pCB->image_layout_map) {
- const auto image = layout_map_entry.first;
+ const auto *image_state = layout_map_entry.first;
const auto &subres_map = layout_map_entry.second;
- const auto *image_state = GetImageState(image);
- if (!image_state) continue; // Can't set layouts of a dead image
auto *global_map = GetLayoutRangeMap(imageLayoutMap, *image_state);
sparse_container::splice(*global_map, subres_map->GetLayoutMap(), GlobalLayoutUpdater());
}
diff --git a/layers/buffer_validation.h b/layers/buffer_validation.h
index 7f4f3b9..d8284ba 100644
--- a/layers/buffer_validation.h
+++ b/layers/buffer_validation.h
@@ -34,6 +34,6 @@
uint32_t FullMipChainLevels(VkExtent2D);
GlobalImageLayoutRangeMap *GetLayoutRangeMap(GlobalImageLayoutMap &map, const IMAGE_STATE &image_state);
-const GlobalImageLayoutRangeMap *GetLayoutRangeMap(const GlobalImageLayoutMap &map, VkImage image);
+const GlobalImageLayoutRangeMap *GetLayoutRangeMap(const GlobalImageLayoutMap &map, const IMAGE_STATE &image);
#endif // CORE_VALIDATION_BUFFER_VALIDATION_H_
diff --git a/layers/cmd_buffer_state.cpp b/layers/cmd_buffer_state.cpp
index 1c6b486..1c77a16 100644
--- a/layers/cmd_buffer_state.cpp
+++ b/layers/cmd_buffer_state.cpp
@@ -466,8 +466,15 @@
if (unlink) {
for (auto *obj : invalid_nodes) {
object_bindings.erase(obj);
- if (obj->Type() == kVulkanObjectTypeCommandBuffer) {
- linkedCommandBuffers.erase(static_cast<CMD_BUFFER_STATE *>(obj));
+ switch (obj->Type()) {
+ case kVulkanObjectTypeCommandBuffer:
+ linkedCommandBuffers.erase(static_cast<CMD_BUFFER_STATE *>(obj));
+ break;
+ case kVulkanObjectTypeImage:
+ image_layout_map.erase(static_cast<IMAGE_STATE *>(obj));
+ break;
+ default:
+ break;
}
}
}
@@ -477,8 +484,8 @@
const CommandBufferImageLayoutMap& CMD_BUFFER_STATE::GetImageSubresourceLayoutMap() const { return image_layout_map; }
// The const variant only need the image as it is the key for the map
-const ImageSubresourceLayoutMap *CMD_BUFFER_STATE::GetImageSubresourceLayoutMap(VkImage image) const {
- auto it = image_layout_map.find(image);
+const ImageSubresourceLayoutMap *CMD_BUFFER_STATE::GetImageSubresourceLayoutMap(const IMAGE_STATE &image_state) const {
+ auto it = image_layout_map.find(&image_state);
if (it == image_layout_map.cend()) {
return nullptr;
}
@@ -487,7 +494,7 @@
// The non-const variant only needs the image state, as the factory requires it to construct a new entry
ImageSubresourceLayoutMap *CMD_BUFFER_STATE::GetImageSubresourceLayoutMap(const IMAGE_STATE &image_state) {
- auto &layout_map = image_layout_map[image_state.image()];
+ auto &layout_map = image_layout_map[&image_state];
if (!layout_map) {
// Was an empty slot... fill it in.
layout_map.emplace(image_state);
@@ -763,9 +770,7 @@
// ValidationStateTracker these maps will be empty, so leaving the propagation in the the state tracker should be a no-op
// for those other classes.
for (const auto &sub_layout_map_entry : sub_cb_state->image_layout_map) {
- const auto image = sub_layout_map_entry.first;
- const IMAGE_STATE *image_state = dev_data->GetImageState(image);
- if (!image_state) continue; // Can't set layouts of a dead image
+ const auto *image_state = sub_layout_map_entry.first;
auto *cb_subres_map = GetImageSubresourceLayoutMap(*image_state);
const auto *sub_cb_subres_map = &sub_layout_map_entry.second;
diff --git a/layers/cmd_buffer_state.h b/layers/cmd_buffer_state.h
index eebb1b1..40bff88 100644
--- a/layers/cmd_buffer_state.h
+++ b/layers/cmd_buffer_state.h
@@ -167,11 +167,7 @@
std::vector<BufferBinding> vertex_buffer_bindings;
};
-typedef subresource_adapter::BothRangeMap<VkImageLayout, 16> GlobalImageLayoutRangeMap;
-typedef layer_data::unordered_map<VkImage, layer_data::optional<GlobalImageLayoutRangeMap>> GlobalImageLayoutMap;
-
-typedef layer_data::unordered_map<VkImage, layer_data::optional<ImageSubresourceLayoutMap>> CommandBufferImageLayoutMap;
-
+typedef layer_data::unordered_map<const IMAGE_STATE *, layer_data::optional<ImageSubresourceLayoutMap>> CommandBufferImageLayoutMap;
class CMD_BUFFER_STATE : public REFCOUNTED_NODE {
public:
@@ -344,9 +340,9 @@
void ResetPushConstantDataIfIncompatible(const PIPELINE_LAYOUT_STATE *pipeline_layout_state);
+ const ImageSubresourceLayoutMap *GetImageSubresourceLayoutMap(const IMAGE_STATE &image_state) const;
ImageSubresourceLayoutMap *GetImageSubresourceLayoutMap(const IMAGE_STATE &image_state);
const CommandBufferImageLayoutMap& GetImageSubresourceLayoutMap() const;
- const ImageSubresourceLayoutMap *GetImageSubresourceLayoutMap(VkImage image) const;
const QFOTransferBarrierSets<QFOImageTransferBarrier> &GetQFOBarrierSets(const QFOImageTransferBarrier &type_tag) const {
return qfo_transfer_image_barriers;
diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp
index 1fc9874..ab3de7f 100644
--- a/layers/core_validation.cpp
+++ b/layers/core_validation.cpp
@@ -252,7 +252,7 @@
}
const DEVICE_MEMORY_STATE *mem_info = ValidationStateTracker::GetDevMemState(mem);
if (mem_info) {
- const auto prev_binding = mem_binding.MemState();
+ const auto *prev_binding = mem_binding.MemState();
if (prev_binding) {
if (!prev_binding->Destroyed()) {
const char *error_code = nullptr;
@@ -12543,11 +12543,10 @@
// initial layout usage of secondary command buffers resources must match parent command buffer
const auto *const_cb_state = static_cast<const CMD_BUFFER_STATE *>(cb_state);
for (const auto &sub_layout_map_entry : sub_cb_state->image_layout_map) {
- const auto image = sub_layout_map_entry.first;
- const auto *image_state = GetImageState(image);
- if (!image_state) continue; // Can't set layouts of a dead image
+ const auto *image_state = sub_layout_map_entry.first;
+ const auto image = image_state->image();
- const auto *cb_subres_map = const_cb_state->GetImageSubresourceLayoutMap(image);
+ const auto *cb_subres_map = const_cb_state->GetImageSubresourceLayoutMap(*image_state);
// Const getter can be null in which case we have nothing to check against for this image...
if (!cb_subres_map) continue;
@@ -14053,7 +14052,7 @@
if (swapchain_data) {
for (const auto &swapchain_image : swapchain_data->images) {
if (!swapchain_image.image_state) continue;
- imageLayoutMap.erase(swapchain_image.image_state->image());
+ imageLayoutMap.erase(swapchain_image.image_state);
qfo_release_image_barrier_map.erase(swapchain_image.image_state->image());
}
}
diff --git a/layers/core_validation.h b/layers/core_validation.h
index dac4979..170b6b2 100644
--- a/layers/core_validation.h
+++ b/layers/core_validation.h
@@ -110,6 +110,9 @@
const char *base_mip_err, *mip_count_err, *base_layer_err, *layer_count_err;
};
+typedef subresource_adapter::BothRangeMap<VkImageLayout, 16> GlobalImageLayoutRangeMap;
+typedef layer_data::unordered_map<const IMAGE_STATE*, layer_data::optional<GlobalImageLayoutRangeMap>> GlobalImageLayoutMap;
+
class CoreChecks : public ValidationStateTracker {
public:
using StateTracker = ValidationStateTracker;
