-
Notifications
You must be signed in to change notification settings - Fork 43
Improve Image Views chapter by adding further explanation of views, using VulkanHpp enums, and overall better restructuring #203
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
Conversation
This is something we are going to fix, see #122. But removing the C syntax blocks might not be the right option, see #202.
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. |
|
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:
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. |
|
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. |
|
@gpx1000 - Then I will add the C snippets (the same way you are doing in the #202 PR) and open a PR targeting: 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. |
|
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. |
|
Sure - I will start with:
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 :-) |
|
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. |

This PR attempts to improve a couple of things in the Image Views chapter.
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.
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:
There are some code snippets that are unnecessarily repeated (I think).
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).