stb_image inconsistencies

[TomJGooding]

stb_image is used to load images in various example/* and demo/* examples.

There's currently two different stb_image versions, where example/stb_image.h is v2.08 and demo/common/filebrowser/stb_image.h is v2.28.

stb_image also isn't compatible with C89 because of the C++ style comments.

Would it make sense to create a single stb_image file with the latest version (2.30) that is compatible with C89?

[RobLoach]

Yeah, optimally we'd update example/stb_image.h to the latest with C89 compatibility.

[sleeptightAnsiC]

from #895 (comment)

demos using stb_image [...] wont compile with pedantic on c89 (because using features from more recent standard)

IIRC those were mostly caused by C++ comments // and upstream didn't want to fix it (nothings/stb#1328). I remember writing some POSIX AWK script for replacing // and preserving their logical structure (but I don't remember where I put it, so I'll probably have to start over...)

I can probably write up some script for fetching, replacing comments, and applying our patches to all stb libraries. This would help with issues such as #905 (comment) where the changes are being repeatedly lost.

Maybe we can add new folder for all those vendored libraries? ./ven ? ./deps ? This would make it more clear and scalable, since we're already considering GLAD addition (#777), and I've seen few other PRs where it would help (e.g. #664, #902)

What do you guys think?

[RobLoach]

Yeah. vendor them, though may want a readme since it's essentially a fork for the C89 support.

[TomJGooding]

I made a start on this in #956, but might have jumped the gun after reading this more carefully..Sorry @sleeptightAnsiC, I missed your comment about potentially having a generic script to replace C++ comments for various other files used in Nuklear.

My approach is a bit more complicated than the simple re.sub(r"//(.*)(\n|$)", "/* \\1 */\\2", str) one-liner in the header packer script! I think this results in nicer comments but might be overkill. Replacing comments also has some annoying edge cases, so my script might not work correctly for other files.

I'll keep my PR as a draft so you can at least take a look and let me know the best way to proceed.

[sleeptightAnsiC]

No worries. Yeah, #956 isn't really what I meant.

The way how Nuklear's dependencies are being vendored, fetched, and updated, it's a big problem in general. Multiple files for stb_image with C++ comments are just few out of many symptoms. Sorry, if my previous comment was unclear.

[TomJGooding]

My fault for diving in without understanding the broader context.

Just to note, a simple one-liner like re.sub(r"//(.*)(\n|$)", "/* \\1 */\\2", str) unfortunately won't work for stb_image.h. Replacing comments has some annoying edge cases - for example this fails on the very first line!

/* stb_image - v2.30 - public domain image loader - http:/* nothings.org/stb */
                                  no warranty implied; use at your own risk

[RobLoach]

Not the best solution, but we could also likely convince RandyGaul to port cute_png to C89, and use that. Doesn't have as many features, but is smaller.

[sleeptightAnsiC]

we could also likely convince RandyGaul to port cute_png to C89, and use that

IMO stb_image is just way too established at this point; we're using bunch of other STB* libraries anyway

also, the Nuklear's license is a literal copy-paste of STB's license but the cute_headers' license is not exactly the same (AFAIK it's still "compatible", but STB is just "more compatible", if you know what I mean)

@RobLoach However, you may find this one interesting: there's a kickass fork created by some NVidia employee at nvpro-samples/stb/tree/nv/all-fixes which is way ahead of upstream and confirmed to be actively maintained (nvpro-samples/stb#3 (comment)), and although Sean Barret is back (yay!) we may consider pulling from that fork in the future.

[RobLoach]

there's a kickass fork created by some NVidia employee at nvpro-samples/stb/tree/nv/all-fixes

Worth considering 👍... Think they'd be up for C89?

[sleeptightAnsiC]

Nope. I don't think so. We'll fix it ourself later. Replacing comments shouldn't be a problem.

[ColleagueRiley]

STB libraries do not support C89 with -pedantic, and it's far more than just comments. C89 has a ton of reistrictions, that I do not believe are worth abiding by unfortunately, and in practice tons of C89 compilers supported a ton of violations that -pedantic does not like, such as mixed declaration.

This is generally why a ton of the push for C89 is SUPER misguided and based on a ton of false naratives. In the end, using C89 is a determent to any modern codebase as they will be stuck with some of the worst design choices in C. It results in worse code overall, less maintainable, LESS PORTABLE OVERALL and less performant than a C99 alternative.

[sleeptightAnsiC]

STB libraries do not support C89 with -pedantic, and it's far more than just comments

This thread is mostly about stb_image and replacing comments in that file is enough https://godbolt.org/z/7PrnocWWE

the push for C89 is SUPER misguided and based on a ton of false naratives.

This is not a good place for such discussion. Take a look at #815