-
Notifications
You must be signed in to change notification settings - Fork 1k
Add Custom Shader Uniforms for Cursor Effects #6912
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
Add Custom Shader Uniforms for Cursor Effects #6912
Conversation
- Changed Uniforms struct in custom.zig to incorporate cursor data - Added iCurrentCursor, iPreviousCursor, and iTimeCursorChange to the shadertoy_prefix.glsl
Holy crap that is awesome. I was meh on cursor trails before but I def want this. |
if (self.gl_state) |*gl_state| { | ||
if (gl_state.custom) |*custom_state| { | ||
custom_state.addCursor(self.size, screen.cursor, cursor_style, render.glyph); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at other parts of the code, I wonder if we should use the "deferred" pattern as used by SetFontSize
, SetScreenSize
, etc. The custom state seems to be more commonly modified there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look into it and see how I can adapt it.
// Additionally, the y-position is adjusted to account for the inverted Y-axis | ||
// in the window coordinate system used by gl_FragCoord, where the origin is at the bottom-left. | ||
// The offset from the glyph is also added to position the cursor correctly. | ||
const current_x: f32 = @as(f32, @floatFromInt(cursor.x * size.cell.width + size.padding.left)) + @as(f32, @floatFromInt(glyph.offset_x)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be misreading this, but since the two floats added together both came from integers, why not just:
const current_x: f32 = @as(f32, @floatFromInt(cursor.x * size.cell.width + size.padding.left)) + @as(f32, @floatFromInt(glyph.offset_x)); | |
const current_x: f32 = @floatFromInt(cursor.x * size.cell.width + size.padding.left + glyph.offset_x); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't add glyph.offset directly because it's an i32, while all the other variables are u32.
Fix comment space Co-authored-by: Kat <[email protected]>
I tried to run a couple of your shaders (blaze and smear) but they result in nothing being drawn. I've verified other [non-cursor] shaders work. I'm still trying to debug why that is but just wanted to let you know in case you knew. Thanks 😄 I just want something working here so I can better iterate on this PR. EDIT: Ah, I see they're not really setup to be Ghostty shaders directly yet since they don't sample the screen. Looking at it. |
This comment has been minimized.
This comment has been minimized.
This requires some modifications so it works with Metal (something I plan to do) and some good example shaders (which were provided in the discussion). It'll be merged eventually... |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
It should be noted that PRs are not spaces for tech support. They are for reviewers to review code. Please leave your issues in the original discussion (#6901) going forwards. |
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.
Based on / supercedes PR ghostty-org#6912, implements discussion ghostty-org#6909 Co-authored-by: Krone Corylus <[email protected]>
Based on / supersedes PR ghostty-org#6912, implements discussion ghostty-org#6901 Co-authored-by: Krone Corylus <[email protected]>
Superseded by #7648, thank you for this work though! I based my implementation on yours and credited you as a Co-author on the relevant commit. |
Supersedes #6912, implements #6901 Also included in this PR is a fix/cleanup of the custom shader uniform handling, moved the CPU-side custom shader uniforms struct to the main renderer struct instead of having it be per-frame, moved the layout struct to `shadertoy.zig` since it has the `std140` layout for both backends. Also, I added the current/previous cursor colors to the uniforms, since I figured they'd be useful to have and it's a trivial addition. ### Future Work - This extension to the shadertoy uniforms needs to be documented somewhere so it's discoverable by users. - The flipped Y axis on Metal still needs to be fully addressed instead of just being patched over like it is right now.
Hi,
As requested, this is a pull request for discussion #6901. It adds cursor position data to the uniforms for custom shaders, enabling more dynamic and interactive effects.
Let me know if any adjustments are needed. Thanks!
blaze_trail.mp4