Skip to content

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

@azure-pipelines
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
@DHowett DHowett moved this from Cherry Picked to Shipped in 1.23 Servicing Pipeline Sep 5, 2025
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

Development

Successfully merging this pull request may close these issues.

EraseCells should work on multiple rows

3 participants