Clarify Vulkan-specific items #1443
Conversation
|
CI gfxreconstruct build queued with queue ID 148599. |
|
CI gfxreconstruct build # 3853 running. |
|
CI gfxreconstruct build # 3853 passed. |
|
CI gfxreconstruct build queued with queue ID 149400. |
|
CI gfxreconstruct build # 3864 running. |
|
CI gfxreconstruct build # 3864 passed. |
| @@ -78,7 +78,7 @@ class ParameterEncoder | |||
| void EncodeFunctionPtr(T value) { EncodeValue(reinterpret_cast<format::AddressEncodeType>(value)); } | |||
|
|
|||
| template<typename Wrapper> | |||
| void EncodeHandleValue(typename Wrapper::HandleType value) { EncodeHandleIdValue(GetWrappedId<Wrapper>(value)); } | |||
| void EncodeHandleValue(typename Wrapper::HandleType value) { EncodeHandleIdValue(GetVulkanWrappedId<Wrapper>(value)); } | |||
There was a problem hiding this comment.
Add Vulkan to the parameter encoder name as well? e.g., EncodeHandleValue -> EncodeVulkanHandleValue
| @@ -164,7 +164,7 @@ static void EncodeDescriptorUpdateTemplateInfo(VulkanCaptureManager* manager | |||
| { | |||
| size_t offset = entry_info.offset + (entry_info.stride * i); | |||
| const VkBufferView* entry = reinterpret_cast<const VkBufferView*>(bytes + offset); | |||
| encoder->EncodeHandleValue<BufferViewWrapper>(*entry); | |||
| encoder->EncodeHandleValue<vulkan_wrappers::BufferViewWrapper>(*entry); | |||
There was a problem hiding this comment.
I wonder if there is another solution for delineating these Vulkan wrappers that doesn't require the vulkan_wrappers:: prefix on every object. Some potential ideas:
- Add a Vulkan-specific namespace (e.g.,
gfxrecon::encode::vulkanorgfxrecon::encode::vk) for all Vulkan-specific encode files. A Vulkan-specific namespace could also be added forgfxrecon::decode, e.g.,gfxrecon::decode::vulkan - Adopt the DX12 wrapper naming pattern
[DX12 object name]_Wrapper, sovulkan_wrappers::BufferViewWrapperbecomesVkBufferView_Wrapper
There was a problem hiding this comment.
Your idea of the full name might work, but doing additional namespaces was much more involved than I wanted to go down.
Vulkan Instance and Device functions (and tables) were only identified by the Instance or Device label and not clearly identified as being Vulkan-specific. Add "Vulkan" to each to clarify their usage so that they do not conflict with other Khronos-related API items.
Update Vulkan-specific wrappers in the encoder to have a "Vulkan" identifier to differentiate it from other Khronos API related items.
Move a number of Vulkan-specific items into a few namespaces so that it is clear which items are Vulkan in the future when other APIs have been added into the standard layer.
bradgrantham-lunarg
force-pushed
the
marky_separate_vulkan_items
branch
from
8253c30 to
d16973b
Compare
|
CI gfxreconstruct build queued with queue ID 157704. |
|
CI gfxreconstruct build # 3926 running. |
|
CI gfxreconstruct build # 3926 passed. |
|
CI gfxreconstruct build queued with queue ID 158183. |
|
CI gfxreconstruct build # 3930 running. |
|
CI gfxreconstruct build # 3930 passed. |
Since GFXR started as a Vulkan project, many of the items have no clear identifier in their name. But now that GFXR supports DX and may support other APIs in the future, we should clarify Vulkan-specific items.