vblanco20-1 / vkguide-comments

Storage for the comments of vulkan-guide
0 stars 0 forks source link

Chapter 1 Comments #2

Open utterances-bot opened 3 years ago

utterances-bot commented 3 years ago

Vulkan Initialization Code - Vulkan Guide

Brand new guide to vulkan graphics programming

https://www.vkguide.dev/docs/chapter-1/vulkan_init_code/

vblanco20-1 commented 3 years ago

test

zhihao95 commented 3 years ago

Hi, i think the code will throw here when it returns VK_NOT_READY: VK_CHECK(vkAcquireNextImageKHR(_device, _swapchain, 0, _presentSemaphore, nullptr, &swapchainImageIndex));

vblanco20-1 commented 3 years ago

@zhihao95 It does indeed return not-ready in some cases, but i dont have any of the hardware where it happens. I believe its an AMD issue.

galorojo commented 3 years ago

@vblanco20-1 I have an AMD card (RX 580 specifically) and I'm getting a VK_TIMEOUT result on the vkAcquireNextImageKHR call. Poking around a bit I deduced it might have to do with vsync? Setting the timeout on the vkAcquireNextImageKHR to something bigger than 17ms makes the issue go away (making it exactly 17ms seems to work until the application loses focus, which in my system drops the frame-rate a bit and I get a VK_TIMEOUT again).

Currently I just made the timeout std::numeric_limits<uint64_t>::max() so I can keep going with the tutorial.

Feel free to contact me if you'd like me to do any testing, want any other information, or know of a better way to fix the issue.

InsigMath commented 3 years ago

I got the expected result, i.e., the screen starting off blue and changing colour but in the debug console I get these errors:

[ERROR: Validation] Validation Error: [ VUID-vkDestroyDevice-device-00378 ] Object 0: handle = 0x20577dfae88, type = VK_OBJECT_TYPE_DEVICE; Object 1: handle = 0xc6ac6a000000000f, type = VK_OBJECT_TYPE_FENCE; | MessageID = 0x71500fba | OBJ ERROR : For VkDevice 0x20577dfae88[], VkFence 0xc6ac6a000000000f[] has not been destroyed. The Vulkan spec states: All child objects created on device must have been destroyed prior to destroying device (https://vulkan.lunarg.com/doc/view/1.2.154.1/windows/1.2-extensions/vkspec.html#VUID-vkDestroyDevice-device-00378)

I haven't just looked at the github repo to see if I am doing anything wrong yet but I am thinking maybe this is a cleanup issue, just wondering if anyone has had this issue?

vblanco20-1 commented 3 years ago

@insigMath At the moment, those validation errors are expected. Proper cleanup is only done by https://vkguide.dev/docs/chapter-2/cleanup/ chapter. You dont need to worry about those validation errors, as the vulkan driver will clean everything on app launch anyway. If you want to fix that one, you need to add VkDestroyFence to the cleanup function in the engine, making sure its called before VkDestroyDevice

vblanco20-1 commented 3 years ago

The chapter has been updated adding a timeout of 1 second to swapchain acquire image. It should now work well for some AMD drivers where it had issues before.

David-DiGioia commented 3 years ago

its best if the framerate is capped and not reaching 5000 FPS which can cause issues.

What types of issues could this cause?

vblanco20-1 commented 3 years ago

@David-DiGioia Some gpus can overheat when running at thousands of fps in that way, specially integrated ones and laptop ones. Its also a bit of a energy waste to put the gpu to loop so much

kvatigabi commented 3 years ago

why do we use underscore in VkInstance for example?

neverhood311 commented 3 years ago

When I run the code, I get this error right off the bat:

[ERROR: General]
loader_get_json: Failed to open JSON file C:\Program Files\NVIDIA Corporation\Nsight Systems 2019.5.2\Target-Windows\x86_64\VkLayers\VkLayer_nsight-sys_windows.json

I found that .json file, but it's in a slightly different location. Is that expected?

alexpanter commented 3 years ago

MAILBOX will use significantly more power, so it's recommended to stay away from that when developing more mobile devices! MAILBOX is suitable, though, when developing fast-paced interactive programs where user input latency is a concern.

Hochheilige commented 3 years ago

Hi I've got a question about validation layers and debug messages. Is this okay that when I run program all information about all vulkan functions I used out to the console? It slow the application starting and I can't understand is that okay or not.

alexpanter commented 3 years ago

@Hochheilige If you didn't already, you should probably disabled the VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT flag from your VkDebugUtilsMessengerCreateInfoEXT struct. That will limit verbosity of debug messages (ie. print less). Otherwise you should be more specific.

Hochheilige commented 3 years ago

@alexpanter Thanks, but I can't clearly understand how to disable this with vkBootstrap library using in this guide. I tried something like this

auto instRet = builder
                .set_app_name("Example Vulkan Application")
        .request_validation_layers(true)
        .require_api_version(1, 1, 0)
            .set_debug_messenger_severity(VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT)
        .build();

And result unfortunately still the same

charles-lunarg commented 3 years ago

@Hochheilige because use_debug_messenger() was never called, I think you are getting the output put to stdout rather than through the not-set-up debug messenger. But it sounds like you have API dump enabled based on the description of all information about all vulkan functions I used which is a completely separate thing than validation layers.

dylan-conway commented 3 years ago

If I use the flash variable in the clear color my screen will not flash blue and will only be black. If I change the flash variable to 1.0f then the screen will be solid blue. Any idea why this is happening? Maybe I missed a setting for colors changing from the clear color?

dylan-conway commented 3 years ago

Figured it out. I was using the wrong abs() function

dortamiguel commented 2 years ago

Why VK_CHECK is a macro if it can be a function?

alexpanter commented 2 years ago

@ellipticaldoor Because that way it can easily be customized/disabled for debug or release builds. This is a very common approach. A function would always be called, but a macro can be trimmed away from the executable.

patrick-han commented 2 years ago

Visual Studio throws "Access violation executing location" right after SDL_DestroyWindow(_window) in cleanup() sometimes. I've localized it to this because commenting out the destroy function consistently prevents this exception from throwing.

And I emphasize sometimes because it really does only happen sometimes when I close the window... which is why I am very confused. Any ideas?

Kaminate commented 2 years ago

And what's stopping VK_CHECK from being trimmed away if it was a function?

alexpanter commented 2 years ago

Maybe if the compiler can detect it isn't being used or its function body is empty. But this might depend on optimization flags for compiler or linker. Macro functions can be trimmed away even before reaching the compilation stage.. but is this a trick question?

jlakin67 commented 2 years ago

I'm not sure what you mean by "list of images" for your explanation for VK_PRESENT_MODE_MAILBOX_KHR. There's only a single-entry queue and a presentation request replaces anything in it.

Lucodivo commented 2 years ago

Why do we hold onto the vector of swapchain images when the imageViews seem to be the thing we actually need for framebuffer creation? Is it just that we may want to use them to create more imageViews in the future?

cdgiessen commented 2 years ago

Why do we hold onto the vector of swapchain images when the imageViews seem to be the thing we actually need for framebuffer creation? Is it just that we may want to use them to create more imageViews in the future?

While the code doesn't use the images right now, they don't take up much space (just a few handles) and are necessary for advanced features not covered in the API. It copying the image to a file for screenshots or using them in some other capacity.

Catddly commented 2 years ago

nice article

Abdullah-Mert commented 1 year ago

After compiling chapter 0 through cmake and opening the project.sln in visual studio I am getting multiple errors regarding SDL, like "cannot open source file SDL.h and vulkan/vulkan.h", "identifier SDL_init is undefined" etc. CMake also throws an error while generating the files:

"CMake Warning (dev) in CMakeLists.txt: No project() command is present. The top-level CMakeLists.txt file must contain a literal, direct call to the project() command."

I can very well keep building on the starting point though so it is fine, but I still would like to know what the problem is

yelibumeng commented 1 year ago

Why do all calls to functions beginning with vkXXX report an error

cdgiessen commented 1 year ago

@yelibumeng because Vulkan is a C API, and error codes are the primary error reporting mechanism in C API's.

monde-lointain commented 1 year ago

Anyone having trouble with the timeout period expiring still, you can lock it in a do-while loop to ensure that it waits the required amount of time.

// Wait until the GPU has finished rendering, looping in case it takes longer
// than expected
VkResult result;
do
{
    result = vkWaitForFences(_device, 1, &_renderFence, true, 1000000000);
} while (result == VK_TIMEOUT);
rafaelbeckel commented 1 year ago

For MacOS, you have to enable VK_KHR_portability_enumeration extension in your device creation. The version of VkBootstrap included in this tutorial as the time of my comment do not support this extension. The most recent version handles it automatically: https://github.com/charles-lunarg/vk-bootstrap

You can just copy+paste it and replace the contents of your third_party/vkbootstrap directory.

kepler-5 commented 8 months ago

Instead of abstracting the CreateInfo spam into vk_initializers.h by hand, for those willing to go more "off-book" I'd recommend trying out the Vulkan C++ API from Khronos. It provides "builder"-like versions of all the CreateInfo structs (on top of setting sType automatically etc.) which eliminates a TON of hassle and makes your code way shorter (and more type-safe). Also, the C++ API provides RAII-style handles (UniqueHandle) that clean up resources automatically when they're destroyed, so you get to not worry about all the cleanup boilerplate as long as your class members are declared in the correct order.

https://github.com/KhronosGroup/Vulkan-Hpp

vblanco20-1 commented 8 months ago

@kepler-5 Vulkan hpp was considered for the tutorial, but it ended up being a bad choice. The header has some deep issues and ruins compile times and intellisense. While creating the initializers is a bit annoying, you can use them to abstract at a slightly higher level, and then you avoid the bloat problems of vulkan.hpp . vulkan.hpp was alright when it was first released, as it was mostly the classes and enums and little more. The bloat has gotten completely out of control since then.

You would still need to create something similar to the initializers even with vulkan.hpp but now you also have multiple seconds extra compile time per file and your autocomplete system ruined. We are talking about 100.000+ lines of header code. Its bad enough that ive gone through the effort of refactoring entire engines away from it due to how badly it works. I did that after vulkan.hpp broke the code for the 3rd time in that project after updating the Vulkan SDK.

The RAII handles should be avoided. You want to hook cleanup functions into a destruction queue that can guarantee the resources arent used by the GPU like the tutorial does, or manage resource lifetime in some other related way. The raii handles in vulkan.hpp dont give you that, and they are a quick path into crashes.

charles-lunarg commented 8 months ago

If you want struct initializers, I recently worked on making vk_struct_helper.hpp available in the (new) Vulkan-Utility-Libraries repository.

https://github.com/KhronosGroup/Vulkan-Utility-Libraries/blob/main/include/vulkan/utility/vk_struct_helper.hpp

The header provides a mechanism to zero init & setup the correct sType. It works by deducing the type based on the value being assigned. For example:

VkDeviceCreateInfo device_create_info = vku::InitStructHelper();

I plan on adding more utilities to this repo over time - trying to solve some of the things vulkan.hpp has without the bloat. Its by no means perfect, but feel free to try out and see how it works for your use case.

kepler-5 commented 8 months ago

@vblanco20-1 @charles-lunarg Thanks for the feedback! I can definitely see how the massive size of the header would cause problems like you mentioned, especially as your engine grows. As for the RAII stuff, I guess I still don't see why you can't express destruction order, even when it's subtle, through object lifetimes. If you need a deferred destruction queue for example, it seems like you could just std::move your handles into a vector, and simply .clear() the vector later when it's safe and have the destructors free the actual resources. So the lifetimes of the Vulkan objects are 1:1 with their handles' lifetimes. And when storing them as long-lived interrelated class members it prevents you from having to meticulously free them in the reverse order you create them.

Anyway I probably have a lot more to learn and will possibly see the light at some point. :) And I've really been loving the tutorial so far, thank you sincerely.

alexpanter commented 8 months ago

@kepler-5 one of my main problems with introducing object-oriented patterns on top of vulkan types is that it sometimes screws with the manual allocation schemes which is an inherent part of using vulkan.

Smart-pointer types can be used to some extent, but introduce problems when transferring ownership of vulkan handles. For example, if an image is created as part of a framebuffer object, if RAII is forced, then ownership of the image cannot be transferred to another object as it gets destroyed by destructor/out-of-scope. This can be mitigated by additional logic, but gets messy fast.

For my own custom vulkan library (cannot share code atm), we do:

struct device; // opaque handle, defined in source file
device* dev = device_create(...);
// do stuff
device_destroy(dev);

I know people often get afraid of / hesitant to use (raw) pointers, but having tried different ways of building vulkan libraries (at least 3 of them!), I personally prefer this approach the most.

‐-----------

I see other more subtle problems like, a default-created class (empty constructor) may be interpreted as "valid"/"not valid", which often inspires the addition of an "isValid()" member function, and checks inside its other member functions. So using a class means that we cannot place objects inside an array without constructing them (well we can, but it's not pretty), and then we break with RAII because an object must always be valid when constructed. A raw pointer does not have this restriction - it can be null, but we check for that once, upon creation.

I realize this is certainly opinionated, but so are a lot of things in the world. I also don't use vulkan.hpp because it's an extra layer of abstraction and makes reading the spec more painful. Besides, the C api is pretty good and well-structured.

And just because we use C++, doesn't mean object-oriented programming is our only option.

reinderhe commented 6 months ago

I keep getting this error whenever I finish chapter one. What am I doing wrong here?

Exception thrown at 0x00007FFEC8876E1C (nvoglv64.dll) in vulkan_guide.exe: 0xC0000005: Access violation reading location 0x00000000000000F0.

Unhandled exception at 0x00007FFEC8876E1C (nvoglv64.dll) in vulkan_guide.exe: 0xC0000005: Access violation reading location 0x00000000000000F0.

The program '[19968] vulkan_guide.exe' has exited with code 0 (0x0).

alexpanter commented 6 months ago

@reinderhe it sounds like the program throws an exception. You should try running the application through a debugger, e.g. in Visual Studio.

reinderhe commented 6 months ago

@alexpanter Thanks for the quick reply. I did run it through VS2022. And figured out quickly that I had the initialization order wrong in the init() function. Now my screen flashes blue and its launching correctly. It was trying to grab an empty fence.