Repository: dankamongmen/notcurses (language: python)

Issue:
ncsixel_as_rgba fails when sixel image height is greater than 13
`ncsixel_as_rgba` fails when the height of the sixel image exceeds 13 pixels. as far as i can tell, the width is irrelevant -- i tested sixel images over 1000 pixels wide without issue.

the sixel images were mostly generated with `img2sixel`, but i also tested with random sixel images i found online, thinking maybe `img2sixel` was doing something outside of the sixel spec.

but it always fails when the height exceeds 13 pixels.

with `loglevel` set to `trace`, these are the relevant errors:

```
ncsixel_as_rgba:59:expected octothorpe, got 63
ncvisual_from_sixel:803:failed converting sixel to rgba
ncvisual_from_file failed
```

and here is an example sixel image that triggers the bug (22x14)

[alien14.txt](https://github.com/user-attachments/files/15784143/alien14.txt)

thanks!

---

* `export | egrep 'LANG|LC_CTYPE|TERM'`
```
declare -x LANG="en_US.UTF-8"
declare -x TERM="xterm-256color"
declare -x XTERM_LOCALE="en_US.UTF-8"
declare -x XTERM_SHELL="/usr/bin/bash"
declare -x XTERM_VERSION="XTerm(392)"
```
* notcurses version (available from `notcurses-demo i`)
`3.0.9`
* terminal name + version
`XTerm(392)`


ncsixel_as_rgba: incorrect state is set following '-'
oi, there seems to be another bug in `ncsixel_as_rgba` !

```
      }else if(*sx == '$'){
        x = 0;
        state = STATE_WANT_DATA;
      }else if(*sx == '-'){
        x = 0;
        y += 6;
        state = STATE_WANT_HASH;
      }else{
```

setting the state to `STATE_WANT_DATA` after the `-` command fixed it for me

or more concisely, since the state is already `STATE_WANT_DATA` at this point

```
      }else if(*sx == '$'){
        x = 0;
      }else if(*sx == '-'){
        x = 0;
        y += 6;
      }else{
```

the following sixel images will trigger the bug:

[laughing.txt](https://github.com/user-attachments/files/15930780/laughing.txt)
[red40.txt](https://github.com/user-attachments/files/15930781/red40.txt)

been using `notcurses` for a little while now -- really great project :)


Discussion / hints:
i think i found the issue:

```
}else if(*sx == '$'){
  x = 0;
  state = STATE_WANT_HASH;
```

the state should instead be set to `STATE_WANT_DATA` after the carriage return command. this fixed the issue for me, and in the case that a hash is encounter after this, `sx` pointer will be decremented and the state set to `STATE_WANT_HASH`, so i don't think there's any risk of this breaking anything
interesting; i have `src/tests/sixels.cpp` which ought be testing this. let me check it out...that's part of `notcurses-tester` and ought be running hrmm.
test `SixelRoundTrip` for instance works with `worldmap.png`, which is definitely more than 13 rows. let me look at your comment+example more closely...
ok, the only test i have of `ncvisual_from_sixel()` etc. is `LoadSixel`, which just uses

`"\x1bP0;1;0q\"1;1;1;6#0;2;66;18;80#0@-\x1b\\"`

let's see what happens if i plug your file in there....
looks confirmed, new test `LoadBigSixel` fails. good find, thanks @waveplate !

```c

  SUBCASE("LoadBigSixel") {
    auto ncv = ncvisual_from_sixel("\x1bPq\"1;1;22;14#0;2;9;13;13#1;2;9;16;19#2;2;16;22;25#3;2;53;56;56#4;2;66;63;60#5;2;72;75;69#6;2;69;72;66#7;2;60;63;63#8;2;35;41;44#9;2;28;35;35#10;2;25;28;28#11;2;19;22;25#12;2;0;0;0#13;2;16;19;22#14;2;31;38;38#15;2;63;66;66#16;2;19;25;28#17;2;3;3;6#18;2;47;50;50#19;2;69;72;72#20;2;25;31;31#21;2;6;9;13#22;2;47;47;47#23;2;6;13;19#24;2;47;53;50#25;2;19;19;19#26;2;6;6;13#27;2;44;41;38#28;2;56;53;53#29;2;50;47;47#30;2;19;16;16#31;2;47;50;47#32;2;63;60;56#33;2;47;47;44#34;2;25;25;28#35;2;35;31;31#36;2;53;50;50#37;2;6;3;6#1eLB#14A!7?_K???GO#12@^~~$#0H#13qK#18C???_!6?_#11BDC#0G#26_$#23O#20?_!9?_OC#2?A#13AO$#16??O#3G@??O_?U#22G???_O#25__$#7???O!6?H#8V?_WG#1?@#17A$#4???__@#15o?]M#24_!4?O#21??C$#2???@#5[[FB?o#9??RN#10@C_G$#6!4?Aa??@@#16!4?A$#19!6?GK-#0KO!4?G!6?oG?`X???C$#1@B#10_??_?G???GwCA!4?C#12_`$#21o#13K???G#30oo!5?GCA??F`#1K$#23A#17_#22O?CC??@??A?A#17_G!4?PQ$#27??@!4?CgG#3A#20?@!5?_O$#28??KH??@A?_#8??A@@#11DC_#25GA$#31??A#4_?A#35C?O??o#37??OO#21W???AG$#32???U#14O!6?@C!6?G$#9!4?G!6?C#26???_#34AAO$#29!4?`@#33A?CCG#13!6?C$#5!4?A#25O#24?@#18A?D$#36!9?Oo$#7!9?B-#0B?@#8@A#36@#27@!4?A#10A??AA@#17A@$?BA#1A#32@A??@AA#35@#34@#11BB#13@@A@#12ABB$#28!6?AA?@@$#29!7?@#4A\x1b\\", 1, 1);
    REQUIRE(ncv);
    uint32_t p;
    ncvisual_at_yx(ncv, 0, 0, &p);
    /*CHECK(0xff == ncpixel_a(p));
    CHECK(0xa8 == ncpixel_r(p));
    CHECK(0x2d == ncpixel_g(p));
    CHECK(0xcc == ncpixel_b(p));*/
    ncvisual_destroy(ncv);
  }
```
yeah your fix seems reasonable; thanks!
hrmmm, even with that change, we're running into two possible problems...with dimensions of `0, 0` we get:

```c
ncsixel_as_rgba:164:too many rows 0 + 6 > 0
ncvisual_from_sixel:805:failed converting sixel to rgba
```

and with `1, 1` we get:

```c
ncsixel_as_rgba:168:invalid rle 1 + 1 > 1
ncvisual_from_sixel:805:failed converting sixel to rgba
```

with `100, 100` things work. let me investigate this...
i get these same errors with these dimensions using the old code, but now `100, 100` also breaks, with:

```c
ncsixel_as_rgba:59:expected octothorpe, got 63

ncvisual_from_sixel:805:failed converting sixel to rgba
```
ok, `ncsixel_as_rgba()` clearly oughtn't be accepting 0 as either argument, at least as currently written:


```c
  uint32_t* rgba = (uint32_t*)calloc(leny * lenx, sizeof(*rgba));                                                                   
  if(rgba == NULL){                                                                                                                 
    return NULL;                                                                                                                    
  }        
```
i've added code to reject these invalid geometries
this particular sixel appears to require at least `18, 22` geometry
Things ought now work for you, @waveplate ; thanks for the report and fix! I've added a unit test for this case.
good find! as you can see, this code has not been heavily used nor properly tested =].

i'm glad to hear you're liking the project. i'm hoping to do a release very soon that will wrap up these fixes.
```c
process_escape:2219:walk result on 99 (c): 1 386
handoff_initial_responses_late:1198:handing off initial responses
block_on_input:2457:blocking on input availability
block_on_input:2514:waiting on 1 fds (ibuf: 0/8192)
ncplane_new_internal:692:created new 64x132 plane "std" @ 0x0
ncsixel_as_rgba:172:invalid rle 1 + 1 > 1
ncvisual_from_sixel:809:failed converting sixel to rgba
===============================================================================
/home/dank/src/dankamongmen/notcurses/src/tests/sixels.cpp:17:
TEST CASE:  Sixels
  LoadBiggerSixel

/home/dank/src/dankamongmen/notcurses/src/tests/sixels.cpp:46: FATAL ERROR: REQUIRE( ncv ) is NOT correct!
  values: REQUIRE( nullptr )

===============================================================================
[doctest] test cases:  1 |  0 passed | 1 failed | 45 skipped
[doctest] assertions: 18 | 17 passed | 1 failed |
[doctest] Status: FAILURE!
[schwarzgerat](1) $ 
```
i'm reproducing this in unit tests now.

Fix patch:
diff --git a/src/lib/sixel.h b/src/lib/sixel.h
index b95a18379..67f13e588 100644
--- a/src/lib/sixel.h
+++ b/src/lib/sixel.h
@@ -161,7 +161,7 @@ uint32_t* ncsixel_as_rgba(const char *sx, unsigned leny, unsigned lenx){
       }else if(*sx == '-'){
         x = 0;
         y += 6;
-        state = STATE_WANT_HASH;
+        state = STATE_WANT_DATA;
       }else{
 //fprintf(stderr, "RLE: %d pos: %d x %d\n", rle, y, x);
         if(y + 6 > (leny + 5) / 6 * 6){


Test patch:
diff --git a/src/tests/sixels.cpp b/src/tests/sixels.cpp
index 90760d4b1..a6f09510e 100644
--- a/src/tests/sixels.cpp
+++ b/src/tests/sixels.cpp
@@ -35,12 +35,20 @@ TEST_CASE("Sixels") {
     ncvisual_destroy(ncv);
   }
 
+  // checks https://github.com/dankamongmen/notcurses/issues/2784
   SUBCASE("LoadBigSixel") {
     auto ncv = ncvisual_from_sixel("\x1bPq\"1;1;22;14#0;2;9;13;13#1;2;9;16;19#2;2;16;22;25#3;2;53;56;56#4;2;66;63;60#5;2;72;75;69#6;2;69;72;66#7;2;60;63;63#8;2;35;41;44#9;2;28;35;35#10;2;25;28;28#11;2;19;22;25#12;2;0;0;0#13;2;16;19;22#14;2;31;38;38#15;2;63;66;66#16;2;19;25;28#17;2;3;3;6#18;2;47;50;50#19;2;69;72;72#20;2;25;31;31#21;2;6;9;13#22;2;47;47;47#23;2;6;13;19#24;2;47;53;50#25;2;19;19;19#26;2;6;6;13#27;2;44;41;38#28;2;56;53;53#29;2;50;47;47#30;2;19;16;16#31;2;47;50;47#32;2;63;60;56#33;2;47;47;44#34;2;25;25;28#35;2;35;31;31#36;2;53;50;50#37;2;6;3;6#1eLB#14A!7?_K???GO#12@^~~$#0H#13qK#18C???_!6?_#11BDC#0G#26_$#23O#20?_!9?_OC#2?A#13AO$#16??O#3G@??O_?U#22G???_O#25__$#7???O!6?H#8V?_WG#1?@#17A$#4???__@#15o?]M#24_!4?O#21??C$#2???@#5[[FB?o#9??RN#10@C_G$#6!4?Aa??@@#16!4?A$#19!6?GK-#0KO!4?G!6?oG?`X???C$#1@B#10_??_?G???GwCA!4?C#12_`$#21o#13K???G#30oo!5?GCA??F`#1K$#23A#17_#22O?CC??@??A?A#17_G!4?PQ$#27??@!4?CgG#3A#20?@!5?_O$#28??KH??@A?_#8??A@@#11DC_#25GA$#31??A#4_?A#35C?O??o#37??OO#21W???AG$#32???U#14O!6?@C!6?G$#9!4?G!6?C#26???_#34AAO$#29!4?`@#33A?CCG#13!6?C$#5!4?A#25O#24?@#18A?D$#36!9?Oo$#7!9?B-#0B?@#8@A#36@#27@!4?A#10A??AA@#17A@$?BA#1A#32@A??@AA#35@#34@#11BB#13@@A@#12ABB$#28!6?AA?@@$#29!7?@#4A\x1b\\", 18, 22);
     REQUIRE(ncv);
     ncvisual_destroy(ncv);
   }
 
+  // checks https://github.com/dankamongmen/notcurses/issues/2787
+  SUBCASE("LoadBiggerSixel") {
+    auto ncv = ncvisual_from_sixel("\x1bPq\"1;1;18;18#0;2;0;0;0#1;2;6;6;0#2;2;35;28;9#3;2;69;56;19#4;2;91;72;28#5;2;97;78;28#6;2;91;72;25#7;2;94;75;28#8;2;72;53;16#9;2;56;41;6#10;2;78;63;19#11;2;60;44;9#12;2;85;66;22#13;2;53;44;22#14;2;60;72;75#15;2;75;63;31#16;2;66;56;22#17;2;50;50;44#18;2;25;35;35#19;2;88;85;82#20;2;19;35;50#21;2;38;47;44#22;2;47;38;16#23;2;13;22;31#24;2;47;47;31#25;2;41;25;0#26;2;0;3;3#27;2;53;38;6#1O???@!8?@???O$#2_GCA?@!6?@?ACG_$#0NFB@#8O!8?O#0@BFN$#3?O??A?@!4?@?A??O$#5?_OGKEm}~~}mEKGO_$#7??_O?G!6?G?O_$#6??GC#9?O!6?O#6?CG$#11???__!8?__$#12!5?_!6?_$#10!6?O!4?O$#4!7?@??@-Q??C???GSSG???C??Q$#5KLC??HH@JJ@HH??CLK$#3@#7QG?@!8?@?GQ#3@$#16_#14_oww!8?wwo_#16_$#11??A@?AA!4?AA?@A$#8??@#6A???O??O???A#8@$#12!4?A?OA??AO?A$#13!4?C?!6_?C$#9!5?CCC??CCC$#15!5?O!6?O$#17!5?_!6?_-#0oo__#1_!8?_#0__oo$#26G?O#2O?_!6?_?O#26O?G$#20AG#3??O?_!4?_?O#20??GA$#18@#14FNNN@!6?@NNNF#18@$#23C#4!4?G?_OO_?G#23!4?C$#5!5?OOO__OOO$#21!5?A!6?A$#24!5?C!6?C$#8!6?G!4?G$#19!6?!6@$#22!6?A!4?A$#25!6?!6C$#9!7?G??G$#17!7?A??A$#15!8?AA$#27!8?GG\x1b\\", 18, 22);
+    REQUIRE(ncv);
+    ncvisual_destroy(ncv);
+  }
+
   // remaining tests can only run with a Sixel backend
   if(notcurses_check_pixel_support(nc_) <= 0){
     CHECK(0 == notcurses_stop(nc_));