<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jul 27, 2016 at 12:47 AM, Rob Landley <span dir="ltr"><<a href="mailto:rob@landley.net" target="_blank">rob@landley.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 07/26/2016 02:22 AM, Toni Spets wrote:<br>
> @hifi <<a href="https://github.com/hifi" rel="noreferrer" target="_blank">https://github.com/hifi</a>> pushed 1 commit.<br>
><br>
> * 79ca172 <<a href="https://github.com/landley/toybox/commit/79ca172" rel="noreferrer" target="_blank">https://github.com/landley/toybox/commit/79ca172</a>> Less<br>
> magic, more printf<br>
<br>
It is so weird that github shows that commit as living in my tree<br>
instead of yours. I'm glad I only use that site for distribution of<br>
masters I keep elsewhere. (Is every commit in the whole of github in a<br>
single giant tree with a flat sha1hash collision namespace? I wonder how<br>
git catches that, or if the don't care until<br>
<a href="http://valerieaurora.org/hash.html" rel="noreferrer" target="_blank">http://valerieaurora.org/hash.html</a> goes red?)<br></blockquote><div><br></div><div>Well, that's odd.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Ok, looking at git diff 7918d9ff8..79ca172eb0d0<br>
<br>
The patch adds scr_init() allocating a screen buffer with 4 bytes per<br>
character... not an array of wide characters, but a char * with a width<br>
you multiply by 4. Ok? I have no idea how you're going to fit combining<br>
characters into there. You also don't appear to be storing color<br>
information, which is something less -R outputs...<br></blockquote><div><br></div><div>So the first reasoning was that the longest UTF-8 sequence that apparently is possible is 4 bytes so I threw a dice that at the worst possible case you draw 4 byte sequences all over and it would fit in the buffer. This didn't take into account TTY escapes yet which are in the same buffer as well.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
(So when you scroll the screen back up, you're not just outputting ESC[T<br>
and writing a new line of data at the top, but instead doing a memmove<br>
on your internal screen buffer? Why?)<br></blockquote><div><br></div><div>Simplification of the implementation. The screen is double buffered and the app/toy can update all or only some parts of the backbuffer and doesn't need to care how the update is sent to TTY.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Next function: the patch adds scr_linediff()... which has no comment<br>
explaining what it does. It's doing a memcmp on an an array of char<br>
pointers... scr_init() allocated exacty 2 char pointers, where does this<br>
array with a length come from...<br>
<br>
Ok, let's jump down to the last function added to this file and see if<br>
it calls the earlier ones... scr_printf() is printing into SCR_PTR().<br>
What's SCR_PTR? It's added to lib.h as<br>
<br>
#define SCR_PTR(scr) (scr->buf[0])<br>
<br>
And this is the only user of that macro anywhere in the code so why is<br>
it a macro?<br></blockquote><div><br></div><div>The idea was that a toy can access the current backbuffer with SCR_PTR if it wants to draw things directly instead of scr_printf(). Accessing it as scr->buf[0] is an ugly implementation details so I hid it.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
If you try to write to >x or >y it returns with no error. Otherwise,<br>
this function is a wrapper around:<br>
<br>
vsnprintf(SCR_PTR(scr) + (scr->l * y), scr->l - x, fmt, va);<br>
<br>
That finds the start of the y line (scr->l is scr->w*4 and I don't know<br>
why you needed to save that to avoid a <<2 in the users), does _not_<br>
advance x bytes (or bytes*4) into it, but truncates what we're printing<br>
to what presumably _would_ be the end of the line if we had advanced.<br>
<br>
Again, our screen width is *4 but we're not printing wide characters<br>
here. (Did you mean vswprintf?)<br></blockquote><div><br></div><div>It's still incomplete but yes, the only point of it is to safely wrap drawing lines on the screen so that it will truncate them properly keeping in mind UTF-8 sequences and TTY escapes. It also makes the toy code a bit more clean.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
ah, the second buffer _isn't_ to store color information, it's so you<br>
can diff what you think is on the screen (before the xterm was resized?)<br>
with what is now on the screen.<br></blockquote><div><br></div><div>Both buffers contain the whole screen and application always draws to the swapped backbuffer.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
So, next function up: scr_update() and THAT calls scr_linediff.<br>
Iterating through scr->h. What is scr->h? In lib.h it's "unsigned".<br>
Great. Back to look at scr_init()... Ah, it's screen height. Ok...<br>
<br>
+ char *line = scr_linediff(scr->buf[1] + (scr->l * i),<br>
+ scr->buf[0] + (scr->l * i),<br>
+ scr->l,<br>
+ i + 1);<br>
<br>
This is the only user of scr_linediff() in the code. Why it's a<br>
function, I couldn't tell you.<br></blockquote><div><br></div><div>It's hinted inline but it could be unrolled into the loop itself. It's mostly to keep the logic separate so I could focus on how would I do the diff and not look at the loop itself.<br><br></div><div>So scr_linediff tries to find the optimal sequence of TTY escapes and screen data to update a single line. It isn't complete but it does work with ASCII at this point and can draw something like a single or two character change anywhere on the screen quite efficiently. When you scroll the screen in less/vi it can have a huge impact on the speed if you're on 9600 or slower.<br><br></div><div>If you don't see it being useful enough, this can all be dumbed down a lot to the full line update. The good thing about the way I did it is it makes it dead simple for the toy to update the screen and it's also efficient on the TTY but it has the memory impact of double buffering.<br><br></div><div>The buffer size could be adjusted on-the-fly if someone is trying to draw something that fits on the screen but doesn't fit in the buffer but it would require a complete memcpy of every old line at that point for both buffers.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Ah, I see! Those for() loops at the start of scr_linediff() are<br>
open-coding strnlen(). A non-wide-character version, which will stop at<br>
the first zero byte (which wide characters tend to be full of).<br></blockquote><div><br></div><div>I assumed we worked with UTF-8 which doesn't have null bytes, right?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
And that's the chunk of time I had, I have a conference call in 12<br>
minutes so need to go prepare for that...<br>
<span class="HOEnZb"><font color="#888888"><br>
Rob<br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Toni Spets</div>
</div></div>