PATCH: don't tinker with the palette or use hard-coded colors in grepview part
Matthew Woehlke
mw_triad at users.sourceforge.net
Tue Dec 12 22:53:13 UTC 2006
Matthew Woehlke wrote:
> Matthew Woehlke wrote:
>> Matthew Woehlke wrote:
>>> ProcessWidget is tinkering with its palette, causing two undesirable
>>> effects:
>>>
>>> 1. The colors it chooses are commonly indistinguishable from the
>>> window background (i.e. in any scheme where background() == mid(),
>>> e.g. '[Digital] CDE', 'Dark Blue', 'Solaris', 'High Contrast Black')
>>>
>>> 2. Styles that draw using the highlight color draw incorrectly.
>>>
>>> ...not to mention that hard-coding of colors as done by several parts
>>> (grepview, to name one) is A Bad Thing; these need to be configurable
>>> somehow (for a REALLY nasty example, consider the effect of using
>>> 'High Contrast Blue'). Is this being done in KDevelop4? (I wonder if
>>> this sort of thing should be system-wide - and more importantly,
>>> scheme specific - ala
>>> <https://bugs.kde.org/show_bug.cgi?id=136719>...) KATE gets away with
>>> its application scheme system because you are never combining a
>>> system scheme color with a KATE scheme color. The alternative of
>>> course would be for ProcessWidget to do likewise; draw the list
>>> itself (NOT THE SCROLLBARS!) and use an application scheme background
>>> color rather than the system scheme color.
>>>
>>> Is the palette changing really needed, or can it be removed?
>>
>> No, it isn't... that's what QListBoxItem::setCustomHighlighting is
>> for. I'm having problems with getting the width right
>> (QListBoxItem::width gives text width, not control width - I just
>> don't have time to dig until tomorrow), but once I can fix that, I'll
>> have a patch that draws ProcessListBoxItem and GrepListBoxItem using
>> strictly palette colors (I use QColorGroup::link and
>> QColorGroup::linkVisited, and blending of colors to get more *safe*
>> colors than just QColorGroup::text).
>
> Since I haven't heard anything else, I'm posting a patch to fix these
> issues. It leverages link colors and some color blending to continue to
> have the same set of distinct colors without the need to use hard-coded,
> out-of-scheme colors. It also drops the palette tinkering and instead
> uses custom drawing to alter the selection background color. Can this
> just be applied or should I open a few bugs for tracking purposes as well?
>
> Note that any subclasses of ProcessListBoxItem need to be updated to do
> the same drawing, or the 'selected' drawing will be lost. Optimally this
> should probably be fixed by providing a paintBackground() method in
> ProcessListBoxItem that shall ("shall" = prescriptive) be called by
> paint() in derived classes. This is a trivial change; I can make
> additional patches if someone wants to point out classes that are affected?
>
>
> ------------------------------------------------------------------------
>
> Index: parts/grepview/grepviewwidget.cpp
> ===================================================================
> --- parts/grepview/grepviewwidget.cpp (revision 609464)
> +++ parts/grepview/grepviewwidget.cpp (working copy)
> @@ -69,19 +69,45 @@
>
> void GrepListBoxItem::paint(QPainter *p)
> {
> + QColor base, dim, result, bkground;
> + if (listBox()) {
> + const QColorGroup& group = listBox()->palette().active();
> + if (isSelected()) {
> + bkground = group.button();
> + base = group.buttonText();
> + }
> + else
> + {
> + bkground = group.base();
> + base = group.text();
> + }
> + dim = blend(base, bkground);
> + result = group.link();
> + }
> + else
> + {
> + base = Qt::black;
> + dim = Qt::darkGreen;
> + result = Qt::blue;
> + if (isSelected())
> + bkground = Qt::lightGray;
> + else
> + bkground = Qt::white;
> + }
> QFontMetrics fm = p->fontMetrics();
> QString stx = lineNumber + ": ";
> int y = fm.ascent()+fm.leading()/2;
> int x = 3;
> + p->fillRect(p->window(), QBrush(bkground));
> if (show)
> {
> - p->setPen(Qt::darkGreen);
> + p->setPen(dim);
> p->drawText(x, y, fileName);
> x += fm.width(fileName);
> }
> else
> {
> - p->setPen(Qt::black);
> + p->setPen(base);
> QFont font1(p->font());
> QFont font2(font1);
> font2.setBold(true);
> @@ -90,7 +116,7 @@
> p->setFont(font1);
> x += fm.width(stx);
>
> - p->setPen(Qt::blue);
> + p->setPen(result);
> p->drawText(x, y, text);
> }
> }
> Index: lib/widgets/processwidget.cpp
> ===================================================================
> --- lib/widgets/processwidget.cpp (revision 609464)
> +++ lib/widgets/processwidget.cpp (working copy)
> @@ -31,7 +31,9 @@
>
> ProcessListBoxItem::ProcessListBoxItem(const QString &s, Type type)
> : QListBoxText(s), t(type)
> -{}
> +{
> + setCustomHighlighting(true);
> +}
>
>
> bool ProcessListBoxItem::isCustomItem()
> @@ -39,11 +41,58 @@
> return false;
> }
>
> +static inline unsigned char normalize(int a)
> +{
> + return (a < 0 ? 0 : a > 255 ? 255 : a);
> +}
> +
> +static inline double blend1(double a, double b, double k)
> +{
> + return a + (b - a) * k;
> +}
> +
> +QColor ProcessListBoxItem::blend(const QColor &c1, const QColor &c2, double k) const
> +{
> + if (k < 0.0) return c1;
> + if (k > 1.0) return c2;
> +
> + int r = normalize((int)blend1((double)c1.red(), (double)c2.red(), k));
> + int g = normalize((int)blend1((double)c1.green(), (double)c2.green(), k));
> + int b = normalize((int)blend1((double)c1.blue(), (double)c2.blue(), k));
> +
> + return QColor(qRgb(r, g, b));
> +}
>
> void ProcessListBoxItem::paint(QPainter *p)
> {
> - p->setPen((t==Error)? Qt::darkRed :
> - (t==Diagnostic)? Qt::black : Qt::darkBlue);
> + QColor dim, warn, err, back;
> + if (listBox()) {
> + const QColorGroup& group = listBox()->palette().active();
> + if (isSelected()) {
> + back = group.button();
> + warn = group.buttonText();
> + }
> + else
> + {
> + back = group.base();
> + warn = group.text();
> + }
> + err = group.linkVisited();
> + dim = blend(warn, back);
> + }
> + else
> + {
> + warn = Qt::black;
> + dim = Qt::darkBlue;
> + err = Qt::darkRed;
> + if (isSelected())
> + back = Qt::lightGray;
> + else
> + back = Qt::white;
> + }
> + p->fillRect(p->window(), QBrush(back));
> + p->setPen((t==Error)? err :
> + (t==Diagnostic)? warn : dim);
> QListBoxText::paint(p);
> }
>
> @@ -52,12 +102,9 @@
> : KListBox(parent, name)
> {
> setFocusPolicy(QWidget::NoFocus);
> - QPalette pal = palette();
> - pal.setColor(QColorGroup::HighlightedText,
> - pal.color(QPalette::Normal, QColorGroup::Text));
> - pal.setColor(QColorGroup::Highlight,
> - pal.color(QPalette::Normal, QColorGroup::Mid));
> - setPalette(pal);
> +
> + // Don't override the palette, as that can mess up styles. Instead, draw
> + // the background ourselves (see ProcessListBoxItem::paint).
>
> childproc = new KProcess();
> childproc->setUseShell(true);
> Index: lib/widgets/processwidget.h
> ===================================================================
> --- lib/widgets/processwidget.h (revision 609464)
> +++ lib/widgets/processwidget.h (working copy)
> @@ -42,6 +42,9 @@
>
> virtual bool isCustomItem();
>
> +protected:
> + QColor blend(const QColor &c1, const QColor &c2, double k = 0.5) const;
> +
> private:
> virtual void paint(QPainter *p);
> Type t;
No comments?
--
Matthew
Hi! I'm a .signature virus! Copy me into your ~/.signature, please!
More information about the KDevelop-devel
mailing list