Skip to content

Comments

fix: calculate cell size before presenting gtk window#10459

Open
ajbucci wants to merge 2 commits intoghostty-org:mainfrom
ajbucci:main
Open

fix: calculate cell size before presenting gtk window#10459
ajbucci wants to merge 2 commits intoghostty-org:mainfrom
ajbucci:main

Conversation

@ajbucci
Copy link

@ajbucci ajbucci commented Jan 27, 2026

Fixes #7937

Added computeInitialSize to GTK Surface and call it in GTK Application before the first present(), so the window manager centers the correct size on initial show.

The issue occurs because the core Surface.recomputeInitialSize() runs only after the renderer is initialized. In GTK, the GLArea isn’t realized until after present(), so the initial size arrives too late for WM centering.

Limitations: when we precompute size before present() we do not have access to padding, so the sizing will be very slightly off... but since it is only off a few pixels I was unable to tell visually that it wasn't perfectly centered.

Other thoughts: I was hesitant to make changes to core Surface because the issue is Linux-specific, but it may make sense to extract a helper from recomputeInitialSize to avoid duplicating the sizing math.

AI Disclosure: I used AI to explore the project, help with any language / API questions (I've never used zig before and rarely use gtk), and make implementation suggestions.

@ajbucci ajbucci requested a review from a team as a code owner January 27, 2026 05:05
@jcollie
Copy link
Member

jcollie commented Jan 28, 2026

I think that we should at least look at refactoring recomputeInitialSize in the core Surface to see how much we can deduplicate. If we can get some unit tests from the refactor that would be interesting as well.

const cell = font_grid.cellSize();

// Calculate size (matching recomputeInitialSize logic)
const width = @max(config.@"window-width", 10) * cell.width;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely do not want a magic number 10 here.


// Calculate size (matching recomputeInitialSize logic)
const width = @max(config.@"window-width", 10) * cell.width;
const height = @max(config.@"window-height", 4) * cell.height;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nor the magic number 4. These need to reference the constants in src/Surface.zig somehow.

Copy link
Author

@ajbucci ajbucci Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. I'll take a look at refactoring recomputeInitialSize and adding some unit tests; then I won't have to worry about the private constants.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, I think we can safely leave out the max() altogether because this is just our 'best guess' initial size. When the window eventually presents it will resize to the 'real' size which already has this logic baked in.

@ajbucci
Copy link
Author

ajbucci commented Feb 2, 2026

I took a closer look at this and I think the duplication is minimal enough that we can probably leave it as proposed. Here are the other options I explored:

1) Extract just the final window-size math (cells × cell size ÷ content scale + padding) into a shared helper.

  • Removes a very small amount of duplicated logic, which would only exist in one other place and is fairly straightforward
  • Would require introducing a new static helper on core Surface (Surface currently doesn't have any static helpers), or introduce a separate util
  • Would include padding / min-size logic that isn't relevant pre-init
  • Does allow for ergonomic unit testing of the sizing math

2) Extract the entire sizing pipeline (content scale -> DPI -> DesiredSize -> DerivedConfig -> font grid lookup -> cell size -> window size)

  • Fully deduplicates the logic
  • Callers would have different ownership semantics for font_grid_key: Surface.init needs to retain it, while the GTK pre-init path wants to deref immediately
  • Ends up being fairly invasive to Surface.init for what is effectively a GTK-only quirk

I’m hesitant to introduce new public/static APIs solely to support an apprt quirk, especially if it increases core complexity. Let me know if you'd like me to explore further or have any suggestions!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GTK: Window is off-center on Linux + GNOME when changing initial window dimensions

2 participants