jerch / xterm-addon-image

Image addon for xterm.js
MIT License
51 stars 6 forks source link

Sticky image tiles #32

Closed jerch closed 1 year ago

jerch commented 1 year ago

Coming from #16 - there are circumstances, where image tiles are not properly cleared from screen output.

Until we figured out the root of the issue (prolly some xterm.js action not properly updating the changed lines record), we should do a full clear + redraw cycle for image tiles to get rid of outdated image parts.

pmp-p commented 1 year ago

i gathered the unpkg cdn prebuilts from #16 + https://unpkg.com/xterm@4.19.0/lib/xterm.js and set up with my repro case: https://pygame-web.github.io/archives/0.2.0/pythons.html?cpython3.11&noapp#main.py%20arg1%20arg2

bug:

while green progress bar is displaying , select xterm with mouse and pres "CTRL+L" to clear screen => sixels tiles stick and we can see by transparency that the ">>>" prompt is not present and that cursor is homed which is wrong

expected behaviour :

while green progress bar is displaying , select xterm with mouse and press enter a few time to scroll the whole terminal then "CTRL+L" to clear screen => sixels tiles go away , screen is cleared, prompt is there and cursor is one space after it as expected.

my thoughts :

given the weird prompt position i suspect it's an xterm.js bug, but i'm not sure about my ctrl+l implementation javascript+xterm.js is not really my thing and xterm.js is far from fully vtXXX compliant.

jerch commented 1 year ago

@pmp-p Hmm thats weird - your example build actually works for me with Ctrl-L. Maybe its a browser cache issue still delivering old xterm-addon-image resources?

Regarding FF (which is Ctrl-L) - xterm.js does the same as an old vt100-vt510 would do (treated as LF). This is furthermore tested against xterm behavior. On newer systems the shell will fake the old behavior of Ctrl-L by different approaches (like bash sends the equivalent escape sequences as clear -x would do), and xterm.js shows the exact same behavior as any other xterm-like emulator. So maybe your impl in not in line?

Regarding xterm.js vtxxx compliance - yes that not a goal at all, as most skipped features are so ancient, that it does not makes sense to bloat the code further. But thats not the case for basic C0/C1 codes, those are all in line with VT terminals.

pmp-p commented 1 year ago

Hmm thats weird - your example build actually works for me with Ctrl-L. Maybe its a browser cache issue still delivering old xterm-addon-image resources?

what browser/os are you on ? i get sticky tiles in brave(v8) in dev mode and firefox private browsing too ( where wasm/js cache is off - devtools closed) both on linux/x11

jerch commented 1 year ago

Ubuntu, tested in latest Chromium and Firefox.

pmp-p commented 1 year ago

before ctrl+l https://pmp-p.ddns.net/paste/screen/shot-2022-08-30_1661886843.png after https://pmp-p.ddns.net/paste/screen/shot-2022-08-30_1661886853.png

jerch commented 1 year ago

Oh thats weird - I get the right behavior, if devtools with console is open (as I had before), but the faulty one if devtools is at any other tab or closed. Wth...

Btw on both browser, very strange

pmp-p commented 1 year ago

argh incredible, but too late i already launched my apt-get dist-upgrade XD

and yeah on v8 i always have devtools open in a separate window on another monitor

jerch commented 1 year ago

Eww lol plz dont break your system just because of this, might not be worth it (had to revert to 18 LTS the other day, because 22 was to buggy at several ends)

This devtools.console open thingy is really strange, Ive never seen anything like that...

jerch commented 1 year ago

Do you have a pointer to the code of your Ctrl-L impl? And how do you do PTY emulation (if at all)?

pmp-p commented 1 year ago

no pty, it's a hack of a readline history + buffer current line + cursor position client side https://github.com/pygame-web/archives/blob/main/0.2.0/vtx.js

ha thanks i see the problem i've hardcoded wrong addon path https://github.com/pygame-web/archives/blob/e0d06e9b85b49cb8d363d5ceacabe0ca62eb7e8e/0.2.0/vtx.js#L56

fixing and restesting the new addon asap

pmp-p commented 1 year ago

yeah new addon is good ! really good job, that issue can stay closed :) sorry for the noise !

jerch commented 1 year ago

Sweet :) Well I still wonder what was going on with the devtool.console open/closed thingy - guess we will never find out :rofl:

pmp-p commented 1 year ago

ho i've found a new one let's call it "blinking tiles" :D open : https://pygame-web.github.io/archives/0.2.0/pythons.html type:

debug
cd /data/data/org.python/assets
more cpython.six
more pygame.six

now use up arrow or down arrow => sixel vanish type enter => they come back !

jerch commented 1 year ago

@pmp-p Hmm looks like a bug to me :smile:

I never seen that in my local test env. Can you do a build, where the JS terminal object is bound to global variable, so I can access it directly from console.

pmp-p commented 1 year ago

so I can access it directly from console.

the current terminal window.python.vt and the xterm.js object is window.python.vt.xterm both on js console or python console

jerch commented 1 year ago

It does not remove the images anymore if I set your canvas with the green bar to display:none. So this seems to be some weird canvas overlay artefact from the browser. No wait, it still happens, but less often, wth?

jerch commented 1 year ago

which xterm version is that?

jerch commented 1 year ago

Guess I found the bug - forgot to reset the needsFullClear flag. Doing another patch.

Done, plz try version 0.1.2.

pmp-p commented 1 year ago

Done, plz try version 0.1.2.

works fine !

pmp-p commented 1 year ago

@jerch i think you left some debug log, when sending ESC c SixelHandler.reset... appears on console :)

jerch commented 1 year ago

This is part of the non-worker rewrite, currently stubbed only, as this is quite involved to shape nicely. TS lacks preprocessing features to get this done w'o duplicating half of the code... (and yeah forgot to comment out the log message)