Skip to content

Commit 01ba4b9

Browse files
artem-lunargartem-lunarg
committed
sync: Add ImageClearCommand
1 parent 4a06fda commit 01ba4b9

7 files changed

Lines changed: 179 additions & 62 deletions

File tree

‎layers/sync/sync_command.cpp‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,10 @@ bool ReplayCommands(SyncEnvironment& env, AccessContext& destination_access_cont
156156
replay_common(command_data.image_resolve_commands[index], access_context, replay_tag);
157157
continue;
158158
}
159+
case CommandType::kImageClear: {
160+
replay_common(command_data.image_clear_commands[index], access_context, replay_tag);
161+
continue;
162+
}
159163
case CommandType::kPipelineBarrier: {
160164
replay_common(command_data.barrier_commands[index], access_context, replay_tag);
161165
continue;
@@ -281,6 +285,7 @@ void CommandData::Reset() {
281285
buffer_image_copy_commands.clear();
282286
image_blit_commands.clear();
283287
image_resolve_commands.clear();
288+
image_clear_commands.clear();
284289
barrier_commands.clear();
285290
set_event_commands.clear();
286291
reset_event_commands.clear();
@@ -320,6 +325,7 @@ void CommandData::Reset() {
320325
buffer_image_copy_regions.clear();
321326
image_blit_regions.clear();
322327
image_resolve_regions.clear();
328+
image_clear_ranges.clear();
323329
barrier_sets.clear();
324330
events.clear();
325331
rendering_attachments.clear();
@@ -811,6 +817,52 @@ void ImageResolveCommand::Apply(SyncEnvironment& env, ResourceUsageTag tag, Acce
811817
}
812818
}
813819

820+
ImageClearCommand ImageClearCommand::Storage::MakeCommand(const CommandData& command_data) const {
821+
vvl::span<const VkImageSubresourceRange> ranges;
822+
if (range_count != 0) {
823+
ranges = vvl::make_span(&command_data.image_clear_ranges[first_range], range_count);
824+
}
825+
return {*image, ranges, handle_index};
826+
}
827+
828+
ImageClearCommand::Storage ImageClearCommand::MakeStorage(CommandData& command_data) const {
829+
command_data.AddImage(image);
830+
const uint32_t first_range = uint32_t(command_data.image_clear_ranges.size());
831+
const uint32_t range_count = uint32_t(ranges.size());
832+
vvl::Append(command_data.image_clear_ranges, ranges);
833+
return {&image, first_range, range_count, handle_index};
834+
}
835+
836+
bool ImageClearCommand::Validate(const CommandBufferContext& cb_context, const Location& loc) const {
837+
return Validate(cb_context.GetSyncEnvironment(), cb_context.GetCbAccessContext(), cb_context, kInvalidTag, loc);
838+
}
839+
840+
bool ImageClearCommand::Validate(const SyncEnvironment& env, const AccessContext& access_context,
841+
const CommandBufferContext& cb_context, ResourceUsageTag replay_tag, const Location& loc) const {
842+
bool skip = false;
843+
const SyncValidator& validator = env.validator;
844+
for (const auto [range_index, range] : vvl::enumerate(ranges)) {
845+
const auto hazard = access_context.DetectHazard(image, range, SYNC_CLEAR_TRANSFER_WRITE);
846+
if (hazard.IsHazard()) {
847+
const LogObjectList objlist = BaseObjectList(env, cb_context, image.Handle());
848+
const std::string resource_description = validator.FormatHandle(image);
849+
const std::string error = validator.error_messages_.ImageClearError(env, hazard, cb_context, replay_tag, loc,
850+
resource_description, uint32_t(range_index), range);
851+
skip |= validator.SyncError(hazard.Hazard(), objlist, loc, error);
852+
}
853+
}
854+
return skip;
855+
}
856+
857+
void ImageClearCommand::Apply(SyncEnvironment& env, ResourceUsageTag tag, AccessContext& access_context) const {
858+
const ResourceUsageTagEx tag_ex{tag, handle_index};
859+
const auto& image_state = SubState(image);
860+
for (const VkImageSubresourceRange& range : ranges) {
861+
ImageRangeGen range_gen = image_state.MakeImageRangeGen(range, false);
862+
access_context.UpdateAccessState(range_gen, SYNC_CLEAR_TRANSFER_WRITE, tag_ex, 0, env.queue_id);
863+
}
864+
}
865+
814866
BufferImageCopyCommand BufferImageCopyCommand::Storage::MakeCommand(const CommandData& command_data) const {
815867
vvl::span<const VkBufferImageCopy> regions;
816868
if (region_count != 0) {

‎layers/sync/sync_command.h‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ enum class CommandType : uint32_t {
6464
kBufferImageCopy,
6565
kImageBlit,
6666
kImageResolve,
67+
kImageClear,
6768
kPipelineBarrier,
6869
kSetEvent,
6970
kResetEvent,
@@ -274,6 +275,25 @@ struct ImageResolveCommand {
274275
static small_vector<VkImageResolve, 1> MakeRegions(vvl::span<const VkImageResolve2> regions);
275276
};
276277

278+
struct ImageClearCommand {
279+
const vvl::Image& image;
280+
vvl::span<const VkImageSubresourceRange> ranges;
281+
uint32_t handle_index = vvl::kNoIndex32;
282+
283+
struct Storage {
284+
const vvl::Image* image;
285+
uint32_t first_range;
286+
uint32_t range_count;
287+
uint32_t handle_index;
288+
ImageClearCommand MakeCommand(const CommandData& command_data) const;
289+
};
290+
Storage MakeStorage(CommandData& command_data) const;
291+
bool Validate(const CommandBufferContext& cb_context, const Location& loc) const;
292+
bool Validate(const SyncEnvironment& env, const AccessContext& access_context, const CommandBufferContext& cb_context,
293+
ResourceUsageTag replay_tag, const Location& loc) const;
294+
void Apply(SyncEnvironment& env, ResourceUsageTag tag, AccessContext& access_context) const;
295+
};
296+
277297
struct BarrierCommand {
278298
const BarrierSet& barrier_set;
279299

@@ -876,6 +896,7 @@ struct CommandData {
876896
std::vector<BufferImageCopyCommand::Storage> buffer_image_copy_commands;
877897
std::vector<ImageBlitCommand::Storage> image_blit_commands;
878898
std::vector<ImageResolveCommand::Storage> image_resolve_commands;
899+
std::vector<ImageClearCommand::Storage> image_clear_commands;
879900
std::vector<BarrierCommand::Storage> barrier_commands;
880901
std::vector<SetEventCommand::Storage> set_event_commands;
881902
std::vector<ResetEventCommand::Storage> reset_event_commands;
@@ -922,6 +943,7 @@ struct CommandData {
922943
std::vector<VkBufferImageCopy> buffer_image_copy_regions;
923944
std::vector<VkImageBlit> image_blit_regions;
924945
std::vector<VkImageResolve> image_resolve_regions;
946+
std::vector<VkImageSubresourceRange> image_clear_ranges;
925947
std::vector<BarrierSet> barrier_sets;
926948
std::vector<std::shared_ptr<const vvl::Event>> events;
927949
std::vector<RenderingAttachment> rendering_attachments;
@@ -963,6 +985,9 @@ struct CommandData {
963985
CommandRef Store(const ImageResolveCommand::Storage& storage) {
964986
return Store(CommandType::kImageResolve, image_resolve_commands, storage);
965987
}
988+
CommandRef Store(const ImageClearCommand::Storage& storage) {
989+
return Store(CommandType::kImageClear, image_clear_commands, storage);
990+
}
966991
CommandRef Store(const BufferImageCopyCommand::Storage& storage) {
967992
return Store(CommandType::kBufferImageCopy, buffer_image_copy_commands, storage);
968993
}

‎layers/sync/sync_command_buffer.cpp‎

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
#include <vulkan/utility/vk_format_utils.h>
1919
#include "sync/sync_command_buffer.h"
2020
#include "error_message/error_location.h"
21-
#include "sync/sync_image.h"
2221
#include "sync/sync_replay.h"
2322
#include "sync/sync_reporting.h"
2423
#include "sync/sync_validation.h"
@@ -227,13 +226,6 @@ static SyncAccessIndex GetSyncStageAccessIndexsByDescriptorSet(VkDescriptorType
227226
}
228227
}
229228

230-
static void UpdateImageAccessState(AccessContext& access_context, const vvl::Image& image, SyncAccessIndex current_usage,
231-
const VkImageSubresourceRange& subresource_range, const ResourceUsageTag& tag) {
232-
const auto& sub_state = SubState(image);
233-
ImageRangeGen range_gen = sub_state.MakeImageRangeGen(subresource_range, false);
234-
access_context.UpdateAccessState(range_gen, current_usage, ResourceUsageTagEx{tag});
235-
}
236-
237229
SyncEnvironment::SyncEnvironment(const SyncValidator& validator, VkQueueFlags queue_flags, QueueId queue_id,
238230
VulkanTypedHandle handle, SyncEventsContext& events_context,
239231
const ResourceUsageInfoProvider& usage_info_provider)
@@ -857,6 +849,10 @@ void CommandBufferContext::RecordExecutedCommandBuffer(const CommandBufferContex
857849
import_common(command_data.image_resolve_commands[index], command_data, tag, entry.tag_count);
858850
continue;
859851
}
852+
case CommandType::kImageClear: {
853+
import_common(command_data.image_clear_commands[index], command_data, tag, entry.tag_count);
854+
continue;
855+
}
860856
case CommandType::kPipelineBarrier: {
861857
import_common(command_data.barrier_commands[index], command_data, tag, entry.tag_count);
862858
continue;
@@ -1425,32 +1421,31 @@ void CommandBufferSubState::RecordResolveImage2(vvl::Image& src_image_state, vvl
14251421
}
14261422
}
14271423

1428-
void CommandBufferSubState::RecordClearColorImage(vvl::Image& image_state, VkImageLayout, const VkClearColorValue*,
1429-
uint32_t range_count, const VkImageSubresourceRange* ranges,
1430-
const Location& loc) {
1424+
static void RecordImageClear(CommandBufferContext& cb_context, vvl::Image& image, vvl::span<const VkImageSubresourceRange> ranges,
1425+
const Location& loc) {
14311426
const auto tag = cb_context.NextCommandTag(loc.function);
1432-
AccessContext& context = cb_context.GetCbAccessContext();
1433-
1434-
cb_context.AddCommandHandle(tag, image_state.Handle());
1427+
const auto tag_ex = cb_context.AddCommandHandle(tag, image.Handle());
1428+
const ImageClearCommand command{image, ranges, tag_ex.handle_index};
14351429

1436-
for (uint32_t index = 0; index < range_count; index++) {
1437-
const auto& range = ranges[index];
1438-
UpdateImageAccessState(context, image_state, SYNC_CLEAR_TRANSFER_WRITE, range, tag);
1430+
const auto& settings = cb_context.GetSyncState().syncval_settings;
1431+
if (settings.IsRecordTimeValidationEnabled()) {
1432+
command.Apply(cb_context.GetSyncEnvironment(), tag, cb_context.GetCbAccessContext());
14391433
}
1434+
if (settings.full_validation) {
1435+
cb_context.StoreCommand(tag, command);
1436+
}
1437+
}
1438+
1439+
void CommandBufferSubState::RecordClearColorImage(vvl::Image& image_state, VkImageLayout, const VkClearColorValue*,
1440+
uint32_t range_count, const VkImageSubresourceRange* ranges,
1441+
const Location& loc) {
1442+
RecordImageClear(cb_context, image_state, {ranges, range_count}, loc);
14401443
}
14411444

14421445
void CommandBufferSubState::RecordClearDepthStencilImage(vvl::Image& image_state, VkImageLayout, const VkClearDepthStencilValue*,
14431446
uint32_t range_count, const VkImageSubresourceRange* ranges,
14441447
const Location& loc) {
1445-
const auto tag = cb_context.NextCommandTag(loc.function);
1446-
AccessContext& context = cb_context.GetCbAccessContext();
1447-
1448-
cb_context.AddCommandHandle(tag, image_state.Handle());
1449-
1450-
for (uint32_t index = 0; index < range_count; index++) {
1451-
const auto& range = ranges[index];
1452-
UpdateImageAccessState(context, image_state, SYNC_CLEAR_TRANSFER_WRITE, range, tag);
1453-
}
1448+
RecordImageClear(cb_context, image_state, {ranges, range_count}, loc);
14541449
}
14551450

14561451
void CommandBufferSubState::RecordClearAttachments(uint32_t attachment_count, const VkClearAttachment* pAttachments,

‎layers/sync/sync_error_messages.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,8 @@ std::string ErrorMessages::ImageCopyResolveBlitError(const SyncEnvironment& env,
214214
std::move(additional_info));
215215
}
216216

217-
std::string ErrorMessages::ImageClearError(const HazardResult& hazard, const CommandBufferContext& cb_context, vvl::Func command,
217+
std::string ErrorMessages::ImageClearError(const SyncEnvironment& env, const HazardResult& hazard,
218+
const CommandBufferContext& cb_context, ResourceUsageTag replay_tag, const Location& loc,
218219
const std::string& resource_description, uint32_t subresource_range_index,
219220
const VkImageSubresourceRange& subresource_range) const {
220221
std::ostringstream ss;
@@ -223,11 +224,11 @@ std::string ErrorMessages::ImageClearError(const HazardResult& hazard, const Com
223224
ss << "}\n";
224225

225226
AdditionalMessageInfo additional_info;
227+
const vvl::Func command = AddReplayInfo(env, hazard, cb_context, replay_tag, loc, additional_info);
226228
additional_info.message_end_text = ss.str();
227229
additional_info.properties.Add(kPropertyRegionIndex, subresource_range_index);
228230

229-
return Error(cb_context.GetSyncEnvironment(), hazard, command, resource_description, "ImageSubresourceRangeError",
230-
additional_info);
231+
return Error(env, hazard, command, resource_description, "ImageSubresourceRangeError", additional_info);
231232
}
232233

233234
static void PrepareCommonDescriptorMessage(Logger& logger, const vvl::Pipeline& pipeline, uint32_t descriptor_set_number,

‎layers/sync/sync_error_messages.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,9 @@ class ErrorMessages {
8181
const std::string& resource_description, uint32_t region_index, const VkOffset3D& offset,
8282
const VkExtent3D& extent, const VkImageSubresourceLayers& subresource) const;
8383

84-
std::string ImageClearError(const HazardResult& hazard, const CommandBufferContext& cb_context, vvl::Func command,
85-
const std::string& resource_description, uint32_t subresource_range_index,
86-
const VkImageSubresourceRange& subresource_range) const;
84+
std::string ImageClearError(const SyncEnvironment& env, const HazardResult& hazard, const CommandBufferContext& cb_context,
85+
ResourceUsageTag replay_tag, const Location& loc, const std::string& resource_description,
86+
uint32_t subresource_range_index, const VkImageSubresourceRange& subresource_range) const;
8787

8888
std::string BufferDescriptorError(const SyncEnvironment& env, const HazardResult& hazard,
8989
const CommandBufferContext& cb_context, ResourceUsageTag replay_tag, const Location& loc,

‎layers/sync/sync_validation.cpp‎

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1538,47 +1538,35 @@ void SyncValidator::PostCallRecordCmdDrawIndirectByteCountEXT(VkCommandBuffer co
15381538
bool SyncValidator::PreCallValidateCmdClearColorImage(VkCommandBuffer commandBuffer, VkImage image, VkImageLayout imageLayout,
15391539
const VkClearColorValue* pColor, uint32_t rangeCount,
15401540
const VkImageSubresourceRange* pRanges, const ErrorObject& error_obj) const {
1541-
bool skip = false;
1541+
if (!syncval_settings.IsRecordTimeValidationEnabled()) {
1542+
return false;
1543+
}
1544+
const auto image_state = Get<vvl::Image>(image);
1545+
if (!image_state) {
1546+
return false;
1547+
}
15421548
const auto cb_state = Get<vvl::CommandBuffer>(commandBuffer);
15431549
const CommandBufferContext& cb_context = GetCommandBufferContext(*cb_state);
1544-
const AccessContext& access_context = cb_context.GetCbAccessContext();
1545-
1546-
if (auto image_state = Get<vvl::Image>(image)) {
1547-
for (const auto [range_index, range] : vvl::enumerate(pRanges, rangeCount)) {
1548-
auto hazard = access_context.DetectHazard(*image_state, range, SYNC_CLEAR_TRANSFER_WRITE);
1549-
if (hazard.IsHazard()) {
1550-
const LogObjectList objlist(commandBuffer, image);
1551-
const auto error = error_messages_.ImageClearError(hazard, cb_context, error_obj.location.function,
1552-
FormatHandle(image), range_index, range);
1553-
skip |= SyncError(hazard.Hazard(), objlist, error_obj.location, error);
1554-
}
1555-
}
1556-
}
1557-
return skip;
1550+
const ImageClearCommand command{*image_state, {pRanges, rangeCount}};
1551+
return command.Validate(cb_context, error_obj.location);
15581552
}
15591553

15601554
bool SyncValidator::PreCallValidateCmdClearDepthStencilImage(VkCommandBuffer commandBuffer, VkImage image,
15611555
VkImageLayout imageLayout,
15621556
const VkClearDepthStencilValue* pDepthStencil, uint32_t rangeCount,
15631557
const VkImageSubresourceRange* pRanges,
15641558
const ErrorObject& error_obj) const {
1565-
bool skip = false;
1559+
if (!syncval_settings.IsRecordTimeValidationEnabled()) {
1560+
return false;
1561+
}
1562+
const auto image_state = Get<vvl::Image>(image);
1563+
if (!image_state) {
1564+
return false;
1565+
}
15661566
const auto cb_state = Get<vvl::CommandBuffer>(commandBuffer);
15671567
const CommandBufferContext& cb_context = GetCommandBufferContext(*cb_state);
1568-
const AccessContext& access_context = cb_context.GetCbAccessContext();
1569-
1570-
if (auto image_state = Get<vvl::Image>(image)) {
1571-
for (const auto [range_index, range] : vvl::enumerate(pRanges, rangeCount)) {
1572-
auto hazard = access_context.DetectHazard(*image_state, range, SYNC_CLEAR_TRANSFER_WRITE);
1573-
if (hazard.IsHazard()) {
1574-
const LogObjectList objlist(commandBuffer, image);
1575-
const auto error = error_messages_.ImageClearError(hazard, cb_context, error_obj.location.function,
1576-
FormatHandle(image), range_index, range);
1577-
skip |= SyncError(hazard.Hazard(), objlist, error_obj.location, error);
1578-
}
1579-
}
1580-
}
1581-
return skip;
1568+
const ImageClearCommand command{*image_state, {pRanges, rangeCount}};
1569+
return command.Validate(cb_context, error_obj.location);
15821570
}
15831571

15841572
bool SyncValidator::PreCallValidateCmdClearAttachments(VkCommandBuffer commandBuffer, uint32_t attachmentCount,

‎tests/unit/sync_val.cpp‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3275,6 +3275,62 @@ TEST_F(NegativeSyncVal, CmdClear) {
32753275
m_command_buffer.End();
32763276
}
32773277

3278+
TEST_F(NegativeSyncVal, ClearColorImageRanges) {
3279+
TEST_DESCRIPTION("A clear in another command buffer conflicts with the second clear range");
3280+
RETURN_IF_SKIP(InitSyncVal());
3281+
3282+
const auto image_ci = vkt::Image::ImageCreateInfo2D(8, 8, 1, 2, VK_FORMAT_R8G8B8A8_UNORM, VK_IMAGE_USAGE_TRANSFER_DST_BIT);
3283+
vkt::Image image(*m_device, image_ci);
3284+
image.SetLayout(VK_IMAGE_LAYOUT_GENERAL);
3285+
3286+
const VkClearColorValue clear_color = {};
3287+
const VkImageSubresourceRange ranges[] = {
3288+
{VK_IMAGE_ASPECT_COLOR_BIT, 0, 1, 0, 1},
3289+
{VK_IMAGE_ASPECT_COLOR_BIT, 0, 1, 1, 1},
3290+
};
3291+
3292+
m_command_buffer.Begin();
3293+
// clear both layers
3294+
vk::CmdClearColorImage(m_command_buffer, image, VK_IMAGE_LAYOUT_GENERAL, &clear_color, 2, ranges);
3295+
m_command_buffer.End();
3296+
3297+
vkt::CommandBuffer clear_cb(*m_device, m_command_pool);
3298+
clear_cb.Begin();
3299+
// clear the second layer
3300+
vk::CmdClearColorImage(clear_cb, image, VK_IMAGE_LAYOUT_GENERAL, &clear_color, 1, &ranges[1]);
3301+
clear_cb.End();
3302+
3303+
m_errorMonitor->SetDesiredError("SYNC-HAZARD-WRITE-AFTER-WRITE");
3304+
m_default_queue->Submit({m_command_buffer, clear_cb});
3305+
m_errorMonitor->VerifyFound();
3306+
m_default_queue->Wait();
3307+
}
3308+
3309+
TEST_F(NegativeSyncVal, ClearDepthStencilImageAcrossCommandBuffers) {
3310+
TEST_DESCRIPTION("Depth/stencil clears in separate command buffers write the same subresource");
3311+
RETURN_IF_SKIP(InitSyncVal());
3312+
3313+
vkt::Image image(*m_device, 8, 8, FindSupportedDepthStencilFormat(Gpu()), VK_IMAGE_USAGE_TRANSFER_DST_BIT);
3314+
image.SetLayout(VK_IMAGE_LAYOUT_GENERAL);
3315+
3316+
const VkClearDepthStencilValue clear_value = {};
3317+
const VkImageSubresourceRange range{VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, 0, 1, 0, 1};
3318+
3319+
m_command_buffer.Begin();
3320+
vk::CmdClearDepthStencilImage(m_command_buffer, image, VK_IMAGE_LAYOUT_GENERAL, &clear_value, 1, &range);
3321+
m_command_buffer.End();
3322+
3323+
vkt::CommandBuffer clear_cb(*m_device, m_command_pool);
3324+
clear_cb.Begin();
3325+
vk::CmdClearDepthStencilImage(clear_cb, image, VK_IMAGE_LAYOUT_GENERAL, &clear_value, 1, &range);
3326+
clear_cb.End();
3327+
3328+
m_errorMonitor->SetDesiredError("SYNC-HAZARD-WRITE-AFTER-WRITE");
3329+
m_default_queue->Submit({m_command_buffer, clear_cb});
3330+
m_errorMonitor->VerifyFound();
3331+
m_default_queue->Wait();
3332+
}
3333+
32783334
TEST_F(NegativeSyncVal, CmdQuery) {
32793335
// CmdCopyQueryPoolResults
32803336
all_queue_count_ = true;

0 commit comments

Comments
 (0)