Bug 456354 - New text writes on top of sixel image instead of erasing part of the image
Summary: New text writes on top of sixel image instead of erasing part of the image
Status: REPORTED
Alias: None
Product: konsole
Classification: Applications
Component: emulation (show other bugs)
Version: 22.04.2
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Konsole Developer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-07-05 14:24 UTC by badnam3o.0
Modified: 2024-03-19 20:56 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
different behaviours with sixel images of konsole and xterm vt340 (34.56 KB, image/png)
2022-07-05 14:24 UTC, badnam3o.0
Details
screenshot-broken-tmux (460.76 KB, image/png)
2024-03-17 08:42 UTC, Arturo
Details

Note You need to log in before you can comment on or make changes to this bug.
Description badnam3o.0 2022-07-05 14:24:43 UTC
Created attachment 150416 [details]
different behaviours with sixel images of konsole and xterm vt340

SUMMARY
***
NOTE: If you are reporting a crash, please try to attach a backtrace with debug symbols.
See https://community.kde.org/Guidelines_and_HOWTOs/Debugging/How_to_create_useful_crash_reports
***
in other terminals such as foot, mlterm and xterm (emulating vt-340) , when new text is written where a sixel image is present, part of the image is erased. In Konsole, the text is written on top of the image instead. The erasing behaviour can be useful to TUI apps for deleting an image without needing to clear the entire screen. 

STEPS TO REPRODUCE
1.  print a sixel image
2.  move cursor to somewhere in the image and print a normal string

Here's a sample shell script with outputs in the attached picture
```
#!/bin/sh
clear
printf '\x1b[2;2H'
printf '\x1bP0;1;0q#0;2;50;49;99#0!255~-#0!255~-#0!255~-#0!255~-#0!255~-#0!255~-#0!255~-#0!255~-\x1b\\'
printf '\x1b[2;2H'
printf 'Sample Text Here'
printf '\n\n'
```

OBSERVED RESULT
Text written on top of image

EXPECTED RESULT
Text overwrites/erases part of the image

SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma: Arch Linux kernel 5.18.9-zen
(available in About System)
KDE Plasma Version: 5.25.2
KDE Frameworks Version: 5.95.0
Qt Version: 5.15.5

ADDITIONAL INFORMATION
Comment 1 badnam3o.0 2022-07-05 15:04:33 UTC
Explanation for the example script:
```
#!/bin/sh
clear

# move cursor to position (2, 2)
printf '\x1b[2;2H'
# print a purple rectangle using SIXEL
printf '\x1bP0;1;0q#0;2;50;49;99#0!255~-#0!255~-#0!255~-#0!255~-#0!255~-#0!255~-#0!255~-#0!255~-\x1b\\'

# move cursor back to position (2, 2)
printf '\x1b[2;2H'
# write some text
printf 'Sample Text Here'

printf '\n\n'
```
Comment 2 Matan Ziv-Av 2022-07-07 08:41:47 UTC
(In reply to badnam3o.0 from comment #0)
> Created attachment 150416 [details]
> different behaviours with sixel images of konsole and xterm vt340
> 
> in other terminals such as foot, mlterm and xterm (emulating vt-340) , when
> new text is written where a sixel image is present, part of the image is
> erased. In Konsole, the text is written on top of the image instead. The
> erasing behaviour can be useful to TUI apps for deleting an image without
> needing to clear the entire screen. 

This was a conscious decision by me. While the interaction between text and image you describe is common, or even universal, I do not think it was in any way defined formally in any standard.

In KDE, the default background color was always treated as transparent (see when a background image is used).

So an application may get the common behaviour of text hiding image by changing the background color to a similar color (like (1,1,1) instead of (0,0,0)), while having the option of mixing text and graphics.
Comment 3 alexkurisu+github 2023-12-02 04:48:18 UTC
(In reply to Matan Ziv-Av from comment #2)
> (In reply to badnam3o.0 from comment #0)
> > Created attachment 150416 [details]
> > different behaviours with sixel images of konsole and xterm vt340
> > 
> > in other terminals such as foot, mlterm and xterm (emulating vt-340) , when
> > new text is written where a sixel image is present, part of the image is
> > erased. In Konsole, the text is written on top of the image instead. The
> > erasing behaviour can be useful to TUI apps for deleting an image without
> > needing to clear the entire screen. 
> 
> This was a conscious decision by me. While the interaction between text and
> image you describe is common, or even universal, I do not think it was in
> any way defined formally in any standard.
> 
> In KDE, the default background color was always treated as transparent (see
> when a background image is used).
> 
> So an application may get the common behaviour of text hiding image by
> changing the background color to a similar color (like (1,1,1) instead of
> (0,0,0)), while having the option of mixing text and graphics.

Please, remove this change. Many applications rely on ability to erase sixels by writing text over it.
Comment 4 panjea.developers 2023-12-28 16:09:18 UTC
please keep this change, i'm making a TUI app, and have a special case for Konsole to draw extra text ontop of Sixels.
Maybe an escape sequence could be added to enable or disable this as a feature.  I also don't mind Matan's proposal that the TUI developer should just set the background color for the text before drawing ontop of a sixel to force erase of the sixel underneath.  

In other terminals, when drawing text ontop of sixels, i already set the background color to match the average sixel color, that way it more closely matches how Konsole works now.
Comment 5 Arturo 2024-03-17 08:42:47 UTC
Created attachment 167360 [details]
screenshot-broken-tmux
Comment 6 Arturo 2024-03-17 08:45:03 UTC
You should provide the common behaviour by default, and the modified behaviour by choice.  I expect this to break every application out there using sixel.  See TMUX in the attachment.  Not even scrolling works.

Can you point us to the bit of code in konsole where this choice was made, so I can patch my version?  Thanks!
Comment 7 Matan Ziv-Av 2024-03-19 20:56:01 UTC
This is far from a trivial change. This simple patch changes behaviour of sixel images, but breaks at least kitty images, terminal background image, and terminal background transparency.

To correctly implement this, it is needed to know at the time of drawing text background if underneath it is a sixel image, another image, transparent background, etc.

diff --git a/src/Screen.cpp b/src/Screen.cpp
index f5047d1f17..626292a098 100644
--- a/src/Screen.cpp
+++ b/src/Screen.cpp
@@ -2523,7 +2523,14 @@ void Screen::addPlacement(QPixmap pixmap,
     addPlacement(p);
     int needScroll = qBound(0, row + rows - _lines, rows);
     if (moveCursor && scrolling && needScroll > 0) {
-        scrollUp(needScroll);
+        if (!leaveText) {
+            for (int i = needScroll; i > 0; i--) {
+                scrollUp(1);
+                eraseBlock(_lines - 2, col, 1, cols);
+            }
+        } else {
+            scrollUp(needScroll);
+        }
     }
     if (moveCursor) {
         if (rows - needScroll - 1 > 0) {
@@ -2532,6 +2539,9 @@ void Screen::addPlacement(QPixmap pixmap,
         if (moveCursor == 2 || _cuX + cols >= _columns) {
             toStartOfLine();
             newLine();
+            if (!leaveText) {
+                eraseBlock(_lines - 2, col, rows, cols);
+            }
         } else {
             cursorRight(cols);
         }
diff --git a/src/terminalDisplay/TerminalPainter.cpp b/src/terminalDisplay/TerminalPainter.cpp
index f19b0e6245..399b09cb2b 100644
--- a/src/terminalDisplay/TerminalPainter.cpp
+++ b/src/terminalDisplay/TerminalPainter.cpp
@@ -823,6 +823,7 @@ void TerminalPainter::drawBelowText(QPainter &painter,
                 backgroundColor = background;
             }
             drawBG = backgroundColor != colorTable[DEFAULT_BACK_COLOR];
+            drawBG = true;
             if (style[x].rendition.f.transparent) {
                 drawBG = false;
             }