Repository: dankamongmen/notcurses (language: python)

Issue:
New function ncvisual_from_rgb
I belive it would be very handy to have an `ncvisual_from_rgb()` function to read rgb data from a buffer without the alpha byte, and apply the same alpha to all pixels, through an additional argument. With a signature like:

```c
API ALLOC struct ncvisual* ncvisual_from_rgb(const void* rgb, int rows,
                                              int rowstride, int cols, unsigned char alpha);
```

I'm encountering the case where having to interface with the output of other libraries that doesn't deal with alpha makes it somewhat cumbersome.

And maybe in that case it should also be added `ncvisual_from_bgr` for completion?
New function ncvisual_from_rgb
I belive it would be very handy to have an `ncvisual_from_rgb()` function to read rgb data from a buffer without the alpha byte, and apply the same alpha to all pixels, through an additional argument. With a signature like:

```c
API ALLOC struct ncvisual* ncvisual_from_rgb(const void* rgb, int rows,
                                              int rowstride, int cols, unsigned char alpha);
```

I'm encountering the case where having to interface with the output of other libraries that doesn't deal with alpha makes it somewhat cumbersome.

And maybe in that case it should also be added `ncvisual_from_bgr` for completion?


Discussion / hints:
I'm pretty sure I already added something like this in response to a request from dear @kaniini , let me go check...
```c
// Supply an alpha value [0..255] to be applied throughout. linesize must be                               
// a multiple of 3 for this RGB data.                                                                      
API int ncblit_rgb_packed(const void* data, int linesize,                                                  
                          const struct ncvisual_options* vopts, int alpha);                                
                                                                                                           
// Supply an alpha value [0..255] to be applied throughout. linesize must be                               
// a multiple of 4 for this RGBx data.                                                                     
API int ncblit_rgb_loose(const void* data, int linesize,                                                   
                         const struct ncvisual_options* vopts, int alpha);                                 
```

so that looks like exactly what you want? minus the `bgr` completeness.
oh this doesn't create a visual, though. ok, we can add

```c
ncvisual* ncvisual_from_rgb_packed(...);
ncvisual* ncvisual_from_rgb_loose(...);
```

and maybe

```c
ncvisual* ncvisual_from_bgr_packed(...);
ncvisual* ncvisual_from_bgr_loose(...);
```

does BGR really come up when you're not doing 32-bit words? i assumed that was a relic of endianness issues.
I personally don't care about bgr at all. So maybe we can leave that out.
Although if it's left out it will probably came again in the future by some else...
I'm pretty sure I already added something like this in response to a request from dear @kaniini , let me go check...
```c
// Supply an alpha value [0..255] to be applied throughout. linesize must be                               
// a multiple of 3 for this RGB data.                                                                      
API int ncblit_rgb_packed(const void* data, int linesize,                                                  
                          const struct ncvisual_options* vopts, int alpha);                                
                                                                                                           
// Supply an alpha value [0..255] to be applied throughout. linesize must be                               
// a multiple of 4 for this RGBx data.                                                                     
API int ncblit_rgb_loose(const void* data, int linesize,                                                   
                         const struct ncvisual_options* vopts, int alpha);                                 
```

so that looks like exactly what you want? minus the `bgr` completeness.
oh this doesn't create a visual, though. ok, we can add

```c
ncvisual* ncvisual_from_rgb_packed(...);
ncvisual* ncvisual_from_rgb_loose(...);
```

and maybe

```c
ncvisual* ncvisual_from_bgr_packed(...);
ncvisual* ncvisual_from_bgr_loose(...);
```

does BGR really come up when you're not doing 32-bit words? i assumed that was a relic of endianness issues.
I personally don't care about bgr at all. So maybe we can leave that out.
Although if it's left out it will probably came again in the future by some else...

Fix patch:
diff --git a/NEWS.md b/NEWS.md
index 03f358ae6d..ba1fec1e9e 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -26,6 +26,7 @@ rearrangements of Notcurses.
     `const ncdirect*`, since they update the term parameters. Sorry!
   * Added `NCDIRECT_OPTION_VERBOSE` and `NCDIRECT_OPTION_VERY_VERBOSE`.
     They map to `NCLOGLEVEL_WARNING` and `NCLOGLEVEL_TRACE`, respectively.
+  * New functions `ncvisual_from_rgb_packed()` and `ncvisual_from_rgb_loose()`.
 
 * 2.3.4 (2021-06-12)
   * Added the flag `NCVISUAL_OPTION_NOINTERPOLATE` to use non-interpolative
diff --git a/USAGE.md b/USAGE.md
index 1d2bf8f98e..3b97c31df9 100644
--- a/USAGE.md
+++ b/USAGE.md
@@ -1600,7 +1600,7 @@ int ncblit_rgb_loose(const void* data, int linesize,
                      const struct ncvisual_options* vopts, int alpha);
 
 // Same as ncblit_rgba(), but for RGB, with 'alpha' supplied as an alpha value
-// throughout, 0 <= 'alpha' <= 255. linesize ought be a multiple of 3.
+// throughout, 0 <= 'alpha' <= 255.
 int ncblit_rgb_packed(const void* data, int linesize,
                       const struct ncvisual_options* vopts, int alpha);
 
@@ -3087,6 +3087,16 @@ constructed directly from RGBA or BGRA 8bpc memory:
 struct ncvisual* ncvisual_from_rgba(const void* rgba, int rows,
                                     int rowstride, int cols);
 
+// ncvisual_from_rgba(), but the pixels are 4-byte RGBx. A is filled in
+// throughout using 'alpha'. rowstride must be a multiple of 4.
+struct ncvisual* ncvisual_from_rgb_packed(const void* rgba, int rows,
+                                          int rowstride, int cols, int alpha);
+
+// ncvisual_from_rgba(), but the pixels are 3-byte RGB. A is filled in
+// throughout using 'alpha'.
+struct ncvisual* ncvisual_from_rgb_loose(const void* rgba, int rows,
+                                         int rowstride, int cols, int alpha);
+
 // ncvisual_from_rgba(), but for BGRA.
 struct ncvisual* ncvisual_from_bgra(struct notcurses* nc, const void* bgra,
                                     int rows, int rowstride, int cols);
diff --git a/doc/man/man3/notcurses_visual.3.md b/doc/man/man3/notcurses_visual.3.md
index 55884a5010..e705a7058c 100644
--- a/doc/man/man3/notcurses_visual.3.md
+++ b/doc/man/man3/notcurses_visual.3.md
@@ -56,6 +56,10 @@ typedef int (*streamcb)(struct notcurses*, struct ncvisual*, void*);
 
 **struct ncvisual* ncvisual_from_rgba(const void* ***rgba***, int ***rows***, int ***rowstride***, int ***cols***);**
 
+**struct ncvisual* ncvisual_from_rgb_packed(const void* ***rgba***, int ***rows***, int ***rowstride***, int ***cols***, int ***alpha***);**
+
+**struct ncvisual* ncvisual_from_rgb_loose(const void* ***rgba***, int ***rows***, int ***rowstride***, int ***cols***, int ***alpha***);**
+
 **struct ncvisual* ncvisual_from_bgra(const void* ***bgra***, int ***rows***, int ***rowstride***, int ***cols***);**
 
 **struct ncvisual* ncvisual_from_plane(struct ncplane* ***n***, ncblitter_e ***blit***, int ***begy***, int ***begx***, int ***leny***, int ***lenx***);**
@@ -129,10 +133,14 @@ thus must be at least ***rowstride*** * ***rows*** bytes, of which a
 ***cols*** * ***rows*** * 4-byte subset is used. It is not possible to **mmap(2)** an image
 file and use it directly--decompressed, decoded data is necessary. The
 resulting plane will be ceil(**rows**/2) rows, and **cols** columns.
+
+**ncvisual_from_rgb_packed** performs the same using 3-byte RGB source data.
+**ncvisual_from_rgb_loose** uses 4-byte RGBx source data. Both will fill in
+the alpha component of every target pixel with the specified **alpha**.
+
 **ncvisual_from_plane** requires specification of a rectangle via ***begy***,
-***begx***, ***leny***, and ***lenx***. The only valid characters within this
-region are those used by the **NCBLIT_2x2** blitter, though this may change
-in the future.
+***begx***, ***leny***, and ***lenx***, and also a blitter. The only valid
+glyphs within this region are those used by the specified blitter.
 
 **ncvisual_rotate** executes a rotation of ***rads*** radians, in the clockwise
 (positive) or counterclockwise (negative) direction.
diff --git a/include/notcurses/notcurses.h b/include/notcurses/notcurses.h
index 1cff5c2884..553c855d50 100644
--- a/include/notcurses/notcurses.h
+++ b/include/notcurses/notcurses.h
@@ -2531,6 +2531,18 @@ API ALLOC struct ncvisual* ncvisual_from_file(const char* file);
 API ALLOC struct ncvisual* ncvisual_from_rgba(const void* rgba, int rows,
                                               int rowstride, int cols);
 
+// ncvisual_from_rgba(), but the pixels are 4-byte RGBx. A is filled in
+// throughout using 'alpha'. rowstride must be a multiple of 4.
+API ALLOC struct ncvisual* ncvisual_from_rgb_packed(const void* rgba, int rows,
+                                                    int rowstride, int cols,
+                                                    int alpha);
+
+// ncvisual_from_rgba(), but the pixels are 3-byte RGB. A is filled in
+// throughout using 'alpha'.
+API ALLOC struct ncvisual* ncvisual_from_rgb_loose(const void* rgba, int rows,
+                                                   int rowstride, int cols,
+                                                   int alpha);
+
 // ncvisual_from_rgba(), but 'bgra' is arranged as BGRA. note that this is a
 // byte-oriented layout, despite being bunched in 32-bit pixels; the lowest
 // memory address ought be B, and A is reached by adding 3 to that address.
@@ -2739,8 +2751,7 @@ API int ncblit_rgba(const void* data, int linesize,
 API int ncblit_bgrx(const void* data, int linesize,
                     const struct ncvisual_options* vopts);
 
-// Supply an alpha value [0..255] to be applied throughout. linesize must be
-// a multiple of 3 for this RGB data.
+// Supply an alpha value [0..255] to be applied throughout.
 API int ncblit_rgb_packed(const void* data, int linesize,
                           const struct ncvisual_options* vopts, int alpha);
 
diff --git a/src/lib/visual.c b/src/lib/visual.c
index 5be1353149..4ce8c88fb7 100644
--- a/src/lib/visual.c
+++ b/src/lib/visual.c
@@ -285,9 +285,6 @@ void* rgb_loose_to_rgba(const void* data, int rows, int* rowstride, int cols, in
 }
 
 void* rgb_packed_to_rgba(const void* data, int rows, int* rowstride, int cols, int alpha){
-  if(*rowstride % 3){ // must be a multiple of 3 bytes
-    return NULL;
-  }
   if(*rowstride < cols * 3){
     return NULL;
   }
@@ -581,6 +578,7 @@ pad_for_image(size_t stride){
 
 ncvisual* ncvisual_from_rgba(const void* rgba, int rows, int rowstride, int cols){
   if(rowstride % 4){
+    logerror("Rowstride %d not a multiple of 4\n", rowstride);
     return NULL;
   }
   ncvisual* ncv = ncvisual_create();
@@ -604,6 +602,68 @@ ncvisual* ncvisual_from_rgba(const void* rgba, int rows, int rowstride, int cols
   return ncv;
 }
 
+ncvisual* ncvisual_from_rgb_packed(const void* rgba, int rows, int rowstride,
+                                   int cols, int alpha){
+  ncvisual* ncv = ncvisual_create();
+  if(ncv){
+    ncv->rowstride = pad_for_image(rowstride);
+    ncv->pixx = cols;
+    ncv->pixy = rows;
+    uint32_t* data = malloc(ncv->rowstride * ncv->pixy);
+    if(data == NULL){
+      ncvisual_destroy(ncv);
+      return NULL;
+    }
+    const unsigned char* src = rgba;
+    for(int y = 0 ; y < rows ; ++y){
+//fprintf(stderr, "ROWS: %d STRIDE: %d (%d) COLS: %d %08x\n", ncv->pixy, ncv->rowstride, ncv->rowstride / 4, cols, data[ncv->rowstride * y / 4]);
+      for(int x = 0 ; x < cols ; ++x){
+        unsigned char r, g, b;
+        memcpy(&r, src + rowstride * y + 3 * x, 1);
+        memcpy(&g, src + rowstride * y + 3 * x + 1, 1);
+        memcpy(&b, src + rowstride * y + 3 * x + 2, 1);
+        ncpixel_set_a(&data[y * ncv->rowstride / 4 + x], alpha);
+        ncpixel_set_r(&data[y * ncv->rowstride / 4 + x], r);
+        ncpixel_set_g(&data[y * ncv->rowstride / 4 + x], g);
+        ncpixel_set_b(&data[y * ncv->rowstride / 4 + x], b);
+//fprintf(stderr, "RGBA: 0x%02x 0x%02x 0x%02x 0x%02x\n", r, g, b, alpha);
+      }
+    }
+    ncvisual_set_data(ncv, data, true);
+    ncvisual_details_seed(ncv);
+  }
+  return ncv;
+}
+
+ncvisual* ncvisual_from_rgb_loose(const void* rgba, int rows, int rowstride,
+                                  int cols, int alpha){
+  if(rowstride % 4){
+    logerror("Rowstride %d not a multiple of 4\n", rowstride);
+    return NULL;
+  }
+  ncvisual* ncv = ncvisual_create();
+  if(ncv){
+    ncv->rowstride = pad_for_image(rowstride);
+    ncv->pixx = cols;
+    ncv->pixy = rows;
+    uint32_t* data = malloc(ncv->rowstride * ncv->pixy);
+    if(data == NULL){
+      ncvisual_destroy(ncv);
+      return NULL;
+    }
+    for(int y = 0 ; y < rows ; ++y){
+//fprintf(stderr, "ROWS: %d STRIDE: %d (%d) COLS: %d %08x\n", ncv->pixy, ncv->rowstride, ncv->rowstride / 4, cols, data[ncv->rowstride * y / 4]);
+      memcpy(data + (ncv->rowstride * y) / 4, (const char*)rgba + rowstride * y, rowstride);
+      for(int x = 0 ; x < cols ; ++x){
+        ncpixel_set_a(&data[y * ncv->rowstride / 4 + x], alpha);
+      }
+    }
+    ncvisual_set_data(ncv, data, true);
+    ncvisual_details_seed(ncv);
+  }
+  return ncv;
+}
+
 ncvisual* ncvisual_from_bgra(const void* bgra, int rows, int rowstride, int cols){
   if(rowstride % 4){
     return NULL;


Test patch:
diff --git a/src/tests/visual.cpp b/src/tests/visual.cpp
index d2d01ca104..fe635b0550 100644
--- a/src/tests/visual.cpp
+++ b/src/tests/visual.cpp
@@ -10,6 +10,42 @@ TEST_CASE("Visual") {
   auto n_ = notcurses_stdplane(nc_);
   REQUIRE(n_);
 
+  // check that we properly populate RGB + A -> RGBA
+  SUBCASE("VisualFromRGBPacked") {
+    unsigned char rgb[] = "\x88\x77\x66\x55\x44\x33\x22\x11\x00\x99\xaa\xbb";
+    unsigned char alpha = 0xff;
+    auto ncv = ncvisual_from_rgb_packed(rgb, 2, 6, 2, alpha);
+    REQUIRE(nullptr != ncv);
+    for(int y = 0 ; y < 2 ; ++y){
+      for(int x = 0 ; x < 2 ; ++x){
+        uint32_t p;
+        CHECK(0 == ncvisual_at_yx(ncv, y, x, &p));
+        CHECK(ncpixel_r(p) == rgb[y * 6 + x * 3]);
+        CHECK(ncpixel_g(p) == rgb[y * 6 + x * 3 + 1]);
+        CHECK(ncpixel_b(p) == rgb[y * 6 + x * 3 + 2]);
+        CHECK(ncpixel_a(p) == alpha);
+      }
+    }
+  }
+
+  // check that we properly populate RGBx + A -> RGBA
+  SUBCASE("VisualFromRGBxPacked") {
+    unsigned char rgb[] = "\x88\x77\x66\x12\x55\x44\x33\x10\x22\x11\x00\xdd\x99\xaa\xbb\xcc";
+    unsigned char alpha = 0xff;
+    auto ncv = ncvisual_from_rgb_loose(rgb, 2, 8, 2, alpha);
+    REQUIRE(nullptr != ncv);
+    for(int y = 0 ; y < 2 ; ++y){
+      for(int x = 0 ; x < 2 ; ++x){
+        uint32_t p;
+        CHECK(0 == ncvisual_at_yx(ncv, y, x, &p));
+        CHECK(ncpixel_r(p) == rgb[y * 8 + x * 4]);
+        CHECK(ncpixel_g(p) == rgb[y * 8 + x * 4 + 1]);
+        CHECK(ncpixel_b(p) == rgb[y * 8 + x * 4 + 2]);
+        CHECK(ncpixel_a(p) == alpha);
+      }
+    }
+  }
+
   // check that NCVISUAL_OPTION_HORALIGNED works in all three cases
   SUBCASE("VisualAligned") {
     const uint32_t pixels[4] = { htole(0xffff0000), htole(0xff00ff00), htole(0xff0000ff), htole(0xffffffff) };