rswinkle / PortableGL

An implementation of OpenGL 3.x-ish in clean C
MIT License
1.04k stars 49 forks source link

CVector #19

Closed RobLoach closed 5 months ago

RobLoach commented 5 months ago

I see lots of cvector_*.h implementations. We could adopt a nice little vector header to accomplish some of what it does: https://github.com/eteran/c-vector

rswinkle commented 5 months ago

To be honest I've gone back and forth about whether I even want my vector types in there as I only use ~3 functions for each but I always come back to the idea that by having them, I'm leaving the possibility of doing more complicated things in the future open for myself and other users.

In response to your suggestion, here's my analysis of the pros and cons with commentary

Pros:

Cons:

Here is a discussion I had years ago about this very issue comparing my style of CVector with the prefix allocation style.

In addition, I could easily remove two of the Pros of c-vector if instead of including generated CVector type files, I just included my own cvector.h or even just cvector_macro.h which is already in src/ and mentioned in the generator script as an option.

Obviously I haven't come to a final decision. I love the convenience of using the macros like C++ templates (it's not quite as convenient as your style of vector but still very convenient) especially early in a project/prototyping but on the other hand, part of me likes having the actual code for some of the same reasons I prefer CVector over c-vector.

If nothing else, looking at c-vector makes me want to add shrink_to_fit to CVector. I don't know why I never added it before, since that's pretty much all I would use my set capacity function for anyway, except that C++ vector didn't have it either at the time which is a really stupid reason smh.

EDIT: minor correction.

RobLoach commented 5 months ago

Agreed. It could introduce a lot of inline bloat since they're a bunch of macros :wink:

rswinkle commented 5 months ago

Agreed. It could introduce a lot of inline bloat since they're a bunch of macros 😉

I didn't even think of that issue. While it wouldn't actually make a huge difference internally since there aren't a ton of vector calls outside initialization and shutdown, that would be an issue for its use in general.

Whereas my macro template system might have a slight compile time overhead but would generate the exact same object code as we get currently. Hmmm...

I also feel, comparing the two libraries, that mine is far more tested (both formally and just from me using it forever).

I remember dealing with this exact issue way back when I first added destructors and later constructors. The issue you bring up is also solved by my method since no allocation is needed to create a vector and intializing a vector type to 0 is a valid state (cvector_i myvec = { 0 }) which I do all the time. Needing an allocation even for an empty vector definitely seems like a shortcoming that would come up a decent amount in practice.

Long story short, I'll stick with my library but I'm open to switching to using my macros (or making that a configuration option of the generation script at the very least) if you feel that would be worth it.

364 cvector_float.h 364 cvector_glBuffer.h 364 cvector_glProgram.h 364 cvector_glTexture.h 364 cvector_glVertex_Array.h 364 cvector_glVertex.h 951 cvector_macro.h

So it would save just over 1200 lines to make the switch, out of 14175 lines in PGL atm.

Thoughts? I'll leave this issue open for you and anyone else who wants to chime in for the foreseeable future since there's no rush.

EDIT: Actually it would save a more like 3400 lines because of the duplication of inclusion that I never really thought about till now oops, but that duplication could be fixed with changes to the script and/or breaking the files up into c/h pairs. so 1200 is still the real number.

RobLoach commented 5 months ago

I vote close.