Generate bindings from the JSON files #60

planetis-m · 2021-12-27T12:36:03Z

No description provided.

planetis-m · 2022-01-03T22:36:12Z

I don't know if you agree with the last change but I decided to produce distinct cint instead of enums. Reasons are the usual: duplicate values, enums with holes used as bitsets. The last straw for me was having to support MaterialMapDiffuse and ShaderLocDiffuse, without the prefixes there are name collisions...

greenfork · 2022-01-19T05:35:16Z

The last straw for me was having to support MaterialMapDiffuse and ShaderLocDiffuse, without the prefixes there are name collisions...

If I'm not mistaken, there are a bunch of #define statements in C file for a bunch of things. But they all serve only a backwards-compatibility purpose because of renames. I think we should strive for better Nim and put backwards-compatibility about such renames in the Changelog notes or just special case them. At least this is a strategy I was going for so far. Looks like it works fine, I don't get a lot of complaints when the changes are reasonable.

If we don't try to be fully backwards-compatible, are we okay with overloadable enums as discussed in #55?

greenfork · 2022-01-20T15:08:30Z

Saw latest changes, looks really great

greenfork · 2022-02-04T19:54:23Z

@planetis-m I found these functions which are not in the set of destructors in #60, do we include them as well or there are some problems?

unloadVrStereoConfig*(config: VrStereoConfig)
unloadFileData*(data: ptr uint8)
unloadFileText*(text: cstring)
unloadImageColors*(colors: ptr Color)
unloadImagePalette*(colors: ptr Color)
unloadFontData*(chars: ptr GlyphInfo; glyphCount: cint)
unloadCodepoints*(codepoints: ptr cint)
unloadWaveSamples*(samples: ptr cfloat)

unloadVrStereoConfig seems alright to add
unloadFileData and unloadFileText seem too general
unloadImageColors and unloadImagePalette can use distinct ptr Color for load... functions
unloadFontData can partially use raylib_fields.nim accessors like []?
unloadCodepoints could use distinct ptr cint and related to Can codepoints map to unicode.Rune #69
unloadWaveSamples could use distinct ptr cfloat

Distinct types will have to also convert other function signatures which use these types though. And it is less maintainable regarding our automation feature of c2nim. Maybe there are some tricks we could use to avoid a lot of text replacement and somehow do it more smartly.

greenfork · 2022-02-05T06:49:03Z

raylib_wrap.nim

+proc drawTexturePoly*(texture: Texture2D, center: Vector2, points: openarray[Vector2], texcoords: openarray[Vector2], tint: Color) =
+  ## Draw a textured polygon
+  drawTexturePolyPriv(texture, center, cast[ptr UncheckedArray[Vector2]](points), cast[ptr UncheckedArray[Vector2]](texcoords), points.len.int32, tint)


We don't really need to rename the original function to drawTexturePolyPriv, it can have same name

greenfork · 2022-02-05T07:28:08Z

raylib_wrap.nim

+proc loadFontData*(fileData: openarray[uint8], fontSize: int32, fontChars: openarray[int32], `type`: FontType): seq[GlyphInfo] =
+  ## Load font data for further use
+  let data = loadFontDataPriv(cast[ptr UncheckedArray[uint8]](fileData), fileData.len.int32,
+      fontSize, cast[ptr UncheckedArray[int32]](fontChars), fontChars.len.int32, `type`)
+  result = newSeq[GlyphInfo](fontChars.len)
+  copyMem(result[0].addr, data, fontChars.len * sizeof(GlyphInfo))
+  memFree(data)


I'm not sure we can allow ourself doing newSeq because it does extra allocation. Allocations are generally considered slow and this here will allocate twice the amount of memory and do twice the number of calls to allocate.

As I understand it is also impossible to make a sequence without addition allocations because of GC. Also, what would be a use case for sequences which we want to modify? There are length checks already in raylib_fields.nim. Here I assume we need sequences to dynamically modify them, but why? We should also take into considerations that operations on sequences will allocate memory uncontrollably which is sometimes undesirable for games.

There's also a View type. I'm not sure about the progress on this. But it looks like a good option here. We can create a custom container which implements toOpenArray for each type as well as an option.

type GlyphInfoArray = object len: int, p: ptr UncheckedArray[GlyphInfo] proc toOpenArray(obj: GlyphInfoArray) ...

I will do that.

Okay, great! Do you want to discuss these options on the specific examples? It could make sense to try several ways of doing this conversion before rewriting the whole file to conform to the one specific variant. There's at least View and custom container options. But we want to minimize the amount of work we are going to do. And possibly automate it.

Regarding automation, it really seems that your version with generation from JSON is going to be better in terms of maintainability. But it's not entirely clear if this is the case.

planetis-m · 2022-02-05T10:19:30Z

unloadVrStereoConfig I removed it, its a noop for now.
unloadWaveSamples just a for loop that calls unloadWaveSample, since we used seq[Wave] it was not need.
unloadImageColors, unloadFontData, unloadImagePalette I think same as above
unloadFileData, unloadFileText it's hard to tell what to do with these, could we instead use std equivalents instead of LoadFile*
unloadCodepoints haven't decided what to do with codepoints yet.

greenfork · 2022-02-07T15:26:09Z

@planetis-m I made an attempt at using destructors #72

greenfork · 2022-02-07T15:34:51Z

Does raylib_fields.nim work now? There's a comment about .copy pragma and I'm not sure what's going to be the best fix for this file. Just adding .copy to each proc with var parameter?

planetis-m · 2022-02-07T21:18:33Z

They work, I used templates as this comment suggests https://forum.nim-lang.org/t/8871#57987 but for the [] proc the problem is still there, but it won't cause any issue. So it's good to go.

Just adding .copy to each proc with var parameter?

Not yet implemented but it's a good idea imo.

greenfork · 2022-02-08T05:33:26Z

On each type c2nim automatically adds bycopy pragma. I would assume that this would fix the same issue?

planetis-m · 2022-02-09T10:01:28Z

Yes but now .bycopy would only be added in the .importc procs and that would allow to write a native Nim procedure using .importc types without any issue.

planetis-m added 9 commits December 27, 2021 15:35

Generate bindings from the JSON files

cfb3970

Attempt to refactor raymath_gen

c8d2cb1

refactor raylib_gen

143f5db

add indWidth to common

b87001f

Update common.nim

c3abeea

Update raylib_gen.nim

7ce91bf

Update raymath_gen.nim

24948c2

Create raylib_wrap.nim

29a8da1

Update raylib_gen.nim

cf67075

planetis-m added 4 commits January 20, 2022 16:55

Update raylib_gen.nim

817536c

Update raylib_wrap.nim

7bb479b

Create raylib_fields.nim

7901781

Create raylib_types.nim

f4bd707

greenfork reviewed Feb 5, 2022

View reviewed changes

planetis-m closed this Feb 11, 2023

planetis-m deleted the planetis-m-patch-1 branch February 11, 2023 10:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate bindings from the JSON files #60

Generate bindings from the JSON files #60

planetis-m commented Dec 27, 2021

planetis-m commented Jan 3, 2022 •

edited

Loading

greenfork commented Jan 19, 2022

greenfork commented Jan 20, 2022

greenfork commented Feb 4, 2022 •

edited

Loading

greenfork Feb 5, 2022

greenfork Feb 5, 2022

greenfork Feb 5, 2022

planetis-m Feb 5, 2022

greenfork Feb 5, 2022 •

edited

Loading

planetis-m commented Feb 5, 2022

greenfork commented Feb 7, 2022

greenfork commented Feb 7, 2022

planetis-m commented Feb 7, 2022

greenfork commented Feb 8, 2022

planetis-m commented Feb 9, 2022

Generate bindings from the JSON files #60

Generate bindings from the JSON files #60

Conversation

planetis-m commented Dec 27, 2021

planetis-m commented Jan 3, 2022 • edited Loading

greenfork commented Jan 19, 2022

greenfork commented Jan 20, 2022

greenfork commented Feb 4, 2022 • edited Loading

greenfork Feb 5, 2022

Choose a reason for hiding this comment

greenfork Feb 5, 2022

Choose a reason for hiding this comment

greenfork Feb 5, 2022

Choose a reason for hiding this comment

planetis-m Feb 5, 2022

Choose a reason for hiding this comment

greenfork Feb 5, 2022 • edited Loading

Choose a reason for hiding this comment

planetis-m commented Feb 5, 2022

greenfork commented Feb 7, 2022

greenfork commented Feb 7, 2022

planetis-m commented Feb 7, 2022

greenfork commented Feb 8, 2022

planetis-m commented Feb 9, 2022

planetis-m commented Jan 3, 2022 •

edited

Loading

greenfork commented Feb 4, 2022 •

edited

Loading

greenfork Feb 5, 2022 •

edited

Loading