[ncplane_put] subtle cursor verification fix

We were checking for an off-plane cursor destination on
the X axis when it was provided explicitly, but not when
-1 was used to indicate the current position (contradicting
the comment immediately above the test). We also want to do
so when we're using the current position, though in this
case scrolling must be taken into account. Also, we were
placing the cursor on such a validation failure, despite not
intending to write anything.

Not testing for a current x (the -1 case) usually worked
because it was thrown out by ncplane_move_cursor_yx() below.
That fails, however, when we're working with an EGC that's
more than one column.

Delicate!
pull/2444/head
nick black 3 years ago committed by nick black
parent b468c3fc83
commit 5eb23cefb0

@ -654,7 +654,7 @@ void ncplane_home(ncplane* n){
int ncplane_cursor_move_yx(ncplane* n, int y, int x){ int ncplane_cursor_move_yx(ncplane* n, int y, int x){
if(x < 0){ if(x < 0){
if(x < -1){ if(x < -1){
logerror("Negative target x %d\n", x); logerror("negative target x %d\n", x);
return -1; return -1;
} }
}else if((unsigned)x >= n->lenx){ }else if((unsigned)x >= n->lenx){
@ -665,7 +665,7 @@ int ncplane_cursor_move_yx(ncplane* n, int y, int x){
} }
if(y < 0){ if(y < 0){
if(y < -1){ if(y < -1){
logerror("Negative target y %d\n", y); logerror("negative target y %d\n", y);
return -1; return -1;
} }
}else if((unsigned)y >= n->leny){ }else if((unsigned)y >= n->leny){
@ -1713,39 +1713,50 @@ static int
ncplane_put(ncplane* n, int y, int x, const char* egc, int cols, ncplane_put(ncplane* n, int y, int x, const char* egc, int cols,
uint16_t stylemask, uint64_t channels, int bytes){ uint16_t stylemask, uint64_t channels, int bytes){
if(n->sprite){ if(n->sprite){
logerror("Can't write [%s] to sprixelated plane\n", egc); logerror("can't write [%s] to sprixelated plane\n", egc);
return -1; return -1;
} }
// reject any control character for output other than newline (and then only // reject any control character for output other than newline (and then only
// on a scrolling plane). // on a scrolling plane).
if(*egc == '\n'){ if(*egc == '\n'){
// if we're not scrolling, autogrow would be to the right (as opposed to
// down), and thus it still wouldn't apply to the case of a newline.
if(!n->scrolling){ if(!n->scrolling){
logerror("Rejecting newline on non-scrolling plane\n"); logerror("rejecting newline on non-scrolling plane\n");
return -1; return -1;
} }
}else if(is_control_egc((const unsigned char*)egc, bytes)){ }else if(is_control_egc((const unsigned char*)egc, bytes)){
logerror("Rejecting %dB control character\n", bytes); logerror("rejecting %dB control character\n", bytes);
return -1; return -1;
} }
// check *before ncplane_cursor_move_yx()* whether we're past the end of the // check *before ncplane_cursor_move_yx()* whether we're past the end of the
// line. if scrolling is enabled, move to the next line if so. if x or y are // line. if scrolling is enabled, move to the next line if so. if x or y are
// specified, we must always try to print at exactly that location. // specified, we must always try to print at exactly that location, and
if(x != -1){ // there's no need to check the present location in that dimension.
if((unsigned)x + cols > n->lenx){ //
logerror("Target x %d + %d cols [%.*s] > length %d\n", x, cols, bytes, egc, n->lenx); // check x for all negatives; only -1 is valid, but our else clause is
ncplane_cursor_move_yx(n, y, x); // update cursor, though // predicated on a non-negative x.
return -1; if(x < 0){
if(n->x + cols - 1 >= n->lenx){
if(!n->scrolling){
logerror("target x %d [%.*s] > length %d\n", n->x, bytes, egc, n->lenx);
return -1;
}
scroll_down(n);
} }
}else if(y == -1 && n->x + cols > n->lenx){ }else{
if(!n->scrolling){ if((unsigned)x + cols - 1 >= n->lenx){
logerror("No room to output [%.*s] %d/%d\n", bytes, egc, n->y, n->x); logerror("no room for %d cols [%.*s] at length %d\n", cols, bytes, egc, n->lenx);
return -1; return -1;
} }
scroll_down(n);
} }
// explicit targets outside the plane will be rejected here.
if(ncplane_cursor_move_yx(n, y, x)){ if(ncplane_cursor_move_yx(n, y, x)){
return -1; return -1;
} }
// FIXME scroll_down() always performs a virtual scroll of the plane, right?
// we don't need that unless we're at the bottom! is this right?!? FIXME
// FIXME note also the scroll_down() above--very fishy! FIXME
if(*egc == '\n'){ if(*egc == '\n'){
scroll_down(n); scroll_down(n);
} }

@ -41,7 +41,6 @@ TEST_CASE("Fade") {
unsigned dimy, dimx; unsigned dimy, dimx;
ncplane_dim_yx(n_, &dimy, &dimx); ncplane_dim_yx(n_, &dimy, &dimx);
nccell c = NCCELL_CHAR_INITIALIZER('*'); nccell c = NCCELL_CHAR_INITIALIZER('*');
nccell_set_fg_rgb8(&c, 0xff, 0xff, 0xff);
unsigned rgb = 0xffffffu; unsigned rgb = 0xffffffu;
CHECK(!ncplane_set_scrolling(n_, true)); CHECK(!ncplane_set_scrolling(n_, true));
for(unsigned y = 0 ; y < dimy ; ++y){ for(unsigned y = 0 ; y < dimy ; ++y){

@ -57,7 +57,7 @@ TEST_CASE("Wide") {
unsigned y, x; unsigned y, x;
ncplane_cursor_yx(n_, &y, &x); ncplane_cursor_yx(n_, &y, &x);
CHECK(0 == y); CHECK(0 == y);
CHECK(dimx - 1 == x); CHECK(0 == x);
CHECK(0 == notcurses_render(nc_)); CHECK(0 == notcurses_render(nc_));
} }

Loading…
Cancel
Save