Misleading comment about nuklear math in SDL3 demo?

[TomJGooding]

The comment about nuklear's math functions in the SDL3 demo could be misleading:

Nuklear/demo/sdl3_renderer/main.c

Lines 80 to 84 in e5a3839

	/* SDL can also provide us with math functions, but beware that Nuklear's own
	* implementation can be slightly faster at the cost of some precision */
	#define NK_INV_SQRT(f) (1.0f / SDL_sqrtf(f))
	#define NK_SIN(f) SDL_sinf(f)
	#define NK_COS(f) SDL_cosf(f)

The nuklear math functions were tailored for internal use only, since "nuklear only needs sine and cosine to generate circles" as documented in nuklear_math.c.

This comment might suggest that nuklear's implementations are a faster replacement for general purpose math?

See #960 for context.

[sleeptightAnsiC]

I'm the author of that comment. Not entirely sure why do you think it would suggest that?

It says how SDL's math primitives compare to the Nuklear's ones. It doesn't say anything about reusing them for general purpose math. My intention was to warn that providing your own symbols may not always be worth it.

Don't get me wrong - I'll be fine with removing the comment if it's really confusing. However, it's hard for me to believe it caused whole #960 situation. IIRC docs also mention those functions being simple approximations.

Am I missing something?

[sleeptightAnsiC]

How about

 /* SDL can supply math functions for Nuklear, but beware you may not always want it.
  * Nuklear internally uses imprecise approximations, which can be slightly faster. */

Would it still be misleading? If yes, then I guess we have to remove it.

[TomJGooding]

I might be overthinking this. My thought process was that if even the Nuklear maintainers were caught out by the limitations of these math functions, someone new to Nuklear could misinterpret this comment?]

Feel free to close this if you don't see how the wording could possibly be misleading! But I think something like your suggestion could help make it clearer that these are designed for internal use.

[sleeptightAnsiC]

Smells like XY Problem ?

The real issue here is people not understanding what's the difference between "public" and "private" symbols in this library, and because everything/everyone is using amalgamation, situations like #960 go unnoticed until it breaks something else.

Only symbols from src/nuklear.h can be public. The functions must be marked as NK_API in order to be public. Only those are safe to use in your code.

Everything else, e.g. macros from src/nuklear_internal.h or symbols marked as NK_INTERN/ NK_LIB, such as match primitives, those are all private and meant for internal library use. Using these outside of library breaks API contract. It simply shouldn't have happened in the first place.

Libraries such as stb_image solve this problem by having different prefix for all private symbols (see explanation here) but Nuklear doesn't use such convention and it's too late to add one.

Demos/examples are riddled with reuse of private Nuklear symbols. TBH, I already knew #942 was wrong back when it was submitted, same with #911 (comment) - it just wasn't worth arguing about this back then. If people don't understand the basic concept of private vs public, there is really no point, unless it seriously breaks something, like in case of #959

[sleeptightAnsiC]

I think something like your suggestion could help make it clearer

@TomJGooding Feel free to submit PR. If you see more comments worth changing in demo/sdl3_renderer, make sure to tweak them too. I do care about this backend a lot. Some people find it extremely useful.

[TomJGooding]

Closing as I am probably just overthinking this. Sorry for the noise and thanks for taking the time to talk it through.