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

Windows/Vulkan build fix #469

Merged
merged 1 commit into from Aug 18, 2022
Merged

Windows/Vulkan build fix #469

merged 1 commit into from Aug 18, 2022

Conversation

stanciuadrian
Copy link
Contributor

I've tested these changes on a clean MSYS2 install.

The source build now requires the Vulkan SDK Core to be preinstalled before starting the MSYS2 environment. This ensures the VK_SDK_PATH and VULKAN_SDK environment variables are detected by the include(FindVulkan) step from CMakeLists.txt.

Right now the build fails with:

C:/msys64/home/adi/scopehal-apps/lib/scopehal/AcceleratorBuffer.h: In member function 'void AcceleratorBuffer<T>::FreeCpuPointer(T*, MemoryType, size_t)':
C:/msys64/home/adi/scopehal-apps/lib/scopehal/AcceleratorBuffer.h:585:39: error: 'm_tempFileHandle' was not declared in this scope
  585 |                                 close(m_tempFileHandle);
      |                                       ^~~~~~~~~~~~~~~~

To be continued...

@azonenberg
Copy link
Collaborator

m_tempFileHandle is for memory mapped files on Linux and should be gated by a #ifndef _WIN32. If I failed to do that, it's on me. Don't touch that code, I have a bunch of un-pushed changes to it and will fix as part of that later today. For your own testing purposes you can just comment out that line, AcceleratorBuffer is not yet used for anything other than a unit test.

We need to figure out how to get the Vulkan SDK into the CI environment so that the GitHub Actions builder can use it. Any ideas there?

@stanciuadrian
Copy link
Contributor Author

Thanks for the tip. After adding the _WIN32 guard, the build finishes with success:

[1/7] Copying rendering kernels...
[2/7] Copying styles...
[3/7] Copying filter kernels...
[4/7] Copying gradients...
[5/7] Copying shaders...
[6/7] Copying icons...
[6/7] Install the project...
==> Tidying install...
  -> Removing libtool files...
  -> Purging unwanted files...
  -> Stripping unneeded symbols from binaries and libraries...
  -> Compressing man and info pages...
==> Checking for packaging issues...
==> Creating package "mingw-w64-x86_64-scopehal-apps"...
  -> Generating .PKGINFO file...
  -> Generating .BUILDINFO file...
  -> Generating .MTREE file...
  -> Compressing package...
==> Finished making: mingw-w64-scopehal-apps 0.0.0.r1.g3e91713-1 (Tue Aug 16 01:53:15 2022)

Regarding the CI, the Vulkan SDK setup must be run from an admin session as documented here in the Unattended Installation section. The command

VulkanSDK-1.3.216.0-Installer.exe --accept-licenses --default-answer --confirm-command install

successfully installs the SDK unless the target directory (C:/VulkanSDK/1.3.216.0 in my case) already exists. You can override it with the --root parameter.

@azonenberg
Copy link
Collaborator

Nice! Can you try to modify the CI script (in the .github directory) and add commands to download and install the SDK? The CI builder is running with UAC disabled in an account that has admin access.

Also, can you please send a companion PR to scopehal-docs adding the necessary install commands to the "Getting Started" section?

@azonenberg azonenberg merged commit 7e600ce into ngscopeclient:master Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants