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

Fix Windows build with /p:CharacterSet=Unicode #88

Merged

Conversation

mikes-lunarg
Copy link
Contributor

Use TCHAR/TEXT macros for strings when interfacing with the Win32 API. Use native Win32 GetFileAttributes call to check if a file exists. Pass the layer settings file path around as a std::filesystem::path to avoid the need for an encoding conversion on Windows.

This fixes a downstream compile error on windows when building with MSVC and /p:CharacterSet=Unicode

FAILED: obj/third_party/vulkan-deps/vulkan-utility-libraries/src/vulkan_layer_settings/layer_settings_manager.obj
ninja -t msvc -e environment.x64 -- "C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.36.32532\bin\Hostx64\x64\cl.exe" /c ../../third_party/vulkan-deps/vulkan-utility-libraries/src/src/layer/layer_settings_manager.cpp /Foobj/third_party/vulkan-deps/vulkan-utility-libraries/src/vulkan_layer_settings/layer_settings_manager.obj /nologo /showIncludes -DDCHECK_ALWAYS_ON=1 -DUSE_AURA=1 -D_CRT_NONSTDC_NO_WARNINGS -D_WINSOCK_DEPRECATED_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -DCOMPONENT_BUILD -D__STD_C -D_CRT_RAND_S -D_CRT_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_DEPRECATE -D_ATL_NO_OPENGL -D_WINDOWS -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -DPSAPI_VERSION=2 -DWIN32 -D_SECURE_ATL -DWINAPI_FAMILY=WINAPI_FAMILY_DESKTOP_APP -DWIN32_LEAN_AND_MEAN -DNOMINMAX -D_UNICODE -DUNICODE -DNTDDI_VERSION=NTDDI_WIN10_NI -D_WIN32_WINNT=0x0A00 -DWINVER=0x0A00 -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -D_HAS_ITERATOR_DEBUGGING=0 -DVK_USE_PLATFORM_WIN32_KHR -I../.. -Igen -I../../third_party/vulkan-deps/vulkan-utility-libraries/src/include -I../../third_party/vulkan-deps/vulkan-headers/src/include /WX /wd4244 /Gy /FS /bigobj /utf-8 /Zc:sizedDealloc- /wd4117 /D__DATE__= /D__TIME__= /D__TIMESTAMP__= /Od /Ob0 /GF /Zi /MDd /std:c++20 /TP /GR- /Fd"obj/third_party/vulkan-deps/vulkan-utility-libraries/src/vulkan_layer_settings_cc.pdb"
../../third_party/vulkan-deps/vulkan-utility-libraries/src/src/layer/layer_settings_manager.cpp(148): error C2664: 'LSTATUS RegOpenKeyExW(HKEY,LPCWSTR,DWORD,REGSAM,PHKEY)': cannot convert argument 2 from 'const char [33]' to 'LPCWSTR'
../../third_party/vulkan-deps/vulkan-utility-libraries/src/src/layer/layer_settings_manager.cpp(148): note: Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or parenthesized function-style cast
C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\um\winreg.h(780): note: see declaration of 'RegOpenKeyExW'
../../third_party/vulkan-deps/vulkan-utility-libraries/src/src/layer/layer_settings_manager.cpp(148): note: while trying to match the argument list '(const _Ty, const char [33], int, long, HKEY *)'
        with
        [
            _Ty=HKEY
        ]
../../third_party/vulkan-deps/vulkan-utility-libraries/src/src/layer/layer_settings_manager.cpp(152): error C2664: 'LSTATUS RegEnumValueW(HKEY,DWORD,LPWSTR,LPDWORD,LPDWORD,LPDWORD,LPBYTE,LPDWORD)': cannot convert argument 3 from 'char [2048]' to 'LPWSTR'
../../third_party/vulkan-deps/vulkan-utility-libraries/src/src/layer/layer_settings_manager.cpp(152): note: Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or parenthesized function-style cast
C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\um\winreg.h(665): note: see declaration of 'RegEnumValueW'
../../third_party/vulkan-deps/vulkan-utility-libraries/src/src/layer/layer_settings_manager.cpp(152): note: while trying to match the argument list '(HKEY, DWORD, char [2048], DWORD *, nullptr, DWORD *, LPBYTE, DWORD *)'

Use TCHAR/TEXT macros for strings when interfacing with the Win32 API.
Use native Win32 GetFileAttributes call to check if a file exists. Pass
the layer settings file path around as a std::filesystem::path to avoid
the need for an encoding conversion on Windows.
Copy link
Contributor

@juan-lunarg juan-lunarg left a comment

Choose a reason for hiding this comment

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

LGTM but I'd wait for @christophe-lunarg's approval as well since they wrote the code in question

@mikes-lunarg
Copy link
Contributor Author

Another option: If we don't care about unicode file paths on windows then the fix could be much simpler:

diff --git a/src/layer/layer_settings_manager.cpp b/src/layer/layer_settings_manager.cpp
index fc932c0..d8aeeee 100644
--- a/src/layer/layer_settings_manager.cpp
+++ b/src/layer/layer_settings_manager.cpp
@@ -145,12 +145,12 @@ std::string LayerSettings::FindSettingsFile() {
     const size_t hives_to_check_count = IsHighIntegrity() ? 1 : hives.size();  // Admin checks only the default hive

     for (size_t hive_index = 0; hive_index < hives_to_check_count; ++hive_index) {
-        LSTATUS err = RegOpenKeyEx(hives[hive_index], "Software\\Khronos\\Vulkan\\Settings", 0, KEY_READ, &key);
+        LSTATUS err = RegOpenKeyExA(hives[hive_index], "Software\\Khronos\\Vulkan\\Settings", 0, KEY_READ, &key);
         if (err == ERROR_SUCCESS) {
             char name[2048];
             DWORD i = 0, name_size, type, pValues, value_size;
-            while (ERROR_SUCCESS == RegEnumValue(key, i++, name, &(name_size = sizeof(name)), nullptr, &type,
-                                                 reinterpret_cast<LPBYTE>(&pValues), &(value_size = sizeof(pValues)))) {
+            while (ERROR_SUCCESS == RegEnumValueA(key, i++, name, &(name_size = sizeof(name)), nullptr, &type,
+                                                  reinterpret_cast<LPBYTE>(&pValues), &(value_size = sizeof(pValues)))) {
                 // Check if the registry entry is a dword with a value of zero
                 if (type != REG_DWORD || pValues != 0) {
                     continue;

@christophe-lunarg christophe-lunarg self-assigned this Aug 31, 2023
@christophe-lunarg christophe-lunarg added enhancement New feature or request P1 labels Aug 31, 2023
@christophe-lunarg christophe-lunarg merged commit dd26ae7 into KhronosGroup:main Aug 31, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants