stb_image "inline" mode for distribution in header-only libraries

[rcurtin]

Is your feature request related to a problem? Please describe.

We use stb_image and related tools as part of mlpack (https://github.com/mlpack/mlpack), and there we are trying to revamp our library such that it is truly header-only. But actually stb_image's design presents a problem here: currently one must define STB_IMAGE_IMPLEMENTATION in one specific non-header file so that the actual implementation of the stb_image functions are not defined in multiple places.

That makes a big distribution problem for us: we can't distribute as header-only in any easy way if we also have to tell the user that, e.g., when they include mlpack, they must also include stb_image.h but only after defining STB_IMAGE_IMPLEMENTATION, and only once in a translation unit.

Describe the solution you'd like

Perhaps it would be possible to allow an additional macro to be defined, like STB_IMAGE_INLINED_IMPLEMENTATION, which would mark all stb_image implementation functionality as inline, thus allowing it to exist multiple times in the same translation unit.

e.g.:

static void stbi__float_postprocess(float *result, int *x, int *y, int *comp, int req_comp)

might become

STBI_INLINE stbi__float_postprocess(float *result, int *x, int *y, int *comp, int req_comp)

where STBI_INLINE (which is a poor name) is static if STB_IMAGE_INLINED_IMPLEMENTATION is not defined, and inline if it is. Or, something to this effect.

Describe alternatives you've considered

I'm not really sure there are other ways to address this issue. But, perhaps my creativity is stifled and you might be able to think of a better solution.

Additional context

Thank you for STB! We have found it very useful. :+1:

[nothings]

I guess the issue is that you're a C++ library and stb_image is a C library (with compatibility with C++). C++ header-file-only libraries are very straightforward to make because of the semantics of inline (indeed, my innovation with stb libraries was taking this existing C++ library idea and figuring out a way to make it work in C). The semantics of "inline" in C are not the same as C++, so I'm not 100% sure the same solution would work, although if it were an optional #ifdef'd option that wouldn't matter.

Anyway, I'm not a big fan of the idea, because then it's something else we have to be careful we don't break during maintenance, even though it's something we don't use ourselves. I understand your problem, though, and it doesn't make much sense to fork it just for that, either. Hmm.

[rcurtin]

Yeah, definitely it would be possible to fork and add something like that, but that would be a bunch of extra overhead for us so I'd like to avoid it if possible. You could perhaps minimize any maintenance overhead by just marking everything with, e.g., an STBI_EXTRA_ATTRS macro, and then functionality like what I'm looking for could either be left undocumented or mentioned as a set STBI_EXTRA_ATTRS to any extra attributes you want stbi functions to be compiled with or similar; in my case, that would be inline. I suppose for Windows libraries, some users might want to do strange things with dllexport or dllimport or whatever, so perhaps something like this would be more generally useful too.

Let me know what you think---I'm happy to whip up a patch if you're cool with the general solution idea (or something similar).

[nothings]

Yeah, the extra maintenance is just literally having to add that 'attribute tag' to everything, it's just it'll be easy to forget it. I guess we don't see a lot of new functions these days, though, since we no longer add new file formats.

[mackron]

Something I've started doing in my single file libraries (which I was originally hesitant about, but has actually been quite good) is to wrap the implementation section in an include guard:

#if defined(MINIAUDIO_IMPLEMENTATION) || defined(MA_IMPLEMENTATION)
#ifndef miniaudio_c
#define miniaudio_c

...

#endif /* miniaudio_c */
#endif /* MINIAUDIO_IMPLEMENTATION */

Would that perhaps help in this scenario?

[nothings]

Something I've started doing in my single file libraries (which I was originally hesitant about, but has actually been quite good) is to wrap the implementation section in an include guard:

It's a good idea, but it wouldn't help in this scenario.

[rcurtin]

It seems that I have misunderstood static when I opened this issue. To the best of my understanding, a static function in either C or C++ is visible only to that translation unit. Thus, with the include guard trick suggested by @mackron, I can ensure that an STB function's implementation appears only once in a translation unit, and using STB_IMAGE_STATIC will ensure that all functions have static linkage. This should solve the original problem! Some basic tests seem to verify that this works fine.

Let me know if I overlooked something---otherwise, I think it's fine to close this issue. :+1:

Thank you for the quick responses!

[nothings]

Using STB_IMAGE_STATIC will cause EVERY translation unit that include stb_image.h to gets its own static copy of the entire library, including every image format. There is no guarantee the unused copies will be stripped out, so this would bloat the object files and executables unreasonably large. This is why I didn't mention that option.

Also some compilers at some warning settings will warn about unused static functions, which matters when you make a header-only library and have to cope with the warning settings made by clients of your library.

[rcurtin]

Yeah, which is unfortunate but I think overall it's okay for our use case, at least. I'll go ahead and close this then, because it seems like there's nothing else to solve here. Thank you again for your help. :+1: