anxb26 / fritzing

Automatically exported from code.google.com/p/fritzing
1 stars 1 forks source link

upgrade to qt 5 #2437

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.
2.
3.

Attach your sketch file and/or custom part files to the bug report. If the
sketch uses custom parts, save the sketch as shareable (under the file
menu).

What is the expected output? What do you see instead?

What version of Fritzing are you using? On what operating system?

Please use labels and text to provide additional information.

Original issue reported on code.google.com by irasc...@gmail.com on 21 Feb 2013 at 1:46

GoogleCodeExporter commented 9 years ago
Here is my patch:
https://github.com/davidperrenoud/fritzing/commit/94fcd0baa431906906163949b05776
4e82b5e722.patch

You can read it in a more pleasant way in its GitHub original format:
https://github.com/davidperrenoud/fritzing/commit/94fcd0baa431906906163949b05776
4e82b5e722

You have to install QHttp as described here: 
http://qt-project.org/forums/viewthread/24466 (don't forget to run syncqt.pl 
before running qmake)

Original comment by david.pe...@gmail.com on 1 Feb 2014 at 9:15

GoogleCodeExporter commented 9 years ago
Regressions:
* crashes like hell
* can't use backspace or delete key to delete a part
* form shows "♀ )gfnbmf*"

Original comment by david.pe...@gmail.com on 1 Feb 2014 at 3:00

GoogleCodeExporter commented 9 years ago
The most common crash happens when saving a sketch just after having moved a 
part with the mouse (it doesn't crash if the part is moved with arrow keys). I 
think something is wrong in FileProgressDialog.

Original comment by david.pe...@gmail.com on 1 Feb 2014 at 6:16

GoogleCodeExporter commented 9 years ago
This crash seems to be related to the deletion of FileProgressDialog and calls 
of processEvents(). Maybe this page can help, but not sure: 
http://blogs.kde.org/2009/03/26/how-crash-almost-every-qtkde-application-and-how
-fix-it-0

Original comment by david.pe...@gmail.com on 2 Feb 2014 at 12:57

GoogleCodeExporter commented 9 years ago
Thanks for your continued research. I had anticipated that there would be a 
period of testing required before the upgrade to qt5 would be ready. It's great 
that you have already identified some of the issues.

Original comment by irasc...@gmail.com on 5 Feb 2014 at 8:46

GoogleCodeExporter commented 9 years ago
Hi David, thanks for this nice first step! Would you be willing to continue 
this investigation? We can give you access to the repo and setup a branch. 

Original comment by andre.knoerig@gmail.com on 22 Feb 2014 at 11:28

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I'm experiencing the same regressions when using the patch, but it's loads 
better than dealing with the insane amount of latency present in the current 
version.

Original comment by petryk.s...@gmail.com on 23 Feb 2014 at 5:28

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
@André Yes, I will continue this investigation every now and then. To help me, 
could you ask Jonathan what the use of ProcessEventBlocker is? (if he's still 
working at IXDS)

Original comment by david.pe...@gmail.com on 25 Feb 2014 at 9:54

GoogleCodeExporter commented 9 years ago
Awesome, should I set up a git branch for that?
I'll see if Jonathan has an idea, he's on holiday at the moment though.

Original comment by andre.knoerig@gmail.com on 26 Feb 2014 at 8:51

GoogleCodeExporter commented 9 years ago
Comment from Jonathan:
"I think it had a larger role at one time; at the moment it seems limited to 
making sure that autosave isn't called during lengthy processes, such as 
autorouting. For the most ProcessEventBlocker::processEvents() calls 
QApplication::processEvents(), which is used to make sure that certain events 
get dealt with (like screen updates), when control is passed to something slow 
(like opening a sketch file)."

Original comment by andre.knoerig@gmail.com on 26 Feb 2014 at 9:58

GoogleCodeExporter commented 9 years ago
I have also had a go at getting it to build on Qt5. 

I have a git branch that builds/runs with both Qt4 and Qt5

https://bitbucket.org/rohanl/fritzing/branch/qt5

I don't see any crashes like you mention above.

However, I have a theory as to what is causing them

QtMsgHandler (qt4) and QtMessageHandler (qt5) have different signatures ( an 
extra context arg, and msg is QString instead of char *)

so the fMessageHandler method needs to be changed, otherwise at runtime you get 
a mismatch between args passed and expected. 

Original comment by rohan.ll...@gmail.com on 26 Apr 2014 at 10:31

GoogleCodeExporter commented 9 years ago
A couple of things I should mention:

 I took the easy way out and just conditionally excluded all the version check code for now to remove the dependency on QHttp

I have built/run on Mac only. So if there's some issue with FileProgressDialog 
on lInux, I wouldn't have seen it

Original comment by rohan.ll...@gmail.com on 26 Apr 2014 at 10:40

GoogleCodeExporter commented 9 years ago
Indeed, FileProgressDialog works fine with your version! (at least on Mac)

When I said "crashes like hell", I meant that I tried to use Fritzing Qt5 for 
production work and had to revert to Fritzing 0.8.7b to be productive. For 
example, both our versions sometimes crash when moving a part or right-clicking 
it. And instead of aligning on the grid on x=0 and x=1, parts align on x=0.5, 
x=1.5.

Also, your version seems to lose connections when moving parts in schematic 
view. I will try to compare our versions and send you a pull request.

Original comment by david.pe...@gmail.com on 27 Apr 2014 at 10:20

GoogleCodeExporter commented 9 years ago
I'm just a very beginner hobbyist with an itch to scratch... Given a choice 
between learning how to use Eagle, or something else, I liked the idea of 
Fritzing, and thought I could contribute.

So, I don't have any "production" work to try out. Are there any large non 
trivial sketches available on the web somewhere to use for more realistic 
testing?

Now that I've looked at it a bit more closely:

  * I see the problem with alignment (PCB vew only?) I will have a closer look at this, and see if I can get anywhere...

  * "Delete" doesn't work to delete parts. This looks like a Mac only issue with Qt5. See http://qt-project.org/forums/viewthread/36174

  * I haven't seen any lost connections, but as I said, I've only been using very simple sketches

  * When I change views, the Views menu ends up with multiple items "checked" with a dot. There is an existing issue about refactoring that code anyway to use native checked items (https://code.google.com/p/fritzing/issues/detail?id=2950)

  * Non ascii text gets corrupted (eg preferences->language, or rotate 90 <degree symbol>)

I haven't done a detailed diff of our two versions, but from a quick look at 
the diffs, the changes seem to be restricted to:

  * The QtMessageHandler code which I mentioned above. However, I notice that this is only installed on Windows, so it cannot explain any crashes on other platforms

  * I've made all changes compile on both Qt4/Qt5

  * Instead of including <QWidgets>, I've explicitly included the individual classes required.

There doesn't seem to be that much different between the 2.

Original comment by rohan.ll...@gmail.com on 27 Apr 2014 at 10:30

GoogleCodeExporter commented 9 years ago
Hi Rohan, Hi David.

Fritzing is hiring me back for a few days to look into the Qt 5 upgrade, along 
with a couple of high-priority bugs. I am tempted to start with Rohan's branch, 
since I like the idea of being able to build with either Qt 4 or Qt 5. I have 
been reading the comments above, but wondered whether you have any additional 
advice or suggestions.

Thanks.

- j

Original comment by irasc...@gmail.com on 6 May 2014 at 7:48

GoogleCodeExporter commented 9 years ago
I've added a "qt5" branch to the googlecode repository, which starts by pulling 
from Rohan's qt5 branch, then replaces QHttp with QNetworkAccessManager (and 
removes the conditional compilation flag NO_VERSION_CHECK).

So far I have only done the build under Linux 12.04/64 bit. In terms of the 
issues above: 

* crashes like hell
-- unable to replicate so far under linux. I have tested with one of the larger 
example files, but this still does not reach "production work" scale.

* can't use backspace or delete key to delete a part
-- unable to replicate under linux

* form shows "♀ )gfnbmf*"
-- not sure where this happens

* The most common crash happens when saving a sketch just after having moved a 
part with the mouse
-- unable to replicate under linux

* crash when moving a part or right-clicking it
-- unable to replicate under linux

* instead of aligning on the grid on x=0 and x=1, parts align on x=0.5, x=1.5.
-- Seems to happen in all 3 views, but the x and y offset is different. This is 
also true of the 4.8.6 build of the same source.

* seems to lose connections when moving parts in schematic view.
-- unable to replicate under linux

* When I change views, the Views menu ends up with multiple items "checked" 
with a dot
-- unable to replicate under linux

* Non ascii text gets corrupted (eg preferences->language)
-- I see it under preferences->language, but not with the 4.8 build of the same 
source. I don't see the degree-symbol mangling.

Original comment by irasc...@gmail.com on 7 May 2014 at 1:07

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
When I have time, I will try to make a video of all these bugs, log debugging 
outputs and post a test file. I can already say that it tested it on Mac with 
Qt 5.2.1. The Unicode problem ("♀ )gfnbmf*") could be seen on Generic female 
headers.

Original comment by david.pe...@gmail.com on 7 May 2014 at 1:29

GoogleCodeExporter commented 9 years ago
The preferences languaage menu is set up in a file called 
"translatorlistmodel.cpp". Somehow the source is treated differently between 
version 4.8 and 5.2.1, so that some of the characters are not found (UTF-8 vs. 
Latin1?). It may be that the translation files need to be regenerated, or those 
strings have to be translated (in the source) to UTF-16 like some of the other 
language names.

Original comment by irasc...@gmail.com on 7 May 2014 at 1:41

GoogleCodeExporter commented 9 years ago
@comment 20: I am still not clear: is the funky text a part label for a 
particular generic female header? What is the text you were expecting?  Thanks.

Original comment by irasc...@gmail.com on 7 May 2014 at 2:27

GoogleCodeExporter commented 9 years ago
Andre reported that align-to-grid is working fine on his mac build of the qt5 
branch, so I took another look. Oops! align-to-grid is working fine on my linux 
build as well: what aligns to the grid is not a corner of the selection area of 
the part--the alignment uses the connection point--the center--of one of the 
connectors on the part (usually connector 0, but usually all the connectors are 
aligned anyway). 

I am tempted to cross this off the list of problems: please let me know whether 
you are still getting buggy alignment behavior.

Original comment by irasc...@gmail.com on 7 May 2014 at 5:57

GoogleCodeExporter commented 9 years ago
QTimer works differently in Qt 5--the default timer behavior is less accurate 
than in Qt 4. I don't think any QTimer instances in Fritzing require the Qt4 
level of accuracy, but it needs to be looked into. 

Original comment by irasc...@gmail.com on 7 May 2014 at 6:00

GoogleCodeExporter commented 9 years ago
r416e94a992dc: made a few timers 'precise', leaving the rest to default. Could 
probably make a few of them 'coarse', but that's for a rainy day.

Original comment by irasc...@gmail.com on 7 May 2014 at 6:39

GoogleCodeExporter commented 9 years ago
I have only built/run on Mac (Mavericks) here are my findings

> * crashes like hell
> -- unable to replicate so far under linux. I have tested with one of the 
larger example files, but this still does not reach "production work" scale.

I too, couldn't reproduce, but have only tested with "trivial" sketches

> * can't use backspace or delete key to delete a part
> -- unable to replicate under linux

This is a problem with Qt5 on Mac. I have a fix for this. I haven't pushed it 
to my branch yet as my work area has a bunch of other things I've been 
experimenting with, but here is a diff. Note: I also took the opportunity to 
add a new Accelerator on Mac Option-Delete for "Delete Minus"

diff --git a/fritzing/src/mainwindow/mainwindow.cpp 
b/fritzing/src/mainwindow/mainwindow.cpp
index ba00490..78d5bbe 100644
--- a/fritzing/src/mainwindow/mainwindow.cpp
+++ b/fritzing/src/mainwindow/mainwindow.cpp
@@ -1340,6 +1340,28 @@ bool MainWindow::eventFilter(QObject *object, QEvent 
*event) {
                ProcessEventBlocker::processEvents();
        }

+#if defined(Q_OS_MAC) && (QT_VERSION >= 0x050000)
+    
+    // Need to process Backspace on Mac to workaround bug in Qt5
+    // See http://qt-project.org/forums/viewthread/36174
+    
+    if (event->type() == QEvent::KeyPress) {
+        QKeyEvent *keyEvent = static_cast<QKeyEvent *>(event);
+        
+        if (keyEvent->key() == Qt::Key_Backspace) {
+            Qt::KeyboardModifiers modifiers = keyEvent->modifiers();
+            if (modifiers == Qt::NoModifier) {
+                doDelete();
+                return true;
+            }
+            if (modifiers == Qt::AltModifier) {
+                doDeleteMinus();
+                return true;
+            }
+        }
+    }
+#endif
+
        return QMainWindow::eventFilter(object, event);
 }

diff --git a/fritzing/src/mainwindow/mainwindow_menu.cpp 
b/fritzing/src/mainwindow/mainwindow_menu.cpp
index 3b7c4d6..d1fe9b1 100644
--- a/fritzing/src/mainwindow/mainwindow_menu.cpp
+++ b/fritzing/src/mainwindow/mainwindow_menu.cpp
@@ -897,15 +897,18 @@ void MainWindow::createEditMenuActions() {
        m_deleteAct = new QAction(tr("&Delete"), this);
        m_deleteAct->setStatusTip(tr("Delete selection"));
        connect(m_deleteAct, SIGNAL(triggered()), this, SLOT(doDelete()));
-       #ifdef Q_OS_MAC
-               m_deleteAct->setShortcut(Qt::Key_Backspace);
-       #else
-               m_deleteAct->setShortcut(QKeySequence::Delete);
-       #endif
+#ifdef Q_OS_MAC
+    m_deleteAct->setShortcut(Qt::Key_Backspace);
+#else
+    m_deleteAct->setShortcut(QKeySequence::Delete);
+#endif

     m_deleteMinusAct = new QAction(tr("Delete Minus"), this);
        m_deleteMinusAct->setStatusTip(tr("Delete selection without attached wires"));
        connect(m_deleteMinusAct, SIGNAL(triggered()), this, SLOT(doDeleteMinus()));
+#ifdef Q_OS_MAC
+    m_deleteMinusAct->setShortcut(Qt::Key_Backspace | Qt::AltModifier);
+#endif

        m_deleteWireAct = new WireAction(m_deleteAct);
        m_deleteWireAct->setText(tr("&Delete Wire"));

> * The most common crash happens when saving a sketch just after having moved 
a part with the mouse
> -- unable to replicate under linux

I haven't seen - admittedly with very simple sketches

> * crash when moving a part or right-clicking it
> -- unable to replicate under linux

I haven't seen - admittedly with very simple sketches

> * instead of aligning on the grid on x=0 and x=1, parts align on x=0.5, x=1.5.
> -- Seems to happen in all 3 views, but the x and y offset is different. This 
is also true of the 4.8.6 build of the same source.

I was confused by this too. But once I understood that it aligns on the 
connectors, it looks ok

> * seems to lose connections when moving parts in schematic view.
> -- unable to replicate under linux

I haven't seen - admittedly with very simple sketches

> * When I change views, the Views menu ends up with multiple items "checked" 
with a dot
> -- unable to replicate under linux

Happens on Mac. See comment before about a patch to use native checked items 
anyway

> * Non ascii text gets corrupted (eg preferences->language)
> -- I see it under preferences->language, but not with the 4.8 build of the 
same source. I don't see the degree-symbol mangling.

This seems to behave differently under Qt5. String literals in the source are 
not in UTF-8. eg:

In Français, the source contains 0xE7, whereas the correct UTF-8 sequence is 
0xC3 0xA7 (octal: 303 247)

-        m_languages.insert("french", tr("French - %1").arg("Fran<E7>ais"));
+        m_languages.insert("french", tr("French - 
%1").arg(QString::fromUtf8("Fran\303\247ais")));

This could be fixed by explicitly using utf8 in the few cases it is needed like 
above. However, I think the "proper" fix is to fully embrace utf8 and use that 
encoding everywhere:

http://www.macieira.org/blog/2012/05/source-code-must-be-utf-8-and-qstring-wants
-it/

For qt4 compatability, this probably means calling:

QTextCodec::setCodecForCStrings(QTextCodec::codecForName("UTF-8"))
QTextCodec::codecForTr(QTextCodec::codecForName("UTF-8"))

Original comment by rohan.ll...@gmail.com on 7 May 2014 at 11:01

GoogleCodeExporter commented 9 years ago
Just one comment on the crashes... I did see one reproducible crash.

* Add a generic IC
* in Inspector, select the number of pins, and get big menu
* go to an entry in menu that is over anotehr part in the bin
* double click on menu

I accidentally did this one, and found I could reproduce a crash. It crashes 
because the view is NULL somewhere (sorry this is from memory, don't have a 
stack trace handy) in code that is called from a timer

I think what's happening is that the first click is selecting the number of 
pins, then the second click is changing the selection.

Once the timer fires, it is using "global" state that has changed since the 
time the action was done.

If Qt5 timers are not as accurate, this could explain why it is happening more 
often.

I don't quite understand the need for a timer, I;m guessing it's to push the 
processing off the GUI thread. 

I think either:

  * The timer object should essentially be a closure that contains ALL state that will be required later
  * ALL processing needs to be done via timers, such that the second click doesn't change anything before the first action is processed

Anyway, as I said I really don't understand the threading model, so ignore if 
this doesn't make sense.

Original comment by rohan.ll...@gmail.com on 7 May 2014 at 11:13

GoogleCodeExporter commented 9 years ago
@comments 26 & 27: thanks Rohan, this is very helpful. I will add your patch 
for the delete key this weekend, and also the menu checkbox patch. Since my 
time is limited, I converted the preferences language names to UTF-16. I will 
also look into the QTimer crash, though I won't have time to generally 
re-architect that behavior as you suggest. 

Original comment by irasc...@gmail.com on 9 May 2014 at 5:19

GoogleCodeExporter commented 9 years ago
r549ddcc7ff28 and previous: Rohan's delete-key patch is in; did a simpler fix 
for the checkmarks. Was able to replicate the timer crash once in about 10 
tries, but didn't catch the culprit; will have to try again.

Original comment by irasc...@gmail.com on 9 May 2014 at 8:11

GoogleCodeExporter commented 9 years ago
Closing this, since we now released this version. Thanks to all who helped!
Please add individual issues for new bugs.

Original comment by andre.knoerig@gmail.com on 17 Jul 2014 at 2:40