mackron/vkbind

Replacing windows.h include

jesta88 opened this issue · 9 comments

Would you be open to a PR that replaces the two #include <windows.h> for the minimal set of types and procedures? This saves a considerable amount of time when building, and is what volk is doing: https://github.com/zeux/volk/blob/master/volk.h#L32

Yes, I'd be open to that. I have indeed done similar things to this for avoid including windows.h in headers. However, that said, the includes are done programmatically via the code generator and is defined by the Vulkan XML which might make it a bit awkward.

It's been a while since I worked on the code generator (it's a mess - not proud of it!), but I'm thinking a special case would need to be added here:

vkbResult vkbBuildGenerateCode_C_DependencyIncludes(vkbBuild &context, vkbBuildCodeGenState &codegenState, const vkbBuildCodeGenDependencies &dependencies, std::string &codeOut)
{
    const std::vector<size_t> &typeIndices = dependencies.typeIndexes;

    for (size_t iType = 0; iType < typeIndices.size(); ++iType) {
        vkbBuildType &type = context.types[typeIndices[iType]];

        // vkbind doesn't want to depend on vk_platform.h so skip it.
        if (type.name == "vk_platform") {
            continue;
        }

        if (type.category == "include") {
            if (!codegenState.HasOutputType(type.name)) {
                codeOut += "#include <" + type.name + ">\n";
                codegenState.MarkTypeAsOutput(type.name);
            }
        }
    }

    return VKB_SUCCESS;
}

I suppose it'd just check if it's windows.h and if so, just inject the custom code rather than outputting the include.

If you go ahead with this, you'll need a curl executable. I've also not tested the code generator on anything other than Windows.

I was going to look into this one, but I don't think it's possible with vkbind's single file design. If, for example, the application does the following, it's going to break:

#include <windows.h>

#define VK_USE_PLATFORM_WIN32_KHR
#define VKBIND_IMPLEMENTATION
#include "vkbind.h" // <-- Will try redefining DWORD, HANDLE, etc. thereby resulting in an error.

This is a realistic scenario because people are going to want to call things like CreateWindowEx() which will require windows.h.

I'm going to close this one and recommend not making any kind of PR with this change. If someone can provide a good workaround for this I'd be open to suggestions.

Would it be possible to just check if _WINDOWS_ is defined?

I also thought of something like that, but is that portable across all different implementations of the Win32 library? As in, is it consistent with all versions of Visual Studio, and also non-Microsoft implementations such as MinGW? I didn't think there was any standard define for that.

_WINDOWS_ is standard as far as I know, but I only tested on Windows SDKs 8+ and MinGW. Not sure about older or more niche implementations.

The other problem however is if windows.h is included after vkbind:

#define VK_USE_PLATFORM_WIN32_KHR
#define VKBIND_IMPLEMENTATION
#include "vkbind.h"   // <-- Was first to define DWORD, HANDLE, etc.

#include <windows.h>  // <-- Will try redefining DWORD, HANDLE, etc. thereby resulting in an error.

Yeah, if you want to be fool proof, the only way I can think of is an opt-in define. Would work great since this is a header only lib. Default behavior would still include windows.h.

What do you think?

That can probably work. Will reopen this and take a look when I get a chance.

I've added support for a new macro called VKBIND_NO_WIN32_HEADERS. That will exclude the inclusion of windows.h from the header section, but it does not redefine it's own versions of HWND, etc. That means you'll need to do your own declarations and include it before your vkbind.h include:

#include "my_windows.h" // <-- Declare your HWND, etc. here.
#include "vkbind.h"

I cannot embed the HWND, etc. declarations because there is no consensus between all compilers on how to declare these (Digital Mars, for example, declares them different to what volk is doing). This again becomes an issue for unity builds that may include windows.h later on because it'll result in errors about multiple declarations.

Note that the implementation section still includes windows.h because it needs access to things like LoadLibrary(). I tried doing my own declarations of LoadLibrary() and family, but I kept on getting errors complaining about inconsistent linkage specifications when I later included windows.h in the same compilation unit. I'm open to addressing that if someone can provide a patch that works across all compilers, but for me personally I have little interesting in spending time on that.