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

Add used resolutions info #1167

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions framework/decode/vulkan_stats_consumer.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,14 @@
#include "generated/generated_vulkan_struct_decoders.h"
#include "generated/generated_vulkan_consumer.h"
#include "util/defines.h"
#include "vulkan/vulkan.hpp"
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I didn't catch this on first pass. This would be the first inclusion of VulkanHpp and I want to be a little more deliberate about adding it. Can you just use VkExtent2D?

Copy link
Contributor

Choose a reason for hiding this comment

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

@bartosz-muszarski-arm if you haven't done this already I see how it smooths out the feature you added and I'm not in principle against it. I'm doing a straw poll to see if anyone has any issues with including VulkanHpp.

Copy link
Contributor

@andrew-lunarg andrew-lunarg Jul 17, 2023

Choose a reason for hiding this comment

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

Could we at least reduce this to just including the structs header and not taking in the entire C++ wrapper?

#include <vulkan/vulkan_structs.hpp>

EDIT: vulkan_hash.hpp included on the next line includes vulkan.hpp anyway...

Copy link
Contributor

Choose a reason for hiding this comment

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

And also turn off every feature we don't need in the headers that has a macro that defaults to enabled. e.g. in our CMake configs for all projects define VULKAN_HPP_NO_STRUCT_SETTERS. We can then turn stuff on when we need it, where we need it.

#include "vulkan/vulkan_hash.hpp"

#include "vulkan/vulkan.h"

#include <set>
#include <unordered_map>
#include <unordered_set>

GFXRECON_BEGIN_NAMESPACE(gfxrecon)
GFXRECON_BEGIN_NAMESPACE(decode)
Expand All @@ -57,6 +60,7 @@ class VulkanStatsConsumer : public gfxrecon::decode::VulkanConsumer, public gfxr
uint64_t GetMaxAllocationSize() const { return max_allocation_size_; }
uint64_t GetAnnotationCount() const { return annotation_count_; }
const std::vector<std::string>& GetOperationAnnotationDatas() const { return operation_annotation_datas_; }
const auto& GetResolutions() const { return resolutions_; }
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of auto here where it forces the future contributors to scroll down the file to know what the function they might want to call actually returns (std::unordered_set<vk::Extent2D>).


const std::set<gfxrecon::format::HandleId>& GetInstantiatedDevices() const { return used_physical_devices_; }
const VkPhysicalDeviceProperties* GetDeviceProperties(gfxrecon::format::HandleId id) const
Expand Down Expand Up @@ -408,6 +412,40 @@ class VulkanStatsConsumer : public gfxrecon::decode::VulkanConsumer, public gfxr
}
}

virtual void Process_vkCreateSwapchainKHR(
const gfxrecon::decode::ApiCallInfo& call_info,
VkResult returnValue,
gfxrecon::format::HandleId device,
gfxrecon::decode::StructPointerDecoder<gfxrecon::decode::Decoded_VkSwapchainCreateInfoKHR>* pCreateInfo,
gfxrecon::decode::StructPointerDecoder<gfxrecon::decode::Decoded_VkAllocationCallbacks>* pAllocator,
gfxrecon::decode::HandlePointerDecoder<VkSwapchainKHR>* pSwapchain)
{
if (!pCreateInfo->IsNull())
{
const auto& extent = pCreateInfo->GetPointer()->imageExtent;
resolutions_.insert(extent);
}
}

virtual void
Process_vkCreateSharedSwapchainsKHR(const ApiCallInfo& call_info,
VkResult returnValue,
format::HandleId device,
uint32_t swapchainCount,
StructPointerDecoder<Decoded_VkSwapchainCreateInfoKHR>* pCreateInfos,
StructPointerDecoder<Decoded_VkAllocationCallbacks>* pAllocator,
HandlePointerDecoder<VkSwapchainKHR>* pSwapchains)
{
if (!pCreateInfos->IsNull())
{
for (uint32_t i = 0; i < swapchainCount; ++i)
{
const auto& extent = pCreateInfos->GetPointer()[i].imageExtent;
resolutions_.insert(extent);
}
}
}

private:
uint32_t trimmed_frame_{ 0 };

Expand Down Expand Up @@ -438,6 +476,8 @@ class VulkanStatsConsumer : public gfxrecon::decode::VulkanConsumer, public gfxr
// Annotation info.
std::vector<std::string> operation_annotation_datas_;
uint64_t annotation_count_{ 0 };

std::unordered_set<vk::Extent2D> resolutions_;
};

GFXRECON_END_NAMESPACE(decode)
Expand Down
10 changes: 10 additions & 0 deletions tools/info/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,16 @@ void PrintVulkanStats(const gfxrecon::decode::VulkanStatsConsumer& vulkan_stats_
GFXRECON_WRITE_CONSOLE("\tEngine version: %u", vulkan_stats_consumer.GetEngineVersion());
GFXRECON_WRITE_CONSOLE("\tTarget API version: %u (%s)", api_version, GetVersionString(api_version).c_str());

if (!vulkan_stats_consumer.GetResolutions().empty())
{
std::string resolutions = "\tUsed resolutions: ";
for (const auto& resolution : vulkan_stats_consumer.GetResolutions())
{
resolutions += std::to_string(resolution.width) + "x" + std::to_string(resolution.height) + " ";
}
GFXRECON_WRITE_CONSOLE(resolutions.c_str());
}

// Properties for physical devices used to create logical devices.
std::vector<const VkPhysicalDeviceProperties*> used_device_properties;
auto used_devices = vulkan_stats_consumer.GetInstantiatedDevices();
Expand Down