E. g. the sixel painting code doesn't seem to take into account margins etc. The reason I originally put the sixel code in drawContents() was to reuse the position calculation code: https://invent.kde.org/sandsmark/konsole/-/commit/700d9aa85c71fa578067499b50bbd0402b637a5c#44da6138f6731f1622f59c0b0b6c2dd697e2160e_66_68 I haven't looked at the rest of the changes related to it so there might be a good reason for it to be in drawTextFragment (which isn't supposed to know about absolute positions so it gets messy when it is there). There's also minor stuff, like multiple views onto the same terminal session the parser/vt102emulation is a bit broken since it now accesses a random terminaldisplay to do scaling, but I'm not sure if the code is considered done and I should open new bug reports about minor stuff?
Ok, just a couple of other nitpicks: sixel mode isn't exited when you reset (I ctrl+c-ed img2sixel and had to close the window to get back to normal), and it never seems to free memory when clearing the backlog (neither with `clear` nor reset).
A test case that shows positioning and other issues can be found at: https://github.com/hackerb9/vt340test/blob/main/j4james/repeat_introducer.sh VT340 output: https://github.com/hackerb9/vt340test/blob/main/j4james/repeat_introducer.png In general, the whole of https://github.com/hackerb9/vt340test/blob/main/j4james/ exercises a lot of edge cases of sixel.
Some discussion on initial sixel palette: https://github.com/wez/wezterm/issues/217#issuecomment-1019462608
(In reply to Martin Sandsmark from comment #0) > I haven't looked at the rest of the changes related to it so there might be > a good reason for it to be in drawTextFragment (which isn't supposed to know > about absolute positions so it gets messy when it is there). Yes. This was the easiest place to do it for someone who is not well versed in konsole's architecture to do it. As I learn more, I do intend to move the drawing a few levels up. But the drawing infrastructure supports drawing images below the text or above it, so the options are more limited than for a system where text and images exclude each other. > There's also minor stuff, like multiple views onto the same terminal session > the parser/vt102emulation is a bit broken since it now accesses a random > terminaldisplay to do scaling, but I'm not sure if the code is considered > done and I should open new bug reports about minor stuff? This I am not sure I understand. In another discussion, I was told that even if a screen has more then TerminalDisplay, they will have exactly the same fonts/size, so it does not seem to matter. The problem of what to do with the images if the display changes (resize with text reflow, change in font size) needs a serious discussion. Results of such will also be useful for two different displays for the same screen.
(In reply to ninjalj from comment #2) > A test case that shows positioning and other issues can be found at: > > https://github.com/hackerb9/vt340test/blob/main/j4james/repeat_introducer.sh > > VT340 output: > https://github.com/hackerb9/vt340test/blob/main/j4james/repeat_introducer.png > > In general, the whole of > https://github.com/hackerb9/vt340test/blob/main/j4james/ exercises a lot of > edge cases of sixel. With some fixes, the result of repeat_introducer.sh is now close to the VT340 image. And while matching a 40 years old hardware platform quirk for quirk and bug for bug is an interesting and sometimes fun activity, the main aim is to run useful programs. My test cases now include Imagemagick, libsixel (img2sixel) and gnulpot.
(In reply to Matan Ziv-Av from comment #4) > This I am not sure I understand. In another discussion, I was told that even > if a screen has more then TerminalDisplay, they will have exactly the same > fonts/size, so it does not seem to matter. The screen at least, but the emulation punching through all the abstraction straight to the ui/painting class is messy. > The problem of what to do with the images if the display changes (resize > with text reflow, change in font size) needs a serious discussion. Results > of such will also be useful for two different displays for the same screen. Shouldn't be that complex, just need to find the most common uses (gnuplot, lsix, w3mimgdisplay?) and pick the approach that annoys the smallest amount of people. When I did my implementation I thought it made the most sense to treat an image like a single character, and follow the reflow rules, scaling, etc. like everything else. But could really do with some serious rearchitecting related to this as well, imho. The Emulation/Screen/ScreenWindow/TerminalDisplay/TabbedViewContainer/etc. mess seems a bit overkill and confusing. I think Tomaz felt that painfully when working on the tabbing stuff.
A possibly relevant merge request was started @ https://invent.kde.org/utilities/konsole/-/merge_requests/623
The MR was merged - let us know if this is still an issue.