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.


Discussion / hints:
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/gpm.c b/src/lib/gpm.c
index 40f3fafc35..e01e3b9654 100644
--- a/src/lib/gpm.c
+++ b/src/lib/gpm.c
@@ -26,9 +26,9 @@ gpmwatcher(void* vti){
       logwarn("negative input %hd %hd", gev.x, gev.y);
       continue;
     }
-    ++gev.x;
-    ++gev.y;
-    if(snprintf(cmdbuf + 3, space, "%hd;%hd;%hdM", 0, gev.x, gev.y) >= space){
+    // gpm is 0-indexed, but we assume mice reports to be 1-indexed, as they
+    // are in the XTerm protocols. no need to account for margins here.
+    if(snprintf(cmdbuf + 3, space, "%hd;%hd;%hdM", 0, gev.x + 1, gev.y + 1) >= space){
       logwarn("input overflowed %hd %hd\n", gev.x, gev.y);
       continue;
     }
@@ -38,7 +38,6 @@ gpmwatcher(void* vti){
 }
 
 int gpm_connect(tinfo* ti){
-  (void)ti;
   gpm_zerobased = 1;
   // get all of _MOVE, _DRAG, _DOWN, and _UP
   gpmconn.eventMask = GPM_DRAG | GPM_DOWN | GPM_UP;
diff --git a/src/lib/in.c b/src/lib/in.c
index 91f8149a07..199cb49e76 100644
--- a/src/lib/in.c
+++ b/src/lib/in.c
@@ -806,10 +806,17 @@ set_sda_version(inputctx* ictx){
 // ictx->numeric, ictx->p3, and ictx->p2 have the two parameters
 static void
 mouse_click(inputctx* ictx){
+  // convert from 1- to 0-indexing, and account for margins
+  const int x = ictx->p3 - 1 - ictx->lmargin;
+  const int y = ictx->numeric - 1 - ictx->tmargin;
+  if(x < 0 || y < 0){ // click was in margins, drop it
+    logwarn("dropping click in margins\n");
+    return;
+  }
   pthread_mutex_lock(&ictx->ilock);
   if(ictx->ivalid == ictx->isize){
     pthread_mutex_unlock(&ictx->ilock);
-    logerror("dropping mouse click 0x%02x %d %d\n", ictx->p2, ictx->p3, ictx->numeric);
+    logerror("dropping mouse click 0x%02x %d %d\n", ictx->p2, y, x);
     return;
   }
   ncinput* ni = ictx->inputs + ictx->iwrite;
@@ -827,9 +834,8 @@ mouse_click(inputctx* ictx){
   ni->ctrl = ictx->p2 & 0x10;
   ni->alt = ictx->p2 & 0x08;
   ni->shift = ictx->p2 & 0x04;
-  // convert from 1- to 0-indexing and account for margins
-  ni->x = ictx->p3 - 1 - ictx->lmargin;
-  ni->y = ictx->numeric - 1 - ictx->tmargin;
+  ni->x = x;
+  ni->y = y;
   if(++ictx->iwrite == ictx->isize){
     ictx->iwrite = 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;
 }
 
diff --git a/src/media/ffmpeg.c b/src/media/ffmpeg.c
index 60123646a0..d3e0e2b7e9 100644
--- a/src/media/ffmpeg.c
+++ b/src/media/ffmpeg.c
@@ -395,7 +395,12 @@ ncvisual* ffmpeg_from_file(const char* filename){
     goto err;
   }
 //av_dump_format(ncv->details->fmtctx, 0, filename, false);
-  if((averr = av_find_best_stream(ncv->details->fmtctx, AVMEDIA_TYPE_SUBTITLE, -1, -1, &ncv->details->subtcodec, 0)) >= 0){
+  if((averr = av_find_best_stream(ncv->details->fmtctx, AVMEDIA_TYPE_SUBTITLE, -1, -1,
+#if LIBAVFORMAT_VERSION_MAJOR >= 59
+                                  (const AVCodec**)&ncv->details->subtcodec, 0)) >= 0){
+#else
+                                  &ncv->details->subtcodec, 0)) >= 0){
+#endif
     ncv->details->sub_stream_index = averr;
     if((ncv->details->subtcodecctx = avcodec_alloc_context3(ncv->details->subtcodec)) == NULL){
       //fprintf(stderr, "Couldn't allocate decoder for %s\n", filename);
@@ -414,7 +419,12 @@ ncvisual* ffmpeg_from_file(const char* filename){
     // fprintf(stderr, "Couldn't allocate packet for %s\n", filename);
     goto err;
   }
-  if((averr = av_find_best_stream(ncv->details->fmtctx, AVMEDIA_TYPE_VIDEO, -1, -1, &ncv->details->codec, 0)) < 0){
+  if((averr = av_find_best_stream(ncv->details->fmtctx, AVMEDIA_TYPE_VIDEO, -1, -1,
+#if LIBAVFORMAT_VERSION_MAJOR >= 59
+                                  (const AVCodec**)&ncv->details->codec, 0)) < 0){
+#else
+                                  &ncv->details->codec, 0)) < 0){
+#endif
     // fprintf(stderr, "Couldn't find visuals in %s (%s)\n", filename, av_err2str(*averr));
     goto err;
   }


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: |