From 877bfe7586c1c33df93d9b6d5744785c7519f057 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 26 Sep 2024 13:39:19 +0100 Subject: [PATCH 1/2] Improve correctness of Sixel background fill --- src/terminal/adapter/SixelParser.cpp | 44 ++++++++++++++++++---------- src/terminal/adapter/SixelParser.hpp | 2 ++ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/terminal/adapter/SixelParser.cpp b/src/terminal/adapter/SixelParser.cpp index 979dd42371c..46407e2a54a 100644 --- a/src/terminal/adapter/SixelParser.cpp +++ b/src/terminal/adapter/SixelParser.cpp @@ -341,6 +341,11 @@ void SixelParser::_initRasterAttributes(const VTInt macroParameter, const Dispat // By default, the filled area will cover the maximum extent allowed. _backgroundSize = { til::CoordTypeMax, til::CoordTypeMax }; + + // If the requested background area can not initially be filled, we'll + // handle the rest of it on demand when the image is resized. But this + // will be determined later in the _fillImageBackground method. + _resizeFillRequired = false; } void SixelParser::_updateRasterAttributes(const VTParameters& rasterAttributes) @@ -647,6 +652,10 @@ void SixelParser::_resizeImageBuffer(const til::CoordType requiredHeight) { static constexpr auto transparentPixel = IndexedPixel{ .transparent = true }; _imageBuffer.resize(requiredSize, transparentPixel); + if (_resizeFillRequired) + { + _fillImageBackground(requiredHeight); + } } } @@ -656,25 +665,30 @@ void SixelParser::_fillImageBackground() { _backgroundFillRequired = false; - const auto backgroundHeight = std::min(_backgroundSize.height, _availablePixelHeight); - const auto backgroundWidth = std::min(_backgroundSize.width, _availablePixelWidth); - _resizeImageBuffer(backgroundHeight); - // When a background fill is requested, we prefill the buffer with the 0 // color index, up to the boundaries set by the raster attributes (or if - // none were given, up to the page boundaries). The actual image output - // isn't limited by the background dimensions though. - static constexpr auto backgroundPixel = IndexedPixel{}; - const auto backgroundOffset = _imageCursor.y * _imageMaxWidth; - auto dst = std::next(_imageBuffer.begin(), backgroundOffset); - for (auto i = 0; i < backgroundHeight; i++) - { - std::fill_n(dst, backgroundWidth, backgroundPixel); - std::advance(dst, _imageMaxWidth); - } + // none were given, up to the page boundaries). If the requested height + // is more than the available height, we'll continue filling the rest of + // it on demand when the image is resized (see above). + const auto backgroundHeight = std::min(_backgroundSize.height, _availablePixelHeight); + _resizeImageBuffer(backgroundHeight); + _fillImageBackground(backgroundHeight); + _resizeFillRequired = _backgroundSize.height > _availablePixelHeight; + } +} - _imageWidth = std::max(_imageWidth, backgroundWidth); +void SixelParser::_fillImageBackground(const int backgroundHeight) +{ + static constexpr auto backgroundPixel = IndexedPixel{}; + const auto backgroundWidth = std::min(_backgroundSize.width, _availablePixelWidth); + const auto backgroundOffset = _imageCursor.y * _imageMaxWidth; + auto dst = std::next(_imageBuffer.begin(), backgroundOffset); + for (auto i = 0; i < backgroundHeight; i++) + { + std::fill_n(dst, backgroundWidth, backgroundPixel); + std::advance(dst, _imageMaxWidth); } + _imageWidth = std::max(_imageWidth, backgroundWidth); } void SixelParser::_writeToImageBuffer(int sixelValue, int repeatCount) diff --git a/src/terminal/adapter/SixelParser.hpp b/src/terminal/adapter/SixelParser.hpp index 1966228d77f..0c9285881e4 100644 --- a/src/terminal/adapter/SixelParser.hpp +++ b/src/terminal/adapter/SixelParser.hpp @@ -89,6 +89,7 @@ namespace Microsoft::Console::VirtualTerminal til::CoordType _pendingTextScrollCount = 0; til::size _backgroundSize; bool _backgroundFillRequired = false; + bool _resizeFillRequired = false; void _initColorMap(const VTParameter backgroundColor); void _defineColor(const VTParameters& colorParameters); @@ -109,6 +110,7 @@ namespace Microsoft::Console::VirtualTerminal void _initImageBuffer(); void _resizeImageBuffer(const til::CoordType requiredHeight); void _fillImageBackground(); + void _fillImageBackground(const int backgroundHeight); void _writeToImageBuffer(const int sixelValue, const int repeatCount); void _eraseImageBufferRows(const int rowCount, const til::CoordType startRow = 0) noexcept; void _maybeFlushImageBuffer(const bool endOfSequence = false); From 12816830de3ec4fc315d100090780a7e2fcd3557 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 26 Jan 2025 12:04:59 +0000 Subject: [PATCH 2/2] Improve correctness of Sixel background fill (take 2) --- src/terminal/adapter/SixelParser.cpp | 55 ++++++++++++++++++++++------ src/terminal/adapter/SixelParser.hpp | 4 +- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/src/terminal/adapter/SixelParser.cpp b/src/terminal/adapter/SixelParser.cpp index 46407e2a54a..ac7fe4e389c 100644 --- a/src/terminal/adapter/SixelParser.cpp +++ b/src/terminal/adapter/SixelParser.cpp @@ -237,6 +237,7 @@ void SixelParser::_executeNextLine() _imageCursor.y += _sixelHeight; _availablePixelHeight -= _sixelHeight; _resizeImageBuffer(_sixelHeight); + _fillImageBackgroundWhenScrolled(); } void SixelParser::_executeMoveToHome() @@ -342,10 +343,11 @@ void SixelParser::_initRasterAttributes(const VTInt macroParameter, const Dispat // By default, the filled area will cover the maximum extent allowed. _backgroundSize = { til::CoordTypeMax, til::CoordTypeMax }; - // If the requested background area can not initially be filled, we'll - // handle the rest of it on demand when the image is resized. But this - // will be determined later in the _fillImageBackground method. - _resizeFillRequired = false; + // If the image ends up extending beyond the bottom of the page, we may need + // to perform additional background filling as the screen is scrolled, which + // requires us to track the area filled so far. This will be initialized, if + // necessary, in the _fillImageBackground method below. + _filledBackgroundHeight = std::nullopt; } void SixelParser::_updateRasterAttributes(const VTParameters& rasterAttributes) @@ -652,10 +654,6 @@ void SixelParser::_resizeImageBuffer(const til::CoordType requiredHeight) { static constexpr auto transparentPixel = IndexedPixel{ .transparent = true }; _imageBuffer.resize(requiredSize, transparentPixel); - if (_resizeFillRequired) - { - _fillImageBackground(requiredHeight); - } } } @@ -667,13 +665,17 @@ void SixelParser::_fillImageBackground() // When a background fill is requested, we prefill the buffer with the 0 // color index, up to the boundaries set by the raster attributes (or if - // none were given, up to the page boundaries). If the requested height - // is more than the available height, we'll continue filling the rest of - // it on demand when the image is resized (see above). + // none were given, up to the page boundaries). The actual image output + // isn't limited by the background dimensions though. const auto backgroundHeight = std::min(_backgroundSize.height, _availablePixelHeight); _resizeImageBuffer(backgroundHeight); _fillImageBackground(backgroundHeight); - _resizeFillRequired = _backgroundSize.height > _availablePixelHeight; + // When the image extends beyond the page boundaries, and the screen is + // scrolled, we also need to fill the newly exposed lines, so we keep a + // record of the area filled so far. Initially this is considered to be + // the available height, even if it wasn't all filled to start with. + _filledBackgroundHeight = _imageCursor.y + _availablePixelHeight; + _fillImageBackgroundWhenScrolled(); } } @@ -691,6 +693,33 @@ void SixelParser::_fillImageBackground(const int backgroundHeight) _imageWidth = std::max(_imageWidth, backgroundWidth); } +void SixelParser::_fillImageBackgroundWhenScrolled() +{ + // If _filledBackgroundHeight is set, that means a background fill has been + // requested, and we need to extend that area whenever the image is about to + // overrun it. The newly filled area is a multiple of the cell height (this + // is to match the behavior of the original hardware terminals). + const auto imageHeight = _imageCursor.y + _sixelHeight; + if (_filledBackgroundHeight && imageHeight > _filledBackgroundHeight) [[unlikely]] + { + _filledBackgroundHeight = (imageHeight + _cellSize.height - 1) / _cellSize.height * _cellSize.height; + const auto additionalFillHeight = _filledBackgroundHeight.value() - _imageCursor.y; + _resizeImageBuffer(additionalFillHeight); + _fillImageBackground(additionalFillHeight); + } +} + +void SixelParser::_decreaseFilledBackgroundHeight(const int decreasedHeight) +{ + // Sometimes the top of the image buffer may be clipped (e.g. when the image + // scrolls off the top of a margin area). When that occurs, our record of + // the filled height will need to be decreased to account for the new start. + if (_filledBackgroundHeight) [[unlikely]] + { + _filledBackgroundHeight = _filledBackgroundHeight.value() - decreasedHeight; + } +} + void SixelParser::_writeToImageBuffer(int sixelValue, int repeatCount) { // On terminals that support the raster attributes command (which sets the @@ -731,11 +760,13 @@ void SixelParser::_eraseImageBufferRows(const int rowCount, const til::CoordType const auto bufferOffsetEnd = bufferOffset + pixelCount * _imageMaxWidth; if (static_cast(bufferOffsetEnd) >= _imageBuffer.size()) [[unlikely]] { + _decreaseFilledBackgroundHeight(_imageCursor.y); _imageBuffer.clear(); _imageCursor.y = 0; } else { + _decreaseFilledBackgroundHeight(pixelCount); _imageBuffer.erase(_imageBuffer.begin() + bufferOffset, _imageBuffer.begin() + bufferOffsetEnd); _imageCursor.y -= pixelCount; } diff --git a/src/terminal/adapter/SixelParser.hpp b/src/terminal/adapter/SixelParser.hpp index 0c9285881e4..bc8355e327c 100644 --- a/src/terminal/adapter/SixelParser.hpp +++ b/src/terminal/adapter/SixelParser.hpp @@ -89,7 +89,7 @@ namespace Microsoft::Console::VirtualTerminal til::CoordType _pendingTextScrollCount = 0; til::size _backgroundSize; bool _backgroundFillRequired = false; - bool _resizeFillRequired = false; + std::optional _filledBackgroundHeight; void _initColorMap(const VTParameter backgroundColor); void _defineColor(const VTParameters& colorParameters); @@ -111,6 +111,8 @@ namespace Microsoft::Console::VirtualTerminal void _resizeImageBuffer(const til::CoordType requiredHeight); void _fillImageBackground(); void _fillImageBackground(const int backgroundHeight); + void _fillImageBackgroundWhenScrolled(); + void _decreaseFilledBackgroundHeight(const int decreasedHeight); void _writeToImageBuffer(const int sixelValue, const int repeatCount); void _eraseImageBufferRows(const int rowCount, const til::CoordType startRow = 0) noexcept; void _maybeFlushImageBuffer(const bool endOfSequence = false);