D17744: New search result look, resurrected

Wed Dec 26 14:39:20 GMT 2018

mglb added a comment.

  >> 2. Font size change (Ctrl+Scroll, Ctrl++, Ctrl+-) during search is not handled
  > 
  > Not sure what you mean, it seems to work here.

  Sorry, I've checked again and you are right. Not sure how it did happen before.

  >> - What do you think about making the lines 1px tall, drawn on inner edge of the rectangle? This would match lines in Breeze widget style.
  > 
  > Not entirely sure what you mean.

  F6508690: 1px-line.png <https://phabricator.kde.org/F6508690>

  Code:

    if (drawSearchResults) {
        paint.setPen(QPen(QColor(255, 255, 255, 64), 1));
        paint.drawLine(QLineF(_searchResultRect.topLeft(), _searchResultRect.topRight()).translated(0, 0.5));
        paint.drawLine(QLineF(_searchResultRect.bottomLeft(), _searchResultRect.bottomRight()).translated(0, 0.5));
    }

  >> - Configurable colors, or colors taken from system theme/konsole theme
  > 
  > Tested this a bit first, but it's hard to make it work with all kinds of different application (`make menuconfig` is what broke it for me, but mc etc. is also good at breaking this).

  What about something like this (uses Konsole color scheme)?

  F6509810: color-from-palette.png <https://phabricator.kde.org/F6509810>

    diff --git a/src/TerminalDisplay.cpp b/src/TerminalDisplay.cpp
    index fb4ea1c4..b6bf1a4f 100644
    --- a/src/TerminalDisplay.cpp
    +++ b/src/TerminalDisplay.cpp
    @@ -1639,5 +1639,6 @@ void TerminalDisplay::paintEvent(QPaintEvent* pe)

    -        QColor shadeColor(0, 0, 0, 192);
    +        QColor shadeColor(_colorTable[DEFAULT_BACK_COLOR]);
    +        shadeColor.setAlpha(192);

             // Shade area above result
    @@ -1646,5 +1647,5 @@ void TerminalDisplay::paintEvent(QPaintEvent* pe)
             paint.fillRect(0, _searchResultRect.bottom() + 1, contentsRect().width(), contentsRect().height(), shadeColor);
             // Lightly shade line with results
    -        shadeColor.setAlpha(120);
    +        shadeColor.setAlpha(128);
             paint.fillRect(_searchResultRect, shadeColor);
         } else {
    @@ -1657,5 +1658,9 @@ void TerminalDisplay::paintEvent(QPaintEvent* pe)
         // from paintFilters because the lines potentially overlap some results
         if (drawSearchResults) {
    -        paint.setPen(QPen(QColor(255, 255, 255, 64), 2));
    +        CharacterColor ccFg(COLOR_SPACE_DEFAULT, DEFAULT_FORE_COLOR);
    +        ccFg.setIntensive();
    +        QColor fg = ccFg.color(_colorTable);
    +        fg.setAlpha(96);
    +        paint.setPen(QPen(fg, 2));
             paint.drawLine(QLine(_searchResultRect.topLeft(), _searchResultRect.topRight()));
             paint.drawLine(QLine(_searchResultRect.bottomLeft(), _searchResultRect.bottomRight()).translated(0, 1));

  bug: first line is selected by default
  --------------------------------------

  - Start Konsole
  - Ctrl+Shift+F

  Result: first line is highlighted

  F6508699: Screenshot_2018-12-25-15-49-25.png <https://phabricator.kde.org/F6508699>

  bug: shade size does not reach bottom of a window
  -------------------------------------------------

  - Start Konsole
  - Generate some text to have a few lines in history
  - Ctrl+Shift+F

  Result: bottom dim rectangle is not drawn to the bottom of a window. When you scroll up, you'll see the first line is highlighted and the shade is under it. This also happen when you search for something and scroll down (so the result line is out of screen).

  F6508713: Screenshot_2018-12-25-15-33-42.png <https://phabricator.kde.org/F6508713>

  bug: margins area repaints wrong
  --------------------------------

  - set margins to make them visible
  - generate some text (to have something in history)
  - highlight any line
  - scroll a bit

  Result: highlight on a margin is updated wrong.

  F6509820: Screenshot_2018-12-26-12-29-08.png <https://phabricator.kde.org/F6509820>

  Fix (horizontal lines changed to be drawn always on inner side of the rect):

    diff --git a/src/TerminalDisplay.cpp b/src/TerminalDisplay.cpp
    index fb4ea1c4..d548a62e 100644
    --- a/src/TerminalDisplay.cpp
    +++ b/src/TerminalDisplay.cpp
    @@ -1386,2 +1386,12 @@ void TerminalDisplay::processFilters()

    +inline QRect TerminalDisplay::calculateSearchResultRect() const
    +{
    +    const int searchLine = _screenWindow->currentResultLine() - _screenWindow->currentLine();
    +    QRect resultRect = imageToWidget(QRect(0, searchLine, _columns, 1))
    +                       .adjusted(0, -_fontHeight/3, 0, _fontHeight/3);
    +    resultRect.setLeft(0);
    +    resultRect.setRight(contentsRect().width());
    +    return resultRect;
    +}
    +
     void TerminalDisplay::updateImage()
    @@ -1559,4 +1569,3 @@ void TerminalDisplay::updateImage()
             // Highlight new result region
    -        dirtyRegion |= QRect(0, _contentRect.top() + (_screenWindow->currentResultLine() - _screenWindow->currentLine()) * _fontHeight,
    -                             _columns * _fontWidth, _fontHeight);
    +        dirtyRegion |= calculateSearchResultRect();
         }
    @@ -1632,9 +1641,3 @@ void TerminalDisplay::paintEvent(QPaintEvent* pe)
         if (drawSearchResults) {
    -        const int searchLine = _screenWindow->currentResultLine() - _screenWindow->currentLine();
    -        _searchResultRect = imageToWidget(QRect(0, searchLine, _columns, 1));
    -        _searchResultRect.setTop(_searchResultRect.top() - _fontHeight / 3);
    -        _searchResultRect.setBottom(_searchResultRect.bottom() + _fontHeight / 3);
    -        _searchResultRect.setLeft(0);
    -        _searchResultRect.setRight(contentsRect().width());
    -
    +        _searchResultRect = calculateSearchResultRect();

    @@ -1659,4 +1662,5 @@ void TerminalDisplay::paintEvent(QPaintEvent* pe)
             paint.setPen(QPen(QColor(255, 255, 255, 64), 2));
    -        paint.drawLine(QLine(_searchResultRect.topLeft(), _searchResultRect.topRight()));
    -        paint.drawLine(QLine(_searchResultRect.bottomLeft(), _searchResultRect.bottomRight()).translated(0, 1));
    +        const qreal lineOffset = paint.pen().width()/2.;
    +        paint.drawLine(QLineF(_searchResultRect.topLeft(), _searchResultRect.topRight()).translated(0, lineOffset));
    +        paint.drawLine(QLineF(_searchResultRect.bottomLeft(), _searchResultRect.bottomRight()).translated(0, 1 - lineOffset));
         }
    diff --git a/src/TerminalDisplay.h b/src/TerminalDisplay.h
    index 1b7dccc1..f22b8125 100644
    --- a/src/TerminalDisplay.h
    +++ b/src/TerminalDisplay.h
    @@ -921,2 +921,4 @@ private:

    +    QRect calculateSearchResultRect() const;
    +
         // the window onto the terminal screen which this display

  bug: highlight stays in same place when the screen content changes
  ------------------------------------------------------------------

  - start konsole
  - run `mc`
  - search for something; for best results highlight something between center and bottom of the window
  - click terminal to focus it
  - exit (f10)

  Result: the highlight rectangle is still where it was.

  F6509816: Screenshot_2018-12-26-15-29-26.png <https://phabricator.kde.org/F6509816>

REPOSITORY
  R319 Konsole

REVISION DETAIL
  https://phabricator.kde.org/D17744

To: sandsmark, hindenburg, #konsole, thsurrel, tcanabrava, #vdg
Cc: ngraham, mglb, konsole-devel, #konsole, thsurrel, maximilianocuria, hindenburg
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20181226/d6792c06/attachment-0001.html>