Skip to content
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

Vulkan codegen refactor tranche 4 #1950

Open
wants to merge 32 commits into
base: dev
Choose a base branch
from

Conversation

jzulauf-lunarg
Copy link
Contributor

Another 30 commit branch roughly

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 344024.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5779 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5779 passed.

Update the referenced resource consumer generators to use the Khronos
variables.  Interestingly enough, it removes 4 functions which appeared
to have incorrectly been added by the previous codegen.
Update the command buffer util header codegen to remove unused types
and use Khronos types when necessary.
Remove unused types and use Khronos types.
Update the api call encoder body generation to use Khronos types.
Now that the porting is done, rename "global_structs_with_handle"
with "structs_with_handle".
Update the struct handle wrapper header generation to move a majority
of the code into the Khronos folder as it is mostly API agnostic.
Use "isHandleLike" when determining if we need to treat a type like a
handle, atom, or opaque type.
Move the structure handle wrapping body code generation to the Khronos
version except for special cases.
This actually exposed a bug where we were incorrectly handling
an Arm flag type.
We had 2 global methods that could be incorporated directly into the
Khronos base class in a cleaner way.  These were removed:
BitsEnumToFlagsTypedef, removeSuffix
@jzulauf-lunarg jzulauf-lunarg force-pushed the zulauf-vulkan-codegen-checkpoint-4 branch from 542ed04 to 1e74722 Compare January 22, 2025 17:40
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 352934.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5900 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5900 passed.

template <> std::string ToString<VkPhysicalDeviceSchedulingControlsFlagBitsARM>(const VkPhysicalDeviceSchedulingControlsFlagBitsARM& value, ToStringFlags toStringFlags, uint32_t tabCount, uint32_t tabSize);
template <> std::string ToString<VkPhysicalDeviceSchedulingControlsFlagBitsARM>(VkFlags vkFlags, ToStringFlags toStringFlags, uint32_t tabCount, uint32_t tabSize);
std::string VkPhysicalDeviceSchedulingControlsFlagBitsARMToString(const VkPhysicalDeviceSchedulingControlsFlagBitsARM value);
std::string VkPhysicalDeviceSchedulingControlsFlagsARMToString(VkFlags64 vkFlags);
Copy link
Contributor

Choose a reason for hiding this comment

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

why change the signature for those 2 functions?

another remark, not very important:
the header 'could' be left out from codegen, only define:

template <typename T> std::string ToString<T>(const T& value, ToStringFlags toStringFlags, uint32_t tabCount, uint32_t tabSize) = delete;

codegen would then only need to generate the .cpp
and the header could be manually edited and the API documented.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok nevermind. I see the header already contained other, non-templated helpers

Copy link
Contributor

@fabian-lunarg fabian-lunarg left a comment

Choose a reason for hiding this comment

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

changes in generated code look oversee-able, only left a minor remark.
the PR seems to focus on refactoring common KHR things out of vulkan_generators,
and into a common KHR base. CI ran successfully, approved

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

Successfully merging this pull request may close these issues.

5 participants