Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qtcurve-qt4-1.8.14: poor rendering of "up" icon with 'V' style arrows enabled in Qt apps #3

Open
Coacher opened this issue Aug 5, 2013 · 21 comments

Comments

@Coacher
Copy link

Coacher commented Aug 5, 2013

Hello.

There is an issue in qtcurve-qt4 which makes "up" icon rendering be different from "down" icon and looking pretty bad. Here's the link with the picture illustrating this bug:

http://13p.imghost.us/GN/bug.png

As you can see "up" button icon is rendered assimetrically while "down" button icon is rendered fine. This happens in all Qt apps, this screenshot was made from qtcurve configuration tool. Theme used is default "Murrine" with enabled 'V' style arrows. GTK apps using gtk-engines-qtcurve are not affected: "up" button drawn properly there.

I am using gtk-engines-qtcurve-1.8.16, qtcurve-qt4-1.8.14 on Gentoo amd64 machine, my Qt is 4.8.5 and GTK+ is 2.24.17. If you need any other info I am ready to provide it.

@yuyichao
Copy link
Member

yuyichao commented Aug 5, 2013

Thank you very much for the bug reports (as well as QtCurve/qtcurve-gtk2#2). I will try my best to fix them. However, as you may know, I am not the original author. I am still reading the code myself and I can only spend a little time on it so it may take relatively a long time to fix. I'm sorry if this will cause any inconvenience to you. Meanwhile, any bug report and, especially, patches are REALLY welcome.

@Coacher
Copy link
Author

Coacher commented Aug 5, 2013

I see. I'd contacted Craig before and he said that he doesn't actively develop qtcurve anymore. I understand that you are most probably working on qtcurve in your free time, so no problem if fixing will take a while.

Unfourtunately, I don't have much free time myself and even worse my knowledge of C++ is low and GTK and Qt are completely unknown to me. I'd be glad to help you with patches, but it is impossible right now. Maybe I'll be more of use in autumn when I hope I'll have more spare time.

@tehnick
Copy link
Contributor

tehnick commented Aug 5, 2013

However, as you may know, I am not the original author.

Would you like to become the upstream developer of this project officially? Craig said many times that he stopped developing it. I think he will be happy if someone continue the work. We may ask him about this.

@yuyichao
Copy link
Member

yuyichao commented Aug 5, 2013

@Coacher Can you give bf0d2dc a try. It seems to fix this issue here at least. According to the comment, antialiasing was disabled probably because of the blur looking, however I personally cannot see any here. Please let me know if there is an unacceptable blur somewhere.

@tehnick I would love to and that's the reason I asked Craig for the original svn repos. He seems to be happy with that and here's the reply.

Hi,
Gald to see there's interest in keeping QtCurve alive :-)
When I get time, I will upload my local svn to googlecode.

At the same time, I am not that familiar with the code (even some options actually) yet and I hope putting the repo on github can make it easier for people to contribute.

yuyichao added a commit that referenced this issue Aug 5, 2013
@Coacher
Copy link
Author

Coacher commented Aug 5, 2013

@yuyichao going to test it soon. Turns out git has deviated from the last release quite much so the patch won't just apply as is (aside from style/ -> qt4/ migration which I've taken into account).

@yuyichao
Copy link
Member

yuyichao commented Aug 5, 2013

Try this

From 2e8e46475818e5aa74ab06fba58c1447fa99d77a Mon Sep 17 00:00:00 2001
From: Yichao Yu <[email protected]>
Date: Mon, 5 Aug 2013 08:54:37 -0400
Subject: [PATCH] [draw] try fixing
https://github.com/QtCurve/qtcurve-qt4/issues/3 with antialiasing

Conflicts:
style/qtcurve.cpp
---
style/qtcurve.cpp | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/style/qtcurve.cpp b/style/qtcurve.cpp
index 30ce021..2bc7c6b 100644
--- a/style/qtcurve.cpp
+++ b/style/qtcurve.cpp
@@ -1465,8 +1465,7 @@ void Style::polish(QApplication *app)
else if("Kde4ToolkitLibrary"==appName)
theThemedApp=APP_OPERA;

-    if(NULL!=getenv("QTCURVE_DEBUG"))
-    {
+    if (NULL != getenv("QTCURVE_DEBUG")) {
QByteArray l1(appName.toLatin1());
std::cout << "QtCurve: Application name: "" << l1.constData() << ""n";
}
@@ -12492,14 +12491,15 @@ void Style::drawArrow(QPainter *p, const QRect &rx, PrimitiveElement pe, QColor
a.translate((r.x()+(r.width()>>1)), (r.y()+(r.height()>>1)));

#ifdef QTC_OLD_NVIDIA_ARROW_FIX
-    path.moveTo(a[0].x()+0.5, a[0].y()+0.5);
-    for(int i=1; i<a.size(); ++i)
-        path.lineTo(a[i].x()+0.5, a[i].y()+0.5);
-    path.lineTo(a[0].x()+0.5, a[0].y()+0.5);
-#endif
-    // This all looks like overkill - but seems to fix issues with plasma and nvidia
-    // Just using 'aa' and drawing the arrows would be fine - but this makes them look
-    // slightly blurry, and I dont like that.
+    path.moveTo(a[0].x() + 0.5, a[0].y() + 0.5);
+    for(int i = 1;i < a.size();i++)
+        path.lineTo(a[i].x() + 0.5, a[i].y() + 0.5);
+    path.lineTo(a[0].x() + 0.5, a[0].y() + 0.5);
+#endif
+    // This all looks like overkill - but seems to fix issues with plasma and
+    // nvidia. Just using 'aa' and drawing the arrows would be fine -
+    // but this makes them look slightly blurry, and I dont like that.
+    //   -- Craig
p->save();
col.setAlpha(255);
#ifdef QTC_OLD_NVIDIA_ARROW_FIX
@@ -12510,7 +12510,12 @@ void Style::drawArrow(QPainter *p, const QRect &rx, PrimitiveElement pe, QColor
#ifdef QTC_OLD_NVIDIA_ARROW_FIX
p->fillPath(path, col);
#endif
-    p->setRenderHint(QPainter::Antialiasing, false);
+    // Disabling antialiasing here seems to cause weird looking up arrow here,
+    // and I personally don't see a blurry arrow with antialiasing enabled.
+    // See https://github.com/QtCurve/qtcurve-qt4/issues/3.
+    //   -- Yichao Yu
+    // p->setRenderHint(QPainter::Antialiasing, false);
+    p->setRenderHint(QPainter::Antialiasing, true);
p->drawPolygon(a);
p->restore();
}
--
1.8.3.4

@Coacher
Copy link
Author

Coacher commented Aug 5, 2013

Oh thanks, but I've simply did minor changes to your commit mentioned above and it helped, kind of. The arrows are indeed became blurry near their 'tails', but they are all symmetric now. I must say I liked previous non-antialisaed look more. :(

Here's the example of how it is rendered now: http://13p.imghost.us/HP3/test.png

@tehnick
Copy link
Contributor

tehnick commented Aug 5, 2013

When I get time, I will upload my local svn to googlecode.

There is no need in separate svn repo now. He may join to this project on GitHub.

I hope putting the repo on github can make it easier for people to contribute.

Exactly. This simplifies cooperative work a lot.

@yuyichao
Copy link
Member

yuyichao commented Aug 5, 2013

@tehnick That email was sent before he published the commit history and all these repos are svn2git'd from his svn repos @ googlecode.

@yuyichao
Copy link
Member

yuyichao commented Aug 5, 2013

@Coacher I will probably leave it like this for now. I'm not sure why only the up arrow was asymmetric but at least this seems to be the right function to solve the problem. Probably keep this issue open before a better solution is found.

@tehnick
Copy link
Contributor

tehnick commented Aug 5, 2013

That email was sent before he published the commit history and all these repos are svn2git'd from his svn repos @ googlecode.

Ok. Thanks for an explanation.

@Coacher
Copy link
Author

Coacher commented Aug 6, 2013

@yuyichao ok, fair enough.

@Coacher
Copy link
Author

Coacher commented Sep 15, 2013

Since this issue has appeared after upgrade to Qt-4.8.5 if I remember correctly, my guess that it has something to do with Qt itself rather than with qtcurve. After some search I found these bugreports which could be related:
https://bugreports.qt-project.org/browse/QTBUG-26013
https://bugreports.qt-project.org/browse/QTBUG-27053
https://bugreports.qt-project.org/browse/QTBUG-31579

@yuyichao
Copy link
Member

@Coacher Thank you very much. The patch for the third bug (which is the only one that is not released yet) is actually already merged in the official ArchLinux package but the shape of the v arrow still looks terrible. A quick test of the example in the bug report shows the following result.
Test Result
in which the red and the white line looks shifted and I'm not sure if this is normal since I don't have a older version of Qt.

@yuyichao
Copy link
Member

I wrote a minimum script in pyqt to demonstrate how qtcurve draws the v-arrow

https://gist.github.com/yuyichao/753fc4b2e85fb27b31dc

Here is my result for qt5 (5.1.1),
qt5-version
and qt4 (4.8.5 with patch for QTBUG-31579)
qt4-version

Since the two version looks very different and the qt5 version looks terrible, I guess there is something wrong with qt.

It will be very helpful if you can check how it works with older version of Qt as a comparison, otherwise, I will just report the bug to Qt with my current results.

@yuyichao
Copy link
Member

Result on 4.8.2 on Debian Stable:
qt-4.8.2
Indeed look fine.

@yuyichao
Copy link
Member

Upstream bug report here.

@yuyichao
Copy link
Member

(keep open until fixed upstream).

@Coacher
Copy link
Author

Coacher commented Sep 27, 2013

Same results on Debian virtual machine. Voted for upstream bug. Thank you for reporting!

@yuyichao
Copy link
Member

Just an update, a Qt dev told me yesterday that they are busying with other stuff but will have a look at this issue in 1-2 weeks..
= = not sure if I should call this good news....

@yuyichao
Copy link
Member

Update:
I've just bisect'ed this bug and I've found the corresponding commit in Qt4 (which is between 4.8.4 and 4.8.5 so yes, 4.8.5 is indeed the first affected qt4 version). Although I'm still not sure what is the real cause of the problem, the commit diff looks very small so hopefully a Qt dev will be able to find and fix the problem in Qt4.

For Qt5, I haven't been able to find a good commit yet. 5.0.0 is bad (and is worse than any qt4.8 versions) and I am trying to go further. However, the build system was not really ready for those versions yet and there also seems to be some refactoring going on so I'm not sure if I will have any luck doing this.

More details in the Qt bug report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants