Adds CRC calculation to draw command buffer

[awschult002]

Adding a rolling CRC caluclation based off the commands going into the draw buffer allows for really quick and cheap difference detection. That is, for an application to know if it needs to nk_convert and paint the screen again. All that needs to be done now, is to store the previous CRC value and compare that to the latest value to determine if you should draw or not.

[riri]

Nice addition!

But... :)

I would prefer this is conditionally included with a define (aka NK_DRAW_BUFFER_CRC) because it induces some static memory for the crc table and additional computation on each draw command push, a waist for those not using this optimization. Then a minor version bump will be needed.

[awschult002]

@riri
conditional compilation added. Where would be a good place to discuss whether or not this should be a default option?

[riri]

Here seems to be the perfect place to discuss about it. Others can join if they have their own idea about it.

My argument is that by default, Nuklear does not keep track of what was rendered, and we should stay on that line.
Adding the checksum as an option makes it a nice shortcut for those using it and had to either make a copy of the draw buffer, or do a checksum externally, but still having the option to not do it (for whatever reason).

Dunno, maybe I'm a bit conservative :)
@RobLoach what you think?

[awschult002]

This update still keeps with that ethos. Nuklear isn't tracking anything between separate nk_begin calls. Within the call, nuklear updates the draw buffer. so its holding all of the information internally. All of that gets cleared on the next nk_begin; thus state is not retained (a feature of this framework). The addition of the CRC is the same thing. A single integer has been allocated next to the draw buffer and a single XOR operation is performed on that integer for every draw call just before the draw call is placed into the buffer. That integer CRC is cleared on every call to nk_begin thus state is not retained. its the same idea as the buffer. As such, if the user wants to retain the CRC state, they have to do that on their own by just copying the CRC value just before they nk_convert.

computationally, the CRC is a lookup operation and a XOR arithmetic operation. super fast. super cheap. This is a very common technique used in resource constrained embedded systems everywhere. So common, that most processors have SIMD or AVX or dedicated hardware to support it.

I will concede that a 256 element array of 32 bit integers is a little bit large (1KB). but even in embedded systems with memory constraints, if you can't find 1KB of space, then you problem went wrong somewhere else. (makes me wonder how large the draw buffer gets).

Also, I am sure that the compiler would optimize a lot of this out, especially if the developer never calls the function to retain the CRC value.

I would be happy to do some performance checks on this if you are concerned.

[awschult002]

also. let me know if I am coming off as too combative. I can get passionate sometimes. Ultimately, this is your project, not mine. I will concede to your will because I believe in this project and want to be a respected contributor. I respect your opinions and comments even if I push back strongly.

I really do love this project. It is the best gui framework that I have come across for C and I really do want to see it excel.

I share your opinion on being conservative; which is why i enjoy this single header, render backend agnostic approach. I work in embedded systems and have chosen this project for most gui work because of how small and efficient it is. I can really tailor this thing into many memory constrained and processor constrained spaces. Which is actually the inspiration for this update. I really don't want to run nk_convert all of the time. i want to be efficient and process it only when needed. The current solution is to scan the entire draw buffer after its been filled, and compare that with a stored copy of the previous buffer.... which means that for every update, you need to scan the old buffer, scan the new buffer, then scan the new buffer again to memcpy it to the old spot? That is a lot of lookup and a lot of looping on every cycle. How big can these draw buffers get? how many calls? thousands?

With the CRC approach, we take advantage of the fact that the frame work already loops through the buffer (to fill it with draw calls in the first place). Then we just add a single basic add instruction, and magically, our double buffering and double loop comparison with memcpy disappears. Reducing our memory complexity and reducing our loop complexity.

[RobLoach]

Disabled by default seems most appropriate, as not every usage of Nuklear requires validation of the draw buffer.

Two other considerations...

1. It looks like this isn't thread-safe as it uses a NK_BUFFER_CRC global. Possibly move that variable into struct nk_command_buffer, or elsewhere.
2. If this is disabled by default, should we also skip the definition of crc32c_table?

[awschult002]

The CRC Table definition should be guarded by the NK_DRAW_BUFFER_CRC, so if it is disabled, that shouldn't get compiled in.

[awschult002]

but you are right, this is not thread safe and i will make the update to move it.

[awschult002]

i am trying to update to use the struct nk_command_buffer however, i am having a little trouble understanding where in the nk_context the command buffer resides? at the moment, a struct nk_context contains a nk_buffer memory and a nk_command_buffer overlay. The memory doesn't have any comments or descriptions and is ultimately what gets nk_buffer_clear() or nk_buffer_reset. But i don't see where the overlay gets cleared. Also... is memory where the draw commands are stored?? is the memory buffer just an open buffer where the nk_command_buffer gets allocated to? are all of the draw commands considered overlay?

[awschult002]

But i don't see where the overlay gets cleared. Also... is memory where the draw commands are stored??

i found that you memset the overlay to 0 later in that function. Which makes sense. I am going to assume that the memory buffer in the ctx is just a pool where the draw buffer overlay is being allocated to.

[riri]

also. let me know if I am coming off as too combative. I can get passionate sometimes. Ultimately, this is your project, not mine. I will concede to your will because I believe in this project and want to be a respected contributor. I respect your opinions and comments even if I push back strongly.

Nothing wrong with being passionate, I'm actually in the same vein, but with different output.

i am trying to update to use the struct nk_command_buffer however, i am having a little trouble understanding where in the nk_context the command buffer resides? at the moment, a struct nk_context contains a nk_buffer memory and a nk_command_buffer overlay. The memory doesn't have any comments or descriptions and is ultimately what gets nk_buffer_clear() or nk_buffer_reset. But i don't see where the overlay gets cleared. Also... is memory where the draw commands are stored?? is the memory buffer just an open buffer where the nk_command_buffer gets allocated to? are all of the draw commands considered overlay?

Yes Nuklear is meant to be used anywhere, even where dynamic allocation is not available, so a memory buffer (which can be initialized either dynamically or statically) is used for all the things that cannot be determined in advance.

draw commands are inverted in that buffer, but it's transparent to the user if using default allocation.

[awschult002]

Version number bumped (i think i did it right). PR should be good to go.

[awschult002]

PR is not good. Turns out that I don't understand how to work with the buffer system

[awschult002]

I am asking for some help in figuring out a couple of bugs. When I try to run an example, I get Seg fault because the CRC read function gets ran at the end of all of the draw calls.... but the ctx->current pointer is NULL at that point? I don't understand how we can go through the draw loop, then end up with a null pointer reference where all of the current draw command should be. Do we just try to draw a bunch of stuff, even though its null while we wait for someone else to allocate it?

Note: I was testing in the sdl_renderer demo.

[awschult002]

okay. i have figured out some of it. it turns out that nk_end() wipes out the ctx->current pointer. which is why my stuff is failing

[awschult002]

I think I have it figured out. All window command buffers are "finished" upon nk_end being called. Which means they are no longer available for use in any direct manner. However their list of draw calls just gets appended to the context's "global" draw buffer so that the NK Convert can just run on one long array regardless of how many buffers or windiws or popups got created.

I think the proper fix for this would be to have the CTX store the CRC value, then update that on NK_finish_buffer

[awschult002]

i got the CRCs mostly working again. i had to add CRC into the context, then update that as the ctx->window->current->buffer would get deleted every time nk_end() was called. The SDL_Renderer demo seems to work fine and it does printout the CRC values. The values change almost perfectly. There seems to be an issue with the CRC not changing for some "highlights" when you mouse over an object. Some mouse over changes the crc some mouse over does not. I need to investigate why this is happening.

[riri]

There seems to be an issue with the CRC not changing for some "highlights" when you mouse over an object. Some mouse over changes the crc some mouse over does not. I need to investigate why this is happening.

Isn't it because redraw becomes too cheap with only one loop? If I remember well, hovered status is marked but drawn in next loop (I remember a similar thing with combobox dropdown on a different approach, running next loop only on platform events)

[awschult002]

Oh. Interesting. I guess that makes sense. It would be the same draw call, just a different parameter.

So I will have to CRC more than just the pointer... I need to learn more