Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix incorrect state reset after carriage return command #2785

Closed
wants to merge 1 commit into from

Conversation

waveplate
Copy link

@waveplate waveplate commented Jun 11, 2024

i ran into an issue with ncsixel_as_rgba where it always failed with sixel images exceeding 13 pixels in height

the cause of the issue seems to be

}else if(*sx == '$'){
  x = 0;
  state = STATE_WANT_HASH;

this prevents sixel images from being correctly parsed -- i haven't investigated what the sixel standard says about this, but img2sixel outputs don't follow carriage returns with a hash, just more data

changing STATE_WANT_HASH to STATE_WANT_DATA resolved the issue

in any case, i don't think this change is likely to break anything:

if(state == STATE_WANT_DATA){
  if(*sx == '#'){
    state = STATE_WANT_HASH;
    --sx;

as if a hash is encountered, it will just decrement the pointer and set the state to STATE_WANT_HASH

@dankamongmen
Copy link
Owner

ugh, sorry, i went and handled this when i saw your report in #2784. i thanked you in the commit and in the bug, but i preempted your PR =\ my bad =[

@dankamongmen dankamongmen self-requested a review June 11, 2024 13:07
@@ -153,7 +153,7 @@ uint32_t* ncsixel_as_rgba(const char *sx, unsigned leny, unsigned lenx){
--sx;
}else if(*sx == '$'){
x = 0;
state = STATE_WANT_HASH;
state = STATE_WANT_DATA;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good fix, already committed this morning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants