Qt: Add antialiasing to traffic graph widget

JosuGZ commented at 4:49 pm on June 5, 2019: contributor

Before:

After:

fanquake added the label GUI on Jun 5, 2019

in src/qt/trafficgraphwidget.cpp:68 in a89c808af5 outdated

64@@ -65,6 +65,7 @@ void TrafficGraphWidget::paintPath(QPainterPath &path, QQueue<float> &samples)
65 void TrafficGraphWidget::paintEvent(QPaintEvent *)
66 {
67     QPainter painter(this);
68+    painter.setRenderHints(painter.renderHints() | QPainter::Antialiasing);

promag commented at 5:00 pm on June 5, 2019:

Not sure if relevant but could be after fill?

JosuGZ commented at 6:55 pm on June 5, 2019:

I believe configuration should come before usage.

promag commented at 2:20 pm on June 6, 2019:

You can change render hints at any time.

Anyway, you can simplify this:

0painter.setRenderHint(QPainter::Antialiasing);

JosuGZ commented at 4:40 pm on June 6, 2019:

You can, but should you?

Your suggestion is good, pushing.

promag commented at 4:53 pm on June 6, 2019:

You can, but should you?

It depends if fillRect is faster without antialiasing - in this case it might make no difference as the rect is aligned and fills the entire surface.

More generally when you “paint/compose” an image you might want some parts/primitives antialiased and others not.

promag commented at 6:22 pm on June 5, 2019: member

Concept ACK. Can you check if rounded join style looks better? See https://doc.qt.io/qt-5/qpen.html#join-style.

JosuGZ commented at 7:11 pm on June 5, 2019: contributor

Concept ACK. Can you check if rounded join style looks better? See https://doc.qt.io/qt-5/qpen.html#join-style.

That is a different part of the code. I can’t really see the difference, probably because the points are too close. It looks like this:

I can push the change anyway.

 0diff --git a/src/qt/trafficgraphwidget.cpp b/src/qt/trafficgraphwidget.cpp
 1index 97a8599fb..4c245ad0c 100644
 2--- a/src/qt/trafficgraphwidget.cpp
 3+++ b/src/qt/trafficgraphwidget.cpp
 4@@ -109,14 +109,18 @@ void TrafficGraphWidget::paintEvent(QPaintEvent *)
 5         QPainterPath p;
 6         paintPath(p, vSamplesIn);
 7         painter.fillPath(p, QColor(0, 255, 0, 128));
 8-        painter.setPen(Qt::green);
 9+        QPen pen(Qt::green);
10+        pen.setJoinStyle(Qt::RoundJoin);
11+        painter.setPen(pen);
12         painter.drawPath(p);
13     }
14     if(!vSamplesOut.empty()) {
15         QPainterPath p;
16         paintPath(p, vSamplesOut);
17         painter.fillPath(p, QColor(255, 0, 0, 128));
18-        painter.setPen(Qt::red);
19+        QPen pen(Qt::red);
20+        pen.setJoinStyle(Qt::RoundJoin);
21+        painter.setPen(pen);
22         painter.drawPath(p);
23     }
24 }

If I have time I might play a bit with the whole graph to make it better though. That was initially my idea but this fix was simple and looks much better :)

laanwj commented at 8:50 am on June 6, 2019: member

ACK Code change and result looks good to me, haven’t tested, we should probably try it out on a few OSes to be sure the setting doesn’t cause problems.

MarcoFalke added the label Needs gitian build on Jun 6, 2019

promag commented at 2:24 pm on June 6, 2019: member

That is a different part of the code. I can’t really see the difference, probably because the points are too close. It looks like this:

Thanks for testing, I’ve also checked it.

I think this is a nice improvement and it doesn’t take more CPU to be that noticeable (measured in macos with instruments with and without this change).

Other visual improvements are possible but this is fine as it is. ACK a89c808.

jonasschnelli commented at 2:42 pm on June 6, 2019: contributor

Tested on OSX with HiDPI (retina) screen and I can’t see any difference. I believe that using HiDPI on OSX sets antialiasing by default (unsure though).

JosuGZ force-pushed on Jun 6, 2019

promag commented at 5:01 pm on June 6, 2019: member

ACK 4211bdb.

hebasto commented at 11:48 am on June 7, 2019: member

https://doc.qt.io/qt-5/qpainter.html#rendering-quality:

The QPainter class also provides a means of controlling the rendering quality through its RenderHint enum and the support for floating point precision: All the functions for drawing primitives has a floating point version. These are often used in combination with the QPainter::Antialiasing render hint.

Could using of floating point versions for drawing primitives (e.g., QLineF) be considered?

DrahtBot commented at 11:10 pm on June 8, 2019: member

Gitian builds for commit 5d2ccf0ce9ca1571c650a69319fb9c1e0b626ecb (master):

Gitian builds for commit 28af91115fb83010bf2675796bfd635766ecf537 (master and this pull):

DrahtBot removed the label Needs gitian build on Jun 8, 2019

promag commented at 10:58 pm on June 12, 2019: member

@hebasto changed coordinates to float but result is apparently the same, so I don’t think it’s worth the change. @JosuGZ I still think you should enable antialiasing only on L106 (I’ve tested it), this way the grid is always crisp - you can see on your screenshots that the horizontal lines are blurred and a bit darker.

laanwj commented at 1:01 pm on June 13, 2019: member

@JosuGZ I still think you should enable antialiasing only on L106 (I’ve tested it), this way the grid is always crisp - you can see on your screenshots that the horizontal lines are blurred and a bit darker.

Good catch, I think I agree, anti-aliasing improves diagonal lines but for axis-aligned lines (and the grid is that, by definition) it can make it look blurrier for no good reason.

fanquake added the label Waiting for author on Jun 14, 2019

laanwj commented at 6:18 pm on June 25, 2019: member

@JosuGZ at least please reply to the comments (even if you disagree)

JosuGZ commented at 2:15 pm on June 26, 2019: contributor

@JosuGZ at least please reply to the comments (even if you disagree)

Hi, I haven’t been on my Linux machine these days so I was waiting for that.

About horizontal and vertical lines: I think it is pointless to disable it as I don’t think performance should be an issue, and sometimes lines fall between 2 pixels (if using floats, which I recall it has been mentioned). I could do it though. Same for enabling antialiasing after filling the rect with black, this one at least does not add complexity.

When I’m on it I will review a little more and I will comment.

JosuGZ commented at 2:19 pm on June 26, 2019: contributor

Do you think horizontal lines look better without antialiasing?

fanquake removed the label Waiting for author on Jul 2, 2019

fanquake commented at 3:50 am on July 2, 2019: member

@JosuGZ Can you please squash your commits.

Add antialiasing to traffic graph widget db26e8e228

JosuGZ force-pushed on Jul 2, 2019

laanwj commented at 5:00 pm on July 2, 2019: member

ACK db26e8e22822c65a3817b16805f5ba9ad2235f93

laanwj merged this on Jul 2, 2019

laanwj closed this on Jul 2, 2019

laanwj referenced this in commit 4db2f8cf0f on Jul 2, 2019

promag commented at 5:03 pm on July 2, 2019: member

ACK db26e8e, could you update OP screenshots?

JosuGZ commented at 8:42 pm on July 2, 2019: contributor

ACK db26e8e, could you update OP screenshots?

Done!

Not sure if it’s useful now :P

promag commented at 8:47 pm on July 2, 2019: member

Thanks! it can be for forks :trollface:

luke-jr referenced this in commit 91805487f6 on Sep 3, 2019

jasonbcox referenced this in commit aa80b9c746 on Oct 8, 2020

barton2526 referenced this in commit 36835f2932 on May 26, 2021

kittywhiskers referenced this in commit 2cf1075854 on Nov 3, 2021

Munkybooty referenced this in commit 8bb20061dc on Nov 4, 2021

kittywhiskers referenced this in commit 61084394a2 on Nov 4, 2021

kittywhiskers referenced this in commit 6f98a32403 on Nov 11, 2021

Munkybooty referenced this in commit 7edc856d57 on Nov 12, 2021

Munkybooty referenced this in commit 0aac6acd4f on Nov 16, 2021

pravblockc referenced this in commit 0d74ae2ac4 on Nov 18, 2021

Munkybooty referenced this in commit bc29164aec on Nov 18, 2021

DrahtBot locked this on Dec 16, 2021

Qt: Add antialiasing to traffic graph widget #16153