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

Code seems desperately un-modular and hard to reuse. #245

Open
mutantbob opened this issue Jan 22, 2024 · 2 comments
Open

Code seems desperately un-modular and hard to reuse. #245

mutantbob opened this issue Jan 22, 2024 · 2 comments
Labels
tutorial An issue relating to the tutorial

Comments

@mutantbob
Copy link

The AppData class is just an ever-growing pile of related fields. A great many of those fields should be grouped together in independent pluggable/reusable structs.

A great many functions accept an AppData just so they can access the device and queue. Those fields could probably be grouped in a context struct that is not tangled with the buffers.

So many functions write to fields in AppData. E.g. create_command_pool() when they should probably just return the command pool and let App::create() store it in the correct field.

The App::create() function instantiates an incomplete AppData object and then calls multiple methods to mutate it. This is not optimal code style since the data variable spends a great deal of time in an incomplete state. Ideally there should be an AppData::new(...) that accepts the instance, device, etc.; constructs the necessary fields; and returns a valid and complete AppData object.
Basically, any place you pass &mut should be reviewed to see if it can be improved.

Solving this issue will be quite traumatic. The code right now is optimized for brevity and simplicity of the tutorial. "fixing" the code to be more Rust-y would be a major effort (having to redo all of the stages, both code and text narrative).

@KyleMayes
Copy link
Owner

KyleMayes commented Jan 22, 2024

I didn't know Vulkan when I started this project, the tutorial code is mostly a pretty direct translation of the code from the original C++ tutorial.

As you've said, significantly changing it would be a massive amount of work and I really don't have any interest in making these changes myself or even reviewing someone else's.

Maybe it would be enough to call out how janky this code is and how it mostly serves to illustrate how Vulkan works and now how one should go about structuring a Vulkan application.

@KyleMayes KyleMayes added the tutorial An issue relating to the tutorial label Apr 2, 2024
@chuigda
Copy link
Contributor

chuigda commented Nov 24, 2024

Recently I'm trying to create a sowewhat "bigger" Vulkan project (well though in Java instead of Rust but I think that may be useful), and I'm trying to implement a slightly better code style. You can find the relevant code here. I'm also a Vulkan beginner and maybe what I did is not the best practise, but hope that being useful.

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

No branches or pull requests

3 participants