Problems with cut texts on resizing

[tsujan]

When the window is resized such that the old texts (= those before the last prompt) are cut, they can't be selected correctly, and so, they or their URLs can't be copied/opened correctly.

This is a result of lxqt/qterminal#76

[tsujan]

This isn't the same as lxqt/qterminal#76. Here, the problem is about copying (URLs) and opening URLs, and there may be a way of doing it without implementing what is called "text reflow" in that report — I don't think "text reflow" is a good idea, BTW.

[luebking]

Largely disregard the below, that part actually works. The mutt/ncurses-specific problem is that it wraps and re-flos internally, so the line isn't wrapped by qtermwidget

The bug is about shrinking qtermwidget what results in a truncated line buffer (so you get the link but it's now too short) - something I had not noticed because tmux re-flows text :)

---

qtermwidget/lib/Filter.cpp

Line 195 in 57b2539

// TODO - Use the "line wrapped" attribute associated with lines in a

But that comment is 16 years old and the subsequent lines introduced 15 years ago in the mega-patch 6ad5d70 seem to do exactly that and discriminate by the LINE_WRAPPED attribute which gets added a couple of times in Screen.cpp

Fwiw I don't think that actual reflow needs to be implemented, you only need to know whether that line was wrapped and account for that.
Ie should™ work, no idea why it doesn't but would come handy w/ urls in mail clients…

[luebking]

diff --git a/lib/Screen.cpp b/lib/Screen.cpp
index 064ac1e..fe136a0 100644
--- a/lib/Screen.cpp
+++ b/lib/Screen.cpp
@@ -80,7 +80,7 @@ Character Screen::defaultChar = Character(' ',
     selBegin(0), selTopLeft(0), selBottomRight(0),
     blockSelectionMode(false),
     effectiveForeground(CharacterColor()), effectiveBackground(CharacterColor()), effectiveRendition(0),
-    lastPos(-1)
+    lastPos(-1),_screenLineBuffer(nullptr), _screenLineBufferSize(-1)
 {
     lineProperties.resize(lines+1);
     for (int i=0;i<lines+1;i++)
@@ -97,6 +97,7 @@ Character Screen::defaultChar = Character(' ',
 Screen::~Screen()
 {
     delete[] screenLines;
+    delete[] _screenLineBuffer;
     delete history;
 }
 
@@ -480,6 +481,22 @@ void Screen::copyFromHistory(Character* dest, int startLine, int count) const
     }
 }
 
+Character *Screen::screenLineBuffer(int &fullCols)
+{
+    fullCols = columns;
+    for (int i = 0; i < lines; ++i)
+        fullCols = qMax(fullCols, screenLines[i].size());
+    if (_screenLineBufferSize != lines*fullCols)
+    {
+        delete[] _screenLineBuffer;
+        _screenLineBuffer = new Character[lines*fullCols];
+    }
+    for (int i = 0; i < lines; ++i)
+        for (int j = 0; j < fullCols; ++j)
+            _screenLineBuffer[i*fullCols+j] = screenLines[i].value(j,defaultChar);
+    return _screenLineBuffer;
+}
+
 void Screen::copyFromScreen(Character* dest , int startLine , int count) const
 {
     Q_ASSERT( startLine >= 0 && count > 0 && startLine + count <= lines );
diff --git a/lib/Screen.h b/lib/Screen.h
index 22e2997..32f1cef 100644
--- a/lib/Screen.h
+++ b/lib/Screen.h
@@ -386,6 +386,14 @@ public:
      */
     void getImage( Character* dest , int size , int startLine , int endLine ) const;
 
+    /**
+     * Updates and returns the current screen image.
+     * The result is an array of Characters of size [getLines()][_maximum_column_length_]
+     * which is maintained internally and must NOT be freed by the caller after use.
+     * This provides access to the truncated lines on screen
+     */
+    Character *screenLineBuffer(int &fullColumnCount);
+
     /**
      * Returns the additional attributes associated with lines in the image.
      * The most important attribute is LINE_WRAPPED which specifies that the
@@ -720,6 +728,8 @@ private:
     unsigned short lastDrawnChar;
 
     static Character defaultChar;
+    Character *_screenLineBuffer;
+    int _screenLineBufferSize;
 };
 
 }
diff --git a/lib/TerminalDisplay.cpp b/lib/TerminalDisplay.cpp
index 2d4c7d5..4b4177b 100644
--- a/lib/TerminalDisplay.cpp
+++ b/lib/TerminalDisplay.cpp
@@ -1171,10 +1171,9 @@ void TerminalDisplay::processFilters()
     // ScreenWindow emits a scrolled() signal - which will happen before
     // updateImage() is called on the display and therefore _image is
     // out of date at this point
-    _filterChain->setImage( _screenWindow->getImage(),
-                            _screenWindow->windowLines(),
-                            _screenWindow->windowColumns(),
-                            _screenWindow->getLineProperties() );
+    int fullCols;
+    Character *image = _screenWindow->screen()->screenLineBuffer(fullCols);
+    _filterChain->setImage( image, _screenWindow->windowLines(), fullCols, _screenWindow->getLineProperties() );
     _filterChain->process();
 
     QRegion postUpdateHotSpots = hotSpotRegion();

This works, but I kinda don't like copying around the screenlines from the QVector to the Character array (and keeping that second buffer around for that)
I'm not quite sure about the design, the Filter could just operate on the sparse line buffer from Screen and eg. use the screen boundaries as a soft constraint (and from there to the next word boundary)?

Do you have opinions or insights on this?

[tsujan]

Do you have opinions or insights on this?

Sorry, my focus is elsewhere for now, but I think you're quite right not to like keeping that buffer.

I trust your judgment.

[luebking]

No worries, take your time.
The concern is that this would imply some refactoring of the FilterChain and I'd need to know to what extent the API needs to be protected (ie. whether to keep the present as alternative or if anything besides qtermwidget.h is free game.