nothings / stb

stb single-file public domain libraries for C/C++
https://twitter.com/nothings
Other
26.58k stars 7.7k forks source link

stb_image_write: possibility to avoid use of global variables #1087

Open flriancu opened 3 years ago

flriancu commented 3 years ago

Hi!

Currently, there are some global variables in stb_image_write, including:

stbi_write_png_compression_level
stbi_write_force_png_filter
stbi_write_tga_with_rle

To avoid using these three particular variables, I made a patch that:

STBIWDEF int stbi_write_png_to_func_prefs(stbi_write_func func, void context, int w, int h, int comp, const void data, int stride_bytes, const stbiw_png_prefs prefs); STBIWDEF int stbi_write_tga_to_func_prefs(stbi_write_func func, void context, int w, int h, int comp, const void data, const stbiw_tga_prefs prefs);

- adds one new internal function:

STBIWDEF unsigned char stbi_write_png_to_mem_prefs(const unsigned char pixels, int stride_bytes, int x, int y, int n, int out_len, const stbiw_png_prefs prefs)

- and changes one internal function:

static int stbi_write_tga_core(stbi__write_context s, int x, int y, int comp, void data, const stbiw_tga_prefs *prefs)



The `stbiw_*_prefs` are structures that will allow backward-compatibility in the future via their `struct_size` member.

The pull request should link automatically to this issue.

Please let me know what you think. Thanks!
nothings commented 3 years ago

why

flriancu commented 3 years ago

One immediate advantage is, you can call stbi_write_png_prefs from multiple threads with different compression levels. Unless something major escapes me, these *_prefs functions are thread-safe (except for the remaining issue of stbi__flip_vertically_on_write).

nothings commented 3 years ago

If the goal is to introduce thread-compatability for these global variables, I'd prefer to do it the way we do it in stb_image.c with thread-locals, rather than by significantly increasing the number of main API entrypoints, which adds a lot of complexity for users to understand with very little benefit.

nothings commented 3 years ago

But really it might be better just to remove these variables from the public API (but possibly keep them as undocumented), as I doubt they receive signifcant usage.

flriancu commented 3 years ago

If the goal is to introduce thread-compatability for these global variables

Yes, sorry for not mentioning that from the start.

I can file a PR with thread-local variables, would that be okay for the short term?

nothings commented 3 years ago

Like I said, I think it's probably better to remove the public API to these entirely, as I doubt they're used at all except in special circumstances.

justinmeiners commented 3 years ago

@nothings If it's helpful to know, I use stbi_write_png_compression_level to set a lower (and hopefully faster?) quality for in-memory compression and another for disk.