Open bmcdorman opened 3 years ago
Hello,
That's great to hear! Most certainly it will be useful for some people in the future. I will add the link to your repo to the description in the next update. If you have some thoughts/questions about VkFFT you encountered during the development - feel free to share them!
Best regards, Dmitrii Tolmachev
Great! There were some things that could've made the process easier, but they're all from a code perspective. Functionality and performance are superb.
static inline
, which prevents the symbols from being exported in a library. We have to rewrite the VkFFT.h header as a part of vkfft-rs
's build process to remove those keywords to compile a static library Rust can link against. I could probably open a PR to add a #define
for this, though. One probably doesn't want those very large functions being marked inline
anyway, as it could dramatically increase code size, and as a result, decrease cache hits. I imagine the C++ compiler isn't inlining them, though, as inline
is merely a suggestion to the compiler.Configuration
or LaunchParams
. If they're present in the LaunchParams
, the values in the Configuration are overwritten. This mutability causes issues with Rust's safety guarantees, as a shared pointer to a buffer specified in the LaunchParams
will persist in the Configuration
. If the user constructs another LaunchParams
without the buffer specified the VkFFT implementation could attempt to use a buffer that went out of scope and was deallocated. Under typical use these issues shouldn't arise, and there are workarounds (for example, nulling the "cached" values in the Configuration
after every launch operation), but as a result of things like that it's very difficult to make the safety guarantees Rust expects.I'm still working through some parts of the implementation, so I'll keep you posted as questions arise!
Thanks again!
Thank you for your response! I will briefly go over your suggestions:
Best regards, Dmitrii Tolmachev
Thanks for the information! I'll probably require the buffers are specified in the LaunchParams
. Retaining the original performance characteristics of the core library is a priority for us. In regards to the single header, I understand the appeal... perhaps as a part of the compilation process a single "amalgamated" header could be produced? I think that might strike the right balance between ease-of-use and implementation organization. I understand that's probably not a priority regardless, but thought I'd throw it out there.
In regards to the functions being static, one approach might be something like dr_wav does. They have a preprocessor define that determines if the implementations are included or just the prototypes (e.g., #define DR_WAV_IMPLEMENTATION
). That said, the rewriting method we're using will suffice.
Code organization improvement is a continuous process in VkFFT. Originally, I had over 100k lines of Vulkan static shaders (version 1.1.0) covering only a fraction of functionality that it is there today. The run-time generation solved this but if I started writing in it from the beginning - I probably would not have finished at all. The performance and features are the main driving force for me - and this is what really matters to the majority of users. Rest will come along with time.
I already have #define VKFFT_H and it will probably suffice to the need of static keyword (it will disable any further inclusions), though I have not tested it much. I just went with the safest way for now.
Hi,
I've put together some Rust bindings for VkFFT. You can find it here. It's certainly incomplete (particularly in documentation, examples, and safety guarantees), but I think it's close enough for other people to get some use from. Hopefully I'll be able to keep giving it some love for a while.
It assumes the Vulkan backend is used. Unfortunately the other backends would probably be best expressed through completely separate bindings. It further assumes vulkano (idiomatic Rust bindings for Vulkan) is also used.
I've appended a snippet of the first kernel FFT from
sample_9
to give you a taste of the API.Thanks for creating VkFFT!