Why on earth are we hashing window names?

[ronaaron]

The "name" is only used as an id for the window. So we hash it on creation of the window. Then, nk_find_window() (which is called all the time), looks for a matching window hash (recreating the hash): and then checks the name (running strlen again!).

Would it not make a whole lot more sense to say, "use a unique integer identifier" for each window (or other item that needs an id), and just store that id ... thereby not needed to do a hash nor compare strings nor do strlen just to find a window?

You could even have a "nk_unique_id()" to return, per context, a new int that is unique (just increment a counter). No need to be fancy.

What am I missing?

[Hejsil]

One benefit to the approach right now is that windows with titles can use their titles as identifiers, which is convenient (I guess). It is probably also easier for humans to pick a unique string rather than a unique integer ("color_picker", "file_dialog", "main_window" vs 42 65 33 42 (ops, picked 42 twice)). Other than that, I don't think string "ids" really buys us anything.

You could even have a "nk_unique_id()" to return, per context, a new int that is unique (just increment a counter). No need to be fancy.

There are a few problems with this approach, depending on how it is actually implemented. If it is just an incrementing counter on each call, then a simple program does not work:

while (running) {
    // On each iteration of the loop, nk_unique_id returns a new, unique id, so
    // the window will have a different id each frame. No good.
    if (nk_begin(..., nk_unique_id())) {}
}

Otherwise, if it is a counter that resets at the start of each frame, then this becomes a problem:

while (running) {
    // This condition determins which of the two windows should be show. Both
    // should have unique ids from eachother, but `nk_unique_id` will always return
    // `0` here, as it is only ever called once per frame.
    if (condition) {
        if (nk_begin(..., nk_unique_id())) {}
    } else {
        if (nk_begin(..., nk_unique_id())) {}
    }
}

Ofc, the above might be an invalid use of that API, but it is a mistake that is easy to make, so I'm not a fan

[ronaaron]

OK, scratch the "nk_unique_id()" :)

Nevertheless, I find it irritating that we have so much fairly inefficient code (all the strlen and murmur_hash business).

[Hejsil]

I can see your point there. Ofc, if one actually wants to be maximally efficient, Nuklear should allow the user to manage the window state themselves. Allowing one to store the state on the stack, heap, global memory, whatever they want. This also solves the "id" problem (The window state is its id), but makes the API more cumbersome to use. I think, if we want to walk down a path that changes how this id stuff works, we might as well explore ideas like this :)

Anyway, not sure if this is something we can realistically change at this point (see #81, for another big MR that is kinda stuck in limbo)

[ronaaron]

Yeah, I'm actually changing (re #81) in my own repo because all that strlen stuff makes no sense, and in my use-case I always have the string length at hand. Just for this reason.

[Elkantor]

And just using the pointer address value of the string isn't enough? I mean, you just cast the pointer address value as a uintptr_t, and you then use this unique identifier to authenticate the window.

If you use whatever string in the codebase, the compiler will automatically create a global variable for that string. So you can get its address, and use it as a value to determine a unique element.