-
-
Notifications
You must be signed in to change notification settings - Fork 906
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
sixel support #970
sixel support #970
Conversation
@tsl0922 Imho this is ready for review. about the flow control issues: Or lets discuss things here if you think thats a showstopper. |
The worker js string can be embeded with webpack, try the following patch: --- a/html/src/components/terminal/index.tsx
+++ b/html/src/components/terminal/index.tsx
@@ -4,11 +4,14 @@ import { ITerminalOptions, RendererType, Terminal } from 'xterm';
import { FitAddon } from 'xterm-addon-fit';
import { WebglAddon } from 'xterm-addon-webgl';
import { WebLinksAddon } from 'xterm-addon-web-links';
-
+import { ImageAddon } from 'xterm-addon-image';
import { OverlayAddon } from './overlay';
import { ZmodemAddon, FlowControl } from '../zmodem';
import 'xterm/css/xterm.css';
+import worker from 'xterm-addon-image/lib/xterm-addon-image-worker';
+
+const imageWorkerUrl = window.URL.createObjectURL(new Blob([worker], { type: 'text/javascript' }));
interface TtydTerminal extends Terminal {
fit(): void;
@@ -176,6 +179,7 @@ export class Xterm extends Component<Props> {
terminal.loadAddon(overlayAddon);
terminal.loadAddon(new WebLinksAddon());
terminal.loadAddon(this.zmodemAddon);
+ terminal.loadAddon(new ImageAddon(imageWorkerUrl));
terminal.onTitleChange(data => {
if (data && data !== '' && !this.titleFixed) { --- a/html/webpack.config.js
+++ b/html/webpack.config.js
@@ -37,6 +37,15 @@ const baseConfig = {
'sass-loader',
],
},
+ {
+ test: /xterm-addon-image-worker/,
+ type: 'asset/inline',
+ generator: {
+ dataUrl: content => {
+ return content.toString();
+ }
+ }
+ },
]
},
resolve: { |
@tsl0922 Oh well, this looks much better and less error-prone 👍 |
@rihaq Yes the gh actions should produce the binaries containing it. Sorry, there is nothing like a "sixel community", there are just a bunch peeps trying to get a hold of old DEC specs and working around issues, to get something usable in newer terminal emulators. If you are interested in sixel, these might be starting points to dig deeper: https://github.com/hackerb9/vt340test (also contains original DEC docs) or https://github.com/hackerb9/lsix. |
Might be a browser rounding bug/artefact (cannot repro it here)? Whats your system, screen resolution and browser version? If you find bugs, plz file an issue in the addon repo, not here in some already closed PR. |
@rihaq Have created a patch release v0.1.3 which hopefully fixes the line striping (unclear since I cannot repro). Btw the striping is most likely from your browser having a pixelDeviceRatio with fractions (high res display, or you have some zoom level activated). |
WIP to get sixel support in ttyd.
@tsl0922 This already works as intended, but I only tested the yarn build + make paths (there is a high chance that the other dev setups in package.json do not work yet). For the bundling of the worker file I only found the awkward way to cat the contents with gulp into inline.html. Maybe you have a better idea to solve this. If you are not interested in sixel support, just drop me a note.
Edit/Sidenote: While messing around with sixels I figured that the zmodem impl is a real throughput killer, and I even ran into random flow control issues. No clue if thats zmodems fault as well, I cannot repro it reliable yet. (Just tested it to see fault tolerance with bigger sixels, as flow control issues are more likely with image data...)