RobertBeckebans / RBDOOM-3-BFG

Doom 3 BFG Edition source port with updated DX12 / Vulkan renderer and modern game engine features
https://www.moddb.com/mods/rbdoom-3-bfg
GNU General Public License v3.0
1.38k stars 247 forks source link

Support Optick with Vulkan and label Optick Vsync queue with frame ID #780

Closed SRSaunders closed 10 months ago

SRSaunders commented 12 months ago

This pull request adds 2 main items:

  1. Adds support for Optick profiling with the Vulkan renderer on all platforms: Windows, linux, macOS (solves #766)
  2. Adds support for labeling the Optick Vsync/Present queue with frame ID information for DX12 and Vulkan*

* _requires the Vulkan VK_GOOGLE_display_timing extension - currently supported on macOS with MoltenVK, too bad this is not available generally on Vulkan for Windows and linux, at least for my AMD graphics card. Note the MoltenVK implementation has bug fixes for improved presentation timing in the coming Vulkan SDK 1.3.250 and merged pull request https://github.com/KhronosGroup/MoltenVK/pull/1936 (pending MVK 1.2.5 release, date TBD)._

Item 2 above is cool since it allows direct visualization of game input to display presentation latency end-to-end. I have compared the DX12 results with PresentMon (https://github.com/GameTechDev/PresentMon) on Windows and they line up very well. I will use this capability in a follow-up PR regarding latency optimization for RBDoom3BFG.

To achieve the above, the Optick profiler was bug-fixed / extended as follows:

  1. Fixed threadTLS nullptr check (now works cross platform)
  2. Fixed OPTICK_SET_MEMORY_ALLOCATOR() stub definition
  3. Improved accuracy of GPU to CPU timestamp conversion by using floating point doubles
  4. Improved Vulkan GPU to CPU clock offset calibration by leveraging Vulkan Events (thanks to cdwfs for concept at https://gist.github.com/cdwfs/4222ca09cb259f8dd50f7f2cf7d09179)
  5. Changed vkCmdResetQueryPool() to vkResetQueryPool() in GPUProfilerVulkan::ResolveTimestamps() to avoid Vulkan validation errors
  6. Added proper index, query pool, and present stats initialization for each Optick capture run (Vulkan and DX12)
  7. Resolved delayed GPU frame timestamps before dumping data by extending ResolveTimestamps() and WaitForFrame() with node visibility and making their interfaces public
  8. Added mutex lock to GPUProfiler::Dump() to prevent random failures during data dump (stop capture)
  9. Extended GPUProfiler::AddVSyncEvent() to support an optional text label (vs hardcoded VSync label)
  10. Added GPUProfiler::AddVSyncTag() to allow labeling of Vsync event with frame ID information
  11. Changed Platform::GetTime() to use Monotonic clock on macOS - now the same as other platforms and works with data returned by vkGetPastPresentationTimingGOOGLE()
  12. Added support for vkGetPastPresentationTimingGOOGLE() if VK_GOOGLE_display_timing available
  13. Added support for DX12 presentation ID to frame ID mapping to permit VSync queue frame labeling
  14. Changed OPTICK_GPU_FLIP() to support optional frameID parameter (needed for DX12 presentID to frameID mapping for VSync queue labeling, not required for Vulkan)
  15. Added typeless void* function prototype to GPUContextScope() for runtime selection of graphics API

I kept all the Optick-specific commits separate from RBDoom3BFG commits to allow potential future pushing of these changes to the upstream Optick repo.

This has been tested on Windows 10, Manjaro linux, and macOS Monterey, using the Optick GUI app running on a separate, networked PC.

Here's a screen grab for borderless fullscreen Vulkan running on macOS/MoltenVK with VSync/Present queue frameID labeling: macOS Vulkan Fullscreen Present

Here's another screen grab for borderless fullscreen DX12 on Windows with VSync/Present queue frameID labeling: Windows DX12 Fullscreen Present Buffers=2

SRSaunders commented 10 months ago

Added a few minor fixes for:

  1. Using gcc compiler with Optick (i.e. support PCH and gnu extensions for ##__VA_ARGS__ in Optick macros)
  2. MSVC warnings re type mismatches in Optick GPUProfiler::Stop()
RobertBeckebans commented 10 months ago

This is a big one. Good job! I only tested DX12 vs Vulkan on Windows. You might also want to make a PR to the original Optick repo because many might be interested in your changes.

SRSaunders commented 10 months ago

Thanks Robert. I will likely prepare a PR for the original Optick repo, but I notice that no commits have been applied for over 1 year. I wonder if the developer is still active, or perhaps just busy on other things?