Skip to content

Rewrite codegen to use Vulkan Object & refactor API Dump extensively #2431

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

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

charles-lunarg
Copy link
Contributor

Rewrites Api Dump to use vulkan_object.py and fix many errors.

The PR starts by modifying the existing code to sort the generated code and cleanup small issues (like extra spaces). The rewrite itself took place over many commits, as the initial one was geared towards getting near-identical generated code output, then later on rewriting the codegen to be more maintainable and readable.

The biggest "issue" is that both base_generator.py and vulkan_object.py are checked into the repo with modifications necessary for API dump to run. In the future, these files will be fixed upstream in order to allow the local copies to be dropped. But since the changes needed aren't going to be up streamed, or just need more development time, it was decided to have the necessary changes to allow this rewrite to be reviewed and published while work continues upstream.

Simplifies codegen a bit by not needing to track the constants
Use the string instead of their numbers.
This makes transitioning to VulkanObject much easier, since extra spaces
make the changes much longer for no good reason.
The include files only need the function declarations, so we can safely remove
everything else from it. That does require moving the union and pNext function
declarations into the .cpp files.
@charles-lunarg charles-lunarg added the Project - apidump apidump issues label Jun 18, 2025
@ci-tester-lunarg
Copy link

CI VulkanTools build queued with queue ID 468027.

@ci-tester-lunarg
Copy link

CI VulkanTools build # 4648 running.

@ci-tester-lunarg
Copy link

CI VulkanTools build # 4648 passed.

Copy link
Contributor

@spencer-lunarg spencer-lunarg left a comment

Choose a reason for hiding this comment

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

clang-format wasn't working, so hard for me to really "check" code gen

(will re-review when that is fixed and comited)

tested the new layer and the output looks great and happy with it!

Major rewrite of the api_dump_generator.py script to use VulkanObject
instead of its own OutputGenerator subclass. This rewrite attemps to
change as little as possible in the generated code, to aid in validating
the rewrite. But doing so introduced a few small errors as the default
sorting was disabled. Subsequent commits will enable sorting and clean
up the introduced mistakes.
Needed codegen changes to get the full count and new dump_format_array() templates
to handle the types.
These were added to help keep straight the different kinds of output
(value, pointer, array) and different output formats. But, with the
consolidation work going on, they obscure actual differences in
behavoir whenever working on the codegen.
The pNext chain iteration functions diverged between text, html, and json.
This commit rectifies the situation, and correctly type-puns the incoming
void* into a VkBaseInStructure using memcpy to get the sType and pNext
correctly.
Makes sweeping changes to consolidate the printing of a type into a single function that
handles all three output types. This makes it much easier to share common codegen logic
between all output types, where previously it had to be duplicated.
Removes duplicate functions which only differed by whether they took a type
by value or by reference. Greatly improves the readibility of api_dump.h since
it removes a bunch of duplicated logic.
Modfies the existing helper functions in api_dump.h as well as adding new ones so
that the code generator can directly call the appropriate types, instead of passing
the dumping function to helper functions as a templated parameter.

This makes the code easier to understand and walk through because the logic to
print isn't being mixed with the logic to call other functions. This also simplifies the
interface to helper functions by removing the function pointer argument, and any
const/non-const overloads necessary for it.
Reduces the code variance between the generated output format code.
Allows codegen to not care at all about formatting.
Also allows more use of '''<code>''' in codegen for long sequences of codegen, as
before doing that would need to un-indent the block in order for the output to appear
correct. Now, clang-format will remove the extra indents.
@mark-lunarg
Copy link
Contributor

Hey Charles, is the GN build still happy?

Uses a simple helper to remove redundant platform guards.
@charles-lunarg charles-lunarg force-pushed the vulkan_object_rewrite branch from 3704ebc to 93e1e89 Compare June 20, 2025 17:35
@ci-tester-lunarg
Copy link

CI VulkanTools build queued with queue ID 469353.

@charles-lunarg
Copy link
Contributor Author

@mark-lunarg I do believe the GN build is good because no changes were made to the names of generated files, adding/removing files, and GN doesn't have any logic to run the generator anyhow means there isn't place for GN to be broken. Not to mention the android/chromium build steps which pass.

@ci-tester-lunarg
Copy link

CI VulkanTools build # 4649 running.

@ci-tester-lunarg
Copy link

CI VulkanTools build # 4649 failed.

@ci-tester-lunarg
Copy link

CI VulkanTools build queued with queue ID 469387.

@ci-tester-lunarg
Copy link

CI VulkanTools build # 4650 running.

@ci-tester-lunarg
Copy link

CI VulkanTools build # 4650 passed.

Copy link
Contributor

@spencer-lunarg spencer-lunarg left a comment

Choose a reason for hiding this comment

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

some nits, but this looks great!

return 'union' if self.vk.structs[var.type].union else 'struct'
return 'value'

def write_value(self, output_format, var, parent):
Copy link
Contributor

Choose a reason for hiding this comment

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

add a comment or update the name, what value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You come up with a name that can mean either parameter or member, and is 'print me code that will dispatch the value needing to be printed without thinking of the word value'

Moves the handwritten implementations for various functions into a dedicated file.
This makes reading the code easier and makes api_dump_generator.py only contain
code that requires code generation to function.

Also went through and cleaned up api_dump_generator.py a bit, using a consistent
naming scheme, removing commented out code, and adding types to functions.
@ci-tester-lunarg
Copy link

CI VulkanTools build queued with queue ID 469540.

@ci-tester-lunarg
Copy link

CI VulkanTools build # 4651 running.

@charles-lunarg
Copy link
Contributor Author

@mark-lunarg I lied, just pushed up a change that added a new .cpp file. But I did make sure to add that to the GN file because of your comment, so thanks for making it a thing to watch out for!

@ci-tester-lunarg
Copy link

CI VulkanTools build # 4651 passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Project - apidump apidump issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants