hackerb9 / lsix

Like "ls", but for images. Shows thumbnails in terminal using sixel graphics.
GNU General Public License v3.0
3.97k stars 129 forks source link

DECSDM (private mode 80) is interpreted incorrectly #41

Closed j4james closed 2 years ago

j4james commented 3 years ago

I'm fairly certain that you've got the DECSDM mode backwards. I think it is XTerm that has implemented it incorrectly, and MLterm that has it the right way around. CSI ?80l should enable sixel scrolling, and CSI ?80h should disable it.

Other terminals that match MLterm's behaviour include RLogin (you can see the source here), Tanasinn (see here), and WRQ Reflection (it's a commercial app, but you can see the documentation describing the behaviour of DECSDM here).

I suspect the misunderstanding came about because the VT330/340 manual documented DECSDM the way XTerm has currently implemented it, but I believe that was a mistake. The various VT382 manuals have it the other way around. See here, and here.

The author of Tera Term, who has a collection of DEC terminals, actually tested this on a VT330 and VT382 and reported the results here: https://twitter.com/ttdoda/status/479053314412126208

According to Google translate:

The actual machine and manual of VT382 and the actual machine of VT330 are DECSDM reset and sixel scrolling mode is enabled.

Which I understand to mean that sixel scrolling mode is enabled when DECSDM is reset.

Now I guess it's possible that everyone else is wrong about this, and XTerm is right, but all the evidence appears to point in the other direction.

hackerb9 commented 3 years ago

Thank you for gathering all that evidence. How do you explain the "mistake" in the DEC documentation? It's not a simple transposition as it discusses both behaviors.

j4james commented 3 years ago

Reading the VT3xx manual here, my thought was that the documentation writer probably got confused between the concept of the "Sixel Scrolling" feature in the Graphics Set-Up screen, and "Sixel Display Mode", which are really the opposite of each other.

When you enable "Sixel Display Mode", you're requesting Sixel images be displayed full-screen, starting in the top left corner. You're not asking to enable scrolling. If DECSDM was intended to enable scrolling you would think they would have called it something like "Sixel Scrolling Mode".

Anyway, I don't want to claim that I'm absolutely certain about this, but it just seems very likely, given the evidence, that XTerm got it wrong.

j4james commented 3 years ago

The other thing I meant to ask, was do you really need to set this mode at all? You'll probably have tested this on more terminals than I have, but all the ones I've tried have scrolling enabled by default, regardless of how they interpret DECSDM. So I would have thought the safest thing to do was just to leave the setting alone.

hackerb9 commented 3 years ago

Please share with me the list of terminals you've seen that have scrolling enabled by default. It makes sense that lsix should default to whatever people most commonly use, whether it is "right" or "wrong". I believe the main reason I added the escape sequence to enable scrolling in the first place was because some terminals (mlterm?) had it off by default. If that's no longer an issue, then removing the control sequence may be the best option.

I see how you could think the DEC VT3xx manual is in error. I have been working off of a more thorough reference from DEC, EL-SM070-00 DEC STD 070 Video Systems Reference Manual Dec91 .

j4james commented 3 years ago

Please share with me the list of terminals you've seen that have scrolling enabled by default.

XTerm (360), MLterm (3.8.4), Contour (0.1.1), WezTerm (20210502), Mintty (3.5.0), RLogin (2.25.3), and Reflection Desktop (16.2.0).

I added the escape sequence to enable scrolling in the first place was because some terminals (mlterm?) had it off by default.

I figured that might have been the case, but hopefully whoever that was will have fixed the issue by now.

I have been working off of a more thorough reference from DEC, EL-SM070-00 DEC STD 070 Video Systems Reference Manual Dec91 .

Yeah I've got the STD 070 manual, but that has no mention of DECSDM mode as far as I'm aware. It just points out that the VT125 and VT240 don't support scrolling, but that doesn't really help us. Or is there something in there that I've missed?

hackerb9 commented 3 years ago

Thank you for that research. I think you're right that if all modern terminals start with scrolling on, we might be able to skip the sequence. [Reminder to self: if we do this, check TERM and send it for those terminals that start with scrolling off (vt125?).]

However, there is no way I know of to poll the current setting, so lsix cannot know what state the last sixel application has left the terminal in. Maybe this is worrying about nothing, but I notice other projects, like notcurses seem to think it is important to always enable sixel scrolling.

hackerb9 commented 3 years ago

By the way, as I reread the specs, I became less sure I read them correctly. I'd like to know what the actual hardware behaviour is, so I have purchased a VT340+. Hopefully we'll get this resolved in two weeks when it arrives.

j4james commented 3 years ago

I'd like to know what the actual hardware behaviour is, so I have purchased a VT340+. Hopefully we'll get this resolved in two weeks when it arrives.

This is brilliant news! Once you've got it set up, do you think you might be interested in running some other Sixel compatibility tests too? I've been putting together a small set of tests covering edge cases in the spec that I was hoping to be able to run on a VT340 one day. The idea being to create a repository of examples with expected output which terminal developers could refer to if they want to be VT340 compatible.

Each test is just a small text file that can be output to the terminal, and the result then photographed as a reference. For example, this is my current DECSDM test case:

sixel_display_mode.txt

And this is what I would expect as output (assuming I'm correct in my understanding of the VT340 behaviour).

Click to view ![image](https://user-images.githubusercontent.com/4181424/122402391-56d73600-cf75-11eb-93d9-172aad1f808a.png)
j4james commented 3 years ago

I notice other projects, like notcurses seem to think it is important to always enable sixel scrolling.

Yeah, I've been slowing raising issues with other apps that I've seen doing this. So far I've yet to find anyone that had a legitimate reason for it. I haven't got around to notcurses yet because I wanted to be able to actually test their code first, and I couldn't get it to build. I should probably just raise the issue anyway.

check TERM and send it for those terminals that start with scrolling off (vt125?)

The VT125 (and the VT240) don't support Sixel scrolling at all. They're effectively always in "display mode", so the image will always start in the top left corner of the screen, and will be clipped when it reaches the bottom.

wez commented 3 years ago

@dankamongmen seems like the kind of person that would happily join this discussion even without a formal issue being filed :) I'm no expert on this, and I'm just idly perusing this issue from the one that links here in wezterm, but this is the relevant section from notcurses:

https://github.com/dankamongmen/notcurses/blob/fe61082c5cc3439083ea80fad19c0811cfd2683b/src/lib/sixel.c#L863-L872

dankamongmen commented 3 years ago

i'm familiar with my valued comrade @j4james 's DECSDM march:

https://github.com/dankamongmen/notcurses/issues/1782

i think he has the right idea, but i think in practice it would turn out flawed, as the (over)eager merging of my PR to mlterm kinda supports IMHO:

https://github.com/arakiken/mlterm/pull/23#

in the end, pretty much no one got everything they wanted, and all parties seemed a bit less happy than they were before. i apologize for my role in this expansion of sadness, but have high hopes for a brighter tomorrow.

hackerb9 commented 3 years ago

Yes, I was thinking of asking @dankamongmen his opinion about omitting the scroll initialization. I have a guess, though, given his frequently expletive laden comments about other software packages. 😏 Like XTerm author @ThomasDickey, he clearly cares about good engineering.

If @j4james is correct, his suggestion would work as long as no other application had changed the default setting. lsix might be able to get away with it as simplicity is one of its design goals. My guess is Nick Black would reject it out of hand since notcurses has to be more robust than that.

Given that different terminals (XTerm and mlterm) interpret the same sequence in opposite ways, a reasonable solution might be a TERMCAP entry for "enable sixel scrolling". But I would want to get @ThomasDickey's opinion first.

dankamongmen commented 3 years ago

Given that different terminals (XTerm and mlterm) interpret the same sequence in opposite ways, a reasonable solution might be a TERMCAP entry for "enable sixel scrolling". But I would want to get @ThomasDickey's opinion first.

a terminfo entry definitely would be the preferred way to get this information (aside from issues with TERM possibly being incorrect, le sigh).

Yes, I was thinking of asking @dankamongmen his opinion about omitting the scroll initialization. I have a guess, though, given his frequently expletive laden comments about other software packages

=D i'm not sure i've ever spoken with you, @hackerb9 , but i dig your style

ThomasDickey commented 3 years ago

I can wait til you've investigated the VT340. The existing documentation is too sketchy to say whether one was wrong or whether DEC happened to implement the feature differently: The PDF for the first editing of the VT330/VT340 graphics manual doesn't mention the feature, and the VT382 manual is undated.

j4james commented 3 years ago

the VT382 manual is undated.

For the record, the Thai VT382 user guide is dated August 1989, and the second edition VT330/340 programmer reference is from May 1988.

hackerb9 commented 3 years ago

Preliminary tests with an actual VT340+ vindicate @j4james and mlterm. I will likely remove the sixel scrolling sequence from lsix and just hope that no application ever disables it.

dankamongmen commented 3 years ago

appreciate you getting a firm answer!

dankamongmen commented 3 years ago

so now the question becomes: is a great effort to be made in reforming other terminals? i know many of them were "copied from XTerm in the way of lemmings" in @j4james 's approximate words, but i doubt they're so broadly actively pulling in fixes.

either way, what i would like to see is a new terminfo capability covering this. i would then be inclined to (in Notcurses) assume the inverse meaning for terminals lacking the entry in their terminfo databases (since that seems to be the most widespread interpretation currently), possibly special-casing the {small} number of terminals known to use the faithful interpretation. where the terminfo capability was defined, i'd of course use that.

@ThomasDickey, especially if you end up changing XTerm behavior to reflect @hackerb9 's confirmation of @j4james 's , I'd very much appreciate a terminfo entry abstracting away DECSDM. thoughts?

dankamongmen commented 3 years ago

either way, what i would like to see is a new terminfo capability covering this. i would then be inclined to (in Notcurses) assume the inverse meaning for terminals lacking the entry in their terminfo databases (since that seems to be the most widespread interpretation currently), possibly special-casing the {small} number of terminals known to use the faithful interpretation. where the terminfo capability was defined, i'd of course use that. @ThomasDickey, especially if you end up changing XTerm behavior to reflect @hackerb9 's confirmation of @j4james 's , I'd very much appreciate a terminfo entry abstracting away DECSDM. thoughts?

for what it's worth, i already have ghastly XTVERSION-based heuristics to invert the commonly-understood sense for MLterm, but only MLterm versions less than 3.9.1 (when the sense was inverted, and XTVERSION was added to MLterm). i'd really prefer not letting this kind of thing proliferate further than needed.

ThomasDickey commented 3 years ago

If you have XTVERSION working reliably, that might be the best approach to dealing with this feature. Making this a terminfo capability doesn't guarantee that the terminal description is up-to-date.

dankamongmen commented 3 years ago

If you have XTVERSION working reliably, that might be the best approach to dealing with this feature. Making this a terminfo capability doesn't guarantee that the terminal description is up-to-date.

well, sure, but (especially given the widespread differences in implementation) it means i (and you, as NCURSES maintainer, should NCURSES ever use this capability) have to effectively classify, record, and track every terminal in existence =].

if we can't have it in terminfo base -- iirc, you've stated that terminfo will "only hold those capabilities needed/used by NCURSES" (apologies if i'm misquoting) -- i will move to introduce it as a terminfo extension capability. it seems better for all involved if we can have it as a base capability, but i am of course ignorant of the costs involved in maintaining terminfo.

i try to use XTVERSION as little as possible, relying on terminfo (in almost all instances) except where capabilities aren't expressed therein. i'd like to continue doing so; terminfo is very valuable.

dankamongmen commented 3 years ago

well, sure, but (especially given the widespread differences in implementation) it means i (and you, as NCURSES maintainer, should NCURSES ever use this capability) have to effectively classify, record, and track every terminal in existence =].

and there are of course many (most?) terminals which don't support a meaningful XTVERSION. the author of Alacritty has for instance refused a patch to add it for philosophical reasons. use of XTVERSION is far inferior to terminfo when terminfo is available.

ThomasDickey commented 3 years ago

Alacritty (reading the source code...) identifies itself as a VT102 (which makes it off-topic in this issue). Given that, you needn't worry about any features which are neither in a standard VT102 nor in the terminal description.

hackerb9 commented 3 years ago

[...] given the widespread differences in implementation) it means i (and you, as NCURSES maintainer, should NCURSES ever use this capability) have to effectively classify, record, and track every terminal in existence =].

if we can't have it in terminfo base -- iirc, you've stated that terminfo will "only hold those capabilities needed/used by NCURSES" (apologies if i'm misquoting) -- i will move to introduce it as a terminfo extension capability. it seems better for all involved if we can have it as a base capability, but i am of course ignorant of the costs involved in maintaining terminfo.

I'm pretty sure that quote about terminfo being limited to only what ncurses uses is out of context since IIRC the manpage references capabilities that ncurses does not use.

I hope @ThomasDickey corrects me if I'm wrong, but I believe he has taken the Noah Webster approach to maintaining terminfo: description, not prescription. He doesn't add capabilities because they are potentially useful, he adds them because they are actually used. While he has good insights, since he is an engineer and a developer, it would be up to us to coin a new capability and start writing code that uses it.

As for Augean task of adding the behaviour of every sixel capable terminal to terminfo, I do not think that is part of the terminfo maintainer's job. If people care about a terminal, they will submit a patch for it. Terminals that people don't use don't need to get updated. Terminfo has never been "complete", it just looks that way because it's been accreting like a coral reef for eons.

dankamongmen commented 3 years ago

I'm pretty sure that quote about terminfo being limited to only what ncurses uses is out of context since IIRC the manpage references capabilities that ncurses does not use.

i certainly meant at no time to impugn Mr. Dickey, whose torch we merely carry, and of whom i am a great admirer.

i cannot find this quote, and thus i cheerfully withdraw it. in any case, it was not meant to be a judgment of Mr. Dickey's maintainership, but a recognition of his prerogative.

i would be happy to collect this information and prepare it; i did not mean to imply that any other free software volunteers ought do the work. i did not, however, want to do this if there was no chance of having the compiled data entered into terminfo.

i apologize if any of my communication was unclear. i have no place nor desire to dictate what ought be done with terminfo. i simply think this would be useful to all terminfo clients.

dankamongmen commented 3 years ago

i would be happy to collect this information and prepare it; i did not mean to imply that any other free software volunteers ought do the work. i did not, however, want to do this if there was no chance of having the compiled data entered into terminfo.

looking back over the exchange, i think this was insufficiently surfaced. @ThomasDickey , by no means did i mean that you ought go collect this information for my benefit =] i would be more than happy to perform the investigation if the data will be admitted to terminfo (i've submitted patches to terminfo--and xterm, and ncurses--before, so i hope i have some small cachet with the Grandmaster from the Invisible Island),

j4james commented 3 years ago

Just FYI, as soon as XTerm has shipped a build with DECSDM fixed, I intend to raise bug reports with the other terminals that are doing this incorrectly, or at least those that I think might be more interested in matching XTerm than they would be in preserving a broken behaviour that nobody really needs.

It still baffles me that you think the solution to this problem requires a complex rigmarole of terminal version detection and terminfo queries, when the whole issue could be avoided simply by not sending the mode in the first place. You're focusing on the hypothetical problem of somebody possibly changing the mode from the default (which they could easily fix themselves), and ignoring the very real problem of your code just not working on multiple terminals.

ThomasDickey commented 3 years ago

agreed: adding a feature to TERM=xterm (or TERM=xterm-256color) doesn't mean that it corresponds to the running xterm (or any of those where the developers don't care much about having a correct terminal description).

hackerb9 commented 3 years ago

Just FYI, as soon as XTerm has shipped a build with DECSDM fixed, I intend to raise bug reports with the other terminals that are doing this incorrectly, or at least those that I think might be more interested in matching XTerm than they would be in preserving a broken behaviour that nobody really needs.

Are you saying there were no physical terminals that used DECSDM in the (backwards) way DEC documented for the VT340?

If you think nobody needs this behavior, why bother asking terminal emulators to interpret it correctly?

(Note: I'd like to see them interpret it correctly because I think DECSDM can be useful. For example, the sixel screen-dumps from the VT340 always end with a graphic newline that would scroll the top of the image off the screen. It's handy to disable scrolling before displaying such images.)

You're focusing on the hypothetical problem of somebody possibly changing the mode from the default (which they could easily fix themselves)

I get what you're saying, and it will be removed from the next release of lsix, but it is a bit harsh to say it is a hypothetical problem. One very real example: As you may recall, I added that escape sequence to lsix because some terminals were defaulting to not scrolling. Now that same sequence may actually cause the problem I was trying to fix. I have another tool (sixgif, never released) which does the opposite, using the no-scroll mode. A person using either tool would not necessarily know how to fix scrolling problems in other sixel programs they run later.

and ignoring the very real problem of your code just not working on multiple terminals.

Your proposal to not use that sequence (and hope no one else does) is good enough for lsix, but I have no illusions that it is a proper solution. I am coming to the conclusion that the "correct answer" is to define a way to query the current DECSDM state so it can be restored when a program exits instead of having lingering effects. That way, for example, lsix might have scrolling issues, but it wouldn't affect anything run afterward. Of course, the hard part is convincing all the terminal emulator developers to implement it. If that were to happen, there would be real benefit in asking them to also swap how they handle DECSDM.

j4james commented 3 years ago

Are you saying there were no physical terminals that used DECSDM in the (backwards) way DEC documented for the VT340?

To the best of my knowledge, yes. The VT125 and VT240 didn't support scrolling at all. And both the VT340 and VT382 work as documented in the VT382 manuals (as originally pointed out by @ttdoda and confirmed by you). I don't know of any other hardware terminals that natively support sixel images.

If you think nobody needs this behavior, why bother asking terminal emulators to interpret it correctly?

Sorry, that wasn't very clear. I'm not saying nobody needs DECSDM - I'm saying nobody needs a broken DECSDM. I want to get more sixel terminals implementing things interoperably so people can actually use DECSDM - correctly - if they need it.

There are a few more things that need to be fixed to really take advantage of it though - like most terminals don't handle background select correctly, and clipping is often broken - but getting DECSDM working is a step in the right direction. I really thought this would be the easy part.

One very real example: As you may recall, I added that escape sequence to lsix because some terminals were defaulting to not scrolling.

I'm sorry my message above came across as harsh. I completely understand why you may have chosen to do that initially, but there's no evidence that that problem still exists - certainly not to the point that it outweighs all the terminals that you now know are broken by enabling DECSDM.

A person using either tool would not necessarily know how to fix scrolling problems in other sixel programs they run later.

I know it's not ideal, but this can happen with any number of modes, and users can always ask for help and get the problem solved. As things stand now, though, there is absolutely nothing they can do if they're trying to use an app like lsix or notcurses and it doesn't work on their terminal (well lsix I suppose could reasonably be patched by a user, but that's really not feasible for notcurses).

the "correct answer" is to define a way to query the current DECSDM state so it can be restored when a program exits

There is already a well-defined way to do that - the DECRQM query. Not everyone supports it, but that's a better starting point than trying to invent a new sequence.

But as you say, lsix is still going to have scrolling issues if you aren't setting the mode correctly in the first place - whatever you want it to be. If you have a genuine use case for setting DECSDM, I think the only practical solution in the long term is to encourage all terminals to implement it correctly, so you can reliably use it correctly.

hackerb9 commented 3 years ago

the "correct answer" is to define a way to query the current DECSDM state so it can be restored when a program exits

There is already a well-defined way to do that - the DECRQM query. Not everyone supports it, but that's a better starting point than trying to invent a new sequence.

That is precisely what I was thinking. If I recall correctly, the VT340 manual does not list DECSDM as queryable, which may explain why terminal emulators haven't implemented it yet. Here's a little tool you can use to check if DECRQM works for detecting DECSDM: checksixelmode.sh. I'll test my VT340 hardware when I get home.

j4james commented 3 years ago

If I recall correctly, the VT340 manual does not list DECSDM as queryable, which may explain why terminal emulators haven't implemented it yet.

I actually meant that they don't implement DECRQM at all. I didn't think that anyone supporting DECRQM would deliberately not respond to the DECSDM mode but I suppose that is possible too - I need to double check that.

As for why it's not in the VT340 manual, remember that DECSDM itself wasn't documented in the first edition of the graphics manual, so it's not hugely surprising that it wouldn't be listed under the DECRQM query in the text manual. It's possible it may be in the second edition, but I haven't been able to find a copy online.

Btw, can I ask again if you would be willing to run some other compatibility tests on your VT340 when you have time? It's totally OK if the answer is no - I just want to know if I need to look elsewhere for help with that.

hackerb9 commented 3 years ago

@j4james Sure, I can help with running some tests. I'll start a new project for that so we don't spam everyone on this bug. Once you get your results, are you thinking of adding your VT340 compatibility tests to @ThomasDickey's vttest?

j4james commented 3 years ago

@j4james Sure, I can help with running some tests. I'll start a new project for that so we don't spam everyone on this bug. Once you get your results, are you thinking of adding your VT340 compatibility tests to @ThomasDickey's vttest?

My initial plan was just to produce a simple set of text files that you can cat or type to the terminal, so they're completely OS independent. And the idea is we would also have a matching screenshot for each test case showing what it is expected to look like. Initially that could just be a photograph of the VT340 screen.

Producing tests along the lines of vttest is more complicated, though, because you'd ideally want to break things down into many more test cases that just cover one issue at a time, and you need to be able to describe the expected output in text. We can maybe progress to something like that eventually if you're keen, but it'll take a lot more work.

Anyway, I'll put together a PR on your new repo tomorrow with the tests I have so far, and we can discuss further there.

hackerb9 commented 3 years ago

Anyway, I'll put together a PR on your new repo tomorrow with the tests I have so far, and we can discuss further there.

Okay, I've got a skeleton up here: https://github.com/hackerb9/vt340test . We'll talk more about non-DECSDM matters there.

Speaking of which, I can confirm that on the VT340 DECSDM is always exactly the opposite of "Sixel Scrolling" in the Setup Screen. The VT340 does respond correctly to DECRQM (as does XTerm, of course). So, eventually it may be possible for lsix (and notcurses) to limit the damage caused by using DECSDM if it does the opposite of what is expected.

dankamongmen commented 3 years ago

Speaking of which, I can confirm that on the VT340 DECSDM is always exactly the opposite of "Sixel Scrolling" in the Setup Screen. The VT340 does respond correctly to DECRQM (as does XTerm, of course). So, eventually it may be possible for lsix (and notcurses) to limit the damage caused by using DECSDM if it does the opposite of what is expected.

i can dig it!

hackerb9 commented 2 years ago

Release 1.8 is out and, as @j4james suggested, lsix no longer attempts to set DECSDM. I still see this as non-optimal as it is conceivable a program will have sent the escape sequence that disables sixel scrolling before lsix gets run, but at least we will no longer be guilty of doing so.

Thank you very much, @j4james, and to all who helped with this.

ThomasDickey commented 2 years ago

fwiw, I made the suggested change to disable SIXEL scrolling on August 10, but have another feature improvement to complete before patch 369 (and am at the moment deep in a change for ncurses). The comment here about DECRQM is ambiguous, but I interpreted that to mean that there's no change to that aspect of the control sequences.

dankamongmen commented 2 years ago

fwiw, I made the suggested change to disable SIXEL scrolling on August 10, but have another feature improvement to complete before patch 369 (and am at the moment deep in a change for ncurses). The comment here about DECRQM is ambiguous, but I interpreted that to mean that there's no change to that aspect of the control sequences.

you feel relatively certain that the scrolling change will be present in Patch 369? if so, i'll likely add code to anticipate that.

hackerb9 commented 2 years ago

By the by, I've written a little shell script, testdecsdm.sh that attempts to detect if DECSDM is reversed on a terminal or not by checking if the cursor row moves down when sixels are displayed. It works for me, but has not been extensively tested.

ghost commented 2 years ago

While we're all here, is DECSDM also included with the other flags in save/restore cursor DECSC/DECRC?

j4james commented 2 years ago

I strongly suspect the answer to that is no. The only mode that DECSC is documented as saving is DECOM. And that makes sense, since that's closely tied to the cursor position. I know DECSC covers a bunch of stuff that doesn't feel like "cursor state", but DECSDM would really be pushing the limits.

ghost commented 2 years ago

This is a bit out there, but on a hardware terminal would DECSDM permit displaying pixels all the way down to the bottom text row? It sounds like yes, and I have a test that worked in foot and partially in xterm. If this is what is supposed to happen, then I would like to encourage those terminals that choose to support DECSDM to also support writing fully into the bottom row without scrolling.

j4james commented 2 years ago

This is a bit out there, but on a hardware terminal would DECSDM permit displaying pixels all the way down to the bottom text row?

It should, yes. There's a sixel_display_mode test in the vt340test repo that confirms scrolling is disabled when DECSDM is set.

I have a test that worked in foot and partially in xterm.

When you say "partially in xterm", is this related to the DECSDM not working, or some other bug?

If this is what is supposed to happen, then I would like to encourage those terminals that choose to support DECSDM to also support writing fully into the bottom row without scrolling.

I think most of them already do. I probably need to go back and double check this, because some still had DECSDM backwards when I first tested, but once you got the mode right, it did actually prevent scrolling.

ghost commented 2 years ago

I have a test that worked in foot and partially in xterm.

When you say "partially in xterm", is this related to the DECSDM not working, or some other bug?

Probably a different bug. The image is being drawn from the top-left corner as expected without scrolling, but only the first sixel row (6 pixels) is written to the bottom text line. Example: https://gitlab.com/klamonte/jexer/uploads/8e957be7b082702c6f6370a1a10e62d8/xterm_sixel_bottom.png . The "text cursor" on the bottom row is actually an image, and if you zoom a lot you can see that a bit of the bottom text row is getting pixels.

j4james commented 2 years ago

The image is being drawn from the top-left corner as expected without scrolling, but only the first sixel row (6 pixels) is written to the bottom text line.

Interesting. I hadn't noticed that before, but it's easily reproducible just catting a largish sixel image with DECSDM enabled. Although most images won't have transparency enabled, and in that case it still fills to the bottom of the page with the background color, so it's not as obvious it's being clipped.