Issue with sixel rendering: stretched bands

[PhMajerus]

Windows Terminal version

Canary 1.22.2261.0

Windows build number

10.0.22631.4037 x64

Description of the problem

I believe I may have found an issue with sixels images rendering.

When displaying sixels images with a transparent background (by using P0;1 and not drawing some pixels in any of the colors), it renders perfectly when displayed on its own.

But when it gets close to another sixel image, added either on the previous line, or on the second line of the multi-line image with transparency, its width gets weird.

When the other sixel image is added above, the first band/line of the multi-line image gets slightly stretched.

When the other sixel image is appended after the second line of the multi-line image, that band gets stretched.

Note this also seems to happen in conhost, but with less visible stretching (only noticeable at some font sizes).

Steps to reproduce

Here is the repro text file: test.txt

Expected Behavior

The sixel images to keep their dimensions regardless of other contents displayed in the terminal.

Actual Behavior

Sixel images sometimes get distorted with some bands stretched when other contents is displayed near them.

[DHowett]

@lhecker for Atlas

@PhMajerus do you by chance know whether you’re using the D2D or D3D backend?

I think you can test it by switching to a font with a long single-glyph ligature (Cascadia Code works well) and trying to color it in multiple colors:

\e[31m<\e[32m!\e[33m-\e[34m-\e[m

If it shows up in four colors, you’re on D3D. One color, D2D.

[lhecker]

This happens with both D2D and D3D. It's a lot easier to test now that we can switch between them manually in the "Rendering" settings.

[PhMajerus]

@DHowett the <!-- test only works when using a font that has ligatures. Most users, me included, probably use the Cascadia Mono version in Terminal.
I tested with both D2D and D3D using the manual setting in Rendering settings, and as @lhecker said, the problem happens with both, and even with the GDI? renderer in conhost.

[DHowett]

@DHowett the <!-- test only works when using a font that has ligatures.

I did try to call that out. ;)

[PhMajerus]

Oops, sorry, forgot you mentioned switching to Cascadia Code in the meantime while I was testing.

BTW, showing which rendering engine got selected when set to Automatic in Settings would be nice to have, like a label below the description text that shows the engine currently in use. It would provide a more reliable way to find out without being too intrusive or technical, since the Rendering settings page is pretty much all about those advanced tweaks anyway.

[j4james]

I think what's happening here is that the first image is actual 24 pixels high, due to the nature of sixel images (they're always a multiple of 6). This means it extends over two lines, and the second line's image slice is thus preallocated with a width of 40, even though it's entirely transparent at that point.

Then when you write the second image immediately below that, it's actually overlapping the first, so the first image slice for that image will have a width of 40, but the second slice will have its expected width of 32.

When it comes time to rendering, we've now got two lines that look like they ought to be the same, but the image slice for the first line has additional transparent padding that's making it wider. Ideally this shouldn't be a problem, but I think there are possibly rounding errors when we scale the image slices that results in them being misaligned.

For the GDI renderer in particular, this calculation looks wrong.

terminal/src/renderer/gdi/paint.cpp

Lines 687 to 688 in 9c14367

	const auto dstWidth = srcWidth * dstCellSize.width / srcCellSize.width;
	const auto dstHeight = srcHeight * dstCellSize.height / srcCellSize.height;

I'm assuming it should be rounding, rather than truncating, although it may be more complicated than that.

I don't know if the Atlas engine is affected in the same way, or there's something else involved there.

[j4james]

On further investigation, I think the solution may be much simpler. The rounding shouldn't be an issue if the image slices were always a multiple of the virtual cell width, and they already would be if it weren't for this line:

terminal/src/buffer/out/ImageSlice.cpp

Line 68 in 1511d2c

_pixelWidth = (_pixelWidth + 3) & ~3; // Renderer needs this as a multiple of 4

But I'm not sure that's actually needed. I think it's the byte width of the image buffer that needs to be a multiple of 4, and that's already going to be the case if we're storing our pixels as RGBQUAD. I suspect this code is just left over from when I was experimenting with a palette renderer, which would have had just one byte per pixel.

I need to do some more tests to confirm, but dropping that line seems to fix the issue in both the GDI and Atlas renderers.

[lhecker]

At least the CreateBitmap documentation states:

Each scan line in the rectangle must be word aligned (scan lines that are not word aligned must be padded with zeros).

So, I think removing that line should be safe.

[PhMajerus]

I think it's the byte width of the image buffer that needs to be a multiple of 4, and that's already going to be the case if we're storing our pixels as RGBQUAD.

Yeah, the Windows DIB format uses a whole number of bytes per line. A monochrome DIB that's 12px wide will use 2 bytes and waste 4 bits per line.
If you use a 256 colors palette or RGBQUADs, you'll always use whole bytes already.
I need to check if it has to be a multiple of 4, I don't remember that part.