KhronosGroup / Vulkan-Loader

Vulkan Loader
https://vulkan.lunarg.com/doc/sdk/latest/linux/LoaderInterfaceArchitecture.html
Other
509 stars 278 forks source link

Vulkan-Loader/Loader/Loader.rc lists dll as dev build #1571

Open sushraja-msft opened 6 days ago

sushraja-msft commented 6 days ago

This may sound like a nit but we are trying to understand the significance of vulkan loader dll naming in Chromium browsers. In loader.rc the VER_FILE_DESCRIPTION_STR reads as .Dev build, is there a reason to call vulkan loader dev build instead of just "Vulkan Loader".

Environment (please complete the following information):

To Reproduce Steps to reproduce the behavior:

  1. On any windows 10/11 machine navigate to C:\Program Files (x86)\Microsoft\Edge\Application\129.0.2792.89. Version number would vary based on the version of Edge installed. This is essentially the path to the binary files for stable Edge.
  2. Right click on vulkan-1.dll and view its properties
  3. File Description reads as "1.3.293.dev" build for stable versions of any Chromium based browser.

Is it reasonable to rename these strings ?

charles-lunarg commented 6 days ago

Less of a bug, and more of a 'missing plumbing". The loader.rc file contains defaults meant to indicate that a development build is occurring. When a release build is performed (which isn't a release mode build, but rather a full Windows Runtime Installer package build that LunarG performs in the SDK build process), there is CMake code which reads an appropriately set CMake cache variable and modifies the .rc file accordingly. https://github.com/KhronosGroup/Vulkan-Loader/blob/1a337fe32d4d5be2ec2af7e02647005aeb358faa/loader/CMakeLists.txt#L39

The Chromium build uses gn rather than CMake, so is missing the equivalent logic. Additionally, the build scripts which invoke the loader's gn script would need to provide the "correct" version. https://github.com/KhronosGroup/Vulkan-Loader/blob/main/BUILD.gn

It wouldn't be too difficult to modify the gn script to modify the rc file as necessary for 'release' builds.

sushraja-msft commented 6 days ago

Thanks for the prompt reply. Thoughts on how build.gn can determine the "correct" version and if it is in a "release" build?

Since chromium browsers are just using VER_FILE_VERSION as the version today, a simple step of having the build.gn modify

define VER_FILE_DESCRIPTION_STR "1.3.298.Dev Build"

define VER_FILE_VERSION_STR "Vulkan Loader - Dev Build"

to

define VER_FILE_DESCRIPTION_STR "1.3.298.0 Build"

define VER_FILE_VERSION_STR "Vulkan Loader"

in a chromium is_official_build feels like an appropriate fix to me, WDYT ?

charles-lunarg commented 6 days ago

I think that is an appropriate change. Unfortunately, I do not know GN well enough to implement the changes, as that has always been the purview of chromium engineers.