add tb_get_cell
#65
add tb_get_cell
#65
Conversation
| if_not_init_return(); | ||
| int rv; | ||
| struct tb_cell *cellp = NULL; | ||
| rv = cellbuf_get(back ? &global.back : &global.front, x, y, &cellp); |
There was a problem hiding this comment.
if x or y are out of bounds is it going to fail?
I think it needs check as in tb_set_cell -> if_err_return(...)
There was a problem hiding this comment.
Yes cellbuf_get will return an error code if coords are out of bounds. There's a test assertion for this case here: https://github.com/termbox/termbox2/pull/65/files#diff-ff257682d1f1e118ed7186fee4abce09fca625c01728795e04d3c9a4fe95eadfR19
|
I tested change and it works as supposed to. |
| int rv; | ||
| struct tb_cell *cellp = NULL; | ||
| rv = cellbuf_get(back ? &global.back : &global.front, x, y, &cellp); | ||
| if (cellp) memcpy(cell, cellp, sizeof(*cell)); |
There was a problem hiding this comment.
for better performance can we avoid memcpy and just return pointer to cell like in cellbuf_get function?
There was a problem hiding this comment.
Good point. I figure it's a trade-off between safety and perf. I was going for by-value semantics here (like struct tb_cell tb_get_cell(int x, int y) but reserving the return value for error codes per usual). Then the user can do whatever with their cell and it won't affect tb internals.
If the caller is concerned about an extra memcpy maybe we can assume they'd also want to avoid function overhead per-cell, so maybe tb_cell_buffer is for the perf critical use case? But tb_cell_buffer doesn't support returning the front-buffer currently. We could add another function for that (not my pref to bloat the API -- I already regret adding some like tb_set_func). Or we could change the signature of tb_cell_buffer to int tb_cell_buffer(int back, struct tb_cell **cellbuf) (also not my pref to break ABI).
Or like you suggest maybe tb_get_cell returns the pointer to our memory, and the caller could simply ask for 0, 0 if they want to index the cellbuf themselves for perf. We can warn users in documentation that mutating any memory results in undefined behavior. That seems like the best option to me.
There was a problem hiding this comment.
You have point, I forgot about safety issues that could appear, but I will also prefer the latter. It is going to be more flexible code where one can get what one needs.
Useful or bloat?
There's currently no way to fetch cells from the front buffer. This does close that gap. Otherwise this is a safer version of
tb_cell_bufferwhich returns a pointer to the entire back buffer.