Skip to content

Conversation

@walcht
Copy link

@walcht walcht commented Oct 18, 2025

This PR attempts to improve a couple of things in the Image Views chapter.

  1. I think the explanation of the views can be enhanced by introducing a short explanation of buffer views which, I assume, are easier to explain than image views.

  2. There is a mixture of C and C++ styles - Throughout this tutorial, C++ is used with VulkanHpp and in this chapter we have some code snippets like this:

    createInfo.components.r = VK_COMPONENT_SWIZZLE_IDENTITY;
  3. There are some code snippets that are unnecessarily repeated (I think).

  4. Finally, I did clang-format the code snippets so that they are within 80 column width limit - having to side scroll an embedded code snippet is not ideal.

Please do let me know if PRs like this are welcome (If so, I have done similar changes/improvements to other previous chapters that I can also open a PR for).

@SaschaWillems
Copy link
Collaborator

SaschaWillems commented Oct 18, 2025

There is a mixture of C and C++ styles - Throughout this tutorial, C++ is used with VulkanHpp and in this chapter we have some code snippets like this:

This is something we are going to fix, see #122. But removing the C syntax blocks might not be the right option, see #202.

Finally, I did clang-format the code snippets so that they are within 80 column width limit - having to side scroll an embedded code snippet is not ideal.

For clang format: We plan on adding that, see #131. Once that's merged, we'll also try to adjust the documentation accordingly. The limit of 80 columns is something we need to discuss first. That was fine in the early days of programming, but nowadays might be too small.

But in general, for designated initializers, I prefer having values line-by-line, which improves readability a lot.

@walcht
Copy link
Author

walcht commented Oct 18, 2025

Apologies if the PR seems to include a lot of things (this is my first in this project and I really want to contribute to it :-)).

I wasn't aware of #202 - it makes sense then to fix these language consistencies there.

Concerning #131, it is important to note that I am talking about embedded C++ formatting and not sample/attachment formatting. I don't know if a single clang-format file is planned for both or if they are separate. My reasoning for the 80 column width (or something like a 100 - definitely not a 160) is the following: some people might open an editor to the side and re-write the code snippets along the tutorial (that's at least me) - in this example, the longest line in the code snippet is 94 characters but is still occluded:

Screenshot from 2025-10-18 21-52-43

But this is just nitpicking and getting at least an agreed upon clang-format is good enough.

I will close this PR since most of the issues are already being addressed in other PRs.

@gpx1000
Copy link
Contributor

gpx1000 commented Oct 18, 2025

Thanks for the effort to help. If you're willing it night make sense to combine efforts rather than ask you to simply close the pr. You're more than welcome to target my branch so we can capture your work. I think we need to add a contributor list as well and make things easier for those willing to help out.

@walcht
Copy link
Author

walcht commented Oct 19, 2025

@gpx1000 - Then I will add the C snippets (the same way you are doing in the #202 PR) and open a PR targeting:
gpx1000:use-multi-language-in-source. I will close this PR since I can't re-target to a different repository.

I want to help with adjusting other chapters (adding C snippets, fixing C++ inconsistencies, etc) - for instance I can address the Drawing a triangle > Setup sub-chapter. If you are ok with this, then you can just tell me which chapters I can help with so. Thanks.

@gpx1000
Copy link
Contributor

gpx1000 commented Oct 19, 2025

Sounds great! Maybe post a list of which chapters you'd like to take on here and they will be all yours! We will readjust if needed as we go. Thanks for the help.

@walcht
Copy link
Author

walcht commented Oct 19, 2025

Sure - I will start with:

  • 03_Drawing_a_triangle => 00_Setup/*
  • 03_Drawing_a_triangle => 02_Graphics_pipeline_basics/*
  • 03_Drawing_a_triangle => 01_Presentation => 01_Swap_chain.adoc
  • 03_Drawing_a_triangle => 01_Presentation => 02_Image_views.adoc (done - see: this PR )

I will commit each chapter (i.e., each adoc) file separately and open PRs targeting your branch gpx1000:use-multi-language-in-source (each PR targeting a single adoc chapter/file).

And, finally, just to confirm, I saw that the C language snippets are using C++ concepts (foreach loop, etc.) - I guess what is meant by C is that the Vulkan C-API will be used directly within C++ and not a complete re-write in C, right? (will assume it is just C Vulkan API).

Since I currently have some free time, I think I can finish this by tomorrow (Monday).

More than happy to contribute :-)

@gpx1000
Copy link
Contributor

gpx1000 commented Oct 19, 2025

Sounds great on all of that.

And yes Vulkan c-api over cpp is the standard way we try to document rather than entirely c.

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