Repository: dankamongmen/notcurses (language: python)

Issue:
ncdirect_dump_plane needs to use fbuf-based output
We're seeing `ncdirect_dump_plane()` die on macOS due to `stdout` blocking up. We need to move off stdio for cell blitting, as we've already done for pixel blitting in the same function.
ncdirect_dump_plane needs to use fbuf-based output
We're seeing `ncdirect_dump_plane()` die on macOS due to `stdout` blocking up. We need to move off stdio for cell blitting, as we've already done for pixel blitting in the same function.


Discussion / hints:
we'd need `fbuf` versions of the various color functionality and at least `cursor_down()`, ugh
we'd need `fbuf` versions of the various color functionality and at least `cursor_down()`, ugh

Fix patch:
diff --git a/src/lib/direct.c b/src/lib/direct.c
index 19b8b84706..57701b0e54 100644
--- a/src/lib/direct.c
+++ b/src/lib/direct.c
@@ -130,6 +130,11 @@ int ncdirect_cursor_down(ncdirect* nc, int num){
   return ret;
 }
 
+static inline int
+ncdirect_cursor_down_f(ncdirect* nc, int num, fbuf* f){
+  return emit_scrolls(&nc->tcache, num, f);
+}
+
 int ncdirect_clear(ncdirect* nc){
   const char* clearscr = get_escape(&nc->tcache, ESCAPE_CLEAR);
   if(clearscr){
@@ -426,87 +431,104 @@ ncdirect_align(struct ncdirect* n, ncalign_e align, int c){
   return INT_MAX;
 }
 
+// y is an out-only param, indicating the location where drawing started
 static int
-ncdirect_dump_plane(ncdirect* n, const ncplane* np, int xoff){
-  const int toty = ncdirect_dim_y(n);
+ncdirect_dump_sprixel(ncdirect* n, const ncplane* np, int xoff, int* y, fbuf* f){
   int dimy, dimx;
   ncplane_dim_yx(np, &dimy, &dimx);
-  if(np->sprite){
-    int y;
-    const char* u7 = get_escape(&n->tcache, ESCAPE_U7);
-    if(cursor_yx_get(n, n->tcache.ttyfd, u7, &y, NULL)){
-      return -1;
+  const int toty = ncdirect_dim_y(n);
+  // flush our FILE*, as we're about to use UNIX I/O (since we can't rely on
+  // stdio to transfer large amounts at once).
+  if(ncdirect_flush(n)){
+    return -1;
+  }
+  if(ncdirect_cursor_yx(n, y, NULL)){
+    return -1;
+  }
+  if(toty - dimy < *y){
+    int scrolls = *y - 1;
+    *y = toty - dimy;
+    if(*y < 0){
+      *y = 0;
     }
-    if(ncdirect_cursor_move_yx(n, y, xoff)){
+    scrolls -= *y;
+    // perform our scrolling outside of the fbuf framework, as we need it
+    // to happen immediately for fbcon
+    if(ncdirect_cursor_move_yx(n, *y, xoff)){
       return -1;
     }
-    // flush our FILE*, as we're about to use UNIX I/O (since we can't rely on
-    // stdio to transfer large amounts at once).
-    if(ncdirect_flush(n)){
+    if(emit_scrolls(&n->tcache, scrolls, f) < 0){
       return -1;
     }
-    fbuf f = {};
-    if(cursor_yx_get(n, n->tcache.ttyfd, u7, &y, NULL)){
+  }
+  if(sprite_draw(&n->tcache, NULL, np->sprite, f, *y, xoff) < 0){
+    return -1;
+  }
+  if(sprite_commit(&n->tcache, f, np->sprite, true)){
+    return -1;
+  }
+  return 0;
+}
+
+static int
+ncdirect_set_bg_default_f(ncdirect* nc, fbuf* f){
+  if(ncdirect_bg_default_p(nc)){
+    return 0;
+  }
+  const char* esc;
+  if((esc = get_escape(&nc->tcache, ESCAPE_BGOP)) != NULL){
+    if(fbuf_emit(f, esc) < 0){
       return -1;
     }
-    if(fbuf_init(&f)){
+  }else if((esc = get_escape(&nc->tcache, ESCAPE_OP)) != NULL){
+    if(fbuf_emit(f, esc) < 0){
       return -1;
     }
-    if(toty - dimy < y){
-      int scrolls = y - 1;
-      y = toty - dimy;
-      if(y < 0){
-        y = 0;
-      }
-      scrolls -= y;
-      // perform our scrolling outside of the fbuf framework, as we need it
-      // to happen immediately for fbdcon
-      if(ncdirect_cursor_move_yx(n, y, xoff)){
-        fbuf_free(&f);
-        return -1;
-      }
-      if(emit_scrolls(&n->tcache, scrolls, &f) < 0){
-        fbuf_free(&f);
+    if(!ncdirect_fg_default_p(nc)){
+      if(ncdirect_set_fg_rgb_f(nc, ncchannels_fg_rgb(nc->channels), f)){
         return -1;
       }
     }
-    if(sprite_draw(&n->tcache, NULL, np->sprite, &f, y, xoff) < 0){
-      fbuf_free(&f);
-      return -1;
-    }
-    if(sprite_commit(&n->tcache, &f, np->sprite, true)){
-      fbuf_free(&f);
+  }
+  ncchannels_set_bg_default(&nc->channels);
+  return 0;
+}
+
+static int
+ncdirect_set_fg_default_f(ncdirect* nc, fbuf* f){
+  if(ncdirect_fg_default_p(nc)){
+    return 0;
+  }
+  const char* esc;
+  if((esc = get_escape(&nc->tcache, ESCAPE_FGOP)) != NULL){
+    if(fbuf_emit(f, esc) < 0){
       return -1;
     }
-    if(fbuf_finalize(&f, n->ttyfp)){
+  }else if((esc = get_escape(&nc->tcache, ESCAPE_OP)) != NULL){
+    if(fbuf_emit(f, esc) < 0){
       return -1;
     }
-    if(n->tcache.pixel_draw_late){
-      if(n->tcache.pixel_draw_late(&n->tcache, np->sprite, y, xoff) < 0){
+    if(!ncdirect_bg_default_p(nc)){
+      if(ncdirect_set_bg_rgb_f(nc, ncchannels_bg_rgb(nc->channels), f)){
         return -1;
       }
     }
-    int targy = y + dimy;
-    if(targy > toty){
-      targy = toty;
-    }
-    if(ncdirect_cursor_move_yx(n, targy, xoff)){
-      return -1;
-    }
-    return 0;
   }
-//fprintf(stderr, "rasterizing %dx%d+%d\n", dimy, dimx, xoff);
+  ncchannels_set_fg_default(&nc->channels);
+  return 0;
+}
+
+static int
+ncdirect_dump_cellplane(ncdirect* n, const ncplane* np, fbuf* f){
+  int dimy, dimx;
+  ncplane_dim_yx(np, &dimy, &dimx);
+  const int toty = ncdirect_dim_y(n);
   // save the existing style and colors
   const bool fgdefault = ncdirect_fg_default_p(n);
   const bool bgdefault = ncdirect_bg_default_p(n);
   const uint32_t fgrgb = ncchannels_fg_rgb(n->channels);
   const uint32_t bgrgb = ncchannels_bg_rgb(n->channels);
   for(int y = 0 ; y < dimy ; ++y){
-    if(xoff){
-      if(ncdirect_cursor_move_yx(n, -1, xoff)){
-        return -1;
-      }
-    }
     for(int x = 0 ; x < dimx ; ++x){
       uint16_t stylemask;
       uint64_t channels;
@@ -515,17 +537,18 @@ ncdirect_dump_plane(ncdirect* n, const ncplane* np, int xoff){
         return -1;
       }
       if(ncchannels_fg_alpha(channels) == NCALPHA_TRANSPARENT){
-        ncdirect_set_fg_default(n);
+        ncdirect_set_fg_default_f(n, f);
       }else{
-        ncdirect_set_fg_rgb(n, ncchannels_fg_rgb(channels));
+        ncdirect_set_fg_rgb_f(n, ncchannels_fg_rgb(channels), f);
       }
       if(ncchannels_bg_alpha(channels) == NCALPHA_TRANSPARENT){
-        ncdirect_set_bg_default(n);
+        ncdirect_set_bg_default_f(n, f);
       }else{
-        ncdirect_set_bg_rgb(n, ncchannels_bg_rgb(channels));
+        ncdirect_set_bg_rgb_f(n, ncchannels_bg_rgb(channels), f);
       }
 //fprintf(stderr, "%03d/%03d [%s] (%03dx%03d)\n", y, x, egc, dimy, dimx);
-      if(fprintf(n->ttyfp, "%s", strlen(egc) == 0 ? " " : egc) < 0){
+      size_t egclen = strlen(egc);
+      if(fbuf_putn(f, egclen == 0 ? " " : egc, egclen == 0 ? 1 : egclen) < 0){
         free(egc);
         return -1;
       }
@@ -535,27 +558,73 @@ ncdirect_dump_plane(ncdirect* n, const ncplane* np, int xoff){
     // each line of output; this is necessary if our output is lifted out and
     // used in something e.g. paste(1).
     // FIXME replace with a SGR clear
-    ncdirect_set_fg_default(n);
-    ncdirect_set_bg_default(n);
-    if(ncfputc('\n', n->ttyfp) == EOF){
+    ncdirect_set_fg_default_f(n, f);
+    ncdirect_set_bg_default_f(n, f);
+    if(fbuf_putc(f, '\n') < 0){
       return -1;
     }
     if(y == toty){
-      if(ncdirect_cursor_down(n, 1)){
+      if(ncdirect_cursor_down_f(n, 1, f)){
         return -1;
       }
     }
   }
   // restore the previous colors
   if(fgdefault){
-    ncdirect_set_fg_default(n);
+    ncdirect_set_fg_default_f(n, f);
   }else{
-    ncdirect_set_fg_rgb(n, fgrgb);
+    ncdirect_set_fg_rgb_f(n, fgrgb, f);
   }
   if(bgdefault){
-    ncdirect_set_bg_default(n);
+    ncdirect_set_bg_default_f(n, f);
+  }else{
+    ncdirect_set_bg_rgb_f(n, bgrgb, f);
+  }
+  return 0;
+}
+
+static int
+ncdirect_dump_plane(ncdirect* n, const ncplane* np, int xoff){
+//fprintf(stderr, "rasterizing %dx%d+%d\n", dimy, dimx, xoff);
+  if(xoff){
+    if(ncdirect_cursor_move_yx(n, -1, xoff)){
+      return -1;
+    }
+  }
+  fbuf f;
+  if(fbuf_init(&f)){
+    return -1;
+  }
+  if(np->sprite){
+    int y;
+    if(ncdirect_dump_sprixel(n, np, xoff, &y, &f)){
+      fbuf_free(&f);
+      return -1;
+    }
+    if(fbuf_finalize(&f, n->ttyfp)){
+      return -1;
+    }
+    if(n->tcache.pixel_draw_late){
+      if(n->tcache.pixel_draw_late(&n->tcache, np->sprite, y, xoff) < 0){
+        return -1;
+      }
+    }
+    int targy = y + ncplane_dim_y(np);
+    const int toty = ncdirect_dim_y(n);
+    if(targy > toty){
+      targy = toty;
+    }
+    if(ncdirect_cursor_move_yx(n, targy, xoff)){
+      return -1;
+    }
   }else{
-    ncdirect_set_bg_rgb(n, bgrgb);
+    if(ncdirect_dump_cellplane(n, np, &f)){
+      fbuf_free(&f);
+      return -1;
+    }
+    if(fbuf_finalize(&f, n->ttyfp)){
+      return -1;
+    }
   }
   return 0;
 }
diff --git a/src/lib/internal.h b/src/lib/internal.h
index cf04c3f6e1..4f70102402 100644
--- a/src/lib/internal.h
+++ b/src/lib/internal.h
@@ -1607,6 +1607,8 @@ ncdirect_bg_palindex_p(const ncdirect* nc){
   return ncchannels_bg_palindex_p(ncdirect_channels(nc));
 }
 
+int ncdirect_set_fg_rgb_f(ncdirect* nc, unsigned rgb, fbuf* f);
+int ncdirect_set_bg_rgb_f(ncdirect* nc, unsigned rgb, fbuf* f);
 int term_fg_rgb8(const tinfo* ti, fbuf* f, unsigned r, unsigned g, unsigned b);
 
 const struct blitset* lookup_blitset(const tinfo* tcache, ncblitter_e setid, bool may_degrade);
diff --git a/src/lib/render.c b/src/lib/render.c
index 19b15f60e9..3d5fc79cf9 100644
--- a/src/lib/render.c
+++ b/src/lib/render.c
@@ -1603,7 +1603,7 @@ char* notcurses_at_yx(notcurses* nc, int yoff, int xoff, uint16_t* stylemask, ui
   return egc;
 }
 
-int ncdirect_set_bg_rgb(ncdirect* nc, unsigned rgb){
+int ncdirect_set_bg_rgb_f(ncdirect* nc, unsigned rgb, fbuf* f){
   if(rgb > 0xffffffu){
     return -1;
   }
@@ -1611,22 +1611,29 @@ int ncdirect_set_bg_rgb(ncdirect* nc, unsigned rgb){
      && ncchannels_bg_rgb(nc->channels) == rgb){
     return 0;
   }
+  if(term_bg_rgb8(&nc->tcache, f, (rgb & 0xff0000u) >> 16u, (rgb & 0xff00u) >> 8u, rgb & 0xffu)){
+    return -1;
+  }
+  ncchannels_set_bg_rgb(&nc->channels, rgb);
+  return 0;
+}
+
+int ncdirect_set_bg_rgb(ncdirect* nc, unsigned rgb){
   fbuf f = {};
   if(fbuf_init_small(&f)){
     return -1;
   }
-  if(term_bg_rgb8(&nc->tcache, &f, (rgb & 0xff0000u) >> 16u, (rgb & 0xff00u) >> 8u, rgb & 0xffu)){
+  if(ncdirect_set_bg_rgb_f(nc, rgb, &f)){
     fbuf_free(&f);
     return -1;
   }
   if(fbuf_finalize(&f, nc->ttyfp) < 0){
     return -1;
   }
-  ncchannels_set_bg_rgb(&nc->channels, rgb);
   return 0;
 }
 
-int ncdirect_set_fg_rgb(ncdirect* nc, unsigned rgb){
+int ncdirect_set_fg_rgb_f(ncdirect* nc, unsigned rgb, fbuf* f){
   if(rgb > 0xffffffu){
     return -1;
   }
@@ -1634,18 +1641,25 @@ int ncdirect_set_fg_rgb(ncdirect* nc, unsigned rgb){
      && ncchannels_fg_rgb(nc->channels) == rgb){
     return 0;
   }
+  if(term_fg_rgb8(&nc->tcache, f, (rgb & 0xff0000u) >> 16u, (rgb & 0xff00u) >> 8u, rgb & 0xffu)){
+    return -1;
+  }
+  ncchannels_set_fg_rgb(&nc->channels, rgb);
+  return 0;
+}
+
+int ncdirect_set_fg_rgb(ncdirect* nc, unsigned rgb){
   fbuf f = {};
   if(fbuf_init_small(&f)){
     return -1;
   }
-  if(term_fg_rgb8(&nc->tcache, &f, (rgb & 0xff0000u) >> 16u, (rgb & 0xff00u) >> 8u, rgb & 0xffu)){
+  if(ncdirect_set_fg_rgb_f(nc, rgb, &f)){
     fbuf_free(&f);
     return -1;
   }
   if(fbuf_finalize(&f, nc->ttyfp) < 0){
     return -1;
   }
-  ncchannels_set_fg_rgb(&nc->channels, rgb);
   return 0;
 }
 


Test patch:
diff --git a/.github/workflows/macos_test.yml b/.github/workflows/macos_test.yml
index 8100d11323..58ebbef382 100644
--- a/.github/workflows/macos_test.yml
+++ b/.github/workflows/macos_test.yml
@@ -26,7 +26,6 @@ jobs:
             ffmpeg \
             libunistring \
             ncurses \
-            pandoc \
             readline
 
       - uses: actions/checkout@v2
@@ -35,7 +34,8 @@ jobs:
         run: |
           mkdir build && cd build
           cmake .. \
-            -DCMAKE_BUILD_TYPE=Release
+            -DCMAKE_BUILD_TYPE=Release \
+            -DUSE_PANDOC=off
 
       - name: make
         run: |
@@ -45,7 +45,8 @@ jobs:
       - name: ctest
         run: |
           cd build
-          ctest --output-on-failure
+          make test
+          #ctest --output-on-failure
 
       - name: make install
         run: |