Planepiles, part 2 of 3

src/lib/debug.c

@@ -2,22 +2,26 @@
 
 static void
 ncpile_debug(const ncpile* p, FILE* debugfp){
+  fprintf(debugfp, "  -=+********************** %16p pile *************************+=-\n", p);
   const ncplane* n = p->top;
   const ncplane* prev = NULL;
   int planeidx = 0;
   while(n){
     fprintf(debugfp, "%04d off y: %3d x: %3d geom y: %3d x: %3d curs y: %3d x: %3d %p %.8s\n",
             planeidx, n->absy, n->absx, n->leny, n->lenx, n->y, n->x, n, n->name);
     if(n->boundto || n->bnext || n->bprev || n->blist){
-      fprintf(debugfp, " bound %p → %p ← %p binds %p\n",
-              n->boundto, n->bnext, n->bprev, n->blist);
+      fprintf(debugfp, " bound %p ← %p → %p binds %p\n",
+              n->boundto, n->bprev, n->bnext, n->blist);
     }
     if(n->bprev && (*n->bprev != n)){
       fprintf(stderr, " WARNING: expected *->bprev %p, got %p\n", n, *n->bprev);
     }
     if(n->above != prev){
       fprintf(stderr, " WARNING: expected ->above %p, got %p\n", prev, n->above);
     }
+    if(ncplane_pile_const(n) != p){
+      fprintf(stderr, " WARNING: expected pile %p, got %p\n", p, ncplane_pile_const(n));
+    }
     prev = n;
     n = n->below;
     ++planeidx;
@@ -33,7 +37,11 @@ void notcurses_debug(notcurses* nc, FILE* debugfp){
   const ncpile* p0 = p;
   do{
     ncpile_debug(p0, debugfp);
+    const ncpile* prev = p0;
     p0 = p0->next;
+    if(p0->prev != prev){
+      fprintf(stderr, "WARNING: expected ->prev %p, got %p\n", prev, p0->prev);
+    }
   }while(p != p0);
   fprintf(debugfp, " ******************************************************************************\n");
 }

src/lib/internal.h

@@ -73,8 +73,8 @@ typedef struct ncplane {
   // and is the only stack which is rendered. each stack has its own z-axis.
   struct ncplane* above; // plane above us, NULL if we're on top
   struct ncplane* below; // plane below us, NULL if we're on bottom
-  struct ncplane* bnext; // next in the bound list of plane to which we are bound
-  struct ncplane** bprev;// link to us iff we're bound, NULL otherwise
+  struct ncplane* bnext; // next in the blist iff we're bound, NULL otherwise
+  struct ncplane** bprev;// blist link to us iff we're bound, NULL otherwise
   struct ncplane* blist; // head of list of bound planes
   // a root plane is bound to itself. every other plane has a path to its
   // stack's root via boundto. the standard plane is always bound to itself.
@@ -301,7 +301,6 @@ typedef struct ncdirect {
 typedef struct ncpile {
   ncplane* top;     // topmost plane, never NULL
   ncplane* bottom;  // bottommost plane, never NULL 
-  ncplane* root;    // first plane of the root set
   struct notcurses* nc; // notcurses context
   struct ncpile *prev, *next; // circular list
 } ncpile;

src/lib/notcurses.c

@@ -297,13 +297,12 @@ void free_plane(ncplane* p){
 
 // create a new ncpile. only call with pilelock held.
 static ncpile*
-make_ncpile(notcurses* nc){
+make_ncpile(notcurses* nc, ncplane* n){
   ncpile* ret = malloc(sizeof(*ret));
   if(ret){
     ret->nc = nc;
-    ret->top = NULL;
-    ret->bottom = NULL;
-    ret->root = NULL;
+    ret->top = n;
+    ret->bottom = n;
     if(nc->stdplane){
       ret->prev = ncplane_pile(nc->stdplane)->prev;
       ncplane_pile(nc->stdplane)->prev->next = ret;
@@ -313,6 +312,9 @@ make_ncpile(notcurses* nc){
       ret->prev = ret;
       ret->next = ret;
     }
+    n->pile = ret;
+    n->above = NULL;
+    n->below = NULL;
   }
   return ret;
 }
@@ -386,27 +388,25 @@ ncplane* ncplane_new_internal(notcurses* nc, ncplane* n,
   egcpool_init(&p->pool);
   cell_init(&p->basecell);
   p->userptr = nopts->userptr;
-  p->above = NULL;
   if(nc == NULL){ // fake ncplane backing ncdirect object
+    p->above = NULL;
     p->below = NULL;
   }else{
     pthread_mutex_lock(&nc->pilelock);
     ncpile* pile = n ? ncplane_pile(n) : NULL;
     if( (p->pile = pile) ){ // existing pile
+      p->above = NULL;
       if( (p->below = pile->top) ){ // always happens save initial plane
         pile->top->above = p;
       }else{
         pile->bottom = p;
       }
       pile->top = p;
-      nc->stats.fbbytes += fbsize;
-      ++nc->stats.planes;
     }else{ // new pile
-      p->pile = make_ncpile(nc);
-      p->pile->top = p;
-      p->pile->bottom = p;
-      p->below = NULL;
+      make_ncpile(nc, p);
     }
+    nc->stats.fbbytes += fbsize;
+    ++nc->stats.planes;
     pthread_mutex_unlock(&nc->pilelock);
   }
   loginfo(nc, "Created new %dx%d plane \"%s\" @ %dx%d\n",
@@ -946,6 +946,30 @@ int ncinputlayer_init(ncinputlayer* nilayer, FILE* infp){
   return 0;
 }
 
+// initialize a recursive mutex lock in a way that works on both glibc + musl
+int recursive_lock_init(pthread_mutex_t *lock){
+#ifndef __GLIBC__
+#define PTHREAD_MUTEX_RECURSIVE_NP PTHREAD_MUTEX_RECURSIVE
+#endif
+  pthread_mutexattr_t attr;
+  if(pthread_mutexattr_init(&attr)){
+    return -1;
+  }
+  if(pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE_NP)){
+    pthread_mutexattr_destroy(&attr);
+    return -1;
+  }
+  if(pthread_mutex_init(lock, &attr)){
+    pthread_mutexattr_destroy(&attr);
+    return -1;
+  }
+  pthread_mutexattr_destroy(&attr);
+  return 0;
+#ifndef __GLIBC__
+#undef PTHREAD_MUTEX_RECURSIVE_NP
+#endif
+}
+
 notcurses* notcurses_init(const notcurses_options* opts, FILE* outfp){
   notcurses_options defaultopts;
   memset(&defaultopts, 0, sizeof(defaultopts));
@@ -981,7 +1005,7 @@ notcurses* notcurses_init(const notcurses_options* opts, FILE* outfp){
   if(outfp == NULL){
     outfp = stdout;
   }
-  if(pthread_mutex_init(&ret->pilelock, NULL)){
+  if(recursive_lock_init(&ret->pilelock)){
     fprintf(stderr, "Couldn't initialize pile mutex\n");
     free(ret);
     return NULL;
@@ -1124,6 +1148,7 @@ notcurses* notcurses_init(const notcurses_options* opts, FILE* outfp){
 // updates *pile to point at (*pile)->next, frees all but standard pile/plane
 static void
 ncpile_drop(notcurses* nc, ncpile** pile){
+  ncpile* next = (*pile)->next;
   ncplane* p = (*pile)->top;
   while(p){
     ncplane* tmp = p->below;
@@ -1132,11 +1157,7 @@ ncpile_drop(notcurses* nc, ncpile** pile){
     }
     p = tmp;
   }
-  ncpile* tmp = (*pile)->next;
-  if(*pile != ncplane_pile(nc->stdplane)){
-    ncpile_destroy(*pile);
-  }
-  *pile = tmp;
+  *pile = next;
 }
 
 // drop all piles and all planes, save the standard plane and its pile
@@ -1956,10 +1977,16 @@ int ncplane_move_yx(ncplane* n, int y, int x){
 }
 
 int ncplane_y(const ncplane* n){
+  if(n->boundto == n){
+    return n->absy - ncplane_notcurses_const(n)->margin_t;
+  }
   return n->absy - n->boundto->absy;
 }
 
 int ncplane_x(const ncplane* n){
+  if(n->boundto == n){
+    return n->absx - ncplane_notcurses_const(n)->margin_t;
+  }
   return n->absx - n->boundto->absx;
 }
 
@@ -2150,49 +2177,93 @@ int ncplane_resize_realign(ncplane* n){
 }
 
 // The standard plane cannot be reparented; we return NULL in that case.
-// If provided a NULL |newparent|, we are moving |n| to its own stack. If |n|
-// is already root of its own stack in this case, we return NULL. If |n| is
-// already bound to |newparent|, this is a no-op, and we return |n|.
+// If provided |newparent|==|n|, we are moving |n| to its own stack. If |n|
+// is already bound to |newparent|, this is a no-op, and we return |n|.
 ncplane* ncplane_reparent(ncplane* n, ncplane* newparent){
-  if(n == ncplane_notcurses(n)->stdplane || n == newparent){
-    return NULL; // can't reparent standard plane, can't reparent to self
+  if(n == ncplane_notcurses(n)->stdplane){
+    return NULL; // can't reparent standard plane
   }
-  if(n->boundto == n && newparent == NULL){
-    return NULL; // can't make new stack out of a stack's root
+  if(n->boundto == newparent){
+    return n;
+  }
+  if(n->boundto == n){ // children become new root planes
+    for(ncplane* child = n->blist ; child ; child = child->bnext){
+      child->boundto = child;
+    }
+  }else{ // children are rebound to current parent
+    if(n->blist){
+      if( (n->blist->bnext = n->boundto->blist) ){
+        n->boundto->blist->bprev = &n->blist->bnext;
+      }
+      n->blist->bprev = &n->boundto->blist;
+      n->boundto->blist = n->blist;
+      n->blist = NULL;
+    }
   }
-  // FIXME take blist, add it to boundto
   return ncplane_reparent_family(n, newparent);
 }
 
 ncplane* ncplane_reparent_family(ncplane* n, ncplane* newparent){
-  if(n == ncplane_notcurses(n)->stdplane || n == newparent){
-    return NULL; // can't reparent standard plane, can't reparent to self
-  }
-  if(n->boundto == n && newparent == NULL){
-    return NULL; // can't make new stack out of a stack's root
-  }
-  if(newparent == NULL){ // FIXME make a new stack
-    newparent = ncplane_notcurses(n)->stdplane;
+  if(n == ncplane_notcurses(n)->stdplane){
+    return NULL; // can't reparent standard plane
   }
-  if(n->boundto == newparent){
+  if(n->boundto == newparent){ // no-op
     return n;
   }
-  if(n->bprev){
+  // 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;
     }
   }
+  // if leaving a pile, extract n from the old zaxis
+  if(n == newparent || ncplane_pile(n) != ncplane_pile(newparent)){
+    // FIXME need remove full family from z-axis, not just n!
+    if(ncplane_pile(n)->top == n){
+      ncplane_pile(n)->top = n->below;
+    }else{
+      n->above->below = n->below;
+    }
+    if(ncplane_pile(n)->bottom == n){
+      ncplane_pile(n)->bottom = n->above;
+    }else{
+      n->below->above = n->above;
+    }
+  }
   n->boundto = newparent;
-  if(newparent == NULL){
+  if(n == n->boundto){ // we're a new root plane
     n->bnext = NULL;
     n->bprev = NULL;
-    return n;
-  }
-  if( (n->bnext = newparent->blist) ){
-    n->bnext->bprev = &n->bnext;
+    pthread_mutex_lock(&ncplane_notcurses(n)->pilelock);
+    if(ncplane_pile(n)->top == NULL){ // did we just empty our pile?
+      ncpile_destroy(ncplane_pile(n));
+    }
+    make_ncpile(ncplane_notcurses(n), n);
+    pthread_mutex_unlock(&ncplane_notcurses(n)->pilelock);
+  }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;
+    }
+    n->bprev = &newparent->blist;
+    newparent->blist = n;
+    // place it immediately above the new binding plane if crossing piles
+    if(n->pile != ncplane_pile(n->boundto)){
+      n->pile = ncplane_pile(n->boundto);
+      if((n->above = n->boundto->above) == NULL){
+        n->pile->top = n;
+      }else{
+        n->boundto->above->below = n;
+      }
+      n->below = n->boundto;
+      n->boundto->above = n;
+    }
   }
-  n->bprev = &newparent->blist;
-  newparent->blist = n;
   return n;
 }
 

tests/ncplane.cpp

@@ -551,6 +551,7 @@ TEST_CASE("NCPlane") {
     CHECK(0 == testcell.gcluster);
     CHECK(0 == testcell.stylemask);
     CHECK(0 == testcell.channels);
+    cell_release(n_, &testcell);
     int dimy, dimx;
     ncplane_dim_yx(n_, &dimy, &dimx);
     REQUIRE(0 == ncplane_cursor_move_yx(n_, 1, dimx - strlen(STR2)));
@@ -588,6 +589,7 @@ TEST_CASE("NCPlane") {
     CHECK(0 == testcell.gcluster);
     CHECK(0 == testcell.stylemask);
     CHECK(0 == testcell.channels);
+    cell_release(n_, &testcell);
     int dimy, dimx;
     ncplane_dim_yx(n_, &dimy, &dimx);
     REQUIRE(0 == ncplane_cursor_move_yx(n_, 1, dimx - mbstowcs(nullptr, STR2, 0)));
@@ -897,25 +899,29 @@ TEST_CASE("NCPlane") {
       .x = 1,
       .rows = 2,
       .cols = 2,
-      nullptr, nullptr, nullptr, 0,
+      nullptr, "ndom", nullptr, 0,
     };
     struct ncplane* ndom = ncplane_create(n_, &nopts);
+    CHECK(ncplane_pile(ndom) == ncplane_pile(n_));
     REQUIRE(ndom);
+    nopts.name = "sub";
     struct ncplane* nsub = ncplane_create(ndom, &nopts);
+    CHECK(ncplane_pile(nsub) == ncplane_pile(ndom));
     REQUIRE(nsub);
     int absy, absx;
     CHECK(0 == notcurses_render(nc_));
     ncplane_yx(nsub, &absy, &absx);
     CHECK(1 == absy); // actually at 2, 2
     CHECK(1 == absx);
-    ncplane_reparent(nsub, nullptr);
+    ncplane_reparent(nsub, nsub);
+    CHECK(ncplane_pile(nsub) != ncplane_pile(ndom));
     ncplane_yx(nsub, &absy, &absx);
-    CHECK(2 == absy); // now we recognize 2, 2
-    CHECK(2 == absx);
+    CHECK(1 == absy); // now truly at 1, 1
+    CHECK(1 == absx);
     CHECK(0 == ncplane_move_yx(ndom, 0, 0));
     ncplane_yx(nsub, &absy, &absx);
-    CHECK(2 == absy); // still at 2, 2
-    CHECK(2 == absx);
+    CHECK(1 == absy); // still at 1, 1
+    CHECK(1 == absx);
   }
 
   SUBCASE("NoReparentStdPlane") {