Skip to content

Comments

Daemon-vulkan infrastructure 2#1845

Open
VReaperV wants to merge 19 commits intoDaemonEngine:masterfrom
VReaperV:vulkan-infrastructure2
Open

Daemon-vulkan infrastructure 2#1845
VReaperV wants to merge 19 commits intoDaemonEngine:masterfrom
VReaperV:vulkan-infrastructure2

Conversation

@VReaperV
Copy link
Contributor

@VReaperV VReaperV commented Oct 5, 2025

Adds some more cmake changes required for daemon-vulkan, slightly modified version of the Khronos Vulkan header generator, as well as a shader generator with support for #insert/#include and SPIR-V generation. Also added some of the embed stuff from #1842 with more functionality.

@VReaperV VReaperV added A-Renderer T-Feature-Request Proposed new feature labels Oct 5, 2025
@VReaperV
Copy link
Contributor Author

VReaperV commented Oct 5, 2025

Most of the changes are just the XML spec and existing generator code.

@VReaperV VReaperV force-pushed the vulkan-infrastructure2 branch from afbdcb0 to f8462a8 Compare October 5, 2025 00:14
@VReaperV VReaperV mentioned this pull request Oct 5, 2025
Draft
@VReaperV VReaperV force-pushed the vulkan-infrastructure2 branch from f8462a8 to 72c00bf Compare October 5, 2025 00:22
slipher
slipher reviewed Oct 5, 2025
View reviewed changes
Copy link
Member

@slipher slipher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VulkanHeaders seems to be a third-party library, so it should go in libs. Is there some good reason to check in the files directly instead of using a submodule?

@VReaperV VReaperV force-pushed the vulkan-infrastructure2 branch from 72c00bf to 4d9ae49 Compare October 5, 2025 00:23
@VReaperV
Copy link
Contributor Author

VReaperV commented Oct 5, 2025

VulkanHeaders seems to be a third-party library, so it should go in libs. Is there some good reason to check in the files directly instead of using a submodule?

Most of it is not used by daemon-vulkan.

@slipher
Copy link
Member

slipher commented Oct 5, 2025

So what is the future maintenance plan for VulkanHeaders: do we expect to maintain it ourselves or periodically sync updates from upstream?

@VReaperV
Copy link
Contributor Author

VReaperV commented Oct 5, 2025

So what is the future maintenance plan for VulkanHeaders: do we expect to maintain it ourselves or periodically sync updates from upstream?

Periodic sync. Header generator rarely ever gets updates and likely would continue working as is without issue.

@VReaperV VReaperV force-pushed the vulkan-infrastructure2 branch from 2cdc4da to 2ce79c6 Compare October 5, 2025 00:38
@slipher
Copy link
Member

slipher commented Oct 5, 2025

OK cool. In any case it should go in libs/

@VReaperV VReaperV force-pushed the vulkan-infrastructure2 branch from 2ce79c6 to 64f9b54 Compare October 5, 2025 01:27
@VReaperV
Copy link
Contributor Author

VReaperV commented Oct 5, 2025

Moved it.

slipher
slipher reviewed Oct 5, 2025
View reviewed changes
Copy link
Member

@slipher slipher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slightly modified version of the Khronos Vulkan header generator,

If there are modifications of the files in libs/ relative to upstream, would you mind checking in the original upstream version as the first commit, and then the local changes as a second commit? That way we can see what was changed in the history.

Also I request that the commit hash of the Vulkan Headers repo be recorded in the commit message.

cmake/DaemonVulkan.cmake
option( VULKAN_SPIRV_LTO "Enable link-time SPIR-V optimisations." ON )

if( USE_VULKAN )
add_executable( VulkanShaderParser "${DAEMON_DIR}/cmake/DaemonVulkan/VulkanShaderParser.cpp" )
Copy link
Member

@slipher slipher Oct 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Host-mode tools mess up cross compiling, so it would be nice to rewrite that in Python at some point (non-blocking comment)

Copy link
Contributor Author

@VReaperV VReaperV Oct 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah probably. This is mostly just the #insert code from gl_shader.cpp, with support for recursion and glslangValidator calls.

@VReaperV VReaperV force-pushed the vulkan-infrastructure2 branch 3 times, most recently from 6a64000 to 21f9e08 Compare October 5, 2025 12:12
@VReaperV
Copy link
Contributor Author

VReaperV commented Oct 5, 2025

slightly modified version of the Khronos Vulkan header generator,

If there are modifications of the files in libs/ relative to upstream, would you mind checking in the original upstream version as the first commit, and then the local changes as a second commit? That way we can see what was changed in the history.

Also I request that the commit hash of the Vulkan Headers repo be recorded in the commit message.

Sure, done.

illwieckz added 10 commits October 6, 2025 02:09
@illwieckz
cmake: rename DaemonBuildInfo as DaemonSourceGenerator
d62c2c4
@illwieckz
DaemonSourceGenerator: rework it a bit
7c106ed
@illwieckz
DaemonSourceGenerator: make the embedded file generator reusable
10f1524
@illwieckz
EmbedText make possible to chose between TEXT and BINARY formats when…
fb9e39f 
… embedding files
@illwieckz
DaemonSourceGenerator: provide embedded files as part as the namespace
9af0bec
@illwieckz
EmbedText: split embedded source lines just in case if an editor or d…
59386eb 
…ebugger opens it
@illwieckz
DaemonSourceGenerator: also add the generated source entry-point to t…
14a748e 
…he target
@illwieckz
DaemonSourceGenerator: lowercase local variables (even if the leak ou…
b96492c 
…tside)
@illwieckz
DaemonSourceGenerator: also make generated source depend on generator…
f174fb6 
…s to regenerate the target
@illwieckz
DaemonSourceGenerator: explicit mkdir
f87d034
@VReaperV VReaperV force-pushed the vulkan-infrastructure2 branch from 21f9e08 to 1fd0e0b Compare October 7, 2025 02:49
@VReaperV VReaperV mentioned this pull request Oct 7, 2025
Merged
slipher
slipher reviewed Oct 8, 2025
View reviewed changes
@VReaperV VReaperV force-pushed the vulkan-infrastructure2 branch 5 times, most recently from f81e205 to 984d775 Compare October 8, 2025 12:32
@VReaperV VReaperV force-pushed the vulkan-infrastructure2 branch from 984d775 to cdfea57 Compare October 17, 2025 23:31
@VReaperV
VulkanHeaders generator: generate sType as const with default value
a80ab6d 
Also add a script to generate all the headers we need along with function loading.
@VReaperV
VulkanHeaders generator: use sized enums
f10467d
@VReaperV VReaperV force-pushed the vulkan-infrastructure2 branch from cdfea57 to f10467d Compare October 17, 2025 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slipher slipher slipher left review comments

Assignees

No one assigned

Labels

A-Renderer T-Feature-Request Proposed new feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@VReaperV @slipher @illwieckz