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

Sticky image tiles #32

Closed
jerch opened this issue Aug 30, 2022 · 21 comments · Fixed by #33
Closed

Sticky image tiles #32

jerch opened this issue Aug 30, 2022 · 21 comments · Fixed by #33
Labels
bug Something isn't working

Comments

@jerch
Copy link
Owner

jerch commented Aug 30, 2022

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.

@jerch jerch added the bug Something isn't working label Aug 30, 2022
This was referenced Aug 30, 2022
@jerch jerch closed this as completed in #33 Aug 30, 2022
@pmp-p
Copy link

pmp-p commented Aug 30, 2022

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
Copy link
Owner Author

jerch commented Aug 30, 2022

@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
Copy link

pmp-p commented Aug 30, 2022

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
Copy link
Owner Author

jerch commented Aug 30, 2022

Ubuntu, tested in latest Chromium and Firefox.

@jerch
Copy link
Owner Author

jerch commented Aug 30, 2022

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
Copy link

pmp-p commented Aug 30, 2022

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
Copy link
Owner Author

jerch commented Aug 30, 2022

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
Copy link
Owner Author

jerch commented Aug 30, 2022

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
Copy link

pmp-p commented Aug 30, 2022

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
Copy link

pmp-p commented Aug 30, 2022

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

@jerch
Copy link
Owner Author

jerch commented Aug 30, 2022

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

pmp-p added a commit to pygame-web/archives that referenced this issue Aug 30, 2022
@pmp-p
Copy link

pmp-p commented Sep 3, 2022

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
Copy link
Owner Author

jerch commented Sep 3, 2022

@pmp-p Hmm looks like a bug to me 😄

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
Copy link

pmp-p commented Sep 3, 2022

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
Copy link
Owner Author

jerch commented Sep 3, 2022

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
Copy link
Owner Author

jerch commented Sep 3, 2022

which xterm version is that?

@jerch
Copy link
Owner Author

jerch commented Sep 3, 2022

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

Done, plz try version 0.1.2.

@pmp-p
Copy link

pmp-p commented Sep 15, 2022

Done, plz try version 0.1.2.

works fine !

@pmp-p
Copy link

pmp-p commented Sep 16, 2022

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

@jerch
Copy link
Owner Author

jerch commented Sep 16, 2022

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants