Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A handful of interface changes to support OpenXR future rebasing #1846

Merged
merged 8 commits into from
Oct 31, 2024
8 changes: 1 addition & 7 deletions framework/decode/dx_replay_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,7 @@ struct DxReplayOptions : public ReplayOptions
bool override_object_names{ false };
bool enable_dump_resources{ false };
DumpResourcesTarget dump_resources_target{};

util::ScreenshotFormat screenshot_format{ util::ScreenshotFormat::kBmp };
std::vector<ScreenshotRange> screenshot_ranges;
std::string screenshot_dir;
std::string screenshot_file_prefix{ kDefaultScreenshotFilePrefix };
std::string replace_dir;
int32_t memory_usage{ kDefaultBatchingMemoryUsage };
int32_t memory_usage{ kDefaultBatchingMemoryUsage };
};

GFXRECON_END_NAMESPACE(decode)
Expand Down
49 changes: 28 additions & 21 deletions framework/decode/replay_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,27 +41,34 @@ struct ScreenshotRange

struct ReplayOptions
{
bool enable_validation_layer{ false };
bool sync_queue_submissions{ false };
bool enable_debug_device_lost{ false };
bool create_dummy_allocations{ false };
bool omit_null_hardware_buffers{ false };
bool quit_after_measurement_frame_range{ false };
bool flush_measurement_frame_range{ false };
bool flush_inside_measurement_range{ false };
bool force_windowed{ false };
uint32_t windowed_width{ 0 };
uint32_t windowed_height{ 0 };
bool force_windowed_origin{ false };
int32_t window_topleft_x{ 0 };
int32_t window_topleft_y{ 0 };
int32_t override_gpu_index{ -1 };
std::string capture_filename;
bool enable_print_block_info{ false };
int64_t block_index_from{ -1 };
int64_t block_index_to{ -1 };
int32_t num_pipeline_creation_jobs{ 0 };
std::string asset_file_path;
bool enable_validation_layer{ false };
bool sync_queue_submissions{ false };
bool enable_debug_device_lost{ false };
bool create_dummy_allocations{ false };
bool omit_null_hardware_buffers{ false };
bool quit_after_measurement_frame_range{ false };
bool flush_measurement_frame_range{ false };
bool flush_inside_measurement_range{ false };
bool force_windowed{ false };
uint32_t windowed_width{ 0 };
uint32_t windowed_height{ 0 };
bool force_windowed_origin{ false };
int32_t window_topleft_x{ 0 };
int32_t window_topleft_y{ 0 };
int32_t override_gpu_index{ -1 };
std::string capture_filename;
bool enable_print_block_info{ false };
int64_t block_index_from{ -1 };
int64_t block_index_to{ -1 };
bool skip_failed_allocations{ false };
bool remove_unsupported_features{ false };
util::ScreenshotFormat screenshot_format{ util::ScreenshotFormat::kBmp };
std::vector<ScreenshotRange> screenshot_ranges;
std::string screenshot_dir;
std::string screenshot_file_prefix{ kDefaultScreenshotFilePrefix };
uint32_t screenshot_width, screenshot_height;
int32_t num_pipeline_creation_jobs{ 0 };
std::string asset_file_path;
};

GFXRECON_END_NAMESPACE(decode)
Expand Down
33 changes: 31 additions & 2 deletions framework/decode/struct_pointer_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,35 @@

#include <cassert>
#include <memory>
#include <type_traits>
#include <utility>

GFXRECON_BEGIN_NAMESPACE(gfxrecon)
GFXRECON_BEGIN_NAMESPACE(decode)

template <typename T, typename = void>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Put a blank line above this.

struct DecoderHasOutputAllocator : std::false_type
{};

template <typename T>
struct DecoderHasOutputAllocator<T, std::void_t<decltype(std::declval<T>().AllocateOutputData(size_t(1)))>>
: std::true_type
{};

template <typename T>
std::enable_if_t<!DecoderHasOutputAllocator<T>::value, typename T::struct_type*>
StructPointerOutputDataAllocator(T*, size_t len)
{
return DecodeAllocator::Allocate<typename T::struct_type>(len);
}

template <typename T>
std::enable_if_t<DecoderHasOutputAllocator<T>::value, typename T::struct_type*>
StructPointerOutputDataAllocator(T* decoded_value, size_t len)
{
return decoded_value->AllocateOutputData(len);
}

template <typename T>
class StructPointerDecoder : public PointerDecoderBase
{
Expand All @@ -68,12 +93,13 @@ class StructPointerDecoder : public PointerDecoderBase
typename T::struct_type* AllocateOutputData(size_t len)
{
output_len_ = len;
output_data_ = DecodeAllocator::Allocate<typename T::struct_type>(len);
output_data_ = StructPointerOutputDataAllocator(decoded_structs_, len);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change for a new use of StructPointerDecoder? Or is there an existing use that wasn't being handled correclty?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New use.

return output_data_;
}

typename T::struct_type* AllocateOutputData(size_t len, const typename T::struct_type& init)
{
assert(!DecoderHasOutputAllocator<T>::value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer GFXRECON_ASSERT to assert. The CMake option GFXRECON_ENABLE_RELEASE_ASSERTS can be enabled to log failed GFXRECON_ASSERTs as errors in release builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest of this file is all assert(...)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine. For new code please prefer GFXRECON_ASSERT. There may come a day when we change as many assert as we can to GFXRECON_ASSERT

output_len_ = len;
output_data_ = DecodeAllocator::Allocate<typename T::struct_type>(len);

Expand Down Expand Up @@ -222,7 +248,10 @@ class StructPointerDecoder<T*> : public PointerDecoderBase

typename T::struct_type* inner_struct_memory =
DecodeAllocator::Allocate<typename T::struct_type>(inner_lens_[i]);
T* inner_decoded_structs = DecodeAllocator::Allocate<T>(inner_lens_[i]);
// TODO: We initialize == true because the nexe field isn't always cleared on kIsNull in the lower
// level decoders. If this is a performance bottleneck, can clean up the lower decoders to
// initialize all fields.
T* inner_decoded_structs = DecodeAllocator::Allocate<T>(inner_lens_[i], true);

for (size_t j = 0; j < inner_lens_[i]; ++j)
{
Expand Down
10 changes: 10 additions & 0 deletions framework/decode/vulkan_object_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,16 @@ struct VulkanPhysicalDeviceInfo : public VulkanObjectInfo<VkPhysicalDevice>

// Closest matching replay device.
VulkanReplayDeviceInfo* replay_device_info{ nullptr };

// Because Vulkan captures unwrapped handles, and layered APIs (like OpenXR)
// capture wrapped handles, during replay two HandleId values will reference
// the same VkPhysical device. The vulkan_alias is the handleId as known by
// the vulkan_consumers, which will be created/updated, etc, by all Vulkan replay
// calls, s.t. the information known by the vulkan_consumer need not be duplicated.
// Operations on this, will use the vulkan_alias as the effective HandleId when set.

// When Non-null, the GetVkObject will recur on the alias Id
format::HandleId vulkan_alias{ format::kNullHandleId };
};

struct VulkanDeviceInfo : public VulkanObjectInfo<VkDevice>
Expand Down
99 changes: 69 additions & 30 deletions framework/decode/vulkan_object_info_table_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

#include <cassert>
#include <functional>
#include <type_traits>
#include <unordered_map>

GFXRECON_BEGIN_NAMESPACE(gfxrecon)
Expand All @@ -49,6 +50,72 @@ struct has_handle_future<T, decltype((void)T::future, 0)>
template <typename T>
inline constexpr bool has_handle_future_v = has_handle_future<T>::value;

// NOTE: There's nothing VulkanSpecific in these utilities
// TODO: Find a better home for these

// Utility functors to implement const and non-const versions of getters in a common impl
template <typename Container>
using ConstCorrectMappedTypePtr = decltype(&(std::declval<Container>().begin()->second));

struct ObjectInfoGetterBase
{
template <typename Map, typename MappedTypePtr>
MappedTypePtr GetObjectInfoImpl(format::HandleId id, Map* map)
{
assert(map != nullptr);

MappedTypePtr object_info = nullptr;

if (id != 0)
{
const auto entry = map->find(id);

if (entry != map->end())
{
object_info = &entry->second;
}
}

return object_info;
}
template <typename Map, typename MappedTypePtr>
MappedTypePtr GetAliasingObjectInfoImpl(format::HandleId id, Map* map)
{
MappedTypePtr object_info = GetObjectInfoImpl<Map, MappedTypePtr>(id, map);
if (object_info && (object_info->vulkan_alias != format::kNullHandleId))
{
object_info = GetObjectInfoImpl<Map, MappedTypePtr>(object_info->vulkan_alias, map);
// Note: if id has an alias and the alias is valid, the alias must not alias. Aliasing is single level.
assert(!object_info || (object_info->vulkan_alias == format::kNullHandleId));
}
return object_info;
}
};

// Because of " explicit specialization in non-namespace scope" these must be implemented outside the class below
template <typename T>
struct ObjectInfoGetter : public ObjectInfoGetterBase
{
template <typename Map, typename MappedTypePtr = ConstCorrectMappedTypePtr<Map>>
MappedTypePtr operator()(format::HandleId id, Map* map)
{
return GetObjectInfoImpl<Map, MappedTypePtr>(id, map);
}
};

// Specialize to handle physical device aliasing. See comments for VulkanPhysicalDeviceInfo::vulkan_alias
// Note: could do SFINAE a "has member" check on vulkan_alias, but as only physical device needs aliasing support at
// this time, it's simpler just to specialize for VulkanPhysicalDeviceInfo
template <>
struct ObjectInfoGetter<VulkanPhysicalDeviceInfo> : public ObjectInfoGetterBase
{
template <typename Map, typename MappedTypePtr = ConstCorrectMappedTypePtr<Map>>
MappedTypePtr operator()(format::HandleId id, Map* map)
{
return GetAliasingObjectInfoImpl<Map, MappedTypePtr>(id, map);
}
};

class VulkanObjectInfoTableBase
{
protected:
Expand Down Expand Up @@ -132,41 +199,13 @@ class VulkanObjectInfoTableBase
template <typename T>
const T* GetVkObjectInfo(format::HandleId id, const std::unordered_map<format::HandleId, T>* map) const
{
assert(map != nullptr);

const T* object_info = nullptr;

if (id != 0)
{
const auto entry = map->find(id);

if (entry != map->end())
{
object_info = &entry->second;
}
}

return object_info;
return ObjectInfoGetter<T>()(id, map);
}

template <typename T>
T* GetVkObjectInfo(format::HandleId id, std::unordered_map<format::HandleId, T>* map)
{
assert(map != nullptr);

T* object_info = nullptr;

if (id != 0)
{
auto entry = map->find(id);

if (entry != map->end())
{
object_info = &entry->second;
}
}

return object_info;
return ObjectInfoGetter<T>()(id, map);
}
};

Expand Down
Loading
Loading