sync: Categorize error messages #8936
Conversation
That's all syncval error messages that has different structure. Made some deduplication work here (single message is shared by different places), but the main goal was to move to a single place. More work is needed to classify this further.
|
CI Vulkan-ValidationLayers build queued with queue ID 311690. |
|
CI Vulkan-ValidationLayers build # 18149 running. |
| #include <cstdarg> | ||
| #include <sstream> | ||
|
|
||
| // TODO: update algorith in logging.cpp to be similar to this version. |
There was a problem hiding this comment.
As per comment, after some testing period this formatting code can be used to update existing code, there are some C++ spec problems with logging.cpp code (but works in practice!), but also it leads to some hacky manipulations that are not needed here.
| // in the char indexed as str[std::string::size()] and it should not be | ||
| // accesses in this way by client code, only by std::string itself. | ||
|
|
||
| static std::string Format(const char* format, ...) { |
There was a problem hiding this comment.
Two reasons why Format() is used, say as opposite to streams:
- Allowed to copy directly existing formatting snippets here
- In the future this will be easy to replace with std::format() with less changes comparing to using streams.
There was a problem hiding this comment.
makes sense... good to add as a comment here
|
CI Vulkan-ValidationLayers build # 18149 passed. |
| @@ -302,11 +294,9 @@ bool RenderPassAccessContext::ValidateStoreOperation(const CommandBufferAccessCo | |||
| const char *const op_type_string = checked_stencil ? "stencilStoreOp" : "storeOp"; | |||
| const char *const store_op_string = string_VkAttachmentStoreOp(checked_stencil ? ci.stencilStoreOp : ci.storeOp); | |||
There was a problem hiding this comment.
(can be done in a followup PR) but we could pass these in as vvl::Field::stencilStoreOp and then use the String method to print it later
That's all syncval error messages that have different structure. More work is needed to classify this further.
The idea that this should simplify further work when it's in a single place (already this helped to detect some inconsistencies and opportunities to organize this better).
Additional progress on this might allow to filter error messages (as ANGLE does) in more reliable way. Currently ANGLE specifies multiple parts of the error message to identify specific use case. A potential improvement is to filter in a more structured way and to specify more stable values than just arbitrary text snippets.
Initial motivation behind the change is to minimize breaking changes for the clients that filter error messages based on current structure. Having this in a centralized place it should be easier to provide some backward compatibility option if necessary while this is in active development.
TESTING: For testing purposes on the local machine I forced dispatchable handles to have the same value. This allowed to ensure that outputs of all negative syncval tests before and after this change are binary identical (the only potential bug if two dispatchable handles are swapped somehow).