Add background image support for Metal #5233
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hey @Rohit-Bevinahally, thank you for this PR, I can't wait to have it merged! What's you opinion on this? Thanks again for the PR! P.S. also noticed that the image doesn't cover vertical padding |
This is because we don't have a renderer layer (Metal or OpenGL) that covers the entire window, only each individual surface. Adding that would require major surgery in how rendering is implemented and out-of-scope for this PR, and not something to be attempted without some in-depth discussion. |
Hi @sagg0t. Sorry I missed your comment. Patchdiff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig
index dc3cf26b..54c155dd 100644
--- a/src/renderer/Metal.zig
+++ b/src/renderer/Metal.zig
@@ -1725,8 +1725,8 @@ fn drawBackgroundImage(
const Buffer = mtl_buffer.Buffer(mtl_shaders.BgImage);
var buf = try Buffer.initFill(self.gpu_state.device, &.{.{
.terminal_size = .{
- @as(f32, @floatFromInt(self.size.terminal().width)),
- @as(f32, @floatFromInt(self.size.terminal().height)),
+ @as(f32, @floatFromInt(self.size.screen.width)),
+ @as(f32, @floatFromInt(self.size.screen.height)),
},
.mode = self.background_image_mode,
}}, .{
diff --git a/src/renderer/shaders/cell.metal b/src/renderer/shaders/cell.metal
index 3d18c646..34b88e22 100644
--- a/src/renderer/shaders/cell.metal
+++ b/src/renderer/shaders/cell.metal
@@ -780,6 +780,8 @@ vertex BgImageVertexOut bg_image_vertex(
break;
}
+ offset -= uniforms.grid_padding.wx;
+
out.position = uniforms.projection_matrix * float4(final_image_size + offset, 0.0, 1.0);
out.tex_coord = position;
if (in.mode == MODE_TILED) {
|
Rohit-Bevinahally
force-pushed
the
bg-image-metal
branch
from
8a36212 to
0cbf2bb
Compare
yunusey
approved these changes
Jun 3, 2025
qwerasd205
added a commit
that referenced
this pull request
Jun 22, 2025
It's here, the long-foretold and long-procrastinated renderer rework! Hopefully this makes it easier to adapt and modify the renderer in the future and ensures feature parity between Metal and OpenGL. Despite having been a lot of work to write initially, with the abstraction layer in place I feel like working on the renderer will be a much more pleasant experience going forward. ## Key points - CPU-side renderer logic is now mostly unified via a generic `Renderer`. - A graphics API abstraction layer over OpenGL and Metal has been introduced. - Minimum OpenGL version bumped to `4.3`, so can no longer be run on macOS; I used the nix VM stuff for my testing during development. (Edit by @mitchellh: Note for readers that Ghostty still works on macOS, but the OpenGL backend doesn't, only the Metal one) - The OpenGL backend now supports linear blending! Woohoo! The default `alpha-blending` has been updated to `linear-corrected` since it's essentially a strict improvement over `native`. The default on macOS is still `native` though to match other mac apps in appearance, since macOS users are more sensitive to text appearance. - Custom shaders can now be hot reloaded. - The background color is once again drawn by us, so custom shaders can interact with it properly. In general, custom shaders should be a little more robust. ## The abstraction layer The general hierarchy of the abstraction layer is as such: ``` [ GraphicsAPI ] - Responsible for configuring the runtime surface | | and providing render `Target`s that draw to it, | | as well as `Frame`s and `Pipeline`s. | V | [ Target ] - Represents an abstract target for rendering, which | could be a surface directly but is also used as an | abstraction for off-screen frame buffers. V [ Frame ] - Represents the context for drawing a given frame, | provides `RenderPass`es for issuing draw commands | to, and reports the frame health when complete. V [ RenderPass ] - Represents a render pass in a frame, consisting of : one or more `Step`s applied to the same target(s), [ Step ] - - - - each describing the input buffers and textures and : the vertex/fragment functions and geometry to use. :_ _ _ _ _ _ _ _ _ _/ v [ Pipeline ] - Describes a vertex and fragment function to be used for a `Step`; the `GraphicsAPI` is responsible for these and they should be constructed and cached ahead of time. [ Buffer ] - An abstraction over a GPU buffer. [ Texture ] - An abstraction over a GPU texture. ``` More specific documentation can be found on the relevant structures. ## Miscellany Renderers (which effectively just means the generic renderer) are now expected to only touch GPU resources in `init`, certain lifecycle functions such as the `displayRealized`/`displayUnrealized` callbacks from GTK-- and `drawFrame`; and are also expected to be thread-safe. This allows the renderer thread to build the CPU-side buffers (`updateFrame`) even if we can only *draw* from the app thread. Because of this change, we can draw synchronously from the main thread on macOS when necessary to always have a frame of the correct size during a resize animation. This was necessary to allow the background to be drawn by our GPU code (instead of setting a background color on the layer) while still avoiding holes during resize. The OpenGL backend now theoretically has access to multi-buffering, but it's disabled (by setting the buffer count to 1) because it synchronously waits for frames to complete anyway which means that the extra buffers were just a waste of memory. ## Validation To validate that there are no significant or obvious problems, I exercised both backends with a variety of configurations, and visually inspected the results. Everything looks to be in order. The images are available in a gist here: https://gist.github.com/qwerasd205/c1bd3e4c694d888e41612e53c0560179 ## Memory Here's a comparison of memory usage for ReleaseFast builds on macOS, between `main` and this branch. Memory figures given are values from Activity Monitor measuring windows of the same size, with two tabs with 3 splits each. ||Before|After| |-:|-|-| |**Memory**|247.9 MB|224.2 MB| |**Real Memory**|174.4 MB|172.5 MB| Happily, the rework has slightly *reduced* the memory footprint- likely due to removing the overhead of `CAMetalLayer`. (The footprint could be reduced much further if we got rid of multi-buffering and satisfied ourselves with blocking for each frame, but that's a discussion for another day.) If someone could do a similar comparison for Linux, that'd be much appreciated! ## Notes / future work - There are a couple structures that *can* be unified using the abstraction layer, but I haven't gotten around to unifying yet. Specifically, in `renderer/(opengl|metal)/`, there's `cell.zig` and `image.zig`, both of which are substantially identical between the two backends. `shaders.zig` may also be a candidate for unification, but that might be *overly* DRY. - ~~I did not double-check the documentation for config options, which may mention whether certain options can be hot-reloaded; if it does then that will need to be updated.~~ Fixed: be5908f - The `fragCoord` for custom shaders originates at the top left for Metal, but *bottom* left for OpenGL; fixing this will be a bit annoying, since the screen texture is likewise vertically flipped between the two. Some shaders rely on the fragcoord for things like falling particles, so this does need to be fixed. - `Target` should be improved to support multiple types of targets right now it only represents a framebuffer or iosurface, but it should also be able to represent a texture; right now a kind of messy tagged union is used so that steps can accept both. - Custom shader cursor uniforms (#6912) and terminal background images (#4226, #5233) should be much more straightforward to implement on top of this rework, and I plan to make follow-up PRs for them once this is merged. - I *do* want to do a rework of the pipelines themselves, since the way we're rendering stuff is a bit messy currently, but this is already a huge enough PR as it is- so for now the renderer still uses the same rendering passes that Metal did before. - We should probably add a system requirements section to the README where we can note the minimum required OpenGL version of `4.3`, any even slightly modern Linux system will support this, but it would be good to document it somewhere user-facing anyway. # TODO BEFORE MERGE - [x] Have multiple people test this on both macOS and linux. - [ ] ~~Have someone with a better dev setup on linux check for memory leaks and other problems.~~ (Skipped, will merge and let tip users figure this out, someone should *specifically* look for memory leaks before the next versioned release though.) - [x] Address any code review feedback.
qwerasd205
added a commit
to qwerasd205/ghostty
that referenced
this pull request
Jun 25, 2025
Adds support for background images via the `background-image` config. Resolves ghostty-org#3645, supersedes PRs ghostty-org#4226 and ghostty-org#5233. See docs of added config keys for usage details.
|
This PR is superseded by #7686. Thanks for this work, regardless! |
mitchellh
pushed a commit
to qwerasd205/ghostty
that referenced
this pull request
Jun 25, 2025
Adds support for background images via the `background-image` config. Resolves ghostty-org#3645, supersedes PRs ghostty-org#4226 and ghostty-org#5233. See docs of added config keys for usage details.
mitchellh
added a commit
that referenced
this pull request
Jun 25, 2025
Adds support for background images via the `background-image` config. Resolves #3645, supersedes PRs #4226 and #5233. See docs of added config keys for usage details. > [!NOTE] > Unlike what is implied by the original issue, because this is implemented in the renderer it is inherently per-surface not per-window, meaning a window with a split will have two copies of the background image. ### Future work - We should probably introduce code in the apprts that tells surfaces their position and size relative to the window, which would allow us to add a `background-image-area` config with options for `surface` and `window` to control that behavior (and probably default it to `window`). That apprt code would also allow for window-relative custom shader locations, which is also a fairly common user request, so I think it's worth it. - Currently if you use a high res background image this is fairly inefficient, since each surface independently loads a copy of the background image. On systems with limited VRAM this could be an issue for users who use a lot of surfaces, so it may be worth making a shared image cache to avoid this problem. - ~~It's probably worth using compressed texture formats for images, I'll look in to doing that.~~ (c43702c)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.