-
-
Notifications
You must be signed in to change notification settings - Fork 442
Update vkd3d + vkd3d-compiler #1678
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
Conversation
Co-authored-by: The Silk.NET Automaton <9011267+dotnet-bot@users.noreply.github.com>
| option->value = true; | ||
| } | ||
|
|
||
| if (flags & (D3DCOMPILE_PACK_MATRIX_ROW_MAJOR | D3DCOMPILE_PACK_MATRIX_COLUMN_MAJOR)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does the code in this file actually come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comes from this file
All i have done is strip out the irrelevant parts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like a bit too much human intervention (and code that isn't ours, in our repo, effectively untracked)
I know it's cursed, but we could do something like
#include "vkd3d_utils_main.c"
#undef D3D12GetDebugInterface
#undef D3D12CreateDeviceVKD3D
#undef D3D12CreateRootSignatureDeserializer
#undef D3D12CreateVersionedRootSignatureDeserializer
// etc...Alternatively we could leave the irrelevant parts in... just not really keen on us having our own "buffer" of upstream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like a bit too much human intervention (and code that isn't ours, in our repo, effectively untracked)
I know it's cursed, but we could do something like
#include "vkd3d_utils_main.c" #undef D3D12GetDebugInterface #undef D3D12CreateDeviceVKD3D #undef D3D12CreateRootSignatureDeserializer #undef D3D12CreateVersionedRootSignatureDeserializer // etc...Alternatively we could leave the irrelevant parts in... just not really keen on us having our own "buffer" of upstream
The irrelevant parts more meant it complicated the build process, since stuff like the logging macros were causing a billion undefined symbol errors i was not too keen on debugging/fixing, but i can give using the original source a shot with some well placed def and undef if desired
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the main thing is I had no idea this existed (yes I know clearly I don't pay that much attention) and wouldn't have thought to check here when updating vkd3d, and the same will definitely be said for whoever comes after you & I.
Eliminating as much human error as possible should be key, and this I feel like is definitely human-error-prone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the main thing is I had no idea this existed (yes I know clearly I don't pay that much attention) and wouldn't have thought to check here when updating vkd3d, and the same will definitely be said for whoever comes after you & I.
Eliminating as much human error as possible should be key, and this I feel like is definitely human-error-prone
to be fair i also had to reference my own commits to figure out what i had done, so its a bit error prone yeah, i'll try to get the original source working at some point soon (and also update past .405 so we get them LLVM 17 benefits)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notably 4 months ago they removed ms_abi so we are likely to have fixed #1634
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...although you probably already knew that as you've already linked the issue 🤦
|
Converting back to draft for the time being (so I stop checking this!) |
|
2.18 snap is on Tuesday, is this likely to make it in? |
uhhhhh maybe? ive been very lazy and tired the past week so it depends on if i have the energy to work on it before then (which itself depends on my AC getting fixed, i dont have the energy to fight C when its already like 30 degrees C in my house) |
… SMP Thu Sep 7 14:07:14 UTC 2023 (#1720) Co-authored-by: The Silk.NET Automaton <9011267+dotnet-bot@users.noreply.github.com>
…untu SMP Thu Sep 7 14:07:14 UTC 2023 (#1717) Co-authored-by: The Silk.NET Automaton <9011267+dotnet-bot@users.noreply.github.com>
…Ubuntu SMP Thu Sep 7 14:07:14 UTC 2023 (#1711) Co-authored-by: The Silk.NET Automaton <9011267+dotnet-bot@users.noreply.github.com>
|
Assuming this is probably stale? |
|
closing as stale |
Summary of the PR
Update vkd3d to my latest fork, and to Zig
0.12.0-dev.405+c1e94b28aRelated issues, Discord discussions, or proposals
Closes #1634
Further Comments
My fork now has the ms_abi patch applied, so it should fix the ABI issues we sometimes saw