D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

This revision was automatically updated to reflect the committed changes. Closed by commit R216:bbc465981bd5: SyntaxHighlighter: Fix foreground color for text without special highlighting (authored by dhaumann). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D21295?vs=58315=58511#toc

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

cullmann accepted this revision. cullmann added a comment. This revision is now accepted and ready to land. Given nobody proposes a working alternative, lets go with this. REPOSITORY R216 Syntax Highlighting BRANCH fix-syntax-highlighter-default-color REVISION DETAIL

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

cullmann added a comment. :=) You are right, without that, it might be even faster as we only do it once. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D21295 To: dhaumann, vkrause, cullmann, kossebau Cc: kwrite-devel, kde-frameworks-devel, domson,

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

dhaumann added a comment. @cullmann I don't see how we can optimize unneed `hasXXX` calls away: `Format::isDefaultTextStyle(const Theme )` internally uses the same 'hasXXX' calls, i.e. if we use both, we do it twice. Right now I don't see a trivial way to improve the patch. REPOSITORY

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

kossebau added a comment. I can confirm that this fixes the thumbnail bug for me: when I change the color theme, get rid of current thumbnail process (`killall thumbnail.so`) and trigger a reload of text thumbnails in Dolphin, I now always get consistently same colors with enough contrast

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

dhaumann added a comment. Well, this change only affects the SyntaxHighlighter, which KTextEditor does not use ;) For completeness: https://code.woboq.org/qt5/qtbase/src/gui/text/qtextdocument.cpp.html#673 Here, we can see that QTextDocument::drawContents() internally uses a

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

cullmann added a comment. I think one reason for the early out was that actually even the "isXXX" checks are very expensive, at least if I remember correctly they did show up a lot in my profiling in the past. Perhaps one should profile this once more (and if still visible) at least skip

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

dhaumann added a comment. The text thumbnailer was added in: D19432 . REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D21295 To: dhaumann, vkrause, cullmann, kossebau Cc: kwrite-devel, kde-frameworks-devel, domson,

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

dhaumann created this revision. dhaumann added reviewers: vkrause, cullmann, kossebau. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. dhaumann requested review of this revision. REVISION SUMMARY QTextDocument seems to use the widget