fix(TextAreaControl): measure glyph width for the horizontal scroll extent by DraftingDreamer · Pull Request #63 · gitextensions/ICSharpCode.TextEditor

DraftingDreamer · 2026-07-26T06:58:37Z

Proposed changes

TextView.GetVisualWidthColumns() (new) returns the painted width of a line in WideSpaceWidth units. It walks line.Words and measures each one with the font the highlighting assigns (word.GetFont), mirroring the tab-stop arithmetic of PaintLinePart. LineSegment.Words is null until the line has been highlighted and may cover only part of it, so any remainder is measured per character through the existing width cache instead of being dropped.
TextAreaControl.UpdateLayout() uses it instead of GetVisualColumnFast(), which counted characters and so underestimated every glyph wider than x (CJK, emoji, fallback-font glyphs). The Graphics is created lazily, so a warm lineLengthCache still measures nothing.
TextAreaControl.OptionsChanged() clears lineLengthCache: the cached widths now depend on the rendering options - font, tab size and highlighting alike.

Screenshots

All shots are Git Extensions built from master at 200% scaling (192 dpi), on a throwaway repository containing CJK source lines, cropped to the diff pane.

Before

The scrollbar has been dragged fully right and the right arrow clicked repeatedly afterwards - the view does not move any further, yet the string literal is cut off mid-sentence:

A file whose longest line has fewer characters than the viewer has columns, but whose glyphs need about twice that width: the line is clipped and there is no scrollbar at all.

The Commit dialog, same story:

After

Dragging fully right now reaches the end of the line, closing quote and semicolon included:

Same file and same scroll position as the second "before" shot - the scrollbar is now there:

Test methodology

Two new tests in ScrollBarTests, both failing without the change:
- HScrollBar_should_be_shown_when_wide_glyphs_exceed_the_visible_width
- HScrollBar_should_allow_scrolling_to_the_end_of_a_line_of_wide_glyphs
They derive their thresholds from the control at run time (glyph width, WideSpaceWidth, DrawingPosition) rather than hard-coding pixels, and they assert the precondition that the glyph really is wider than one column, so on a machine without CJK fallback fonts they fail loudly instead of silently passing.
Whole suite: 53/53 pass (dotnet test --test-adapter-path:.).
Pure ASCII is provably unchanged: with Consolas 10, TextRenderer.MeasureText reports exactly 100 x WideSpaceWidth for 100 ASCII characters, so the computed extent is identical to before.
Verified end to end in Git Extensions itself - Diff tab and Commit dialog, before and after builds - see the screenshots above.

Notes for the reviewer

After this change GetVisualColumnFast() has no callers left, here or in gitextensions. I left it in place because it is public API - happy to remove it in this PR if you would rather.
Cost is one MeasureStringWidth call per word - already served by the existing measureCache - plus a dictionary lookup per character for anything the highlighting has not covered, and only for lines that miss lineLengthCache (the visible lines right after a document change or resize).

Test environment(s)

GIT 2.54.0.windows.1
Windows NT 10.0.26200.0, .NET SDK 10.0.302
200% scaling (192 dpi) - the configuration the issue was reported on

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).

✒️ I contribute this code under The Developer Certificate of Origin.

…xtent The horizontal scrollbar works in WideSpaceWidth units (the width of 'x'), but its extent was derived from GetVisualColumnFast, which counts characters and assumes every one of them is exactly one column wide. Any glyph wider than 'x' - CJK, emoji, glyphs coming from a fallback font, or any proportional font - therefore makes the scroll range shorter than the line actually painted. Depending on how far off it is, either the scrollbar is not shown at all, or its thumb cannot be dragged to the end of the line. Setting HScrollBar.Value from code (caret movement, selection drag) uses the accurate GetVisualColumn and can reach Maximum, so those paths scroll further right than the thumb allows - which is how the mismatch becomes visible to the user. Replace the character count with GetVisualWidthColumns, which measures the glyphs using the existing per-character width cache and mirrors the tab handling of PaintLinePart. The cached line widths now depend on the font, so they are invalidated on OptionsChanged.

Copilot

Pull request overview

This PR fixes horizontal scrollbar sizing in TextAreaControl by measuring the painted glyph widths (rather than counting characters) so wide glyphs (e.g., CJK) properly contribute to horizontal scroll extent, addressing gitextensions/gitextensions#13200.

Changes:

Add TextView.GetVisualWidthColumns() to compute a line’s visual width in WideSpaceWidth units using glyph measurements and tab-stop arithmetic.
Update TextAreaControl.UpdateLayout() to use the measured visual width (with lazy Graphics creation) and clear cached widths on option changes.
Add two scrollbar regression tests covering wide-glyph overflow and rightmost reachability.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
Test/TextEditorControl/ScrollBarTests.cs	Adds regression tests to ensure wide glyphs trigger and correctly size horizontal scrolling.
Project/Src/Gui/TextView.cs	Introduces a new API to compute measured visual line width in scrollbar column units.
Project/Src/Gui/TextAreaControl.cs	Switches scrollbar extent calculation to measured widths and invalidates cached widths when editor options change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

DraftingDreamer · 2026-07-27T12:38:12Z

+        public int GetVisualWidthColumns(Graphics g, LineSegment line)
+        {
+            var font = TextEditorProperties.FontContainer.RegularFont;
+            var tabWidth = WideSpaceWidth*Document.TextEditorProperties.TabIndent;
+            var lineOffset = line.Offset;
+            var width = 0;
+
+            for (var i = 0; i < line.Length; ++i)
+            {
+                var ch = Document.GetCharAt(lineOffset + i);
+                if (ch == '\t')
+                    width = (width + MinTabWidth)/tabWidth*tabWidth + tabWidth;
+                else
+                    width += GetWidth(g, ch, font);
+            }
+
+            // round up so that a glyph which only partially reaches into the last column stays reachable
+            return (width + WideSpaceWidth - 1)/WideSpaceWidth;
+        }


Good catch - adopted. GetVisualWidthColumns now walks line.Words and measures each word with word.GetFont(fontContainer), keeping the tab-stop arithmetic, so it lines up with PaintLinePart and CountColumns.

One thing I kept from the previous version: LineSegment.Words is null until the line has been highlighted (which is why CountColumns bails out with return 0), and it can cover only part of the line. Returning 0 there would be worse than the bug this PR fixes, so anything the words do not cover is still measured per character with the regular font.

For the record, how much this matters in practice - measured with the same TextRenderer.MeasureText flags:

font x U+4E2D 50 ASCII chars

Consolas Regular 8 px 14 px 400 px

Consolas Bold 8 px 15 px 400 px

Consolas Italic 8 px 14 px 400 px

Segoe UI Regular 6 px 14 px 310 px

Segoe UI Bold 8 px 15 px 355 px

So with the default monospaced diff font it is 1 px per bold CJK glyph, but a bold run in a proportional font was off by ~14%. Worth fixing either way.

DraftingDreamer · 2026-07-27T12:38:25Z

        {
            TextArea.OptionsChanged();

+            // the cached line widths are measured with the current font, so they are stale after a font change


Agreed, reworded: the cached widths depend on the rendering options in general - font, tab size and highlighting alike - not just on the font.

PaintLinePart draws each TextWord with the font the highlighting assigns to it, so measuring the whole line with RegularFont could still be off - by 1px per CJK glyph in bold Consolas, and by ~14% for a bold run in a proportional font. Walk line.Words instead and use word.GetFont, keeping the tab-stop arithmetic. LineSegment.Words is null until the line has been highlighted (CountColumns guards against that too), and it may cover only part of the line, so anything left over is still measured per character with the regular font rather than being silently dropped. Also widen the cache invalidation comment: the cached widths depend on tab size and highlighting as well as on the font.

mstv

Looks sensible; have not run

mstv · 2026-07-28T20:54:44Z

+                        default:
+                            width += MeasureStringWidth(
+                                g,
+                                Document.GetText(lineOffset + measured, word.Length),


Why not word.Word as currentWord.Word in PaintLinePart?

mstv · 2026-07-28T20:55:43Z

+            if (words != null)
+                for (var i = 0; i < words.Count; i++)


style

Suggested change

if (words != null)

for (var i = 0; i < words.Count; i++)

if (words is not null)

{

for (var i = 0; i < words.Count; i++)

mstv · 2026-07-28T21:00:23Z

                }
+                finally
+                {
+                    measureGraphics?.Dispose();


Rather keep the instance for the entire lifetime of the TextAreaControl, i.e. readonly field _measureGraphics?

DraftingDreamer mentioned this pull request Jul 26, 2026

Diff: horizontal scrollbar does not cover lines containing wide characters (CJK, emoji) gitextensions/gitextensions#13200

Open

RussKie requested a review from Copilot July 26, 2026 08:06

Copilot started reviewing on behalf of RussKie July 26, 2026 08:07 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

mstv reviewed Jul 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(TextAreaControl): measure glyph width for the horizontal scroll extent - #63

fix(TextAreaControl): measure glyph width for the horizontal scroll extent#63
DraftingDreamer wants to merge 2 commits into
gitextensions:masterfrom
DraftingDreamer:fix/hscrollbar-measure-glyph-width

DraftingDreamer commented Jul 26, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

DraftingDreamer Jul 27, 2026

Uh oh!

DraftingDreamer Jul 27, 2026

Uh oh!

mstv left a comment

Uh oh!

mstv Jul 28, 2026

Uh oh!

mstv Jul 28, 2026

Uh oh!

mstv Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

font	`x`	U+4E2D	50 ASCII chars
Consolas Regular	8 px	14 px	400 px
Consolas Bold	8 px	15 px	400 px
Consolas Italic	8 px	14 px	400 px
Segoe UI Regular	6 px	14 px	310 px
Segoe UI Bold	8 px	15 px	355 px

Uh oh!

Conversation

DraftingDreamer commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Screenshots

Before

After

Test methodology

Notes for the reviewer

Test environment(s)

Merge strategy

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

DraftingDreamer Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

DraftingDreamer Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

mstv left a comment

Choose a reason for hiding this comment

Uh oh!

mstv Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

mstv Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

mstv Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DraftingDreamer commented Jul 26, 2026 •

edited

Loading