Skip to content

Add clang format from samples repo #131

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 7 commits into
base: main
Choose a base branch
from
Open

Conversation

SaschaWillems
Copy link
Collaborator

@SaschaWillems SaschaWillems commented Aug 6, 2025

As noted in #126 code style is all over the place, making things hard to read.

We should start using a clang format file like we did in the samples to make sure code style follows a fixed set of guidelines.

This PR is there to discuss that. It adds the clang format file from the samples and applies that to one chapter.

We prob. still would have to do at least some manual fixups, as even clang format can't get everything into a good-to-read shape. But it should do hopefully around 90% of the job.

While modern C++ is generally hard on the eyes, this makes it at least somewhat easier to read and (more importantly) more consistent.

Format compute shader chapter as an example
@asuessenbach
Copy link
Contributor

Besides lots of other settings I'm using with the Vulkan-Hpp project, something like
ColumnLimit : 160
helps very much in getting some nice formatting, preventing arbitrary long lines.

Just for reference, I've attached the clang-format from Vulkan-Hpp as a text file: clang-format.txt

@SaschaWillems
Copy link
Collaborator Author

As agreed on the last call: I'll apply the clang format to all files and update this PR.

@SaschaWillems SaschaWillems marked this pull request as ready for review August 17, 2025 17:10
@SaschaWillems
Copy link
Collaborator Author

No idea why the Android build fails.

@asuessenbach
Copy link
Contributor

No idea why the Android build fails.

clang_format has sorted the includes. Now, vulkan_android.h is included before vulkan_core.h in 34_android.cpp.

@SaschaWillems
Copy link
Collaborator Author

Oh, that was prob. stupid. Will redo this PR without reordering the includes :/

@asuessenbach
Copy link
Contributor

asuessenbach commented Aug 18, 2025

Oh, that was prob. stupid.

No, that was not stupid. It's vulkan, that is stupid.
I would prefer to just exclude those two #includes from reordering/formatting.

Or maybe just #include <vulkan/vulkan.h>, instead.

@SaschaWillems
Copy link
Collaborator Author

That would be the better option. Didn't realize that the tutorial did explicitly include the core/platform specific headers. That indeed does not make much sense and should be changed indeed. Also feels odd that every chapter explicitly includes "vk_platform.h".

Copy link
Contributor

@gpx1000 gpx1000 left a comment

Choose a reason for hiding this comment

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

looks like merge conflicts. Could you address?

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.

3 participants