-
Notifications
You must be signed in to change notification settings - Fork 1
Improve the CMake port interface layout #2
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
base: main
Are you sure you want to change the base?
Conversation
…ded through direct paths instead of through the deprecated dir when using the C impl or through the rust project layout when using the Rust bindings
|
Thank you for your improvements, But I intentionally left the include prefixes because I'm pretty sure headers with such names are very common. It also adds clarity to the backend being included. |
|
I comprehend. Indeed the prefixes allow the usage of both implementations at the same time for testing purposes, given that the CMake port allows the including the two implementations at the library. |
hm, but I, on the contrary, believe that the C implementation should be supported just like Rust and moved from _old to the root directory. The thing is, despite the Rust implementation being close to perfect now, and itself better than C, two things bother me.
So, If you have a desire to improve Version C, I will be only glad, and at the very least, I would be happy to take such changes here to this repository. |
|
Hey @EndrII, sorry for only answering now, didn't saw the subsequent message. I agree with you that integrating Rust implementations can be very inconvenient, and in some more strict environments, bureaucratic or even impossible, making the usage of the C implementation way more convenient. I also noticed that the implementations produce completely different outputs, I don't know if due to bugs or intentional changes/improvements during the library course of development. The most screaming difference was the fact that the Rust implementation outputs floating point values of 32 bits while the C one outputs 64 bits ones, but I don't know if that represent a real loss of precision or was a matter of optimization/improvement. The biggest question/challenge is how the library developer plans/prefers to maintains it, given that while the C official version are bindinds to a Rust implementation, the 'versions' for the other languages are independent implementations, but I didn't analyzed them to see if they have any kind of functional difference over the C/Rust ones. While I would prefer to have a centralized implementation and bindings to other languages, we apparently concur that this is not always ergonomic/possible. And given that the library already have separate implementations for other languages. Maybe the owner would be open to also keep a C and/or C++ implementation too. Although I don't have much knowledge about that area(gradient noise functions), I think I would be able to produce a C++ implementation matching the Rust one, if the usage of modern C++ standards is possible. For a C version I'm not sure if that is possible given that much of the performance improvements that we saw were made with compile time programming features that C, far as I know(I don't know much), doesn't provides. |
this is looks as critical bug, please show where is happened and how you detect it. |
I'm not sure if that is bug, it may be intended. But if you look at the Rust implementation you can see that the noise functions return 32 bits floats. |
32-bit output is critically bad; we need to open an issue. I'll review the entire code later, so personally, I'll probably stick with C. And if it's confirmed that there are genuine problems with the algorithm itself, I'll probably support the C/C++ implementation here. |
|
@gabrielcfvg i create issue on main library KdotJPG#32 |
Currently, when using the cmake port with both the available impls, the user needs to include the header through the path "c/OpenSimplex2F.h" when using the C impl and through "rust/OpenSimplex2.h" when using the Rust impl.
I think that would be more interesting to abstract the used implementation from the end-code, allowing it to be controlled through the CMake scripts that are including the library.
Although, given that the implementations provide different interfaces, changing the implementation would still not be a transparent change.