Fix examples to compile in MSVC

[Admer456]

I recently cloned Vookoo to try it out and learn a little bit of Vulkan, and I had issues compiling it in Visual Studio 2022 / MSVC v143.

The first issue was that the examples were using designated initialisers; a C++20 feature, but their CMakeLists configured them to use C++11. After fixing that, there were a couple of errors with using designated initialisers themselves (error C7562: designated initialization can only be used to initialize aggregate class types). Finally, the multi-threaded example was doing the following:

  // define number of threads
  const uint32_t Nthreads = std::thread::hardware_concurrency();
  std::cout << "Nthreads = " << Nthreads << std::endl;

  // allocate array of threads to be reused from frame to frame
  std::thread v[Nthreads];

Arrays like these are not allowed in MSVC, so I've changed it to std::vector<std::thread> v( Nthreads );. After successful compilation, I tested all the examples on my main PC with an RTX 3060 and they work without any runtime errors, except for tripping some Vulkan validation layers (primarily about this one), but my assumption is that this is "normal".

[lhog]

  // define number of threads
  const uint32_t Nthreads = std::thread::hardware_concurrency();
  std::cout << "Nthreads = " << Nthreads << std::endl;

  // allocate array of threads to be reused from frame to frame
  std::thread v[Nthreads];

Doesn't look correct indeed, worth fixing for sure.

Designated initializers used to work on prev generations of MSVC certainly, but you're right they are cpp20 class semantics. I see you bumped up C++ standard version to 20, yet removed designated initializers. Why is that?

[Admer456]

Now that I think of it, it must be an accidental leftover. When I initially tried building the examples, as mentioned, they were in C++11, which didn't support designated initialisers. That was the reason I changed it to C++20.

Then I encountered a different error about certain classes not being aggregate types. Apparently, if a class has user-defined constructors, it may not be initialised with designated initialisers. That was the reason I ultimately removed them from the examples.

After that, I guess I simply forgot to change it back to C++11. I'll correct this tomorrow.

[Admer456]

I have just checked and setting it back to C++11 will result in errors. In helloTriangle.cpp, designated initialisers are used here:

  // This is our triangles.
  const std::vector<Vertex> vertices = {
    {.pos={ 0.5f,  0.5f}, .colour={0.0f, 1.0f, 0.0f}},
    {.pos={-0.5f,  0.5f}, .colour={0.0f, 0.0f, 1.0f}},
    {.pos={ 0.5f, -0.5f}, .colour={1.0f, 0.0f, 0.0f}},

    {.pos={ 0.5f, -0.5f}, .colour={1.0f, 0.0f, 0.0f}},
    {.pos={-0.5f,  0.5f}, .colour={0.0f, 0.0f, 1.0f}},
    {.pos={-0.5f, -0.5f}, .colour={0.0f, 0.0f, 0.0f}},
  };

Which results in: error C7555: use of designated initializers requires at least '/std:c++latest'

With C++11 on, I've tried building the same in MSVC v142 (2019) and I am getting the same error, meanwhile v141 (2017) did not recognise this style of initialisation, resulting in error C2059: syntax error: '.' and a dozen others. I haven't tested with v140 (2015) and earlier, but I am guessing it'd be similar. Only v142 and above support C++20.

Just in case, I've also tried building this on Linux in GCC 11. Originally, before my changes, it'd report designated initializers cannot be used with a non-aggregate type 'vku::Window'. With my changes, in both C++11 and C++20, it works.

In short, modern MSVC seems rather strict about the use of designated initialisers. Considering all of the above, I think it is alright to keep the C++ standard at 20. Alternatively, C++11 could be kept if designated initialisers are removed from every example.

[lhog]

In short, modern MSVC seems rather strict about the use of designated initialisers. Considering all of the above, I think it is alright to keep the C++ standard at 20.

I think so too.

[lhog]

Is this ready for merge? Not around the PC to test it.

[Admer456]

It should be. I'll test again tomorrow to make sure.

[Admer456]

Apologies for the delay. College can sometimes be a time sink.

I've tested it again, on Windows 10, compiled in x64 under MSVC v142 and v143.

Under MSVC v142, 13-crystalLogo crashes on GIMP_TEXT1_RUN_LENGTH_DECODE, but that is unrelated to Vookoo and my changes. Under MSVC v143, this crash does not occur. Maybe it's some sort of bug in MSVC.

Another, albeit non-critical thing I've noticed is that in Debug mode, in 12-renderToCubemapByMultiview, the background colour of each cubemap face is exactly as it is defined in code, i.e.: Meanwhile in Release mode, it's black: This could be a separate issue though, as it seems to be unrelated to my changes.

Other than that, I've observed no issues. This PR is ready to be merged.

[pocdn]

In short, modern MSVC seems rather strict about the use of designated initialisers. Considering all of the above, I think it is alright to keep the C++ standard at 20.

I think so too.

Since vookoo is now at cpp 20, could we re-enable all the designated initialisers. The above comment seems to say that MSVC (and gcc/c++20 on my windows/linux machines) all work fine with the prior initializers like

vku::Window window{ .instance = fw.instance(), .device = device, .physicalDevice = fw.physicalDevice(), .graphicsQueueFamilyIndex = fw.graphicsQueueFamilyIndex(), .window = glfwwindow };

compared to the present usage as vku::Window window{ fw.instance(), device, fw.physicalDevice(), fw.graphicsQueueFamilyIndex(), glfwwindow };

[lhog]

Oh well idk. We kinda switched, yet it's still possible to get back to c++17, which is still more commonly used than c++20.

[Admer456]

Note that even with designated initialisers enabled, I was getting the following error in MSVC: error C7562: designated initialization can only be used to initialize aggregate class types And the equivalent sort of thing in GCC.

According to cppreference, an aggregate class type is one which, among other things, doesn't have a user-defined constructor, something that is done in vku::Window for example.

Refer to 3) and 4) on this page: https://en.cppreference.com/w/cpp/language/aggregate_initialization, and the definition of an aggregate. It seems like it would require major changes to Vookoo if you wish to support designated initialisers.