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

Implement a canvas renderer #935

Closed
Tyriar opened this issue Sep 2, 2017 · 16 comments
Closed

Implement a canvas renderer #935

Tyriar opened this issue Sep 2, 2017 · 16 comments
Assignees
Labels
breaking-change Breaks API and requires a major version bump type/enhancement Features or improvements to existing features
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Sep 2, 2017

A work in progress is already well under way. The performance benefits seem to far outweigh anything that can be accomplished in the DOM.

The main drawbacks of using canvas over the DOM:

  • We lose functionality provided by text and node in the DOM, namely:
    • Selection & context menus, this doesn't matter as we already reimplemented them to work around then DOM getting in the way
    • Links just work in the DOM; create an <a> and just put it somewhere. This needs more handling code to work in a canvas.
  • Zooming in will blur the text, so it's best used at 100% page scale

I'll list most of the improvements and bug fixes that canvas provided in the PR which should be out soon 😄

Marking as breaking as we need new API around colors and font styles which will be managed in TS.

@Tyriar Tyriar added breaking-change Breaks API and requires a major version bump feature labels Sep 2, 2017
@Tyriar Tyriar added this to the 3.0.0 milestone Sep 2, 2017
@Tyriar Tyriar self-assigned this Sep 2, 2017
@mofux
Copy link
Contributor

mofux commented Sep 2, 2017

♥️♥️♥️♥️

@lucat1
Copy link
Contributor

lucat1 commented Sep 3, 2017

Looks super cool! But i have two questions:
How will colors be managed in the new v3?
What will change with fonts exactly?

@Tyriar
Copy link
Member Author

Tyriar commented Sep 3, 2017

@lucat1 feedback welcome:

var term = new Terminal({
  fontSize: 10,
  fontFamily: 'Hack',
  lineHeight: 1.2
});

term.setOption('fontSize', 12);
term.setOption('fontFamily', 'courier');
term.setOption('lineHeight', 1);

term.setTheme({
  background: '#333',
  red: '#F00',
  brightRed: '#F22',
  // ... (can be a partial list)
});

The theme interface is currently:

export interface ITheme {
  foreground?: string;
  background?: string;
  cursor?: string;
  black?: string;
  red?: string;
  green?: string;
  yellow?: string;
  blue?: string;
  magenta?: string;
  cyan?: string;
  white?: string;
  brightBlack?: string;
  brightRed?: string;
  brightGreen?: string;
  brightYellow?: string;
  brightBlue?: string;
  brightMagenta?: string;
  brightCyan?: string;
  brightWhite?: string;
}

@lucat1
Copy link
Contributor

lucat1 commented Sep 3, 2017

This looks very promising and performance improvements are always welcome😍

As feedback I have two ideas:

  • Couldn't the theme be inside of the ISettings just like the fonts, bell, etc..?

  • AFAIK, in the current structure, we have the theme property that's set to default, so couldn't reuse it for a multiple-theme support built-in?

    Like term.useTheme('whatever') and term.defineTheme(name, colors)(addons could modify the terminal class to implement themes too, without creating the terminal instance). Theoretically, this would open the way for extension-like themes(making them "global" and custom themes just in some terminals instances). Right?

Then I have a few questions:

  • How would 256 colors support work? Would xterm generate the other shades starting from these 16 colors? Or would we drop 256 support for truecolor?
  • Will this remove completely CSS or use classes for parts like cursor?

    A possible use of CSS customization would be like making the line cursor thicker. How would this be handled with the canvas?

I really love the idea of canvas, and I really hope it will give better performance especially when resizing since both in Electron and on the web it's quite laggy.

@Tyriar
Copy link
Member Author

Tyriar commented Sep 4, 2017

@lucat1 great feedback/questions 😃

Couldn't the theme be inside of the ISettings just like the fonts, bell, etc..?

I was actually thinking this as I wrote up my comment 😄; could just make it an option so it works in the constructor too.

AFAIK, in the current structure, we have the theme property that's set to default, so couldn't reuse it for a multiple-theme support built-in?

I think there is colors which was getting removed

Theoretically, this would open the way for extension-like themes(making them "global" and custom themes just in some terminals instances). Right?

Good ideas. This sort of thing did cross my mind, maybe "themes" and "colors" should be differentiated. Maybe this could be a good idea for an addon, it could bundle multiple names themes and expose useTheme(theme: string) on Terminal?

How would 256 colors support work? Would xterm generate the other shades starting from these 16 colors? Or would we drop 256 support for truecolor?

The 256 colors include the 16 ansi colors defined in the theme. The others (240) cannot be changed. I don't think it's a good idea to allow people to change this as they have strictly defined colors. True color can be supported fairly easily, it just needs the buffer memory improvements to come #791

Will this remove completely CSS or use classes for parts like cursor?

There is still some CSS, but it's very slim now as there are no color styles, no cursor styles, no link styles, etc.

A possible use of CSS customization would be like making the line cursor thicker. How would this be handled with the canvas?

In the current model this would need to be a setting.

@Tyriar Tyriar mentioned this issue Sep 4, 2017
22 tasks
@vincentwoo
Copy link
Contributor

One thing I am concerned about (and I am still all for canvas rendering) is that I also pull out HTML from xterm.js for storage. For instance, if a user runs some code on my site that returns results that are rendered via xterm, I pull out the DOM nodes representing those results and save them. Hopefully there will be an analogous approach I can take in the post-canvas world.

@Tyriar
Copy link
Member Author

Tyriar commented Sep 7, 2017

Do you mean saving and restoring the current buffer? There's the serialization stuff that's planned for v3 still #613 when @parisk frees up, we're targeting November so there's still plenty of time. If worst comes to worst you can stick on 2.9 until it's supported.

If you just need to pull out the data then you could reach into the Terminal.buffer if you needed to. This function pulls out the string representing that line with no styling info: https://github.com/sourcelair/xterm.js/blob/575c10114be5002b45d4d7842a6f8741ba87f5e3/src/Buffer.ts#L199

@vincentwoo
Copy link
Contributor

I did mean just "pulling out the data" but I did want to preserve styling. Would that still be possible?

@Tyriar
Copy link
Member Author

Tyriar commented Sep 8, 2017

@vincentwoo harder but still doable, may break across versions of course. You can grab character data off the buffer object, they include attributes on the characters which encode the color/style.

@Tyriar Tyriar added type/enhancement Features or improvements to existing features and removed type/feature labels Apr 4, 2018
@ghost
Copy link

ghost commented May 17, 2018

Canvas renderer is way too slow for me. I need 60 fps full screen random characters and attributes buttery smoothness. My use case is a full screen roguelike browser game. What about opening up this api for custom renderers? I know #1294 is on the way but I was thinking about something more webgl-y https://www.eventbrite.com/engineering/its-2015-and-drawing-text-is-still-hard-webgl-threejs/ cheers 🍻

@Tyriar
Copy link
Member Author

Tyriar commented May 17, 2018

A full webgl approach may be able to boost perf more, it would be fun to experiment to see how much time could be saved by using it. Currently I'm working in the opposite direction; a fallback to DOM since some systems don't handle canvas/webgl solutions #1360

@Tyriar
Copy link
Member Author

Tyriar commented May 17, 2018

Also I suspect the boost in perf won't actually be that great. There are far better opportunities to boost overall FPS by working on the parser #1399 or involving web workers #955

@jerch
Copy link
Member

jerch commented May 18, 2018

@Tyriar
I played a little bit around with the text renderer and found that a cell based buffer can speed up the render process ~2-3 times if a certain cell that did not change is not rendered again (by checking against the buffered value). This of course is dependent of the amount of not changed cells, also my quick hack did not include nasty context checks where neighbors cells might interfere with each other.

For my typical benchmark call ls -lR /usr/lib I was able to drop the runtime for drawChar from ~600 ms to ~250 ms.

Maybe it is worth to investigate a cell based redraw thingy, idk ... (just wanted to let you know, I am not familiar with that part of the emulator anyway)

@ghost
Copy link

ghost commented May 18, 2018

I'm not sure if I'm doing this right, but I could find that canvas.drawImage and the parser are the biggest sluggish chunks.

screen shot 2018-05-18 at 22 45 04

@jerch
Copy link
Member

jerch commented May 18, 2018

@odrzutowiec Might be better to open a new issue since the original one is about having a canvas renderer at all and therefore closed while your issue is more about a specific performance detail...

Btw Im pretty sure you are seeing the non atlas cached performance for the canvas thing here, which might raise a different question (more clever atlas caching strategy?)

@Tyriar
Copy link
Member Author

Tyriar commented May 18, 2018

@jerch that's how it was, and yeah it was a lot faster. The reason it switched back is because characters need to overlap in order for parts of characters not to be cut off. I plan on getting to minimizing draws eventually.

@odrzutowiec that does seem a bit slow, what browser/version/os are you on? Let's create a separate issue to discuss. Also only ascii is optimized currently, there's a lot of opportunity to speed up for non-ascii characters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Breaks API and requires a major version bump type/enhancement Features or improvements to existing features
Projects
None yet
Development

No branches or pull requests

5 participants