EraseCells should work on multiple rows

[carlos-zamora]

Thanks for raising this issue. The reason this occurs is because the color command uses the console FillConsoleOutputAttribute API to update the colors, and these legacy console APIs aren't intended to be mixed with VT operations like Sixel, so this is essentially undefined behavior.

In the current implementation the fill is treated as a destructive operation. When translated to VT it rewrites all the text using the new colors, and that erases any images that are overwritten. I think we did discuss the possibility of translating this API into a VT DECCARA sequence, which would be non-destructive, so it's possible this could be fixed (at least on some terminals).

Either way, there is definitely a bug evident here, because in conhost the color command currently doesn't erase the image, and that wasn't actually the intended behavior. I think the problem is in the call here:

terminal/src/host/_output.cpp

Lines 242 to 243 in f28f658

	// If we've overwritten image content, it needs to be erased.
	ImageSlice::EraseCells(screenInfo.GetTextBuffer(), startingCoordinate, result.cellsModified);

That EraseCells method is only capable of erasing a single row, but the APIs being dealt with here are designed to fill a range that could wrap over multiple rows. In the case of the color command, it's a single fill call that covers the entire buffer. So if you had a Sixel image at the very top of the buffer, you'd see just the first row of the image erased, but in most cases nothing would be erased.

Originally posted by @j4james in #18556