-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
DX10/11: Remove d3dcompiler.lib dependency #638
Conversation
Thanks! How did you come with the 30-47 range? Is this technique something that people typically use? (references?) Could the code be reworked a little:
Thanks! Sorry for being nitpicky. As a project maintainer I have the choice to either janitor and fix every PR or eventually try to get people to make perfectly formed PR. Unfortunately the later is hard but we've got to try. |
My code was just something that I quickly put together to bring up the issue to you. I agree with the reworks you suggested and with the fact that you must be "nitpicky" when maintaining this project :)
D3DCompiler_47 – Windows 10 (inbox), Windows 10 SDK; Windows 8.1 (inbox), Windows 8.1 SDK, Visual Studio 2013 In short:
I will install an older VS Platform Toolset and see if I manage to make the project load an older D3DCompiler DLL. In any case, I think it would be nice if Imgui managed to automatically detect which DLL version is present in the system, especially since you just need one function from it: D3DCompile. The pros:
|
…ich DLL version is present in the system.
Just pushed to my fork the code reworks that you suggested. Let me know if they are fine or if I still need to change anything else. I left the for loop from 30 to 47, because d3dcompiler_30.dll was the oldest DLL reference that I could find on Google. Maybe we could increase this 47 as a "version safeguard", but who knows if newer D3DCompile's will behave exactly like the previous ones...
|
Works for me with VS10, tested both 32/64 bits. While I understand the initial reasoning and it is totally legit, I'm beginning to wonder if this should really be part of the example code. It is a little fishy and it WILL break when _48.dll comes out. By doing this in code that user are likely to copy we are basically breaking something in the future. Even minor, it is weird. I'd rather leave to the user responsibility to either provide a DLL, either tweak the code locally? Here's final code btw, caching the function pointer for subsequent calls:
I considering leaving this code in comments and adding a note for people who want to remove dependency on that DLL.? EDIT |
Indeed, this solution is quite fishy and I also agree that it might be better to leave it commented. We already customize the Btw, the sizes of my blobs are 888 and 672 respectively. I hardcoded the blobs' buffer and it worked just fine:
These blobs were compiled with D3DCompiler_47.dll's D3DCompile and as we noticed, different DLL versions gave us different blob sizes. I will make a Test build of my project with the above solution and send to a couple users to see what happens. If it works well, I think I will just leave the blobs hardcoded in my code, instead of trying to find a D3DCompiler_XX.dll in the user's system. |
Yeah I think it is more reasonable and safe to use the precompiled blobs. Let me know if you find something more, thanks for looking into that and taking the time to explain the situation. |
Can we give this another shot? Pros:
Cons:
|
In the grand scheme of things, I don’t feel that those pros are very valuable to be honest. |
Oh, that why it asks for d3dcompiler_43.dll on my users computer, I never know how to solve that beside asking them to install directx end user runtime. Removing this sounds like a huge benefit to me. I develop desktop apps and not games so I'm not at all familiar with graphics stuff, how do I compile the shaders to hex blob like @reliasn posted? Or do I just need to copy paste that, it's working for me but are there any chances that it's outdated? It might seems unnecessary to make this change because you'd think it's up to the user to choose whether to do it or not. But a noob like me don't understand how imgui works and never look inside the internal code to see that comment, i think you should maybe add this to the docs? I usually have to install my app on my customer PC in real life, and sometimes their computer are not connected to the internet, but the DirectX Installer requires having internet connection to run, that wastes a lot of my time |
Ah, I misunderstood that. On whicb systems wouldn’t those DLL be installed by default? Would that mostly ever happen on a virgin systems where eg no games are installed? (Not suggesting we shouldn’t support that case) If your target user base for a precompiled application are not games people i would suggest in the meanwhile including that DLL along with your executable. |
@sakiodre Can you explain the situation which made you want this? Could we get details of a system where d3d_compiler_43.dll is not installed, and see if older versions are installed (and which) ? Back when we had this discussion, the backends and examples code were the same thing. Nowadays they grew in complexity and are separate I would strongly encourage people to reuse our backends and not modify them, making this. I agree we could come up with solution resilient to install on new systems but we’d need a bit more data on those. Thanks |
My customer computer are often old and uses windows xp or 7. I just booted fresh windows 10 and 11 in VM and they also don't have d3dcompiler_43.dll, but there are d3dcompiler_47.dll, i'm not sure how to compile my code to make it use that dll instead? |
We can probably aim to improve this code but imho you can simply ship that the DLL next to your exe. |
Reopening |
(Github doesn’t seem to allow me to reopen the issue) |
Oh... right, stupid me |
The main issue is that Windows XP & 7 don't come with any of the d3dcompiler_xx.dll versions so I often have to include that, Windows 8 and above do come with d3dcompiler_47.dll. Another issue is the loading time and CPU usage on low-end computers, it's not much but the first call to |
@excellentcucumber You can either hack at the backend to dump the contents of the
@sakiodre As Omar noted, you can redistribute If you're supporting XP (or Windows 7 for that matter) you should probably be building against an older version of the Windows SDK anyway, which would by extension mean using the legacy June 2010 DirectX SDK (which provides
If it weren't the shaders it'd be the font texture (which tends to be the heavy part for any apps supporting CJK, using icon fonts, etc.) I think you're better off just handling this like you said if it bothers you. I think a possible compromise here could be to allow users to provide their own shader blobs to the backend. However I would worry some people might see this as an extension point to modify the internal backend shaders for their own purposes. This would also run the risk of weird issues when the backend shaders change but the user's blobs do not. I will say though that iterating through older versions of the compiler at runtime like this old PR proposed is almost certainly the wrong solution and could lead to very confusing gremlins. If we're going to explicitly try to provide in-box support these legacy platforms we should just embed pre-compiled shader blobs (and maybe use CI to ensure they remain up-to-date.) |
It's surprising that compiling those very small shaders has a meaningful effect, have you confirmed that feeding precompiled shaders removes the white flash? If you can indeed hide the window I would recommend that.
Which DLL are you including that works on Windows XP ? (since accordingly to David's link _47.dll may not support it?) In principle I do understand the issue but actively pursuing "nicer" support of Windows XP for us seems like a misallocation of resource. Emphasis on "nicer" since as you suggested it does work already. I would suggest any app wanting to support XP or Windows 8 may want to ship with that DLL, or possibly use DX9/DX10 if that makes any difference.
Here's an in-between proposal:
From our pov that's rather harmless (in term of complexity/maintenance) but it creates a solution for you, and since we have (and will have) comments linking to this issue and near the shader sources, I envision that people who care about it would naturally find their way to use it. EDIT |
Yes, it is barely noticeable, it only takes about 60ms on low-end PCs, it's just me being a perfectionist.
I include the d3dcompiler_43.dll and use the June 2010 DirectX SDK since that'll work for XP and above. Just using pre-compiled shaders is the perfect solution for me personally, but I understand that would add gibberish blobs and obfuscate the code. |
See my proposal above (I edited the post slightly after initial post), I think adding semi private |
any idea how to perform this solution on D3D12? |
For D3D12 here how you can achieve without the d3dcompiler dependency
Then apply:
|
To add to what @lguilhermee has posted, you can compile the shaders using the In visual studio, you can add a |
MOD: If you come here from the links in imgui_impl_dx10.cpp and imgui_impl_dx11.cpp to remove dependency on d3dcompiler_XX.dll, you can use A) #638 (comment) to find a DLL available in the system or preferably B) #638 (comment) to use a precompiled shader.
The idea is to remove the d3dcompiler.lib dependency by dynamically loading D3DCompiler_XX.dll and finding the address of D3DCompile with GetProcAddress.
This is the solution that I came up with after seeing some users complaining about "D3DCompiler_47.dll missing" errors when running my application built on Release configuration.
From what I read, there are 3 common solutions to this problem:
Is there another better way to remove this dependency?
I'm currently using VS 2015 and I only have VS 2015 Platform Toolset installed (v140) on my Win10, but I thought it would be a nice idea to install an old Platform Toolset (e.g. 2010) to "force" load an older D3DCompiler_XX.dll. Not sure if it will work though.