ngscopeclient / scopehal-apps

ngscopeclient and other client applications for libscopehal.
https://www.ngscopeclient.org/
BSD 3-Clause "New" or "Revised" License
608 stars 100 forks source link

Windows/Vulkan build fix #469

Closed stanciuadrian closed 2 years ago

stanciuadrian commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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?