[Toybox] [landley/toybox] Simple implementation of less (#39)

[Toni Spets]

On Wed, Jul 27, 2016 at 12:47 AM, Rob Landley <rob at landley.net> wrote:

> On 07/26/2016 02:22 AM, Toni Spets wrote:
> > @hifi <https://github.com/hifi> pushed 1 commit.
> >
> >   * 79ca172 <https://github.com/landley/toybox/commit/79ca172> Less
> >     magic, more printf
>
> It is so weird that github shows that commit as living in my tree
> instead of yours. I'm glad I only use that site for distribution of
> masters I keep elsewhere. (Is every commit in the whole of github in a
> single giant tree with a flat sha1hash collision namespace? I wonder how
> git catches that, or if the don't care until
> http://valerieaurora.org/hash.html goes red?)
>

Well, that's odd.


>
> Ok, looking at git diff 7918d9ff8..79ca172eb0d0
>
> The patch adds scr_init() allocating a screen buffer with 4 bytes per
> character... not an array of wide characters, but a char * with a width
> you multiply by 4. Ok? I have no idea how you're going to fit combining
> characters into there. You also don't appear to be storing color
> information, which is something less -R outputs...
>

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.


>
> (So when you scroll the screen back up, you're not just outputting ESC[T
> and writing a new line of data at the top, but instead doing a memmove
> on your internal screen buffer? Why?)
>

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.


>
> Next function: the patch adds scr_linediff()... which has no comment
> explaining what it does. It's doing a memcmp on an an array of char
> pointers... scr_init() allocated exacty 2 char pointers, where does this
> array with a length come from...
>
> Ok, let's jump down to the last function added to this file and see if
> it calls the earlier ones... scr_printf() is printing into SCR_PTR().
> What's SCR_PTR? It's added to lib.h as
>
>   #define SCR_PTR(scr) (scr->buf[0])
>
> And this is the only user of that macro anywhere in the code so why is
> it a macro?
>

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.


>
> If you try to write to >x or >y it returns with no error. Otherwise,
> this function is a wrapper around:
>
>   vsnprintf(SCR_PTR(scr) + (scr->l * y), scr->l - x, fmt, va);
>
> That finds the start of the y line (scr->l is scr->w*4 and I don't know
> why you needed to save that to avoid a <<2 in the users), does _not_
> advance x bytes (or bytes*4) into it, but truncates what we're printing
> to what presumably _would_ be the end of the line if we had advanced.
>
> Again, our screen width is *4 but we're not printing wide characters
> here. (Did you mean vswprintf?)
>

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.


>
> ah, the second buffer _isn't_ to store color information, it's so you
> can diff what you think is on the screen (before the xterm was resized?)
> with what is now on the screen.
>

Both buffers contain the whole screen and application always draws to the
swapped backbuffer.


>
> So, next function up: scr_update() and THAT calls scr_linediff.
> Iterating through scr->h. What is scr->h? In lib.h it's "unsigned".
> Great. Back to look at scr_init()... Ah, it's screen height. Ok...
>
> +    char *line = scr_linediff(scr->buf[1] + (scr->l * i),
> +                              scr->buf[0] + (scr->l * i),
> +                              scr->l,
> +                              i + 1);
>
> This is the only user of scr_linediff() in the code. Why it's a
> function, I couldn't tell you.
>

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.

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.

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.

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.


>
> Ah, I see! Those for() loops at the start of scr_linediff() are
> open-coding strnlen(). A non-wide-character version, which will stop at
> the first zero byte (which wide characters tend to be full of).
>

I assumed we worked with UTF-8 which doesn't have null bytes, right?


>
> And that's the chunk of time I had, I have a conference call in 12
> minutes so need to go prepare for that...
>
> Rob
>



-- 
Toni Spets
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20160727/7d9d57e0/attachment-0002.htm>