Skip to content

Fix two image erasure bugs #18855

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 29, 2025
Merged

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented Apr 29, 2025

Summary of the Pull Request

This PR fixes two cases where image content wasn't correctly erased when
overwritten.

  1. When legacy console APIs fill an area of the buffer using a starting
    coordinate and a length, the affected area could potentially wrap over
    multiple rows, but we were only erasing the overwritten image content on
    the first affected row.

  2. When copying an area of the buffer with text content over another
    area that contained image content, the image in the target area would
    sometimes not be erased, because we ignored the _eraseCells return
    value which indicated that the image slice needed to be removed.

References and Relevant Issues

The original code was from the Sixel implementation in PR #17421.

Validation Steps Performed

I've manually verified that these two cases are now working as expected.

PR Checklist

@DHowett
Copy link
Member

DHowett commented Apr 29, 2025

Thanks! I'm gonna mark this one up for servicing to 1.22 and 1.23. :)

@DHowett
Copy link
Member

DHowett commented Apr 29, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@carlos-zamora carlos-zamora enabled auto-merge (squash) April 29, 2025 21:54
@carlos-zamora carlos-zamora merged commit 08e76da into microsoft:main Apr 29, 2025
14 checks passed
@j4james j4james deleted the fix-sixel-erase branch May 3, 2025 11:45
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.22 Servicing Pipeline May 5, 2025
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.23 Servicing Pipeline May 5, 2025
DHowett pushed a commit that referenced this pull request May 5, 2025
This PR fixes two cases where image content wasn't correctly erased when
overwritten.

1. When legacy console APIs fill an area of the buffer using a starting
coordinate and a length, the affected area could potentially wrap over
multiple rows, but we were only erasing the overwritten image content on
the first affected row.

2. When copying an area of the buffer with text content over another
area that contained image content, the image in the target area would
sometimes not be erased, because we ignored the `_eraseCells` return
value which indicated that the image slice needed to be removed.

## References and Relevant Issues

The original code was from the Sixel implementation in PR #17421.

## Validation Steps Performed

I've manually verified that these two cases are now working as expected.

## PR Checklist
- [x] Closes #18568

(cherry picked from commit 08e76da)
Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgZ3XT4
Service-Version: 1.23
DHowett pushed a commit that referenced this pull request May 5, 2025
This PR fixes two cases where image content wasn't correctly erased when
overwritten.

1. When legacy console APIs fill an area of the buffer using a starting
coordinate and a length, the affected area could potentially wrap over
multiple rows, but we were only erasing the overwritten image content on
the first affected row.

2. When copying an area of the buffer with text content over another
area that contained image content, the image in the target area would
sometimes not be erased, because we ignored the `_eraseCells` return
value which indicated that the image slice needed to be removed.

## References and Relevant Issues

The original code was from the Sixel implementation in PR #17421.

## Validation Steps Performed

I've manually verified that these two cases are now working as expected.

## PR Checklist
- [x] Closes #18568

(cherry picked from commit 08e76da)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgZ3XUA
Service-Version: 1.22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Cherry Picked
Status: Cherry Picked
Development

Successfully merging this pull request may close these issues.

EraseCells should work on multiple rows
3 participants