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

[very WIP] Make Alacritty into a library #1023

Closed
wants to merge 4 commits into from

Conversation

tbodt
Copy link
Contributor

@tbodt tbodt commented Jan 13, 2018

My work on #450. As far as I can tell very little progress has been made on that issue, so I'm posting this just to get a bit of code out there, that maybe others can build on. Feedback would be appreciated.

@jwilm
Copy link
Contributor

jwilm commented Jan 13, 2018

Thanks for the PR! It's great to see some action on this front. Would you mind adding a bit of discussion here about the overall approach? It would help me and others when reviewing the code.

@tbodt
Copy link
Contributor Author

tbodt commented Jan 13, 2018

The first step, which I've done, is make Display not depend on Window. The API for Display would be like:

  • Create with size and config
  • Draw in current OpenGL context
  • Get/set size/config

You could then write a GUI app using the standard platform-specific tools which includes an OpenGL context, and invoke the Display to do the rendering in the context. There will need to be a C API for embedders, since writing native apps in pure Rust is suboptimal (as yet, there are no UI frameworks that use Rust as their native language).

Code from event::Processor that's still applicable to embedders should be moved to input::Processor if possible. There's not a whole lot of it, but there are things like handling of Focus events.

Ultimately, alacritty should no longer depend on glutin or winit. The existing alacritty app and all of the code such as Window and event::Processor that's tightly bound to glutin/winit would be moved into a separate crate, and be renamed to something like alacritty-minimalist (any better ideas?).

@jwilm
Copy link
Contributor

jwilm commented Jan 13, 2018

Thanks for the summary! I generally with the overall approach.

There will need to be a C API for embedders

This sounds right. The hardest part of this will be pushing input events into alacritty in a relatively clean way.

alacritty should no longer depend on glutin or winit

👍

The existing alacritty app and all of the code such as Window, event::Processor that's tightly bound to glutin/winit would be moved into a separate crate

To me, this sounds like the alacritty crate, and all of the generalized stuff should be moved into something like alacritty-core.

@tbodt
Copy link
Contributor Author

tbodt commented Jan 13, 2018

It might be sane to put both the existing app and the library in the same crate, since cargo lets you build a crate both as a library and a binary. The code specific to the alacritty app would go in src/app, and Cargo.toml would say

[[bin]]
name = "alacritty"
path = "src/app/main.rs"

@nixpulvis
Copy link
Contributor

@tbodt but then crates that depend on alacritty(-core) will need to fetch the whole thing, right?

@tbodt
Copy link
Contributor Author

tbodt commented Jan 13, 2018

Actually that's a bad idea, you can't have a different set of dependencies for a binary than the library. And things only depending on the core would have to download and build the whole thing as @nixpulvis pointed out.

@jD91mZM2
Copy link

This is when we'd need rust-lang/cargo#1982 :|

@RedHatter
Copy link

What's the status of this, is it still being worked on?

@tbodt
Copy link
Contributor Author

tbodt commented Mar 3, 2018

@RedHatter I was planning on doing more work on this but school got in the way. I'll probably get back to it eventually. Of course, you're welcome to work on it too if you have time.

@valpackett
Copy link

Hi! I'm interested in making a GTK frontend to alacritty, so I picked up this work: https://github.com/myfreeweb/alacritty/commits/alacritty-split-new (rebased "Refactor Display to not use Window" onto master, did not take the unfinished app crate extraction)

Here's a proof of concept rendering Alacritty to a GTK GLArea:

wayland-screenshot-2018-04-01_15-15-06-fs8

@tbodt tbodt force-pushed the alacritty-split branch 2 times, most recently from 6dbfe54 to 3649c64 Compare April 1, 2018 20:11
@tbodt
Copy link
Contributor Author

tbodt commented Apr 1, 2018

Nice work @myfreeweb! After a bit of git wizardry I think I've managed to get your work into this PR. I'll try and get it to compile before pushing.

@tbodt
Copy link
Contributor Author

tbodt commented Apr 1, 2018

Do you have your GTK frontend on github somewhere?

@valpackett
Copy link

I'm still working on this! I think I'd rather start a new PR.

Frontend is https://github.com/myfreeweb/galacritty

@valpackett
Copy link

Hm, okay, I realized something. I was going in the direction of faking glutin events from my app to use with the Processor. But then I realized I don't need it, it's better for me to use the lower level APIs — this way, the refactoring here doesn't need to be as painful. I don't need to touch the (rather complicated) event processing that's done here. (Except to eventually extract the mouse handling algorithms from impl Processor.)

I currently have working key input, window resize and font size change. Modifiers like Ctrl type garbage for now :D but adding their handling would give me a 100% working minimum viable terminal!

So actually @tbodt you can keep pulling into this PR, I don't think I'll need any huge changes :)

The most important thing for me is making the Window notifier a trait, that's where I tell my GLArea to redraw.

@tbodt
Copy link
Contributor Author

tbodt commented Apr 1, 2018

Awesome! Now that a working prototype exists, I plan on doing some more work on this in the next week. I'll do another rebase magic trick to get the window::Notifier thing.

@tbodt
Copy link
Contributor Author

tbodt commented Apr 2, 2018

It's looking like half the things in the config object shouldn't be the responsibility of alacritty-core:

  • dimensions: embedder gets to size the window
  • env, shell: embedder is the one that starts the shell
  • window.decorations: embedder creates the window
  • rest of window seems to be duplicates
  • config_path: not entirely sure what this is for (edit: realized it's for config reloading, which the embedder should handle)
  • hide_cursor_when_typing: embedder can just call set_cursor_visible
  • live_config_reload: embedder should do config loading
  • key_bindings, mouse_bindings, mouse: embedder should handle these

@jwilm thoughts?

@tbodt
Copy link
Contributor Author

tbodt commented Apr 2, 2018

Most of the rest of the config options could put into a term::Config with:

  • custom_cursor_colors
  • draw_bold_text_with_bright_colors
  • tabspaces
  • cursor_style
  • selection
  • visual_bell
  • dynamic_title
  • cursor_style

display::Config can have render_timer, font, and padding. colors really wants to go in term::Config but Display needs to use the background color one time on startup to work around that Mesa bug.

@jwilm
Copy link
Contributor

jwilm commented Apr 3, 2018

It's exciting to see your work on this @myfreeweb, and thanks to both you and @tbodt for continuing to push this forward.

Regarding event processing, I was hoping that Alacritty could still handle most of this, and the embedder would just need to provide some translation layer to turn things into the types Alacritty expects. The idea is that it would be less work for the embedder.

@tbodt

I feel that the key/mouse bindings functionality should be exposed for the embedder even if its their responsibility to populate it. The rest of the organization sounds right, though.

@tbodt
Copy link
Contributor Author

tbodt commented Apr 3, 2018

I mostly agree about event processing, but the one area where it would be annoying is with key bindings, since they're based on virtual key codes. In order to pass virtual key codes to alacritty, the embedder would almost certainly need a 151-line match to translate them.

@valpackett
Copy link

The key/mouse bindings system is coupled to glutin pretty tightly. Indeed, the encoding of the incoming events depends on the toolkit, but also the Action type is specific to the glutin-based app.

This is how I handle keyboard input: https://github.com/myfreeweb/galacritty/blob/9180f3097489bca7925721802a2a57d6f521d5f0/src/widget.rs#L207-L311 I don't handle non-terminal-input actions (copy-paste, font size changes) through this system, that's what GTK accelerators are for, so I don't need an Action type.

What I do want is the separation of mouse handling algorithms from that system (from the Processor).

LegNeato added a commit to LegNeato/asciinema-rs that referenced this pull request Apr 4, 2018
@LegNeato
Copy link

LegNeato commented Apr 5, 2018

I am in the process of making an example repo, but I want to drop this feedback/usecase here in case I don't get to it in a timely manner. First, thanks! This is exactly what I need and I am so excited you are working on this. My usecase (as mentioned in the related issue) is to use alacritty in headless mode to generate gifs of asciinema recordings.

There are two ways to go about it:

  1. Do actual shell work--send events / input into alacritty, having the pty actually handle the input and call the underlying commands.
  2. Only display--drive the Terminal with alacritty::ansi::Processor with pre-canned output, not actually using a pty and just using alacritty as a renderer (a rough example can be seen in https://github.com/LegNeato/asciinema-rs/blob/gif_output/src/commands/convert.rs#L99).

I have implemented both and encountered 2 problems:

  • Problem 1: For methods 1 and 2 above, the glutin HeadlessContext and OpenGL framebuffer needs a size before alacritty is initialized. This is an issue in the existing code...a window is always created and then resized after the fact. There is actually a comment pointing out that's kind of wonky. My glutin and OpenGL-fu isn't good enough to deal with resizing, and making large surfaces would be a sufficient workaround but would be wasteful. So, a way to get the actual rendered size in pixels before rending the window/HeadlessContext would benefit the current state of the project as well as libification.
  • Problem 2: For method 1 above, sending events via event_loop::Notifier works great...but because everything is async when I read the OpenGL pixels alacritty hasn't finished drawing yet and I get a blank image. Adding sleeps fix the issue but is less-than-ideal of course. I need a way to make the notifications blocking and/or a way to flush all alacritty events and subsequent rendering. Note that this is not an issue for method 2 above, as it doesn't use the async event sending infra.

@nixpulvis
Copy link
Contributor

Totally oddball question, but how hard do you think it'd be to wire this up as a login manager for linux?

@sodiumjoe
Copy link
Contributor

will this work enable rendering with metal on osx? I ask because as you've all probably heard, apple is deprecating opengl for macos. 😞

@valpackett
Copy link

@nixpulvis well, my galacritty project wires this up to GTK, and there are GTK based login managers (gdm), so connect the dots :) and you don't have to use GTK specifically, you can wire it up to a smaller toolkit or whatever

(but why use a terminal emulator for a login manager? if you want text based login, the system terminal should be fine, or kmscon.)

@sodiumjoe no, this is making Alacritty into a library, not adding new render backends to Alacritty.

@valpackett
Copy link

Rebased alacritty-split-new.

@tbodt
Copy link
Contributor Author

tbodt commented Apr 13, 2019

Coming back to this now, there are too many merge conflicts for there to be any way to rebase anymore.. Feels like it would have been best to do the refactoring in small steps that could have already been merged...too late now.

My plan now is to abandon this PR and basically do the whole thing again in a new PR, but this time try to get it merged before it bitrots.

@tbodt tbodt closed this Apr 14, 2019
@tbodt tbodt deleted the alacritty-split branch April 14, 2019 00:25
@chrisduerr
Copy link
Member

@tbodt Thanks for actually staying on top of this and not just abandoning this completely.

I'm totally in favor of doing this incrementally and I'd be happy to cooperate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants