code generation and update to raylib 4 #100
Comments
|
This is awesome, konsumer! |
|
I am interested to see if generating the API this way works too. The performance tests I did definitely were not by any means scientific, and 3.5->4.0 may be accounting for the improvements - for me the difference was about double (in the bunnymark example) which seemed a little far fetched. I am thinking if the binding is going to be generated this way - why bother using the JSON to generate ToValue.h and ToObject.h - the code for those functions doesn't really need to change drastically, and raylib doesn't really introduce many new structs between versions. Even in the generator code many of the conversion functions would just be hardcoded, so its probably just easier to use the existing files for that, and only generate node-raylib.h based on the JSON |
|
Is there a way to (when developing) configure cmake to only clone the raylib repo once? This would be fine when the user ultimately installs the package, but it takes me about 3 minutes to clone raylib each time I want to recompile to test |
Me too! Once it's building, I will definitely be comparing the 2 methods. I think my stuff can be pretty easily moved more to the other style, if it's better, and I'm all for it.
It changed enough to be a challenge for me to port the code in this repo from 3.5 to 4.0 by hand, so I imagine future migrations will be similar. After the initial generation in this style, if it's roughly the same performance, I don't disagree with you, though. I am a big fan of "the code is the configuration" and it very well may make sense to drop the generation stuff, and just tweak the source directly, and maybe come back to it later, if it has serious problems with new API versions.
Yeh, agreed. I did that because I couldn't figure out a good way to automate it. if you have a look at It looks like this: So, I think it's a combo of the commas and how I'm ignoring numbers (for size-set vars.) This one issue could easily be worked around (to make parser more robust) and resolved, if it's a big deal to hand-code the one def. |
|
I think for uniformity, it should probly look like this, which maybe we can take up with the upstream parser team: [
{
"name": "m0",
"type": "float"
},
{
"name": "m1",
"type": "float"
},
{
"name": "m2",
"type": "float"
},
{
"name": "m3",
"type": "float"
},
{
"name": "m4",
"type": "float"
},
{
"name": "m5",
"type": "float"
},
{
"name": "m6",
"type": "float"
},
{
"name": "m7",
"type": "float"
},
{
"name": "m8",
"type": "float"
},
{
"name": "m9",
"type": "float"
},
{
"name": "m10",
"type": "float"
},
{
"name": "m11",
"type": "float"
},
{
"name": "m12",
"type": "float"
},
{
"name": "m13",
"type": "float"
},
{
"name": "m14",
"type": "float",
},
{
"name": "m15",
"type": "float"
}
] |
|
RobLoach mentioned this in the raylib discord earlier today too - I went around that by manually overriding the matrix entry in the structs before I process any of the text: https://github.com/twuky/raylib-4.0/blob/279b6a4c4e08701e2c26e18c84d227c539cbed28/src/generate.ts#L41 |
|
Yep, I did similar, I just have an ignore list, and I added it to the top of the other files. I suspect it will never change. |
|
I'm going to try and see if I can get this closer to compiling just by modifying the existing ToValue/ToObject scripts. There are only a few differences between 3.5 and 4.0, such as CharInfo being renamed to GlyphInfo |
I am not sure. I noticed this problem, too. very annoying. For end-users it has a lot of things that make it easier, but it makes building while deving go very slow. I will look into it. |
|
pushed a couple changes to the generator that handle pointers and inner structs the way the current bindings do. It looks like it is still hanging up on a function that depends on a Matrix& arg and another with const float [2] (Vector2?). |
|
I looked into cmake git prob more. I think it might be related to npm/cmake-js. if I make a new dir and run
Pushed another thing that fixed that I think, so yeh, now I get a ton of array and Matrix-related errors, probly in the same spot you are. Looks like it's all |
|
pushed another few changes - GetArgFromParam used some of the old names of structs that had been changed since 3.5. Also, a lot of the errors with Matrix were that there wasn't a conversion in ToValue.h (since its being filtered out) so I hardcoded one to that section as well |
Ah, right. good catch.
That makes sense. May be another good future-thing to explore with the upstream parser. I added a few more of the built-in converters. Here is my current log. Seems like remaining stuff is arrays, a bunch of Matrix things, and some |
|
I talked to ray on discord. He said he likes the visual structure of I was looking at these: I wonder if I just prepocess all of these, it would have all the right stuff. Some, like What happened with the other ignored types in there? I am wondering if there is some simple things we can do like above texture thing, to keep it simple and minimize edge-cases. Update: I got rid of |
|
Ok, so this weekend I have a little time to look at this. Seems like remaining issues are sized-arrays like this: I will try to figure out how to make a type-converter for these. Sidenote, I think I see why |
|
With help from @RobLoach I got my fork generating & building from current JSON, and able to run basic example on raylib4. It's still missing a few things (mostly around arrays.) I added some new info in the in-memory JSON (before it does the code-generation, but after it reads the JSON file from upstream raylib) Our current blocklist is this: // use this to keep from wrapping things
const blocklist = [
// error: invalid conversion from ‘void (*)(int, const char*, ...)’ to ‘void (*)()’ [-fpermissive]
'TraceLog',
'TextFormat',
// these appear to not be defined, even though they are in JSON
'SetWindowOpacity',
'GetRenderWidth',
'GetRenderHeight',
'ExportFontAsCode'
]so I got all our blocks documented, which is good. Additionally, I am currently adding all structs that use arrays to the blocklist, and any function that uses those: [
'Material',
'BoneInfo',
'VrDeviceInfo',
'VrStereoConfig'
]So I think all that is left is figuring out how to do this for inline Napi::Value ToValue(Napi::Env& env, float[4] value) {
// return a NAPI array of floats here
}
float[4] Tofloat4(Napi::Env& env, Napi::Value value) {
// return float[4] here
}and then make sure to check the new This PR shows an example of using a float-array inline, so it should be a good start. We could just do things in this sort of 1-off way, to wrap it in the generated wrapper, and I think it'd work great. Since there are only 4, they could also just made by hand and added to the scalar converters that are in there now for float, etc. It's not as nice of a solution, but it might make the fix go a bit faster, now. Also, aside from the missing array stuff, the wrappers still need some work. For example Might just be that the demo needs to be updated for ray4, though. to summarizeFor this to be ready to PR, and allow us to say "node-wrapper with full raylib 4 support", I think we need this:
|
|
good work on this! In general, after looking more closely at raylib and how its functions work, there are a lot more functions that while compiling fine won't have the expected behavior, which may be solvable with some further wrapped functions. I think even for the example of UpdateCamera - that function allows a 2D or 3D camera to be passed in, so i'm wondering if we actually need two wrapped functions, that determine the type first FROM the JS binding, then call the adequate wrapped function - UpdateCamera2DWrap vs UpdateCamera3DWrap i started documenting some functions i've found that have additional issues here: https://github.com/twuky/raylib-4.0/blob/master/unsupported_functions.md |
|
Added both of you as collaborators on the project! The updated code is coming along very nicely 👍 |
My thinking, after making a few wrappers in various languages, is that the target language is the right place for this. Like it'll be easier & more ergonomic to wrap in node-space. If we can generate a basic wrapper and it has enough of an interface to work with, we can add a few little things in js, if needed. I also think index.js should expose more constants from the C-lib. |
|
calling into the node addon can be expensive, so i agree that the enumerators and other raylib constants could just be implemented in plain javascript. adding that to the parser wouldn't be too difficult. it is probably possible to get some of the missing functions implemented on index.js - like But there are still some functions that will take additional work on the C side to function - anything expecting the user to pass in an array (or a pointer to an array) is a good example. some of those functions could have improvements on the side of javascript though, for example most those array functions also require you to pass in the array length, but here the binding could handle that for users. |
|
I currently expose all the constants in the c-wrapper. It might be fine, as it would be an at-import cost, and my feeling is it would actually be pretty light, because of how NAPI exposes constants. I agree with you on the other stuff, too, but maybe we should measure the performance with js vs NAPI. I know they do a bunch of stuff to optimize it, and it's orders of magnitude better than ffi, for example, so it might be fine. Maybe we should identify what needs special-handling and focus on that in JS, next (possibly also generated.)
Agreed, JS can do this easily. Can you find some examples? While I was working on it, I only saw fixed-length arrays, but I didn't take too close of a look. I don't have much time left this weekend, and a pretty packed schedule this week, with work, so I won't have much time to work on it. I am totally open to any ideas either of you have to improve it, so just go for it. Should I PR the currently incomplete notnullgames raylib4 branch into this repo, to keep it all here? |
|
Take a look at that Unsupported_functions.md link I posted for a few of the array examples, and some other functions I noted that use pointers in ways that may take some extra attention. I started noting how they don't currently work. The way I understand most of them, it may be difficult to fix those without hand-writing solutions for each case somewhat separately.
The problem with
I think this sort of thing will need to be on a by-case basis though. It's probably a bigger priority to see about the fixed-length arrays and other Material issues first, since I am mostly talking about a few outlier functions. I'm fine with working out of either repo at the moment, but it seems like it would be fine to pull the branch, since it is at least building? |
|
There's no worry in implementing some of the functions in JavaScript when possible. There need to be some manual wrappers sometimes. |
|
So over this weekend I've been writing some more function bindings to test different methods of using node-addon-api with performance in mind. The main challenge performance-wise with this library is that native addon function calls have a lot of overhead when converting parameters from JS to C++. The expectation seems to be that native addons focus on libraries where the amount of work the function needs to do outweighs the cost of translating data between the languages. Raylib is an example of the opposite, where we do smaller amounts of work many times over such as rendering different objects. That being said, I have found some general principles with node-addon-api that I think could be used to squeeze the maximum amount of speed out of this 'converting' process. My focus was on improving the throughput of DrawTexture() with the bunnymark example in mind. Main Guidelines
BenchmarksHere is some more detailed information on the benchmarks I put together while testing this. These are all based on the 'bunnymark' example code from raylib. Maximum Draw CallsThe first test was to continually spawn new bunnies until the framerate reached below 60FPS. Stress Test FramerateThe second test was to pre-generate an array of 20,000 bunnies, and record whatever the framerate was after a few seconds of updating their positions. Some of these look like really drastic performance improvements! Being able to render a texture 79000 times/frame is actually more than you can achieve with something like love2d. But it may be a little tricky to implement these functions in a way that matches with the raylib API, since they drastically change how the arguments need to be presented. It would be necessary to wrap on the JS side in order to use these in the same way you currently use DrawTexture. DrawTextureparameters: DrawTextureShortparameters: DrawTextureShortWhiteparameters: DrawBunnyWhiteparameters: This would mean introducing a new API that does not exactly exist in raylib proper. But it represents the most performant way to issue draw calls over node-addon-api that I could find. It may be possible, in a JS wrapper, to 'detect' when multiple sequential draw calls use the same color or textre, and 'switch over' to this API behind-the-scenes. But if properties like the color or texture are changing every single draw call, splitting the addon call into 2-3 calls would likely be slower than just 1 call with more parameters. DrawBunnyparameters: DrawBunnyBufferparameters: DrawTextureFlatparameters: DrawTextureFlatWhiteparameters: DrawTextureShortBufferparameters: Other performance considerationsBased on all these experiments, there are a few other ideas that I haven't figured out the implementation details for that could further optimize these calls. Caching all textures.It might be possible to override some functionality during LoadTexture where the C++ addon caches a reference to the texture in some global list (maybe an arena allocator?). Then when a user wants to call DrawTexture, in stead of passing in 3-5 parameters, they only need to pass in the index/id/pointer to the texture, which the addon then looks up in its cache . This would probably work okay for Textures, because the struct does not store any actual texture data, just some information about the texture. This is sort of where my inexperience with C++ is limiting, because I would also worry about memory issues on the C++ side, and making sure that the user always has a correct index / pointer to their desired texture. It seems like raylib by default is pretty smart about loading textures and assigning IDs, but I'd want to be totally sure that a memory safe model could be built on top of that. Colors as intsRaylib already has some built-in functionality to convert a 32 bit unsigned int to a color. It should be possible on the JS side to do some bitwise operations (you wouldnt want to call the built in raylib function unless you did it in advance to cache) to produce a number that could be converted by C++ to the color again. This would cut down 3 function arguments anywhere a color is needed. I tried to attempt this while experimenting, and while the addon compiles I can't call the function - which means something in the addon code probably is actually wrong. I pushed the experimental bindings over to https://github.com/twuky/raylib-texture-performance-experiments if you want to take a look at how each of those functions were implemented. It basically only includes as many functions as necessary to run the tests. I think ideally what would happen is that DrawTexture() (and similar, like DrawTexturePRO etc) would have a JS wrapper that analyzes the supplied arguments, and finds the node-addon function that gives the best performance (IE requires the minimum amount of arguments to match functionality). Though that may take some testing of itself to see how much that improves performance, because I could also see it becoming a mess of branches. The alternative would be to expose these alternative drawing functions directly to the user (possibly with some simpler JS wrappers to unfold the objects for the user) as functions that deviate from the standard raylib API. Knowing this about the performance specifics of node-addon-api.. it seems that in order to make the most out of it would require writing the addon bindings in a completely new way, which would essentially mean starting node-raylib from scratch. There are some things, like "flattening" function arguments, that would be pretty trivial to implement in the code generator. Some of the other concepts like caching would need a lot of manual setup. |
Lots of discussion on #98
Basically, we can update to 4.0 raylib, and the new code-generation stuff seems like a really good path (so it will be some up-front work now, but later API changes will be easier to manage.) I started work on this fork, whcih isn't currently building, but I think just needs a few little things.
The text was updated successfully, but these errors were encountered: