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

Status of glad2 #208

Closed
Zingam opened this issue Apr 30, 2019 · 15 comments
Closed

Status of glad2 #208

Zingam opened this issue Apr 30, 2019 · 15 comments

Comments

@Zingam
Copy link

Zingam commented Apr 30, 2019

What is the status of glad2? I've been using the OpenGL part for a while and I haven't experienced any issues yet.

What is the situation with the vulkan part? Does it work? There isn't any documentation yet. For example what does the " loader Include internal loaders for APIs" do and when does it do anything? I guess it is the same as in GL? The service may use an interface that is relevant to the corresponding API.

IMHO it would make sense to put all the platform headers in a subdirectory to not the polute the include intellisense functionality. e.g.
include/glad/KHR/khrplatform.h
include/glad/KHR/vk_platform.h
include/glad/gl.h
include/glad/vulkan.h

@Dav1dde
Copy link
Owner

Dav1dde commented Apr 30, 2019

glad2 is basically done, there is some cleanup left, I wanna finish the documentation and improve the web loader first before updating all the links in this repo.

Vulkan works (at least with my tests).

The Include internal loaders for APIs option is like the old Loader option, it includes code to load all required symbols from the most common drivers (but usually you want to use your windowing library instead, like glfw).

Regarding the includes, I don't wanna do that because those headers are not generated by glad and may also be provided by the system. If you want to get rid of them you can use the header only build which embeds them in the glad header.

@Zingam
Copy link
Author

Zingam commented May 5, 2019

Could you please update the APIs in the web service? The Vulkan API seems to be quite old.

@Dav1dde
Copy link
Owner

Dav1dde commented May 5, 2019

It is 5 days old, the last change to the specification was on April 19th. What are you missing, maybe something else is causing your problem.

@Zingam
Copy link
Author

Zingam commented May 5, 2019

In vulkan.h:
#define VK_HEADER_VERSION 86
and here:
https://github.com/KhronosGroup/Vulkan-Headers/blob/5f1ceaad4c22d08f4fcac9db3046afbd64358249/include/vulkan/vulkan_core.h#L46
it is:
#define VK_HEADER_VERSION 107
I generated the loader a few days ago.

@Dav1dde
Copy link
Owner

Dav1dde commented May 5, 2019

I cannot reproduce this, maybe you used the --reproducible option?

image

@Zingam
Copy link
Author

Zingam commented May 5, 2019

I regenerated the loader and now it is:
#define VK_HEADER_VERSION 107

This is also newer than I remember:
SPECIFICATIONS LAST UPDATED: AN HOUR AGO

The last time I checked it used to be something like 6-7 months older.

I have also another question: Is it possible to make the loader generate the function macros without the vk... and gl.. prefixes?

@Dav1dde
Copy link
Owner

Dav1dde commented May 5, 2019

I regenerated the loader and now it is:
#define VK_HEADER_VERSION 107

Okay good, glad it's working now.

This is also newer than I remember:
SPECIFICATIONS LAST UPDATED: AN HOUR AGO

Yeah, I checked on the server and ran the update just to make sure it's the same files. I'll have it update daily like glad1 soon.

The last time I checked it used to be something like 6-7 months older.

Yeah, when you opened the issue initially the specifications where that old.

I have also another question: Is it possible to make the loader generate the function macros without the vk... and gl.. prefixes?

Currently no, this would be what I consider a post processing step, which probably should be doable with a simple sed -i -e 's/gl//g' -e 's/GL_//g' -e 's/vk//g' -e 's/VK_//g'.

The prefixes are stripped in a MX context, but you'll have to use a context for that.

@Zingam
Copy link
Author

Zingam commented May 5, 2019

I use some ugly caller macro vk_\gl_ to call the functions that's why I'd like to get rid of the prefixes to reduce the ugliness.

The Vulkan loader provides:

GLAD_API_CALL int gladLoadVulkanUserPtr( VkPhysicalDevice physical_device, GLADuserptrloadfunc load, void *userptr);
GLAD_API_CALL int gladLoadVulkan( VkPhysicalDevice physical_device, GLADloadfunc load);

Wouldn't it make sense to also have a

GLAD_API_CALL int gladLoadVulkan( VkInstance instance, GLADloadfunc load);

for loading the instance level functions and so skipping the 'loader' loader part if desired? For example SDL2 provides SDL_Vulkan_GetVkGetInstanceProcAddr(); anyway.

So more control could be possible:

  1. Load global functions
  2. Do stuff - like creating instance
  3. Load instance level functions
  4. Do stuff -> like enumerating devices
  5. Load device level functions

I don't have enough experience with the stuff yet. Excuse me if I am writing non-sense here!

@Dav1dde
Copy link
Owner

Dav1dde commented May 5, 2019

This is basically what you'd be doing, calling the loader 3 times (with that said skipping device level functions can be considered since the loader function has to filter out functions properly due to: KhronosGroup/Vulkan-LoaderAndValidationLayers#2323)

gladLoadVulkanUserPtr(NULL, glfwGetInstanceProcAddress, NULL);
gladLoadVulkanUserPtr(physical_device, glfwGetInstanceProcAddress, instance);

With the builtin loader it would be like this:

gladLoaderLoadVulkan(NULL, NULL, NULL);
gladLoaderLoadVulkan(instance, phyiscal_device, NULL);
gladLoaderLoadVulkan(instance, phyiscal_device, device);

The pyhsical device passed to gladLoadVulkanUserPtr is required as an explicit parameter because the extension recognition mechanism requires it, everything else that is required for function pointer loading should be provided via the user pointer to the loader function.

Thanks to @elmindreda I recently swapped the arguments in the loader function, so it allows you to pass the instance as user pointer and accepts the instance loader procs as loader function.

Maybe this helps for clarity, the builtin loader function and its user pointer:

struct _glad_vulkan_userptr {
    void *vk_handle;
    VkInstance vk_instance;
    VkDevice vk_device;
    PFN_vkGetInstanceProcAddr get_instance_proc_addr;
    PFN_vkGetDeviceProcAddr get_device_proc_addr;
};

static GLADapiproc glad_vulkan_get_proc(void *vuserptr, const char *name) {
    struct _glad_vulkan_userptr userptr = *(struct _glad_vulkan_userptr*) vuserptr;
    PFN_vkVoidFunction result = NULL;

    if (userptr.vk_device != NULL && glad_vulkan_is_device_function(name)) {
        result = userptr.get_device_proc_addr(userptr.vk_device, name);
    }

    if (result == NULL && userptr.vk_instance != NULL) {
        result = userptr.get_instance_proc_addr(userptr.vk_instance, name);
    }

    if(result == NULL) {
        result = (PFN_vkVoidFunction) glad_dlsym_handle(userptr.vk_handle, name);
    }

    return (GLADapiproc) result;
}

@Zingam
Copy link
Author

Zingam commented May 5, 2019

Thank you for the clarification.

My orignal point was to skip calling the dlopen part, etc since in my case SDL2 was doing already (probably). I tested the instance initialization procedure on Windows and Android. I am yet to render anything.

BTW. The VS static analyzer reports these issues:

vulkan.c(416): warning C6387: 'extensions[i]' could be '0': this does not adhere to the specification for the function 'memcpy'.
vulkan.c(434): warning C6387: 'extensions[instance_extension_count+i]' could be '0': this does not adhere to the specification for the function 'memcpy'.
vulkan.c(412): warning C6385: Reading invalid data from 'ext_properties': the readable size is 'max_extension_count*sizeof(VkExtensionProperties)' bytes, but '520' bytes may be read.

@Dav1dde
Copy link
Owner

Dav1dde commented May 5, 2019

My orignal point was to skip calling the dlopen part, etc since in my case SDL2 was doing already (probably). I tested the instance initialization procedure on Windows and Android. I am yet to render anything.

SDL should work like the GLFW example, just with SDL_Vulkan_GetVkInstanceProcAddr() (the result, vkGetInstanceProcAddr, not the function itself) instead of glfwGetInstanceProcAddress.

Something like:

gladLoadVulkanUserPtr(NULL, (GLADuserptrloadfunc) SDL_Vulkan_GetVkInstanceProcAddr(), NULL);
gladLoadVulkanUserPtr(physical_device, (GLADuserptrloadfunc) SDL_Vulkan_GetVkInstanceProcAddr(), instance);

BTW. The VS static analyzer reports these issues: [...]

Thanks, I'll look into it.

@Zingam
Copy link
Author

Zingam commented May 20, 2019

I still haven't completed my journey to setup a basic Vulkan sample with glad2, so I might not be fully aware of how to use it properly and to comment on it without talking non-sense.
The loaders doesn't seem to support these macros, which are defined in vulkan_core.h
VK_EXT_DEBUG_UTILS_EXTENSION_NAME
Is that on purpose or an omission?

@Zingam Zingam closed this as completed May 20, 2019
@Zingam Zingam reopened this May 20, 2019
@Dav1dde
Copy link
Owner

Dav1dde commented May 22, 2019

image

image

Do you mean those or am I missing something?

Maybe you forgot to add the extensions to the loader?

@Zingam
Copy link
Author

Zingam commented May 22, 2019

I completely ignored the extensions. For some reason I assumed they'll be included by default. As there are so many extensions it may be helpful if the generator offers a filter/option to select a minimal core selection of the most essential extensions (like validation layers, etc).

I also noticed a bunch of empty #if defined(...) like these:

#if defined(VK_USE_PLATFORM_XLIB_XRANDR_EXT) || defined(VK_USE_PLATFORM_XLIB_KHR)

#endif

#if defined(VK_USE_PLATFORM_XLIB_XRANDR_EXT)

#endif

#if defined(VK_USE_PLATFORM_WAYLAND_KHR)

#endif

Also there is an empty line before #endifs which seems to serve no purpose but makes the code more confusing to read.

#if defined(VK_USE_PLATFORM_XCB_KHR)
typedef VkBool32 (GLAD_API_PTR *PFN_vkGetPhysicalDeviceXcbPresentationSupportKHR)(VkPhysicalDevice   physicalDevice, uint32_t   queueFamilyIndex, xcb_connection_t *  connection, xcb_visualid_t   visual_id);

#endif

@Dav1dde
Copy link
Owner

Dav1dde commented May 22, 2019

There is room for improvement on the generator, but at the end of the day it is generated code not meant to be read, that's why I am not bothered by the "useless" if/endif's, it's just easier to generate that way.

I was thinking about having a few pre-selections, not sure about it yet.

@Dav1dde Dav1dde closed this as completed Jun 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants