On Thu, Jul 17, 2014 at 10:09:03AM -0700, Alan Irwin wrote: > On 2014-07-17 11:21-0400 Hazen Babcock wrote: > > > On 7/11/2014 2:38 PM, Alan W. Irwin wrote: > >> On 2014-07-03 12:33+0930 Jonathan Woithe wrote: > >> > >>> Hi all > >>> > >>> I have a relatively simple Qt program which utilises the PlPlot Qt widget > >>> (QtExtWidget). QtExtWidget is a component of the main application window > >>> and as such is constructed during program startup, prior to the > >>> calling of > >>> QApplication::exec() and before any GUI elements have been made > >>> visible with > >>> show(). I have noticed that during program startup I will sometimes > >>> get the > >>> following warnings: > >>> > >>> QColor::setRgb: RGB parameters out of range > >>> > >>> This doesn't appear all the time and seems to be fairly sensitive to > >>> timing, > >>> making it difficult to debug. However, after much trial and error I have > >>> determined that the warning is triggered from the QtExtWidget > >>> constructor. > >>> Further exploration showed it was coming from QtPLWidget's constructor > >>> (from which QtExtWidget is derived). > >>> > >>> Looking at the source of QtPLWidget::QtPLWidget() in plqt.cpp, I > >>> noticed a > >>> call to QApplication::processEvents() which didn't seem right to me. > >>> Commenting this out resolves the warning. Importantly, the warning > >>> remains > >>> when any of the other Qt functions (such as resize(), QPen()) are > >>> commented > >>> out. > >>> > >>> The reason I'm suspicious of this call is that there is no guarantee that > >>> any event loop has even been initialised at the time the constructors are > >>> called. Certainly in my case the event loop isn't running because > >>> QApplication::exec() has not yet been called at the time of construction. > >>> Similarly, the widget tree is not visible at event creation time, so it > >>> seems reasonable that some parts of the GUI environment (such as QColor > >>> objects) might not have been allocated yet. At a fundamental level, I > >>> expect that the processEvents() call could be triggering a draw of the as > >>> yet incompletely constructed widget. I also recall reading in the > >>> past that > >>> draw actions cannot be triggered in constructors due to the incomplete > >>> nature of the widget being built. > >>> > >>> Regardless, I can't think of any reason for calling > >>> QApplication::processEvents() in the constructor of a Qt widget. Does > >>> anyone know why this call is being made in QtPLWidget's constructor? > >>> Based > >>> on tests here and the reasons given above I think it should be > >>> removed. The > >>> trivial patch to do this is included at the end of this email. > >>> > >>> What do others think? > >> > >> I don't understand Qt that well. However, my own tests with the > >> test_interactive target indicate your change is fine so I have > >> committed it (revision 13137) with appropriate commentary to the svn > >> trunk version (which will end up as our next release). > >> > >> @Hazen: I thought I better commit this before it got lost. > >> However, as our Qt expert feel free to revert this change, if you > >> prefer some different solution. > > > > I'm not sure. If it doesn't break anything I guess it is probably ok. It > > looks like this was added by Andrew Ross in commit #10306 for Alban Rochel. > > Thanks for digging out the relevant commit. Since you are not > completely sure, I followed up further with > > svn diff --revision 10305:10306 bindings/qt_gui/plqt.cpp |less > > to confirm that the above call to QApplication::processEvents() was > part of a much more substantial change which probably nobody but Alban > understood completely, and which may well have been trying that call > experimentally (since Alban, who is no longer contributing code to > PLplot, was an extremely fast developer who > generally relied on us to test his changes). > > Here is the relevant commit message: > > ________________ > ANR for Alban Rochel - fixes for qt driver to support > > - command line arguments for the qtext driver > - flushing (as used in example 17) > ________________ > > It turns out that for a number of reasons the > test_interactive target does not test either of those capabilities. > So I have made the following tests of those capabilities by hand > (after running test_interactive to get all the required dependencies > built): > > examples/c++/qt_example -h > examples/c++/qt_example -bg 00FF00 > examples/c/x17c -dev qtwidget > > All appears to be well so combined with Jonathan Woithe's evidence, > this appears to reinforce the conclusion that removing that > QApplication::processEvents() call in revision 13137 was the right > thing to do.
I must admit I don't recall the details and your analysis seems sound, so go ahead and commit from me. Sorry I've not had time to contribute further. Andrew ------------------------------------------------------------------------------ Want fast and easy access to all the code in your enterprise? Index and search up to 200,000 lines of code with a free copy of Black Duck Code Sight - the same software that powers the world's largest code search on Ohloh, the Black Duck Open Hub! Try it now. http://p.sf.net/sfu/bds _______________________________________________ Plplot-devel mailing list Plplot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/plplot-devel
