Skip to content

Commit

Permalink
notcurses_at_yx(): value-result u32+u64, not cell
Browse files Browse the repository at this point in the history
Resolves #410. notcurses_at_yx() accepted a cell*, but the
gcluster of this cell was always set to 0. The EGC is instead
a heap-allocated copy, returned as the primary return value.
This is due to the absence of an egcpool to bind against.
Existing callers can be converted thus:

* instead of passing cell 'c', pass &(c)->attrword, &(c)->channels
* either initialize 'c' with CELL_TRIVIAL_INITIALIZER, or set its
   gcluster field to 0 following the call

I've updated all calls from tests/demos, updated the docs, and
updated the C++ and Python wrappers.
  • Loading branch information
dankamongmen committed Mar 27, 2020
1 parent 73b61f6 commit a77774f
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 40 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ rearrangements of Notcurse.
** `ncvisual_render()` now interprets length parameters of -1 to mean "to the
end along this axis", and no longer interprets 0 to mean this. 0 now means
"a length of 0", resulting in a zero-area rendering.
** `notcurses_at_yx()` no longer accepts a `cell*` as its last parameter.
Instead, it accepts a `uint32_t*` and a `uint64_t*`, and writes the
attribute and channels to these parameters. This was done because the
`gcluster` field of the `cell*` was always set to 0, which was surprising
and a source of blunders. The EGC is returned via the `char*` return
value. https://github.com/dankamongmen/notcurses/issues/410

* 1.2.4 2020-03-24
** Add ncmultiselector
Expand Down
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,10 @@ updated to reflect the changes:
int notcurses_render(struct notcurses* nc);
// Retrieve the contents of the specified cell as last rendered. The EGC is
// returned, or NULL on error. This EGC must be free()d by the caller. The cell
// 'c' is not bound to a plane, and thus its gcluster value must not be used--
// use the return value only.
char* notcurses_at_yx(struct notcurses* nc, int yoff, int xoff, cell* c);
// returned, or NULL on error. This EGC must be free()d by the caller. The
// attrword and channels are written to 'attrword' and 'channels', respectively.
char* notcurses_at_yx(struct notcurses* nc, int yoff, int xoff,
uint32_t* attrword, uint64_t* channels);
```

One `ncplane` is guaranteed to exist: the "standard plane". The user cannot
Expand Down
8 changes: 7 additions & 1 deletion doc/man/man3/notcurses_render.3.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ notcurses_render - sync the physical display to the virtual ncplanes

**int notcurses_render(struct notcurses* nc);**

**char* notcurses_at_yx(struct notcurses* nc, int yoff, int xoff, cell* c);**
**char* notcurses_at_yx(struct notcurses* nc, int yoff, int xoff, uint32_t* attrword, uint64_t* channels);**

# DESCRIPTION

Expand Down Expand Up @@ -63,12 +63,18 @@ If the algorithm concludes without an EGC, the cell is rendered with no glyph
and a default background. If the algorithm concludes without a color locked in,
the color as computed thus far is used.

**notcurses_at_yx** retrieves a call *as rendered*. The EGC in that cell is
copied and returned; it must be **free(3)**d by the caller.

# RETURN VALUES

On success, 0 is returned. On failure, a non-zero value is returned. A success
will result in the **renders** stat being increased by 1. A failure will result
in the **failed_renders** stat being increased by 1.

**notcurses_at_yx** returns a heap-allocated copy of the cell's EGC on success,
and **NULL** on failure.

# BUGS

In addition to the RGB colors, it is possible to use the "default foreground color"
Expand Down
4 changes: 2 additions & 2 deletions include/ncpp/NotCurses.hh
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,9 @@ namespace ncpp
return notcurses_getc_nblock (nc, ni);
}

char* get_at (int yoff, int xoff, Cell &c) const noexcept
char* get_at (int yoff, int xoff, uint32_t* attr, uint64_t* channels) const noexcept
{
return notcurses_at_yx (nc, yoff, xoff, c);
return notcurses_at_yx (nc, yoff, xoff, attr, channels);
}

void drop_planes () const noexcept
Expand Down
8 changes: 4 additions & 4 deletions include/notcurses/notcurses.h
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,10 @@ notcurses_term_dim_yx(struct notcurses* n, int* RESTRICT rows, int* RESTRICT col
}

// Retrieve the contents of the specified cell as last rendered. The EGC is
// returned, or NULL on error. This EGC must be free()d by the caller. The cell
// 'c' is not bound to a plane, and thus its gcluster value must not be used--
// use the return value only.
API char* notcurses_at_yx(struct notcurses* nc, int yoff, int xoff, cell* c);
// returned, or NULL on error. This EGC must be free()d by the caller. The
// attrword and channels are written to 'attrword' and 'channels', respectively.
API char* notcurses_at_yx(struct notcurses* nc, int yoff, int xoff,
uint32_t* attrword, uint64_t* channels);

// Alignment within the ncplane. Left/right-justified, or centered.
typedef enum {
Expand Down
2 changes: 1 addition & 1 deletion python/src/notcurses/build_notcurses.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
int ncplane_move_below(struct ncplane* n, struct ncplane* below);
int ncplane_move_above(struct ncplane* n, struct ncplane* above);
struct ncplane* ncplane_below(struct ncplane* n);
char* notcurses_at_yx(struct notcurses* nc, int yoff, int xoff, cell* c);
char* notcurses_at_yx(struct notcurses* nc, int yoff, int xoff, uint32_t* attrword, uint64_t* channels);
int ncplane_at_cursor(struct ncplane* n, cell* c);
int ncplane_at_yx(struct ncplane* n, int y, int x, cell* c);
void* ncplane_set_userptr(struct ncplane* n, void* opaque);
Expand Down
14 changes: 7 additions & 7 deletions src/lib/render.c
Original file line number Diff line number Diff line change
Expand Up @@ -1040,16 +1040,16 @@ int notcurses_render(notcurses* nc){
return ret;
}

char* notcurses_at_yx(notcurses* nc, int y, int x, cell* c){
char* notcurses_at_yx(notcurses* nc, int yoff, int xoff, uint32_t* attrword, uint64_t* channels){
char* egc = NULL;
if(nc->lastframe){
if(y >= 0 && y < nc->lfdimy){
if(x >= 0 || x < nc->lfdimx){
const cell* srccell = &nc->lastframe[y * nc->lfdimx + x];
memcpy(c, srccell, sizeof(*c)); // unsafe copy of gcluster
//fprintf(stderr, "COPYING: %d from %p\n", c->gcluster, &nc->pool);
if(yoff >= 0 && yoff < nc->lfdimy){
if(xoff >= 0 || xoff < nc->lfdimx){
const cell* srccell = &nc->lastframe[yoff * nc->lfdimx + xoff];
*attrword = srccell->attrword;
*channels = srccell->channels;
//fprintf(stderr, "COPYING: %d from %p\n", srccell->gcluster, &nc->pool);
egc = pool_egc_copy(&nc->pool, srccell);
c->gcluster = 0; // otherwise cell_release() will blow up
}
}
}
Expand Down
24 changes: 12 additions & 12 deletions tests/ncplane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -922,14 +922,14 @@ TEST_CASE("NCPlane") {
REQUIRE(0 < ncplane_at_yx(ncp, 1, 0, &c));
CHECK(!strcmp(cell_extended_gcluster(ncp, &c), ""));
cell_release(ncp, &c);
char* egc = notcurses_at_yx(nc_, 1, 0, &c);
char* egc = notcurses_at_yx(nc_, 1, 0, &c.attrword, &c.channels);
REQUIRE(egc);
CHECK(!strcmp(egc, ""));
free(egc);
REQUIRE(0 < ncplane_at_yx(ncp, 1, 3, &c));
CHECK(!strcmp(cell_extended_gcluster(ncp, &c), ""));
cell_release(ncp, &c);
egc = notcurses_at_yx(nc_, 1, 3, &c);
egc = notcurses_at_yx(nc_, 1, 3, &c.attrword, &c.channels);
REQUIRE(egc);
CHECK(!strcmp(egc, ""));
free(egc);
Expand All @@ -950,16 +950,16 @@ TEST_CASE("NCPlane") {
ncplane_at_yx(n_, 0, 4, &c);
CHECK(!cell_double_wide_p(&c));
REQUIRE(0 == notcurses_render(nc_));
notcurses_at_yx(nc_, 0, 0, &c);
CHECK(cell_double_wide_p(&c));
notcurses_at_yx(nc_, 0, 1, &c);
CHECK(cell_double_wide_p(&c));
notcurses_at_yx(nc_, 0, 2, &c);
CHECK(cell_double_wide_p(&c));
notcurses_at_yx(nc_, 0, 3, &c);
CHECK(cell_double_wide_p(&c));
notcurses_at_yx(nc_, 0, 4, &c);
CHECK(!cell_double_wide_p(&c));
notcurses_at_yx(nc_, 0, 0, &c.attrword, &c.channels);
CHECK(0 != (c.channels & 0x8000000080000000ull));
notcurses_at_yx(nc_, 0, 1, &c.attrword, &c.channels);
CHECK(0 != (c.channels & 0x8000000080000000ull));
notcurses_at_yx(nc_, 0, 2, &c.attrword, &c.channels);
CHECK(0 != (c.channels & 0x8000000080000000ull));
notcurses_at_yx(nc_, 0, 3, &c.attrword, &c.channels);
CHECK(0 != (c.channels & 0x8000000080000000ull));
notcurses_at_yx(nc_, 0, 4, &c.attrword, &c.channels);
CHECK(!(c.channels & 0x8000000080000000ull));
}

SUBCASE("Perimeter") {
Expand Down
2 changes: 1 addition & 1 deletion tests/palette.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ TEST_CASE("Palette256") {
cell_release(n_, &c);
CHECK(0 == notcurses_render(nc_));
cell r = CELL_TRIVIAL_INITIALIZER;
CHECK(nullptr != notcurses_at_yx(nc_, 0, 0, &r));
CHECK(nullptr != notcurses_at_yx(nc_, 0, 0, &r.attrword, &r.channels));
CHECK(cell_fg_palindex_p(&r));
CHECK(cell_bg_palindex_p(&r));
CHECK(CELL_ALPHA_OPAQUE == cell_fg_alpha(&r));
Expand Down
16 changes: 8 additions & 8 deletions tests/render.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ TEST_CASE("RenderTest") {
CHECK(!strcmp("\xe5\x85\xa8", egc));
CHECK(cell_double_wide_p(&c));
free(egc);
egc = notcurses_at_yx(nc_, 0, 0, &c);
egc = notcurses_at_yx(nc_, 0, 0, &c.attrword, &c.channels);
REQUIRE(egc);
CHECK(!strcmp("\xe5\x85\xa8", egc));
CHECK(cell_double_wide_p(&c));
Expand All @@ -61,7 +61,7 @@ TEST_CASE("RenderTest") {
CHECK(!strcmp("", egc));
CHECK(cell_double_wide_p(&c));
free(egc);
egc = notcurses_at_yx(nc_, 0, 1, &c);
egc = notcurses_at_yx(nc_, 0, 1, &c.attrword, &c.channels);
REQUIRE(egc);
CHECK(!strcmp("", egc));
CHECK(cell_double_wide_p(&c));
Expand All @@ -75,7 +75,7 @@ TEST_CASE("RenderTest") {
CHECK(!strcmp("\xe5\xbd\xa2", egc));
CHECK(cell_double_wide_p(&c));
free(egc);
egc = notcurses_at_yx(nc_, 0, 2, &c);
egc = notcurses_at_yx(nc_, 0, 2, &c.attrword, &c.channels);
REQUIRE(egc);
CHECK(!strcmp("\xe5\xbd\xa2", egc));
CHECK(cell_double_wide_p(&c));
Expand All @@ -88,7 +88,7 @@ TEST_CASE("RenderTest") {
CHECK(!strcmp("", egc));
CHECK(cell_double_wide_p(&c));
free(egc);
egc = notcurses_at_yx(nc_, 0, 3, &c);
egc = notcurses_at_yx(nc_, 0, 3, &c.attrword, &c.channels);
REQUIRE(egc);
CHECK(!strcmp("", egc));
CHECK(cell_double_wide_p(&c));
Expand All @@ -101,26 +101,26 @@ TEST_CASE("RenderTest") {
CHECK(!notcurses_render(nc_));

// should be nothing, having been stomped
egc = notcurses_at_yx(nc_, 0, 0, &c);
egc = notcurses_at_yx(nc_, 0, 0, &c.attrword, &c.channels);
REQUIRE(egc);
CHECK(!strcmp(" ", egc));
free(egc);
cell_init(&c);
// should be character from higher plane
egc = notcurses_at_yx(nc_, 0, 1, &c);
egc = notcurses_at_yx(nc_, 0, 1, &c.attrword, &c.channels);
REQUIRE(egc);
CHECK(!strcmp("A", egc));
free(egc);
cell_init(&c);

egc = notcurses_at_yx(nc_, 0, 2, &c);
egc = notcurses_at_yx(nc_, 0, 2, &c.attrword, &c.channels);
REQUIRE(egc);
CHECK(!strcmp("B", egc));
free(egc);
cell_init(&c);

// should be nothing, having been stomped
egc = notcurses_at_yx(nc_, 0, 3, &c);
egc = notcurses_at_yx(nc_, 0, 3, &c.attrword, &c.channels);
REQUIRE(egc);
CHECK(!strcmp(" ", egc));
free(egc);
Expand Down

0 comments on commit a77774f

Please sign in to comment.