search

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.37k stars 244 forks source link

nvrhi rebase with ShaderMake support #857

Closed SRSaunders closed 2 months ago

SRSaunders commented 3 months ago

I just wanted to put this up before end of day for @RobertBeckebans. I had to merge your latest changes to master.

This addresses the RBDoom3BFG project part of #853.

The complete solution requires:

  1. Fast-forward nvrhi fork to head at https://github.com/NVIDIAGameWorks/nvrhi. This will require fixing at least one merge conflict, but not a big deal. I was able to complete this step pretty easily.
  2. git submodule update --init --recursive to make sure nvrhi submodule dependencies are updated
  3. Clone a new fork of https://github.com/NVIDIAGameWorks/ShaderMake into your GitHub repo
  4. git submodule add https://github.com/RobertBeckebans/ShaderMake.git neo/extern/ShaderMake
  5. Make sure your Windows SDK is at version 2104 (10.0.20348.0) or above. A patch may be needed for Windows 10, Windows 11 already satifies this. See https://developer.microsoft.com/en-us/windows/downloads/sdk-archive/

This should be enough for Windows. For macOS (possibly linux too), I have some additional commits required to address clang build errors for the nvrhi and ShaderMake projects (these clang changes are now merged into nVidia's nvrhi and ShaderMake main branches). I can push these if and when you decide to proceed, and once your nvrhi and ShaderMake repos can accept these PRs.

On the performance side I don't see much change. DX12 and Vulkan for Windows, Vulkan for linux and Vulkan for macOS all benchmark out to about the same frame rates before and after the rebase.

RobertBeckebans commented 2 months ago

I hope to get to this, this weekend.

SRSaunders commented 2 months ago

nvrhi recently updated their included Vulkan-Headers submodule to version 277, which unfortunately breaks the RBDoom3BFG build, plus any other build that uses cmake's find_package(Vulkan) and includes nvrhi. I have been working with @apanteleev on a fix (see https://github.com/NVIDIAGameWorks/nvrhi/issues/45). This is not completely resolved yet, but in the mean time you can use the attached patch file to update nvrhi's CMakeLists.txt file to temporarily correct the issue: nvrhi-cmakelists.patch

The good thing is that once resolved, it will finally correct the long standing issue with precompiled header version conflicts with vulkan.hpp includes. This means that DeviceManager_VK.cpp will no longer need special handling re precompiled headers for all platforms. UPDATE: I have gone ahead and pushed this change here since the remaining issue with nvrhi's use of Vulkan-Headers v277 is somewhat independent. Note the attached CMakeLists patch file is still required until such time as nVidia fixes the conflict with find_package(Vulkan).

Thanks to @apanteleev the Vulkan-Headers issue is now resolved and no workaround patch is needed when using the latest main at nvrhi.

RobertBeckebans commented 2 months ago

Oh well I rebased RobertBeckebans/nvrhi based on the newest Nvidia/nvrhi and merged your PR into master.

Can you rebase your macOS specific changes into a new PR for RobertBeckebans/nvrhi ?

SRSaunders commented 2 months ago

Thanks for doing the merge. Some of my changes were already merged into nvrhi and ShaderMake main with nVidia - I have closed those PRs. However, I would appreciate you merging the remaining rebased PRs at: https://github.com/RobertBeckebans/nvrhi/pulls

https://github.com/RobertBeckebans/nvrhi/pull/5 is generic, while https://github.com/RobertBeckebans/nvrhi/pull/6 and https://github.com/RobertBeckebans/nvrhi/pull/7 have macOS stuff.