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

Add "--position" argument? #146

Closed
Delgan opened this issue May 29, 2023 · 26 comments
Closed

Add "--position" argument? #146

Delgan opened this issue May 29, 2023 · 26 comments
Labels
compatibility Compatibility (e.g. terminal quirks) feature New feature or request
Milestone

Comments

@Delgan
Copy link
Contributor

Delgan commented May 29, 2023

Hi.

I'm in a situation where I'd like to display a Sixel image at some specific position of the terminal. It works fine if I move the cursor and then call the chafa command. However, in the case of animated images such as GIFs, this does not work well. The 1st frame is in the right position, but subsequent frames are displayed from the beginning of the line, and are therefore incorrectly positioned on the horizontal axis:

screenshot

I don't know how if we can fix it "by default" because as far as I know:

  • Chafa is not aware of the absolute cursor position when it's invoked
  • it's not possible to avoid the final carriage return while displaying a Sixel image

Consequently, after a frame has been displayed, there's no way to reposition the cursor at the origin of the image, either absolutely or relatively.

That leads me to the suggestion of adding a new --position argument with the idea of calling chafa_term_info_emit_cursor_to_pos() before displaying each frame.

Would you accept such PR, or do you have maybe another workaround in mind?

@Delgan
Copy link
Contributor Author

Delgan commented May 29, 2023

Hum... Actually I think a --margin-left argument is preferable.

@hpjansson
Copy link
Owner

There are potentially two ways this can be solved without adding a new argument:

  • Using DECSC and DECRC (ESC 7 and ESC 8) to save and restore the initial position. These are VT100+ and widely supported. However, it doesn't work if the first frame causes scrolling, so it'd be necessary to reserve the vertical space first, possibly by printing newlines.
  • Using relative positioning (cursor left + up) and avoiding newlines. The sixel inconsistency could be solved by assuming the cursor gets placed to the right of the final row, which appears to be the original DEC default. We could also issue an ESC [?8452h sequence first when --polite off is passed. We're already issuing DECSDM to enable sixel scrolling mode when impolite. We'd have to do a survey to see where sixel terminals are at now -- I think implementations have gotten more consistent over the past couple of years.

I think the second approach would be preferable, since we're already using relative cursor movement.

Would this work for your use case? I assume you're splitting symbol rows on \n and doing your own per-row positioning already? In any case, we already need new C API to let you get rows in a strv without any explicit carriage returns (see #141 (comment)).

Also see #140 with regards to positioning.

@hpjansson
Copy link
Owner

I'm also open to adding an argument to make it use cursor movement instead of \n between rows.

@Delgan
Copy link
Contributor Author

Delgan commented May 30, 2023

Thanks for your insight.

About my use case, actually I just want to display a Sixel image with a margin of one space on the left. I'm not even using the C API. Typing echo -n " " && chafa image.png works fine, all rows are vertically aligned. I'm not using symbols formatting (only Sixel), so I didn't need to solve this particular problem.

I wasn't aware of the ESC [?8452h sequence. This should work well and allow to use relative re-positioning. I'm only worried about terminal support and the --polite argument which looks quite cryptic to me, but it sounds like I would prefer not to turn it off. 😄

@Delgan
Copy link
Contributor Author

Delgan commented Jun 4, 2023

@hpjansson So I made tests regarding the ESC [?8452h sequence, support is pretty good although not all terminals handle it.

Layout is:

wezterm | konsole | mlterm
--------------------------
 xterm  |  zellij |  foot

Results:
sixel_bottom_right

Basically, among the six tested terminals, only konsole and zellij do not seem to support it.

What do you suggest to implement? Do you think that if polite=off, we should emit ESC [?8452h and use relative re-positioning?
This only applies to Sixel images, htough. Symbolic images can be handled in another ticket and in another way.

@hpjansson
Copy link
Owner

What do you suggest to implement? Do you think that if polite=off, we should emit ESC [?8452h and use relative re-positioning?

It could be a --relative <bool> switch, which defaults to off in Chafa 1.x and eventually on in 2.x (API break). If your terminal defaults to ESC [?8452h it would look fine with sixels, but if not, animations would look broken. Then you'd use --polite off to address that (or issue the control code yourself, or reconfigure your terminal).

Another question is which defaults terminals are using. I still have to think about this a little and run some tests (what happens when the image extends all the way to the right? Do terminals wrap the cursor to the next line like kitty does, or is it more common to defer logical position wrapping until a character is printed, as most terminals do with regular symbols?).

@hpjansson
Copy link
Owner

Looking at this now. May be easier to do than I first thought, without causing any regressions.

@hpjansson
Copy link
Owner

hpjansson commented Jun 11, 2023

@Delgan I added commits to master to implement --relative <bool> just now. It's the default, so you don't need to use any extra switches at all. Please test and let me know if it meets your requirements.

Changeset.

@Delgan
Copy link
Contributor Author

Delgan commented Jun 15, 2023

Hi @hpjansson, thanks for the update!

I made some tests using master branch of chafa (4bdffef2) and v1.14.0 of foot terminal.

Unfortunately, I noticed a regression even without the new --relative argument. When pressing Ctrl+C to stop infinite rendering of a GIF file, it can happen that the final frame is shifted to the right. I observed it with a terminal just opened (so no "impolite" mode before), simply with chafa --format=sixels chafa.gif (without left-padding).
20230615_19h03m10s_grim

Regarding the new --relative option, well, unfortunately I observed no difference compared to before. The echo -n " " && chafa --format=sixels --relative=on --polite=off chafa.gif command displays all but first frame without expected left-padding.

So I guess we'll have to find out whether it's a bug on the Foot or Chafa side?

@AnonymouX47
Copy link

Sorry to intrude...

Hum... Actually I think a --margin-left argument is preferable.

If I understand the problem correctly, I think the above solution might be less complex and would probably yield less surprises.

Basically, prepending every line of output with the given number of spaces or a CUF sequence with the given number of columns (the two are quite different though) should do the trick. Works fine with CR and CUU in use before.

I may not be seeing all the angles involved though, so I can't say for sure that this will be a viable solution for chafa.


When pressing Ctrl+C to stop infinite rendering of a GIF file, it can happen that the final frame is shifted to the right.

Most terminal emulators write ^C to the screen at the current cursor position when Ctrl-C is hit (with ECHO on)... hence the shift by 2 columns. The ^C would be obscured by the image in this case.

@hpjansson
Copy link
Owner

hpjansson commented Jun 17, 2023

Hi @hpjansson, thanks for the update!

I made some tests using master branch of chafa (4bdffef2) and v1.14.0 of foot terminal.

Unfortunately, I noticed a regression even without the new --relative argument. When pressing Ctrl+C to stop infinite rendering of a GIF file, it can happen that the final frame is shifted to the right. I observed it with a terminal just opened (so no "impolite" mode before), simply with chafa --format=sixels chafa.gif (without left-padding).

--relative on is the default, so you need to issue --relative off for the old behavior. I think we'd better invert it, though, so the old behavior becomes default again. That's because it'd be nice if symbol mode integrated well with less, and it gets confused when there's cursor movement instead of newlines.

If you issue --polite off, it will turn off echo and eliminate the ^C problem.

Anyway. There's the drawback of relative positioning; the image or animation will appear where the cursor was placed, but if you inject anything in the stream, it will perturb the positioning. We could work around it by save/restoring the cursor position, but we're already using the register to return to the beginning of the next relative line in symbols mode.

So for symbols we're currently doing this for each row:

  1. Save cursor position.
  2. Print row.
  3. Restore cursor position.
  4. Cursor down.

We could instead do:

  1. Print row.
  2. Cursor left to beginning of row.
  3. Cursor down.

Then we could store the position at the start of the animation and restore it before each frame, which would be more robust against injected text.

The only thing I'm not sure about is how fullwidth characters (occupying two columns) would impact this. When moving the cursor over fullwidth symbols, normally only one step is required to move over it instead of two. But when testing this in VTE, doing e.g. \e[20D seems to skip over exactly 20 cols regardless of fullwidth characters. So maybe it's fine -- except I don't know what other terminals do.

Regarding the new --relative option, well, unfortunately I observed no difference compared to before. The echo -n " " && chafa --format=sixels --relative=on --polite=off chafa.gif command displays all but first frame without expected left-padding.

That's odd. I don't have a working Wayland server at the moment, but I'll look into it once that's solved. If you have the time to investigate, you could cat the Chafa output to a file and edit the esc sequences to get a feel for what's going on. Maybe foot is leaving the cursor on column 1 after sixels, instead of right below the leftmost edge of the image?

So I guess we'll have to find out whether it's a bug on the Foot or Chafa side?

Yeah... It seems to work fine in mlterm, which has good sixel support. Best guess is foot leaves the cursor in the wrong position after sixels (or maybe mlterm has it wrong?).

@hpjansson
Copy link
Owner

Sorry to intrude...

No such thing -- it's nice to have your input :-)

Hum... Actually I think a --margin-left argument is preferable.

If I understand the problem correctly, I think the above solution might be less complex and would probably yield less surprises.

Basically, prepending every line of output with the given number of spaces or a CUF sequence with the given number of columns (the two are quite different though) should do the trick. Works fine with CR and CUU in use before.

I may not be seeing all the angles involved though, so I can't say for sure that this will be a viable solution for chafa.

That's definitely more robust. It'd be nice to have the cake and eat it too, though. What I've been thinking (and trying to implement) is two modes selectable with the --relative switch.

The first mode uses spaces and newlines for formatting (--relative off). It's more robust (and works with line-based tools like less), but will only work relative to the leftmost edge of the terminal, and will overwrite any existing text with spaces.

The second mode (--relative on) uses cursor movement and save/restore for all positioning, so if you for instance are making a shell program that draws a UI on the screen, you can issue a cursor position and then call Chafa to fill in a 2D space anywhere in the UI (for instance, inside a picture frame).

Regarding --margin-left, I think it could be useful to add separately, but it's orthogonal to how we do cursor movement. We already have --center, which will now use cursor movement or spaces depending on the --relative setting, and I think we could add general --align and --margin options to go with it, maybe with (crudely) similar semantics to CSS.

When pressing Ctrl+C to stop infinite rendering of a GIF file, it can happen that the final frame is shifted to the right.

Most terminal emulators write ^C to the screen at the current cursor position when Ctrl-C is hit (with ECHO on)... hence the shift by 2 columns. The ^C would be obscured by the image in this case.

--polite off works around it, but see above for a more cunning plan to address the changing offset.

@hpjansson
Copy link
Owner

Regarding the new --relative option, well, unfortunately I observed no difference compared to before. The echo -n " " && chafa --format=sixels --relative=on --polite=off chafa.gif command displays all but first frame without expected left-padding.

So I guess we'll have to find out whether it's a bug on the Foot or Chafa side?

I made a comparison, and it turns out that after \e[?8452h all the tested terminals agree. After \e[?8452l, xterm and foot agree with each other, while mlterm and wezterm agree with each other on the horizontal but not on the vertical position. The latter also seems to have an issue ordering the stream (spaces inserted after picture) unless I switch the spaces and control sequence around as shown.

I haven't been able to test with mintty, but its wiki says:

After output of a Sixel image in Sixel scrolling mode, or other image, the final cursor position can be next to the right bottom of the image, below the left bottom of the image (default), or at the line beginning below the image (like xterm).

So it sounds like it agrees with mlterm by default, but if you issue private mode 8452 (either enable or disable), you get xterm behavior.

sixels-mlterm-foot
sixels-xterm-wezterm

I'm inclined to side with xterm and foot, but I haven't been able to verify the behavior with other references or glass ttys.

@dnkl Do you know of any reference or testing that verifies that the cursor should advance to the beginning-of-line and not below the bottom-left corner of sixels?

@dnkl
Copy link
Contributor

dnkl commented Jun 19, 2023

I'd say most terminals are converging towards wezterms(?s) behavior; the cursor is positioned on the last row of the sixel, at the first sixel column. Or will converge, once XTerm's updated cursor placement logic has caught on...

This is xterm-382 (the weird looking prompt after the sixel is a color issue, not a placement issue):
xterm-sixel

There's a PR for foot (not quite functional yet): https://codeberg.org/dnkl/foot/pulls/1373

There's a code comment in latest xterm on this:

	/* NOTE: XTerm follows the VT382 behavior in text cursor placement. 
	 * The VT382's vertical position appears to be truncated (rounded
	 * toward zero) after converting to character row.  While rounding up
	 * is more often what is desired, so as to not overwrite the image,
	 * doing so automatically would cause text or graphics to scroll off
	 * the top of the screen.  Therefore, applications must add their own
	 * newline character, if desired, after a sixel image.
	 *
	 * FIXME: The VT340 also rounds down, but it seems to have a strange
	 * behavior where, on rare occasions, two newlines are required to
	 * advance beyond the end of the image.  This appears to be a firmware
	 * bug, but it should be added as an option for compatibility.
	 */

The most obvious advantage with this cursor placement is it allows you to print a sixel to the bottom row (without enabling private mode 8452). But does mean you have to emit an explicit newline to prevent e.g. the shell from overwriting the last row of the sixel.

@AnonymouX47
Copy link

Then we could store the position at the start of the animation and restore it before each frame, which would be more robust against injected text.

Yeah, this should overcome the issue of injected text... though, I guess you meant "restore it before each line of each frame".

The only thing I'm not sure about is how fullwidth characters (occupying two columns) would impact this. When moving the cursor over fullwidth symbols, normally only one step is required to move over it instead of two. But when testing this in VTE, doing e.g. \e[20D seems to skip over exactly 20 cols regardless of fullwidth characters. So maybe it's fine -- except I don't know what other terminals do.

Hmm... that's actually quite concerning. 🤔
Too make things worse, ECMA-48 Section 8.3.20 states:

CUB - CURSOR LEFT

Notation: (Pn)
Representation: CSI Pn 04/04

Parameter default value: Pn = 1

CUB causes the active presentation position to be moved leftwards in the presentation component by n
character positions if the character path is horizontal, or by n line positions if the character path is
vertical, where n equals the value of Pn.

Note the use of "character positions", not "columns" 🥲... though I don't think there was the concept of Unicode or fullwidth symbols back when it was written. Anyway, this indicates there are chances some terminal emulators implement horizontal cursor movement otherwise.

@dnkl
Copy link
Contributor

dnkl commented Jun 19, 2023

Do you know of any reference or testing

I believe the work done by @j4james and @hackerb9 is what have ultimately led to the changes in cursor positioning. See for example hackerb9/vt340test#25, and hackerb9/vt340test#11

@hpjansson
Copy link
Owner

Thanks, that's great information and some superb analysis there. The consensus -- leaving the cursor on the last row to avoid scrolling -- sounds like a good approach. I guess we'll keep using cursor save/restore (CSI s and CSI u) while reserving a parking row as implementations (hopefully) converge.

Oh, and let me know if I can help with testing and such.

@hpjansson
Copy link
Owner

hpjansson commented Jun 19, 2023

Yeah, this should overcome the issue of injected text... though, I guess you meant "restore it before each line of each frame".

If you mean "save, print row, restore, down, print row, restore, down down, etc", I think it might not be the best idea because the rate of growth in downwards movement would be superlinear. Instead I'd be willing to accept that an entire frame gets mangled if the subsequent frame corrects the offset. And you'd always mangle at least one row anyway...

Edit: Actually this would not be a big problem as we'd emit a single cursor movement sequence each time, and the number of digits in the offset wouldn't grow that fast :-)

Note the use of "character positions", not "columns" smiling_face_with_tear... though I don't think there was the concept of Unicode or fullwidth symbols back when it was written. Anyway, this indicates there are chances some terminal emulators implement horizontal cursor movement otherwise.

VTE does this:

$ echo -e 'a¥b¥c\e[DX'
a¥b¥X
$ echo -e 'a¥b¥c\e[D\e[DX'
a¥b Xc
$ echo -e 'a¥b¥c\e[D\e[D\e[DX'
a¥bX c

Which seems fine for our purposes. But when moving the cursor interactively in readline, it highlights the entire fullwidth character, and replaces both cells on overwrite (effectively deleting a column instead of replacing it with a space). So there's likely more going on under the hood in that case.

@hackerb9
Copy link

leaving the cursor on the last row to avoid scrolling -- sounds like a good approach

Yes. At the end of receiving sixel data, a VT340's text cursor is positioned on the last row of that data at the same column position as when the sixel data started. This is both the documented behavior and what tests on my hardware VT340 has shown.

@AnonymouX47
Copy link

Instead I'd be willing to accept that an entire frame gets mangled if the subsequent frame corrects the offset. And you'd always mangle at least one row anyway...

Edit: Actually this would not be a big problem as we'd emit a single cursor movement sequence each time, and the number of digits in the offset wouldn't grow that fast :-)

To be honest, I'm not sure I completely understand what you mean here. :-/

VTE does this:

$ echo -e 'a¥b¥c\e[DX'
a¥b¥X
$ echo -e 'a¥b¥c\e[D\e[DX'
a¥b Xc
$ echo -e 'a¥b¥c\e[D\e[D\e[DX'
a¥bX c

Which seems fine for our purposes. But when moving the cursor interactively in readline, it highlights the entire fullwidth character, and replaces both cells on overwrite (effectively deleting a column instead of replacing it with a space). So there's likely more going on under the hood in that case.

I see, that's good. I tested Konsole earlier and it was pretty much the same. It has the same replacement behaviour described above (even without readline, not sure if you meant with or without above).

Wezterm on the other hand, has a different replacement behaviour. It only overwrites the cell at the cursor position... though it has the same cursor highlight behaviour with readline as you described above.

Anyways, cursor movement was always column-by-column, not character-by-character in the few terminal emulators I tested, which is what matters in this use case IMO... maybe except fullwidth characters on a line would be overwritten by halfwidth characters when drawing animations.

@dnkl
Copy link
Contributor

dnkl commented Jun 20, 2023

Oh, and let me know if I can help with testing and such.

@hpjansson https://codeberg.org/dnkl/foot/pulls/1373 should now be ready for testing. I'll be doing more testing myself, but mostly focusing on ensuring I haven't introduced any crashes.

@hpjansson
Copy link
Owner

Great, I'll test it and follow up there.

@hpjansson
Copy link
Owner

Instead I'd be willing to accept that an entire frame gets mangled if the subsequent frame corrects the offset. And you'd always mangle at least one row anyway...
Edit: Actually this would not be a big problem as we'd emit a single cursor movement sequence each time, and the number of digits in the offset wouldn't grow that fast :-)

To be honest, I'm not sure I completely understand what you mean here. :-/

Just that if you're printing an ANSI art image, and you're returning the cursor to the top-left corner of the image before every row and then moving the cursor down to the row you're about to print, you'll have to move it down zero places for the first row, one for the second, two for the third, etc. So a 10-row image would have 0+1+2+3+4+5+6+7+8+9=45 cursor movement escape sequences if you're using index (\eD) to cause scrolling. But one can use CUD instead if scrolling isn't desired, and that takes a numeric argument, so you'd only need 9 of those for a 10-row image. And you could reserve space in advance, or use a combination of index and CUD.

So this was mostly me thinking out loud -- feel free to ignore.

I see, that's good. I tested Konsole earlier and it was pretty much the same. It has the same replacement behaviour described above (even without readline, not sure if you meant with or without above).

Wezterm on the other hand, has a different replacement behaviour. It only overwrites the cell at the cursor position... though it has the same cursor highlight behaviour with readline as you described above.

Anyways, cursor movement was always column-by-column, not character-by-character in the few terminal emulators I tested, which is what matters in this use case IMO... maybe except fullwidth characters on a line would be overwritten by halfwidth characters when drawing animations.

Ok, thanks for testing. Fortunately what happens with readline doesn't affect us, so I'll assume we're good on that front, while ambiguities in overwriting should only be a problem for whatever ends up being to the right of the ANSI art (if the terminal contracts the line by one column when overwriting fullwidth with halfwidth).

@AnonymouX47
Copy link

I realized this approach (restore per line) has a subtle flaw and thought I should point it out since I brought up the idea... It breaks if the image is taller than the terminal screen since it'll cause the terminal to scroll and reserving vertical space as you mentioned earlier can't help.

This doesn't affect your original proposition:

We could instead do:

  1. Print row.
  2. Cursor left to beginning of row.
  3. Cursor down.

Then we could store the position at the start of the animation and restore it before each frame, which would be more robust against injected text.

except, of course, it's an animation... for which, in my opinion, I wouldn't understand what the user's expectation was trying to draw an animation taller than the terminal screen.

@hpjansson
Copy link
Owner

hpjansson commented Jun 21, 2023

Yeah. I think I'd just file that under "don't do that then", or "I guess you're tee'ing it to a file to cat back out in a bigger window later. Maybe?"

@hpjansson hpjansson added feature New feature or request compatibility Compatibility (e.g. terminal quirks) labels Jul 14, 2023
@hpjansson hpjansson added this to the 1.14 milestone Aug 15, 2023
@hpjansson
Copy link
Owner

This discussion has run its course, and afaict everything that came up has been addressed, so I'm closing it.

About my use case, actually I just want to display a Sixel image with a margin of one space on the left. I'm not even using the C API. Typing echo -n " " && chafa image.png works fine, all rows are vertically aligned. I'm not using symbols formatting (only Sixel), so I didn't need to solve this particular problem.

--relative on should be solving this use case correctly now. Please open new issues if you run into any problems. Thanks, all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility Compatibility (e.g. terminal quirks) feature New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants