fix(diff-overlay): scale down oversized emoji glyphs to fit row height#231
fix(diff-overlay): scale down oversized emoji glyphs to fit row height#231forketyfork merged 2 commits intomainfrom
Conversation
Issue: Emoji in diff lines are rendered at the system emoji font's native size (Apple Color Emoji on macOS is several times taller than the monospace row height), causing the glyph to overflow its row and obscure adjacent lines. Solution: Before drawing each segment texture, clamp the height to row_height and scale width proportionally. The same approach is already used in story_overlay and reader_overlay. Also fixed the width-clip guard to compare against the post-scale render_w instead of the raw segment width.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 933311ec22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| render_w = @max(1, @as(c_int, @intFromFloat(@as(f32, @floatFromInt(render_w)) * scale))); | ||
| render_h = row_height; |
There was a problem hiding this comment.
After scaling oversized glyph textures (render_w/render_h are reduced), the existing width-clip branch still reuses those post-scale values for clip_src.w/clip_src.h, but SDL source rectangles are in original texture pixels. In lines that both contain tall fallback glyphs (emoji) and still exceed max_width, this clips to only a small top-left part of the original texture instead of clipping the scaled output, so text/emoji appears heavily truncated. Convert clip dimensions back to source-space (or avoid source clipping for the scaled path) to preserve full glyph content.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 093a3b1. The clip rect is now in source texture space: max_width is multiplied by segment.w / render_w to get the correct texture-pixel width, and clip_src.h uses segment.h. Works the same as before when no height scaling is applied (ratio is 1:1), and clips the right portion when it is.
There was a problem hiding this comment.
Pull request overview
This PR fixes a rendering bug where emoji and other oversized glyphs from system fonts overflow their row in the diff overlay, obscuring adjacent lines. The fix scales down texture dimensions proportionally when a segment's height exceeds the row height, following the same pattern used in story_overlay and reader_overlay (via their fitTextureHeight helper). Additionally, the width-clipping logic is updated to compare the post-scale render_w instead of the original segment.w.
Changes:
- Add proportional downscaling for oversized segment textures (lines 2012-2017)
- Change render_h from const to var to allow modification (line 2010)
- Update width-clipping comparison to use scaled render_w instead of segment.w (line 2027)
Comments suppressed due to low confidence (1)
src/ui/components/diff_overlay.zig:2033
- When both scaling and width clipping are applied, the source rectangle dimensions are incorrect. After scaling down (e.g., a 200x200 emoji texture scaled to 20x20 using scale=0.1), if width clipping is needed to fit max_width=15, the source rect should be {0, 0, 150, 200} (i.e., 15/0.1 = 150 for width, and segment.h for full height), allowing SDL to scale it to the 15x20 destination. Currently, the code sets clip_src to {0, 0, 15, 20}, which takes a 15x20 pixel region from the 200x200 source without scaling it. To fix this: store the scale factor when scaling is applied, then use (render_w / scale, segment.h) for the source rect width and height instead of (render_w, render_h).
clip_src = c.SDL_FRect{
.x = 0,
.y = 0,
.w = @floatFromInt(render_w),
.h = @floatFromInt(render_h),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When height scaling reduces render_w below segment.w, the clip rect needs to be expressed in original texture pixels, not destination pixels. Using render_w directly clips a tiny sliver of the texture instead of the proportional left portion of the glyph.
1 participant
When a diff line contains an emoji, Apple Color Emoji on macOS produces a surface several times taller than the monospace row height. The segment texture was drawn at its natural height with no upper bound, so the emoji overflowed its row and obscured adjacent lines.
renderDiffContentindiff_overlay.zignow scales both dimensions down proportionally when a segment texture exceedsrow_height. SDL scales the texture to the destination rect, so no source-rect clipping is needed for height. The width-clip guard was also fixed to compare against the post-scalerender_wrather than the rawsegment.w.story_overlayandreader_overlayalready handle this via theirfitTextureHeighthelper and are not affected.Closes #230
Test plan
+ State: 🟢 good progress)