Fixes#5522
This commit re-dispatches command inputs that are unhandled by our macOS
app so they can be encoded to the pty and handled by the core libghostty
key callback system.
We've had a special case `cmd+period` handling in Ghostty for a very
long time (since well into the private beta). `cmd+period` by default
binds to "cancel" in macOS, so it doesn't encode to the pty. We don't
handle "cancel" in any meaningful way in Ghostty, so we special-cased it
to encode properly to the pty.
However, as shown in #5522, if the user rebinds `cmd+period` at the
system level to some other operation, then this is ignored and we encode
it still. This isn't desirable, we just want to work around not caring
about "cancel."
The callback path that AppKit takes for key events is a bit convoluted.
For command keys, it first calls `performKeyEquivalent`. If this returns
false (we want to continue standard processing), then it calls EITHER
`keyDown` or `doCommand(by:)`. It calls the latter if there is a
standard system command that matches the key event. For `cmd+period` by
default, this is "cancel." Unfortunately, from `doCommand` we can't say
"oops, we don't want to handle this, please continue processing." Its
too late.
So, this commit stores the last command key event from
`performKeyEquivalent` and if we reach `doCommand` for it without having
called `keyDown`, we re-dispatch the event and send it to keyDown.
I'm honestly pretty sus about this whole logic but it is scoped to only
command-keys and I couldn't trigger any adverse behavior in my testing.
It also definitely fixed#5522 as far as I could reproduce it before.
Fixes#5522
This commit re-dispatches command inputs that are unhandled by our macOS
app so they can be encoded to the pty and handled by the core libghostty
key callback system.
We've had a special case `cmd+period` handling in Ghostty for a very
long time (since well into the private beta). `cmd+period` by default
binds to "cancel" in macOS, so it doesn't encode to the pty. We don't
handle "cancel" in any meaningful way in Ghostty, so we special-cased it
to encode properly to the pty.
However, as shown in #5522, if the user rebinds `cmd+period` at the
system level to some other operation, then this is ignored and we encode
it still. This isn't desirable, we just want to work around not caring
about "cancel."
The callback path that AppKit takes for key events is a bit convoluted.
For command keys, it first calls `performKeyEquivalent`. If this returns
false (we want to continue standard processing), then it calls EITHER
`keyDown` or `doCommand(by:)`. It calls the latter if there is a
standard system command that matches the key event. For `cmd+period` by
default, this is "cancel." Unfortunately, from `doCommand` we can't say
"oops, we don't want to handle this, please continue processing." Its
too late.
So, this commit stores the last command key event from
`performKeyEquivalent` and if we reach `doCommand` for it without having
called `keyDown`, we re-dispatch the event and send it to keyDown.
I'm honestly pretty sus about this whole logic but it is scoped to only
command-keys and I couldn't trigger any adverse behavior in my testing.
It also definitely fixed#5522 as far as I could reproduce it before.
This updates our bundled Harfbuzz from 8.4 to 11.0. The changes from 8
to 11 include a number of correctness and performance improvements.
Packaged releases tend to dynamically link so this won't affect existing
users, but build-from-source users hopefully get an improvement.
This updates our bundled Harfbuzz from 8.4 to 11.0. The changes from 8
to 11 include a number of correctness and performance improvements.
Packaged releases tend to dynamically link so this won't affect
existing users, but build-from-source users hopefully get an
improvement.
We have always tracked a post-3.4 release, this brings us up to date.
There's no real motivation beyond this other than keeping up to date
since we're already on non-release versions anyways. The diff is
dominated by auto-generated headers from Wayland-scanner.
We have always tracked a post-3.4 release, this brings us up to date.
There's no real motivation beyond this other than keeping up to date
since we're already on non-release versions anyways.
Fixes#6872
This commit explicitly acquires the GL context in the `unrealize` signal
handler of the GTK Surface prior to cleaning up GPU resources.
A GLArea only guarantees that the associated GdkContext is current for
the `render` signal (see the docs[1]). This is why our OpenGL renderer
"defers" various operations such as resize, font grid changing, etc.
(see the `deferred_`-prefix fields in `renderer/OpenGL.zig`). However,
we missed a spot.
The `gtk-widget::unrealize` signal is emitted when the widget is
destroyed, and it is the last chance we have to clean up our GPU
resources. But it is not guaranteed that the GL context is current at
that point, and we weren't making it current. On the NGL GTK renderer,
this was freeing GPU resources we didn't own.
As best I can understand, the old GL renderer only ever used a handful
of GL resources that were early in the ID space, so by coincidence we
were probably freeing nothing and everything was fine. But with the new
NGL renderer uses a LOT more GL resources (make a few splits and the ID
space is already in the thousands, from GTK!), so we were freeing real
resources that we didn't own, which caused rendering issues. :)
I suspect the above also resulted in VRAM memory leaks (which would be
RAM memory leaks for unified memory GPUs). This potentially relates to
#5491.
The fix is to explicitly make the GL context current in the `unrealize`
handler.
[1]: https://docs.gtk.org/gtk4/method.GLArea.make_current.html
This was a hack, and is no longer required since #6877. Users can
explicitly override the GTK GSK renderer by setting the standard GTK env
var `GSK_RENDERER`.
Fixes#6872
This commit explicitly acquires the GL context in the `unrealize`
signal handler of the GTK Surface prior to cleaning up GPU resources.
A GLArea only guarantees that the associated GdkContext is current for
the `render` signal (see the docs[1]). This is why our OpenGL renderer
"defers" various operations such as resize, font grid changing, etc.
(see the `deferred_`-prefix fields in `renderer/OpenGL.zig`).
However, we missed a spot.
The `gtk-widget::unrealize` signal is emitted when the widget is
destroyed, and it is the last chance we have to clean up our GPU
resources. But it is not guaranteed that the GL context is current at
that point, and we weren't making it current. On the NGL GTK renderer,
this was freeing GPU resources we didn't own.
As best I can understand, the old GL renderer only ever used a handful
of GL resources that were early in the ID space, so by coincidence we
were probably freeing nothing and everything was fine. But with the new
NGL renderer uses a LOT more GL resources (make a few splits and the ID
space is already in the thousands, from GTK!), so we were freeing real
resources that we didn't own, which caused rendering issues. :)
I suspect the above also resulted in VRAM memory leaks (which would be
RAM memory leaks for unified memory GPUs). This potentially relates to
#5491.
The fix is to explicitly make the GL context current in the `unrealize`
handler.
[1]: https://docs.gtk.org/gtk4/method.GLArea.make_current.html
> 3. If you want to live dangerously, open a pull request and hope for
the best.
Sure, why not!
---
This is a *super common* ask in both the GitHub Discussions and on
Discord; I thus decided to add a small(ish) note to the help output
directing users to try the open command. I did not include a note to
check the man page, as the text was already getting a bit long, but I
can change that if requested. Strings open for bike-shedding, of course.
Of course, feel free to close this if this is not a desirable change for
the project; I would appreciate a note about that though, rather than a
random unexpected close without any reason, as that would prevent any
future PRs about this from others.
As I do not use macOS, I was unable to test the appearance of the string
I edited in `main_ghostty.zig`.
On a slightly related note: are there any plans to translate the CLI's
strings? I assume they're in the same boat as the configuration parsing
errors which were [discussed in #maintainers] on [the Ghostty Discord].
[discussed in #maintainers]:
https://discord.com/channels/1005603569187160125/1337443701403815999/1352390511553417327
[the Ghostty Discord]: https://discord.org/ghostty
We don't currently support rendering SVG glyphs so they should be
ignored when loading. Additionally, the check for whether a glyph is
colored has been simplified by just checking the pixel mode of the
rendered bitmap.
This commit also fixes a bug caused by calling the color check inside of
`renderGlyph`, which caused the bitmap to be freed creating a chance for
memory corruption and garbled glyphs. This bug was introduced by 40c1140
(#6602) and discussed in #6781.
Fixes#6821
UTF8 translation using KeymapDarwin requires a buffer and the buffer was
stack allocated in the coreKeyEvent call and returned from the function.
We need the buffer to live longer than this.
Long term, we're removing KeymapDarwin (there is a whole TODO comment in
there about how to do it), but this fixes a real problem today.
Fixes#6821
UTF8 translation using KeymapDarwin requires a buffer and the buffer was
stack allocated in the coreKeyEvent call and returned from the function.
We need the buffer to live longer than this.
Long term, we're removing KeymapDarwin (there is a whole TODO comment in
there about how to do it), but this fixes a real problem today.
Also update shaper test that fails because the run iterator can't apply
that logic since `testWriteString` doesn't do proper grpaheme clustering
so the parts are actually split across multiple cells.
Several other tests are technically incorrect for the same reason but
still pass, so I've decided not to fix them here.
This tweak is minor and fixes some grammatical errors.
I did not change `kopier` to `kopiér`, even though that is a common way
to write the word. The language council of Norway suggest writing the
word as it is written here, without the accent, but it does read weird
and allows for misunderstanding. If this was my project I would have
added the accent. Let me know if you want the accent added.
What do you think regarding the accents @Uzaaft ?
this fix addresses issue occurring in nvim/vim where Backspace deletes
previous letter while in preedit state.
related to #1638
> same issue reproducible
> However, that only fixed for legacy encoding.
Unanswered Discussion
https://github.com/ghostty-org/ghostty/discussions/5312<div
type='discussions-op-text'> can be closed afterwards
tested on macos sequioa IME '2-set Korean', 'Japanese Romaji'
This tweak is minor and fixes some grammatical errors.
I did not change `kopier` to `kopiér`, even though that is a common way
to write the word. The language council of Norway suggest writing the
word as it is written here, without the accent, but it does read weird
and allows for misunderstanding. If this was my project I would have
added the accent. Let me know if you want the accent added.
Closes#6760
Supersedes #6762
This introduces the concept of a "dist resource" (specifically a
`GhosttyDist.Resource` type). This is a resource that may be present in
dist tarballs but not in the source tree. If the resource is present and
we're not in a Git checkout, then we use it directly instead of
generating it. This is a generic concept we can apply to any
preprocessing we want to do that we don't want users/packagers to do.
This is used for the first time in this commit for the gresource c/h
files, which depend on a variety of external tools (blueprint-compiler,
glib-compile-resources, etc.) that we do not want to require downstream
users/packagers to have and we also do not want to worry about them
having the right versions.
This also adds a check for `distcheck` to ensure our distribution
contains all the expected files.
Note @jcollie that there may be elements of 6762 you'll want to bring
back after this.
This introduces the concept of a "dist resource" (specifically a
`GhosttyDist.Resource` type). This is a resource that may be present in
dist tarballs but not in the source tree. If the resource is present and
we're not in a Git checkout, then we use it directly instead of
generating it.
This is used for the first time in this commit for the gresource c/h
files, which depend on a variety of external tools (blueprint-compiler,
glib-compile-resources, etc.) that we do not want to require downstream
users/packagers to have and we also do not want to worry about them
having the right versions.
This also adds a check for `distcheck` to ensure our distribution
contains all the expected files.
We don't currently support rendering SVG glyphs so they should be
ignored when loading. Additionally, the check for whether a glyph is
colored has been simplified by just checking the pixel mode of the
rendered bitmap.
This commit also fixes a bug caused by calling the color check inside of
`renderGlyph`, which caused the bitmap to be freed creating a chance for
memory corruption and garbled glyphs.
Discover resourcesdir with `terminfo/g/ghostty`
as well as existing `terminfo/x/xterm-ghostty`.
This allows either terminfo file to be installed,
notably ncurses only provides a `terminfo/g/ghostty`.
It was brought up that the `ncurses-term` package on Fedora 42 at
<https://packages.fedoraproject.org/pkgs/ncurses/ncurses-term/fedora-42.html>
now provides a `/g/ghostty` terminfo entry which conflicts with
installing the similarly named file from ghostty's build process.
However a build with `zig build -Demit-terminfo=false` won't work as the
`x/xterm-ghostty` terminfo entry is used as a sentinel to discover the
`GHOSTTY_RESOURCES_DIR` used as a reference path for finding locale and
theme files.
```
src/Surface.zig|546 col 43| .resources_dir = global_state.resources_dir,
src/cli/list_themes.zig|112 col 22| if (global_state.resources_dir == null)
src/global.zig|38 col 5| resources_dir: ?[]const u8,
src/global.zig|173 col 14| self.resources_dir = try internal_os.resourcesDir(self.alloc);
src/global.zig|174 col 27| errdefer if (self.resources_dir) |dir| self.alloc.free(dir);
src/global.zig|177 col 18| if (self.resources_dir) |v| internal_os.i18n.init(v) catch |err| {
src/global.zig|185 col 18| if (self.resources_dir) |dir| self.alloc.free(dir);
```
We also have some comments that intend to change how the terminfo
database is discovered.
https://github.com/ghostty-org/ghostty/blob/main/src/termio/Exec.zig#L776C1-L781C60
