From 5a72383cb0386e276d7f55ae5ef8a3276e562c25 Mon Sep 17 00:00:00 2001 From: nick black Date: Sat, 24 Apr 2021 04:41:23 -0400 Subject: [PATCH] reject sprixels larger than plane, add unit test #1572 --- doc/man/man3/notcurses_plane.3.md | 8 ++++--- src/lib/notcurses.c | 7 +++--- src/lib/visual.c | 10 ++++++++ src/tests/bitmap.cpp | 40 +++++++++++++++++++++++++++---- 4 files changed, 53 insertions(+), 12 deletions(-) diff --git a/doc/man/man3/notcurses_plane.3.md b/doc/man/man3/notcurses_plane.3.md index 8512a80cc..fcb069ad2 100644 --- a/doc/man/man3/notcurses_plane.3.md +++ b/doc/man/man3/notcurses_plane.3.md @@ -352,6 +352,8 @@ last row is cleared, and output begins at the beginning of the last row. This does not take place until output is generated (i.e. it is possible to fill a plane when scrolling is enabled). +## Bitmaps + **ncplane_pixelgeom** retrieves pixel geometry details. **pxy** and **pxx** return the size of the plane in pixels. **celldimy** and **celldimx** return the size of a cell in pixels (these ought be the same across planes). @@ -364,9 +366,9 @@ parameter (save **n**) may be **NULL**. When a plane is blitted to using **ncvisual_render** and **NCBLIT_PIXEL** (see **notcurses_visual(3)**), it ceases to accept cell-based output. The sprixel will remain associated until a new sprixel is blitted to the plane, the plane -is resized, or the plane is destroyed. The base cell of a sprixelated plane -has no effect; if the sprixel is not even multiples of the cell geometry, the -"excess plane" is ignored during rendering. +is resized, the plane is erased, or the plane is destroyed. The base cell of a +sprixelated plane has no effect; if the sprixel is not even multiples of the +cell geometry, the "excess plane" is ignored during rendering. # RETURN VALUES diff --git a/src/lib/notcurses.c b/src/lib/notcurses.c index aabf9440a..3bddf1f0a 100644 --- a/src/lib/notcurses.c +++ b/src/lib/notcurses.c @@ -583,10 +583,6 @@ int resize_callbacks_children(ncplane* n){ // can be used on stdplane, unlike ncplane_resize() which prohibits it. int ncplane_resize_internal(ncplane* n, int keepy, int keepx, int keepleny, int keeplenx, int yoff, int xoff, int ylen, int xlen){ - if(n->sprite){ - logerror(ncplane_notcurses_const(n), "Can't resize sprixelated (id %d) plane\n", n->sprite->id); - return -1; - } if(keepleny < 0 || keeplenx < 0){ // can't retain negative size logerror(ncplane_notcurses_const(n), "Can't retain negative size %dx%d\n", keepleny, keeplenx); return -1; @@ -626,6 +622,9 @@ int ncplane_resize_internal(ncplane* n, int keepy, int keepx, int keepleny, } loginfo(ncplane_notcurses_const(n), "%dx%d @ %d/%d → %d/%d @ %d/%d (keeping %dx%d from %d/%d)\n", rows, cols, n->absy, n->absx, ylen, xlen, n->absy + keepy + yoff, n->absx + keepx + xoff, keepleny, keeplenx, keepy, keepx); notcurses* nc = ncplane_notcurses(n); + if(n->sprite){ + sprixel_hide(n->sprite); + } // we're good to resize. we'll need alloc up a new framebuffer, and copy in // those elements we're retaining, zeroing out the rest. alternatively, if // we've shrunk, we will be filling the new structure. diff --git a/src/lib/visual.c b/src/lib/visual.c index 8020b7e40..4dec619c3 100644 --- a/src/lib/visual.c +++ b/src/lib/visual.c @@ -158,6 +158,16 @@ ncvisual_blitset_geom(const notcurses* nc, const ncvisual* n, logerror(nc, "Non-origin x placement %d for sprixel\n", vopts->x); return -1; } + int rows = (*leny + nc->tcache.cellpixy - 1) / nc->tcache.cellpixy; + if(rows > ncplane_dim_y(vopts->n)){ + logerror(nc, "Sprixel too tall %d for plane %d\n", *leny, ncplane_dim_y(vopts->n) * nc->tcache.cellpixy); + return -1; + } + int cols = (*lenx + nc->tcache.cellpixx - 1) / nc->tcache.cellpixx; + if(cols > ncplane_dim_x(vopts->n)){ + logerror(nc, "Sprixel too wide %d for plane %d\n", *lenx, ncplane_dim_x(vopts->n) * nc->tcache.cellpixx); + return -1; + } } } if(n){ diff --git a/src/tests/bitmap.cpp b/src/tests/bitmap.cpp index 48aae0d0d..c4582f833 100644 --- a/src/tests/bitmap.cpp +++ b/src/tests/bitmap.cpp @@ -38,7 +38,7 @@ TEST_CASE("Bitmaps") { .flags = NCVISUAL_OPTION_NODEGRADE, .transcolor = 0, }; - CHECK(0 == ncvisual_resize(ncv, 6, 1)); // FIXME get down to 1, 1 + CHECK(0 == ncvisual_resize(ncv, 6, 1)); // FIXME get down to 1, 1 (sixel needs handle) auto n = ncvisual_render(nc_, ncv, &vopts); REQUIRE(nullptr != n); auto s = n->sprite; @@ -46,6 +46,38 @@ TEST_CASE("Bitmaps") { ncvisual_destroy(ncv); } + // a sprixel requires a plane large enough to hold it + SUBCASE("SprixelTooBig") { + auto y = nc_->tcache.cellpixy + 1; + auto x = nc_->tcache.cellpixx + 1; + std::vector v(x * y, htole(0xe61c28ff)); + auto ncv = ncvisual_from_rgba(v.data(), y, sizeof(decltype(v)::value_type) * x, x); + REQUIRE(nullptr != ncv); + struct ncplane_options nopts = { + .y = 0, .x = 0, + .rows = 1, .cols = 1, + .userptr = nullptr, .name = "small", .resizecb = nullptr, + .flags = 0, .margin_b = 0, .margin_r = 0, + }; + auto n = ncplane_create(n_, &nopts); + struct ncvisual_options vopts = { + .n = n, + .scaling = NCSCALE_NONE, + .y = 0, + .x = 0, + .begy = 0, .begx = 0, + .leny = 0, .lenx = 0, + .blitter = NCBLIT_PIXEL, + .flags = NCVISUAL_OPTION_NODEGRADE, + .transcolor = 0, + }; + CHECK(nullptr == ncvisual_render(nc_, ncv, &vopts)); + CHECK(0 == notcurses_render(nc_)); +sleep(2); + ncvisual_destroy(ncv); + CHECK(0 == ncplane_destroy(n)); + } + #ifdef NOTCURSES_USE_MULTIMEDIA SUBCASE("PixelRender") { auto ncv = ncvisual_from_file(find_data("worldmap.png")); @@ -294,8 +326,6 @@ TEST_CASE("Bitmaps") { CHECK(0 == ncplane_destroy(n)); } - // too much output -- OOMs ctest FIXME - /* #ifdef NOTCURSES_USE_MULTIMEDIA SUBCASE("PixelWipeImage") { uint64_t channels = 0; @@ -315,7 +345,8 @@ TEST_CASE("Bitmaps") { for(int y = 0 ; y < s->dimy ; ++y){ for(int x = 0 ; x < s->dimx ; ++x){ CHECK(1 == ncplane_putchar_yx(n_, y, x, 'x')); - CHECK(0 == notcurses_render(nc_)); + // FIXME generates too much output, OOMing ctest + // CHECK(0 == notcurses_render(nc_)); } } CHECK(0 == ncplane_destroy(newn)); @@ -323,7 +354,6 @@ TEST_CASE("Bitmaps") { ncvisual_destroy(ncv); } #endif -*/ CHECK(!notcurses_stop(nc_)); }