In response to the feedback, an alternative implementation of scr_* that doesn't pre-allocate too much and all redraw optimizations have been removed except complete lines.
On Wed, Jul 27, 2016 at 7:42 AM, Toni Spets <[email protected]> wrote: > On Wed, Jul 27, 2016 at 12:47 AM, Rob Landley <[email protected]> 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 > -- Toni Spets
From ca3d69c5acc4b501af25a5819c679508ae42f081 Mon Sep 17 00:00:00 2001 From: Toni Spets <[email protected]> Date: Mon, 25 Jul 2016 02:24:23 +0300 Subject: [PATCH] Simple implementation of less Still TODO: * Read only up to terminal height of lines at startup * Continue reading up to EOF as the user scrolls down This commit includes scr_* code to manage a text screen. It isn't yet quite ready regarding UTF-8 but it reserves enough space to hold any valid sequence. Other upcoming user for scr_* would be vi. NOTE: This version simplifies scr_* family of functions. --- lib/interestingtimes.c | 87 +++++++++++++++++++++++++++++++++++ lib/lib.h | 9 ++++ toys/pending/less.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+) create mode 100644 toys/pending/less.c diff --git a/lib/interestingtimes.c b/lib/interestingtimes.c index 62670cb..ee1e9b4 100644 --- a/lib/interestingtimes.c +++ b/lib/interestingtimes.c @@ -1,6 +1,7 @@ /* interestingtimes.c - cursor control * * Copyright 2015 Rob Landley <[email protected]> + * Copyright 2016 Toni Spets <[email protected]> */ #include "toys.h" @@ -241,3 +242,89 @@ void tty_sigreset(int i) tty_reset(); _exit(i ? 128+i : 0); } + +// at minimum, this allocates 16 kB of memory for buffers +struct screen *scr_init(void) +{ + int i; + struct screen *scr = xzalloc(sizeof(struct screen)); + + scr->width = 80; + scr->height = 25; + terminal_probesize(&scr->width, &scr->height); + + scr->buf[0] = xzalloc(sizeof(struct ptr_len) * scr->height); + scr->buf[1] = xzalloc(sizeof(struct ptr_len) * scr->height); + + // allocate exactly two screenfuls of ASCII + for (i = 0; i < scr->height; i++) { + scr->buf[0][i].len = scr->width; + scr->buf[0][i].ptr = xzalloc(scr->buf[0][i].len); + scr->buf[1][i].len = scr->width; + scr->buf[1][i].ptr = xzalloc(scr->buf[1][i].len); + } + + tty_esc("0m"); + tty_esc("2J"); + xset_terminal(1, 1, 0); + sigatexit(tty_sigreset); + + return scr; +} + +void scr_update(struct screen *scr) +{ + unsigned i, from_len, to_len; + + tty_esc("s"); // save cursor + + // diff and update screen + for (i = 0; i < scr->height; i++) { + struct ptr_len *from = &scr->buf[1][i]; + struct ptr_len *to = &scr->buf[0][i]; + + // calculate real length of lines + for (from_len = 0; from_len < from->len && ((char *)from->ptr)[from_len]; from_len++); + for (to_len = 0; to_len < to->len && ((char *)to->ptr)[to_len]; to_len++); + + if (to_len == from_len && memcmp(from->ptr, to->ptr, to_len) == 0) + continue; + + printf("\033[%u;1H\033[2K%.*s", i + 1, to_len, (char *)to->ptr); + } + + tty_esc("u"); // restore cursor + + // swap buffers + void *tmp = scr->buf[0]; + scr->buf[0] = scr->buf[1]; + scr->buf[1] = tmp; + + fflush(stdout); +} + +// TODO: make ANSI escape and UTF-8 aware (x offset & total visible length) +void scr_printf(struct screen *scr, unsigned x, unsigned y, const char *fmt, ...) +{ + va_list va; + int len; + struct ptr_len *l; + + if (x >= scr->width || y >= scr->height) return; + + l = &scr->buf[0][y]; + + va_start(va, fmt); + len = vsnprintf(l->ptr + x, l->len - x, fmt, va); + va_end(va); + + // if we truncated, try again with enough space + if (len >= l->len) { + free(l->ptr); + l->len = len + 1; + l->ptr = xzalloc(l->len); + va_start(va, fmt); + vsnprintf(l->ptr + x, l->len - x, fmt, va); + va_end(va); + } +} diff --git a/lib/lib.h b/lib/lib.h index f97940a..2e5dd0f 100644 --- a/lib/lib.h +++ b/lib/lib.h @@ -260,6 +260,15 @@ void tty_jump(int x, int y); void tty_reset(void); void tty_sigreset(int i); +struct screen { + unsigned width; + unsigned height; + struct ptr_len *buf[2]; +}; +struct screen *scr_init(void); +void scr_update(struct screen *scr); +void scr_printf(struct screen *scr, unsigned x, unsigned y, const char *fmt, ...); + // net.c int xsocket(int domain, int type, int protocol); void xsetsockopt(int fd, int level, int opt, void *val, socklen_t len); diff --git a/toys/pending/less.c b/toys/pending/less.c new file mode 100644 index 0000000..55f0eca --- /dev/null +++ b/toys/pending/less.c @@ -0,0 +1,121 @@ +/* less.c - opposite of more + * + * Copyright 2016 Toni Spets <[email protected]> + * + * No standard. + +USE_LESS(NEWTOY(less, "<1>1", TOYFLAG_USR|TOYFLAG_BIN)) + +config LESS + bool "less" + default n + help + usage: less FILE +*/ + +#define FOR_less +#include "toys.h" + +GLOBALS( + struct linestack *ls; + struct screen *scr; + + // view offset + int x_off; + int y_off; + int y_max; +) + +static void redraw(void) +{ + unsigned y; + + for (y = 0; y < TT.scr->height - 1; y++) { + if (y + TT.y_off < TT.ls->len) { + if (TT.x_off < TT.ls->idx[y + TT.y_off].len) { + scr_printf(TT.scr, 0, y, "%.*s", + TT.ls->idx[y + TT.y_off].len - TT.x_off, + (char *)TT.ls->idx[y + TT.y_off].ptr + TT.x_off); + } else { + scr_printf(TT.scr, 0, y, ""); + } + } else { + scr_printf(TT.scr, 0, y, "~"); + } + } + + scr_printf(TT.scr, 0, TT.scr->height - 1, toybuf); + scr_update(TT.scr); +} + +void less_main(void) +{ + char scratch[16]; + unsigned run = 1; + + if (!isatty(0) || !isatty(1)) + error_exit("no tty"); + + if (!(TT.ls = linestack_load(*toys.optargs))) + TT.ls = xzalloc(sizeof(struct linestack)); + + TT.scr = scr_init(); + + TT.y_max = TT.ls->len - TT.scr->height + 1; + if (TT.y_max < 0) TT.y_max = 0; + + sprintf(toybuf, "%s%s", *toys.optargs, (TT.y_off == TT.y_max ? " (END)" : "")); + + *scratch = 0; + do { + int key; + + printf("\033[%u;%luH", TT.scr->height, strlen(toybuf) + 1); + redraw(); + + key = scan_key(scratch, -1); + + sprintf(toybuf, ":"); + + switch (key) { + case 'q': run = 0; break; + case 0x100: // arrow up + case 'k': if (TT.y_off > 0) TT.y_off--; break; + case 0x101: // arrow down + case 0xD: // return + case 'j': if (TT.y_off + TT.scr->height <= TT.ls->len) TT.y_off++; break; + case 0x102: // arrow right + case 'l': TT.x_off += TT.scr->width; break; + case 0x103: // arrow left + case 'h': if (TT.x_off > 0) TT.x_off -= TT.scr->width; break; + case 0x15: // ^U + if (TT.y_off - (int)(TT.scr->height / 2) >= 0) TT.y_off -= TT.scr->height / 2; + else TT.y_off = 0; + break; + case 0x4: // ^D + if (TT.y_off + (TT.scr->height / 2) <= TT.y_max) TT.y_off += TT.scr->height / 2; + else TT.y_off = TT.y_max; + break; + case 0x2: // ^B + case 0x104: // pg up + if (TT.y_off - (int)TT.scr->height >= 0) TT.y_off -= TT.scr->height; + else TT.y_off = 0; + break; + case 0x6: // ^F + case 0x20: // space + case 0x105: // pg dn + if (TT.y_off + TT.scr->height <= TT.y_max) TT.y_off += TT.scr->height; + else TT.y_off = TT.y_max; + break; + default: + if (CFG_TOYBOX_DEBUG) + sprintf(toybuf, "Unhandled key %d (%Xh)", key, key); + } + + if (TT.y_off == TT.y_max) + sprintf(toybuf, "(END)"); + + } while (run); + + tty_reset(); +} -- 2.7.4
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
