From 2c5d938cbdf309a4f676115fedef9ff79eeb3638 Mon Sep 17 00:00:00 2001 From: Nick Black Date: Sat, 24 Apr 2021 13:08:09 -0400 Subject: [PATCH] Paint sprixels bottom-to-top (#1589) * Paint sprixels in order, bottom-to-top We don't want to have to track sprixel order whenever someone moves an ncplane, so just keep a list growing backwards as we pass top-to-bottom in notcurses_render_internal(). Each time we hit a sprixel plane, splice it out of the sprixel list, and add it to the front of our temporary list. When we hit the bottom, stick this temporary list on the end of our existing list (any such planes are to be deleted, which comes before drawing). Closes #1575. * reorder collected sprixellist; solves kitty but breaks sixel =/ #1575 * remove debugging cruft * [rust] fix up mergedown mutability --- USAGE.md | 4 ++ cffi/src/notcurses/build_notcurses.py | 4 +- doc/man/man3/notcurses_plane.3.md | 4 +- include/notcurses/notcurses.h | 1 + rust/src/plane/methods.rs | 4 +- src/lib/internal.h | 8 ++-- src/lib/render.c | 55 ++++++++++++++++++++++++--- src/lib/sprite.c | 10 +++-- src/tests/bitmap.cpp | 12 +++++- 9 files changed, 83 insertions(+), 19 deletions(-) diff --git a/USAGE.md b/USAGE.md index 78d1e3174..c1c1dc468 100644 --- a/USAGE.md +++ b/USAGE.md @@ -811,6 +811,10 @@ struct ncplane* ncplane_dup(struct ncplane* n, void* opaque); // this operation. Do not supply the same plane for both 'src' and 'dst'. int ncplane_mergedown(struct ncplane* restrict src, struct ncplane* restrict dst); +// If 'src' does not intersect with 'dst', 'dst' will not be changed, but it is +// not an error. If 'dst' is NULL, the operation will target the standard plane. +int ncplane_mergedown_simple(const ncplane* restrict src, ncplane* restrict dst); + // Erase every cell in the ncplane, resetting all attributes to normal, all // colors to the default color, and all cells to undrawn. All cells associated // with this ncplane are invalidated, and must not be used after the call, diff --git a/cffi/src/notcurses/build_notcurses.py b/cffi/src/notcurses/build_notcurses.py index ba9442e42..7124faa13 100644 --- a/cffi/src/notcurses/build_notcurses.py +++ b/cffi/src/notcurses/build_notcurses.py @@ -97,8 +97,8 @@ bool notcurses_canutf8(const struct notcurses* nc); int notcurses_mouse_enable(struct notcurses* n); int notcurses_mouse_disable(struct notcurses* n); int ncplane_destroy(struct ncplane* ncp); -int ncplane_mergedown(const struct ncplane* src, struct ncplane* dst, int begsrcy, int begsrcx, int leny, int lenx, int dsty, int dstx); -int ncplane_mergedown_simple(const struct ncplane* restrict src, struct ncplane* restrict dst); +int ncplane_mergedown(struct ncplane* src, struct ncplane* dst, int begsrcy, int begsrcx, int leny, int lenx, int dsty, int dstx); +int ncplane_mergedown_simple(struct ncplane* restrict src, struct ncplane* restrict dst); void ncplane_erase(struct ncplane* n); int ncplane_cursor_move_yx(struct ncplane* n, int y, int x); void ncplane_cursor_yx(struct ncplane* n, int* y, int* x); diff --git a/doc/man/man3/notcurses_plane.3.md b/doc/man/man3/notcurses_plane.3.md index fcb069ad2..5d050a4b8 100644 --- a/doc/man/man3/notcurses_plane.3.md +++ b/doc/man/man3/notcurses_plane.3.md @@ -186,9 +186,9 @@ typedef struct ncplane_options { **void notcurses_drop_planes(struct notcurses* ***nc***);** -**int ncplane_mergedown(const struct ncplane* ***src***, struct ncplane* ***dst***, int ***begsrcy***, int ***begsrcx***, int ***leny***, int ***lenx***, int ***dsty***, int ***dstx***);** +**int ncplane_mergedown(struct ncplane* ***src***, struct ncplane* ***dst***, int ***begsrcy***, int ***begsrcx***, int ***leny***, int ***lenx***, int ***dsty***, int ***dstx***);** -**int ncplane_mergedown_simple(const struct ncplane* restrict ***src***, struct ncplane* restrict ***dst***);** +**int ncplane_mergedown_simple(struct ncplane* restrict ***src***, struct ncplane* restrict ***dst***);** **void ncplane_erase(struct ncplane* ***n***);** diff --git a/include/notcurses/notcurses.h b/include/notcurses/notcurses.h index 24adcf690..b6c00af40 100644 --- a/include/notcurses/notcurses.h +++ b/include/notcurses/notcurses.h @@ -1965,6 +1965,7 @@ API int ncplane_mergedown_simple(struct ncplane* RESTRICT src, // is an error to define a target origin such that the projected subregion is // not entirely contained within 'dst'. Behavior is undefined if 'src' and // 'dst' are equivalent. 'dst' is modified, but 'src' remains unchanged. +// neither 'src' nor 'dst' may have sprixels. API int ncplane_mergedown(struct ncplane* RESTRICT src, struct ncplane* RESTRICT dst, int begsrcy, int begsrcx, int leny, int lenx, diff --git a/rust/src/plane/methods.rs b/rust/src/plane/methods.rs index f4773c15a..642a91af2 100644 --- a/rust/src/plane/methods.rs +++ b/rust/src/plane/methods.rs @@ -998,7 +998,7 @@ impl NcPlane { /// *C style function: [ncplane_mergedown()][crate::ncplane_mergedown].* pub fn mergedown( &mut self, - source: &NcPlane, + source: &mut NcPlane, source_y: NcDim, source_x: NcDim, len_y: NcDim, @@ -1038,7 +1038,7 @@ impl NcPlane { // // TODO: maybe create a reversed method, and/or an associated function, // for `mergedown` too. - pub fn mergedown_simple(&mut self, source: &NcPlane) -> NcResult<()> { + pub fn mergedown_simple(&mut self, source: &mut NcPlane) -> NcResult<()> { error![ unsafe { crate::ncplane_mergedown_simple(source, self) }, "NcPlane.mergedown_simple(NcPlane)" diff --git a/src/lib/internal.h b/src/lib/internal.h index 5366d4f6e..bdbcf33b0 100644 --- a/src/lib/internal.h +++ b/src/lib/internal.h @@ -141,11 +141,12 @@ typedef enum { SPRIXCELL_ANNIHILATED, // this cell has been wiped (all trans) } sprixcell_e; -// there is a context-wide set of displayed pixel glyphs ("sprixels"); i.e. -// these are independent of particular piles. there should never be very many +// a sprixel represents a bitmap, using whatever local protocol is available. +// there is a list of sprixels per ncpile. there ought never be very many // associated with a context (a dozen or so at max). with the kitty protocol, // we can register them, and then manipulate them by id. with the sixel -// protocol, we just have to rewrite them. +// protocol, we just have to rewrite them. there's a doubly-linked list of +// sprixels per ncpile, to which the pile keeps a head link. typedef struct sprixel { char* glyph; // glyph; can be quite large int glyphlen; // length of the glyph in bytes @@ -155,6 +156,7 @@ typedef struct sprixel { struct ncplane* n; // associated ncplane sprixel_e invalidated;// sprixel invalidation state struct sprixel* next; + struct sprixel* prev; int y, x; int dimy, dimx; // cell geometry int pixy, pixx; // pixel geometry (might be smaller than cell geo) diff --git a/src/lib/render.c b/src/lib/render.c index 5d8ca4791..733863e8d 100644 --- a/src/lib/render.c +++ b/src/lib/render.c @@ -194,9 +194,16 @@ paint_sprixel(ncplane* p, struct crender* rvec, int starty, int startx, // only those cells where 'p' intersects with the target rendering area are // rendered. // +// the sprixelstack orders sprixels of the plane (so we needn't keep them +// ordered between renders). each time we meet a sprixel, extract it from +// the pile's sprixel list, and update the sprixelstack. +// +// FIXME lift the cell_sprixel_p() variant out and run it its own way +// (unless we want to let sprixels live off-origin in ncplanes), eliminating +// per-cell sprixel_by_id() check static void paint(ncplane* p, struct crender* rvec, int dstleny, int dstlenx, - int dstabsy, int dstabsx){ + int dstabsy, int dstabsx, sprixel** sprixelstack){ int y, x, dimy, dimx, offy, offx; ncplane_dim_yx(p, &dimy, &dimx); offy = p->absy - dstabsy; @@ -220,6 +227,22 @@ paint(ncplane* p, struct crender* rvec, int dstleny, int dstlenx, if(p->sprite){ paint_sprixel(p, rvec, starty, startx, dimy, dimx, offy, offx, dstleny, dstlenx); + // decouple from the pile's sixel list + if(p->sprite->next){ + p->sprite->next->prev = p->sprite->prev; + } + if(p->sprite->prev){ + p->sprite->prev->next = p->sprite->next; + }else{ + ncplane_pile(p)->sprixelcache = p->sprite->next; + } + // stick on the head of the running list: top sprixel is at end + if(*sprixelstack){ + (*sprixelstack)->prev = p->sprite; + } + p->sprite->next = *sprixelstack; + p->sprite->prev = NULL; + *sprixelstack = p->sprite; return; } for(y = starty ; y < dimy ; ++y){ @@ -482,6 +505,10 @@ int ncplane_mergedown(ncplane* restrict src, ncplane* restrict dst, leny, lenx, src->leny, src->lenx); return -1; } + if(src->sprite || dst->sprite){ + logerror(ncplane_notcurses_const(dst), "Can't merge sprixel planes\n"); + return -1; + } const int totalcells = dst->leny * dst->lenx; nccell* rendfb = calloc(sizeof(*rendfb), totalcells); const size_t crenderlen = sizeof(struct crender) * totalcells; @@ -493,8 +520,8 @@ int ncplane_mergedown(ncplane* restrict src, ncplane* restrict dst, return -1; } init_rvec(rvec, totalcells); - paint(src, rvec, dst->leny, dst->lenx, dst->absy, dst->absx); - paint(dst, rvec, dst->leny, dst->lenx, dst->absy, dst->absx); + paint(src, rvec, dst->leny, dst->lenx, dst->absy, dst->absx, NULL); + paint(dst, rvec, dst->leny, dst->lenx, dst->absy, dst->absx, NULL); //fprintf(stderr, "Postpaint start (%dx%d)\n", dst->leny, dst->lenx); postpaint(rendfb, dst->leny, dst->lenx, rvec, &dst->pool); //fprintf(stderr, "Postpaint done (%dx%d)\n", dst->leny, dst->lenx); @@ -883,7 +910,9 @@ clean_sprixels(notcurses* nc, ncpile* p, FILE* out){ if(s->invalidated == SPRIXEL_HIDE){ //fprintf(stderr, "OUGHT HIDE %d [%dx%d @ %d/%d] %p\n", s->id, s->dimy, s->dimx, s->y, s->x, s); if(sprite_destroy(nc, p, out, s) == 0){ - *parent = s->next; + if( (*parent = s->next) ){ + s->next->prev = s->prev; + } sprixel_free(s); }else{ ret = -1; @@ -1242,11 +1271,25 @@ int notcurses_render_to_file(notcurses* nc, FILE* fp){ static void ncpile_render_internal(ncplane* n, struct crender* rvec, int leny, int lenx, int absy, int absx){ - ncplane* p = ncplane_pile(n)->top; + ncpile* np = ncplane_pile(n); + ncplane* p = np->top; + sprixel* sprixel_list = NULL; while(p){ - paint(p, rvec, leny, lenx, absy, absx); + paint(p, rvec, leny, lenx, absy, absx, &sprixel_list); p = p->below; } + if(sprixel_list){ + if(np->sprixelcache){ + sprixel* s = sprixel_list; + while(s->next){ + s = s->next; + } + if( (s->next = np->sprixelcache) ){ + np->sprixelcache->prev = s; + } + } + np->sprixelcache = sprixel_list; + } } int ncpile_rasterize(ncplane* n){ diff --git a/src/lib/sprite.c b/src/lib/sprite.c index 5c4b78927..68e6ce40c 100644 --- a/src/lib/sprite.c +++ b/src/lib/sprite.c @@ -22,6 +22,7 @@ sprixel_debug(FILE* out, const sprixel* s){ } } +// doesn't splice us out of any lists, just frees void sprixel_free(sprixel* s){ if(s){ if(s->n){ @@ -115,14 +116,17 @@ sprixel* sprixel_alloc(ncplane* n, int dimy, int dimx, int placey, int placex){ //fprintf(stderr, "LOOKING AT %p (p->n = %p)\n", ret, ret->n); if(ncplane_pile(ret->n)){ ncpile* np = ncplane_pile(ret->n); - ret->next = np->sprixelcache; + if( (ret->next = np->sprixelcache) ){ + ret->next->prev = ret; + } np->sprixelcache = ret; + ret->prev = NULL; const notcurses* nc = ncplane_notcurses_const(ret->n); ret->cellpxy = nc->tcache.cellpixy; ret->cellpxx = nc->tcache.cellpixx; //fprintf(stderr, "%p %p %p\n", nc->sprixelcache, ret, nc->sprixelcache->next); - }else{ - ret->next = NULL; + }else{ // ncdirect case + ret->next = ret->prev = NULL; ret->cellpxy = ret->cellpxx = -1; } } diff --git a/src/tests/bitmap.cpp b/src/tests/bitmap.cpp index 5dbc678b6..9aadff657 100644 --- a/src/tests/bitmap.cpp +++ b/src/tests/bitmap.cpp @@ -105,6 +105,7 @@ TEST_CASE("Bitmaps") { CHECK(0 == notcurses_render(nc_)); ncvisual_destroy(ncv); CHECK(0 == ncplane_destroy(n)); + CHECK(0 == notcurses_render(nc_)); } // should not be able to emit glyphs to a sprixelated plane @@ -137,6 +138,7 @@ TEST_CASE("Bitmaps") { CHECK(0 == notcurses_render(nc_)); ncvisual_destroy(ncv); CHECK(0 == ncplane_destroy(n)); + CHECK(0 == notcurses_render(nc_)); } SUBCASE("BitmapStack") { @@ -175,9 +177,13 @@ TEST_CASE("Bitmaps") { // should see only the red one now CHECK(0 == notcurses_render(nc_)); CHECK(0 == ncplane_destroy(botn)); - CHECK(0 == ncplane_destroy(topn)); ncvisual_destroy(ncv); + // now we see only yellow + CHECK(0 == notcurses_render(nc_)); + CHECK(0 == ncplane_destroy(topn)); ncvisual_destroy(ncv2); + // and now we see none + CHECK(0 == notcurses_render(nc_)); } #ifdef NOTCURSES_USE_MULTIMEDIA @@ -193,6 +199,7 @@ TEST_CASE("Bitmaps") { CHECK(0 == ncplane_destroy(newn)); CHECK(0 == notcurses_render(nc_)); ncvisual_destroy(ncv); + CHECK(0 == notcurses_render(nc_)); } #endif @@ -262,6 +269,7 @@ TEST_CASE("Bitmaps") { CHECK(0 == ncplane_destroy(infn)); CHECK(0 == ncplane_destroy(n)); ncvisual_destroy(ncv); + CHECK(0 == notcurses_render(nc_)); } SUBCASE("PixelCellWipe") { @@ -426,6 +434,7 @@ TEST_CASE("Bitmaps") { } } CHECK(0 == ncplane_destroy(n)); + CHECK(0 == notcurses_render(nc_)); } #ifdef NOTCURSES_USE_MULTIMEDIA @@ -454,6 +463,7 @@ TEST_CASE("Bitmaps") { CHECK(0 == ncplane_destroy(newn)); CHECK(0 == notcurses_render(nc_)); ncvisual_destroy(ncv); + CHECK(0 == notcurses_render(nc_)); } #endif