Fix up some subtle pile issues

ncplane_destroy() needs to call ncplane_reparent_family(), not
ncplane_reparent() as it was doing (closes #1291). ->absy and
->absx actually are absolute; return them directly for an O(1)
ncplane_abs_yx() (down from O(N), huzzah). Add some unit tests
related to destroying and reparenting (#1286). Add ncplane_abs_y()
and ncplane_abs_x(), document them, etc.

NEWS.md

@@ -9,8 +9,7 @@ rearrangements of Notcurses.
     `ncplane_putnstr_yx()` now return the number of columns output, as
     long documented (they were mistakenly returning the number of bytes).
   * `ncplane_abs_yx()` has been added, returning the absolute coordinates of
-    the plane's origin (i.e. coordinates relative to its pile). This call is
-    O(N) on the binding depth of the plane in question.
+    the plane's origin (i.e. coordinates relative to its pile).
 
 * 2.1.4 (2021-01-03):
   * Direct mode now supports `NCDIRECT_OPTION_NO_QUIT_SIGHANDLERS`, and by

USAGE.md

@@ -808,9 +808,11 @@ void ncplane_yx(const struct ncplane* n, int* restrict y, int* restrict x);
 int ncplane_y(const struct ncplane* n);
 int ncplane_x(const struct ncplane* n);
 
-// Get the origin of plane 'n' relative to its pile. This is O(N) on the
-// binding depth of 'n'. Either or both of 'x' and y' may be NULL.
+// Get the origin of plane 'n' relative to its pile. Either or both of 'x' and
+// 'y' may be NULL.
 void ncplane_abs_yx(const struct ncplane* n, int* y, int* x);
+int ncplane_abs_y(const struct ncplane* n);
+int ncplane_abs_x(const struct ncplane* n);
 
 // Return the dimensions of this ncplane.
 void ncplane_dim_yx(struct ncplane* n, int* restrict rows, int* restrict cols);

doc/man/man3/notcurses_plane.3.md

@@ -63,6 +63,10 @@ typedef struct ncplane_options {
 
 **void ncplane_abs_yx(const struct ncplane* ***n***, int* ***y***, int* ***x***);**
 
+**int ncplane_abs_y(const struct ncplane* ***n***);**
+
+**int ncplane_abs_x(const struct ncplane* ***n***);**
+
 **struct ncplane* ncplane_parent(struct ncplane* ***n***);**
 
 **const struct ncplane* ncplane_parent_const(const struct ncplane* ***n***);**

include/notcurses/notcurses.h

@@ -1317,9 +1317,11 @@ API void ncplane_yx(const struct ncplane* n, int* RESTRICT y, int* RESTRICT x);
 API int ncplane_y(const struct ncplane* n);
 API int ncplane_x(const struct ncplane* n);
 
-// Get the origin of plane 'n' relative to its pile. This is O(N) on the
-// binding depth of 'n'. Either or both of 'x' and y' may be NULL.
+// Get the origin of plane 'n' relative to its pile. Either or both of 'x' and
+// 'y' may be NULL.
 API void ncplane_abs_yx(const struct ncplane* n, int* RESTRICT y, int* RESTRICT x);
+API int ncplane_abs_y(const struct ncplane* n);
+API int ncplane_abs_x(const struct ncplane* n);
 
 // Get the plane to which the plane 'n' is bound, if any.
 API struct ncplane* ncplane_parent(struct ncplane* n);

src/lib/internal.h

@@ -67,7 +67,7 @@ typedef struct ncplane {
   // ncplane_yx() etc. use coordinates relative to the plane to which this
   // plane is bound, but absx/absy are always relative to the terminal origin.
   // they must thus be translated by any function which moves a parent plane.
-  int absx, absy;        // origin of the plane relative to the screen
+  int absx, absy;        // origin of the plane relative to the pile's origin
   int lenx, leny;        // size of the plane, [0..len{x,y}) is addressable
   egcpool pool;          // attached storage pool for UTF-8 EGCs
   uint64_t channels;     // works the same way as cells

src/lib/notcurses.c

@@ -630,6 +630,7 @@ int ncplane_destroy(ncplane* ncp){
     logerror(ncplane_notcurses(ncp), "Won't destroy standard plane\n");
     return -1;
   }
+//notcurses_debug(ncplane_notcurses(ncp), stderr);
   if(ncp->above){
     ncp->above->below = ncp->below;
   }else{
@@ -649,7 +650,7 @@ int ncplane_destroy(ncplane* ncp){
   struct ncplane* bound = ncp->blist;
   while(bound){
     struct ncplane* tmp = bound->bnext;
-    if(ncplane_reparent(bound, ncp->boundto) == NULL){
+    if(ncplane_reparent_family(bound, ncp->boundto) == NULL){
       ret = -1;
     }
     bound = tmp;
@@ -2147,17 +2148,21 @@ const notcurses* ncplane_notcurses_const(const ncplane* n){
   return ncplane_pile_const(n)->nc;
 }
 
-static void
-ncplane_abs_yx_recurse(const ncplane* n, int* RESTRICT y, int* RESTRICT x){
-  if(n->boundto != n){
-    ncplane_translate(n, n->boundto, y, x);
-    ncplane_abs_yx_recurse(n->boundto, y, x);
-  }
+int ncplane_abs_y(const ncplane* n){
+  return n->absy; // FIXME adjust for margins?
+}
+
+int ncplane_abs_x(const ncplane* n){
+  return n->absx; // FIXME adjust for margins?
 }
 
 void ncplane_abs_yx(const ncplane* n, int* RESTRICT y, int* RESTRICT x){
-  ncplane_yx(n, y, x);
-  ncplane_abs_yx_recurse(n->boundto, y, x);
+  if(y){
+    *y = ncplane_abs_y(n);
+  }
+  if(x){
+    *x = ncplane_abs_x(n);
+  }
 }
 
 ncplane* ncplane_parent(ncplane* n){
@@ -2218,6 +2223,7 @@ ncplane* ncplane_reparent(ncplane* n, ncplane* newparent){
   if(n->boundto == newparent){
     return n;
   }
+//notcurses_debug(ncplane_notcurses(n), stderr);
   if(n->blist){
     if(n->boundto == n){ // children become new root planes
       ncplane* lastlink;
@@ -2244,7 +2250,7 @@ ncplane* ncplane_reparent(ncplane* n, ncplane* newparent){
     }
     n->blist = NULL;
   }
-//notcurses_debug(ncplane_notcurses(n), stderr);
+  // FIXME would be nice to skip ncplane_descendant_p() on this call...:/
   return ncplane_reparent_family(n, newparent);
 }
 
@@ -2292,14 +2298,10 @@ ncplane* ncplane_reparent_family(ncplane* n, ncplane* newparent){
   if(ncplane_descendant_p(newparent, n)){
     return NULL;
   }
+//notcurses_debug(ncplane_notcurses(n), stderr);
   if(n->boundto == newparent){ // no-op
     return n;
   }
-  // if we are not a root plane, adjust our origin
-  if(n->boundto != n){
-    n->absx -= n->boundto->absx;
-    n->absy -= n->boundto->absy;
-  }
   if(n->bprev){ // extract from sibling list
     if( (*n->bprev = n->bnext) ){
       n->bnext->bprev = n->bprev;
@@ -2321,8 +2323,6 @@ ncplane* ncplane_reparent_family(ncplane* n, ncplane* newparent){
     pthread_mutex_unlock(&ncplane_notcurses(n)->pilelock);
     splice_zaxis_recursive(n);
   }else{ // establish ourselves as a sibling of new parent's children
-    n->absx += n->boundto->absx;
-    n->absy += n->boundto->absy;
     if( (n->bnext = newparent->blist) ){
       n->bnext->bprev = &n->bnext;
     }

tests/piles.cpp

@@ -246,12 +246,14 @@ TEST_CASE("Piles") {
     CHECK(30 == x);
     CHECK(10 == ncplane_y(gen3));
     CHECK(10 == ncplane_x(gen3));
+notcurses_debug(nc_, stderr);
     CHECK(nullptr != ncplane_reparent(gen1, gen2));
+notcurses_debug(nc_, stderr);
     ncplane_abs_yx(gen2, &y, &x); // gen2 is now the parent
     CHECK(20 == y);
     CHECK(20 == x);
-    CHECK(20 == ncplane_y(gen1));
-    CHECK(20 == ncplane_x(gen1));
+    CHECK(20 == ncplane_y(gen2));
+    CHECK(20 == ncplane_x(gen2));
     ncplane_abs_yx(gen1, &y, &x); // gen1 is below and to the left of gen2
     CHECK(10 == y);
     CHECK(10 == x);
@@ -260,8 +262,8 @@ TEST_CASE("Piles") {
     ncplane_abs_yx(gen3, &y, &x);
     CHECK(30 == y); // should stay the same
     CHECK(30 == x);
-    CHECK(20 == ncplane_y(gen3)); // should reflect new parentage
-    CHECK(20 == ncplane_x(gen3));
+    CHECK(10 == ncplane_y(gen3)); // remain the same; still a child of gen2
+    CHECK(10 == ncplane_x(gen3));
     ncplane_destroy(gen1);
     ncplane_destroy(gen2);
     ncplane_destroy(gen3);

tests/plane.cpp

@@ -913,17 +913,26 @@ TEST_CASE("Plane") {
     int absy, absx;
     CHECK(0 == notcurses_render(nc_));
     ncplane_yx(nsub, &absy, &absx);
-    CHECK(1 == absy); // actually at 2, 2
+    CHECK(1 == absy); // relatively at 1, 1
     CHECK(1 == absx);
+    ncplane_abs_yx(nsub, &absy, &absx);
+    CHECK(2 == absy); // actually at 2, 2
+    CHECK(2 == absx);
     ncplane_reparent(nsub, nsub);
     CHECK(ncplane_pile(nsub) != ncplane_pile(ndom));
+    ncplane_abs_yx(nsub, &absy, &absx);
+    CHECK(2 == absy); // now relatively at 1, 1
+    CHECK(2 == absx);
     ncplane_yx(nsub, &absy, &absx);
-    CHECK(1 == absy); // now truly at 1, 1
-    CHECK(1 == absx);
+    CHECK(2 == absy);
+    CHECK(2 == absx);
     CHECK(0 == ncplane_move_yx(ndom, 0, 0));
+    ncplane_abs_yx(nsub, &absy, &absx);
+    CHECK(2 == absy); // still at 1, 1
+    CHECK(2 == absx);
     ncplane_yx(nsub, &absy, &absx);
-    CHECK(1 == absy); // still at 1, 1
-    CHECK(1 == absx);
+    CHECK(2 == absy);
+    CHECK(2 == absx);
   }
 
   SUBCASE("NoReparentStdPlane") {