Enable 256-creg sixel output when DA1 advertises feature 4 #2253
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
if we crash while detecting kitty keyboard support, we currently don't pop the keyboard state. since we set our desired state before detecting support, this could leave the terminal in an invalid state. now, we initialize kbdlevel to UINT_MAX before detection, so that in a crash, the pop always happens. we reset it at the end of interrogation, assuming we didn't load it from a query response. the downside is that a terminal which doesn't consume this sequence will get hit with it in a crash, but we're already crashing, so who cares? leaving the terminal in an invalid state, however, is very annoying.
dankamongmen
requested review from
grendello,
igo95862,
joseluis and
michaelsbradleyjr
dankamongmen
force-pushed
the
dankamongmen/sixelmania
branch
from
86ee7b4 to
ee62ff6
Compare
This was
linked to
issues
Oct 9, 2021
|
FYI, I've tested this on VTE, MinTTY, and DomTerm, and they're all working now. Not fantastically, because none of them support transparency, but they are at least displaying images now. I'm still investigating issues on some of the other terminals. |
that's awesome. i assume they support opaque images perfectly (within quantization limits), then? w00t i'm glad you pushed for this! |
|
and thank you tremendously for doing all this testing, which quite frankly i doubt i would get to! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Even if we received no answers to our
XTSMGRAPHICSqueries (any replies here are preferred to this method, and will not be overridden), Sixel might still be supported. See @j4james's extensive list of software (and maybe hardware?) I've never heard of. If 4 is among the attributes listed in a DA1 response, that's a claim of Sixel support. For now, if we receive this in the absence of other Sixel info, enable Sixel with an assumption of 256 color registers.Some of these will only support 16, but our crappy quantization algorithm doesn't support that anyway, so for now they get forcefed a palette of 256 colors like so many geese in a french foie gras shack.