Release v2.4.4#5466
Merged
Merged
Conversation
NeedsDrawRect and the viewPortRelativeRegion parameter of SetNeedsDraw are now in viewport-LOCAL coords: (0, 0) is the top-left visible cell of the View's Viewport, independent of Viewport.Location (scroll offset, possibly negative under AllowNegativeX/Y). Previously the no-arg SetNeedsDraw() passed Viewport (with the scroll offset baked in), DoDrawAdornments escalation did the same, and the subview cascade compared parent's viewport-relative region to subview.Frame (in parent's content coords) without accounting for the parent's scroll. PR #5431 worked around the resulting inconsistency by gating its region-aware ClearViewport narrowing on "Viewport.Location == Point.Empty"; this commit normalizes the convention so that workaround can be removed in a follow-up commit. Changes: - SetNeedsDraw() no-arg now passes new (Point.Empty, Viewport.Size). - DoDrawAdornments NeedsDrawRect escalation does the same. - SetNeedsDraw(Rectangle) subview cascade translates the viewport-local region through: parent scroll -> content coords -> intersect with subview.Frame -> subview frame-local -> subtract subview adornment offset and subview scroll -> clip to subview viewport bounds. If the result is empty (dirty area is only in subview's adornments/scrolled- off content), fall back to a no-arg subview SetNeedsDraw so the subview redraws safely. - View.Layout.SetFrame translates the union(old, new) frame from SuperView content coords to SuperView viewport-local before passing to SuperView.SetNeedsDraw. - Update NeedsDrawRect_Is_Viewport_Relative test scrolled-frame assertions to reflect the new convention (NeedsDrawRect no longer acquires scroll-position offsets). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e normalized PR #5431 added a "Viewport.Location == Point.Empty" early-return to CanNarrowClearToNeedsDrawRect because the cascade in SetNeedsDraw produced rects in a different coordinate space than ViewportToScreen expects. Issue #5359 normalized NeedsDrawRect on viewport-local coordinates, so the conversion via ViewportToScreen is now safe regardless of the view's scroll state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Nine tests pin the viewport-local coordinate convention: - SetNeedsDraw() no-arg on a scrolled view stores (0, 0, W, H). - SetNeedsDraw() no-arg on a view with negative Viewport.Location stores (0, 0, W, H). - Subview cascade from a scrolled parent produces correct subview- viewport-local rect (parent scroll translated out). - Subview-with-padding cascade produces subview-viewport-local coords (adornment offset subtracted), not subview-frame-local. - Scrolled-subview cascade subtracts the subview's own scroll on top of the frame and adornment offsets. - Dirty region overlapping only the subview's adornment area falls back to a full subview SetNeedsDraw (the safe fallback). - Framework ClearViewport narrowing now works on scrolled views (the scrolled-view guard removal from this PR). - Zero-size viewport is a no-op. - Repeated small invalidations on a scrolled view accumulate as a viewport-local union. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ests CoPilot review of PR #5435 caught three issues: 1. (Blocking) The subview cascade in View.NeedsDraw.cs subtracted subview.Viewport.Location on top of the adornment offset when converting to subview-viewport-local. That's wrong — viewport-local coords are scroll-INDEPENDENT ((0, 0) is always the top-left visible cell regardless of scroll; scroll changes which content cell appears there, not the viewport-local coords themselves). Subtracting the scroll shifted the dirty rect to a different on-screen position than the cells that were actually dirtied. ViewportToScreen treats input as scroll-independent, so the bug would also corrupt the framework's narrowing in CanNarrowClearToNeedsDrawRect for the subview's later self-redraw. Fix: only subtract the adornment offset. 2. (Medium) The RegionAwareClearViewportTests.FrameworkDraw_ScrolledView_FallsBackToFullClear test documented the workaround guard removed by this PR. With the guard gone the test was passing accidentally because its dirty rect (1, 6, 3, 2) was outside the 5-row viewport visible area (Y=6 > 4), so the narrowing intersected to empty and fell through for an unrelated reason. Delete the test — NeedsDrawCoordTests's FrameworkNarrowing_NowWorks_OnScrolledView covers the correct behavior. Replace with a comment block explaining the removal. 3. (Low) SetFrame's scroll translation in View.Layout.cs was only exercised by tests with SuperView.Viewport.Location == Point.Empty (where the translation is a no-op). Add SetFrame_ScrolledSuperView_TranslatesInvalidationToViewportLocal to cover the scrolled case. Also update SetNeedsDraw_ScrolledSubview_... test: the previous assertion locked in the buggy shifted rect. With the cascade corrected, parent dirty viewport-local (2, 2, 4, 2) → subview viewport-local (2, 2, 4, 2) (same on-screen cells, just rendered by the subview now). Full suites still green: 17285 / 74 / 433. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Correct the step-4 cascade comment to say GetViewportOffsetFromFrame() is the combined Margin/Border/Padding inset (not just Padding), and update the plan to match the implemented contract — the cascade does not subtract the subview's Viewport.Location because viewport-local coordinates are scroll-independent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Copilot autofix in 491f190 opened <para> twice with a single closing tag. Remove the stray opening tag so the doc comment is well-formed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…zation Fixes #5359. Normalize NeedsDrawRect to viewport-local coordinates
Back-merge v2.4.3 from main into develop
Extract the self-content draw (Text + Content + local-context merge) into a private DrawSelfContent helper that owns the full _localDrawContext lifecycle for a self-redraw pass: create, populate, merge. Move the context creation out of the viewport-clear block — DoClearViewport only writes the shared context, so the local context isn't needed until DrawSelfContent, keeping create/populate /merge in one place. This replaces three separate `if (needsDrawSelf)` blocks (whose alignment had to be maintained by hand) with a clear/self split documented as intentional around DoDrawSubViews, plus an authoritative lifecycle comment on the _localDrawContext field describing why it is preserved across child-only passes (TransparentMouse CachedDrawnRegion contract from #5431). Behavior-preserving; draw order unchanged. Side effect: collapsing creation and use into one method lets nullable flow analysis prove _localDrawContext non-null, removing a CS8602 warning. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes #5433. Make View.Draw self-draw context flow explicit
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…acters (#5454) * Fixes #5453. Fix Windows VT input encoding for IME and non-ASCII characters * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Cleanup code * Apply suggested changes related with review feedback --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add regression coverage for raster command IDs, stale cell invalidation, and ToAnsi layering. Update raster buffer replacement/removal to dirty stale image cells and make ToAnsi emit raster images before text overlays. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… tig/sixel-integration-plan
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27bdcf071b
ℹ️ 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".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release v2.4.4
Version:
2.4.4NuGet Package:
Terminal.Gui 2.4.4What happens when this PR is merged
v2.4.4main→developwill be openedChecklist
2.4.4