Fix outline repeated on padding rows from simple strip cache#6572
Open
ShubhamPhapale wants to merge 1 commit into
Open
Fix outline repeated on padding rows from simple strip cache#6572ShubhamPhapale wants to merge 1 commit into
ShubhamPhapale wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a regression test and accompanying rendering fix to prevent top/bottom outlines from being incorrectly repeated across padding rows, and documents the fix in the changelog.
Changes:
- Added a snapshot regression test covering
outline+ vertical padding rendering (issue #6558). - Updated the styles cache logic to avoid reusing the “simple strip” cache when top/bottom outlines are involved.
- Added a changelog entry under “Unreleased” for the fix.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/snapshot_tests/test_snapshots.py | Adds a regression snapshot test to catch outline duplication across padding rows. |
| tests/snapshot_tests/snapshots/test_snapshots/test_outline_with_padding.svg | Adds the expected rendered output snapshot for the new regression test. |
| src/textual/_styles_cache.py | Adjusts simple-strip caching behavior to prevent incorrect reuse with top/bottom outlines. |
| CHANGELOG.md | Documents the fix in the Unreleased changelog section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+325
to
+328
| # The simple-strip cache assumes every padding row is identical. A top | ||
| # or bottom outline is drawn on a single padding row (the first or | ||
| # last), so the cache must not be used when either is present. | ||
| reuse_simple_strip = not (outline_top or outline_bottom) |
Comment on lines
+435
to
+438
| if reuse_simple_strip: | ||
| if self._simple_strip is not None: | ||
| return self._simple_strip | ||
| cache_simple_strip = True |
Comment on lines
+8
to
+12
| ## Unreleased | ||
|
|
||
| ### Fixed | ||
|
|
||
| - Fixed top/bottom `outline` being repeated on padding rows due to the simple strip cache https://github.com/Textualize/textual/issues/6558 |
The styles cache reuses a cached padding strip for all padding rows, but it was cached after outline processing. With a top/bottom outline, the first padding row carries the outline's top edge, which was then repeated on every padding row (and the bottom edge was lost). Disable the simple-strip cache when a top or bottom outline is present; rows with no outline or only left/right outlines stay uniform and keep the optimization. Fixes Textualize#6558
ShubhamPhapale
force-pushed
the
ShubhamPhapale/fix/outline-padding-strip-cache
branch
from
22a5aaa to
03d11c9
Compare
Author
|
Thanks for the review.
|
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.
Fixes #6558
The "optimize styles cache render" change (8ae3b5f) caches a "simple" padding
strip and reuses it for every padding row. It's cached after outline
processing, so for a widget with an
outlineand padding, the first paddingrow — which carries the outline's top edge — gets cached and repeated on all
padding rows, while the bottom edge is dropped.
A top or bottom outline lands on a single padding row, so those rows aren't
uniform. This disables the simple-strip cache when
outline_toporoutline_bottomis set. The no-outline and left/right-only-outline cases keepuniform padding rows, so the optimization still applies there.
Added a snapshot regression test (fails on
mainwithout this change).LinkedIn: https://www.linkedin.com/in/shubham-phapale/