Skip to content

Commit 93637f4

Browse files
christianparpartchristianparpart
committed
[terminal_renderer] Fixes Sixel rendering for images that show below but should be rendered above text (#831).
1 parent 94fb93f commit 93637f4

File tree

8 files changed

+83
-13
lines changed

8 files changed

+83
-13
lines changed

‎metainfo.xml

+1
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@
106106
<li>Fixes Sixel mode, when updating the color palette with a new color, that color must also be used for subsequent paints.</li>
107107
<li>Fixes vertical cursor movement for Sixel graphics with only newlines (#822).</li>
108108
<li>Fixes Sixel rendering for images with aspect ratios other than 1:1.</li>
109+
<li>Fixes Sixel rendering for images that show below but should be rendered above text (#831).</li>
109110
<li>Removes `images.sixel_cursor_conformance` config option.</li>
110111
</ul>
111112
</description>

‎src/terminal_renderer/ImageRenderer.cpp

+35-4
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,44 @@ void ImageRenderer::renderImage(crispy::Point _pos, ImageFragment const& fragmen
5454
return;
5555

5656
// clang-format off
57-
renderTile(atlas::RenderTile::X { _pos.x },
58-
atlas::RenderTile::Y { _pos.y },
59-
RGBAColor::White,
60-
*tileAttributes);
57+
pendingRenderTilesAboveText_.emplace_back(createRenderTile(atlas::RenderTile::X { _pos.x },
58+
atlas::RenderTile::Y { _pos.y },
59+
RGBAColor::White, *tileAttributes));
6160
// clang-format on
6261
}
6362

63+
void ImageRenderer::onBeforeRenderingText()
64+
{
65+
// We could render here the images that should go below text.
66+
}
67+
68+
void ImageRenderer::onAfterRenderingText()
69+
{
70+
// We render here the images that should go above text.
71+
72+
for (auto& tile: pendingRenderTilesAboveText_)
73+
textureScheduler().renderTile(std::move(tile));
74+
75+
pendingRenderTilesAboveText_.clear();
76+
}
77+
78+
void ImageRenderer::beginFrame()
79+
{
80+
assert(pendingRenderTilesAboveText_.empty());
81+
}
82+
83+
void ImageRenderer::endFrame()
84+
{
85+
if (!pendingRenderTilesAboveText_.empty())
86+
{
87+
// In case some image tiles are still pending but no text had to be rendered.
88+
89+
for (auto& tile: pendingRenderTilesAboveText_)
90+
textureScheduler().renderTile(std::move(tile));
91+
pendingRenderTilesAboveText_.clear();
92+
}
93+
}
94+
6495
Renderable::AtlasTileAttributes const* ImageRenderer::getOrCreateCachedTileAttributes(
6596
ImageFragment const& fragment)
6697
{

‎src/terminal_renderer/ImageRenderer.h

+9-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <terminal/Image.h>
1717

1818
#include <terminal_renderer/RenderTarget.h>
19+
#include <terminal_renderer/TextRenderer.h>
1920

2021
#include <crispy/FNV.h>
2122
#include <crispy/point.h>
@@ -51,7 +52,7 @@ struct ImageFragmentKey
5152
/// Image Rendering API.
5253
///
5354
/// Can render any arbitrary RGBA image (for example Sixel Graphics images).
54-
class ImageRenderer: public Renderable
55+
class ImageRenderer: public Renderable, public TextRendererEvents
5556
{
5657
public:
5758
ImageRenderer(GridMetrics const& gridMetrics, ImageSize cellSize);
@@ -70,8 +71,15 @@ class ImageRenderer: public Renderable
7071

7172
void inspect(std::ostream& output) const override;
7273

74+
void beginFrame();
75+
void endFrame();
76+
77+
void onBeforeRenderingText() override;
78+
void onAfterRenderingText() override;
79+
7380
private:
7481
AtlasTileAttributes const* getOrCreateCachedTileAttributes(ImageFragment const& fragment);
82+
std::vector<atlas::RenderTile> pendingRenderTilesAboveText_;
7583

7684
// private data
7785
//

‎src/terminal_renderer/RenderTarget.cpp

+13-5
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,10 @@ auto Renderable::sliceTileData(Renderable::TextureAtlas::TileCreateData const& c
9191
createData.metadata.fragmentShaderSelector);
9292
}
9393

94-
void Renderable::renderTile(atlas::RenderTile::X x,
95-
atlas::RenderTile::Y y,
96-
RGBAColor color,
97-
Renderable::AtlasTileAttributes const& attributes)
94+
atlas::RenderTile Renderable::createRenderTile(atlas::RenderTile::X x,
95+
atlas::RenderTile::Y y,
96+
RGBAColor color,
97+
Renderable::AtlasTileAttributes const& attributes)
9898
{
9999
auto renderTile = atlas::RenderTile {};
100100
renderTile.x = atlas::RenderTile::X { x };
@@ -105,5 +105,13 @@ void Renderable::renderTile(atlas::RenderTile::X x,
105105
renderTile.normalizedLocation = attributes.metadata.normalizedLocation;
106106
renderTile.targetSize = attributes.metadata.targetSize;
107107
renderTile.tileLocation = attributes.location;
108-
textureScheduler().renderTile(renderTile);
108+
return renderTile;
109+
}
110+
111+
void Renderable::renderTile(atlas::RenderTile::X x,
112+
atlas::RenderTile::Y y,
113+
RGBAColor color,
114+
Renderable::AtlasTileAttributes const& attributes)
115+
{
116+
textureScheduler().renderTile(createRenderTile(x, y, color, attributes));
109117
}

‎src/terminal_renderer/RenderTarget.h

+5
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,11 @@ class Renderable
185185
TileSliceIndex sliceIndex,
186186
atlas::TileLocation tileLocation);
187187

188+
atlas::RenderTile createRenderTile(atlas::RenderTile::X x,
189+
atlas::RenderTile::Y y,
190+
RGBAColor color,
191+
Renderable::AtlasTileAttributes const& attributes);
192+
188193
void renderTile(atlas::RenderTile::X x,
189194
atlas::RenderTile::Y y,
190195
RGBAColor color,

‎src/terminal_renderer/Renderer.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ Renderer::Renderer(PageSize pageSize,
146146
backgroundOpacity_ { backgroundOpacity },
147147
backgroundRenderer_ { gridMetrics_, colorPalette.defaultBackground },
148148
imageRenderer_ { gridMetrics_, cellSize() },
149-
textRenderer_ { gridMetrics_, *textShaper_, fontDescriptions_, fonts_ },
149+
textRenderer_ { gridMetrics_, *textShaper_, fontDescriptions_, fonts_, imageRenderer_ },
150150
decorationRenderer_ { gridMetrics_, hyperlinkNormal, hyperlinkHover },
151151
cursorRenderer_ { gridMetrics_, CursorShape::Block }
152152
{
@@ -313,6 +313,7 @@ uint64_t Renderer::render(Terminal& _terminal, bool _pressure)
313313
#endif // }}}
314314

315315
optional<terminal::RenderCursor> cursorOpt;
316+
imageRenderer_.beginFrame();
316317
textRenderer_.beginFrame();
317318
textRenderer_.setPressure(_pressure && _terminal.isPrimaryScreen());
318319
{
@@ -322,6 +323,7 @@ uint64_t Renderer::render(Terminal& _terminal, bool _pressure)
322323
renderLines(renderBuffer.get().lines);
323324
}
324325
textRenderer_.endFrame();
326+
imageRenderer_.endFrame();
325327

326328
if (cursorOpt && cursorOpt.value().shape != CursorShape::Block)
327329
{

‎src/terminal_renderer/TextRenderer.cpp

+6-1
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,10 @@ constexpr uint32_t TextShapingCacheSize = 4000;
305305
TextRenderer::TextRenderer(GridMetrics const& gridMetrics,
306306
text::shaper& _textShaper,
307307
FontDescriptions& _fontDescriptions,
308-
FontKeys const& _fonts):
308+
FontKeys const& _fonts,
309+
TextRendererEvents& eventHandler):
309310
Renderable { gridMetrics },
311+
textRendererEvents_{ eventHandler },
310312
fontDescriptions_ { _fontDescriptions },
311313
fonts_ { _fonts },
312314
textShapingCache_ { ShapingResultCache::create(crispy::StrongHashtableSize { 16384 },
@@ -633,6 +635,8 @@ void TextRenderer::flushTextClusterGroup()
633635
// _gridMetrics.cellSize.width,
634636
// textClusterGroup_.codepoints.size());
635637

638+
textRendererEvents_.onBeforeRenderingText();
639+
636640
auto hash = hashTextAndStyle(
637641
u32string_view(textClusterGroup_.codepoints.data(), textClusterGroup_.codepoints.size()),
638642
textClusterGroup_.style);
@@ -679,6 +683,7 @@ void TextRenderer::flushTextClusterGroup()
679683
pen.x += static_cast<decltype(pen.x)>(advanceX);
680684
}
681685
}
686+
textRendererEvents_.onAfterRenderingText();
682687
}
683688

684689
textClusterGroup_.resetAndMovePenForward(textClusterGroup_.cellCount

‎src/terminal_renderer/TextRenderer.h

+11-1
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,23 @@ struct FontKeys
5656
text::font_key emoji;
5757
};
5858

59+
struct TextRendererEvents
60+
{
61+
virtual ~TextRendererEvents() = default;
62+
63+
virtual void onBeforeRenderingText() = 0;
64+
virtual void onAfterRenderingText() = 0;
65+
};
66+
5967
/// Text Rendering Pipeline
6068
class TextRenderer: public Renderable
6169
{
6270
public:
6371
TextRenderer(GridMetrics const& gridMetrics,
6472
text::shaper& textShaper,
6573
FontDescriptions& fontDescriptions,
66-
FontKeys const& fontKeys);
74+
FontKeys const& fontKeys,
75+
TextRendererEvents& textRendererEventHandler);
6776

6877
void setRenderTarget(RenderTarget& renderTarget, DirectMappingAllocator& directMappingAllocator) override;
6978
void setTextureAtlas(TextureAtlas& atlas) override;
@@ -135,6 +144,7 @@ class TextRenderer: public Renderable
135144

136145
// general properties
137146
//
147+
TextRendererEvents& textRendererEvents_;
138148
FontDescriptions& fontDescriptions_;
139149
FontKeys const& fonts_;
140150

0 commit comments

Comments
 (0)