fix: clamp gutter indicator dimensions to prevent negative Rect (fixes #313727)#313794
Conversation
When layout.height is very small (< 4px), gutterHeightWithoutPadding becomes negative, causing Rect.fromLeftTopWidthHeight to throw 'Invalid arguments: Vertically offset by N'. Same for width. Clamp both dimensions to Math.max(0, ...) at the computation site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a crash in the inline edits gutter indicator layout computation by preventing invalid Rect construction when the editor is resized/collapsed to very small dimensions.
Changes:
- Clamp computed gutter viewport width/height to be non-negative before creating a
Rect.
Show a summary per file
| File | Description |
|---|---|
| src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/components/gutterIndicatorView.ts | Clamp gutter viewport dimensions to avoid negative sizes that can trigger Rect validation errors. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
| const gutterViewPortWithoutStickyScrollWithoutPaddingTop = gutterViewPortWithStickyScroll.withTop(this._stickyScrollHeight.read(reader)); | ||
| const gutterViewPortWithoutStickyScroll = gutterViewPortWithStickyScroll.withTop(gutterViewPortWithoutStickyScrollWithoutPaddingTop.top + gutterViewPortPaddingTop); |
Follow-up: Review Comment AnalysisTrigger: Review Comment from Copilot PR Reviewer
Suggested FixClamp the sticky-scroll top (and the subsequent const gutterViewPortWithStickyScroll = Rect.fromLeftTopWidthHeight(gutterViewPortPaddingLeft, gutterViewPortPaddingTop, gutterWidthWithoutPadding, gutterHeightWithoutPadding);
-const gutterViewPortWithoutStickyScrollWithoutPaddingTop = gutterViewPortWithStickyScroll.withTop(this._stickyScrollHeight.read(reader));
-const gutterViewPortWithoutStickyScroll = gutterViewPortWithStickyScroll.withTop(gutterViewPortWithoutStickyScrollWithoutPaddingTop.top + gutterViewPortPaddingTop);
+const stickyScrollTop = Math.min(this._stickyScrollHeight.read(reader), gutterViewPortWithStickyScroll.bottom);
+const gutterViewPortWithoutStickyScrollWithoutPaddingTop = gutterViewPortWithStickyScroll.withTop(stickyScrollTop);
+const gutterViewPortWithoutStickyScroll = gutterViewPortWithStickyScroll.withTop(Math.min(gutterViewPortWithoutStickyScrollWithoutPaddingTop.top + gutterViewPortPaddingTop, gutterViewPortWithStickyScroll.bottom));This ensures both Status
|
Summary
The
Rectconstructor inrect.tsthrowsBugIndicatingError("Invalid arguments: Vertically offset by 4")whengutterIndicatorView.tscomputes a negative height for the gutter viewport. Whenlayout.heightis very small (< 4px, e.g. during editor resize or collapse),gutterHeightWithoutPadding = layout.height - 2 * gutterViewPortPaddingTopbecomes negative, andRect.fromLeftTopWidthHeight(left, top, width, negativeHeight)producestop > bottom, triggering the validation error. This affects all platforms (Linux, Mac, Windows).Fixes #313727
Recommended reviewer:
@hedietCulprit Commit
Not identified — pre-existing pattern. This is the same class of bug previously fixed in #301197 (horizontal axis). The vertical axis was not guarded. The error resurfaced because the same computation path lacks clamping for both dimensions.
Code Flow
sequenceDiagram participant Layout as "EditorLayoutInfo" participant Gutter as "GutterIndicatorView._computeFn" participant RectFactory as "Rect.fromLeftTopWidthHeight" participant RectCtor as "Rect constructor" Layout->>Gutter: layout.height is small value Note over Gutter: gutterHeightWithoutPadding = 0 - 4 = -4 Gutter->>RectFactory: fromLeftTopWidthHeight with negative height RectFactory->>RectCtor: new Rect with top greater than bottom Note over RectCtor: throws BugIndicatingErrorAffected Files
src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/components/gutterIndicatorView.tssrc/vs/editor/common/core/2d/rect.tsRepro Steps
How the Fix Works
Chosen approach (
gutterIndicatorView.ts:348-349): Clamp bothgutterWidthWithoutPaddingandgutterHeightWithoutPaddingtoMath.max(0, ...)at the data-producer site. This follows the data-producer principle — fix where invalid data is created, not at the crash site (Rectconstructor). When the editor is too small to display the gutter, a zero-size rect is semantically correct (nothing to render).Alternatives considered: Adding a guard in the
Rectconstructor to clamp instead of throw — rejected because the constructor's validation catches real bugs elsewhere; silencing it would hide future errors across all callers.Recommended Owner
@hediet— owner of inline completions and gutter indicator code, reviewer on prior fix #301197